james-server-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From btell...@apache.org
Subject [1/5] james-project git commit: JAMES-1954 Disablecommand injection detection on DATA
Date Mon, 13 Mar 2017 11:33:22 GMT
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 <btellier@linagora.com>
Authored: Mon Mar 6 16:58:30 2017 +0700
Committer: benwa <btellier@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-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org


Mime
View raw message