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 b7858ddfcb834d27f0238c4dd0c951ac0e1290d1 Author: Benoit TELLIER <[email protected]> AuthorDate: Sun Nov 1 19:50:04 2026 +0000 [FIX] Key record: reject malformed h=, k= and t= lists stringToColonSeparatedList stopped at a trailing ":" and accepted an empty value, and getFlags split t= without checking its entries: keys with h=sha256:sha1:, k=rsa:, t=y: or an empty t= were used, although RFC 6376 section 6.1.2 says to ignore a record that does not follow the section 3.6.1 grammar (key syntax error). With strict validation (-Djames.jdkim.strict.validation, default true), an empty list or a trailing ":" is now a syntax error, t= entries are checked as hyphenated-words, and PublicKeyRecordImpl#validate parses these lists so that publicKeySelector tries the next record. Without it, the previous lenient parsing is kept; the semantic restrictions (h=sha256: still refuses rsa-sha1) apply in both cases. --- docs/modules/ROOT/pages/usage.adoc | 5 +- .../james/jdkim/tagvalue/PublicKeyRecordImpl.java | 9 +- .../org/apache/james/jdkim/tagvalue/TagValue.java | 4 + .../apache/james/jdkim/PublicKeyRecordTest.java | 118 ++++++++++++++++++--- 4 files changed, 120 insertions(+), 16 deletions(-) diff --git a/docs/modules/ROOT/pages/usage.adoc b/docs/modules/ROOT/pages/usage.adoc index 83298bc..73d0732 100644 --- a/docs/modules/ROOT/pages/usage.adoc +++ b/docs/modules/ROOT/pages/usage.adoc @@ -267,8 +267,9 @@ DKIMVerifier verifier = new DKIMVerifier(new VerifierOptions.Builder() === Strict syntax validation jDKIM rejects, as a syntax error (RFC 6376 sections 3.5, 3.6.1 and 6.1), tag -values that do not follow the RFC grammar, such as a malformed `s=` list in a -key record. A verifier stricter than the others may however reject real-world, +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, or an empty +`t=`. 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/PublicKeyRecordImpl.java b/main/src/main/java/org/apache/james/jdkim/tagvalue/PublicKeyRecordImpl.java index 1c9dc60..8ac8e75 100644 --- a/main/src/main/java/org/apache/james/jdkim/tagvalue/PublicKeyRecordImpl.java +++ b/main/src/main/java/org/apache/james/jdkim/tagvalue/PublicKeyRecordImpl.java @@ -88,8 +88,11 @@ public class PublicKeyRecordImpl extends TagValue implements PublicKeyRecord { if ("".equals(getValue("p"))) throw new IllegalStateException("Revoked key. 'p=' in record"); if (isStrictValidation()) { - // a malformed s= is a key syntax error (RFC 6376 section 6.1.2) + // a malformed list is a key syntax error (RFC 6376 section 6.1.2) getServiceTypes(); + getAcceptableHashMethods(); + getAcceptableKeyTypes(); + getFlags(); } } @@ -185,6 +188,10 @@ public class PublicKeyRecordImpl extends TagValue implements PublicKeyRecord { public List<CharSequence> getFlags() { String flags = getValue("t").toString(); + if (isStrictValidation() && containsTag("t")) { + // SPEC: key-t-tag-flag *( [FWS] ":" [FWS] key-t-tag-flag ), each a hyphenated-word + return stringToColonSeparatedList(flags, hyphenatedWordPattern); + } String[] flagsStrings = flags.split(":"); List<CharSequence> res = new ArrayList<>(); for (String flagsString : flagsStrings) { diff --git a/main/src/main/java/org/apache/james/jdkim/tagvalue/TagValue.java b/main/src/main/java/org/apache/james/jdkim/tagvalue/TagValue.java index b4e6189..393e704 100644 --- a/main/src/main/java/org/apache/james/jdkim/tagvalue/TagValue.java +++ b/main/src/main/java/org/apache/james/jdkim/tagvalue/TagValue.java @@ -291,6 +291,10 @@ public class TagValue { protected List<CharSequence> stringToColonSeparatedList(String h, Pattern pattern) { List<CharSequence> headers = new ArrayList<>(); + // SPEC: entry *( [FWS] ":" [FWS] entry ): neither an empty list nor a trailing ":" + if (isStrictValidation() && h.trim().matches("(?s)|.*:")) { + throw new IllegalStateException("Syntax error in list: empty entry in [" + h + "]"); + } for (int i = 0; i < h.length(); i++) { int p = h.indexOf(':', i); if (p == -1) diff --git a/main/src/test/java/org/apache/james/jdkim/PublicKeyRecordTest.java b/main/src/test/java/org/apache/james/jdkim/PublicKeyRecordTest.java index fd43805..e14489e 100644 --- a/main/src/test/java/org/apache/james/jdkim/PublicKeyRecordTest.java +++ b/main/src/test/java/org/apache/james/jdkim/PublicKeyRecordTest.java @@ -229,11 +229,22 @@ public class PublicKeyRecordTest { pkr.validate(); flags = pkr.getFlags(); assertEquals("[y]", flags.toString()); - pkr = new PublicKeyRecordImpl( - "k=rsa; t=; p=MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDIhyR3oItOy22ZOaBrIVe9m/iME3RqOJeasANSpg2YTHTYV+Xtp4xwf5gTjCmHQEMOs0qYu0FYiNQPQogJ2t0Mfx9zNu06rfRBDjiIU9tpx2T+NGlWZ8qhbiLo5By8apJavLyqTLavyPSrvsx0B3YzC63T4Age2CDqZYA+OwSMWQIDAQAB"); - pkr.validate(); - flags = pkr.getFlags(); - assertEquals("[]", flags.toString()); + // an empty t= is a key syntax error, accepted as no flag without strict validation + PublicKeyRecord emptyFlags = new PublicKeyRecordImpl("k=rsa; t=; p=MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDIhyR3oItOy22ZOaBrIVe9m/iME3RqOJeasANSpg2YTHTYV+Xtp4xwf5gTjCmHQEMOs0qYu0FYiNQPQogJ2t0Mfx9zNu06rfRBDjiIU9tpx2T+NGlWZ8qhbiLo5By8apJavLyqTLavyPSrvsx0B3YzC63T4Age2CDqZYA+OwSMWQIDAQAB"); + try { + emptyFlags.validate(); + fail("empty t= accepted"); + } catch (IllegalStateException expected) { + } + System.setProperty(TagValue.STRICT_VALIDATION_PROPERTY, "false"); + try { + PublicKeyRecord lenient = new PublicKeyRecordImpl("k=rsa; t=; p=MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDIhyR3oItOy22ZOaBrIVe9m/iME3RqOJeasANSpg2YTHTYV+Xtp4xwf5gTjCmHQEMOs0qYu0FYiNQPQogJ2t0Mfx9zNu06rfRBDjiIU9tpx2T+NGlWZ8qhbiLo5By8apJavLyqTLavyPSrvsx0B3YzC63T4Age2CDqZYA+OwSMWQIDAQAB"); + lenient.validate(); + List<CharSequence> noFlags = lenient.getFlags(); + assertEquals("[]", noFlags.toString()); + } finally { + System.clearProperty(TagValue.STRICT_VALIDATION_PROPERTY); + } } @Test @@ -246,10 +257,21 @@ public class PublicKeyRecordTest { "k=rsa; t=y; p=MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDIhyR3oItOy22ZOaBrIVe9m/iME3RqOJeasANSpg2YTHTYV+Xtp4xwf5gTjCmHQEMOs0qYu0FYiNQPQogJ2t0Mfx9zNu06rfRBDjiIU9tpx2T+NGlWZ8qhbiLo5By8apJavLyqTLavyPSrvsx0B3YzC63T4Age2CDqZYA+OwSMWQIDAQAB"); pkr.validate(); assertTrue(pkr.isTesting()); - pkr = new PublicKeyRecordImpl( - "k=rsa; t=; p=MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDIhyR3oItOy22ZOaBrIVe9m/iME3RqOJeasANSpg2YTHTYV+Xtp4xwf5gTjCmHQEMOs0qYu0FYiNQPQogJ2t0Mfx9zNu06rfRBDjiIU9tpx2T+NGlWZ8qhbiLo5By8apJavLyqTLavyPSrvsx0B3YzC63T4Age2CDqZYA+OwSMWQIDAQAB"); - pkr.validate(); - assertFalse(pkr.isTesting()); + // an empty t= is a key syntax error, accepted as no flag without strict validation + PublicKeyRecord emptyFlags = new PublicKeyRecordImpl("k=rsa; t=; p=MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDIhyR3oItOy22ZOaBrIVe9m/iME3RqOJeasANSpg2YTHTYV+Xtp4xwf5gTjCmHQEMOs0qYu0FYiNQPQogJ2t0Mfx9zNu06rfRBDjiIU9tpx2T+NGlWZ8qhbiLo5By8apJavLyqTLavyPSrvsx0B3YzC63T4Age2CDqZYA+OwSMWQIDAQAB"); + try { + emptyFlags.validate(); + fail("empty t= accepted"); + } catch (IllegalStateException expected) { + } + System.setProperty(TagValue.STRICT_VALIDATION_PROPERTY, "false"); + try { + PublicKeyRecord lenient = new PublicKeyRecordImpl("k=rsa; t=; p=MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDIhyR3oItOy22ZOaBrIVe9m/iME3RqOJeasANSpg2YTHTYV+Xtp4xwf5gTjCmHQEMOs0qYu0FYiNQPQogJ2t0Mfx9zNu06rfRBDjiIU9tpx2T+NGlWZ8qhbiLo5By8apJavLyqTLavyPSrvsx0B3YzC63T4Age2CDqZYA+OwSMWQIDAQAB"); + lenient.validate(); + assertFalse(lenient.isTesting()); + } finally { + System.clearProperty(TagValue.STRICT_VALIDATION_PROPERTY); + } } @Test @@ -262,10 +284,21 @@ public class PublicKeyRecordTest { "k=rsa; t=y; p=MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDIhyR3oItOy22ZOaBrIVe9m/iME3RqOJeasANSpg2YTHTYV+Xtp4xwf5gTjCmHQEMOs0qYu0FYiNQPQogJ2t0Mfx9zNu06rfRBDjiIU9tpx2T+NGlWZ8qhbiLo5By8apJavLyqTLavyPSrvsx0B3YzC63T4Age2CDqZYA+OwSMWQIDAQAB"); pkr.validate(); assertFalse(pkr.isDenySubdomains()); - pkr = new PublicKeyRecordImpl( - "k=rsa; t=; p=MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDIhyR3oItOy22ZOaBrIVe9m/iME3RqOJeasANSpg2YTHTYV+Xtp4xwf5gTjCmHQEMOs0qYu0FYiNQPQogJ2t0Mfx9zNu06rfRBDjiIU9tpx2T+NGlWZ8qhbiLo5By8apJavLyqTLavyPSrvsx0B3YzC63T4Age2CDqZYA+OwSMWQIDAQAB"); - pkr.validate(); - assertFalse(pkr.isDenySubdomains()); + // an empty t= is a key syntax error, accepted as no flag without strict validation + PublicKeyRecord emptyFlags = new PublicKeyRecordImpl("k=rsa; t=; p=MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDIhyR3oItOy22ZOaBrIVe9m/iME3RqOJeasANSpg2YTHTYV+Xtp4xwf5gTjCmHQEMOs0qYu0FYiNQPQogJ2t0Mfx9zNu06rfRBDjiIU9tpx2T+NGlWZ8qhbiLo5By8apJavLyqTLavyPSrvsx0B3YzC63T4Age2CDqZYA+OwSMWQIDAQAB"); + try { + emptyFlags.validate(); + fail("empty t= accepted"); + } catch (IllegalStateException expected) { + } + System.setProperty(TagValue.STRICT_VALIDATION_PROPERTY, "false"); + try { + PublicKeyRecord lenient = new PublicKeyRecordImpl("k=rsa; t=; p=MIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDIhyR3oItOy22ZOaBrIVe9m/iME3RqOJeasANSpg2YTHTYV+Xtp4xwf5gTjCmHQEMOs0qYu0FYiNQPQogJ2t0Mfx9zNu06rfRBDjiIU9tpx2T+NGlWZ8qhbiLo5By8apJavLyqTLavyPSrvsx0B3YzC63T4Age2CDqZYA+OwSMWQIDAQAB"); + lenient.validate(); + assertFalse(lenient.isDenySubdomains()); + } finally { + System.clearProperty(TagValue.STRICT_VALIDATION_PROPERTY); + } } @Test @@ -346,4 +379,63 @@ public class PublicKeyRecordTest { applyRecord("s=" + s + "; p=" + P); } } + + private static final SignatureRecord SHA1_SIGN = new SignatureRecordImpl( + "v=1; a=rsa-sha1; d=example.com; s=sel; h=from:to; bh=AAAA; b=AAAA"); + + private static final String[] MALFORMED_LIST_RECORDS = { + "t=; p=" + P, + "p=" + P + "; h=sha256:sha1:", + "p=" + P + "; h=", + "p=" + P + "; k=rsa:", + "p=" + P + "; t=y:"}; + + @Test + public void malformedListTagsMakeTheKeyInapplicable() { + for (String record : MALFORMED_LIST_RECORDS) { + try { + PublicKeyRecord pk = new DKIMVerifier().publicKeySelector(Collections.singletonList(record)); + DKIMVerifier.apply(pk, SHA1_SIGN); + fail("accepted: " + record); + } catch (PermFailException expected) { + } + } + } + + @Test + public void malformedListTagsAreAcceptedWithoutStrictValidation() throws Exception { + System.setProperty(TagValue.STRICT_VALIDATION_PROPERTY, "false"); + try { + for (String record : MALFORMED_LIST_RECORDS) { + if (record.contains("h=;") || record.endsWith("h=")) { + continue; + } + DKIMVerifier.apply(new DKIMVerifier().publicKeySelector(Collections.singletonList(record)), SHA1_SIGN); + } + } finally { + System.clearProperty(TagValue.STRICT_VALIDATION_PROPERTY); + } + } + + @Test + public void wellFormedListTagsStillApply() throws Exception { + for (String record : new String[] { + "p=" + P + "; h=sha256 : sha1", + "p=" + P + "; h=sha512:sha1", + "p=" + P + "; k=rsa", + "p=" + P + "; t = y:n:extra"}) { + DKIMVerifier.apply(new DKIMVerifier().publicKeySelector(Collections.singletonList(record)), SHA1_SIGN); + } + } + + @Test(expected = PermFailException.class) + public void hashRestrictionStillAppliesWithATrailingColon() throws Exception { + System.setProperty(TagValue.STRICT_VALIDATION_PROPERTY, "false"); + try { + DKIMVerifier.apply(new DKIMVerifier().publicKeySelector( + Collections.singletonList("p=" + P + "; h=sha256:")), SHA1_SIGN); + } finally { + System.clearProperty(TagValue.STRICT_VALIDATION_PROPERTY); + } + } } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
