From 3b0836f811b71d7330fb887baf7f43beba763c65 Mon Sep 17 00:00:00 2001 From: Alberto Bertogli Date: Fri, 9 Oct 2026 11:43:33 +0100 Subject: [PATCH] dkim: When signing, process repeated headers from bottom to top When signing repeated headers, today we hash each instance of a header from top to bottom. However, RFC 6376 section 5.4.2 is clear they should be processed bottom to top, and this is also what our verifier (correctly) expects. The bug causes our signatures to be (wrongly) invalid for messages that have repeated signed headers with different values. Note that repeated headers that are not signed (e.g. Received:) are not problematic. This only affects headers we sign, like To:, From:, Subject:. This patch fixes the problem by adjusting the order in which Sign processes repeated headers. This bug was found by fuzzing the sign-verify cycle, which will be added in a later patch. --- internal/dkim/sign.go | 6 ++++- internal/dkim/sign_test.go | 45 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/internal/dkim/sign.go b/internal/dkim/sign.go index 1d89394..03bcd7a 100644 --- a/internal/dkim/sign.go +++ b/internal/dkim/sign.go @@ -9,6 +9,7 @@ import ( "crypto/sha256" "encoding/base64" "fmt" + "slices" "strings" "time" ) @@ -96,7 +97,10 @@ func (s *Signer) Sign(ctx context.Context, message string) (string, error) { // First, the canonicalized headers. b := sha256.New() for _, h := range headersToSign { - for _, header := range headers.FindAll(h) { + // If a header appears multiple times, we must hash the instances + // from the bottom of the header block to the top. + // https://datatracker.ietf.org/doc/html/rfc6376#section-5.4.2 + for _, header := range slices.Backward(headers.FindAll(h)) { hsrc := relaxedCanonicalization.header(header).Source + "\r\n" trace(ctx, "Hashing header: %q", hsrc) b.Write([]byte(hsrc)) diff --git a/internal/dkim/sign_test.go b/internal/dkim/sign_test.go index 7184a24..f31e43b 100644 --- a/internal/dkim/sign_test.go +++ b/internal/dkim/sign_test.go @@ -165,6 +165,51 @@ func TestSignManyHeaders(t *testing.T) { } } +func TestSignRepeatedHeaders(t *testing.T) { + // When a signed header appears multiple times (with different values), + // they must be signed from bottom to top, otherwise the signature will + // not verify. + ctx := t.Context() + ctx = WithTraceFunc(ctx, t.Logf) + + pub, priv, err := ed25519.GenerateKey(rand.Reader) + if err != nil { + t.Fatalf("ed25519.GenerateKey: %v", err) + } + + ctx = WithLookupTXTFunc(ctx, makeLookupTXT(map[string][]string{ + "test._domainkey.example.com": { + "v=DKIM1; k=ed25519; p=" + base64.StdEncoding.EncodeToString(pub), + }, + })) + + s := &Signer{ + Domain: "example.com", + Selector: "test", + Signer: priv, + } + + message := toCRLF(`From: from@example.com +To: to1@example.com +Subject: Subject 1 +To: to2@example.com +Subject: Subject 2 +To: to3@example.com + +Hi. +`) + + sig, err := s.Sign(ctx, message) + if err != nil { + t.Fatalf("Sign: %v", err) + } + + res, err := VerifyMessage(ctx, addSig(sig, message)) + if err != nil || res.Valid != 1 { + t.Errorf("VerifyMessage: wanted 1 valid / nil; got %v / %v", res, err) + } +} + func addSig(sig, message string) string { return "DKIM-Signature: " + strings.ReplaceAll(sig, "\r\n", "\r\n\t") +