james-server-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From btell...@apache.org
Subject [4/5] james-project git commit: JAMES-1954 Continue improving command injection detection
Date Mon, 13 Mar 2017 11:33:25 GMT
JAMES-1954 Continue improving command injection detection

 - Check we can use startTls as part of other commands
 - Add Test for injection done in middle of the command


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

Branch: refs/heads/master
Commit: ba48fae165e39a3e7600aaa2cb9a1c8d70f86927
Parents: 81fc006
Author: benwa <btellier@linagora.com>
Authored: Mon Mar 6 17:33:53 2017 +0700
Committer: benwa <btellier@linagora.com>
Committed: Mon Mar 13 18:32:20 2017 +0700

----------------------------------------------------------------------
 .../james/mpt/smtp/SmtpStarttlsCommandTest.java |  8 ++++++
 .../james/smtp/scripts/rcpt_with_starttls.test  | 18 ++++++++++++
 .../AllButStartTlsDelimiterChannelHandler.java  | 30 ++++++++++++++++++--
 .../smtp/netty/NettyStartTlsSMTPServerTest.java | 14 +++++++++
 4 files changed, 68 insertions(+), 2 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/james-project/blob/ba48fae1/mpt/impl/smtp/core/src/main/java/org/apache/james/mpt/smtp/SmtpStarttlsCommandTest.java
----------------------------------------------------------------------
diff --git a/mpt/impl/smtp/core/src/main/java/org/apache/james/mpt/smtp/SmtpStarttlsCommandTest.java
b/mpt/impl/smtp/core/src/main/java/org/apache/james/mpt/smtp/SmtpStarttlsCommandTest.java
index 94141d7..764cdc9 100644
--- a/mpt/impl/smtp/core/src/main/java/org/apache/james/mpt/smtp/SmtpStarttlsCommandTest.java
+++ b/mpt/impl/smtp/core/src/main/java/org/apache/james/mpt/smtp/SmtpStarttlsCommandTest.java
@@ -68,6 +68,14 @@ public class SmtpStarttlsCommandTest extends AbstractSimpleScriptedTestProtocol
         scriptTest("data_with_starttls", Locale.US);
     }
 
