dkim: Validate domain in s= and d= tags

We currently use the s= and d= tags as-is without any validation.

They are used to construct the DNS name to look up the key, which is
generally fine, but also the d= tag is used as-is in the
Authentication-Results header.

The latter can be used to inject malicious content, for example:

  d=example.com dkim=pass header.d=example.net

can result in the header including:

  header.d=example.com dkim=pass header.d=example.net

regardless of the validation status.

To fix that problem, this patch now validates the domain (using the same
normalization we use in other areas, like RCPT TO and MAIL FROM) on both
tags.

If any of the tags have invalid domains, we consider the signature
invalid as per RFC 6376 section 3.5.

internal/dkim/header.go

@@ -9,6 +9,8 @@ import (
 	"strconv"
 	"strings"
 	"time"
+
+	"blitiri.com.ar/go/chasquid/internal/normalize"
 )
 
 // https://datatracker.ietf.org/doc/html/rfc6376#section-6
@@ -136,6 +138,20 @@ func (sig *dkimSignature) checkRequiredTags() error {
 		return fmt.Errorf("%w: s=", errMissingRequiredTag)
 	}
 
+	// d= and s= must be valid domain names (a selector has the same syntax).
+	// They are attacker-controlled, and used to build the DNS name of the
+	// key; d= is also included in the Authentication-Results header, so
+	// otherwise it could be used to inject content into it.
+	// https://datatracker.ietf.org/doc/html/rfc6376#section-3.5
+	if _, err := normalize.Domain(sig.d); err != nil {
+		return fmt.Errorf("%w: d= is not a valid domain: %w",
+			errInvalidTag, err)
+	}
+	if _, err := normalize.Domain(sig.s); err != nil {
+		return fmt.Errorf("%w: s= is not a valid selector: %w",
+			errInvalidTag, err)
+	}
+
 	if len(sig.h) > maxHTagEntries {
 		return fmt.Errorf("%w: h= has too many entries (%d > %d)",
 			errInvalidTag, len(sig.h), maxHTagEntries)

internal/dkim/header_test.go

@@ -333,6 +333,30 @@ func TestCheckRequiredTags(t *testing.T) {
 			},
 			err: "invalid tag: h= does not contain 'from'",
 		},
+		{
+			sig: &dkimSignature{
+				a:  "rsa-sha256",
+				b:  []byte("hola que tal"),
+				bh: []byte("como andas"),
+				d:  "example.com dkim=pass header.d=example.net",
+				h:  []string{"from"},
+				s:  "selector",
+			},
+			err: "invalid tag: d= is not a valid domain: " +
+				"invalid domain: invalid character ' '",
+		},
+		{
+			sig: &dkimSignature{
+				a:  "rsa-sha256",
+				b:  []byte("hola que tal"),
+				bh: []byte("como andas"),
+				d:  "example.com",
+				h:  []string{"from"},
+				s:  "sel/ector",
+			},
+			err: "invalid tag: s= is not a valid selector: " +
+				"invalid domain: invalid character '/'",
+		},
 		{
 			sig: &dkimSignature{
 				a:  "rsa-sha256",
@@ -381,6 +405,17 @@ func TestCheckRequiredTags(t *testing.T) {
 			},
 			err: "<nil>",
 		},
+		{
+			sig: &dkimSignature{
+				a:  "rsa-sha256",
+				b:  []byte("hola que tal"),
+				bh: []byte("como andas"),
+				d:  "ñandú.example", // U-labels are allowed (RFC 8616).
+				h:  []string{"from"},
+				s:  "sel_2026.ector", // "_" and "." are allowed.
+			},
+			err: "<nil>",
+		},
 		{
 			sig: &dkimSignature{
 				a:  "rsa-sha256",

internal/dkim/verify_test.go

@@ -262,6 +262,41 @@ func TestVerifyBodyLength(t *testing.T) {
 	}
 }
 
+func TestVerifyInvalidDomain(t *testing.T) {
+	ctx := t.Context()
+	ctx = WithTraceFunc(ctx, t.Logf)
+	ctx = WithLookupTXTFunc(ctx, makeLookupTXT(map[string][]string{}))
+
+	// A d= tag that is not a valid domain must be rejected, and must not
+	// make it to the Authentication-Results header, where it could inject
+	// content.
+	message := toCRLF(
+		"DKIM-Signature: v=1; a=ed25519-sha256; c=simple/simple;\n" +
+			" d=example.com dkim=pass header.d=example.net; s=sel;\n" +
+			" h=from; bh=AAAA; b=AAAA\n" +
+			"From: Joe SixPack <joe@example.com>\n" +
+			"\n" +
+			"Hi.\n")
+
+	res, err := VerifyMessage(ctx, message)
+	if err != nil {
+		t.Fatalf("VerifyMessage returned error: %v", err)
+	}
+	if res.Found != 1 || res.Valid != 0 {
+		t.Fatalf("wanted 1 found, 0 valid; got %v", res)
+	}
+	r := res.Results[0]
+	if r.State != PERMFAIL || !errors.Is(r.Error, errInvalidTag) {
+		t.Errorf("wanted PERMFAIL / %v; got %v / %v",
+			errInvalidTag, r.State, r.Error)
+	}
+
+	ar := res.AuthenticationResults()
+	if strings.Contains(ar, "example.net") {
+		t.Errorf("Authentication-Results contains the invalid d=: %q", ar)
+	}
+}
+
 func TestHeadersToInclude(t *testing.T) {
 	// Test that headersToInclude returns the expected headers.
 	cases := []struct {