From 55b03c8cf083b410143a573c1ca23e92a33af055 Mon Sep 17 00:00:00 2001 From: Alberto Bertogli Date: Mon, 3 Oct 2016 00:49:32 +0100 Subject: [PATCH] queue: Use a local envelope-from when forwarding If there's an alias to forward email to a non-local domain, using the original From is problematic, as we may not be an authorized sender for it. Some MTAs (like Exim) will do it anyway, others (like gmail) will construct a special address based on the original address. This patch implements the latter approach, which is safer and allows the receiver to properly enforce SPF. We construct a (hopefully) reasonable From based on the local user, and embedding the original From (but transformed for IDNA, as the receiver may not support SMTPUTF8). --- internal/queue/dsn_test.go | 4 +- internal/queue/queue.go | 33 +++++++++++-- internal/queue/queue.pb.go | 58 +++++++++++++---------- internal/queue/queue.proto | 7 +++ internal/queue/queue_test.go | 20 ++++---- test/t-03-queue_persistency/addtoqueue.go | 2 +- 6 files changed, 83 insertions(+), 41 deletions(-) diff --git a/internal/queue/dsn_test.go b/internal/queue/dsn_test.go index 8c3df7e..3e6e625 100644 --- a/internal/queue/dsn_test.go +++ b/internal/queue/dsn_test.go @@ -10,9 +10,9 @@ func TestDSN(t *testing.T) { To: []string{"toto@africa.org", "negra@sosa.org"}, Rcpt: []*Recipient{ {"poe@rcpt", Recipient_EMAIL, Recipient_FAILED, - "oh! horror!"}, + "oh! horror!", ""}, {"newman@rcpt", Recipient_EMAIL, Recipient_FAILED, - "oh! the humanity!"}}, + "oh! the humanity!", ""}}, Data: []byte("data ñaca"), Hostname: "from.org", }, diff --git a/internal/queue/queue.go b/internal/queue/queue.go index 68d534c..1e531c6 100644 --- a/internal/queue/queue.go +++ b/internal/queue/queue.go @@ -28,6 +28,7 @@ import ( "github.com/golang/glog" "github.com/golang/protobuf/ptypes" "github.com/golang/protobuf/ptypes/timestamp" + "golang.org/x/net/idna" "golang.org/x/net/trace" ) @@ -170,8 +171,9 @@ func (q *Queue) Put(hostname, from string, to []string, data []byte) (string, er // not very pretty but at least it's self contained. for _, aliasRcpt := range rcpts { r := &Recipient{ - Address: aliasRcpt.Addr, - Status: Recipient_PENDING, + Address: aliasRcpt.Addr, + Status: Recipient_PENDING, + OriginalAddress: t, } switch aliasRcpt.Type { case aliases.EMAIL: @@ -397,7 +399,24 @@ func (item *Item) deliver(q *Queue, rcpt *Recipient) (err error, permanent bool) if envelope.DomainIn(rcpt.Address, q.localDomains) { return q.localC.Deliver(item.From, rcpt.Address, item.Data) } else { - return q.remoteC.Deliver(item.From, rcpt.Address, item.Data) + from := item.From + if !envelope.DomainIn(item.From, q.localDomains) { + // We're sending from a non-local to a non-local. This should + // happen only when there's an alias to forward email to a + // non-local domain. In this case, using the original From is + // problematic, as we may not be an authorized sender for this. + // Some MTAs (like Exim) will do it anyway, others (like + // gmail) will construct a special address based on the + // original address. We go with the latter. + // Note this assumes "+" is an alias suffix separator. + // We use the IDNA version of the domain if possible, because + // we can't know if the other side will support SMTPUTF8. + from = fmt.Sprintf("%s+fwd_from=%s@%s", + envelope.UserOf(rcpt.OriginalAddress), + strings.Replace(from, "@", "=", -1), + mustIDNAToASCII(envelope.DomainOf(rcpt.OriginalAddress))) + } + return q.remoteC.Deliver(from, rcpt.Address, item.Data) } } } @@ -420,3 +439,11 @@ func timestampNow() *timestamp.Timestamp { ts, _ := ptypes.TimestampProto(now) return ts } + +func mustIDNAToASCII(s string) string { + a, err := idna.ToASCII(s) + if err != nil { + return a + } + return s +} diff --git a/internal/queue/queue.pb.go b/internal/queue/queue.pb.go index 9140e56..a4e47d9 100644 --- a/internal/queue/queue.pb.go +++ b/internal/queue/queue.pb.go @@ -110,10 +110,16 @@ func (m *Message) GetCreatedAtTs() *google_protobuf.Timestamp { } type Recipient struct { + // Address to send the message to. + // This is the final one, after expanding aliases. Address string `protobuf:"bytes,1,opt,name=address" json:"address,omitempty"` Type Recipient_Type `protobuf:"varint,2,opt,name=type,enum=queue.Recipient_Type" json:"type,omitempty"` Status Recipient_Status `protobuf:"varint,3,opt,name=status,enum=queue.Recipient_Status" json:"status,omitempty"` LastFailureMessage string `protobuf:"bytes,4,opt,name=last_failure_message,json=lastFailureMessage" json:"last_failure_message,omitempty"` + // Address that this recipient was originally intended to. + // This is before expanding aliases and only used in very particular + // cases. + OriginalAddress string `protobuf:"bytes,5,opt,name=original_address,json=originalAddress" json:"original_address,omitempty"` } func (m *Recipient) Reset() { *m = Recipient{} } @@ -131,29 +137,31 @@ func init() { func init() { proto.RegisterFile("queue.proto", fileDescriptor0) } var fileDescriptor0 = []byte{ - // 382 bytes of a gzipped FileDescriptorProto - 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x09, 0x6e, 0x88, 0x02, 0xff, 0x64, 0x50, 0x41, 0xcb, 0x9b, 0x40, - 0x10, 0xad, 0x7e, 0x46, 0x93, 0xb1, 0x0d, 0xb2, 0xb4, 0x74, 0x49, 0x2f, 0x41, 0x7a, 0x68, 0x29, - 0x68, 0x49, 0x8f, 0x85, 0x42, 0x40, 0x53, 0x84, 0x26, 0x04, 0xe3, 0x5d, 0x36, 0xba, 0x31, 0x82, - 0x66, 0xad, 0xbb, 0x1e, 0xfa, 0x3b, 0xfb, 0x7b, 0x0a, 0xdd, 0x5d, 0x4d, 0x0a, 0xfd, 0x6e, 0x33, - 0xf3, 0xde, 0xcc, 0xbc, 0xf7, 0xc0, 0xfd, 0x39, 0xd0, 0x81, 0x06, 0x5d, 0xcf, 0x04, 0x43, 0x33, - 0xdd, 0xac, 0xbe, 0x56, 0xb5, 0xb8, 0x0e, 0xe7, 0xa0, 0x60, 0x6d, 0x58, 0xb1, 0x86, 0xdc, 0xaa, - 0x50, 0xe3, 0xe7, 0xe1, 0x12, 0x76, 0xe2, 0x57, 0x47, 0x79, 0x28, 0xea, 0x96, 0x72, 0x41, 0xda, - 0xee, 0x5f, 0x35, 0xde, 0xf0, 0x7f, 0x1b, 0xe0, 0xec, 0x29, 0xe7, 0xa4, 0xa2, 0x68, 0x09, 0x66, - 0x12, 0x61, 0x63, 0x6d, 0x7c, 0x58, 0xa4, 0x66, 0x1d, 0x21, 0x04, 0xd6, 0xa5, 0x67, 0x2d, 0x36, - 0xf5, 0x44, 0xd7, 0x8a, 0x93, 0x31, 0xfc, 0xb4, 0x7e, 0x52, 0x1c, 0xa9, 0xe1, 0x3d, 0x58, 0x7d, - 0xd1, 0x09, 0x6c, 0xc9, 0x89, 0xbb, 0xf1, 0x82, 0x51, 0x5f, 0x4a, 0x8b, 0xba, 0xab, 0xe9, 0x4d, - 0xa4, 0x1a, 0x55, 0x97, 0x4a, 0x22, 0x08, 0x9e, 0xc9, 0x4b, 0x2f, 0x53, 0x5d, 0xa3, 0x6f, 0xf0, - 0xaa, 0xe8, 0x29, 0x11, 0xb4, 0xcc, 0x89, 0xc8, 0x05, 0xc7, 0xb6, 0x04, 0xdd, 0xcd, 0x2a, 0xa8, - 0x18, 0xab, 0x9a, 0xc9, 0xa3, 0xf4, 0x10, 0x64, 0x77, 0xc9, 0xa9, 0x3b, 0x2d, 0x6c, 0x45, 0xc6, - 0xd1, 0x0a, 0xe6, 0x57, 0xc6, 0xc5, 0x8d, 0xb4, 0x14, 0x3b, 0x5a, 0xe1, 0xa3, 0xf7, 0xff, 0x18, - 0xb0, 0x78, 0x68, 0x40, 0x18, 0x1c, 0x52, 0x96, 0xbd, 0x74, 0x39, 0x99, 0xbb, 0xb7, 0xe8, 0x23, - 0x58, 0x2a, 0x20, 0xed, 0x70, 0xb9, 0x79, 0xf3, 0xbf, 0xfa, 0x20, 0x93, 0x60, 0xaa, 0x29, 0x28, - 0x04, 0x5b, 0x8a, 0x10, 0x03, 0x97, 0xe6, 0x15, 0xf9, 0xed, 0x33, 0xf2, 0x49, 0xc3, 0xe9, 0x44, - 0x43, 0x9f, 0xe1, 0x75, 0x43, 0xb8, 0xc8, 0x2f, 0xa4, 0x6e, 0x86, 0x9e, 0xe6, 0xed, 0x98, 0xb2, - 0x4c, 0x4a, 0x49, 0x40, 0x0a, 0xdb, 0x8d, 0xd0, 0x94, 0xbf, 0xff, 0x0e, 0x2c, 0xf5, 0x10, 0x2d, - 0x60, 0x16, 0xef, 0xb7, 0xc9, 0x0f, 0xef, 0x05, 0x9a, 0x83, 0x75, 0x4c, 0x8e, 0xb1, 0x67, 0xf8, - 0x9f, 0xc0, 0x1e, 0x1f, 0x20, 0x17, 0x9c, 0x63, 0x7c, 0x88, 0x92, 0xc3, 0xf7, 0x91, 0x70, 0x8a, - 0x0f, 0x99, 0x67, 0x20, 0x00, 0x7b, 0x27, 0x97, 0xe2, 0xc8, 0x33, 0xcf, 0xb6, 0x0e, 0xef, 0xcb, - 0xdf, 0x00, 0x00, 0x00, 0xff, 0xff, 0xe7, 0x32, 0x94, 0x20, 0x2f, 0x02, 0x00, 0x00, + // 403 bytes of a gzipped FileDescriptorProto + 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x09, 0x6e, 0x88, 0x02, 0xff, 0x64, 0x90, 0x51, 0x8b, 0x9c, 0x30, + 0x14, 0x85, 0xab, 0xeb, 0x38, 0x3b, 0xd7, 0x76, 0x2b, 0xa1, 0xa5, 0x61, 0xfa, 0xb2, 0x48, 0x1f, + 0xba, 0x14, 0xb4, 0x4c, 0x1f, 0x0b, 0x85, 0x01, 0xdd, 0x22, 0x74, 0x87, 0xc1, 0xf5, 0x5d, 0x32, + 0x9a, 0x71, 0x03, 0x6a, 0xac, 0x89, 0x0f, 0xfd, 0x47, 0xfd, 0x3f, 0xfd, 0x43, 0x4d, 0xa2, 0x4e, + 0xa1, 0xfb, 0x96, 0x7b, 0xcf, 0xc9, 0xe5, 0x3b, 0x07, 0xbc, 0x9f, 0x23, 0x1d, 0x69, 0xd8, 0x0f, + 0x5c, 0x72, 0xb4, 0x32, 0xc3, 0xf6, 0x6b, 0xcd, 0xe4, 0xd3, 0x78, 0x0a, 0x4b, 0xde, 0x46, 0x35, + 0x6f, 0x48, 0x57, 0x47, 0x46, 0x3f, 0x8d, 0xe7, 0xa8, 0x97, 0xbf, 0x7a, 0x2a, 0x22, 0xc9, 0x5a, + 0x2a, 0x24, 0x69, 0xfb, 0x7f, 0xaf, 0xe9, 0x46, 0xf0, 0xc7, 0x82, 0xf5, 0x03, 0x15, 0x82, 0xd4, + 0x14, 0xdd, 0x80, 0x9d, 0xc6, 0xd8, 0xba, 0xb5, 0x3e, 0x6e, 0x32, 0x9b, 0xc5, 0x08, 0x81, 0x73, + 0x1e, 0x78, 0x8b, 0x6d, 0xb3, 0x31, 0x6f, 0xed, 0xc9, 0x39, 0xbe, 0xba, 0xbd, 0xd2, 0x1e, 0xc5, + 0xf0, 0x01, 0x9c, 0xa1, 0xec, 0x25, 0x76, 0xd4, 0xc6, 0xdb, 0xf9, 0xe1, 0xc4, 0x97, 0xd1, 0x92, + 0xf5, 0x8c, 0x76, 0x32, 0x33, 0xaa, 0xbe, 0x54, 0x11, 0x49, 0xf0, 0x4a, 0x5d, 0x7a, 0x99, 0x99, + 0x37, 0xfa, 0x06, 0xaf, 0xca, 0x81, 0x12, 0x49, 0xab, 0x82, 0xc8, 0x42, 0x0a, 0xec, 0x2a, 0xd1, + 0xdb, 0x6d, 0xc3, 0x9a, 0xf3, 0xba, 0x99, 0x33, 0xaa, 0x0c, 0x61, 0xbe, 0x20, 0x67, 0xde, 0xfc, + 0x61, 0x2f, 0x73, 0x81, 0xb6, 0x70, 0xfd, 0xc4, 0x85, 0xec, 0x48, 0x4b, 0xf1, 0xda, 0x10, 0x5e, + 0xe6, 0xe0, 0xb7, 0x0d, 0x9b, 0x0b, 0x03, 0xc2, 0xb0, 0x26, 0x55, 0x35, 0xa8, 0x94, 0x73, 0xb8, + 0x65, 0x44, 0x77, 0xe0, 0xe8, 0x82, 0x4c, 0xc2, 0x9b, 0xdd, 0xdb, 0xff, 0xe9, 0xc3, 0x5c, 0x89, + 0x99, 0xb1, 0xa0, 0x08, 0x5c, 0x05, 0x21, 0x47, 0xa1, 0xc2, 0x6b, 0xf3, 0xbb, 0x67, 0xe6, 0x47, + 0x23, 0x67, 0xb3, 0x0d, 0x7d, 0x86, 0x37, 0x0d, 0x11, 0xb2, 0x38, 0x13, 0xd6, 0x8c, 0x03, 0x2d, + 0xda, 0xa9, 0x65, 0xd5, 0x94, 0x46, 0x40, 0x5a, 0xbb, 0x9f, 0xa4, 0xa5, 0xff, 0x3b, 0xf0, 0xf9, + 0xc0, 0x6a, 0xd6, 0x91, 0xa6, 0x58, 0x80, 0x57, 0xc6, 0xfd, 0x7a, 0xd9, 0xef, 0xa7, 0x75, 0xf0, + 0x1e, 0x1c, 0xcd, 0x86, 0x36, 0xb0, 0x4a, 0x1e, 0xf6, 0xe9, 0x0f, 0xff, 0x05, 0xba, 0x06, 0xe7, + 0x98, 0x1e, 0x13, 0xdf, 0x0a, 0x3e, 0x81, 0x3b, 0xb1, 0x20, 0x0f, 0xd6, 0xc7, 0xe4, 0x10, 0xa7, + 0x87, 0xef, 0x93, 0xe1, 0x31, 0x39, 0xe4, 0xbe, 0x85, 0x00, 0xdc, 0x7b, 0xf5, 0x29, 0x89, 0x7d, + 0xfb, 0xe4, 0x9a, 0x9e, 0xbf, 0xfc, 0x0d, 0x00, 0x00, 0xff, 0xff, 0x8f, 0x29, 0x0e, 0xd0, 0x5a, + 0x02, 0x00, 0x00, } diff --git a/internal/queue/queue.proto b/internal/queue/queue.proto index f821e62..2d2d514 100644 --- a/internal/queue/queue.proto +++ b/internal/queue/queue.proto @@ -25,6 +25,8 @@ message Message { } message Recipient { + // Address to send the message to. + // This is the final one, after expanding aliases. string address = 1; enum Type { @@ -41,5 +43,10 @@ message Recipient { Status status = 3; string last_failure_message = 4; + + // Address that this recipient was originally intended to. + // This is before expanding aliases and only used in very particular + // cases. + string original_address = 5; } diff --git a/internal/queue/queue_test.go b/internal/queue/queue_test.go index 5e8262e..11d6ed5 100644 --- a/internal/queue/queue_test.go +++ b/internal/queue/queue_test.go @@ -111,7 +111,7 @@ func TestFullQueue(t *testing.T) { ID: <-newID, From: fmt.Sprintf("from-%d", i), Rcpt: []*Recipient{ - {"to", Recipient_EMAIL, Recipient_PENDING, ""}}, + {"to", Recipient_EMAIL, Recipient_PENDING, "", ""}}, Data: []byte("data"), }, CreatedAt: time.Now(), @@ -163,14 +163,14 @@ func TestAliases(t *testing.T) { expected []*Recipient }{ {[]string{"ab@loco"}, []*Recipient{ - {"pq@loco", Recipient_EMAIL, Recipient_PENDING, ""}, - {"rs@loco", Recipient_EMAIL, Recipient_PENDING, ""}, - {"command", Recipient_PIPE, Recipient_PENDING, ""}}}, + {"pq@loco", Recipient_EMAIL, Recipient_PENDING, "", "ab@loco"}, + {"rs@loco", Recipient_EMAIL, Recipient_PENDING, "", "ab@loco"}, + {"command", Recipient_PIPE, Recipient_PENDING, "", "ab@loco"}}}, {[]string{"ab@loco", "cd@loco"}, []*Recipient{ - {"pq@loco", Recipient_EMAIL, Recipient_PENDING, ""}, - {"rs@loco", Recipient_EMAIL, Recipient_PENDING, ""}, - {"command", Recipient_PIPE, Recipient_PENDING, ""}, - {"ata@hualpa", Recipient_EMAIL, Recipient_PENDING, ""}}}, + {"pq@loco", Recipient_EMAIL, Recipient_PENDING, "", "ab@loco"}, + {"rs@loco", Recipient_EMAIL, Recipient_PENDING, "", "ab@loco"}, + {"command", Recipient_PIPE, Recipient_PENDING, "", "ab@loco"}, + {"ata@hualpa", Recipient_EMAIL, Recipient_PENDING, "", "cd@loco"}}}, } for _, c := range cases { id, err := q.Put("host", "from", c.to, []byte("data")) @@ -179,7 +179,7 @@ func TestAliases(t *testing.T) { } item := q.q[id] if !reflect.DeepEqual(item.Rcpt, c.expected) { - t.Errorf("case %q, expected %v, got %v", c.to, item.Rcpt, c.expected) + t.Errorf("case %q, expected %v, got %v", c.to, c.expected, item.Rcpt) } q.Remove(id) } @@ -193,7 +193,7 @@ func TestPipes(t *testing.T) { ID: <-newID, From: "from", Rcpt: []*Recipient{ - {"true", Recipient_PIPE, Recipient_PENDING, ""}}, + {"true", Recipient_PIPE, Recipient_PENDING, "", ""}}, Data: []byte("data"), }, CreatedAt: time.Now(), diff --git a/test/t-03-queue_persistency/addtoqueue.go b/test/t-03-queue_persistency/addtoqueue.go index 03b2aab..6c770ba 100644 --- a/test/t-03-queue_persistency/addtoqueue.go +++ b/test/t-03-queue_persistency/addtoqueue.go @@ -37,7 +37,7 @@ func main() { From: *from, To: []string{*rcpt}, Rcpt: []*queue.Recipient{ - {*rcpt, queue.Recipient_EMAIL, queue.Recipient_PENDING, ""}, + {*rcpt, queue.Recipient_EMAIL, queue.Recipient_PENDING, "", ""}, }, Data: data, },