dkim: l=0 means none of the body is signed

Today, we treat l=0 to mean the whole body is signed. However, RFC 6376
is clear that means _none_ of the body is signed.

This patch fixes that behaviour to match the RFC.

Cross-checking other implementations for potential compatibility issues,
OpenDKIM and Exim both treat l=0 the same way this patch does.

In practice, l=0 is extremely rare, and that's why this incorrect
behaviour persisted unnoticed for so long.

Note this incorrect behaviour would cause valid signatures to fail to
verify, but never allow invalid signatures to pass.

internal/dkim/header.go

@@ -50,7 +50,9 @@ type dkimSignature struct {
 	i string
 
 	// Body octet count of the canonicalized body.
-	l uint64
+	// nil if not present. Note that 0 is a valid value, and means that none
+	// of the body is signed.
+	l *uint64
 
 	// Query methods used for DNS lookup.
 	// Colon-separated list of methods. Only "dns/txt" is valid.
@@ -213,11 +215,13 @@ func dkimSignatureFromHeader(header string) (*dkimSignature, error) {
 	sig.i = tags["i"]
 
 	if tags["l"] != "" {
-		sig.l, err = strconv.ParseUint(tags["l"], 10, 64)
+		var l uint64
+		l, err = strconv.ParseUint(tags["l"], 10, 64)
 		if err != nil {
 			return nil, fmt.Errorf("%w: failed to parse l: %w",
 				errInvalidSignature, err)
 		}
+		sig.l = &l
 	}
 
 	// q is a colon-separated list of query methods.

internal/dkim/header_test.go

@@ -40,7 +40,7 @@ func TestSignatureFromHeader(t *testing.T) {
 				d:  "example.com",
 				h:  []string{"from", "to", "subject"},
 				i:  "agent@example.com",
-				l:  77,
+				l:  new(uint64(77)),
 				q:  []string{"dns/txt"},
 				s:  "selector",
 				t:  time.Unix(1600700888, 0),

internal/dkim/verify.go

@@ -189,19 +189,19 @@ func verifySignature(ctx context.Context, sigH header,
 	// (if present).
 	// https://datatracker.ietf.org/doc/html/rfc6376#section-3.7
 	bodyC := sig.cB.body(body)
-	if sig.l > 0 {
+	if sig.l != nil {
 		// The l= value MUST NOT be larger than the canonicalized body.
 		// https://datatracker.ietf.org/doc/html/rfc6376#section-3.5
-		if sig.l > uint64(len(bodyC)) {
+		if *sig.l > uint64(len(bodyC)) {
 			trace(ctx, "l= (%d) is larger than the body (%d)",
-				sig.l, len(bodyC))
+				*sig.l, len(bodyC))
 			result.Error = fmt.Errorf(
 				"%w: l= (%d) is larger than the body (%d)",
-				errInvalidTag, sig.l, len(bodyC))
+				errInvalidTag, *sig.l, len(bodyC))
 			result.State = PERMFAIL
 			return result
 		}
-		bodyC = bodyC[:sig.l]
+		bodyC = bodyC[:*sig.l]
 	}
 
 	// Step 2: Compute the hash of the canonicalized body.

internal/dkim/verify_test.go

@@ -217,23 +217,26 @@ func TestVerifyBodyLength(t *testing.T) {
 				"p=11qYAYKxCrfVS/7TyWQHOg7hcvPapiMlrwIaaPcHURo="},
 	}))
 
-	// Use a valid body hash, so we can tell the l= cases apart: when l= is
-	// the body length, the body hash matches and we fail later on (because
-	// b= is bogus).
+	// Use a valid body hash (of the "signed" part of the body), so we can
+	// tell the l= cases apart: when l= covers exactly the signed part, the
+	// body hash matches and we fail later on (because b= is bogus).
 	body := "Hi.\r\n"
-	bh := base64.StdEncoding.EncodeToString(
-		hashWith(crypto.SHA256, []byte(body)))
 
 	cases := []struct {
 		l      string
+		signed string
 		want   error
 	}{
-		{"4", ErrBodyHashMismatch},
-		{"5", ErrVerificationFailed},
-		{"6", errInvalidTag},
-		{"18446744073709551615", errInvalidTag},
+		{"0", "", ErrVerificationFailed},
+		{"0", body, ErrBodyHashMismatch},
+		{"4", body, ErrBodyHashMismatch},
+		{"5", body, ErrVerificationFailed},
+		{"6", body, errInvalidTag},
+		{"18446744073709551615", body, errInvalidTag},
 	}
 	for _, c := range cases {
+		bh := base64.StdEncoding.EncodeToString(
+			hashWith(crypto.SHA256, []byte(c.signed)))
 		message := toCRLF(
 			"DKIM-Signature: v=1; a=ed25519-sha256; c=simple/simple;\n"+
 				" d=example.com; s=brisbane; h=from; l="+c.l+";\n"+
@@ -243,15 +246,17 @@ func TestVerifyBodyLength(t *testing.T) {
 
 		res, err := VerifyMessage(ctx, message)
 		if err != nil {
-			t.Fatalf("l=%s: VerifyMessage returned error: %v", c.l, err)
+			t.Fatalf("l=%s signed=%q: VerifyMessage returned error: %v",
+				c.l, c.signed, err)
 		}
 		if res.Found != 1 || res.Valid != 0 {
-			t.Fatalf("l=%s: wanted 1 found, 0 valid; got %v", c.l, res)
+			t.Fatalf("l=%s signed=%q: wanted 1 found, 0 valid; got %v",
+				c.l, c.signed, res)
 		}
 		r := res.Results[0]
 		if r.State != PERMFAIL || !errors.Is(r.Error, c.want) {
-			t.Errorf("l=%s: wanted PERMFAIL / %v; got %v / %v",
-				c.l, c.want, r.State, r.Error)
+			t.Errorf("l=%s signed=%q: wanted PERMFAIL / %v; got %v / %v",
+				c.l, c.signed, c.want, r.State, r.Error)
 		}
 	}
 }