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 5495cf984c8860cc04a55f0d4b283419c3ec8019 Author: Benoit TELLIER <[email protected]> AuthorDate: Sun Nov 1 19:43:55 2026 +0000 [FIX] DKIMVerifier: report permanent errors as permerror, not fail resultsFromExceptions reported every PermFailException related to a signature record as FAIL. As nearly all of them get one, a missing, revoked, malformed or inapplicable key, an unsupported a=, c= or q=, or a t= in the future were all reported as dkim=fail, like a forged or altered message. The two cases where the signature does not verify (body hash mismatch, header signature mismatch, including a b= that does not fit the key) now throw SignatureMismatchException, a PermFailException subclass, and are the only ones reported as FAIL (RFC 6376 section 6.1, RFC 8601 section 2.7.1). Every other PermFailException becomes PERMERROR. --- docs/modules/ROOT/pages/usage.adoc | 5 ++ .../java/org/apache/james/jdkim/DKIMVerifier.java | 21 ++++---- .../exceptions/SignatureMismatchException.java | 40 +++++++++++++++ .../test/java/org/apache/james/jdkim/DKIMTest.java | 60 ++++++++++++++++++++++ .../org/apache/james/jdkim/DKIMVerifierTest.java | 3 +- 5 files changed, 119 insertions(+), 10 deletions(-) diff --git a/docs/modules/ROOT/pages/usage.adoc b/docs/modules/ROOT/pages/usage.adoc index 452eb5a..f68dff4 100644 --- a/docs/modules/ROOT/pages/usage.adoc +++ b/docs/modules/ROOT/pages/usage.adoc @@ -166,6 +166,11 @@ for (Result result : verifier.getResults()) { The result types follow https://datatracker.ietf.org/doc/html/rfc8601#section-2.7.1[RFC 8601 section 2.7.1], so `getHeaderText()` can feed an `Authentication-Results` header directly. +`FAIL` means the signature does not verify (body hash or header signature +mismatch, `SignatureMismatchException`); `TEMPERROR` a temporary key lookup +failure; `PERMERROR` any other permanent error: a malformed or unsupported +signature, a signature dated in the future or expired, a key that is missing, +revoked, malformed or does not apply. For a DMARC-style verdict -- a message passes when at least one signature is valid -- use: 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 3c501c3..6caee53 100644 --- a/main/src/main/java/org/apache/james/jdkim/DKIMVerifier.java +++ b/main/src/main/java/org/apache/james/jdkim/DKIMVerifier.java @@ -32,6 +32,7 @@ import org.apache.james.jdkim.api.VerifierOptions; import org.apache.james.jdkim.exceptions.CompositeFailException; import org.apache.james.jdkim.exceptions.FailException; import org.apache.james.jdkim.exceptions.PermFailException; +import org.apache.james.jdkim.exceptions.SignatureMismatchException; import org.apache.james.jdkim.exceptions.TempFailException; import org.apache.james.jdkim.impl.BodyHasherImpl; import org.apache.james.jdkim.impl.CompoundBodyHasher; @@ -487,7 +488,7 @@ public class DKIMVerifier { byte[] expectedBodyHash = bhj.getSignatureRecord().getBodyHash(); if (!Arrays.equals(expectedBodyHash, computedHash)) { - job.setFailure(new PermFailException( + job.setFailure(new SignatureMismatchException( "Computed bodyhash is different from the expected one", bhj.getSignatureRecord())); } else { verifiedSignatures.add(bhj.getSignatureRecord()); @@ -549,13 +550,12 @@ public class DKIMVerifier { Result.Type resultType = Result.Type.NONE; if (e instanceof TempFailException) { resultType = Result.Type.TEMPERROR; + } else if (e instanceof SignatureMismatchException) { + // RFC 6376 section 6.1, RFC 8601 section 2.7.1: fail only when the signature does not verify. + resultType = Result.Type.FAIL; } else if (e instanceof PermFailException) { - if (e.getRelatedRecord() == null) { - //FailException without the SignatureRecord - resultType = Result.Type.PERMERROR; - } else { - resultType = Result.Type.FAIL; - } + // malformed or unsupported signature, missing, revoked or inapplicable key... + resultType = Result.Type.PERMERROR; } return new Result(e.getMessage(), signatureField != null ? signatureField : "", rec, resultType); } @@ -607,8 +607,11 @@ public class DKIMVerifier { signatureCheck(h, sign, headers, signature, signatureFieldName); if (!signature.verify(decoded)) - throw new PermFailException("Header signature does not verify", sign); - } catch (InvalidKeyException | NoSuchAlgorithmException | SignatureException e) { + throw new SignatureMismatchException("Header signature does not verify", sign); + } catch (SignatureException e) { + // e.g. a b= value whose length does not fit the key: the signature does not verify against it. + throw new SignatureMismatchException("Header signature does not verify: " + e.getMessage(), sign, e); + } catch (InvalidKeyException | NoSuchAlgorithmException e) { throw new PermFailException(e.getMessage(), sign, e); } } diff --git a/main/src/main/java/org/apache/james/jdkim/exceptions/SignatureMismatchException.java b/main/src/main/java/org/apache/james/jdkim/exceptions/SignatureMismatchException.java new file mode 100644 index 0000000..a1a276a --- /dev/null +++ b/main/src/main/java/org/apache/james/jdkim/exceptions/SignatureMismatchException.java @@ -0,0 +1,40 @@ +/**************************************************************** + * Licensed to the Apache Software Foundation (ASF) under one * + * or more contributor license agreements. See the NOTICE file * + * distributed with this work for additional information * + * regarding copyright ownership. The ASF licenses this file * + * to you under the Apache License, Version 2.0 (the * + * "License"); you may not use this file except in compliance * + * with the License. You may obtain a copy of the License at * + * * + * http://www.apache.org/licenses/LICENSE-2.0 * + * * + * Unless required by applicable law or agreed to in writing, * + * software distributed under the License is distributed on an * + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY * + * KIND, either express or implied. See the License for the * + * specific language governing permissions and limitations * + * under the License. * + ****************************************************************/ + +package org.apache.james.jdkim.exceptions; + +import org.apache.james.jdkim.api.SignatureRecord; + +/** + * The signature does not verify: the body hash or the header signature differs from the computed one. + * This is the only permanent failure reported as dkim=fail (RFC 6376 section 6.1, RFC 8601 section 2.7.1); + * the other {@link PermFailException}s are reported as dkim=permerror. + */ +public class SignatureMismatchException extends PermFailException { + + private static final long serialVersionUID = -4617385071946623147L; + + public SignatureMismatchException(String error, SignatureRecord signatureRecord) { + super(error, signatureRecord); + } + + public SignatureMismatchException(String error, SignatureRecord signatureRecord, Exception e) { + super(error, signatureRecord, e); + } +} diff --git a/main/src/test/java/org/apache/james/jdkim/DKIMTest.java b/main/src/test/java/org/apache/james/jdkim/DKIMTest.java index 76f443e..a1fc67b 100644 --- a/main/src/test/java/org/apache/james/jdkim/DKIMTest.java +++ b/main/src/test/java/org/apache/james/jdkim/DKIMTest.java @@ -602,4 +602,64 @@ public class DKIMTest { assertThat(subjectOnlyVerifier.verify(new ByteArrayInputStream(replayed.getBytes(StandardCharsets.US_ASCII)))).hasSize(1); } + + private static final String KEY = Base64.getEncoder().encodeToString(TestKeys.publicKey.getEncoded()); + + private Result.Type resultTypeWithKeyRecord(String signatureTemplate, String keyRecord, boolean alterBody) throws Exception { + MockPublicKeyRecordRetriever retriever = keyRecord == null + ? new MockPublicKeyRecordRetriever() + : new MockPublicKeyRecordRetriever(Record.of("selector2", "messiah.edu", keyRecord)); + DKIMVerifier verifier = new DKIMVerifier(new VerifierOptions.Builder().withPublicKeyRecordRetriever(retriever).build()); + + String message = "From: [email protected]\r\nSubject: test\r\n\r\nbody\r\n"; + String signature = new DKIMSigner(signatureTemplate, TestKeys.privateKey) + .sign(new ByteArrayInputStream(message.getBytes(StandardCharsets.US_ASCII))); + String signed = signature + "\r\n" + (alterBody ? message.replace("body", "b0dy") : message); + + assertThatThrownBy(() -> verifier.verify(new ByteArrayInputStream(signed.getBytes(StandardCharsets.US_ASCII)))) + .isInstanceOf(PermFailException.class); + assertThat(verifier.getResults()).hasSize(1); + return verifier.getResults().get(0).getResultType(); + } + + @Test + public void unusableKeysShouldBeReportedAsPermerror() throws Exception { + String[] keyRecords = { + null, + "v=DKIM1; k=rsa; p=", + "v=DKIM2; k=rsa; p=" + KEY, + "v=DKIM1; k=rsa; p=AAAA", + "v=DKIM1; k=ec; p=" + KEY, + "v=DKIM1; k=rsa; h=sha1; p=" + KEY}; + for (String keyRecord : keyRecords) { + assertThat(resultTypeWithKeyRecord(SIGNATURE_TEMPLATE, keyRecord, false)) + .as("key record %s", keyRecord) + .isEqualTo(Result.Type.PERMERROR); + } + } + + @Test + public void keyRestrictedToTheSdidShouldBeReportedAsPermerrorForASubdomainIdentity() throws Exception { + String template = "v=1; a=rsa-sha256; c=simple; d=messiah.edu; [email protected]; h=from:subject; q=dns/txt; s=selector2;"; + + assertThat(resultTypeWithKeyRecord(template, "v=DKIM1; k=rsa; t=s; p=" + KEY, false)) + .isEqualTo(Result.Type.PERMERROR); + } + + @Test + public void bodyChangedAfterSigningShouldStillBeReportedAsFail() throws Exception { + assertThat(resultTypeWithKeyRecord(SIGNATURE_TEMPLATE, "v=DKIM1; k=rsa; p=" + KEY, true)) + .isEqualTo(Result.Type.FAIL); + } + + @Test + public void headerChangedAfterSigningShouldStillBeReportedAsFail() throws Exception { + String message = "From: [email protected]\r\nSubject: test\r\n\r\nbody\r\n"; + String signature = dkimSigner.sign(new ByteArrayInputStream(message.getBytes(StandardCharsets.US_ASCII))); + String altered = signature + "\r\n" + message.replace("Subject: test", "Subject: tost"); + + assertThatThrownBy(() -> verifier.verify(new ByteArrayInputStream(altered.getBytes(StandardCharsets.US_ASCII)))) + .isInstanceOf(PermFailException.class); + assertThat(verifier.getResults()).extracting(Result::getResultType).containsExactly(Result.Type.FAIL); + } } diff --git a/main/src/test/java/org/apache/james/jdkim/DKIMVerifierTest.java b/main/src/test/java/org/apache/james/jdkim/DKIMVerifierTest.java index 62ae643..ba5651a 100644 --- a/main/src/test/java/org/apache/james/jdkim/DKIMVerifierTest.java +++ b/main/src/test/java/org/apache/james/jdkim/DKIMVerifierTest.java @@ -150,7 +150,8 @@ public class DKIMVerifierTest { .isInstanceOf(FailException.class); assertThat(calls.get()).isEqualTo(10); assertThat(verifier.getResults()).hasSize(200); - assertThat(verifier.getResults().stream().filter(r -> r.getResultType() == Result.Type.PERMERROR)).hasSize(190); + assertThat(verifier.getResults()).allMatch(r -> r.getResultType() == Result.Type.PERMERROR); + assertThat(verifier.getResults().stream().filter(r -> r.getErrorMessage().startsWith("Too many DKIM-Signature fields"))).hasSize(190); } @Test --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
