This is an automated email from the ASF dual-hosted git repository. chibenwa pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/james-jdkim.git
commit 9516527dfc63dae8f603e6bb0a46e62de342ce40 Author: Benoit TELLIER <[email protected]> AuthorDate: Sun Nov 1 19:58:05 2026 +0000 [FIX] DKIM-Signature: reject a malformed z= z= was never read: empty copies (z=||) or copies without ":" were accepted and the signature reported as pass, although RFC 6376 section 3.5 defines sig-z-tag-copy *( "|" [FWS] sig-z-tag-copy ) with sig-z-tag-copy = hdr-name [FWS] ":" qp-hdr-value, and section 6.1.1 requires a PERMFAIL on unexpected values. With strict validation (-Djames.jdkim.strict.validation, default true), SignatureRecordImpl and SignatureRecordTemplate now check every copy. z= remains diagnostic only and has no other effect on verification. --- docs/modules/ROOT/pages/usage.adoc | 3 ++- .../james/jdkim/tagvalue/SignatureRecordImpl.java | 26 ++++++++++++++++++ .../jdkim/tagvalue/SignatureRecordTemplate.java | 3 +++ .../james/jdkim/SignatureRecordImplTest.java | 31 ++++++++++++++++++++++ 4 files changed, 62 insertions(+), 1 deletion(-) diff --git a/docs/modules/ROOT/pages/usage.adoc b/docs/modules/ROOT/pages/usage.adoc index b25c00c..ca2ef34 100644 --- a/docs/modules/ROOT/pages/usage.adoc +++ b/docs/modules/ROOT/pages/usage.adoc @@ -279,7 +279,8 @@ values that do not follow the RFC grammar, such as an empty entry or a trailing `:` in the `h=`, `k=`, `s=` or `t=` lists of a key record, an empty `t=`, a DKIM-Signature `d=` or `s=` that is not a domain name (a `_`, an empty label, a label over 63 octets), an `i=` whose local part or domain is -malformed, or a `q=` with an empty or malformed entry. A verifier stricter than the others may however reject real-world, +malformed, a `q=` with an empty or malformed entry, or a `z=` whose copies are +not `name:value`. A verifier stricter than the others may however reject real-world, slightly malformed signatures or keys. Start the JVM with `-Djames.jdkim.strict.validation=false` to disable these syntax checks. Checks with a semantic effect -- a key for another service (`s=chat`), an expired diff --git a/main/src/main/java/org/apache/james/jdkim/tagvalue/SignatureRecordImpl.java b/main/src/main/java/org/apache/james/jdkim/tagvalue/SignatureRecordImpl.java index e0e3382..64bfd1a 100644 --- a/main/src/main/java/org/apache/james/jdkim/tagvalue/SignatureRecordImpl.java +++ b/main/src/main/java/org/apache/james/jdkim/tagvalue/SignatureRecordImpl.java @@ -142,6 +142,29 @@ public class SignatureRecordImpl extends TagValue implements SignatureRecord { static final Pattern QUERY_METHOD = Pattern.compile( "[A-Za-z](?:[A-Za-z0-9-]*[A-Za-z0-9])?(?:/(?:[\\x21-\\x3A\\x3C\\x3E-\\x7B\\x7D\\x7E]|=[0-9A-F]{2})*)?"); + // SPEC: sig-z-tag-copy = hdr-name [FWS] ":" qp-hdr-value ; hdr-name = 1*ftext (%d33-57 / %d59-126) + private static final Pattern COPIED_HEADER_FIELD = Pattern.compile( + "[\\x21-\\x39\\x3B-\\x7E]+[ \\t\\r\\n]*:(?:[\\x21-\\x3A\\x3C\\x3E-\\x7B\\x7D\\x7E]|=[0-9A-F]{2}|[ \\t\\r\\n])*"); + + /** + * RFC 6376 section 3.5: checks z= as sig-z-tag-copy *( "|" [FWS] sig-z-tag-copy ). + * Only with strict validation, see {@link TagValue#isStrictValidation()}. + * + * @throws IllegalStateException if it is not + */ + static void checkCopiedHeaderFields(CharSequence z) { + if (!isStrictValidation()) { + return; + } + for (String copy : z.toString().split("\\|", -1)) { + String trimmed = copy.replaceFirst("^[ \\t\\r\\n]+", ""); + if (!COPIED_HEADER_FIELD.matcher(trimmed).matches()) { + throw new IllegalStateException("Invalid z= [" + z + "]: copy [" + copy + + "] is not hdr-name [FWS] \":\" qp-hdr-value (RFC 6376 section 3.5)"); + } + } + } + /** * Checks the a= value of a signature record, see {@link #checkAlgorithm(CharSequence)}. * @@ -194,6 +217,9 @@ public class SignatureRecordImpl extends TagValue implements SignatureRecord { checkAlgorithm(getValue("a")); checkSdidAndSelector(getValue("d"), getValue("s")); getRecordLookupMethods(); + if (getValue("z") != null) { + checkCopiedHeaderFields(getValue("z")); + } CharSequence identity; try { diff --git a/main/src/main/java/org/apache/james/jdkim/tagvalue/SignatureRecordTemplate.java b/main/src/main/java/org/apache/james/jdkim/tagvalue/SignatureRecordTemplate.java index a113ad0..b8275d5 100644 --- a/main/src/main/java/org/apache/james/jdkim/tagvalue/SignatureRecordTemplate.java +++ b/main/src/main/java/org/apache/james/jdkim/tagvalue/SignatureRecordTemplate.java @@ -73,6 +73,9 @@ public class SignatureRecordTemplate extends TagValue implements SignatureRecord } SignatureRecordImpl.checkSdidAndSelector(getValue("d"), getValue("s")); getRecordLookupMethods(); + if (getValue("z") != null) { + SignatureRecordImpl.checkCopiedHeaderFields(getValue("z")); + } // TODO: what about v=0.5 and no v= at all? // do specs allow parsing? what should we check? if (!"1".equals(getValue("v"))) diff --git a/main/src/test/java/org/apache/james/jdkim/SignatureRecordImplTest.java b/main/src/test/java/org/apache/james/jdkim/SignatureRecordImplTest.java index dedfa35..0614da7 100644 --- a/main/src/test/java/org/apache/james/jdkim/SignatureRecordImplTest.java +++ b/main/src/test/java/org/apache/james/jdkim/SignatureRecordImplTest.java @@ -418,4 +418,35 @@ public class SignatureRecordImplTest { System.clearProperty(TagValue.STRICT_VALIDATION_PROPERTY); } } + + private static SignatureRecordImpl recordWithCopiedFields(String z) { + return new SignatureRecordImpl("v=1; a=rsa-sha256; c=relaxed/relaxed; d=example.com; s=sel;" + + " h=from:subject; z=" + z + "; bh=AAAA; b=AAAA"); + } + + @Test + public void malformedCopiedFieldsAreASyntaxError() { + for (String z : new String[] {"|", "||", "|\r\n |", "From:a|", "From", "|From:a", ":a"}) { + assertThatThrownBy(() -> recordWithCopiedFields(z)).as("z=%s", z) + .isInstanceOf(IllegalStateException.class); + } + } + + @Test + public void wellFormedCopiedFieldsAreAccepted() { + recordWithCopiedFields("From:[email protected]|Subject:demo=20run"); + recordWithCopiedFields("From: [email protected]"); + recordWithCopiedFields("To:"); + recordWithCopiedFields("From:[email protected]|\r\n To:[email protected]"); + } + + @Test + public void malformedCopiedFieldsAreAcceptedWithoutStrictValidation() { + System.setProperty(TagValue.STRICT_VALIDATION_PROPERTY, "false"); + try { + recordWithCopiedFields("||"); + } finally { + System.clearProperty(TagValue.STRICT_VALIDATION_PROPERTY); + } + } } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
