Revert "JAMES-1862 Dont frame SMTP commands when STARTTLS is present"
This reverts commit ffc98b795e1e20f6387385e6323bab1e425ecd5d. Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/f0c1792a Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/f0c1792a Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/f0c1792a Branch: refs/heads/master Commit: f0c1792a2913bca02f32307a24ca81fe736a5ef7 Parents: ae47b3b Author: Antoine Duprat <[email protected]> Authored: Tue Dec 6 11:41:07 2016 +0100 Committer: Antoine Duprat <[email protected]> Committed: Tue Dec 6 11:41:07 2016 +0100 ---------------------------------------------------------------------- protocols/smtp/pom.xml | 4 -- .../AllButStartTlsDelimiterChannelHandler.java | 55 -------------------- .../smtp/CommandInjectionDetectedException.java | 23 -------- .../smtp/netty/NettyStartTlsSMTPServerTest.java | 18 +------ .../james/smtpserver/netty/SMTPServer.java | 4 +- .../apache/james/smtpserver/SMTPServerTest.java | 40 -------------- 6 files changed, 4 insertions(+), 140 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/f0c1792a/protocols/smtp/pom.xml ---------------------------------------------------------------------- diff --git a/protocols/smtp/pom.xml b/protocols/smtp/pom.xml index a9f913f..aaf2b49 100644 --- a/protocols/smtp/pom.xml +++ b/protocols/smtp/pom.xml @@ -64,10 +64,6 @@ <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/f0c1792a/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 deleted file mode 100644 index 24e125d..0000000 --- a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/AllButStartTlsDelimiterChannelHandler.java +++ /dev/null @@ -1,55 +0,0 @@ -/**************************************************************** - * 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; - -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) { - StringBuilder builder = new StringBuilder(); - for (int i = 0; i < buffer.capacity(); i++) { - builder.append((char) buffer.getByte(i)); - } - return builder.toString(); - } - - private boolean hasCommandInjection(String trimedLowerCasedInput) { - return trimedLowerCasedInput.contains(STARTTLS) - && !trimedLowerCasedInput.endsWith(STARTTLS); - } -} http://git-wip-us.apache.org/repos/asf/james-project/blob/f0c1792a/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 deleted file mode 100644 index 1c15346..0000000 --- a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/CommandInjectionDetectedException.java +++ /dev/null @@ -1,23 +0,0 @@ -/**************************************************************** - * 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/f0c1792a/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 e08fae3..8bbd1b6 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,11 +44,11 @@ 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.jboss.netty.handler.codec.frame.DelimiterBasedFrameDecoder; import org.jboss.netty.handler.codec.frame.Delimiters; import org.junit.After; import org.junit.Test; @@ -69,7 +69,7 @@ public class NettyStartTlsSMTPServerTest { private ProtocolServer createServer(Protocol protocol, InetSocketAddress address, Encryption enc) { NettyServer server = new NettyServer(protocol, enc, - new AllButStartTlsDelimiterChannelHandler(AbstractChannelPipelineFactory.MAX_LINE_LENGTH, false, Delimiters.lineDelimiter())); + new DelimiterBasedFrameDecoder(AbstractChannelPipelineFactory.MAX_LINE_LENGTH, false, Delimiters.lineDelimiter())); server.setListenAddresses(address); return server; } @@ -162,20 +162,6 @@ 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/f0c1792a/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 cad8941..c7d291c 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,7 +30,6 @@ 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; @@ -38,6 +37,7 @@ 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 AllButStartTlsDelimiterChannelHandler(AbstractChannelPipelineFactory.MAX_LINE_LENGTH, false, Delimiters.lineDelimiter()); + return new DelimiterBasedFrameDecoder(AbstractChannelPipelineFactory.MAX_LINE_LENGTH, false, Delimiters.lineDelimiter()); } } http://git-wip-us.apache.org/repos/asf/james-project/blob/f0c1792a/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 0efae7d..578b824 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,46 +499,6 @@ 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: [email protected] For additional commands, e-mail: [email protected]
