james-server-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From adup...@apache.org
Subject [10/12] james-project git commit: JAMES-1862 Dont frame SMTP commands when STARTTLS is present
Date Tue, 06 Dec 2016 12:25:28 GMT
JAMES-1862 Dont frame SMTP commands when STARTTLS is present


Project: http://git-wip-us.apache.org/repos/asf/james-project/repo
Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/b4f1ed6a
Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/b4f1ed6a
Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/b4f1ed6a

Branch: refs/heads/master
Commit: b4f1ed6ab5ad08feb1d4ad3ac5ef7aa2a2b7a893
Parents: b61399a
Author: Antoine Duprat <aduprat@apache.org>
Authored: Thu Dec 1 13:38:42 2016 +0100
Committer: Antoine Duprat <aduprat@linagora.com>
Committed: Tue Dec 6 11:31:26 2016 +0100

----------------------------------------------------------------------
 protocols/smtp/pom.xml                          |  4 ++
 .../AllButStartTlsDelimiterChannelHandler.java  | 53 ++++++++++++++++++++
 .../smtp/CommandInjectionDetectedException.java | 23 +++++++++
 .../smtp/netty/NettyStartTlsSMTPServerTest.java | 18 ++++++-
 .../james/smtpserver/netty/SMTPServer.java      |  4 +-
 .../apache/james/smtpserver/SMTPServerTest.java | 40 +++++++++++++++
 6 files changed, 138 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/james-project/blob/b4f1ed6a/protocols/smtp/pom.xml
----------------------------------------------------------------------
diff --git a/protocols/smtp/pom.xml b/protocols/smtp/pom.xml
index aaf2b49..a9f913f 100644
--- a/protocols/smtp/pom.xml
+++ b/protocols/smtp/pom.xml
@@ -64,6 +64,10 @@
             <scope>test</scope>
         </dependency>
         <dependency>
+            <groupId>io.netty</groupId>
+            <artifactId>netty</artifactId>
+        </dependency>
+        <dependency>
             <groupId>junit</groupId>
             <artifactId>junit</artifactId>
             <scope>test</scope>

http://git-wip-us.apache.org/repos/asf/james-project/blob/b4f1ed6a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/AllButStartTlsDelimiterChannelHandler.java
----------------------------------------------------------------------
diff --git a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/AllButStartTlsDelimiterChannelHandler.java
b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/AllButStartTlsDelimiterChannelHandler.java
new file mode 100644
index 0000000..ad0db9c
--- /dev/null
+++ b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/AllButStartTlsDelimiterChannelHandler.java
@@ -0,0 +1,53 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one   *
+ * or more contributor license agreements.  See the NOTICE file *
+ * distributed with this work for additional information        *
+ * regarding copyright ownership.  The ASF licenses this file   *
+ * to you under the Apache License, Version 2.0 (the            *
+ * "License"); you may not use this file except in compliance   *
+ * with the License.  You may obtain a copy of the License at   *
+ *                                                              *
+ *   http://www.apache.org/licenses/LICENSE-2.0                 *
+ *                                                              *
+ * Unless required by applicable law or agreed to in writing,   *
+ * software distributed under the License is distributed on an  *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY       *
+ * KIND, either express or implied.  See the License for the    *
+ * specific language governing permissions and limitations      *
+ * under the License.                                           *
+ ****************************************************************/
+package org.apache.james.protocols.smtp;
+
+import org.jboss.netty.buffer.ChannelBuffer;
+import org.jboss.netty.channel.Channel;
+import org.jboss.netty.channel.ChannelHandlerContext;
+import org.jboss.netty.handler.codec.frame.DelimiterBasedFrameDecoder;
+
+import com.google.common.base.Charsets;
+
+public class AllButStartTlsDelimiterChannelHandler extends DelimiterBasedFrameDecoder {
+
+    private static final String STARTTLS = "starttls";
+
+    public AllButStartTlsDelimiterChannelHandler(int maxFrameLength, boolean stripDelimiter,
ChannelBuffer[] delimiters) {
+        super(maxFrameLength, stripDelimiter, delimiters);
+    }
+
+    @Override
+    protected Object decode(ChannelHandlerContext ctx, Channel channel, ChannelBuffer buffer)
throws Exception {
+        String trimedLowerCasedInput = readAll(buffer).trim().toLowerCase();
+        if (hasCommandInjection(trimedLowerCasedInput)) {
+            throw new CommandInjectionDetectedException();
+        }
+        return super.decode(ctx, channel, buffer);
+    }
+
+    private String readAll(ChannelBuffer buffer) {
+        return buffer.toString(Charsets.US_ASCII);
+    }
+
+    private boolean hasCommandInjection(String trimedLowerCasedInput) {
+        return trimedLowerCasedInput.contains(STARTTLS)
+                && !trimedLowerCasedInput.endsWith(STARTTLS);
+    }
+}

