This is an automated email from the ASF dual-hosted git repository. chibenwa pushed a commit to branch refine-header-canonicalization in repository https://gitbox.apache.org/repos/asf/james-jdkim.git
commit dd81a82f811fc25ddf39b4df8c5b321ef375cceb Author: Benoit TELLIER <[email protected]> AuthorDate: Wed Oct 28 22:01:57 2026 +0100 [ENHANCEMENT] Improve non-utf8 header char handling --- .../java/org/apache/james/jdkim/DKIMCommon.java | 19 ++++++++---- .../java/org/apache/james/jdkim/api/Headers.java | 10 +++++++ .../java/org/apache/james/jdkim/impl/Message.java | 4 ++- .../test/java/org/apache/james/jdkim/DKIMTest.java | 34 ++++++++++++++++++++++ 4 files changed, 61 insertions(+), 6 deletions(-) diff --git a/main/src/main/java/org/apache/james/jdkim/DKIMCommon.java b/main/src/main/java/org/apache/james/jdkim/DKIMCommon.java index 853cdd1..919a679 100644 --- a/main/src/main/java/org/apache/james/jdkim/DKIMCommon.java +++ b/main/src/main/java/org/apache/james/jdkim/DKIMCommon.java @@ -26,6 +26,7 @@ import org.apache.james.jdkim.exceptions.PermFailException; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.nio.charset.StandardCharsets; import java.security.Signature; import java.security.SignatureException; import java.util.HashMap; @@ -37,6 +38,14 @@ public abstract class DKIMCommon { private static final boolean DEEP_DEBUG = false; + /** + * Header fields are carried as Strings where each char is one raw octet + * (see {@link Headers}): ISO-8859-1 gives them back unchanged. + */ + private static byte[] octets(String s) { + return s.getBytes(StandardCharsets.ISO_8859_1); + } + protected static void updateSignature(Signature signature, boolean relaxed, CharSequence header, @@ -45,17 +54,17 @@ public abstract class DKIMCommon { if (DEEP_DEBUG) System.out .println("#" + header.toString().toLowerCase() + ":-"); - signature.update(header.toString().toLowerCase().getBytes()); - signature.update(":".getBytes()); + signature.update(octets(header.toString().toLowerCase(Locale.US))); + signature.update(octets(":")); String headerValue = fv.substring(fv.indexOf(':') + 1); headerValue = headerValue.replaceAll("\r\n[\t ]", " "); headerValue = headerValue.replaceAll("[\t ]+", " "); headerValue = headerValue.trim(); - signature.update(headerValue.getBytes()); + signature.update(octets(headerValue)); if (DEEP_DEBUG) System.out.println("#" + headerValue + "#"); } else { - signature.update(fv.getBytes()); + signature.update(octets(fv)); if (DEEP_DEBUG) System.out.println("#" + fv + "#"); } @@ -89,7 +98,7 @@ public abstract class DKIMCommon { if (doneHeaders <= hl.size()) { String fv = hl.get(hl.size() - doneHeaders); updateSignature(signature, relaxedHeaders, header, fv); - signature.update("\r\n".getBytes()); + signature.update(octets("\r\n")); processedHeader.put(lowerCaseHeader, doneHeaders); } } diff --git a/main/src/main/java/org/apache/james/jdkim/api/Headers.java b/main/src/main/java/org/apache/james/jdkim/api/Headers.java index 3e07cff..b7d0106 100644 --- a/main/src/main/java/org/apache/james/jdkim/api/Headers.java +++ b/main/src/main/java/org/apache/james/jdkim/api/Headers.java @@ -21,6 +21,16 @@ package org.apache.james.jdkim.api; import java.util.List; +/** + * Raw header fields of a message, as used for DKIM header canonicalization. + * <p> + * Each returned field is the raw field (name, colon and value, folding + * included, without the trailing CRLF) where every char is one octet of the + * message, as decoded with ISO-8859-1. Implementations must not decode the + * field with any other charset: canonicalization works on the raw octets + * (RFC 6376 section 3.4) and a lossy decoding would let signed bytes be + * altered without breaking the signature. + */ public interface Headers { /** diff --git a/main/src/main/java/org/apache/james/jdkim/impl/Message.java b/main/src/main/java/org/apache/james/jdkim/impl/Message.java index 8e84063..b41de81 100644 --- a/main/src/main/java/org/apache/james/jdkim/impl/Message.java +++ b/main/src/main/java/org/apache/james/jdkim/impl/Message.java @@ -33,6 +33,7 @@ import org.apache.james.mime4j.stream.MimeConfig; import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; +import java.nio.charset.StandardCharsets; import java.util.LinkedList; import java.util.List; @@ -100,8 +101,9 @@ public class Message implements Headers { try { mw.writeField(f, bos); // writeField always ends with CRLF and we don't want it. + // ISO-8859-1 maps each raw octet to one char, losslessly. byte[] fieldbytes = bos.toByteArray(); - field = new String(fieldbytes, 0, fieldbytes.length - 2); + field = new String(fieldbytes, 0, fieldbytes.length - 2, StandardCharsets.ISO_8859_1); } catch (IOException ignored) { } res2.add(field); 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 09f7441..5a176eb 100644 --- a/main/src/test/java/org/apache/james/jdkim/DKIMTest.java +++ b/main/src/test/java/org/apache/james/jdkim/DKIMTest.java @@ -20,6 +20,7 @@ package org.apache.james.jdkim; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.junit.Assert.assertEquals; import java.io.ByteArrayInputStream; @@ -41,6 +42,7 @@ import org.apache.james.jdkim.api.Headers; import org.apache.james.jdkim.api.Result; import org.apache.james.jdkim.api.SignatureRecord; import org.apache.james.jdkim.api.VerifierOptions; +import org.apache.james.jdkim.exceptions.PermFailException; import org.apache.james.jdkim.impl.Message; import org.junit.Test; @@ -146,6 +148,38 @@ public class DKIMTest { } + @Test + public void nonUtf8BytesOfASignedHeaderShouldVerify() throws Exception { + byte[] mail = "From: [email protected]\r\nSubject: café\r\n\r\nbody\r\n".getBytes(StandardCharsets.ISO_8859_1); + String field = dkimSigner.sign(new ByteArrayInputStream(mail)); + + InputStream in = new SequenceInputStream( + new ByteArrayInputStream((field + "\r\n").getBytes(StandardCharsets.US_ASCII)), new ByteArrayInputStream(mail)); + + assertThat(verifier.verify(in)).hasSize(1); + } + + @Test + public void alteringNonUtf8BytesOfASignedHeaderShouldBreakTheSignature() throws Exception { + assertTamperingNonUtf8HeaderBytesIsDetected(dkimSigner); + } + + @Test + public void alteringNonUtf8BytesOfASignedHeaderShouldBreakTheRelaxedSignature() throws Exception { + DKIMSigner relaxedSigner = new DKIMSigner(SIGNATURE_TEMPLATE.replace("c=simple", "c=relaxed"), TestKeys.privateKey); + assertTamperingNonUtf8HeaderBytesIsDetected(relaxedSigner); + } + + private void assertTamperingNonUtf8HeaderBytesIsDetected(DKIMSigner signer) throws Exception { + byte[] mail = "From: [email protected]\r\nSubject: café\r\n\r\nbody\r\n".getBytes(StandardCharsets.ISO_8859_1); + byte[] tampered = "From: [email protected]\r\nSubject: cafè\r\n\r\nbody\r\n".getBytes(StandardCharsets.ISO_8859_1); + String field = signer.sign(new ByteArrayInputStream(mail)); + + InputStream in = new SequenceInputStream( + new ByteArrayInputStream((field + "\r\n").getBytes(StandardCharsets.US_ASCII)), new ByteArrayInputStream(tampered)); + + assertThatThrownBy(() -> verifier.verify(in)).isInstanceOf(PermFailException.class); + } private ByteArrayInputStream readFileToByteArrayInputStream(String fileName) throws URISyntaxException, IOException { URL resource = this.getClass().getResource(fileName); --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
