From 5c4193a8b695eeafafba96ef91fb45099bda3ef9 Mon Sep 17 00:00:00 2001 From: Alberto Bertogli Date: Fri, 9 Oct 2026 00:42:45 +0100 Subject: [PATCH] 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 | 8 ++++++-- internal/dkim/header_test.go | 2 +- internal/dkim/verify.go | 10 +++++----- internal/dkim/verify_test.go | 35 ++++++++++++++++++++--------------- 4 files changed, 32 insertions(+), 23 deletions(-) diff --git a/internal/dkim/header.go b/internal/dkim/header.go index 5558178..680c693 100644 --- a/internal/dkim/header.go +++ b/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. diff --git a/internal/dkim/header_test.go b/internal/dkim/header_test.go index 1ebfbdf..dead777 100644 --- a/internal/dkim/header_test.go +++ b/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), diff --git a/internal/dkim/verify.go b/internal/dkim/verify.go index 207b018..00b5107 100644 --- a/internal/dkim/verify.go +++ b/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. diff --git a/internal/dkim/verify_test.go b/internal/dkim/verify_test.go index ead16f0..4bf718b 100644 --- a/internal/dkim/verify_test.go +++ b/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 - want error + 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) } } }