http://git-wip-us.apache.org/repos/asf/james-project/blob/b4f1ed6a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/CommandInjectionDetectedException.java
----------------------------------------------------------------------
diff --git a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/CommandInjectionDetectedException.java
b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/CommandInjectionDetectedException.java
new file mode 100644
index 0000000..1c15346
--- /dev/null
+++ b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/CommandInjectionDetectedException.java
@@ -0,0 +1,23 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one   *
+ * or more contributor license agreements.  See the NOTICE file *
+ * distributed with this work for additional information        *
+ * regarding copyright ownership.  The ASF licenses this file   *
+ * to you under the Apache License, Version 2.0 (the            *
+ * "License"); you may not use this file except in compliance   *
+ * with the License.  You may obtain a copy of the License at   *
+ *                                                              *
+ *   http://www.apache.org/licenses/LICENSE-2.0                 *
+ *                                                              *
+ * Unless required by applicable law or agreed to in writing,   *
+ * software distributed under the License is distributed on an  *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY       *
+ * KIND, either express or implied.  See the License for the    *
+ * specific language governing permissions and limitations      *
+ * under the License.                                           *
+ ****************************************************************/
+package org.apache.james.protocols.smtp;
+
+public class CommandInjectionDetectedException extends RuntimeException {
+
+}

http://git-wip-us.apache.org/repos/asf/james-project/blob/b4f1ed6a/protocols/smtp/src/test/java/org/apache/james/protocols/smtp/netty/NettyStartTlsSMTPServerTest.java
----------------------------------------------------------------------
diff --git a/protocols/smtp/src/test/java/org/apache/james/protocols/smtp/netty/NettyStartTlsSMTPServerTest.java
b/protocols/smtp/src/test/java/org/apache/james/protocols/smtp/netty/NettyStartTlsSMTPServerTest.java
index e8c16af..161985c 100644
--- a/protocols/smtp/src/test/java/org/apache/james/protocols/smtp/netty/NettyStartTlsSMTPServerTest.java
+++ b/protocols/smtp/src/test/java/org/apache/james/protocols/smtp/netty/NettyStartTlsSMTPServerTest.java
@@ -44,12 +44,12 @@ import org.apache.james.protocols.api.utils.MockLogger;
 import org.apache.james.protocols.api.utils.TestUtils;
 import org.apache.james.protocols.netty.AbstractChannelPipelineFactory;
 import org.apache.james.protocols.netty.NettyServer;
+import org.apache.james.protocols.smtp.AllButStartTlsDelimiterChannelHandler;
 import org.apache.james.protocols.smtp.SMTPConfigurationImpl;
 import org.apache.james.protocols.smtp.SMTPProtocol;
 import org.apache.james.protocols.smtp.SMTPProtocolHandlerChain;
 import org.apache.james.protocols.smtp.utils.TestMessageHook;
 import org.assertj.core.api.AssertDelegateTarget;
-import org.jboss.netty.handler.codec.frame.DelimiterBasedFrameDecoder;
 import org.jboss.netty.handler.codec.frame.Delimiters;
 import org.junit.After;
 import org.junit.Test;
