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 ad7919b28643c6556d5b932533b6c5da2360b6d8 Author: Benoit TELLIER <[email protected]> AuthorDate: Sun Nov 1 19:55:08 2026 +0000 [FIX] DKIM-Signature: reject d= and s= values that are not domain names validate() never checked d= and s=: names with an empty label, a hyphen at the edge of a label, a "_", a label over 63 octets or a key query name over 253 octets were looked up in DNS and could pass, although RFC 6376 section 3.5 requires domain-name and selector values (sub-domain labels of RFC 5321) and section 6.1.1 a PERMFAIL. With strict validation (-Djames.jdkim.strict.validation, default true), SignatureRecordImpl and SignatureRecordTemplate now reject them, so that the verifier reports a permerror without any lookup. The internal placeholder record of failures without a signature skips the check. --- docs/modules/ROOT/pages/usage.adoc | 5 ++- .../java/org/apache/james/jdkim/DKIMVerifier.java | 7 +++- .../james/jdkim/tagvalue/SignatureRecordImpl.java | 47 ++++++++++++++++++++++ .../jdkim/tagvalue/SignatureRecordTemplate.java | 1 + .../james/jdkim/SignatureRecordImplTest.java | 42 +++++++++++++++++++ .../org/apache/james/jdkim/api/ResultTest.java | 14 +++++++ 6 files changed, 113 insertions(+), 3 deletions(-) diff --git a/docs/modules/ROOT/pages/usage.adoc b/docs/modules/ROOT/pages/usage.adoc index e5467ae..8ed5523 100644 --- a/docs/modules/ROOT/pages/usage.adoc +++ b/docs/modules/ROOT/pages/usage.adoc @@ -276,8 +276,9 @@ DKIMVerifier verifier = new DKIMVerifier(new VerifierOptions.Builder() 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 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, +trailing `:` in the `h=`, `k=`, `s=` or `t=` lists of a key record, an empty +`t=`, or a DKIM-Signature `d=` or `s=` that is not a domain name (a `_`, an +empty label, a label over 63 octets). 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/DKIMVerifier.java b/main/src/main/java/org/apache/james/jdkim/DKIMVerifier.java index 47273a9..38f0b8e 100644 --- a/main/src/main/java/org/apache/james/jdkim/DKIMVerifier.java +++ b/main/src/main/java/org/apache/james/jdkim/DKIMVerifier.java @@ -557,7 +557,12 @@ public class DKIMVerifier { private Result resultFromException(String signatureField, FailException e) { SignatureRecord rec = e.getRelatedRecord(); if (rec == null) { - rec = new SignatureRecordTemplate("v=1; d=invalid; h=from; s=invalid; b=invalidsig"); + rec = new SignatureRecordTemplate("v=1; d=invalid; h=from; s=invalid; b=invalidsig") { + @Override + public void validate() { + // placeholder for a failure without a parsed signature, not a record to check + } + }; } Result.Type resultType = Result.Type.NONE; 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 897b220..076ce94 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 @@ -62,6 +62,52 @@ public class SignatureRecordImpl extends TagValue implements SignatureRecord { return hashMethod; } + // SPEC: sub-domain = Let-dig [Ldh-str] (RFC 5321), at most 63 octets (RFC 1035) + private static final Pattern SUB_DOMAIN = Pattern.compile("[A-Za-z0-9](?:[A-Za-z0-9-]{0,61}[A-Za-z0-9])?"); + private static final int MAX_DOMAIN_NAME_LENGTH = 253; + + /** + * RFC 6376 section 3.5: checks d= (domain-name = sub-domain 1*("." sub-domain)), s= (selector = + * sub-domain *("." sub-domain)) and that the key query name s._domainkey.d fits in 253 octets. + * Only with strict validation, see {@link TagValue#isStrictValidation()}. + * + * @throws IllegalStateException if they do not + */ + static void checkSdidAndSelector(CharSequence d, CharSequence selector) { + if (!isStrictValidation()) { + return; + } + checkDomainName("d=", d.toString(), 2); + checkDomainName("s=", selector.toString(), 1); + int queryNameLength = selector.length() + "._domainkey.".length() + d.length(); + if (queryNameLength > MAX_DOMAIN_NAME_LENGTH) { + throw new IllegalStateException("Key query name s._domainkey.d is " + queryNameLength + + " octets long, over " + MAX_DOMAIN_NAME_LENGTH); + } + } + + /** + * @param what the tag, for the error message + * @param name the name to check as sub-domain *("." sub-domain) + * @param minLabels the minimum number of labels + * @throws IllegalStateException if it is not a valid name + */ + static void checkDomainName(String what, String name, int minLabels) { + String[] labels = name.split("\\.", -1); + if (labels.length < minLabels) { + throw new IllegalStateException("Invalid " + what + " [" + name + "]: at least " + minLabels + " labels expected"); + } + for (String label : labels) { + if (!SUB_DOMAIN.matcher(label).matches()) { + throw new IllegalStateException("Invalid " + what + " [" + name + "]: label [" + label + + "] is not a sub-domain (RFC 6376 section 3.5)"); + } + } + if (name.length() > MAX_DOMAIN_NAME_LENGTH) { + throw new IllegalStateException("Invalid " + what + " [" + name + "]: over " + MAX_DOMAIN_NAME_LENGTH + " octets"); + } + } + /** * Checks the a= value of a signature record, see {@link #checkAlgorithm(CharSequence)}. * @@ -112,6 +158,7 @@ public class SignatureRecordImpl extends TagValue implements SignatureRecord { throw new IllegalStateException("Tag h= cannot be empty."); checkAlgorithm(getValue("a")); + checkSdidAndSelector(getValue("d"), getValue("s")); 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 b9c001f..39a09df 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 @@ -71,6 +71,7 @@ public class SignatureRecordTemplate extends TagValue implements SignatureRecord if (getValue("a") != null) { SignatureRecordImpl.checkAlgorithm(getValue("a")); } + SignatureRecordImpl.checkSdidAndSelector(getValue("d"), getValue("s")); // 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 656dab4..376fbf6 100644 --- a/main/src/test/java/org/apache/james/jdkim/SignatureRecordImplTest.java +++ b/main/src/test/java/org/apache/james/jdkim/SignatureRecordImplTest.java @@ -24,6 +24,7 @@ import org.apache.james.jdkim.tagvalue.SignatureRecordTemplate; import java.util.Locale; import org.apache.james.jdkim.api.SignatureRecord; import org.apache.james.jdkim.tagvalue.SignatureRecordImpl; +import org.apache.james.jdkim.tagvalue.TagValue; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; @@ -310,4 +311,45 @@ public class SignatureRecordImplTest { // x= after t= stays valid new SignatureRecordImpl(base + "t=" + now + "; x=" + (now + 1000)); } + + private static SignatureRecordImpl record(String d, String s) { + return new SignatureRecordImpl("v=1; a=rsa-sha256; c=relaxed/relaxed; d=" + d + "; s=" + s + + "; h=from:subject; bh=AAAA; b=AAAA"); + } + + private static final String LONG_LABEL = new String(new char[64]).replace('\0', 'a'); + private static final String LONG_NAME = new String(new char[130]).replace("\0", "a.") + "com"; + + @Test + public void malformedSdidAndSelectorAreSyntaxErrors() { + for (String[] ds : new String[][] { + {"a.", "sel"}, {".example.com", "sel"}, {"example..com", "sel"}, {"com", "sel"}, + {"-example.com", "sel"}, {"example-.com", "sel"}, {"m_ail.example.com", "sel"}, + {LONG_LABEL + ".com", "sel"}, {LONG_NAME, "sel"}, + {"example.com", "-sel"}, {"example.com", "sel."}, {"example.com", "a_b"}, + {"example.com", "a b"}, {"example.com", LONG_LABEL}}) { + assertThatThrownBy(() -> record(ds[0], ds[1])) + .as("d=%s s=%s", ds[0], ds[1]) + .isInstanceOf(IllegalStateException.class); + } + } + + @Test + public void wellFormedSdidAndSelectorAreAccepted() { + record("example.com", "sel"); + record("Mail.Example.COM", "sel-2024.a1"); + record("123.example.com", "1"); + record("xn--bcher-kva.example", "s"); + } + + @Test + public void malformedSdidAndSelectorAreAcceptedWithoutStrictValidation() { + System.setProperty(TagValue.STRICT_VALIDATION_PROPERTY, "false"); + try { + record("example.com", "google_2024"); + record("m_ail.example.com", "sel"); + } finally { + System.clearProperty(TagValue.STRICT_VALIDATION_PROPERTY); + } + } } diff --git a/main/src/test/java/org/apache/james/jdkim/api/ResultTest.java b/main/src/test/java/org/apache/james/jdkim/api/ResultTest.java index 5272525..cf877bd 100644 --- a/main/src/test/java/org/apache/james/jdkim/api/ResultTest.java +++ b/main/src/test/java/org/apache/james/jdkim/api/ResultTest.java @@ -22,10 +22,24 @@ package org.apache.james.jdkim.api; import static org.assertj.core.api.Assertions.assertThat; import org.apache.james.jdkim.tagvalue.SignatureRecordImpl; +import org.apache.james.jdkim.tagvalue.TagValue; +import org.junit.After; +import org.junit.Before; import org.junit.Test; public class ResultTest { + // the records below carry sender-controlled d= and s= values that strict validation would reject + @Before + public void disableStrictValidation() { + System.setProperty(TagValue.STRICT_VALIDATION_PROPERTY, "false"); + } + + @After + public void restoreStrictValidation() { + System.clearProperty(TagValue.STRICT_VALIDATION_PROPERTY); + } + @Test public void headerTextShouldKeepTokensUnquoted() { SignatureRecord record = new SignatureRecordImpl( --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
