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 dc434c3d5679c6907f521b8d6e7b95e46555280d Author: Benoit TELLIER <[email protected]> AuthorDate: Sun Nov 1 19:45:23 2026 +0000 [FIX] DKIMVerifier: honour the s= service type of the key record s= was never read: a key published for another service only (s=chat, s=web) validated e-mail signatures, although RFC 6376 section 3.6.1 says verifiers MUST ignore a record that does not list their service type. PublicKeyRecord gains isServiceTypeSupported, and apply rejects a key whose s= lists neither email nor * as inapplicable. A malformed s= list (s=:email) is a key syntax error. Syntax checks of tag values can be disabled with -Djames.jdkim.strict.validation=false (default true), so that slightly malformed real-world records keep working; the s=chat check stays. --- docs/modules/ROOT/pages/usage.adoc | 11 ++++ .../java/org/apache/james/jdkim/DKIMVerifier.java | 5 ++ .../apache/james/jdkim/api/PublicKeyRecord.java | 10 ++++ .../james/jdkim/tagvalue/PublicKeyRecordImpl.java | 33 ++++++++++++ .../org/apache/james/jdkim/tagvalue/TagValue.java | 14 ++++++ .../apache/james/jdkim/PublicKeyRecordTest.java | 58 ++++++++++++++++++++++ 6 files changed, 131 insertions(+) diff --git a/docs/modules/ROOT/pages/usage.adoc b/docs/modules/ROOT/pages/usage.adoc index f68dff4..10e37a8 100644 --- a/docs/modules/ROOT/pages/usage.adoc +++ b/docs/modules/ROOT/pages/usage.adoc @@ -256,6 +256,17 @@ DKIMVerifier verifier = new DKIMVerifier(new VerifierOptions.Builder() .build()); ---- +[[strict-validation]] +=== 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, +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 +signature, an unsupported algorithm -- are always applied. + == Applying several signatures Signing with more than one key lets you offer several algorithms on one 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 6caee53..1d4c85f 100644 --- a/main/src/main/java/org/apache/james/jdkim/DKIMVerifier.java +++ b/main/src/main/java/org/apache/james/jdkim/DKIMVerifier.java @@ -134,6 +134,11 @@ public class DKIMVerifier { */ public static void apply(PublicKeyRecord pkr, SignatureRecord sign) throws PermFailException { try { + // RFC 6376 section 3.6.1: verifiers MUST ignore a key whose s= does not list their service type. + if (!pkr.isServiceTypeSupported("email")) { + throw new PermFailException("inapplicable key: service type (s=) does not include email", sign); + } + if (!pkr.getGranularityPattern().matcher(sign.getIdentityLocalPart()) .matches()) { throw new PermFailException("inapplicable key identity local=" diff --git a/main/src/main/java/org/apache/james/jdkim/api/PublicKeyRecord.java b/main/src/main/java/org/apache/james/jdkim/api/PublicKeyRecord.java index 746c7ba..da8f585 100644 --- a/main/src/main/java/org/apache/james/jdkim/api/PublicKeyRecord.java +++ b/main/src/main/java/org/apache/james/jdkim/api/PublicKeyRecord.java @@ -33,6 +33,16 @@ public interface PublicKeyRecord { boolean isKeyTypeSupported(CharSequence hash); + /** + * RFC 6376 section 3.6.1, s=: whether the key may be used for the given service type. + * + * @param serviceType a service type, such as "email" + * @return true if s= lists the type or "*" + */ + default boolean isServiceTypeSupported(CharSequence serviceType) { + return true; + } + /** * @return null if "any", otherwise a list of supported methods */ 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 ff2739e..1c9dc60 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 @@ -46,6 +46,10 @@ public class PublicKeyRecordImpl extends TagValue implements PublicKeyRecord { private static final Pattern hyphenatedWordPattern = Pattern .compile("^[a-zA-Z]([a-zA-Z0-9-]*[a-zA-Z0-9])?$"); + // SPEC: key-s-tag-type = "email" / "*" / x-key-s-tag-type ; x-key-s-tag-type = hyphenated-word + private static final Pattern serviceTypePattern = Pattern + .compile("^(\\*|[a-zA-Z]([a-zA-Z0-9-]*[a-zA-Z0-9])?)$"); + public PublicKeyRecordImpl(String data) { super(data); } @@ -83,6 +87,35 @@ public class PublicKeyRecordImpl extends TagValue implements PublicKeyRecord { "Unknown version for v= (expected DKIM1): " + getValue("v")); 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) + getServiceTypes(); + } + } + + /** + * RFC 6376 section 3.6.1, s=: the service types the key may be used for. + * + * @return the listed types; with strict validation, an {@link IllegalStateException} on a malformed list + */ + public List<CharSequence> getServiceTypes() { + String s = getValue("s").toString(); + if (isStrictValidation()) { + return stringToColonSeparatedList(s, serviceTypePattern); + } + List<CharSequence> res = new ArrayList<>(); + for (String type : s.split(":")) { + res.add(trimFWS(type, 0, type.length() - 1, true)); + } + return res; + } + + /** + * @see org.apache.james.jdkim.api.PublicKeyRecord#isServiceTypeSupported(java.lang.CharSequence) + */ + public boolean isServiceTypeSupported(CharSequence serviceType) { + List<CharSequence> types = getServiceTypes(); + return isInListCaseInsensitive("*", types) || isInListCaseInsensitive(serviceType, types); } /** 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 3782c4e..b4e6189 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 @@ -37,6 +37,20 @@ public class TagValue { private static final boolean DEBUG = false; protected static final boolean VALIDATION = true; + /** + * System property disabling the strict syntax checks of tag values (RFC 6376 section 3.5 and 3.6.1) + * when set to false: a verifier stricter than the others may reject real-world, slightly malformed records. + * Checks with a semantic effect are never disabled. Defaults to true. + */ + public static final String STRICT_VALIDATION_PROPERTY = "james.jdkim.strict.validation"; + + /** + * @return whether the strict syntax checks are enabled, see {@link #STRICT_VALIDATION_PROPERTY} + */ + public static boolean isStrictValidation() { + return !"false".equalsIgnoreCase(System.getProperty(STRICT_VALIDATION_PROPERTY, "true").trim()); + } + private static final Pattern tagPattern = Pattern .compile("^[A-Za-z][A-Za-z0-9_]*$"); // RFC 6376 section 3.2: tval = 1*VALCHAR, VALCHAR = %x21-3A / %x3C-7E 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 db5b1aa..fd43805 100644 --- a/main/src/test/java/org/apache/james/jdkim/PublicKeyRecordTest.java +++ b/main/src/test/java/org/apache/james/jdkim/PublicKeyRecordTest.java @@ -23,7 +23,11 @@ import java.security.KeyPairGenerator; import static org.assertj.core.api.Assertions.assertThatThrownBy; import java.util.Base64; import org.apache.james.jdkim.api.PublicKeyRecord; +import org.apache.james.jdkim.api.SignatureRecord; +import org.apache.james.jdkim.exceptions.PermFailException; import org.apache.james.jdkim.tagvalue.PublicKeyRecordImpl; +import org.apache.james.jdkim.tagvalue.SignatureRecordImpl; +import org.apache.james.jdkim.tagvalue.TagValue; import org.junit.Assert; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -34,6 +38,7 @@ import org.junit.Test; import java.math.BigInteger; import java.security.PublicKey; import java.security.interfaces.RSAKey; +import java.util.Collections; import java.util.List; import java.util.regex.Pattern; @@ -288,4 +293,57 @@ public class PublicKeyRecordTest { assertThatThrownBy(new PublicKeyRecordImpl("v=DKIM1; k=rsa; p=" + ec)::getPublicKey) .isInstanceOf(IllegalStateException.class); } + + private static final String P = "MFwwDQYJKoZIhvcNAQEBBQADSwAwSAJBALgSoqXVSEmfcIsOzw7oRuCCOwsmtX/SJnTWxYyj2leFxfS/AVJ+dYfY+hXqMsT7l+MZvvh/R1WzN4MO/kI/7XsCAwEAAQ=="; + private static final SignatureRecord SIGN = new SignatureRecordImpl( + "v=1; a=rsa-sha256; d=example.com; s=sel; h=from:to; bh=AAAA; b=AAAA"); + + private static void applyRecord(String record) throws PermFailException { + PublicKeyRecord pk = new DKIMVerifier().publicKeySelector(Collections.singletonList(record)); + DKIMVerifier.apply(pk, SIGN); + } + + @Test + public void keyRestrictedToAnotherServiceMustNotApplyToEmail() { + for (String s : new String[] {"chat", "web", "web:chat"}) { + try { + applyRecord("v=DKIM1; s=" + s + "; p=" + P); + fail("accepted s=" + s); + } catch (PermFailException expected) { + } + } + } + + @Test(expected = PermFailException.class) + public void malformedServiceTypeListIsRejected() throws Exception { + applyRecord("s=:email; p=" + P); + } + + @Test + public void malformedServiceTypeListIsAcceptedWithoutStrictValidation() throws Exception { + System.setProperty(TagValue.STRICT_VALIDATION_PROPERTY, "false"); + try { + applyRecord("s=:email; p=" + P); + } finally { + System.clearProperty(TagValue.STRICT_VALIDATION_PROPERTY); + } + } + + @Test(expected = PermFailException.class) + public void otherServiceIsRejectedWithoutStrictValidation() throws Exception { + System.setProperty(TagValue.STRICT_VALIDATION_PROPERTY, "false"); + try { + applyRecord("s=chat; p=" + P); + } finally { + System.clearProperty(TagValue.STRICT_VALIDATION_PROPERTY); + } + } + + @Test + public void emailAndWildcardServiceTypesApply() throws Exception { + applyRecord("p=" + P); + for (String s : new String[] {"*", "email", "web:email", "EMAIL", "email:web:fine", "web : email"}) { + applyRecord("s=" + s + "; p=" + P); + } + } } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