@@ -72,7 +72,7 @@ public class NettyStartTlsSMTPServerTest {
         NettyServer server = NettyServer.builder()
                 .protocol(protocol)
                 .secure(enc)
-                .frameHandler(new DelimiterBasedFrameDecoder(AbstractChannelPipelineFactory.MAX_LINE_LENGTH,
false, Delimiters.lineDelimiter()))
+                .frameHandler(new AllButStartTlsDelimiterChannelHandler(AbstractChannelPipelineFactory.MAX_LINE_LENGTH,
false, Delimiters.lineDelimiter()))
                 .build();
         server.setListenAddresses(address);
         return server;
@@ -176,6 +176,20 @@ public class NettyStartTlsSMTPServerTest {
     }
 
     @Test
+    public void startTlsShouldFailWhenFollowedByInjectedCommand() throws Exception {
+        InetSocketAddress address = new InetSocketAddress("127.0.0.1", TestUtils.getFreePort());
+        ProtocolServer server = createServer(createProtocol(Optional.<ProtocolHandler>
absent()), address, Encryption.createStartTls(BogusSslContextFactory.getServerContext()));
 
+        server.bind();
+
+        SMTPSClient client = createClient();
+        client.connect(address.getAddress().getHostAddress(), address.getPort());
+        client.sendCommand("EHLO localhost");
+
+        client.sendCommand("STARTTLS\r\nRSET\r\n");
+        assertThat(SMTPReply.isPositiveCompletion(client.getReplyCode())).isFalse();
+    }
+
+    @Test
     public void startTlsShouldWorkWhenUsingJavamail() throws Exception {
         TestMessageHook hook = new TestMessageHook();
         InetSocketAddress address = new InetSocketAddress("127.0.0.1", TestUtils.getFreePort());

http://git-wip-us.apache.org/repos/asf/james-project/blob/b4f1ed6a/server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/netty/SMTPServer.java
----------------------------------------------------------------------
diff --git a/server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/netty/SMTPServer.java
b/server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/netty/SMTPServer.java
index c7d291c..cad8941 100644
--- a/server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/netty/SMTPServer.java
+++ b/server/protocols/protocols-smtp/src/main/java/org/apache/james/smtpserver/netty/SMTPServer.java
@@ -30,6 +30,7 @@ import org.apache.james.protocols.api.logger.ProtocolLoggerAdapter;
 import org.apache.james.protocols.lib.handler.HandlersPackage;
 import org.apache.james.protocols.lib.netty.AbstractProtocolAsyncServer;
 import org.apache.james.protocols.netty.AbstractChannelPipelineFactory;
+import org.apache.james.protocols.smtp.AllButStartTlsDelimiterChannelHandler;
 import org.apache.james.protocols.smtp.SMTPConfiguration;
 import org.apache.james.protocols.smtp.SMTPProtocol;
 import org.apache.james.smtpserver.CoreCmdHandlerLoader;
@@ -37,7 +38,6 @@ import org.apache.james.smtpserver.ExtendedSMTPSession;
 import org.apache.james.smtpserver.jmx.JMXHandlersLoader;
 import org.jboss.netty.channel.ChannelHandler;
 import org.jboss.netty.channel.ChannelUpstreamHandler;
-import org.jboss.netty.handler.codec.frame.DelimiterBasedFrameDecoder;
 import org.jboss.netty.handler.codec.frame.Delimiters;
 
 /**
@@ -365,7 +365,7 @@ public class SMTPServer extends AbstractProtocolAsyncServer implements
SMTPServe
 
     @Override
     protected ChannelHandler createFrameHandler() {
-        return new DelimiterBasedFrameDecoder(AbstractChannelPipelineFactory.MAX_LINE_LENGTH,
false, Delimiters.lineDelimiter());
+        return new AllButStartTlsDelimiterChannelHandler(AbstractChannelPipelineFactory.MAX_LINE_LENGTH,
false, Delimiters.lineDelimiter());
     }
 
 }

http://git-wip-us.apache.org/repos/asf/james-project/blob/b4f1ed6a/server/protocols/protocols-smtp/src/test/java/org/apache/james/smtpserver/SMTPServerTest.java
----------------------------------------------------------------------
diff --git a/server/protocols/protocols-smtp/src/test/java/org/apache/james/smtpserver/SMTPServerTest.java
b/server/protocols/protocols-smtp/src/test/java/org/apache/james/smtpserver/SMTPServerTest.java
index 578b824..0efae7d 100644
--- a/server/protocols/protocols-smtp/src/test/java/org/apache/james/smtpserver/SMTPServerTest.java
+++ b/server/protocols/protocols-smtp/src/test/java/org/apache/james/smtpserver/SMTPServerTest.java
@@ -499,6 +499,46 @@ public class SMTPServerTest {
 
     }
 
+    @Test
+    public void startTlsCommandShouldWorkWhenAlone() throws Exception {
+        smtpConfiguration.setStartTLS();
+        init(smtpConfiguration);
+
+        SMTPClient smtpProtocol = new SMTPClient();
+        smtpProtocol.connect("127.0.0.1", smtpListenerPort);
+
+        // no message there, yet
+        assertThat(queue.getLastMail())
+            .as("no mail received by mail server")
+            .isNull();;
+
+        smtpProtocol.sendCommand("EHLO " + InetAddress.getLocalHost());
+        smtpProtocol.sendCommand("STARTTLS");
+        assertThat(smtpProtocol.getReplyCode()).isEqualTo(220);
+        
+        smtpProtocol.disconnect();
+    }
+
+    @Test
+    public void startTlsCommandShouldFailWhenFollowedByInjectedCommand() throws Exception
{
+        smtpConfiguration.setStartTLS();
+        init(smtpConfiguration);
+
+        SMTPClient smtpProtocol = new SMTPClient();
+        smtpProtocol.connect("127.0.0.1", smtpListenerPort);
+
+        // no message there, yet
+        assertThat(queue.getLastMail())
+            .as("no mail received by mail server")
+            .isNull();;
+
+        smtpProtocol.sendCommand("EHLO " + InetAddress.getLocalHost());
+        smtpProtocol.sendCommand("STARTTLS\r\nAUTH PLAIN");
+        assertThat(smtpProtocol.getReplyCode()).isEqualTo(451);
+        
+        smtpProtocol.disconnect();
+    }
+
     protected SMTPClient newSMTPClient() throws IOException {
         SMTPClient smtp = new SMTPClient();
         smtp.connect("127.0.0.1", smtpListenerPort);


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


Mime
View raw message