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 <btell...@linagora.com> Authored: Mon Mar 6 17:33:53 2017 +0700 Committer: benwa <btell...@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("start...@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:<matth...@yopmail.com> +C: rcpt to:<start...@mydomain.tld> +C: data +S: 250.* +S: 250-PIPELINING +S: 250-ENHANCEDSTATUSCODES +S: 250-8BITMIME +S: 250 STARTTLS +S: 250 2.1.0 Sender <matth...@yopmail.com> OK +S: 250 2.1.5 Recipient <start...@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-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org
