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 9bb3bdbed97c91185f417ba4132c19ac69f2ca3c Author: Benoit TELLIER <[email protected]> AuthorDate: Wed Nov 4 18:21:17 2026 +0100 [FIX] SignatureRecordImpl: check the key query name without strict validation checkSdidAndSelector returned at once when strict validation was off, skipping the label syntax and the 253-octet limit along with it. Any d=/s= value then became the DNS query name s._domainkey.d: empty labels, ":", 8-bit characters, names over 253 octets. The query name has a semantic effect, and those checks are never disabled. Without strict validation the labels are now checked against a looser pattern that also accepts "_" (google_2024, m_ail.example.com), and the length limit applies in both modes. Two ResultTest cases that built records with such values now override the getters instead. --- .../james/jdkim/tagvalue/SignatureRecordImpl.java | 19 ++++++++++++------- .../james/jdkim/SignatureRecordImplTest.java | 17 +++++++++++++++++ .../org/apache/james/jdkim/api/ResultTest.java | 22 +++++++++++++++++++--- 3 files changed, 48 insertions(+), 10 deletions(-) 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 d2ee419..2e727c2 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 @@ -68,21 +68,22 @@ public class SignatureRecordImpl extends TagValue implements SignatureRecord { // 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])?"); + // Without strict validation: a DNS label that may also hold "_", as some published selectors do + private static final Pattern LENIENT_LABEL = 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()}. + * Without strict validation (see {@link TagValue#isStrictValidation()}) the labels may also hold "_", + * but the query name is still checked: it is sent to DNS. * * @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); + Pattern label = isStrictValidation() ? SUB_DOMAIN : LENIENT_LABEL; + checkDomainName("d=", d.toString(), 2, label); + checkDomainName("s=", selector.toString(), 1, label); 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 @@ -97,12 +98,16 @@ public class SignatureRecordImpl extends TagValue implements SignatureRecord { * @throws IllegalStateException if it is not a valid name */ static void checkDomainName(String what, String name, int minLabels) { + checkDomainName(what, name, minLabels, SUB_DOMAIN); + } + + private static void checkDomainName(String what, String name, int minLabels, Pattern labelPattern) { 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()) { + if (!labelPattern.matcher(label).matches()) { throw new IllegalStateException("Invalid " + what + " [" + name + "]: label [" + label + "] is not a sub-domain (RFC 6376 section 3.5)"); } 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 d213ee0..c7c98c2 100644 --- a/main/src/test/java/org/apache/james/jdkim/SignatureRecordImplTest.java +++ b/main/src/test/java/org/apache/james/jdkim/SignatureRecordImplTest.java @@ -353,6 +353,23 @@ public class SignatureRecordImplTest { } } + @Test + public void sdidAndSelectorMustStillFormADnsNameWithoutStrictValidation() { + System.setProperty(TagValue.STRICT_VALIDATION_PROPERTY, "false"); + try { + for (String[] ds : new String[][] { + {"example.com", ":"}, {"example.com", "a..b"}, {"example.com", "a b"}, + {"example.com", LONG_LABEL}, {"example..com", "sel"}, {"com", "sel"}, + {"example.com", repeat("a.", 120) + "a"}}) { + assertThatThrownBy(() -> record(ds[0], ds[1])) + .as("d=%s s=%s", ds[0], ds[1]) + .isInstanceOf(IllegalStateException.class); + } + } finally { + System.clearProperty(TagValue.STRICT_VALIDATION_PROPERTY); + } + } + private static SignatureRecordImpl recordWithIdentity(String i) { return new SignatureRecordImpl("v=1; a=rsa-sha256; c=relaxed/relaxed; d=example.com; s=sel;" + " h=from:subject; i=" + i + "; bh=AAAA; b=AAAA"); 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 cf877bd..71f3751 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 @@ -29,7 +29,7 @@ import org.junit.Test; public class ResultTest { - // the records below carry sender-controlled d= and s= values that strict validation would reject + // the records below carry sender-controlled values that strict validation would reject @Before public void disableStrictValidation() { System.setProperty(TagValue.STRICT_VALIDATION_PROPERTY, "false"); @@ -52,8 +52,14 @@ public class ResultTest { @Test public void headerTextShouldQuoteSenderValues() { + // even without strict validation, s= must be a DNS name: another implementation may not check it SignatureRecord record = new SignatureRecordImpl( - "v=1; a=rsa-sha256; d=example.com; s=s\"el; h=from; bh=AAAA; b=AA\"A"); + "v=1; a=rsa-sha256; d=example.com; s=sel; h=from; bh=AAAA; b=AA\"A") { + @Override + public CharSequence getSelector() { + return "s\"el"; + } + }; Result result = new Result("Invalid DKIM-Signature version (expected '1'): 1\" x=\"", "", record, Result.Type.FAIL); assertThat(result.getHeaderText()).isEqualTo("dkim=fail header.d=example.com header.s=\"s\\\"el\" header.b=\"AA\\\"A\""); @@ -62,7 +68,17 @@ public class ResultTest { @Test public void headerTextShouldQuoteEmptyValues() { - SignatureRecord record = new SignatureRecordImpl("v=1; a=rsa-sha256; d=; s=; h=from; bh=AAAA; b="); + SignatureRecord record = new SignatureRecordImpl("v=1; a=rsa-sha256; d=example.com; s=sel; h=from; bh=AAAA; b=") { + @Override + public CharSequence getDToken() { + return ""; + } + + @Override + public CharSequence getSelector() { + return ""; + } + }; Result result = new Result("error", "", record, Result.Type.FAIL); assertThat(result.getHeaderText()).isEqualTo("dkim=fail header.d=\"\" header.s=\"\" header.b=\"\""); --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
