git » chasquid » commit 55b03c8

queue: Use a local envelope-from when forwarding

author Alberto Bertogli
2016-10-02 23:49:32 UTC
committer Alberto Bertogli
2016-10-09 23:51:05 UTC
parent afd79dfd8db835e5ff879f11cad0c0edd9c02a04

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 +2 -2
internal/queue/queue.go +30 -3
internal/queue/queue.pb.go +33 -25
internal/queue/queue.proto +7 -0
internal/queue/queue_test.go +10 -10
test/t-03-queue_persistency/addtoqueue.go +1 -1

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