bugfix: when dkim-signing submitted messages, use the domain from the "message from header" instead of "smtp mail from"

dmarc verifiers will only accept a dkim signature if the domain the message From
header matches the domain of the signature (i.e. it is "aligned").

i hadn't run into this before and when testing because thunderbird sets the
"smtp mail from" to the same address as a custom "message from" header. but
other mail clients don't have to do that.

should fix issue #22

queue/queue.go

@@ -331,8 +331,13 @@ func Drop(ID int64, toDomain string, recipient string) (int, error) {
 	return n, nil
 }
 
+type ReadReaderAtCloser interface {
+	io.ReadCloser
+	io.ReaderAt
+}
+
 // OpenMessage opens a message present in the queue.
-func OpenMessage(id int64) (io.ReadCloser, error) {
+func OpenMessage(id int64) (ReadReaderAtCloser, error) {
 	qm := Msg{ID: id}
 	err := queueDB.Get(&qm)
 	if err != nil {

smtpserver/server.go

@@ -1737,19 +1737,18 @@ func (c *conn) submit(ctx context.Context, recvHdrFor func(string) string, msgWr
 	// todo future: in a pedantic mode, we can parse the headers, and return an error if rcpt is only in To or Cc header, and not in the non-empty Bcc header. indicates a client that doesn't blind those bcc's.
 
 	// Add DKIM signatures.
-	domain := c.mailFrom.IPDomain.Domain
-	confDom, ok := mox.Conf.Domain(domain)
+	confDom, ok := mox.Conf.Domain(msgFrom.Domain)
 	if !ok {
-		c.log.Error("domain disappeared", mlog.Field("domain", domain))
+		c.log.Error("domain disappeared", mlog.Field("domain", msgFrom.Domain))
 		xsmtpServerErrorf(codes{smtp.C451LocalErr, smtp.SeSys3Other0}, "internal error")
 	}
 
 	dkimConfig := confDom.DKIM
 	if len(dkimConfig.Sign) > 0 {
-		if canonical, err := mox.CanonicalLocalpart(c.mailFrom.Localpart, confDom); err != nil {
-			c.log.Errorx("determining canonical localpart for dkim signing", err, mlog.Field("localpart", c.mailFrom.Localpart))
-		} else if dkimHeaders, err := dkim.Sign(ctx, canonical, domain, dkimConfig, c.smtputf8, dataFile); err != nil {
-			c.log.Errorx("dkim sign for domain", err, mlog.Field("domain", domain))
+		if canonical, err := mox.CanonicalLocalpart(msgFrom.Localpart, confDom); err != nil {
+			c.log.Errorx("determining canonical localpart for dkim signing", err, mlog.Field("localpart", msgFrom.Localpart))
+		} else if dkimHeaders, err := dkim.Sign(ctx, canonical, msgFrom.Domain, dkimConfig, c.smtputf8, dataFile); err != nil {
+			c.log.Errorx("dkim sign for domain", err, mlog.Field("domain", msgFrom.Domain))
 			metricServerErrors.WithLabelValues("dkimsign").Inc()
 		} else {
 			msgPrefix = append(msgPrefix, []byte(dkimHeaders)...)

smtpserver/server_test.go

@@ -18,6 +18,7 @@ import (
 	"net"
 	"os"
 	"path/filepath"
+	"sort"
 	"strings"
 	"testing"
 	"time"
@@ -1003,3 +1004,96 @@ func TestCatchall(t *testing.T) {
 	tcheck(t, err, "checking delivered messages to catchall account")
 	tcompare(t, n, 1)
 }
+
+// Test DKIM signing for outgoing messages.
+func TestDKIMSign(t *testing.T) {
+	resolver := dns.MockResolver{
+		A: map[string][]string{
+			"mox.example.": {"127.0.0.10"}, // For mx check.
+		},
+		PTR: map[string][]string{
+			"127.0.0.10": {"mox.example."},
+		},
+	}
+
+	ts := newTestServer(t, "../testdata/smtp/mox.conf", resolver)
+	defer ts.close()
+
+	// Set DKIM signing config.
+	var gen byte
+	genDKIM := func(domain string) string {
+		dom, _ := mox.Conf.Domain(dns.Domain{ASCII: domain})
+
+		privkey := make([]byte, ed25519.SeedSize) // Fake key, don't use for real.
+		gen++
+		privkey[0] = byte(gen)
+
+		sel := config.Selector{
+			HashEffective:    "sha256",
+			HeadersEffective: []string{"From", "To", "Subject"},
+			Key:              ed25519.NewKeyFromSeed(privkey),
+			Domain:           dns.Domain{ASCII: "testsel"},
+		}
+		dom.DKIM = config.DKIM{
+			Selectors: map[string]config.Selector{"testsel": sel},
+			Sign:      []string{"testsel"},
+		}
+		mox.Conf.Dynamic.Domains[domain] = dom
+		pubkey := sel.Key.Public().(ed25519.PublicKey)
+		return "v=DKIM1;k=ed25519;p=" + base64.StdEncoding.EncodeToString(pubkey)
+	}
+
+	dkimtxt := genDKIM("mox.example")
+	dkimtxt2 := genDKIM("mox2.example")
+
+	// DKIM verify needs to find the key.
+	resolver.TXT = map[string][]string{
+		"testsel._domainkey.mox.example.":  {dkimtxt},
+		"testsel._domainkey.mox2.example.": {dkimtxt2},
+	}
+
+	ts.submission = true
+	ts.user = "mjl@mox.example"
+	ts.pass = "testtest"
+
+	n := 0
+	testSubmit := func(mailFrom, msgFrom string) {
+		t.Helper()
+		ts.run(func(err error, client *smtpclient.Client) {
+			t.Helper()
+
+			msg := strings.ReplaceAll(fmt.Sprintf(`From: <%s>
+To: <remote@example.org>
+Subject: test
+Message-Id: <test@mox.example>
+
+test email
+`, msgFrom), "\n", "\r\n")
+
+			rcptTo := "remote@example.org"
+			if err == nil {
+				err = client.Deliver(context.Background(), mailFrom, rcptTo, int64(len(msg)), strings.NewReader(msg), false, false)
+			}
+			tcheck(t, err, "deliver")
+
+			msgs, err := queue.List()
+			tcheck(t, err, "listing queue")
+			n++
+			tcompare(t, len(msgs), n)
+			sort.Slice(msgs, func(i, j int) bool {
+				return msgs[i].ID > msgs[j].ID
+			})
+			f, err := queue.OpenMessage(msgs[0].ID)
+			tcheck(t, err, "open message in queue")
+			defer f.Close()
+			results, err := dkim.Verify(context.Background(), resolver, false, dkim.DefaultPolicy, f, false)
+			tcheck(t, err, "verifying dkim message")
+			tcompare(t, len(results), 1)
+			tcompare(t, results[0].Status, dkim.StatusPass)
+			tcompare(t, results[0].Sig.Domain.ASCII, strings.Split(msgFrom, "@")[1])
+		})
+	}
+
+	testSubmit("mjl@mox.example", "mjl@mox.example")
+	testSubmit("mjl@mox.example", "mjl@mox2.example") // DKIM signature will be for mox2.example.
+}

testdata/smtp/domains.conf

@@ -1,10 +1,12 @@
 Domains:
 	mox.example: nil
+	mox2.example: nil
 Accounts:
 	mjl:
 		Domain: mox.example
 		Destinations:
 			mjl@mox.example: nil
+			mjl@mox2.example: nil
 		JunkFilter:
 			Threshold: 0.9
 			Params: