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 <adup...@apache.org> Authored: Thu Dec 1 13:38:42 2016 +0100 Committer: Antoine Duprat <adup...@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-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org
