This is an automated email from the ASF dual-hosted git repository. rcordier pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/james-project.git
commit e982d2a27b081416a92f524931f7eb8d99057829 Author: Rene Cordier <rcord...@linagora.com> AuthorDate: Thu Oct 10 16:10:20 2024 +0700 JAMES-4032 DKIMHook should also validate local from header --- .../servers/partials/configure/smtp-hooks.adoc | 4 +- .../org/apache/james/DKIMHookIntegrationTest.java | 133 +++++++++++++++++++++ .../resources/eml/otherDomainDkimLocalFrom.eml | 59 +++++++++ .../java/org/apache/james/smtpserver/DKIMHook.java | 48 +++++++- .../org/apache/james/smtpserver/DKIMHookTest.java | 29 +++++ 5 files changed, 270 insertions(+), 3 deletions(-) diff --git a/docs/modules/servers/partials/configure/smtp-hooks.adoc b/docs/modules/servers/partials/configure/smtp-hooks.adoc index d2ba36c271..83b821402d 100644 --- a/docs/modules/servers/partials/configure/smtp-hooks.adoc +++ b/docs/modules/servers/partials/configure/smtp-hooks.adoc @@ -359,7 +359,7 @@ some intermediate third parties. See link:https://issues.apache.org/jira/browse/ Supported configuration elements: - *forceCRLF*: Should CRLF be forced when computing body hashes. -- *onlyForSenderDomain*: If specified, the DKIM checks are applied just for the emails whose MAIL FROM specifies this domain. If unspecified, all emails are checked (default). +- *onlyForSenderDomain*: If specified, the DKIM checks are applied just for the emails whose MAIL FROM or from header specifies this domain. If unspecified, all emails are checked (default). - *signatureRequired*: If DKIM signature is checked, the absence of signature will generate failure. Defaults to false. - *expectedDToken*: If DKIM signature is checked, the body should contain at least one DKIM signature with this d token. If unspecified, all d tokens are considered valid (default). @@ -378,5 +378,5 @@ Example handlerchain configuration for `smtpserver.xml`: </handlerchain> .... -Would allow emails using `apache.org` as a MAIL FROM domain if, and only if they contain a +Would allow emails using `apache.org` as a MAIL FROM or from header domain if, and only if they contain a valid DKIM signature for the `apache.org` domain. \ No newline at end of file diff --git a/server/apps/memory-app/src/test/java/org/apache/james/DKIMHookIntegrationTest.java b/server/apps/memory-app/src/test/java/org/apache/james/DKIMHookIntegrationTest.java index fffe6e1637..ac092e7aae 100644 --- a/server/apps/memory-app/src/test/java/org/apache/james/DKIMHookIntegrationTest.java +++ b/server/apps/memory-app/src/test/java/org/apache/james/DKIMHookIntegrationTest.java @@ -190,6 +190,139 @@ public class DKIMHookIntegrationTest { .contains("530 DKIM check failed. Wrong d token. Expecting avocat.fr"); } + @Test + void shouldRejectFromHeaderCheckedDomainWhenNotSigned(GuiceJamesServer guiceJamesServer) throws Exception { + SocketChannel server = SocketChannel.open(); + server.connect(new InetSocketAddress(LOCALHOST_IP, guiceJamesServer.getProbe(SmtpGuiceProbe.class).getSmtpPort().getValue())); + readBytes(server); + + server.write(ByteBuffer.wrap(("EHLO whatever.com\r\n").getBytes(StandardCharsets.UTF_8))); + readBytes(server); + server.write(ByteBuffer.wrap(("MAIL FROM: <u...@whatever.com>\r\n").getBytes(StandardCharsets.UTF_8))); + readBytes(server); + server.write(ByteBuffer.wrap(("RCPT TO: <user@" + DOMAIN.asString() + ">\r\n").getBytes(StandardCharsets.UTF_8))); + readBytes(server); + server.write(ByteBuffer.wrap(("DATA\r\n").getBytes(StandardCharsets.UTF_8))); + readBytes(server); // needed to synchronize + server.write(ByteBuffer.wrap(("from:<user@" + DOMAIN.asString() + ">\r\n\r\nbody").getBytes(StandardCharsets.UTF_8))); + server.write(ByteBuffer.wrap(("\r\n").getBytes(StandardCharsets.UTF_8))); + server.write(ByteBuffer.wrap((".").getBytes(StandardCharsets.UTF_8))); + server.write(ByteBuffer.wrap(("\r\n").getBytes(StandardCharsets.UTF_8))); + byte[] dataResponse = readBytes(server); + server.write(ByteBuffer.wrap(("QUIT\r\n").getBytes(StandardCharsets.UTF_8))); + + assertThat(new String(dataResponse, StandardCharsets.UTF_8)) + .contains("530 DKIM check failed. Expecting DKIM signatures. Got none."); + } + + @Test + void shouldAcceptFromHeaderNotCheckedDomains(GuiceJamesServer guiceJamesServer) throws Exception { + SocketChannel server = SocketChannel.open(); + server.connect(new InetSocketAddress(LOCALHOST_IP, guiceJamesServer.getProbe(SmtpGuiceProbe.class).getSmtpPort().getValue())); + readBytes(server); + + server.write(ByteBuffer.wrap(("EHLO whatever.com\r\n").getBytes(StandardCharsets.UTF_8))); + readBytes(server); + server.write(ByteBuffer.wrap(("MAIL FROM: <u...@whatever.com>\r\n").getBytes(StandardCharsets.UTF_8))); + readBytes(server); + server.write(ByteBuffer.wrap(("RCPT TO: <user@" + DOMAIN.asString() + ">\r\n").getBytes(StandardCharsets.UTF_8))); + readBytes(server); + server.write(ByteBuffer.wrap(("DATA\r\n").getBytes(StandardCharsets.UTF_8))); + readBytes(server); // needed to synchronize + server.write(ByteBuffer.wrap(("from:<u...@whatever.com>\r\n\r\nbody").getBytes(StandardCharsets.UTF_8))); + server.write(ByteBuffer.wrap(("\r\n").getBytes(StandardCharsets.UTF_8))); + server.write(ByteBuffer.wrap((".").getBytes(StandardCharsets.UTF_8))); + server.write(ByteBuffer.wrap(("\r\n").getBytes(StandardCharsets.UTF_8))); + byte[] dataResponse = readBytes(server); + server.write(ByteBuffer.wrap(("QUIT\r\n").getBytes(StandardCharsets.UTF_8))); + + assertThat(new String(dataResponse, StandardCharsets.UTF_8)) + .contains("250 2.6.0 Message received"); + } + + @Test + void shouldAcceptFromHeaderEmailsForCheckedDomainsWhenSigned(GuiceJamesServer guiceJamesServer) throws Exception { + SocketChannel server = SocketChannel.open(); + server.connect(new InetSocketAddress(LOCALHOST_IP, guiceJamesServer.getProbe(SmtpGuiceProbe.class).getSmtpPort().getValue())); + readBytes(server); + + server.write(ByteBuffer.wrap(("EHLO " + DOMAIN.asString() + "\r\n").getBytes(StandardCharsets.UTF_8))); + readBytes(server); + server.write(ByteBuffer.wrap(("MAIL FROM: <u...@whatever.com>\r\n").getBytes(StandardCharsets.UTF_8))); + readBytes(server); + server.write(ByteBuffer.wrap(("RCPT TO: <user@" + DOMAIN.asString() + ">\r\n").getBytes(StandardCharsets.UTF_8))); + readBytes(server); + server.write(ByteBuffer.wrap(("DATA\r\n").getBytes(StandardCharsets.UTF_8))); + readBytes(server); // needed to synchronize + + // goodDkim.eml has a from header with the local domain + server.write(ByteBuffer.wrap(ClassLoaderUtils.getSystemResourceAsByteArray("eml/goodDkim.eml"))); + + server.write(ByteBuffer.wrap(("\r\n").getBytes(StandardCharsets.UTF_8))); + server.write(ByteBuffer.wrap((".").getBytes(StandardCharsets.UTF_8))); + server.write(ByteBuffer.wrap(("\r\n").getBytes(StandardCharsets.UTF_8))); + byte[] dataResponse = readBytes(server); + server.write(ByteBuffer.wrap(("QUIT\r\n").getBytes(StandardCharsets.UTF_8))); + + assertThat(new String(dataResponse, StandardCharsets.UTF_8)) + .contains("250 2.6.0 Message received"); + } + + @Test + void shouldRejectFromHeaderInvalidDKIMSignatures(GuiceJamesServer guiceJamesServer) throws Exception { + SocketChannel server = SocketChannel.open(); + server.connect(new InetSocketAddress(LOCALHOST_IP, guiceJamesServer.getProbe(SmtpGuiceProbe.class).getSmtpPort().getValue())); + readBytes(server); + + server.write(ByteBuffer.wrap(("EHLO " + DOMAIN.asString() + "\r\n").getBytes(StandardCharsets.UTF_8))); + readBytes(server); + server.write(ByteBuffer.wrap(("MAIL FROM: <u...@whatever.com>\r\n").getBytes(StandardCharsets.UTF_8))); + readBytes(server); + server.write(ByteBuffer.wrap(("RCPT TO: <user@" + DOMAIN.asString() + ">\r\n").getBytes(StandardCharsets.UTF_8))); + readBytes(server); + server.write(ByteBuffer.wrap(("DATA\r\n").getBytes(StandardCharsets.UTF_8))); + readBytes(server); // needed to synchronize + + // badDkim.eml has a from header with the local domain + server.write(ByteBuffer.wrap(ClassLoaderUtils.getSystemResourceAsByteArray("eml/badDkim.eml"))); + + server.write(ByteBuffer.wrap(("\r\n").getBytes(StandardCharsets.UTF_8))); + server.write(ByteBuffer.wrap((".").getBytes(StandardCharsets.UTF_8))); + server.write(ByteBuffer.wrap(("\r\n").getBytes(StandardCharsets.UTF_8))); + byte[] dataResponse = readBytes(server); + server.write(ByteBuffer.wrap(("QUIT\r\n").getBytes(StandardCharsets.UTF_8))); + + assertThat(new String(dataResponse, StandardCharsets.UTF_8)) + .contains("530 DKIM check failed. Invalid signature."); + } + + @Test + void shouldRejectFromHeaderCheckedDomainsWhenSignatureOfOtherDomain(GuiceJamesServer guiceJamesServer) throws Exception { + SocketChannel server = SocketChannel.open(); + server.connect(new InetSocketAddress(LOCALHOST_IP, guiceJamesServer.getProbe(SmtpGuiceProbe.class).getSmtpPort().getValue())); + readBytes(server); + + server.write(ByteBuffer.wrap(("EHLO " + DOMAIN.asString() + "\r\n").getBytes(StandardCharsets.UTF_8))); + readBytes(server); + server.write(ByteBuffer.wrap(("MAIL FROM: <u...@whatever.com>\n").getBytes(StandardCharsets.UTF_8))); + readBytes(server); + server.write(ByteBuffer.wrap(("RCPT TO: <user@" + DOMAIN.asString() + ">\r\n").getBytes(StandardCharsets.UTF_8))); + readBytes(server); + server.write(ByteBuffer.wrap(("DATA\r\n").getBytes(StandardCharsets.UTF_8))); + readBytes(server); // needed to synchronize + + server.write(ByteBuffer.wrap(ClassLoaderUtils.getSystemResourceAsByteArray("eml/otherDomainDkimLocalFrom.eml"))); + + server.write(ByteBuffer.wrap(("\r\n").getBytes(StandardCharsets.UTF_8))); + server.write(ByteBuffer.wrap((".").getBytes(StandardCharsets.UTF_8))); + server.write(ByteBuffer.wrap(("\r\n").getBytes(StandardCharsets.UTF_8))); + byte[] dataResponse = readBytes(server); + server.write(ByteBuffer.wrap(("QUIT\r\n").getBytes(StandardCharsets.UTF_8))); + + assertThat(new String(dataResponse, StandardCharsets.UTF_8)) + .contains("530 DKIM check failed. Invalid signature."); + } + private byte[] readBytes(SocketChannel channel) throws IOException { ByteBuffer line = ByteBuffer.allocate(1024); channel.read(line); diff --git a/server/apps/memory-app/src/test/resources/eml/otherDomainDkimLocalFrom.eml b/server/apps/memory-app/src/test/resources/eml/otherDomainDkimLocalFrom.eml new file mode 100644 index 0000000000..356d028a3b --- /dev/null +++ b/server/apps/memory-app/src/test/resources/eml/otherDomainDkimLocalFrom.eml @@ -0,0 +1,59 @@ +Return-Path: <SRS0=fiQ1=NJ=preprod-avocat.fr=sonde-supervision-preprod.linag...@ik2.com> +org.apache.james.rspamd.flag: NO +org.apache.james.rspamd.status: No, actions=no action score=-0.700093 requiredScore=15.0 +Delivered-To: btell...@linagora.com +Received: from 172.17.0.1 (EHLO incoming.linagora.com) ([172.17.0.1]) + by incoming.linagora.com (JAMES SMTP Server ) with ESMTP ID aec1ac7b + for <btell...@linagora.com>; + Fri, 07 Jun 2024 06:32:41 +0000 (UTC) +Received: from obm3-ui.linagora.dc1 (arathi.linagora.com [54.36.8.82]) + by incoming.linagora.com (Postfix) with ESMTP id 07BDE140137 + for <btell...@linagora.com>; Fri, 7 Jun 2024 06:32:41 +0000 (UTC) +Received: from s602g.ik2.com (s602g.ik2.com [66.37.25.74]) + by obm3-ui.linagora.dc1 (Postfix) with ESMTP id 830051D5C6 + for <btell...@linagora.com>; Fri, 7 Jun 2024 08:32:33 +0200 (CEST) +Received: from s214b.ik2.com + by s602g.ik2.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) + (envelope-from <sonde-supervision-preprod.linag...@preprod-avocat.fr>) + id 1sFT9N-0007vZ-O9 + for btell...@linagora.com; Fri, 07 Jun 2024 06:32:38 +0000 +Received: from 185.60.151.64 by s214b.ik2.com (IK2 SMTP Server); Fri, 7 Jun 2024 06:32:33 +0000 +Received: from outgoing.preprod-avocat.fr (unknown [100.96.59.32]) + by outgoing.preprod-avocat.fr (Postfix) with ESMTP id 090772C00FA + for <btell...@linagora.com>; Fri, 7 Jun 2024 08:32:30 +0200 (CEST) +DKIM-Signature: a=rsa-sha256; b=t91ocFueqOR3c2K2TDUz3SUxcVxY5QhaKCrbd9u40WAO11MLxUiEgK7aYTRN/GJJJo6l0sL81kSQbsNPBlcjEGUKhv4wPU34BQko5iAK4gkIGTZ3kWfC1rQo52QK17EqKd9OG9q7vAJh1eA4la5P1Z8DJzxxCB39E0bl9SEtb6s=; s=s1; d=preprod-avocat.fr; v=1; bh=DOV1GxCvw1QI/ZiqKmKwXLguFsLSCuRTQ978r2J4NGs=; h=from : reply-to : subject : date : to : cc : resent-date : resent-from : resent-sender : resent-to : resent-cc : in-reply-to : references : list-id : list-help : list-unsubscribe : list-subscribe : list- [...] +MIME-Version: 1.0 +Subject: DKIM signature of another domain +X-LINAGORA-Copy-Delivery-Done: 1 +From: "Compte Test 15 (Linagora)" <comptetest15.linag...@avocat.fr> +To: "btell...@linagora.com" <btell...@linagora.com> +Reply-To: sonde-supervision-preprod.linag...@preprod-avocat.fr +Date: Fri, 7 Jun 2024 06:32:23 +0000 +Message-ID: <mime4j.19.f679494c3e9bb11d.18ff165f...@preprod-avocat.fr> +User-Agent: Team-Mail/0.11.3 Mozilla/5.0 (X11; Linux x86_64) + AppleWebKit/537.36 (KHTML, like Gecko) Chrome/125.0.0.0 Safari/537.36 +Disposition-Notification-To: + sonde-supervision-preprod.linag...@preprod-avocat.fr +Content-Type: multipart/alternative; + boundary="-=Part.1a.56f2e38fc73ec684.18ff165fa26.911900ebfe468c22=-" +X-SF-RX-Return-Path: <sonde-supervision-preprod.linag...@preprod-avocat.fr> +X-SF-Originating-IP: 185.60.151.64 +X-SF-WhiteListedReason: Whitelisted EmailTO +X-SF-Score: +X-SF-SRS: Sender address rewritten from <sonde-supervision-preprod.linag...@preprod-avocat.fr> to <SRS0=fiQ1=NJ=preprod-avocat.fr=sonde-supervision-preprod.linag...@ik2.com> +X-SF-Domain: qgwlxqsyyb + +---=Part.1a.56f2e38fc73ec684.18ff165fa26.911900ebfe468c22=- +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: quoted-printable + + +DKIM signature of another domain + +---=Part.1a.56f2e38fc73ec684.18ff165fa26.911900ebfe468c22=- +Content-Type: text/html; charset=UTF-8 +Content-Transfer-Encoding: quoted-printable + +<div style=3D"line-height: 100%;"><br style=3D"line-height: 100%;">DKIM sig= +nature of another domain<br style=3D"line-height: 100%;"></div> +---=Part.1a.56f2e38fc73ec684.18ff165fa26.911900ebfe468c22=--- \ No newline at end of file diff --git a/server/protocols/protocols-smtp-dkim/src/main/java/org/apache/james/smtpserver/DKIMHook.java b/server/protocols/protocols-smtp-dkim/src/main/java/org/apache/james/smtpserver/DKIMHook.java index 8a039da4b7..cc8817ed53 100644 --- a/server/protocols/protocols-smtp-dkim/src/main/java/org/apache/james/smtpserver/DKIMHook.java +++ b/server/protocols/protocols-smtp-dkim/src/main/java/org/apache/james/smtpserver/DKIMHook.java @@ -22,13 +22,18 @@ package org.apache.james.smtpserver; import static org.apache.james.protocols.smtp.SMTPRetCode.AUTH_REQUIRED; import static org.apache.james.protocols.smtp.SMTPRetCode.LOCAL_ERROR; +import java.util.Arrays; import java.util.List; import java.util.Objects; import java.util.Optional; import java.util.function.Predicate; +import java.util.stream.Stream; import jakarta.inject.Inject; +import jakarta.mail.Address; import jakarta.mail.MessagingException; +import jakarta.mail.internet.AddressException; +import jakarta.mail.internet.InternetAddress; import org.apache.commons.configuration2.Configuration; import org.apache.james.core.Domain; @@ -41,6 +46,7 @@ import org.apache.james.jdkim.mailets.DKIMVerifier; import org.apache.james.protocols.smtp.SMTPSession; import org.apache.james.protocols.smtp.hook.HookResult; import org.apache.james.protocols.smtp.hook.HookReturnCode; +import org.apache.james.util.StreamUtils; import org.apache.mailet.Mail; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -84,6 +90,11 @@ public class DKIMHook implements JamesMessageHook { @FunctionalInterface interface DKIMCheckNeeded extends Predicate<Mail> { + static DKIMCheckNeeded or(DKIMCheckNeeded... checkNeededs) { + return mail -> Arrays.stream(checkNeededs) + .anyMatch(predicate -> predicate.test(mail)); + } + static DKIMCheckNeeded onlyForSenderDomain(Domain domain) { return mail -> mail.getMaybeSender() .asOptional() @@ -92,6 +103,39 @@ public class DKIMHook implements JamesMessageHook { .orElse(false); } + static DKIMCheckNeeded onlyForHeaderFromDomain(Domain domain) { + return mail -> { + try { + return StreamUtils.ofNullable(mail.getMessage().getFrom()) + .distinct() + .flatMap(DKIMCheckNeeded::parseMailAddress) + .findFirst() + .map(MailAddress::getDomain) + .map(domain::equals) + .orElse(false); + } catch (MessagingException me) { + try { + LOGGER.info("Unable to parse the \"FROM\" header {}; ignoring.", mail.getMessage().getHeader("From")); + } catch (MessagingException e) { + LOGGER.info("Unable to parse the \"FROM\" header; ignoring."); + } + } + return false; + }; + } + + private static Stream<MailAddress> parseMailAddress(Address from) { + if (from instanceof InternetAddress internetAddress) { + try { + return Stream.of(new MailAddress(internetAddress.getAddress())); + } catch (AddressException e) { + // Never happens as valid InternetAddress are valid MailAddress + throw new RuntimeException(e); + } + } + return Stream.empty(); + } + DKIMCheckNeeded ALL = any -> true; } @@ -165,7 +209,9 @@ public class DKIMHook implements JamesMessageHook { DKIMCheckNeeded dkimCheckNeeded() { return onlyForSenderDomain - .map(DKIMCheckNeeded::onlyForSenderDomain) + .map(domain -> DKIMCheckNeeded.or( + DKIMCheckNeeded.onlyForSenderDomain(domain), + DKIMCheckNeeded.onlyForHeaderFromDomain(domain))) .orElse(DKIMCheckNeeded.ALL); } diff --git a/server/protocols/protocols-smtp-dkim/src/test/java/org/apache/james/smtpserver/DKIMHookTest.java b/server/protocols/protocols-smtp-dkim/src/test/java/org/apache/james/smtpserver/DKIMHookTest.java index c6750604cf..fdad0bf7aa 100644 --- a/server/protocols/protocols-smtp-dkim/src/test/java/org/apache/james/smtpserver/DKIMHookTest.java +++ b/server/protocols/protocols-smtp-dkim/src/test/java/org/apache/james/smtpserver/DKIMHookTest.java @@ -26,6 +26,7 @@ import java.util.Optional; import org.apache.commons.configuration2.BaseHierarchicalConfiguration; import org.apache.james.core.Domain; import org.apache.james.core.MaybeSender; +import org.apache.james.core.builder.MimeMessageBuilder; import org.apache.james.jdkim.tagvalue.SignatureRecordImpl; import org.apache.james.protocols.smtp.hook.HookReturnCode; import org.apache.mailet.base.test.FakeMail; @@ -73,6 +74,34 @@ class DKIMHookTest { .build())) .isFalse(); } + + @Test + void onlyForSenderDomainShouldKeepWhenHeaderFromDomainMatches() throws Exception { + assertThat(DKIMHook.DKIMCheckNeeded.onlyForHeaderFromDomain(Domain.LOCALHOST) + .test(FakeMail.builder() + .name("mail") + .sender("b...@other.com") + .mimeMessage(MimeMessageBuilder.mimeMessageBuilder() + .addFrom("bob@localhost") + .setText("This is my email") + .build()) + .build())) + .isTrue(); + } + + @Test + void onlyForSenderDomainShouldRejectWhenHeaderFromDomainDoesNotMatches() throws Exception { + assertThat(DKIMHook.DKIMCheckNeeded.onlyForHeaderFromDomain(Domain.LOCALHOST) + .test(FakeMail.builder() + .name("mail") + .sender("b...@other.com") + .mimeMessage(MimeMessageBuilder.mimeMessageBuilder() + .addFrom("b...@other.com") + .setText("This is my email") + .build()) + .build())) + .isFalse(); + } @Test void allShouldAcceptArbitrarySenderDomains() throws Exception { --------------------------------------------------------------------- To unsubscribe, e-mail: notifications-unsubscr...@james.apache.org For additional commands, e-mail: notifications-h...@james.apache.org