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/05/14 02:04:41 UTC

[james-project] 06/09: JAMES-3574 LMTP should reject EHLO and HELO with 500 error code

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 30d1fc83ee89ffcf8791af7cd1dc6df9bb957989
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Thu May 13 12:03:34 2021 +0700

    JAMES-3574 LMTP should reject EHLO and HELO with 500 error code
    
    https://datatracker.ietf.org/doc/html/rfc2033#section-4.1
    
    ```
       The HELO and EHLO commands of ESMTP are not present in LMTP.  A LMTP
       server MUST NOT return a Postive Completion reply code to these
       commands.  The 500 reply code is recommended.
    ```
    
    A null pointer exception was generated, and the error was handled as
    a transiant error, resulting in a misleading 450 code.
    
    Other protocols backed by the protocols API are affected as well for unknown commands
    (SMTP and POP3)
---
 .../james/protocols/api/ProtocolSession.java       |  2 ++
 .../james/protocols/api/ProtocolSessionImpl.java   |  5 +++++
 .../protocols/api/handler/CommandDispatcher.java   |  4 ++++
 .../james/protocols/pop3/POP3SessionImpl.java      |  5 +++++
 .../james/protocols/smtp/SMTPSessionImpl.java      |  8 +++++++-
 .../protocols/smtp/utils/BaseFakeSMTPSession.java  |  5 +++++
 .../apache/james/lmtpserver/LmtpServerTest.java    | 22 ++++++++++++++++++++++
 7 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/protocols/api/src/main/java/org/apache/james/protocols/api/ProtocolSession.java b/protocols/api/src/main/java/org/apache/james/protocols/api/ProtocolSession.java