+
+    @Test
+    public void shouldNotRejectRcptWithStartTls() throws Exception {
+        hostSystem.addUser("starttls@mydomain.tld", PASSWORD);
+
+        scriptTest("rcpt_with_starttls", Locale.US);
+    }
+
     @Test
     public void shouldNotRejectContentStartsWithStartTls() throws Exception {
         scriptTest("data_starts_with_starttls", Locale.US);

http://git-wip-us.apache.org/repos/asf/james-project/blob/ba48fae1/mpt/impl/smtp/core/src/main/resources/org/apache/james/smtp/scripts/rcpt_with_starttls.test
----------------------------------------------------------------------
diff --git a/mpt/impl/smtp/core/src/main/resources/org/apache/james/smtp/scripts/rcpt_with_starttls.test
b/mpt/impl/smtp/core/src/main/resources/org/apache/james/smtp/scripts/rcpt_with_starttls.test
new file mode 100644
index 0000000..efe719b
--- /dev/null
+++ b/mpt/impl/smtp/core/src/main/resources/org/apache/james/smtp/scripts/rcpt_with_starttls.test
@@ -0,0 +1,18 @@
+S: 220 mydomain.tld smtp
+C: ehlo yopmail.com
+C: mail from:<matthieu@yopmail.com>
+C: rcpt to:<starttls@mydomain.tld>
+C: data
+S: 250.*
+S: 250-PIPELINING
+S: 250-ENHANCEDSTATUSCODES
+S: 250-8BITMIME
+S: 250 STARTTLS
+S: 250 2.1.0 Sender <matthieu@yopmail.com> OK
+S: 250 2.1.5 Recipient <starttls@mydomain.tld> OK
+S: 354 Ok Send data ending with <CRLF>.<CRLF>
+C: subject: test
+C:
+C: content content
+C: .
+S: 250 2.6.0 Message received

http://git-wip-us.apache.org/repos/asf/james-project/blob/ba48fae1/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
index 9d8998e..cd3ae21 100644
--- 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
@@ -18,6 +18,8 @@
  ****************************************************************/
 package org.apache.james.protocols.smtp;
 
+import java.util.List;
+
 import org.apache.james.protocols.netty.HandlerConstants;
 import org.jboss.netty.buffer.ChannelBuffer;
 import org.jboss.netty.channel.Channel;
@@ -25,7 +27,11 @@ import org.jboss.netty.channel.ChannelHandlerContext;
 import org.jboss.netty.channel.ChannelPipeline;
 import org.jboss.netty.handler.codec.frame.DelimiterBasedFrameDecoder;
 
+import com.google.common.base.CharMatcher;
 import com.google.common.base.Charsets;
+import com.google.common.base.Predicate;
+import com.google.common.base.Splitter;
+import com.google.common.collect.FluentIterable;
 
 public class AllButStartTlsDelimiterChannelHandler extends DelimiterBasedFrameDecoder {
 
@@ -55,7 +61,27 @@ public class AllButStartTlsDelimiterChannelHandler extends DelimiterBasedFrameDe
     }
 
     private boolean hasCommandInjection(String trimedLowerCasedInput) {
-        return trimedLowerCasedInput.contains(STARTTLS)
-                && !trimedLowerCasedInput.endsWith(STARTTLS);
+        List<String> parts = Splitter.on(CharMatcher.anyOf("\r\n")).omitEmptyStrings()
+            .splitToList(trimedLowerCasedInput);
+
+        return hasInvalidStartTlsPart(parts) || multiPartsAndOneStartTls(parts);
+    }
+
+    private boolean multiPartsAndOneStartTls(List<String> parts) {
+        return FluentIterable.from(parts).anyMatch(new Predicate<String>() {
+            @Override
+            public boolean apply(String line) {
+                return line.startsWith(STARTTLS);
+            }
+        }) && parts.size() > 1;
+    }
+
+    private boolean hasInvalidStartTlsPart(List<String> parts) {
+        return FluentIterable.from(parts).anyMatch(new Predicate<String>() {
+            @Override
+            public boolean apply(String line) {
+                return line.startsWith(STARTTLS) && !line.endsWith(STARTTLS);
+            }
+        });
     }
 }

http://git-wip-us.apache.org/repos/asf/james-project/blob/ba48fae1/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 66d3f81..7fed268 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
@@ -193,6 +193,20 @@ public class NettyStartTlsSMTPServerTest {
     }
 
     @Test
+    public void startTlsShouldFailWhenFollowedByInjectedCommandAndNotAtBeginningOfLine()
throws Exception {
+        ProtocolServer server = createServer(createProtocol(Optional.<ProtocolHandler>
absent()), Encryption.createStartTls(BogusSslContextFactory.getServerContext()));
+        server.bind();
+
+        SMTPSClient client = createClient();
+        InetSocketAddress bindedAddress = new ProtocolServerUtils(server).retrieveBindedAddress();
+        client.connect(bindedAddress.getAddress().getHostAddress(), bindedAddress.getPort());
+        client.sendCommand("EHLO localhost");
+
+        client.sendCommand("RSET\r\nSTARTTLS\r\nRSET\r\n");
+        assertThat(SMTPReply.isPositiveCompletion(client.getReplyCode())).isFalse();
+    }
+
+    @Test
     public void startTlsShouldWorkWhenUsingJavamail() throws Exception {
         TestMessageHook hook = new TestMessageHook();
         ProtocolServer server = createServer(createProtocol(Optional.<ProtocolHandler>
of(hook)) , Encryption.createStartTls(BogusSslContextFactory.getServerContext()));  


---------------------------------------------------------------------
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