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

@@ -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",
 		},

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
+}

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,
 }

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;
 }
 

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(),

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,
 		},