index b483985..a08c0ae 100644
--- a/protocols/api/src/main/java/org/apache/james/protocols/api/ProtocolSession.java
+++ b/protocols/api/src/main/java/org/apache/james/protocols/api/ProtocolSession.java
@@ -176,6 +176,8 @@ public interface ProtocolSession {
      * @return Response or null if no response should be written before closing the connection
      */
     Response newFatalErrorResponse();
+
+    Response newCommandNotFoundErrorResponse();
     
     /**
      * Returns the user name associated with this interaction.
diff --git a/protocols/api/src/main/java/org/apache/james/protocols/api/ProtocolSessionImpl.java b/protocols/api/src/main/java/org/apache/james/protocols/api/ProtocolSessionImpl.java
index f824826..d713f64 100644
--- a/protocols/api/src/main/java/org/apache/james/protocols/api/ProtocolSessionImpl.java
+++ b/protocols/api/src/main/java/org/apache/james/protocols/api/ProtocolSessionImpl.java
@@ -123,6 +123,11 @@ public class ProtocolSessionImpl implements ProtocolSession {
         return null;
     }
 
+    @Override
+    public Response newCommandNotFoundErrorResponse() {
+        return null;
+    }
+
     /**
      * This implementation just clears the sessions state. Sub-classes should
      * overwrite this if needed
diff --git a/protocols/api/src/main/java/org/apache/james/protocols/api/handler/CommandDispatcher.java b/protocols/api/src/main/java/org/apache/james/protocols/api/handler/CommandDispatcher.java
index bd03ee4..0c9e4b29 100644
--- a/protocols/api/src/main/java/org/apache/james/protocols/api/handler/CommandDispatcher.java
+++ b/protocols/api/src/main/java/org/apache/james/protocols/api/handler/CommandDispatcher.java
@@ -157,6 +157,10 @@ public class CommandDispatcher<SessionT extends ProtocolSession> implements Exte
         List<CommandHandler<SessionT>> commandHandlers = getCommandHandlers(request.getCommand(), session);
         // fetch the command handlers registered to the command
 
+        if (commandHandlers == null) {
+            return session.newCommandNotFoundErrorResponse();
+        }
+
         for (CommandHandler<SessionT> commandHandler : commandHandlers) {
             final long start = System.currentTimeMillis();
             Response response = commandHandler.onCommand(session, request);
diff --git a/protocols/pop3/src/main/java/org/apache/james/protocols/pop3/POP3SessionImpl.java b/protocols/pop3/src/main/java/org/apache/james/protocols/pop3/POP3SessionImpl.java
index 81cc68a..d8d707a 100644
--- a/protocols/pop3/src/main/java/org/apache/james/protocols/pop3/POP3SessionImpl.java
+++ b/protocols/pop3/src/main/java/org/apache/james/protocols/pop3/POP3SessionImpl.java
@@ -75,4 +75,9 @@ public class POP3SessionImpl extends ProtocolSessionImpl implements POP3Session
     public Response newFatalErrorResponse() {
         return POP3Response.ERR;
     }
+
+    @Override
+    public Response newCommandNotFoundErrorResponse() {
+        return POP3Response.ERR;
+    }
 }
diff --git a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/SMTPSessionImpl.java b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/SMTPSessionImpl.java
index 6b02cf4..86a70db 100644
--- a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/SMTPSessionImpl.java
+++ b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/SMTPSessionImpl.java
@@ -32,7 +32,8 @@ public class SMTPSessionImpl extends ProtocolSessionImpl implements SMTPSession
 
     private static final Response LINE_LENGTH_EXCEEDED = new SMTPResponse(SMTPRetCode.SYNTAX_ERROR_COMMAND_UNRECOGNIZED, "Line length exceeded. See RFC 2821 #4.5.3.1.").immutable();
     private static final Response FATAL_ERROR = new SMTPResponse(SMTPRetCode.LOCAL_ERROR, "Unable to process request").immutable();
-    
+    private static final Response UNKNOWN_COMMAND_ERROR = new SMTPResponse(SMTPRetCode.SYNTAX_ERROR_COMMAND_UNRECOGNIZED, "Unable to process request: the command is unknown").immutable();
+
     private boolean relayingAllowed;
     private boolean needsCommandInjectionDetection;
     
@@ -102,6 +103,11 @@ public class SMTPSessionImpl extends ProtocolSessionImpl implements SMTPSession
     }
 
     @Override
+    public Response newCommandNotFoundErrorResponse() {
+        return UNKNOWN_COMMAND_ERROR;
+    }
+
+    @Override
     public SMTPConfiguration getConfiguration() {
         return (SMTPConfiguration) config;
     }
diff --git a/protocols/smtp/src/test/java/org/apache/james/protocols/smtp/utils/BaseFakeSMTPSession.java b/protocols/smtp/src/test/java/org/apache/james/protocols/smtp/utils/BaseFakeSMTPSession.java
index af819ee..46320f2 100644
--- a/protocols/smtp/src/test/java/org/apache/james/protocols/smtp/utils/BaseFakeSMTPSession.java
+++ b/protocols/smtp/src/test/java/org/apache/james/protocols/smtp/utils/BaseFakeSMTPSession.java
@@ -133,6 +133,11 @@ public class BaseFakeSMTPSession implements SMTPSession {
     }
 
     @Override
+    public Response newCommandNotFoundErrorResponse() {
+        throw new UnsupportedOperationException("Unimplemented Stub Method");
+    }
+
+    @Override
     public InetSocketAddress getRemoteAddress() {
         return new InetSocketAddress("localhost", 22);
     }
diff --git a/server/protocols/protocols-lmtp/src/test/java/org/apache/james/lmtpserver/LmtpServerTest.java b/server/protocols/protocols-lmtp/src/test/java/org/apache/james/lmtpserver/LmtpServerTest.java
index 4deaf26..e624f15 100644
--- a/server/protocols/protocols-lmtp/src/test/java/org/apache/james/lmtpserver/LmtpServerTest.java
+++ b/server/protocols/protocols-lmtp/src/test/java/org/apache/james/lmtpserver/LmtpServerTest.java
@@ -338,6 +338,28 @@ class LmtpServerTest {
                     .isEqualTo(1))
                 .doesNotThrowAnyException();
         }
+
+        @Test
+        void ehloShouldBeRejected() throws Exception {
+            SocketChannel server = SocketChannel.open();
+            server.connect(new InetSocketAddress(LOCALHOST_IP, getLmtpPort(lmtpServerFactory)));
+            readBytes(server);
+
+            server.write(ByteBuffer.wrap(("EHLO <" + DOMAIN + ">\r\n").getBytes(StandardCharsets.UTF_8)));
+            assertThat(new String(readBytes(server), StandardCharsets.UTF_8))
+                .contains("500 Unable to process request: the command is unknown");
+        }
+
+        @Test
+        void heloShouldBeRejected() throws Exception {
+            SocketChannel server = SocketChannel.open();
+            server.connect(new InetSocketAddress(LOCALHOST_IP, getLmtpPort(lmtpServerFactory)));
+            readBytes(server);
+
+            server.write(ByteBuffer.wrap(("HELO <" + DOMAIN + ">\r\n").getBytes(StandardCharsets.UTF_8)));
+            assertThat(new String(readBytes(server), StandardCharsets.UTF_8))
+                .contains("500 Unable to process request: the command is unknown");
+        }
     }
 
     private byte[] readBytes(SocketChannel channel) throws IOException {

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