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 <[email protected]> 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: [email protected] For additional commands, e-mail: [email protected]
