You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by bt...@apache.org on 2021/04/02 01:31:07 UTC

[james-project] 01/04: JAMES-3225 Verify identity should also apply for unauthenticated users

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

commit af8356f227c468dd101fc0a48bd5c13437aed459
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Sat Mar 27 23:04:15 2021 +0700

    JAMES-3225 Verify identity should also apply for unauthenticated users
    
    Given this SMTP configuration:
    
    ```
       <smtpserver enabled="true">
            ...
            <authRequired>true</authRequired>
            <verifyIdentity>true</verifyIdentity>
       </smtpserver>
    ```
    
    A non authenticated user can pretend to be a local user...
---
 ...ractSenderAuthIdentifyVerificationRcptHook.java | 21 ++++++++++++++++-----
 .../james/smtp/SmtpIdentityVerificationTest.java   | 22 ++++++++++++++++++++++
 .../james/smtpserver/SMTPTestConfiguration.java    |  4 +---
 3 files changed, 39 insertions(+), 8 deletions(-)

diff --git a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/AbstractSenderAuthIdentifyVerificationRcptHook.java b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/AbstractSenderAuthIdentifyVerificationRcptHook.java
index e709b3e..3cbad39 100644
--- a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/AbstractSenderAuthIdentifyVerificationRcptHook.java
+++ b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/AbstractSenderAuthIdentifyVerificationRcptHook.java
@@ -22,7 +22,6 @@ import org.apache.james.core.Domain;
 import org.apache.james.core.MailAddress;
 import org.apache.james.core.MaybeSender;
 import org.apache.james.core.Username;
-import org.apache.james.protocols.api.ProtocolSession;
 import org.apache.james.protocols.smtp.SMTPRetCode;
 import org.apache.james.protocols.smtp.SMTPSession;
 import org.apache.james.protocols.smtp.dsn.DSNStatus;
@@ -42,22 +41,34 @@ public abstract class AbstractSenderAuthIdentifyVerificationRcptHook implements
         .smtpDescription(DSNStatus.getStatus(DSNStatus.PERMANENT, DSNStatus.SECURITY_AUTH)
             + " Incorrect Authentication for Specified Email Address")
         .build();
+    private static final HookResult AUTH_REQUIRED = HookResult.builder()
+        .hookReturnCode(HookReturnCode.deny())
+        .smtpReturnCode(SMTPRetCode.AUTH_REQUIRED)
+        .smtpDescription(DSNStatus.getStatus(DSNStatus.PERMANENT, DSNStatus.SECURITY_AUTH)
+            + " Authentication Required")
+        .build();
     
     @Override
     public HookResult doRcpt(SMTPSession session, MaybeSender sender, MailAddress rcpt) {
         if (session.getUsername() != null) {
-            MaybeSender senderAddress = session.getAttachment(SMTPSession.SENDER, ProtocolSession.State.Transaction).orElse(MaybeSender.nullSender());
-            
             // Check if the sender address is the same as the user which was used to authenticate.
             // Its important to ignore case here to fix JAMES-837. This is save todo because if the handler is called
             // the user was already authenticated
+
             if (isAnonymous(sender)
                 || !senderMatchSessionUser(sender, session)
-                || !belongsToLocalDomain(senderAddress)) {
+                || !belongsToLocalDomain(sender)) {
                 return INVALID_AUTH;
             }
+            return HookResult.DECLINED;
+        } else {
+            // Validate that unauthenticated users do not use local addresses in MAIL FROM
+            if (belongsToLocalDomain(sender)) {
+                return AUTH_REQUIRED;
+            } else {
+                return HookResult.DECLINED;
+            }
         }
-        return HookResult.DECLINED;
     }
 
     private boolean isAnonymous(MaybeSender maybeSender) {
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 61b21c7..f576aa8 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
@@ -70,6 +70,16 @@ class SmtpIdentityVerificationTest {
     }
 
     @Test
+    void remoteUserCanSendEmailsToLocalUsers(@TempDir File temporaryFolder) throws Exception {
+        createJamesServer(temporaryFolder, SmtpConfiguration.builder()
+            .requireAuthentication()
+            .verifyIdentity());
+
+        messageSender.connect(LOCALHOST_IP, jamesServer.getProbe(SmtpGuiceProbe.class).getSmtpPort())
+            .sendMessage("other@domain.tld", USER);
+    }
+
+    @Test
     void smtpShouldAcceptMessageWhenIdentityIsMatching(@TempDir File temporaryFolder) throws Exception {
         createJamesServer(temporaryFolder, SmtpConfiguration.builder()
             .requireAuthentication()
@@ -80,6 +90,18 @@ class SmtpIdentityVerificationTest {
     }
 
     @Test
+    void rejectUnauthenticatedSendersUsingLocalDomains(@TempDir File temporaryFolder) throws Exception {
+        createJamesServer(temporaryFolder, SmtpConfiguration.builder()
+            .requireAuthentication()
+            .verifyIdentity());
+
+        assertThatThrownBy(() ->
+            messageSender.connect(LOCALHOST_IP, jamesServer.getProbe(SmtpGuiceProbe.class).getSmtpPort())
+                .sendMessage(USER, USER))
+            .isEqualTo(new SMTPSendingException(SmtpSendingStep.RCPT, "530 5.7.1 Authentication Required\n"));
+    }
+
+    @Test
     void smtpShouldAcceptMessageWhenIdentityIsNotMatchingButNotChecked(@TempDir File temporaryFolder) throws Exception {
         createJamesServer(temporaryFolder, SmtpConfiguration.builder()
             .requireAuthentication()
diff --git a/server/protocols/protocols-smtp/src/test/java/org/apache/james/smtpserver/SMTPTestConfiguration.java b/server/protocols/protocols-smtp/src/test/java/org/apache/james/smtpserver/SMTPTestConfiguration.java
index cb695a6..9bde7e9 100644
--- a/server/protocols/protocols-smtp/src/test/java/org/apache/james/smtpserver/SMTPTestConfiguration.java
+++ b/server/protocols/protocols-smtp/src/test/java/org/apache/james/smtpserver/SMTPTestConfiguration.java
@@ -131,9 +131,7 @@ public class SMTPTestConfiguration extends BaseHierarchicalConfiguration {
         addProperty("tls.[@startTLS]", startTLS);
         addProperty("tls.keystore", "test_keystore");
         addProperty("tls.secret", "jamestest");
-        if (verifyIdentity) {
-            addProperty("verifyIdentity", verifyIdentity);
-        }
+        addProperty("verifyIdentity", verifyIdentity);
 
         // add the rbl handler
         if (useRBL) {

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org