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 ed277c9020dde087cb0671e1105170baf5e1a333 Author: Benoit TELLIER <[email protected]> AuthorDate: Fri Oct 30 08:47:50 2026 +0100 [FIX] Message: take header fields from the raw header bytes Fields came from mime4j, which trims control characters from names, removes comments from them and drops the fields whose name it cannot parse: "\0From:" counted as From in h=, "(c) Subject:" counted as Subject, and fields such as "(x: y" were missing, so they could be altered under a valid signature. Message now records the header block as mime4j reads it, splits it into fields itself and matches h= entries on the raw name (the text before the colon, without trailing WSP), case-insensitively. mime4j still parses the body and enforces the size limits. Co-Authored-By: Claude Opus 5.5 <[email protected]> --- .../jdkim/impl/HeaderCapturingInputStream.java | 98 ++++++++++++++++++++ .../java/org/apache/james/jdkim/impl/Message.java | 103 ++++++++++++--------- .../test/java/org/apache/james/jdkim/DKIMTest.java | 46 +++++++++ 3 files changed, 205 insertions(+), 42 deletions(-) diff --git a/main/src/main/java/org/apache/james/jdkim/impl/HeaderCapturingInputStream.java b/main/src/main/java/org/apache/james/jdkim/impl/HeaderCapturingInputStream.java new file mode 100644 index 0000000..efe61ea --- /dev/null +++ b/main/src/main/java/org/apache/james/jdkim/impl/HeaderCapturingInputStream.java @@ -0,0 +1,98 @@ +/**************************************************************** + * 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.impl; + +import java.io.ByteArrayOutputStream; +import java.io.FilterInputStream; +import java.io.IOException; +import java.io.InputStream; + +/** + * Records the bytes of the header block (up to the empty line that ends it) + * while they are read, so that the header fields can be taken from the raw + * bytes instead of from the fields the MIME parser kept. + * <p> + * Expects CRLF line endings. + */ +class HeaderCapturingInputStream extends FilterInputStream { + + private final ByteArrayOutputStream header = new ByteArrayOutputStream(); + // number of bytes of the "\r\n\r\n" terminator seen so far + private int matched = 2; + private boolean complete; + + HeaderCapturingInputStream(InputStream in) { + super(in); + } + + @Override + public int read() throws IOException { + int b = super.read(); + if (b != -1) { + capture((byte) b); + } + return b; + } + + @Override + public int read(byte[] b, int off, int len) throws IOException { + int read = super.read(b, off, len); + for (int i = 0; i < read && !complete; i++) { + capture(b[off + i]); + } + return read; + } + + @Override + public long skip(long n) throws IOException { + // read the skipped bytes so that none of the header is missed + byte[] buffer = new byte[(int) Math.min(n, 2048)]; + int read = read(buffer, 0, buffer.length); + return Math.max(read, 0); + } + + @Override + public boolean markSupported() { + return false; + } + + private void capture(byte b) { + if (complete) { + return; + } + header.write(b); + // the header block starts right after a (virtual) CRLF + if (b == "\r\n\r\n".charAt(matched)) { + matched++; + } else { + matched = b == '\r' ? 1 : 0; + } + if (matched == 4) { + complete = true; + } + } + + /** + * @return the header block read so far, the terminating empty line included + */ + byte[] getHeader() { + return header.toByteArray(); + } +} 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 d59ca40..ae2cb93 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 @@ -24,13 +24,10 @@ import org.apache.james.mime4j.MimeException; import org.apache.james.mime4j.MimeIOException; import org.apache.james.mime4j.dom.MessageBuilder; import org.apache.james.mime4j.dom.MessageServiceFactory; -import org.apache.james.mime4j.dom.MessageWriter; import org.apache.james.mime4j.dom.SingleBody; import org.apache.james.mime4j.io.EOLConvertingInputStream; -import org.apache.james.mime4j.stream.Field; import org.apache.james.mime4j.stream.MimeConfig; -import java.io.ByteArrayOutputStream; import java.io.IOException; import java.io.InputStream; import java.nio.charset.StandardCharsets; @@ -47,9 +44,8 @@ import java.util.Map; public class Message implements Headers { private final org.apache.james.mime4j.dom.Message message; - private final MessageServiceFactory messageServiceFactory; - private List<String> fields; - private Map<String, List<String>> fieldsByName; + private final List<String> fields; + private final Map<String, List<String>> fieldsByName; /** * Creates a new <code>Header</code> from the specified stream. @@ -59,10 +55,21 @@ public class Message implements Headers { * @throws MimeIOException on MIME protocol violations. */ public Message(InputStream is) throws IOException, MimeException { - this.messageServiceFactory = newMessageBuilder(); - MessageBuilder mb = messageServiceFactory.newMessageBuilder(); + MessageBuilder mb = newMessageBuilder().newMessageBuilder(); - this.message = mb.parseMessage(new EOLConvertingInputStream(is)); + // mime4j drops the fields whose name it cannot parse and normalizes the + // others: the fields are taken from the raw header bytes instead. + HeaderCapturingInputStream header = new HeaderCapturingInputStream(new EOLConvertingInputStream(is)); + this.message = mb.parseMessage(header); + + List<String> all = splitFields(new String(header.getHeader(), StandardCharsets.ISO_8859_1)); + Map<String, List<String>> byName = new HashMap<>(); + for (String field : all) { + byName.computeIfAbsent(rawName(field), name -> new ArrayList<>()).add(field); + } + byName.replaceAll((name, list) -> Collections.unmodifiableList(list)); + this.fields = Collections.unmodifiableList(all); + this.fieldsByName = byName; } private MessageServiceFactory newMessageBuilder() throws MimeException { @@ -79,6 +86,50 @@ public class Message implements Headers { return mbf; } + /** + * Splits a CRLF-terminated header block into raw fields: a line starting + * with WSP continues the previous field. Lines without a colon are skipped. + */ + private static List<String> splitFields(String header) { + List<String> all = new ArrayList<>(); + StringBuilder current = null; + int start = 0; + while (start < header.length()) { + int end = header.indexOf("\r\n", start); + if (end == -1) { + end = header.length(); + } + String line = header.substring(start, end); + start = end + 2; + if (line.isEmpty()) { + // end of the header block + break; + } + if (current != null && (line.charAt(0) == ' ' || line.charAt(0) == '\t')) { + current.append("\r\n").append(line); + continue; + } + addField(all, current); + current = new StringBuilder(line); + } + addField(all, current); + return all; + } + + private static void addField(List<String> all, StringBuilder field) { + if (field != null && field.indexOf(":") != -1) { + all.add(field.toString()); + } + } + + /** + * The name DKIM matches against h= is the raw text before the colon, + * without trailing WSP. + */ + private static String rawName(String field) { + return field.substring(0, field.indexOf(':')).replaceAll("[ \t]+$", "").toLowerCase(Locale.US); + } + public InputStream getBodyInputStream() { try { return ((SingleBody) message.getBody()).getInputStream(); @@ -91,45 +142,13 @@ public class Message implements Headers { * @see org.apache.james.jdkim.api.Headers#getFields() */ public List<String> getFields() { - convertFields(); return fields; } - /** - * Serializes every field once, so that lookups by name do not convert them again. - */ - private void convertFields() { - if (fields != null) { - return; - } - List<String> all = new ArrayList<>(); - Map<String, List<String>> byName = new HashMap<>(); - MessageWriter mw = messageServiceFactory.newMessageWriter(); - ByteArrayOutputStream bos = new ByteArrayOutputStream(); - for (Field f : message.getHeader().getFields()) { - bos.reset(); - String field = null; - 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, StandardCharsets.ISO_8859_1); - } catch (IOException ignored) { - } - all.add(field); - byName.computeIfAbsent(f.getName().toLowerCase(Locale.US), name -> new ArrayList<>()).add(field); - } - byName.replaceAll((name, list) -> Collections.unmodifiableList(list)); - fields = Collections.unmodifiableList(all); - fieldsByName = byName; - } - /** * @see org.apache.james.jdkim.api.Headers#getFields(java.lang.String) */ public List<String> getFields(final String name) { - convertFields(); return fieldsByName.getOrDefault(name.toLowerCase(Locale.US), Collections.emptyList()); } @@ -149,4 +168,4 @@ public class Message implements Headers { public void dispose() { this.message.dispose(); } -} \ No newline at end of file +} 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 3a30ca4..b5557a2 100644 --- a/main/src/test/java/org/apache/james/jdkim/DKIMTest.java +++ b/main/src/test/java/org/apache/james/jdkim/DKIMTest.java @@ -318,4 +318,50 @@ public class DKIMTest { assertThat(verifier.verify(new ByteArrayInputStream(mail.getBytes(StandardCharsets.US_ASCII)))).hasSize(1); } + + @Test + public void controlCharacterBeforeASignedFieldNameShouldBreakTheSignature() throws Exception { + DKIMSigner signer = new DKIMSigner("v=1; a=rsa-sha256; c=relaxed/simple; d=messiah.edu; h=from; s=selector2;", TestKeys.privateKey); + byte[] mail = "From: [email protected]\r\n\r\nbody\r\n".getBytes(StandardCharsets.US_ASCII); + byte[] tampered = "\u0000From: [email protected]\r\n\r\nbody\r\n".getBytes(StandardCharsets.US_ASCII); + 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); + } + + @Test + public void fieldsWhoseNameLooksLikeACommentShouldBeSigned() throws Exception { + DKIMSigner signer = new DKIMSigner("v=1; a=rsa-sha256; c=relaxed/simple; d=messiah.edu; h=from:(((((; s=selector2;", TestKeys.privateKey); + byte[] mail = "From: [email protected]\r\n(((((:(( signed\r\n\r\nbody\r\n".getBytes(StandardCharsets.US_ASCII); + byte[] tampered = "From: [email protected]\r\n(((((:(( altered\r\n\r\nbody\r\n".getBytes(StandardCharsets.US_ASCII); + 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); + } + + @Test + public void messageShouldMatchFieldsOnTheirRawName() throws Exception { + Message message = new Message(new ByteArrayInputStream( + "\u0000From: a\r\nFrom : b\r\n(c) Subject: c\r\n\r\n".getBytes(StandardCharsets.US_ASCII))); + + assertThat(message.getFields("from")).containsExactly("From : b"); + assertThat(message.getFields("subject")).isEmpty(); + assertThat(message.getFields("(c) Subject")).containsExactly("(c) Subject: c"); + } + + @Test + public void messageShouldKeepFoldedFieldsAndHeaderOnlyMessages() throws Exception { + Message folded = new Message(new ByteArrayInputStream( + "Subject: a\r\n\tb\r\n c\r\nFrom: d\r\n\r\nbody\r\n".getBytes(StandardCharsets.US_ASCII))); + Message headerOnly = new Message(new ByteArrayInputStream("From: d\nTo: e".getBytes(StandardCharsets.US_ASCII))); + + assertThat(folded.getFields()).containsExactly("Subject: a\r\n\tb\r\n c", "From: d"); + assertThat(headerOnly.getFields()).containsExactly("From: d", "To: e"); + } } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
