This is an automated email from the ASF dual-hosted git repository.
btellier pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/james-project.git
The following commit(s) were added to refs/heads/master by this push:
new b8fc8a0ab2 [FIX] Improve and test SenderAuthIdentifyVerificationHook
resiliency (weird inputs) (#2565)
b8fc8a0ab2 is described below
commit b8fc8a0ab2776ec3621f33f612020b2ee7f033e6
Author: Benoit TELLIER <btell...@linagora.com>
AuthorDate: Thu Dec 12 17:12:37 2024 +0100
[FIX] Improve and test SenderAuthIdentifyVerificationHook resiliency (weird
inputs) (#2565)
* [FIX] SenderAuthIdentifyVerificationHook: Do not interpret From when no
user
This prevents rejection of mails that parses as
InternetAddress but fails MailAddress validation.
* [FIX] SenderAuthIdentifyVerificationHook: reject invalid FROM only for
our users
* [FIX] SenderAuthIdentifyVerificationHook: handle groups
* [FIX] SenderAuthIdentifyVerificationHook: Fix typo
* [FIX] SenderAuthIdentifyVerificationHook covers new edge case
---------
Co-authored-by: Rene Cordier <rcord...@linagora.com>
---
.../james/smtp/SmtpIdentityVerificationTest.java | 149 +++++++++++++++++++++
.../SenderAuthIdentifyVerificationHook.java | 32 ++++-
2 files changed, 175 insertions(+), 6 deletions(-)
diff --git
a/server/mailet/integration-testing/src/test/java/org/apache/james/smtp/SmtpIdentityVerificationTest.java
b/server/mailet/integration-testing/src/test/java/org/apache/james/smtp/SmtpIdentityVerificationTest.java
index 60e3e3d64a..6a5059939a 100644
---
a/server/mailet/integration-testing/src/test/java/org/apache/james/smtp/SmtpIdentityVerificationTest.java
+++
b/server/mailet/integration-testing/src/test/java/org/apache/james/smtp/SmtpIdentityVerificationTest.java
@@ -184,6 +184,155 @@ class SmtpIdentityVerificationTest {
.hasMessageContaining("503 5.7.1 Incorrect Authentication for
Specified Email Address");
}
+ @Test
+ void spoofingAttemptsShouldBeRejectedInFromFieldWhenGroup(@TempDir File
temporaryFolder) throws Exception {
+ createJamesServer(temporaryFolder, SmtpConfiguration.builder()
+ .requireAuthentication()
+ .verifyIdentity());
+
+ String message = """
+ FROM: MyGroup: vic...@spoofed.info;\r
+ subject: test\r
+ \r
+ content\r
+ .\r
+ """;
+
+ assertThatThrownBy(() ->
+ messageSender.connect(LOCALHOST_IP,
jamesServer.getProbe(SmtpGuiceProbe.class).getSmtpPort())
+ .authenticate(USER, PASSWORD)
+ .sendMessageWithHeaders(USER, ImmutableList.of(USER), message))
+ .isInstanceOf(SMTPSendingException.class)
+ .hasMessageContaining("503 5.7.1 Incorrect Authentication for
Specified Email Address");
+ }
+
+ @Test
+ void
spoofingAttemptsShouldBeRejectedInFromFieldWhenGroupWithOtherUsers(@TempDir
File temporaryFolder) throws Exception {
+ createJamesServer(temporaryFolder, SmtpConfiguration.builder()
+ .requireAuthentication()
+ .verifyIdentity());
+
+ String message = """
+ FROM: MyGroup: u...@james.org, vic...@spoofed.info;\r
+ subject: test\r
+ \r
+ content\r
+ .\r
+ """;
+
+ assertThatThrownBy(() ->
+ messageSender.connect(LOCALHOST_IP,
jamesServer.getProbe(SmtpGuiceProbe.class).getSmtpPort())
+ .authenticate(USER, PASSWORD)
+ .sendMessageWithHeaders(USER, ImmutableList.of(USER), message))
+ .isInstanceOf(SMTPSendingException.class)
+ .hasMessageContaining("503 5.7.1 Incorrect Authentication for
Specified Email Address");
+ }
+
+ @Test
+ void shouldRejectEmptyGroups(@TempDir File temporaryFolder) throws
Exception {
+ createJamesServer(temporaryFolder, SmtpConfiguration.builder()
+ .requireAuthentication()
+ .verifyIdentity());
+
+ String message = """
+ FROM: MyGroup: ;\r
+ subject: test\r
+ \r
+ content\r
+ .\r
+ """;
+
+ assertThatThrownBy(() ->
+ messageSender.connect(LOCALHOST_IP,
jamesServer.getProbe(SmtpGuiceProbe.class).getSmtpPort())
+ .authenticate(USER, PASSWORD)
+ .sendMessageWithHeaders(USER, ImmutableList.of(USER), message))
+ .isInstanceOf(SMTPSendingException.class)
+ .hasMessageContaining("503 5.7.1 Incorrect Authentication for
Specified Email Address");
+ }
+
+ @Test
+ void shouldRejectInvalidWhenLocalUser(@TempDir File temporaryFolder)
throws Exception {
+ createJamesServer(temporaryFolder, SmtpConfiguration.builder()
+ .requireAuthentication()
+ .verifyIdentity());
+
+ String message = """
+ FROM: INVALID\r
+ subject: test\r
+ \r
+ content\r
+ .\r
+ """;
+
+ assertThatThrownBy(() ->
+ messageSender.connect(LOCALHOST_IP,
jamesServer.getProbe(SmtpGuiceProbe.class).getSmtpPort())
+ .authenticate(USER, PASSWORD)
+ .sendMessageWithHeaders(USER, ImmutableList.of(USER), message))
+ .isInstanceOf(SMTPSendingException.class)
+ .hasMessageContaining("503 5.7.1 Incorrect Authentication for
Specified Email Address");
+ }
+
+ @Test
+ void shouldAcceptGroupWhenUnauthenticated(@TempDir File temporaryFolder)
throws Exception {
+ createJamesServer(temporaryFolder, SmtpConfiguration.builder()
+ .requireAuthentication()
+ .verifyIdentity());
+
+ String message = """
+ FROM: MyGroup: vic...@spoofed.info;\r
+ subject: test\r
+ \r
+ content\r
+ .\r
+ """;
+
+ assertThatCode(() ->
+ messageSender.connect(LOCALHOST_IP,
jamesServer.getProbe(SmtpGuiceProbe.class).getSmtpPort())
+ .sendMessageWithHeaders("vic...@spoofed.info",
ImmutableList.of(USER), message))
+ .doesNotThrowAnyException();
+ }
+
+ @Test
+ void shouldAcceptInvalidFromWhenUnauthenticated(@TempDir File
temporaryFolder) throws Exception {
+ createJamesServer(temporaryFolder, SmtpConfiguration.builder()
+ .requireAuthentication()
+ .verifyIdentity());
+
+ String message = """
+ FROM: INVALID\r
+ subject: test\r
+ \r
+ content\r
+ .\r
+ """;
+
+ assertThatCode(() ->
+ messageSender.connect(LOCALHOST_IP,
jamesServer.getProbe(SmtpGuiceProbe.class).getSmtpPort())
+ .sendMessageWithHeaders("vic...@spoofed.info",
ImmutableList.of(USER), message))
+ .doesNotThrowAnyException();
+ }
+
+ @Test
+ void shouldAcceptGroupWhenOnlyUser(@TempDir File temporaryFolder) throws
Exception {
+ createJamesServer(temporaryFolder, SmtpConfiguration.builder()
+ .requireAuthentication()
+ .verifyIdentity());
+
+ String message = """
+ FROM: MyGroup: u...@james.org;\r
+ subject: test\r
+ \r
+ content\r
+ .\r
+ """;
+
+ assertThatCode(() ->
+ messageSender.connect(LOCALHOST_IP,
jamesServer.getProbe(SmtpGuiceProbe.class).getSmtpPort())
+ .authenticate(USER, PASSWORD)
+ .sendMessageWithHeaders(USER, ImmutableList.of(USER), message))
+ .doesNotThrowAnyException();
+ }
+
@Test
void errorsShouldBeIgnoredWhenUnAuthed(@TempDir File temporaryFolder)
throws Exception {
createJamesServer(temporaryFolder, SmtpConfiguration.builder()
diff --git
a/server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/SenderAuthIdentifyVerificationHook.java
b/server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/SenderAuthIdentifyVerificationHook.java
index a5a0bc4325..5f908ffa33 100644
---
a/server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/SenderAuthIdentifyVerificationHook.java
+++
b/server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/SenderAuthIdentifyVerificationHook.java
@@ -18,6 +18,7 @@
****************************************************************/
package org.apache.james.smtpserver;
+import java.util.Arrays;
import java.util.Optional;
import java.util.stream.Stream;
@@ -41,11 +42,13 @@ import org.apache.james.protocols.smtp.hook.HookResult;
import org.apache.james.rrt.api.CanSendFrom;
import org.apache.james.user.api.UsersRepository;
import org.apache.james.user.api.UsersRepositoryException;
+import org.apache.james.util.MemoizedSupplier;
import org.apache.james.util.StreamUtils;
import org.apache.mailet.Mail;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import com.github.fge.lambdas.Throwing;
import com.google.common.net.InternetDomainName;
/**
@@ -148,7 +151,7 @@ public class SenderAuthIdentifyVerificationHook extends
AbstractSenderAuthIdenti
// Ignore invalid from header for relays
return HookResult.DECLINED;
} else {
- LOGGER.warn("Local user {} attempted to use an invalid
From header", e);
+ LOGGER.warn("Local user {} attempted to use an invalid
From header", session.getUsername(), e);
throw new RuntimeException(e);
}
}
@@ -168,17 +171,34 @@ public class SenderAuthIdentifyVerificationHook extends
AbstractSenderAuthIdenti
private boolean fromDoesNotMatchAuthUser(SMTPSession session, Address
from) {
if (from instanceof InternetAddress internetAddress) {
try {
- MailAddress mailAddress = new
MailAddress(internetAddress.getAddress());
- return session.getUsername() != null &&
- (!fromMatchSessionUser(mailAddress, session) ||
!belongsToLocalDomain(mailAddress));
+ if (internetAddress.isGroup()) {
+ boolean strict = true;
+ InternetAddress[] addressGroup =
internetAddress.getGroup(!strict);
+ if (session.getUsername() != null && addressGroup.length
== 0) {
+ return true;
+ }
+ return Arrays.stream(addressGroup)
+ .map(address -> fromDoesNotMatchAuthUser(session,
address))
+ .filter(b -> b)
+ .findAny()
+ .orElse(false);
+ }
+ return fromDoesNotMatchAuthUser(session, internetAddress);
} catch (AddressException e) {
- // Never happens as valid InternetAddress are valid MailAddress
- throw new RuntimeException(e);
+ LOGGER.warn("Local user {} attempted to use an invalid From
header", session.getUsername(), e);
+ return session.getUsername() != null; // Accept external
invalid form header, reject invalid from from our users.
}
}
return false;
}
+ private boolean fromDoesNotMatchAuthUser(SMTPSession session,
InternetAddress internetAddress) {
+ MemoizedSupplier<MailAddress> mailAddress =
MemoizedSupplier.of(Throwing.supplier(
+ () -> new
MailAddress(internetAddress.getAddress())).sneakyThrow());
+ return session.getUsername() != null &&
+ (!fromMatchSessionUser(mailAddress.get(), session) ||
!belongsToLocalDomain(mailAddress.get()));
+ }
+
private boolean fromMatchSessionUser(MailAddress from, SMTPSession
session) {
Username authUser = session.getUsername();
Username sender = getUser(from);
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@james.apache.org
For additional commands, e-mail: notifications-h...@james.apache.org
