dkim: Consider only 3 TXT records when fetching keys

Today, when looking up DKIM keys via DNS, we go through all the TXT
records that are published. This is already more lenient than what's
allowed by many implementations (as documented in the code).

Potentially, someone could craft a domain with a lot of TXT records to
make us waste time processing those keys.

To help reduce that possibility, this patch limits the lookup to 3
records. This is still more than normal and may be limited further to 1
in the future.

internal/dkim/dns.go

@@ -13,6 +13,10 @@ import (
 	"strings"
 )
 
+// Maximum number of TXT records we consider when looking up the public key.
+// See findPublicKeys for details.
+const maxKeyRecords = 3
+
 func findPublicKeys(ctx context.Context, domain, selector string) ([]*publicKey, error) {
 	// Subdomain where the key lives.
 	// https://datatracker.ietf.org/doc/html/rfc6376#section-3.6.2
@@ -33,8 +37,16 @@ func findPublicKeys(ctx context.Context, domain, selector string) ([]*publicKey,
 	//  - driusan/dkim: Use the first TXT record that can be parsed as a key.
 	//  - go-msgauth: Reject if there are multiple records.
 	//
-	// What we do: use _all_ TXT records that can be parsed as keys. This is
-	// possibly too much, and we could reconsider this in the future.
+	// What we do: use the TXT records that can be parsed as keys, but only
+	// look at the first few. The records are attacker-controlled, and each
+	// key can cost a signature verification, so we need to cap them.
+	// This is still more lenient than the implementations above, and we
+	// could reconsider this in the future.
+	if len(values) > maxKeyRecords {
+		trace(ctx, "Too many TXT records for %q (%d), using the first %d",
+			d, len(values), maxKeyRecords)
+		values = values[:maxKeyRecords]
+	}
 
 	pks := []*publicKey{}
 	for _, v := range values {

internal/dkim/dns_test.go

@@ -66,6 +66,34 @@ func TestSkipBadRecords(t *testing.T) {
 	}
 }
 
+func TestMaxKeyRecords(t *testing.T) {
+	ctx := WithLookupTXTFunc(t.Context(), testLookupTXT)
+	defer clear(results)
+
+	// Only the first maxKeyRecords records are used, even if the rest are
+	// valid keys.
+	key := "v=DKIM1; p=" + exampleRSAKeyB64
+	results["selector._domainkey.example.com"] = []string{
+		key, key, key, key, key,
+	}
+	pks, err := findPublicKeys(ctx, "example.com", "selector")
+	if err != nil || len(pks) != maxKeyRecords {
+		t.Errorf("findPublicKeys expected %d keys / nil, got %d / %v",
+			maxKeyRecords, len(pks), err)
+	}
+
+	// Records after the first maxKeyRecords are ignored, even if the first
+	// ones are not valid keys.
+	results["selector._domainkey.example.com"] = []string{
+		"not a tag", "not a tag", "not a tag", key,
+	}
+	pks, err = findPublicKeys(ctx, "example.com", "selector")
+	if err != nil || len(pks) != 0 {
+		t.Errorf("findPublicKeys expected 0 keys / nil, got %d / %v",
+			len(pks), err)
+	}
+}
+
 func TestParsePublicKey(t *testing.T) {
 	cases := []struct {
 		in  string