modify/dkim: Do not refold the signature field

Closes #187.

go.mod

@@ -13,12 +13,12 @@ require (
 	github.com/emersion/go-imap-move v0.0.0-20190710073258-6e5a51a5b342
 	github.com/emersion/go-imap-specialuse v0.0.0-20161227184202-ba031ced6a62
 	github.com/emersion/go-imap-unselect v0.0.0-20171113212723-b985794e5f26
-	github.com/emersion/go-message v0.10.9-0.20191116124005-65fd0119e899
+	github.com/emersion/go-message v0.11.1-0.20191229143723-71005f4b31f3
 	github.com/emersion/go-msgauth v0.3.2-0.20191028231513-55b75676976c
 	github.com/emersion/go-sasl v0.0.0-20190817083125-240c8404624e
 	github.com/emersion/go-smtp v0.12.1-0.20191206174923-1f576e0ec85c
 	github.com/foxcpp/go-imap-sql v0.3.2-0.20191208094750-8b4ec6b19a78
-	github.com/foxcpp/go-mockdns v0.0.0-20191216195825-5eabd8dbfe1f
+	github.com/foxcpp/go-mockdns v0.0.0-20191226172053-3b5a6e57c8fe
 	github.com/foxcpp/go-mtasts v0.0.0-20191219193356-62bc3f1f74b8
 	github.com/go-sql-driver/mysql v1.4.1
 	github.com/google/uuid v1.1.1

go.sum

@@ -37,6 +37,8 @@ github.com/emersion/go-message v0.10.4-0.20190609165112-592ace5bc1ca h1:OYhqtJI4
 github.com/emersion/go-message v0.10.4-0.20190609165112-592ace5bc1ca/go.mod h1:3h+HsGTCFHmk4ngJ2IV/YPhdlaOcR6hcgqM3yca9v7c=
 github.com/emersion/go-message v0.10.9-0.20191116124005-65fd0119e899 h1:KQ2ms0F+BcPuPHSDayTbkpsUs+IdbJ4jArwJ8rhiCXQ=
 github.com/emersion/go-message v0.10.9-0.20191116124005-65fd0119e899/go.mod h1:C4jnca5HOTo4bGN9YdqNQM9sITuT3Y0K6bSUw9RklvY=
+github.com/emersion/go-message v0.11.1-0.20191229143723-71005f4b31f3 h1:sYu7DbHUvQ3+HOc2FcbarC2ppPfgm0SYt6P//8QYs3w=
+github.com/emersion/go-message v0.11.1-0.20191229143723-71005f4b31f3/go.mod h1:C4jnca5HOTo4bGN9YdqNQM9sITuT3Y0K6bSUw9RklvY=
 github.com/emersion/go-milter v0.0.0-20190311184326-c3095a41a6fe/go.mod h1:aEaq7U51ARlk+2UeXTtdrDYeYWAUn/QjEwWzs7lD8OU=
 github.com/emersion/go-msgauth v0.3.2-0.20191028231513-55b75676976c h1:0as6ct8PVWrlCofyWPh8o/fNRr6/DPbjkBicSG+3ZQI=
 github.com/emersion/go-msgauth v0.3.2-0.20191028231513-55b75676976c/go.mod h1:7r9HUSXL1dq+KK7Xqg0JlyBxNFGf5+JouRvSz4wBZCQ=
@@ -77,6 +79,8 @@ github.com/foxcpp/go-mockdns v0.0.0-20191211223108-4a6fcad1301d h1:XtbADe4vz/W1k
 github.com/foxcpp/go-mockdns v0.0.0-20191211223108-4a6fcad1301d/go.mod h1:tPg4cp4nseejPd+UKxtCVQ2hUxNTZ7qQZJa7CLriIeo=
 github.com/foxcpp/go-mockdns v0.0.0-20191216195825-5eabd8dbfe1f h1:b/CFmrdqIGU6eV774xeaQwd1VfgiLuR/8jiY3LyLiMc=
 github.com/foxcpp/go-mockdns v0.0.0-20191216195825-5eabd8dbfe1f/go.mod h1:tPg4cp4nseejPd+UKxtCVQ2hUxNTZ7qQZJa7CLriIeo=
+github.com/foxcpp/go-mockdns v0.0.0-20191226172053-3b5a6e57c8fe h1:vzxspt1t/cOPBbDoIfVdS+7Ytdb5B5BJN46fDMCTxkY=
+github.com/foxcpp/go-mockdns v0.0.0-20191226172053-3b5a6e57c8fe/go.mod h1:tPg4cp4nseejPd+UKxtCVQ2hUxNTZ7qQZJa7CLriIeo=
 github.com/foxcpp/go-mtasts v0.0.0-20191219193356-62bc3f1f74b8 h1:k8w0iy6GP9oeSZWUH3p2DqZHaXDKZGNs3NZGZMGfQHc=
 github.com/foxcpp/go-mtasts v0.0.0-20191219193356-62bc3f1f74b8/go.mod h1:HO1YOCbBM8KjpgThMMFejHx6K/UsnEv2Oh9YGtBIlOU=
 github.com/frankban/quicktest v1.5.0 h1:Tb4jWdSpdjKzTUicPnY61PZxKbDoGa7ABbrReT3gQVY=

internal/modify/dkim/dkim.go

@@ -403,7 +403,7 @@ func (s state) RewriteBody(ctx context.Context, h *textproto.Header, body buffer
 		return exterrors.WithFields(err, map[string]interface{}{"modifier": "sign_dkim"})
 	}
 
-	h.Add("DKIM-Signature", signer.SignatureValue())
+	h.AddRaw([]byte(signer.Signature()))
 
 	s.m.log.DebugMsg("signed", "identifier", id)
 

internal/modify/dkim/dkim_test.go

@@ -1,17 +1,164 @@
 package dkim
 
 import (
+	"bytes"
+	"context"
+	"io/ioutil"
+	"net"
+	"os"
+	"path/filepath"
 	"reflect"
 	"sort"
 	"strconv"
 	"testing"
 
 	"github.com/emersion/go-message/textproto"
+	"github.com/emersion/go-msgauth/dkim"
+	"github.com/foxcpp/go-mockdns"
+	"github.com/foxcpp/maddy/internal/buffer"
+	"github.com/foxcpp/maddy/internal/config"
+	"github.com/foxcpp/maddy/internal/module"
 	"github.com/foxcpp/maddy/internal/testutils"
 )
 
-// TODO: Add tests that check actual signing once
-// we have LookupTXT hook for go-msgauth/dkim.
+func newTestModifier(t *testing.T, dir, keyAlgo string) *Modifier {
+	mod, err := New("", "test", nil, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	m := mod.(*Modifier)
+	m.log = testutils.Logger(t, m.Name())
+
+	err = m.Init(config.NewMap(nil, &config.Node{
+		Children: []config.Node{
+			{
+				Name: "domain",
+				Args: []string{"maddy.test"},
+			},
+			{
+				Name: "selector",
+				Args: []string{"default"},
+			},
+			{
+				Name: "key_path",
+				Args: []string{filepath.Join(dir, "testkey.key")},
+			},
+			{
+				Name: "require_sender_match",
+				Args: []string{"off"},
+			},
+			{
+				Name: "newkey_algo",
+				Args: []string{keyAlgo},
+			},
+		},
+	}))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	return m
+}
+
+func signTestMsg(t *testing.T, m *Modifier) (textproto.Header, []byte) {
+	state, err := m.ModStateForMsg(context.Background(), &module.MsgMetadata{})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	testHdr := textproto.Header{}
+	testHdr.Add("From", "<hello@hello>")
+	testHdr.Add("Subject", "heya")
+	testHdr.Add("To", "<heya@heya>")
+	body := []byte("hello there\r\n")
+
+	err = state.RewriteBody(context.Background(), &testHdr, buffer.MemoryBuffer{Slice: body})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	return testHdr, body
+}
+
+func verifyTestMsg(t *testing.T, dnsPath string, hdr textproto.Header, body []byte) {
+	dnsRecord, err := ioutil.ReadFile(dnsPath)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	t.Log("DNS record:", string(dnsRecord))
+
+	zones := map[string]mockdns.Zone{
+		"default._domainkey.maddy.test.": {
+			TXT: []string{string(dnsRecord)},
+		},
+	}
+	t.Log(string(dnsRecord))
+	// dkim.Verify does not allow to override its lookup routine, so we have to
+	// hjack the global resolver object.
+	srv, err := mockdns.NewServer(zones)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer srv.Close()
+	srv.PatchNet(net.DefaultResolver)
+	defer mockdns.UnpatchNet(net.DefaultResolver)
+
+	var fullBody bytes.Buffer
+	if err := textproto.WriteHeader(&fullBody, hdr); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := fullBody.Write(body); err != nil {
+		t.Fatal(err)
+	}
+
+	v, err := dkim.Verify(bytes.NewReader(fullBody.Bytes()))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(v) != 1 {
+		t.Fatal("Expected exactly one verification")
+	}
+	if v[0].Err != nil {
+		t.Fatal("Verification error:", v[0].Err)
+	}
+}
+
+func TestGenerateSignVerify(t *testing.T) {
+	// This test verifies whether a freshly generated key can be used for
+	// signing and verification.
+	//
+	// It is a kind of "integration" test for DKIM modifier, as it tests
+	// whether everything works correctly together.
+
+	test := func(keyAlgo string, headerCanon, bodyCanon dkim.Canonicalization, reload bool) {
+		t.Helper()
+
+		dir, err := ioutil.TempDir("", "maddy-tests-dkim-")
+		if err != nil {
+			t.Fatal(err)
+		}
+		defer os.RemoveAll(dir)
+
+		// Create the key.
+		m := newTestModifier(t, dir, keyAlgo)
+		if reload {
+			m = newTestModifier(t, dir, keyAlgo)
+		}
+
+		testHdr, body := signTestMsg(t, m)
+		verifyTestMsg(t, filepath.Join(dir, "testkey.dns"), testHdr, body)
+	}
+
+	for _, algo := range [2]string{"rsa2048", "ed25519"} {
+		for _, hdrCanon := range [2]dkim.Canonicalization{dkim.CanonicalizationSimple, dkim.CanonicalizationRelaxed} {
+			for _, bodyCanon := range [2]dkim.Canonicalization{dkim.CanonicalizationSimple, dkim.CanonicalizationRelaxed} {
+				test(algo, hdrCanon, bodyCanon, false)
+				test(algo, hdrCanon, bodyCanon, true)
+			}
+		}
+	}
+}
 
 func TestFieldsToSign(t *testing.T) {
 	h := textproto.Header{}