Repository: james-project Updated Branches: refs/heads/master 0ecf6d6a0 -> 71f89a8f8
JAMES-1954 Disablecommand injection detection on DATA Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/81fc0061 Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/81fc0061 Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/81fc0061 Branch: refs/heads/master Commit: 81fc006154c3bf2a136006a27c5501bc3d153583 Parents: 381f2d7 Author: benwa <btell...@linagora.com> Authored: Mon Mar 6 16:58:30 2017 +0700 Committer: benwa <btell...@linagora.com> Committed: Mon Mar 13 18:32:07 2017 +0700 ---------------------------------------------------------------------- .../netty/AbstractChannelPipelineFactory.java | 2 +- .../protocols/netty/ChannelHandlerFactory.java | 3 ++- .../LineDelimiterBasedChannelHandlerFactory.java | 3 ++- .../smtp/AllButStartTlsDelimiterChannelHandler.java | 16 ++++++++++++---- ...tStartTlsLineDelimiterChannelHandlerFactory.java | 5 +++-- .../apache/james/protocols/smtp/SMTPSession.java | 6 ++++++ .../james/protocols/smtp/SMTPSessionImpl.java | 16 ++++++++++++++++ .../james/protocols/smtp/core/DataCmdHandler.java | 3 ++- .../protocols/smtp/utils/BaseFakeSMTPSession.java | 15 +++++++++++++++ .../apache/james/imapserver/netty/IMAPServer.java | 2 +- ...chableLineDelimiterBasedFrameDecoderFactory.java | 3 ++- .../managesieveserver/netty/ManageSieveServer.java | 2 +- 12 files changed, 63 insertions(+), 13 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/81fc0061/protocols/netty/src/main/java/org/apache/james/protocols/netty/AbstractChannelPipelineFactory.java ---------------------------------------------------------------------- diff --git a/protocols/netty/src/main/java/org/apache/james/protocols/netty/AbstractChannelPipelineFactory.java b/protocols/netty/src/main/java/org/apache/james/protocols/netty/AbstractChannelPipelineFactory.java index 04a91ac..a09e665 100644 --- a/protocols/netty/src/main/java/org/apache/james/protocols/netty/AbstractChannelPipelineFactory.java +++ b/protocols/netty/src/main/java/org/apache/james/protocols/netty/AbstractChannelPipelineFactory.java @@ -73,7 +73,7 @@ public abstract class AbstractChannelPipelineFactory implements ChannelPipelineF // Add the text line decoder which limit the max line length, don't strip the delimiter and use CRLF as delimiter - pipeline.addLast(HandlerConstants.FRAMER, frameHandlerFactory.create()); + pipeline.addLast(HandlerConstants.FRAMER, frameHandlerFactory.create(pipeline)); // Add the ChunkedWriteHandler to be able to write ChunkInput pipeline.addLast(HandlerConstants.CHUNK_HANDLER, new ChunkedWriteHandler()); http://git-wip-us.apache.org/repos/asf/james-project/blob/81fc0061/protocols/netty/src/main/java/org/apache/james/protocols/netty/ChannelHandlerFactory.java ---------------------------------------------------------------------- diff --git a/protocols/netty/src/main/java/org/apache/james/protocols/netty/ChannelHandlerFactory.java b/protocols/netty/src/main/java/org/apache/james/protocols/netty/ChannelHandlerFactory.java index eba8210..771ddb2 100644 --- a/protocols/netty/src/main/java/org/apache/james/protocols/netty/ChannelHandlerFactory.java +++ b/protocols/netty/src/main/java/org/apache/james/protocols/netty/ChannelHandlerFactory.java @@ -19,8 +19,9 @@ package org.apache.james.protocols.netty; import org.jboss.netty.channel.ChannelHandler; +import org.jboss.netty.channel.ChannelPipeline; public interface ChannelHandlerFactory { - ChannelHandler create(); + ChannelHandler create(ChannelPipeline pipeline); } http://git-wip-us.apache.org/repos/asf/james-project/blob/81fc0061/protocols/netty/src/main/java/org/apache/james/protocols/netty/LineDelimiterBasedChannelHandlerFactory.java ---------------------------------------------------------------------- diff --git a/protocols/netty/src/main/java/org/apache/james/protocols/netty/LineDelimiterBasedChannelHandlerFactory.java b/protocols/netty/src/main/java/org/apache/james/protocols/netty/LineDelimiterBasedChannelHandlerFactory.java index dcd9b7c..63e39b5 100644 --- a/protocols/netty/src/main/java/org/apache/james/protocols/netty/LineDelimiterBasedChannelHandlerFactory.java +++ b/protocols/netty/src/main/java/org/apache/james/protocols/netty/LineDelimiterBasedChannelHandlerFactory.java @@ -19,6 +19,7 @@ package org.apache.james.protocols.netty; import org.jboss.netty.channel.ChannelHandler; +import org.jboss.netty.channel.ChannelPipeline; import org.jboss.netty.handler.codec.frame.DelimiterBasedFrameDecoder; import org.jboss.netty.handler.codec.frame.Delimiters; @@ -30,7 +31,7 @@ public class LineDelimiterBasedChannelHandlerFactory implements ChannelHandlerFa } @Override - public ChannelHandler create() { + public ChannelHandler create(ChannelPipeline pipeline) { return new DelimiterBasedFrameDecoder(maxLineLength, false, Delimiters.lineDelimiter()); } http://git-wip-us.apache.org/repos/asf/james-project/blob/81fc0061/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 ad0db9c..9d8998e 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,9 +18,11 @@ ****************************************************************/ package org.apache.james.protocols.smtp; +import org.apache.james.protocols.netty.HandlerConstants; import org.jboss.netty.buffer.ChannelBuffer; import org.jboss.netty.channel.Channel; 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.Charsets; @@ -28,16 +30,22 @@ import com.google.common.base.Charsets; public class AllButStartTlsDelimiterChannelHandler extends DelimiterBasedFrameDecoder { private static final String STARTTLS = "starttls"; + private final ChannelPipeline pipeline; - public AllButStartTlsDelimiterChannelHandler(int maxFrameLength, boolean stripDelimiter, ChannelBuffer[] delimiters) { + public AllButStartTlsDelimiterChannelHandler(ChannelPipeline pipeline, int maxFrameLength, boolean stripDelimiter, ChannelBuffer[] delimiters) { super(maxFrameLength, stripDelimiter, delimiters); + this.pipeline = pipeline; } @Override protected Object decode(ChannelHandlerContext ctx, Channel channel, ChannelBuffer buffer) throws Exception { - String trimedLowerCasedInput = readAll(buffer).trim().toLowerCase(); - if (hasCommandInjection(trimedLowerCasedInput)) { - throw new CommandInjectionDetectedException(); + SMTPSession session = (SMTPSession) pipeline.getContext(HandlerConstants.CORE_HANDLER).getAttachment(); + + if (session.needsCommandInjectionDetection()) { + String trimedLowerCasedInput = readAll(buffer).trim().toLowerCase(); + if (hasCommandInjection(trimedLowerCasedInput)) { + throw new CommandInjectionDetectedException(); + } } return super.decode(ctx, channel, buffer); } http://git-wip-us.apache.org/repos/asf/james-project/blob/81fc0061/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/AllButStartTlsLineDelimiterChannelHandlerFactory.java ---------------------------------------------------------------------- diff --git a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/AllButStartTlsLineDelimiterChannelHandlerFactory.java b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/AllButStartTlsLineDelimiterChannelHandlerFactory.java index de33ac6..22939ff 100644 --- a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/AllButStartTlsLineDelimiterChannelHandlerFactory.java +++ b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/AllButStartTlsLineDelimiterChannelHandlerFactory.java @@ -20,6 +20,7 @@ package org.apache.james.protocols.smtp; import org.apache.james.protocols.netty.ChannelHandlerFactory; import org.jboss.netty.channel.ChannelHandler; +import org.jboss.netty.channel.ChannelPipeline; import org.jboss.netty.handler.codec.frame.Delimiters; public class AllButStartTlsLineDelimiterChannelHandlerFactory implements ChannelHandlerFactory { @@ -31,7 +32,7 @@ public class AllButStartTlsLineDelimiterChannelHandlerFactory implements Channel } @Override - public ChannelHandler create() { - return new AllButStartTlsDelimiterChannelHandler(maxFrameLength, false, Delimiters.lineDelimiter()); + public ChannelHandler create(ChannelPipeline pipeline) { + return new AllButStartTlsDelimiterChannelHandler(pipeline, maxFrameLength, false, Delimiters.lineDelimiter()); } } http://git-wip-us.apache.org/repos/asf/james-project/blob/81fc0061/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/SMTPSession.java ---------------------------------------------------------------------- diff --git a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/SMTPSession.java b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/SMTPSession.java index bdc6bd3..71c6714 100644 --- a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/SMTPSession.java +++ b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/SMTPSession.java @@ -51,6 +51,12 @@ public interface SMTPSession extends ProtocolSession{ * @return the relaying status */ boolean isRelayingAllowed(); + + boolean needsCommandInjectionDetection(); + + void startDetectingCommadInjection(); + + void stopDetectingCommandInjection(); /** * Set if reallying is allowed http://git-wip-us.apache.org/repos/asf/james-project/blob/81fc0061/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/SMTPSessionImpl.java ---------------------------------------------------------------------- diff --git a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/SMTPSessionImpl.java b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/SMTPSessionImpl.java index b4662da..124e30f 100644 --- a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/SMTPSessionImpl.java +++ b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/SMTPSessionImpl.java @@ -36,12 +36,28 @@ public class SMTPSessionImpl extends ProtocolSessionImpl implements SMTPSession private static final Response FATAL_ERROR = new SMTPResponse(SMTPRetCode.LOCAL_ERROR, "Unable to process request").immutable(); private boolean relayingAllowed; + private boolean needsCommandInjectionDetection; public SMTPSessionImpl(Logger logger, ProtocolTransport transport, SMTPConfiguration config) { super(logger, transport, config); relayingAllowed = config.isRelayingAllowed(getRemoteAddress().getAddress().getHostAddress()); + needsCommandInjectionDetection = true; } + @Override + public boolean needsCommandInjectionDetection() { + return needsCommandInjectionDetection; + } + + @Override + public void startDetectingCommadInjection() { + needsCommandInjectionDetection = true; + } + + @Override + public void stopDetectingCommandInjection() { + needsCommandInjectionDetection = false; + } /** * @see org.apache.james.protocols.smtp.SMTPSession#isRelayingAllowed() http://git-wip-us.apache.org/repos/asf/james-project/blob/81fc0061/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/DataCmdHandler.java ---------------------------------------------------------------------- diff --git a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/DataCmdHandler.java b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/DataCmdHandler.java index 0f51c9f..c68be56 100644 --- a/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/DataCmdHandler.java +++ b/protocols/smtp/src/main/java/org/apache/james/protocols/smtp/core/DataCmdHandler.java @@ -138,7 +138,7 @@ public class DataCmdHandler implements CommandHandler<SMTPSession>, ExtensibleHa */ public Response onCommand(SMTPSession session, Request request) { TimeMetric timeMetric = metricFactory.timer("SMTP-" + request.getCommand()); - + session.stopDetectingCommandInjection(); try { String parameters = request.getArgument(); Response response = doDATAFilter(session, parameters); @@ -150,6 +150,7 @@ public class DataCmdHandler implements CommandHandler<SMTPSession>, ExtensibleHa } } finally { timeMetric.stopAndPublish(); + session.needsCommandInjectionDetection(); } } http://git-wip-us.apache.org/repos/asf/james-project/blob/81fc0061/protocols/smtp/src/test/java/org/apache/james/protocols/smtp/utils/BaseFakeSMTPSession.java ---------------------------------------------------------------------- diff --git a/protocols/smtp/src/test/java/org/apache/james/protocols/smtp/utils/BaseFakeSMTPSession.java b/protocols/smtp/src/test/java/org/apache/james/protocols/smtp/utils/BaseFakeSMTPSession.java index 396d63a..5b2e371 100644 --- a/protocols/smtp/src/test/java/org/apache/james/protocols/smtp/utils/BaseFakeSMTPSession.java +++ b/protocols/smtp/src/test/java/org/apache/james/protocols/smtp/utils/BaseFakeSMTPSession.java @@ -39,6 +39,21 @@ public class BaseFakeSMTPSession implements SMTPSession { private static final Logger log = new MockLogger(); + @Override + public boolean needsCommandInjectionDetection() { + throw new UnsupportedOperationException("Unimplemented Stub Method"); + } + + @Override + public void startDetectingCommadInjection() { + throw new UnsupportedOperationException("Unimplemented Stub Method"); + } + + @Override + public void stopDetectingCommandInjection() { + throw new UnsupportedOperationException("Unimplemented Stub Method"); + } + /** * @see org.apache.james.protocols.smtp.SMTPSession#getConnectionState() */ http://git-wip-us.apache.org/repos/asf/james-project/blob/81fc0061/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/IMAPServer.java ---------------------------------------------------------------------- diff --git a/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/IMAPServer.java b/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/IMAPServer.java index cc9c300..06488c3 100644 --- a/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/IMAPServer.java +++ b/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/IMAPServer.java @@ -139,7 +139,7 @@ public class IMAPServer extends AbstractConfigurableAsyncServer implements ImapC // Add the text line decoder which limit the max line length, // don't strip the delimiter and use CRLF as delimiter // Use a SwitchableDelimiterBasedFrameDecoder, see JAMES-1436 - pipeline.addLast(FRAMER, getFrameHandlerFactory().create()); + pipeline.addLast(FRAMER, getFrameHandlerFactory().create(pipeline)); Encryption secure = getEncryption(); if (secure != null && !secure.isStartTLS()) { http://git-wip-us.apache.org/repos/asf/james-project/blob/81fc0061/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/SwitchableLineDelimiterBasedFrameDecoderFactory.java ---------------------------------------------------------------------- diff --git a/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/SwitchableLineDelimiterBasedFrameDecoderFactory.java b/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/SwitchableLineDelimiterBasedFrameDecoderFactory.java index 1335ae3..0350f09 100644 --- a/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/SwitchableLineDelimiterBasedFrameDecoderFactory.java +++ b/server/protocols/protocols-imap4/src/main/java/org/apache/james/imapserver/netty/SwitchableLineDelimiterBasedFrameDecoderFactory.java @@ -20,6 +20,7 @@ package org.apache.james.imapserver.netty; import org.apache.james.protocols.netty.ChannelHandlerFactory; import org.jboss.netty.channel.ChannelHandler; +import org.jboss.netty.channel.ChannelPipeline; import org.jboss.netty.handler.codec.frame.Delimiters; public class SwitchableLineDelimiterBasedFrameDecoderFactory implements ChannelHandlerFactory { @@ -31,7 +32,7 @@ public class SwitchableLineDelimiterBasedFrameDecoderFactory implements ChannelH } @Override - public ChannelHandler create() { + public ChannelHandler create(ChannelPipeline pipeline) { return new SwitchableDelimiterBasedFrameDecoder(maxLineLength, false, Delimiters.lineDelimiter()); } } http://git-wip-us.apache.org/repos/asf/james-project/blob/81fc0061/server/protocols/protocols-managesieve/src/main/java/org/apache/james/managesieveserver/netty/ManageSieveServer.java ---------------------------------------------------------------------- diff --git a/server/protocols/protocols-managesieve/src/main/java/org/apache/james/managesieveserver/netty/ManageSieveServer.java b/server/protocols/protocols-managesieve/src/main/java/org/apache/james/managesieveserver/netty/ManageSieveServer.java index 827fa40..9ad91c8 100644 --- a/server/protocols/protocols-managesieve/src/main/java/org/apache/james/managesieveserver/netty/ManageSieveServer.java +++ b/server/protocols/protocols-managesieve/src/main/java/org/apache/james/managesieveserver/netty/ManageSieveServer.java @@ -114,7 +114,7 @@ public class ManageSieveServer extends AbstractConfigurableAsyncServer implement // Add the text line decoder which limit the max line length, // don't strip the delimiter and use CRLF as delimiter // Use a SwitchableDelimiterBasedFrameDecoder, see JAMES-1436 - pipeline.addLast(FRAMER, getFrameHandlerFactory().create()); + pipeline.addLast(FRAMER, getFrameHandlerFactory().create(pipeline)); pipeline.addLast(CONNECTION_COUNT_HANDLER, getConnectionCountHandler()); pipeline.addLast(CHUNK_WRITE_HANDLER, new ChunkedWriteHandler()); --------------------------------------------------------------------- To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org
