[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption - SASL encryption between Drill Client to Drillbit

[Sorabh Hamirwasia (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16095448#comment-16095448
 ] 

Sorabh Hamirwasia commented on DRILL-4335:
--

Repurposing this JIRA to reflect only SASL encryption support between Drill 
Client and Drillbit. Have created [DRILL-5682 | 
https://issues.apache.org/jira/browse/DRILL-5682] which will act as umbrella 
JIRA for other encryption support.

> Apache Drill should support network encryption - SASL encryption between 
> Drill Client to Drillbit
> -
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: doc-impacting, ready-to-commit, security
> Fix For: 1.11.0
>
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.4.14#64029)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16018647#comment-16018647
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user asfgit closed the pull request at:

https://github.com/apache/drill/pull/773


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: ready-to-commit, security
> Fix For: 1.11.0
>
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15998551#comment-15998551
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r114895578
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/BitConnectionConfig.java 
---
@@ -46,16 +47,34 @@ protected BitConnectionConfig(BufferAllocator 
allocator, BootStrapContext contex
 super(allocator, context);
 
 final DrillConfig config = context.getConfig();
+final AuthenticatorProvider authProvider = getAuthProvider();
+
 if (config.getBoolean(ExecConstants.BIT_AUTHENTICATION_ENABLED)) {
   this.authMechanismToUse = 
config.getString(ExecConstants.BIT_AUTHENTICATION_MECHANISM);
   try {
-getAuthProvider().getAuthenticatorFactory(authMechanismToUse);
+authProvider.getAuthenticatorFactory(authMechanismToUse);
   } catch (final SaslException e) {
 throw new DrillbitStartupException(String.format(
 "'%s' mechanism not found for bit-to-bit authentication. 
Please check authentication configuration.",
 authMechanismToUse));
   }
-  logger.info("Configured bit-to-bit connections to require 
authentication using: {}", authMechanismToUse);
+
+  // Update encryption related configurations
+  
encryptionContext.setEncryption(config.getBoolean(ExecConstants.BIT_ENCRYPTION_SASL_ENABLED));
+  final int maxWrappedSize = 
config.getInt(ExecConstants.BIT_ENCRYPTION_SASL_MAX_WRAPPED_SIZE);
+
+  if (maxWrappedSize <= 0 || maxWrappedSize > 
RpcConstants.MAX_WRAPPED_SIZE) {
+throw new DrillbitStartupException("Invalid value configured for 
bit.encryption.sasl.encodesize." +
--- End diff --

encodesize -> max_wrapped_size


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15998547#comment-15998547
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r115039025
  
--- Diff: 
exec/rpc/src/main/java/org/apache/drill/exec/rpc/AbstractRemoteConnection.java 
---
@@ -20,27 +20,35 @@
 import io.netty.channel.Channel;
 import io.netty.channel.ChannelHandlerContext;
 import io.netty.channel.ChannelInboundHandlerAdapter;
+import io.netty.channel.ChannelPipeline;
 import io.netty.channel.socket.SocketChannel;
+import io.netty.handler.codec.LengthFieldBasedFrameDecoder;
 
 import java.net.SocketAddress;
+import java.nio.ByteOrder;
--- End diff --

??


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15998548#comment-15998548
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r115038537
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserClient.java ---
@@ -201,10 +223,12 @@ protected SaslException mapException(Exception e) {
 if (e instanceof ExecutionException) {
   final Throwable cause = Throwables.getRootCause(e);
   if (cause instanceof SaslException) {
-return new SaslException("Authentication failed: " + 
cause.getMessage(), cause);
+return new SaslException(String.format("Authentication 
failed. [Details: %s, Error %s]",
+connection.getEncryptionCtxtString(), 
cause.getMessage()), cause);
   }
 }
-return new SaslException("Authentication failed 
unexpectedly.", e);
+return new SaslException(String.format("Authentication failed. 
[Details: %s, Error %s",
--- End diff --

Two fixes: Authentication failed **unexpectedly.** [Details: %s, Error %s 
**]**


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15998546#comment-15998546
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r114897536
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserConnectionConfig.java
 ---
@@ -35,24 +38,42 @@
   private final UserServerRequestHandler handler;
 
   UserConnectionConfig(BufferAllocator allocator, BootStrapContext 
context, UserServerRequestHandler handler)
-  throws DrillbitStartupException {
+throws DrillbitStartupException {
 super(allocator, context);
 this.handler = handler;
 
-if 
(context.getConfig().getBoolean(ExecConstants.USER_AUTHENTICATION_ENABLED)) {
-  if (getAuthProvider().getAllFactoryNames().isEmpty()) {
+final DrillConfig config = context.getConfig();
+final AuthenticatorProvider authProvider = getAuthProvider();
+
+if (config.getBoolean(ExecConstants.USER_AUTHENTICATION_ENABLED)) {
+  if (authProvider.getAllFactoryNames().isEmpty()) {
 throw new DrillbitStartupException("Authentication enabled, but no 
mechanisms found. Please check " +
 "authentication configuration.");
   }
   authEnabled = true;
-  logger.info("Configured all user connections to require 
authentication using: {}",
-  getAuthProvider().getAllFactoryNames());
+
+  // Update encryption related parameters.
+  
encryptionContext.setEncryption(config.getBoolean(ExecConstants.USER_ENCRYPTION_SASL_ENABLED));
+  final int maxWrappedSize = 
config.getInt(ExecConstants.USER_ENCRYPTION_SASL_MAX_WRAPPED_SIZE);
+
+  if (maxWrappedSize <= 0 || maxWrappedSize > 
RpcConstants.MAX_WRAPPED_SIZE) {
+throw new DrillbitStartupException("Invalid value configured for 
user.encryption.sasl.encodesize." +
--- End diff --

encodesize -> max_wrapped_size


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15998549#comment-15998549
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r115038181
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/security/AuthenticationOutcomeListener.java
 ---
@@ -217,8 +224,9 @@ public SaslMessage process(SaslChallengeContext 
context) throws Exception {
   private static class SaslFailedProcessor implements 
SaslChallengeProcessor {
 
 @Override
-public SaslMessage process(SaslChallengeContext context) throws 
Exception {
-  throw new SaslException("Authentication failed. Incorrect 
credentials?");
+public  SaslMessage 
process(SaslChallengeContext context) throws Exception {
+  throw new SaslException(String.format("Authentication with 
encryption context %s failed. Incorrect credentials?",
--- End diff --

Fix error message format to match the format in UserClient.


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15995726#comment-15995726
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r114360169
  
--- Diff: 
exec/rpc/src/main/java/org/apache/drill/exec/rpc/ChunkCreationHandler.java ---
@@ -0,0 +1,101 @@
+/*
+ * 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.drill.exec.rpc;
+
+
+import io.netty.buffer.ByteBuf;
+import io.netty.buffer.CompositeByteBuf;
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.handler.codec.MessageToMessageEncoder;
+
+import java.util.List;
+
+import static com.google.common.base.Preconditions.checkArgument;
+import static java.lang.Math.min;
+
+/**
+ * Handler that converts an input ByteBuf into chunk size ByteBuf's and 
add it to the
+ * CompositeByteBuf as individual components. If encryption is enabled, 
this is always
+ * added in the channel pipeline.
+ */
+class ChunkCreationHandler extends MessageToMessageEncoder {
+  private static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(
+  ChunkCreationHandler.class.getCanonicalName());
+
+  private final int chunkSize;
+
+  ChunkCreationHandler(int chunkSize) {
+checkArgument(chunkSize > 0);
+this.chunkSize = chunkSize;
+  }
+
+  @Override
+  public void handlerAdded(ChannelHandlerContext ctx) throws Exception {
+super.handlerAdded(ctx);
+logger.trace("Added " + RpcConstants.CHUNK_CREATION_HANDLER + " 
handler!");
+  }
+
+  @Override
+  public void handlerRemoved(ChannelHandlerContext ctx) throws Exception {
+super.handlerRemoved(ctx);
+logger.trace("Removed " + RpcConstants.CHUNK_CREATION_HANDLER + " 
handler");
+  }
+
+  @Override
+  protected void encode(ChannelHandlerContext ctx, ByteBuf msg, 
List out) throws Exception {
+
+if (RpcConstants.EXTRA_DEBUGGING) {
+  logger.debug("ChunkCreationHandler called with msg {} of size {} 
with chunkSize {}",
+  msg, msg.readableBytes(), chunkSize);
+}
+
+if (!ctx.channel().isOpen()) {
+  logger.debug("Channel closed, skipping encode inside {}.", 
RpcConstants.CHUNK_CREATION_HANDLER);
+  msg.release();
+  return;
+}
+
+ByteBuf chunkBuf;
+
+// Calculate the number of chunks based on configured chunk size and 
input msg size
+int numChunks = (int) Math.ceil((double) msg.readableBytes() / 
chunkSize);
+
+// Initialize a composite buffer to hold numChunks chunk.
+final CompositeByteBuf cbb = ctx.alloc().compositeBuffer(numChunks);
--- End diff --

Call to `compositeBuffer(..)` allocates direct or heap buffer based on 
internal implementation of the allocator. In case of Drill by default all 
allocation happens on direct memory by allocator. In future if the internal 
allocation moves from supporting direct to heap then the change won't be needed 
explicitly here. It will be taken care of.


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15995725#comment-15995725
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r114448042
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java ---
@@ -116,6 +116,11 @@
   String BIT_AUTHENTICATION_ENABLED = 
"drill.exec.security.bit.auth.enabled";
   String BIT_AUTHENTICATION_MECHANISM = 
"drill.exec.security.bit.auth.mechanism";
   String USE_LOGIN_PRINCIPAL = 
"drill.exec.security.bit.auth.use_login_principal";
+  String USER_ENCRYPTION_SASL_ENABLED = 
"drill.exec.security.user.encryption.sasl.enabled";
+  String USER_ENCRYPTION_SASL_MAX_WRAPPED_SIZE = 
"drill.exec.security.user.encryption.sasl.max_wrapped_size";
--- End diff --

Sure. Will make sure to have it properly documented as part encryption 
information on Apache portal. I am also planning to update the design doc with 
the new name of the config parameter.


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15995724#comment-15995724
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r114223966
  
--- Diff: 
exec/rpc/src/main/java/org/apache/drill/exec/rpc/SaslDecryptionHandler.java ---
@@ -0,0 +1,162 @@
+/*
+ * 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.drill.exec.rpc;
+
+import io.netty.buffer.ByteBuf;
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.handler.codec.MessageToMessageDecoder;
+
+import org.apache.drill.exec.exception.OutOfMemoryException;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
+import java.util.List;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+/**
+ * Handler to Decrypt the input ByteBuf. It expects input to be in format 
where it has length of the bytes to
+ * decode in network order and actual encrypted bytes. The handler reads 
the length and then reads the
+ * required bytes to pass it to unwrap function for decryption. The 
decrypted buffer is copied to a new
+ * ByteBuf and added to out list.
+ * 
+ * Example:
+ * Input - [EBLN1, EB1, EBLN2, EB2] --> ByteBuf with repeated 
combination of encrypted byte length
+ * in network order (EBLNx) and encrypted bytes (EB)
+ * Output - [DB1] --> Decrypted ByteBuf of first chunk.(EB1)
+ * 
+ */
+class SaslDecryptionHandler extends MessageToMessageDecoder {
+
+  private static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(
+  SaslDecryptionHandler.class.getCanonicalName());
+
+  private final SaslCodec saslCodec;
+
+  private final int maxEncodedSize;
+
+  private final OutOfMemoryHandler outOfMemoryHandler;
+
+  private final byte[] encodedMsg;
+
+  private final ByteBuffer lengthOctets;
+
+  SaslDecryptionHandler(SaslCodec saslCodec, int maxEncodedSize, 
OutOfMemoryHandler oomHandler) {
+this.saslCodec = saslCodec;
+this.outOfMemoryHandler = oomHandler;
+this.maxEncodedSize = maxEncodedSize;
+
+// Allocate the byte array of maxEncodedSize to reuse for each encoded 
packet received on this connection
+// Maximum value of maxEncodedSize can be 16MB (i.e. OXFF)
+encodedMsg = new byte[maxEncodedSize];
+lengthOctets = ByteBuffer.allocate(RpcConstants.LENGTH_FIELD_LENGTH);
+lengthOctets.order(ByteOrder.BIG_ENDIAN);
+  }
+
+  @Override
+  public void handlerAdded(ChannelHandlerContext ctx) throws Exception {
+super.handlerAdded(ctx);
+logger.trace("Added " + RpcConstants.SASL_DECRYPTION_HANDLER + " 
handler");
+  }
+
+  @Override
+  public void handlerRemoved(ChannelHandlerContext ctx) throws Exception {
+super.handlerRemoved(ctx);
+logger.trace("Removed " + RpcConstants.SASL_DECRYPTION_HANDLER + " 
handler");
+  }
+
+  public void decode(ChannelHandlerContext ctx, ByteBuf msg, List 
out) throws IOException {
+
+if (!ctx.channel().isOpen()) {
+  logger.trace("Channel closed before decoding the message of {} 
bytes", msg.readableBytes());
+  msg.skipBytes(msg.readableBytes());
+  return;
+}
+
+try {
+  if(logger.isTraceEnabled()) {
+logger.trace("Trying to decrypt the encrypted message of size: {} 
with maxEncodedSize", msg.readableBytes());
+  }
+
+  final byte[] wrappedMsg;
+
+  // All the encrypted blocks are prefixed with it's length in network 
byte order (or BigEndian format). Netty's
+  // default Byte order of ByteBuf is Little Endian, so we cannot just 
do msg.getInt() as that will read the 4
+  // octets in little endian format.
+  //
+  // 

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15995721#comment-15995721
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r114448532
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserClient.java ---
@@ -201,10 +223,12 @@ protected SaslException mapException(Exception e) {
 if (e instanceof ExecutionException) {
   final Throwable cause = Throwables.getRootCause(e);
   if (cause instanceof SaslException) {
-return new SaslException("Authentication failed: " + 
cause.getMessage(), cause);
+return new SaslException(String.format("Authentication 
failed with encryption context %s with error %s",
--- End diff --

As discussed made encryption context information part of Details section.


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15995722#comment-15995722
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r114453991
  
--- Diff: 
exec/rpc/src/main/java/org/apache/drill/exec/rpc/SaslEncryptionHandler.java ---
@@ -0,0 +1,181 @@
+/*
+ * 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.drill.exec.rpc;
+
+import io.netty.buffer.ByteBuf;
+import io.netty.buffer.CompositeByteBuf;
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.handler.codec.MessageToMessageEncoder;
+
+import org.apache.drill.exec.exception.OutOfMemoryException;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
+import java.util.List;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+
+/**
+ * Handler to wrap the input Composite ByteBuf components separately and 
append the encrypted length for each
+ * component in the output ByteBuf. If there are multiple components in 
the input ByteBuf then each component will be
+ * encrypted individually and added to output ByteBuf with it's length 
prepended.
+ * 
+ * Example:
+ * Input ByteBuf  --> [B1,B2] - 2 component ByteBuf of 16K byte each.
+ * Output ByteBuf --> [[EBLN1, EB1], [EBLN2, EB2]] - List of ByteBuf's 
with each ByteBuf containing
+ *Encrypted Byte Length (EBLNx) in network order as 
per SASL RFC and Encrypted Bytes (EBx).
+ * 
+ */
+class SaslEncryptionHandler extends MessageToMessageEncoder {
+
+  private static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(
+  SaslEncryptionHandler.class.getCanonicalName());
+
+  private final SaslCodec saslCodec;
+
+  private final int maxRawWrapSize;
+
+  private byte[] origMsgBuffer;
+
+  private final ByteBuffer lengthOctets;
+
+  private final OutOfMemoryHandler outOfMemoryHandler;
+
+  /**
+   * We don't provide preference to allocator to use heap buffer instead 
of direct buffer.
+   * Drill uses it's own buffer allocator which doesn't support heap 
buffer allocation. We use
+   * Drill buffer allocator in the channel.
+   */
+  SaslEncryptionHandler(SaslCodec saslCodec, final int maxRawWrapSize, 
final OutOfMemoryHandler oomHandler) {
+this.saslCodec = saslCodec;
+this.maxRawWrapSize = maxRawWrapSize;
+this.outOfMemoryHandler = oomHandler;
+
+// The maximum size of the component will be maxRawWrapSize. Since 
this is maximum size we can allocate once
+// and reuse it for each component encode.
+origMsgBuffer = new byte[this.maxRawWrapSize];
+lengthOctets = ByteBuffer.allocate(RpcConstants.LENGTH_FIELD_LENGTH);
+lengthOctets.order(ByteOrder.BIG_ENDIAN);
+  }
+
+  @Override
+  public void handlerAdded(ChannelHandlerContext ctx) throws Exception {
+super.handlerAdded(ctx);
+logger.trace("Added " + RpcConstants.SASL_ENCRYPTION_HANDLER + " 
handler!");
+  }
+
+  @Override
+  public void handlerRemoved(ChannelHandlerContext ctx) throws Exception {
+super.handlerRemoved(ctx);
+logger.trace("Removed " + RpcConstants.SASL_ENCRYPTION_HANDLER + " 
handler");
+  }
+
+  public void encode(ChannelHandlerContext ctx, ByteBuf msg, List 
out) throws IOException {
+
+if (!ctx.channel().isOpen()) {
+  logger.debug("In " + RpcConstants.SASL_ENCRYPTION_HANDLER + " and 
channel is not open. " +
+  "So releasing msg memory before encryption.");
+  msg.release();
+  return;
+}
+
+try {
+  // If encryption is enabled then this handler will always get 
ByteBuf of type Composite ByteBuf
+  checkArgument(msg instanceof CompositeByteBuf);
+

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15993047#comment-15993047
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r114339835
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserClient.java ---
@@ -201,10 +223,12 @@ protected SaslException mapException(Exception e) {
 if (e instanceof ExecutionException) {
   final Throwable cause = Throwables.getRootCause(e);
   if (cause instanceof SaslException) {
-return new SaslException("Authentication failed: " + 
cause.getMessage(), cause);
+return new SaslException(String.format("Authentication 
failed with encryption context %s with error %s",
--- End diff --

For an end user, this is a dense error message:

> Authentication failed with encryption context Encryption: .. , 
MaxWrappedChunkSize: .. , MaxRawWrapSendSize ..  with error ..

Could we improve the error message, here and below?


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15993046#comment-15993046
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r114334527
  
--- Diff: exec/rpc/src/main/java/org/apache/drill/exec/rpc/RpcEncoder.java 
---
@@ -111,7 +111,7 @@ protected void encode(ChannelHandlerContext ctx, 
OutboundRpcMessage msg, List Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15993045#comment-15993045
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r114334608
  
--- Diff: 
exec/rpc/src/main/java/org/apache/drill/exec/rpc/ChunkCreationHandler.java ---
@@ -0,0 +1,101 @@
+/*
+ * 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.drill.exec.rpc;
+
+
+import io.netty.buffer.ByteBuf;
+import io.netty.buffer.CompositeByteBuf;
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.handler.codec.MessageToMessageEncoder;
+
+import java.util.List;
+
+import static com.google.common.base.Preconditions.checkArgument;
+import static java.lang.Math.min;
+
+/**
+ * Handler that converts an input ByteBuf into chunk size ByteBuf's and 
add it to the
+ * CompositeByteBuf as individual components. If encryption is enabled, 
this is always
+ * added in the channel pipeline.
+ */
+class ChunkCreationHandler extends MessageToMessageEncoder {
+  private static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(
+  ChunkCreationHandler.class.getCanonicalName());
+
+  private final int chunkSize;
+
+  ChunkCreationHandler(int chunkSize) {
+checkArgument(chunkSize > 0);
+this.chunkSize = chunkSize;
+  }
+
+  @Override
+  public void handlerAdded(ChannelHandlerContext ctx) throws Exception {
+super.handlerAdded(ctx);
+logger.trace("Added " + RpcConstants.CHUNK_CREATION_HANDLER + " 
handler!");
+  }
+
+  @Override
+  public void handlerRemoved(ChannelHandlerContext ctx) throws Exception {
+super.handlerRemoved(ctx);
+logger.trace("Removed " + RpcConstants.CHUNK_CREATION_HANDLER + " 
handler");
+  }
+
+  @Override
+  protected void encode(ChannelHandlerContext ctx, ByteBuf msg, 
List out) throws Exception {
+
+if (RpcConstants.EXTRA_DEBUGGING) {
+  logger.debug("ChunkCreationHandler called with msg {} of size {} 
with chunkSize {}",
+  msg, msg.readableBytes(), chunkSize);
+}
+
+if (!ctx.channel().isOpen()) {
+  logger.debug("Channel closed, skipping encode inside {}.", 
RpcConstants.CHUNK_CREATION_HANDLER);
+  msg.release();
+  return;
+}
+
+ByteBuf chunkBuf;
+
+// Calculate the number of chunks based on configured chunk size and 
input msg size
+int numChunks = (int) Math.ceil((double) msg.readableBytes() / 
chunkSize);
+
+// Initialize a composite buffer to hold numChunks chunk.
+final CompositeByteBuf cbb = ctx.alloc().compositeBuffer(numChunks);
--- End diff --

`ctx.alloc().compositeDirectBuffer(...)`


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15993044#comment-15993044
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r114334714
  
--- Diff: 
exec/rpc/src/main/java/org/apache/drill/exec/rpc/SaslEncryptionHandler.java ---
@@ -0,0 +1,184 @@
+/*
+ * 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.drill.exec.rpc;
+
+import io.netty.buffer.ByteBuf;
+import io.netty.buffer.CompositeByteBuf;
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.handler.codec.MessageToMessageEncoder;
+
+import org.apache.drill.exec.exception.OutOfMemoryException;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
+import java.util.List;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+
+/**
+ * Handler to wrap the input Composite ByteBuf components separately and 
append the encrypted length for each
+ * component in the output ByteBuf. If there are multiple components in 
the input ByteBuf then each component will be
+ * encrypted individually and added to output ByteBuf with it's length 
prepended.
+ * 
+ * Example:
+ * Input ByteBuf  --> [B1,B2] - 2 component ByteBuf of 16K byte each.
+ * Output ByteBuf --> [[EBLN1, EB1], [EBLN2, EB2]] - List of ByteBuf's 
with each ByteBuf containing
+ *Encrypted Byte Length (EBLNx) in network order as 
per SASL RFC and Encrypted Bytes (EBx).
+ * 
+ */
+class SaslEncryptionHandler extends MessageToMessageEncoder {
+
+  private static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(
+  SaslEncryptionHandler.class.getCanonicalName());
+
+  private final SaslCodec saslCodec;
+
+  private final int wrapSizeLimit;
+
+  private byte[] origMsgBuffer;
+
+  private final ByteBuffer lengthOctets;
+
+  private final OutOfMemoryHandler outOfMemoryHandler;
+
+  /**
+   * We don't provide preference to allocator to use heap buffer instead 
of direct buffer.
+   * Drill uses it's own buffer allocator which doesn't support heap 
buffer allocation. We use
+   * Drill buffer allocator in the channel.
+   */
+  SaslEncryptionHandler(SaslCodec saslCodec, final int wrapSizeLimit, 
final OutOfMemoryHandler oomHandler) {
+this.saslCodec = saslCodec;
+this.wrapSizeLimit = wrapSizeLimit;
+this.outOfMemoryHandler = oomHandler;
+
+// The maximum size of the component will be wrapSizeLimit. Since this 
is maximum size, we can allocate once
+// and reuse it for each component encode.
+origMsgBuffer = new byte[this.wrapSizeLimit];
+lengthOctets = ByteBuffer.allocate(RpcConstants.LENGTH_FIELD_LENGTH);
+lengthOctets.order(ByteOrder.BIG_ENDIAN);
+  }
+
+  @Override
+  public void handlerAdded(ChannelHandlerContext ctx) throws Exception {
+super.handlerAdded(ctx);
+logger.trace("Added " + RpcConstants.SASL_ENCRYPTION_HANDLER + " 
handler!");
+  }
+
+  @Override
+  public void handlerRemoved(ChannelHandlerContext ctx) throws Exception {
+super.handlerRemoved(ctx);
+logger.trace("Removed " + RpcConstants.SASL_ENCRYPTION_HANDLER + " 
handler");
+  }
+
+  public void encode(ChannelHandlerContext ctx, ByteBuf msg, List 
out) throws IOException {
+
+if (!ctx.channel().isOpen()) {
+  logger.debug("In " + RpcConstants.SASL_ENCRYPTION_HANDLER + " and 
channel is not open. " +
+  "So releasing msg memory before encryption.");
+  msg.release();
+  return;
+}
+
+try {
+  // If encryption is enabled then this handler will always get 
ByteBuf of type Composite ByteBuf
+  checkArgument(msg instanceof CompositeByteBuf);
   

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15991737#comment-15991737
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r114215418
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/ExecConstants.java ---
@@ -116,6 +116,11 @@
   String BIT_AUTHENTICATION_ENABLED = 
"drill.exec.security.bit.auth.enabled";
   String BIT_AUTHENTICATION_MECHANISM = 
"drill.exec.security.bit.auth.mechanism";
   String USE_LOGIN_PRINCIPAL = 
"drill.exec.security.bit.auth.use_login_principal";
+  String USER_ENCRYPTION_SASL_ENABLED = 
"drill.exec.security.user.encryption.sasl.enabled";
+  String USER_ENCRYPTION_SASL_MAX_WRAPPED_SIZE = 
"drill.exec.security.user.encryption.sasl.max_wrapped_size";
--- End diff --

We should document this config parameter due to the change in name (from 
"maximum size of the raw send buffer in bytes" to max_wrapped_size).

From [Sasl.RAW_SEND_SIZE 
doc](http://grepcode.com/file/repository.grepcode.com/java/root/jdk/openjdk/8u40-b25/javax/security/sasl/Sasl.java#151):
> The name of a property that specifies the maximum size of the raw send 
buffer in bytes of SaslClient/SaslServer. The property contains the string 
representation of an integer. The value of this property is negotiated between 
the client and server during the authentication exchange. 


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15991732#comment-15991732
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r114213700
  
--- Diff: 
exec/rpc/src/main/java/org/apache/drill/exec/rpc/ChunkCreationHandler.java ---
@@ -0,0 +1,101 @@
+/*
+ * 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.drill.exec.rpc;
+
+
+import io.netty.buffer.ByteBuf;
+import io.netty.buffer.CompositeByteBuf;
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.handler.codec.MessageToMessageEncoder;
+
+import java.util.List;
+
+import static com.google.common.base.Preconditions.checkArgument;
+import static java.lang.Math.min;
+
+/**
+ * Handler that converts an input ByteBuf into chunk size ByteBuf's and 
add it to the
+ * CompositeByteBuf as individual components. If encryption is enabled, 
this is always
+ * added in the channel pipeline.
+ */
+class ChunkCreationHandler extends MessageToMessageEncoder {
+  private static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(
+  ChunkCreationHandler.class.getCanonicalName());
+
+  private final int chunkSize;
+
+  ChunkCreationHandler(int chunkSize) {
+checkArgument(chunkSize > 0);
+this.chunkSize = chunkSize;
+  }
+
+  @Override
+  public void handlerAdded(ChannelHandlerContext ctx) throws Exception {
+super.handlerAdded(ctx);
+logger.trace("Added " + RpcConstants.CHUNK_CREATION_HANDLER + " 
handler!");
+  }
+
+  @Override
+  public void handlerRemoved(ChannelHandlerContext ctx) throws Exception {
+super.handlerRemoved(ctx);
+logger.trace("Removed " + RpcConstants.CHUNK_CREATION_HANDLER + " 
handler");
+  }
+
+  @Override
+  protected void encode(ChannelHandlerContext ctx, ByteBuf msg, 
List out) throws Exception {
+
+if (RpcConstants.EXTRA_DEBUGGING) {
+  logger.debug("ChunkCreationHandler called with msg {} of size {} 
with chunkSize {}",
+  msg, msg.readableBytes(), chunkSize);
+}
+
+if (!ctx.channel().isOpen()) {
+  logger.debug("Channel closed, skipping encode inside {}.", 
RpcConstants.CHUNK_CREATION_HANDLER);
+  msg.release();
+  return;
+}
+
+ByteBuf chunkBuf;
+
+// Calculate the number of chunks based on configured chunk size and 
input msg size
+int numChunks = (int) Math.ceil((double) msg.readableBytes() / 
chunkSize);
+
+// Initialize a composite buffer to hold numChunks chunk.
+final CompositeByteBuf cbb = ctx.alloc().compositeBuffer(numChunks);
+
+int cbbWriteIndex = 0;
+int currentChunkLen = min(msg.readableBytes(), chunkSize);
+
+// Create slices of chunkSize from input msg and add it to the 
composite buffer.
+while (numChunks > 0) {
+  chunkBuf = msg.slice(msg.readerIndex(), currentChunkLen);
--- End diff --

`final ByteBuf chunkBuf ...`


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15991735#comment-15991735
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r114216967
  
--- Diff: exec/rpc/src/main/java/org/apache/drill/exec/rpc/RpcMetrics.java 
---
@@ -0,0 +1,33 @@
+/*
+ * 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.drill.exec.rpc;
+
+import org.apache.drill.exec.memory.BufferAllocator;
+
+/**
+ * Holder interface for all the metrics used in RPC layer
+ */
+public interface RpcMetrics {
--- End diff --

-0 for this plumbing.

The end result is that only static state (counters) is being modified using 
instances, which is not recommended 
([ref](http://stackoverflow.com/questions/6832767/how-to-modify-private-static-variable-through-setter-method)).
 So the hierarchy is not required.


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15991738#comment-15991738
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r114214085
  
--- Diff: 
exec/rpc/src/main/java/org/apache/drill/exec/rpc/SaslDecryptionHandler.java ---
@@ -0,0 +1,163 @@
+/*
+ * 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.drill.exec.rpc;
+
+import io.netty.buffer.ByteBuf;
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.handler.codec.MessageToMessageDecoder;
+
+import org.apache.drill.exec.exception.OutOfMemoryException;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
+import java.util.List;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+/**
+ * Handler to Decrypt the input ByteBuf. It expects input to be in format 
where it has length of the bytes to
+ * decode in network order and actual encrypted bytes. The handler reads 
the length and then reads the
+ * required bytes to pass it to unwrap function for decryption. The 
decrypted buffer is copied to a new
+ * ByteBuf and added to out list.
+ * 
+ * Example:
+ * Input - [EBLN1, EB1, EBLN2, EB2] --> ByteBuf with repeated 
combination of encrypted byte length
+ * in network order (EBLNx) and encrypted bytes (EB)
+ * Output - [DB1] --> Decrypted ByteBuf of first chunk.(EB1)
+ * 
+ */
+class SaslDecryptionHandler extends MessageToMessageDecoder {
+
+  private static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(
+  SaslDecryptionHandler.class.getCanonicalName());
+
+  private final SaslCodec saslCodec;
+
+  private final int maxEncodedSize;
+
+  private final OutOfMemoryHandler outOfMemoryHandler;
+
+  private final byte[] encodedMsg;
+
+  private final ByteBuffer lengthOctets;
+
+  SaslDecryptionHandler(SaslCodec saslCodec, int maxEncodedSize, 
OutOfMemoryHandler oomHandler) {
+this.saslCodec = saslCodec;
+this.outOfMemoryHandler = oomHandler;
+this.maxEncodedSize = maxEncodedSize;
+
+// Allocate the byte array of maxEncodedSize to reuse for each encoded 
packet received on this connection
+// Maximum value of maxEncodedSize can be 16MB (i.e. OXFF)
+encodedMsg = new byte[maxEncodedSize];
+lengthOctets = ByteBuffer.allocate(RpcConstants.LENGTH_FIELD_LENGTH);
+lengthOctets.order(ByteOrder.BIG_ENDIAN);
+  }
+
+  @Override
+  public void handlerAdded(ChannelHandlerContext ctx) throws Exception {
+super.handlerAdded(ctx);
+logger.trace("Added " + RpcConstants.SASL_DECRYPTION_HANDLER + " 
handler");
+  }
+
+  @Override
+  public void handlerRemoved(ChannelHandlerContext ctx) throws Exception {
+super.handlerRemoved(ctx);
+logger.trace("Removed " + RpcConstants.SASL_DECRYPTION_HANDLER + " 
handler");
+  }
+
+  public void decode(ChannelHandlerContext ctx, ByteBuf msg, List 
out) throws IOException {
+
+if (!ctx.channel().isOpen()) {
+  logger.trace("Channel closed before decoding the message of {} 
bytes", msg.readableBytes());
+  msg.skipBytes(msg.readableBytes());
+  return;
+}
+
+try {
+  if(logger.isTraceEnabled()) {
+logger.trace("Trying to decrypt the encrypted message of size: {} 
with maxEncodedSize", msg.readableBytes());
+  }
+
+
+  // All the encrypted blocks are prefixed with it's length in network 
byte order (or BigEndian format). Netty's
+  // default Byte order of ByteBuf is Little Endian, so we cannot just 
do msg.getInt() as that will read the 4
+  // octets in little endian format.
+  //
+  // We will read the length of 

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15991734#comment-15991734
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r114212187
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/security/AuthenticationOutcomeListener.java
 ---
@@ -165,22 +172,23 @@ public void interrupted(InterruptedException e) {
  *
  * @param context challenge context
  * @return response
- * @throws Exception
+ * @throws Exception in case of any failure
  */
-SaslMessage process(SaslChallengeContext context) throws Exception;
+SaslMessage process(SaslChallengeContext context) throws Exception;
--- End diff --

Why not this signature?

` process(SaslChallengeContext context) 
throws Exception;`


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15991733#comment-15991733
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r114219484
  
--- Diff: protocol/src/main/protobuf/User.proto ---
@@ -124,6 +125,8 @@ message BitToUserHandshake {
   optional RpcEndpointInfos server_infos = 6;
   repeated string authenticationMechanisms = 7;
   repeated RpcType supported_methods = 8;
+  optional bool encrypted = 9;
+  optional int32 wrapChunkSize = 10;
--- End diff --

Change name to `maxWrappedSize`


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15991739#comment-15991739
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r114214449
  
--- Diff: 
exec/rpc/src/main/java/org/apache/drill/exec/rpc/SaslEncryptionHandler.java ---
@@ -0,0 +1,181 @@
+/*
+ * 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.drill.exec.rpc;
+
+import io.netty.buffer.ByteBuf;
+import io.netty.buffer.CompositeByteBuf;
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.handler.codec.MessageToMessageEncoder;
+
+import org.apache.drill.exec.exception.OutOfMemoryException;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
+import java.util.List;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+
+/**
+ * Handler to wrap the input Composite ByteBuf components separately and 
append the encrypted length for each
+ * component in the output ByteBuf. If there are multiple components in 
the input ByteBuf then each component will be
+ * encrypted individually and added to output ByteBuf with it's length 
prepended.
+ * 
+ * Example:
+ * Input ByteBuf  --> [B1,B2] - 2 component ByteBuf of 16K byte each.
+ * Output ByteBuf --> [[EBLN1, EB1], [EBLN2, EB2]] - List of ByteBuf's 
with each ByteBuf containing
+ *Encrypted Byte Length (EBLNx) in network order as 
per SASL RFC and Encrypted Bytes (EBx).
+ * 
+ */
+class SaslEncryptionHandler extends MessageToMessageEncoder {
+
+  private static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(
+  SaslEncryptionHandler.class.getCanonicalName());
+
+  private final SaslCodec saslCodec;
+
+  private final int maxRawWrapSize;
+
+  private byte[] origMsgBuffer;
+
+  private final ByteBuffer lengthOctets;
+
+  private final OutOfMemoryHandler outOfMemoryHandler;
+
+  /**
+   * We don't provide preference to allocator to use heap buffer instead 
of direct buffer.
+   * Drill uses it's own buffer allocator which doesn't support heap 
buffer allocation. We use
+   * Drill buffer allocator in the channel.
+   */
+  SaslEncryptionHandler(SaslCodec saslCodec, final int maxRawWrapSize, 
final OutOfMemoryHandler oomHandler) {
+this.saslCodec = saslCodec;
+this.maxRawWrapSize = maxRawWrapSize;
+this.outOfMemoryHandler = oomHandler;
+
+// The maximum size of the component will be maxRawWrapSize. Since 
this is maximum size we can allocate once
+// and reuse it for each component encode.
+origMsgBuffer = new byte[this.maxRawWrapSize];
+lengthOctets = ByteBuffer.allocate(RpcConstants.LENGTH_FIELD_LENGTH);
+lengthOctets.order(ByteOrder.BIG_ENDIAN);
+  }
+
+  @Override
+  public void handlerAdded(ChannelHandlerContext ctx) throws Exception {
+super.handlerAdded(ctx);
+logger.trace("Added " + RpcConstants.SASL_ENCRYPTION_HANDLER + " 
handler!");
+  }
+
+  @Override
+  public void handlerRemoved(ChannelHandlerContext ctx) throws Exception {
+super.handlerRemoved(ctx);
+logger.trace("Removed " + RpcConstants.SASL_ENCRYPTION_HANDLER + " 
handler");
+  }
+
+  public void encode(ChannelHandlerContext ctx, ByteBuf msg, List 
out) throws IOException {
+
+if (!ctx.channel().isOpen()) {
+  logger.debug("In " + RpcConstants.SASL_ENCRYPTION_HANDLER + " and 
channel is not open. " +
+  "So releasing msg memory before encryption.");
+  msg.release();
+  return;
+}
+
+try {
+  // If encryption is enabled then this handler will always get 
ByteBuf of type Composite ByteBuf
+  checkArgument(msg instanceof 

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15991389#comment-15991389
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r113375341
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/control/ControllerImpl.java
 ---
@@ -24,13 +24,15 @@
 import org.apache.drill.exec.exception.DrillbitStartupException;
 import org.apache.drill.exec.memory.BufferAllocator;
 import org.apache.drill.exec.proto.CoordinationProtos.DrillbitEndpoint;
+import org.apache.drill.exec.rpc.RpcMetrics;
 import org.apache.drill.exec.server.BootStrapContext;
 import org.apache.drill.exec.work.batch.ControlMessageHandler;
 
 import com.google.common.collect.Lists;
 import com.google.protobuf.Message;
 import com.google.protobuf.MessageLite;
 import com.google.protobuf.Parser;
+import org.apache.hadoop.io.ReadaheadPool;
--- End diff --

Removed.


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15991394#comment-15991394
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r113517138
  
--- Diff: 
exec/rpc/src/main/java/org/apache/drill/exec/rpc/ChunkCreationHandler.java ---
@@ -0,0 +1,101 @@
+/*
+ * 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.drill.exec.rpc;
+
+
+import io.netty.buffer.ByteBuf;
+import io.netty.buffer.CompositeByteBuf;
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.handler.codec.MessageToMessageEncoder;
+
+import java.util.List;
+
+import static com.google.common.base.Preconditions.checkArgument;
+import static java.lang.Math.min;
+
+/**
+ * Handler that converts an input ByteBuf into chunk size ByteBuf's and 
add it to the
+ * CompositeByteBuf as individual components. This is done irrespective of 
chunk mode is
+ * enabled or not.
+ */
+class ChunkCreationHandler extends MessageToMessageEncoder {
+  private static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(
+  ChunkCreationHandler.class.getCanonicalName());
+
+  private final int chunkSize;
+
+  ChunkCreationHandler(int chunkSize) {
+checkArgument(chunkSize > 0);
+this.chunkSize = chunkSize;
+  }
+
+  @Override
+  public void handlerAdded(ChannelHandlerContext ctx) throws Exception {
+super.handlerAdded(ctx);
+logger.trace("Added " + RpcConstants.CHUNK_CREATION_HANDLER + " 
handler!");
+  }
+
+  @Override
+  public void handlerRemoved(ChannelHandlerContext ctx) throws Exception {
+super.handlerRemoved(ctx);
+logger.trace("Removed " + RpcConstants.CHUNK_CREATION_HANDLER + " 
handler");
+  }
+
+  @Override
+  protected void encode(ChannelHandlerContext ctx, ByteBuf msg, 
List out) throws Exception {
+
+if (RpcConstants.EXTRA_DEBUGGING) {
+  logger.debug("ChunkCreationHandler called with msg {} of size {} 
with chunkSize {}",
+  msg, msg.readableBytes(), chunkSize);
+}
+
+if (!ctx.channel().isOpen()) {
+  logger.debug("Channel closed, skipping encode inside {}.", 
RpcConstants.CHUNK_CREATION_HANDLER);
+  msg.release();
+  return;
+}
+
+ByteBuf chunkBuf;
+
+// Calculate the number of chunks based on configured chunk size and 
input msg size
+int numChunks = (int) Math.ceil((double) msg.readableBytes() / 
chunkSize);
+
+// Initialize a composite buffer to hold numChunks chunk.
+CompositeByteBuf cbb = new CompositeByteBuf(ctx.alloc(), true, 
numChunks);
--- End diff --

used this based on reference in RpcEncoder. Will change in both the places 
since this is the recommended way.


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15991405#comment-15991405
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r113838045
  
--- Diff: 
exec/rpc/src/main/java/org/apache/drill/exec/rpc/SaslEncryptionHandler.java ---
@@ -0,0 +1,181 @@
+/*
+ * 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.drill.exec.rpc;
+
+import io.netty.buffer.ByteBuf;
+import io.netty.buffer.CompositeByteBuf;
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.handler.codec.MessageToMessageEncoder;
+
+import org.apache.drill.exec.exception.OutOfMemoryException;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
+import java.util.List;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+
+/**
+ * Handler to wrap the input Composite ByteBuf components separately and 
append the encrypted length for each
+ * component in the output ByteBuf. If there are multiple components in 
the input ByteBuf then each component will be
+ * encrypted individually and added to output ByteBuf with it's length 
prepended.
+ * 
+ * Example:
+ * Input ByteBuf  --> [B1,B2] - 2 component ByteBuf of 16K byte each.
+ * Output ByteBuf --> [[EBLN1, EB1], [EBLN2, EB2]] - List of ByteBuf's 
with each ByteBuf containing
+ *Encrypted Byte Length (EBLNx) in network order as 
per SASL RFC and Encrypted Bytes (EBx).
+ * 
+ */
+class SaslEncryptionHandler extends MessageToMessageEncoder {
+
+  private static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(
+  SaslEncryptionHandler.class.getCanonicalName());
+
+  private final SaslCodec saslCodec;
+
+  private final int maxRawWrapSize;
+
+  private byte[] origMsgBuffer;
+
+  private final ByteBuffer lengthOctets;
+
+  private final OutOfMemoryHandler outOfMemoryHandler;
+
+  /**
+   * We don't provide preference to allocator to use heap buffer instead 
of direct buffer.
+   * Drill uses it's own buffer allocator which doesn't support heap 
buffer allocation. We use
+   * Drill buffer allocator in the channel.
+   */
+  SaslEncryptionHandler(SaslCodec saslCodec, final int maxRawWrapSize, 
final OutOfMemoryHandler oomHandler) {
+this.saslCodec = saslCodec;
+this.maxRawWrapSize = maxRawWrapSize;
+this.outOfMemoryHandler = oomHandler;
+
+// The maximum size of the component will be maxRawWrapSize. Since 
this is maximum size we can allocate once
+// and reuse it for each component encode.
+origMsgBuffer = new byte[this.maxRawWrapSize];
+lengthOctets = ByteBuffer.allocate(RpcConstants.LENGTH_FIELD_LENGTH);
+lengthOctets.order(ByteOrder.BIG_ENDIAN);
+  }
+
+  @Override
+  public void handlerAdded(ChannelHandlerContext ctx) throws Exception {
+super.handlerAdded(ctx);
+logger.trace("Added " + RpcConstants.SASL_ENCRYPTION_HANDLER + " 
handler!");
+  }
+
+  @Override
+  public void handlerRemoved(ChannelHandlerContext ctx) throws Exception {
+super.handlerRemoved(ctx);
+logger.trace("Removed " + RpcConstants.SASL_ENCRYPTION_HANDLER + " 
handler");
+  }
+
+  public void encode(ChannelHandlerContext ctx, ByteBuf msg, List 
out) throws IOException {
+
+if (!ctx.channel().isOpen()) {
+  logger.debug("In " + RpcConstants.SASL_ENCRYPTION_HANDLER + " and 
channel is not open. " +
+  "So releasing msg memory before encryption.");
+  msg.release();
+  return;
+}
+
+try {
+  // If encryption is enabled then this handler will always get 
ByteBuf of type Composite ByteBuf
+  checkArgument(msg instanceof CompositeByteBuf);
+

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15991410#comment-15991410
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r113836743
  
--- Diff: 
exec/rpc/src/main/java/org/apache/drill/exec/rpc/SaslDecryptionHandler.java ---
@@ -0,0 +1,162 @@
+/*
+ * 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.drill.exec.rpc;
+
+import io.netty.buffer.ByteBuf;
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.handler.codec.MessageToMessageDecoder;
+
+import org.apache.drill.exec.exception.OutOfMemoryException;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
+import java.util.List;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+/**
+ * Handler to Decrypt the input ByteBuf. It expects input to be in format 
where it has length of the bytes to
+ * decode in network order and actual encrypted bytes. The handler reads 
the length and then reads the
+ * required bytes to pass it to unwrap function for decryption. The 
decrypted buffer is copied to a new
+ * ByteBuf and added to out list.
+ * 
+ * Example:
+ * Input - [EBLN1, EB1, EBLN2, EB2] --> ByteBuf with repeated 
combination of encrypted byte length
+ * in network order (EBLNx) and encrypted bytes (EB)
+ * Output - [DB1] --> Decrypted ByteBuf of first chunk.(EB1)
+ * 
+ */
+class SaslDecryptionHandler extends MessageToMessageDecoder {
+
+  private static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(
+  SaslDecryptionHandler.class.getCanonicalName());
+
+  private final SaslCodec saslCodec;
+
+  private final int maxEncodedSize;
+
+  private final OutOfMemoryHandler outOfMemoryHandler;
+
+  private final byte[] encodedMsg;
+
+  private final ByteBuffer lengthOctets;
+
+  SaslDecryptionHandler(SaslCodec saslCodec, int maxEncodedSize, 
OutOfMemoryHandler oomHandler) {
+this.saslCodec = saslCodec;
+this.outOfMemoryHandler = oomHandler;
+this.maxEncodedSize = maxEncodedSize;
+
+// Allocate the byte array of maxEncodedSize to reuse for each encoded 
packet received on this connection
+// Maximum value of maxEncodedSize can be 16MB (i.e. OXFF)
+encodedMsg = new byte[maxEncodedSize];
+lengthOctets = ByteBuffer.allocate(RpcConstants.LENGTH_FIELD_LENGTH);
+lengthOctets.order(ByteOrder.BIG_ENDIAN);
+  }
+
+  @Override
+  public void handlerAdded(ChannelHandlerContext ctx) throws Exception {
+super.handlerAdded(ctx);
+logger.trace("Added " + RpcConstants.SASL_DECRYPTION_HANDLER + " 
handler");
+  }
+
+  @Override
+  public void handlerRemoved(ChannelHandlerContext ctx) throws Exception {
+super.handlerRemoved(ctx);
+logger.trace("Removed " + RpcConstants.SASL_DECRYPTION_HANDLER + " 
handler");
+  }
+
+  public void decode(ChannelHandlerContext ctx, ByteBuf msg, List 
out) throws IOException {
+
+if (!ctx.channel().isOpen()) {
+  logger.trace("Channel closed before decoding the message of {} 
bytes", msg.readableBytes());
+  msg.skipBytes(msg.readableBytes());
+  return;
+}
+
+try {
+  if(logger.isTraceEnabled()) {
+logger.trace("Trying to decrypt the encrypted message of size: {} 
with maxEncodedSize", msg.readableBytes());
+  }
+
+  final byte[] wrappedMsg;
+
+  // All the encrypted blocks are prefixed with it's length in network 
byte order (or BigEndian format). Netty's
+  // default Byte order of ByteBuf is Little Endian, so we cannot just 
do msg.getInt() as that will read the 4
+  // octets in little endian format.
+  //
+  // 

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15991411#comment-15991411
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r113837851
  
--- Diff: 
exec/rpc/src/main/java/org/apache/drill/exec/rpc/SaslEncryptionHandler.java ---
@@ -0,0 +1,181 @@
+/*
+ * 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.drill.exec.rpc;
+
+import io.netty.buffer.ByteBuf;
+import io.netty.buffer.CompositeByteBuf;
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.handler.codec.MessageToMessageEncoder;
+
+import org.apache.drill.exec.exception.OutOfMemoryException;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
+import java.util.List;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+
+/**
+ * Handler to wrap the input Composite ByteBuf components separately and 
append the encrypted length for each
+ * component in the output ByteBuf. If there are multiple components in 
the input ByteBuf then each component will be
+ * encrypted individually and added to output ByteBuf with it's length 
prepended.
+ * 
+ * Example:
+ * Input ByteBuf  --> [B1,B2] - 2 component ByteBuf of 16K byte each.
+ * Output ByteBuf --> [[EBLN1, EB1], [EBLN2, EB2]] - List of ByteBuf's 
with each ByteBuf containing
+ *Encrypted Byte Length (EBLNx) in network order as 
per SASL RFC and Encrypted Bytes (EBx).
+ * 
+ */
+class SaslEncryptionHandler extends MessageToMessageEncoder {
+
+  private static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(
+  SaslEncryptionHandler.class.getCanonicalName());
+
+  private final SaslCodec saslCodec;
+
+  private final int maxRawWrapSize;
+
+  private byte[] origMsgBuffer;
+
+  private final ByteBuffer lengthOctets;
+
+  private final OutOfMemoryHandler outOfMemoryHandler;
+
+  /**
+   * We don't provide preference to allocator to use heap buffer instead 
of direct buffer.
+   * Drill uses it's own buffer allocator which doesn't support heap 
buffer allocation. We use
+   * Drill buffer allocator in the channel.
+   */
+  SaslEncryptionHandler(SaslCodec saslCodec, final int maxRawWrapSize, 
final OutOfMemoryHandler oomHandler) {
+this.saslCodec = saslCodec;
+this.maxRawWrapSize = maxRawWrapSize;
+this.outOfMemoryHandler = oomHandler;
+
+// The maximum size of the component will be maxRawWrapSize. Since 
this is maximum size we can allocate once
+// and reuse it for each component encode.
+origMsgBuffer = new byte[this.maxRawWrapSize];
--- End diff --

See above.


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15991398#comment-15991398
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r113808826
  
--- Diff: 
exec/rpc/src/main/java/org/apache/drill/exec/rpc/SaslEncryptionHandler.java ---
@@ -0,0 +1,181 @@
+/*
+ * 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.drill.exec.rpc;
+
+import io.netty.buffer.ByteBuf;
+import io.netty.buffer.CompositeByteBuf;
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.handler.codec.MessageToMessageEncoder;
+
+import org.apache.drill.exec.exception.OutOfMemoryException;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
+import java.util.List;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+
+/**
+ * Handler to wrap the input Composite ByteBuf components separately and 
append the encrypted length for each
+ * component in the output ByteBuf. If there are multiple components in 
the input ByteBuf then each component will be
+ * encrypted individually and added to output ByteBuf with it's length 
prepended.
+ * 
+ * Example:
+ * Input ByteBuf  --> [B1,B2] - 2 component ByteBuf of 16K byte each.
+ * Output ByteBuf --> [[EBLN1, EB1], [EBLN2, EB2]] - List of ByteBuf's 
with each ByteBuf containing
+ *Encrypted Byte Length (EBLNx) in network order as 
per SASL RFC and Encrypted Bytes (EBx).
+ * 
+ */
+class SaslEncryptionHandler extends MessageToMessageEncoder {
+
+  private static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(
+  SaslEncryptionHandler.class.getCanonicalName());
+
+  private final SaslCodec saslCodec;
+
+  private final int maxRawWrapSize;
+
+  private byte[] origMsgBuffer;
+
+  private final ByteBuffer lengthOctets;
+
+  private final OutOfMemoryHandler outOfMemoryHandler;
+
+  /**
+   * We don't provide preference to allocator to use heap buffer instead 
of direct buffer.
+   * Drill uses it's own buffer allocator which doesn't support heap 
buffer allocation. We use
+   * Drill buffer allocator in the channel.
+   */
+  SaslEncryptionHandler(SaslCodec saslCodec, final int maxRawWrapSize, 
final OutOfMemoryHandler oomHandler) {
+this.saslCodec = saslCodec;
+this.maxRawWrapSize = maxRawWrapSize;
+this.outOfMemoryHandler = oomHandler;
+
+// The maximum size of the component will be maxRawWrapSize. Since 
this is maximum size we can allocate once
+// and reuse it for each component encode.
+origMsgBuffer = new byte[this.maxRawWrapSize];
+lengthOctets = ByteBuffer.allocate(RpcConstants.LENGTH_FIELD_LENGTH);
+lengthOctets.order(ByteOrder.BIG_ENDIAN);
+  }
+
+  @Override
+  public void handlerAdded(ChannelHandlerContext ctx) throws Exception {
+super.handlerAdded(ctx);
+logger.trace("Added " + RpcConstants.SASL_ENCRYPTION_HANDLER + " 
handler!");
+  }
+
+  @Override
+  public void handlerRemoved(ChannelHandlerContext ctx) throws Exception {
+super.handlerRemoved(ctx);
+logger.trace("Removed " + RpcConstants.SASL_ENCRYPTION_HANDLER + " 
handler");
+  }
+
+  public void encode(ChannelHandlerContext ctx, ByteBuf msg, List 
out) throws IOException {
+
+if (!ctx.channel().isOpen()) {
+  logger.debug("In " + RpcConstants.SASL_ENCRYPTION_HANDLER + " and 
channel is not open. " +
+  "So releasing msg memory before encryption.");
+  msg.release();
+  return;
+}
+
+try {
+  // If encryption is enabled then this handler will always get 
ByteBuf of type Composite ByteBuf
+  checkArgument(msg instanceof CompositeByteBuf);
+

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15991407#comment-15991407
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r113838037
  
--- Diff: 
exec/rpc/src/main/java/org/apache/drill/exec/rpc/SaslEncryptionHandler.java ---
@@ -0,0 +1,181 @@
+/*
+ * 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.drill.exec.rpc;
+
+import io.netty.buffer.ByteBuf;
+import io.netty.buffer.CompositeByteBuf;
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.handler.codec.MessageToMessageEncoder;
+
+import org.apache.drill.exec.exception.OutOfMemoryException;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
+import java.util.List;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+
+/**
+ * Handler to wrap the input Composite ByteBuf components separately and 
append the encrypted length for each
+ * component in the output ByteBuf. If there are multiple components in 
the input ByteBuf then each component will be
+ * encrypted individually and added to output ByteBuf with it's length 
prepended.
+ * 
+ * Example:
+ * Input ByteBuf  --> [B1,B2] - 2 component ByteBuf of 16K byte each.
+ * Output ByteBuf --> [[EBLN1, EB1], [EBLN2, EB2]] - List of ByteBuf's 
with each ByteBuf containing
+ *Encrypted Byte Length (EBLNx) in network order as 
per SASL RFC and Encrypted Bytes (EBx).
+ * 
+ */
+class SaslEncryptionHandler extends MessageToMessageEncoder {
+
+  private static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(
+  SaslEncryptionHandler.class.getCanonicalName());
+
+  private final SaslCodec saslCodec;
+
+  private final int maxRawWrapSize;
+
+  private byte[] origMsgBuffer;
+
+  private final ByteBuffer lengthOctets;
+
+  private final OutOfMemoryHandler outOfMemoryHandler;
+
+  /**
+   * We don't provide preference to allocator to use heap buffer instead 
of direct buffer.
+   * Drill uses it's own buffer allocator which doesn't support heap 
buffer allocation. We use
+   * Drill buffer allocator in the channel.
+   */
+  SaslEncryptionHandler(SaslCodec saslCodec, final int maxRawWrapSize, 
final OutOfMemoryHandler oomHandler) {
+this.saslCodec = saslCodec;
+this.maxRawWrapSize = maxRawWrapSize;
+this.outOfMemoryHandler = oomHandler;
+
+// The maximum size of the component will be maxRawWrapSize. Since 
this is maximum size we can allocate once
+// and reuse it for each component encode.
+origMsgBuffer = new byte[this.maxRawWrapSize];
+lengthOctets = ByteBuffer.allocate(RpcConstants.LENGTH_FIELD_LENGTH);
+lengthOctets.order(ByteOrder.BIG_ENDIAN);
+  }
+
+  @Override
+  public void handlerAdded(ChannelHandlerContext ctx) throws Exception {
+super.handlerAdded(ctx);
+logger.trace("Added " + RpcConstants.SASL_ENCRYPTION_HANDLER + " 
handler!");
+  }
+
+  @Override
+  public void handlerRemoved(ChannelHandlerContext ctx) throws Exception {
+super.handlerRemoved(ctx);
+logger.trace("Removed " + RpcConstants.SASL_ENCRYPTION_HANDLER + " 
handler");
+  }
+
+  public void encode(ChannelHandlerContext ctx, ByteBuf msg, List 
out) throws IOException {
+
+if (!ctx.channel().isOpen()) {
+  logger.debug("In " + RpcConstants.SASL_ENCRYPTION_HANDLER + " and 
channel is not open. " +
+  "So releasing msg memory before encryption.");
+  msg.release();
+  return;
+}
+
+try {
+  // If encryption is enabled then this handler will always get 
ByteBuf of type Composite ByteBuf
+  checkArgument(msg instanceof CompositeByteBuf);
+

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15991395#comment-15991395
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r113804812
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/security/AuthenticationOutcomeListener.java
 ---
@@ -243,4 +249,46 @@ public SaslMessage process(SaslChallengeContext 
context) throws Exception {
   }
 }
   }
+
+  private static void handleSuccess(SaslChallengeContext context) throws 
SaslException {
+final ClientConnection connection = context.connection;
+final SaslClient saslClient = connection.getSaslClient();
+
+try {
+  // Check if connection was marked for being secure then verify for 
negotiated QOP value for
+  // correctness.
+  final String negotiatedQOP = 
saslClient.getNegotiatedProperty(Sasl.QOP).toString();
+  final String expectedQOP = connection.isEncryptionEnabled()
+  ? SaslProperties.QualityOfProtection.PRIVACY.getSaslQop()
+  : SaslProperties.QualityOfProtection.AUTHENTICATION.getSaslQop();
+
+  if (!(negotiatedQOP.equals(expectedQOP))) {
+throw new SaslException(String.format("Mismatch in negotiated QOP 
value: %s and Expected QOP value: %s",
+negotiatedQOP, expectedQOP));
+  }
+
+  // Update the rawWrapChunkSize with the negotiated buffer size since 
we cannot call encode with more than
+  // negotiated size of buffer.
+  if (connection.isEncryptionEnabled()) {
+final int negotiatedRawSendSize = Integer.parseInt(
+
saslClient.getNegotiatedProperty(Sasl.RAW_SEND_SIZE).toString());
+if (negotiatedRawSendSize <= 0) {
+  throw new SaslException(String.format("Negotiated rawSendSize: 
%d is invalid. Please check the configured " +
+  "value of sasl.encryption.encodesize. It might be configured 
to a very small value.",
+  negotiatedRawSendSize));
+}
+connection.setWrapSizeLimit(negotiatedRawSendSize);
--- End diff --

Changed the name in SaslEncryptionHandler to wrapSizeLimit as well. Don't 
want to keep the name as maxSendBufferSize since i.e. very generic name and it 
is actually a rawSendSize from sasl perspective for wrap function. From Drill's 
perspective I think wrapSizeLimit makes more sense.


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15991408#comment-15991408
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r113837790
  
--- Diff: 
exec/rpc/src/main/java/org/apache/drill/exec/rpc/SaslDecryptionHandler.java ---
@@ -0,0 +1,162 @@
+/*
+ * 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.drill.exec.rpc;
+
+import io.netty.buffer.ByteBuf;
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.handler.codec.MessageToMessageDecoder;
+
+import org.apache.drill.exec.exception.OutOfMemoryException;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
+import java.util.List;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+/**
+ * Handler to Decrypt the input ByteBuf. It expects input to be in format 
where it has length of the bytes to
+ * decode in network order and actual encrypted bytes. The handler reads 
the length and then reads the
+ * required bytes to pass it to unwrap function for decryption. The 
decrypted buffer is copied to a new
+ * ByteBuf and added to out list.
+ * 
+ * Example:
+ * Input - [EBLN1, EB1, EBLN2, EB2] --> ByteBuf with repeated 
combination of encrypted byte length
+ * in network order (EBLNx) and encrypted bytes (EB)
+ * Output - [DB1] --> Decrypted ByteBuf of first chunk.(EB1)
+ * 
+ */
+class SaslDecryptionHandler extends MessageToMessageDecoder {
+
+  private static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(
+  SaslDecryptionHandler.class.getCanonicalName());
+
+  private final SaslCodec saslCodec;
+
+  private final int maxEncodedSize;
+
+  private final OutOfMemoryHandler outOfMemoryHandler;
+
+  private final byte[] encodedMsg;
+
+  private final ByteBuffer lengthOctets;
+
+  SaslDecryptionHandler(SaslCodec saslCodec, int maxEncodedSize, 
OutOfMemoryHandler oomHandler) {
+this.saslCodec = saslCodec;
+this.outOfMemoryHandler = oomHandler;
+this.maxEncodedSize = maxEncodedSize;
+
+// Allocate the byte array of maxEncodedSize to reuse for each encoded 
packet received on this connection
+// Maximum value of maxEncodedSize can be 16MB (i.e. OXFF)
+encodedMsg = new byte[maxEncodedSize];
+lengthOctets = ByteBuffer.allocate(RpcConstants.LENGTH_FIELD_LENGTH);
+lengthOctets.order(ByteOrder.BIG_ENDIAN);
+  }
+
+  @Override
+  public void handlerAdded(ChannelHandlerContext ctx) throws Exception {
+super.handlerAdded(ctx);
+logger.trace("Added " + RpcConstants.SASL_DECRYPTION_HANDLER + " 
handler");
+  }
+
+  @Override
+  public void handlerRemoved(ChannelHandlerContext ctx) throws Exception {
+super.handlerRemoved(ctx);
+logger.trace("Removed " + RpcConstants.SASL_DECRYPTION_HANDLER + " 
handler");
+  }
+
+  public void decode(ChannelHandlerContext ctx, ByteBuf msg, List 
out) throws IOException {
+
+if (!ctx.channel().isOpen()) {
+  logger.trace("Channel closed before decoding the message of {} 
bytes", msg.readableBytes());
+  msg.skipBytes(msg.readableBytes());
+  return;
+}
+
+try {
+  if(logger.isTraceEnabled()) {
+logger.trace("Trying to decrypt the encrypted message of size: {} 
with maxEncodedSize", msg.readableBytes());
+  }
+
+  final byte[] wrappedMsg;
+
+  // All the encrypted blocks are prefixed with it's length in network 
byte order (or BigEndian format). Netty's
+  // default Byte order of ByteBuf is Little Endian, so we cannot just 
do msg.getInt() as that will read the 4
+  // octets in little endian format.
+  //
+  // 

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15991388#comment-15991388
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r113807185
  
--- Diff: 
exec/rpc/src/main/java/org/apache/drill/exec/rpc/EncryptionContext.java ---
@@ -0,0 +1,37 @@
+/*
+ * 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.drill.exec.rpc;
+
+public interface EncryptionContext {
+
+  boolean isEncryptionEnabled();
+
+  void setEncryption(boolean encryptionEnabled);
+
+  void setMaxWrappedSize(int maxWrappedChunkSize);
--- End diff --

Same as for MaxRawSendSize. MaxReceiveBufferSize is very generic name. It 
makes more sense to call it MaxWrappedSize since that's what it actually is 
from Drill's perspective.


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15991412#comment-15991412
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r113837839
  
--- Diff: 
exec/rpc/src/main/java/org/apache/drill/exec/rpc/SaslDecryptionHandler.java ---
@@ -0,0 +1,162 @@
+/*
+ * 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.drill.exec.rpc;
+
+import io.netty.buffer.ByteBuf;
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.handler.codec.MessageToMessageDecoder;
+
+import org.apache.drill.exec.exception.OutOfMemoryException;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
+import java.util.List;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+/**
+ * Handler to Decrypt the input ByteBuf. It expects input to be in format 
where it has length of the bytes to
+ * decode in network order and actual encrypted bytes. The handler reads 
the length and then reads the
+ * required bytes to pass it to unwrap function for decryption. The 
decrypted buffer is copied to a new
+ * ByteBuf and added to out list.
+ * 
+ * Example:
+ * Input - [EBLN1, EB1, EBLN2, EB2] --> ByteBuf with repeated 
combination of encrypted byte length
+ * in network order (EBLNx) and encrypted bytes (EB)
+ * Output - [DB1] --> Decrypted ByteBuf of first chunk.(EB1)
+ * 
+ */
+class SaslDecryptionHandler extends MessageToMessageDecoder {
+
+  private static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(
+  SaslDecryptionHandler.class.getCanonicalName());
+
+  private final SaslCodec saslCodec;
+
+  private final int maxEncodedSize;
+
+  private final OutOfMemoryHandler outOfMemoryHandler;
+
+  private final byte[] encodedMsg;
+
+  private final ByteBuffer lengthOctets;
+
+  SaslDecryptionHandler(SaslCodec saslCodec, int maxEncodedSize, 
OutOfMemoryHandler oomHandler) {
+this.saslCodec = saslCodec;
+this.outOfMemoryHandler = oomHandler;
+this.maxEncodedSize = maxEncodedSize;
+
+// Allocate the byte array of maxEncodedSize to reuse for each encoded 
packet received on this connection
+// Maximum value of maxEncodedSize can be 16MB (i.e. OXFF)
+encodedMsg = new byte[maxEncodedSize];
+lengthOctets = ByteBuffer.allocate(RpcConstants.LENGTH_FIELD_LENGTH);
+lengthOctets.order(ByteOrder.BIG_ENDIAN);
+  }
+
+  @Override
+  public void handlerAdded(ChannelHandlerContext ctx) throws Exception {
+super.handlerAdded(ctx);
+logger.trace("Added " + RpcConstants.SASL_DECRYPTION_HANDLER + " 
handler");
+  }
+
+  @Override
+  public void handlerRemoved(ChannelHandlerContext ctx) throws Exception {
+super.handlerRemoved(ctx);
+logger.trace("Removed " + RpcConstants.SASL_DECRYPTION_HANDLER + " 
handler");
+  }
+
+  public void decode(ChannelHandlerContext ctx, ByteBuf msg, List 
out) throws IOException {
+
+if (!ctx.channel().isOpen()) {
+  logger.trace("Channel closed before decoding the message of {} 
bytes", msg.readableBytes());
+  msg.skipBytes(msg.readableBytes());
+  return;
+}
+
+try {
+  if(logger.isTraceEnabled()) {
+logger.trace("Trying to decrypt the encrypted message of size: {} 
with maxEncodedSize", msg.readableBytes());
+  }
+
+  final byte[] wrappedMsg;
+
+  // All the encrypted blocks are prefixed with it's length in network 
byte order (or BigEndian format). Netty's
+  // default Byte order of ByteBuf is Little Endian, so we cannot just 
do msg.getInt() as that will read the 4
+  // octets in little endian format.
+  //
+  // 

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15991396#comment-15991396
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r113522675
  
--- Diff: 
exec/rpc/src/main/java/org/apache/drill/exec/rpc/SaslEncryptionHandler.java ---
@@ -0,0 +1,181 @@
+/*
+ * 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.drill.exec.rpc;
+
+import io.netty.buffer.ByteBuf;
+import io.netty.buffer.CompositeByteBuf;
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.handler.codec.MessageToMessageEncoder;
+
+import org.apache.drill.exec.exception.OutOfMemoryException;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
+import java.util.List;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+
+/**
+ * Handler to wrap the input Composite ByteBuf components separately and 
append the encrypted length for each
+ * component in the output ByteBuf. If there are multiple components in 
the input ByteBuf then each component will be
+ * encrypted individually and added to output ByteBuf with it's length 
prepended.
+ * 
+ * Example:
+ * Input ByteBuf  --> [B1,B2] - 2 component ByteBuf of 16K byte each.
+ * Output ByteBuf --> [[EBLN1, EB1], [EBLN2, EB2]] - List of ByteBuf's 
with each ByteBuf containing
+ *Encrypted Byte Length (EBLNx) in network order as 
per SASL RFC and Encrypted Bytes (EBx).
+ * 
+ */
+class SaslEncryptionHandler extends MessageToMessageEncoder {
+
+  private static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(
+  SaslEncryptionHandler.class.getCanonicalName());
+
+  private final SaslCodec saslCodec;
+
+  private final int maxRawWrapSize;
+
+  private byte[] origMsgBuffer;
+
+  private final ByteBuffer lengthOctets;
+
+  private final OutOfMemoryHandler outOfMemoryHandler;
+
+  /**
+   * We don't provide preference to allocator to use heap buffer instead 
of direct buffer.
+   * Drill uses it's own buffer allocator which doesn't support heap 
buffer allocation. We use
+   * Drill buffer allocator in the channel.
+   */
+  SaslEncryptionHandler(SaslCodec saslCodec, final int maxRawWrapSize, 
final OutOfMemoryHandler oomHandler) {
+this.saslCodec = saslCodec;
+this.maxRawWrapSize = maxRawWrapSize;
+this.outOfMemoryHandler = oomHandler;
+
+// The maximum size of the component will be maxRawWrapSize. Since 
this is maximum size we can allocate once
+// and reuse it for each component encode.
+origMsgBuffer = new byte[this.maxRawWrapSize];
+lengthOctets = ByteBuffer.allocate(RpcConstants.LENGTH_FIELD_LENGTH);
+lengthOctets.order(ByteOrder.BIG_ENDIAN);
+  }
+
+  @Override
+  public void handlerAdded(ChannelHandlerContext ctx) throws Exception {
+super.handlerAdded(ctx);
+logger.trace("Added " + RpcConstants.SASL_ENCRYPTION_HANDLER + " 
handler!");
+  }
+
+  @Override
+  public void handlerRemoved(ChannelHandlerContext ctx) throws Exception {
+super.handlerRemoved(ctx);
+logger.trace("Removed " + RpcConstants.SASL_ENCRYPTION_HANDLER + " 
handler");
+  }
+
+  public void encode(ChannelHandlerContext ctx, ByteBuf msg, List 
out) throws IOException {
+
+if (!ctx.channel().isOpen()) {
+  logger.debug("In " + RpcConstants.SASL_ENCRYPTION_HANDLER + " and 
channel is not open. " +
+  "So releasing msg memory before encryption.");
+  msg.release();
+  return;
+}
+
+try {
+  // If encryption is enabled then this handler will always get 
ByteBuf of type Composite ByteBuf
+  checkArgument(msg instanceof CompositeByteBuf);
+

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15991400#comment-15991400
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r113836168
  
--- Diff: 
exec/rpc/src/main/java/org/apache/drill/exec/rpc/SaslDecryptionHandler.java ---
@@ -0,0 +1,162 @@
+/*
+ * 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.drill.exec.rpc;
+
+import io.netty.buffer.ByteBuf;
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.handler.codec.MessageToMessageDecoder;
+
+import org.apache.drill.exec.exception.OutOfMemoryException;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
+import java.util.List;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+/**
+ * Handler to Decrypt the input ByteBuf. It expects input to be in format 
where it has length of the bytes to
+ * decode in network order and actual encrypted bytes. The handler reads 
the length and then reads the
+ * required bytes to pass it to unwrap function for decryption. The 
decrypted buffer is copied to a new
+ * ByteBuf and added to out list.
+ * 
+ * Example:
+ * Input - [EBLN1, EB1, EBLN2, EB2] --> ByteBuf with repeated 
combination of encrypted byte length
+ * in network order (EBLNx) and encrypted bytes (EB)
+ * Output - [DB1] --> Decrypted ByteBuf of first chunk.(EB1)
+ * 
+ */
+class SaslDecryptionHandler extends MessageToMessageDecoder {
+
+  private static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(
+  SaslDecryptionHandler.class.getCanonicalName());
+
+  private final SaslCodec saslCodec;
+
+  private final int maxEncodedSize;
+
+  private final OutOfMemoryHandler outOfMemoryHandler;
+
+  private final byte[] encodedMsg;
+
+  private final ByteBuffer lengthOctets;
+
+  SaslDecryptionHandler(SaslCodec saslCodec, int maxEncodedSize, 
OutOfMemoryHandler oomHandler) {
+this.saslCodec = saslCodec;
+this.outOfMemoryHandler = oomHandler;
+this.maxEncodedSize = maxEncodedSize;
+
+// Allocate the byte array of maxEncodedSize to reuse for each encoded 
packet received on this connection
+// Maximum value of maxEncodedSize can be 16MB (i.e. OXFF)
+encodedMsg = new byte[maxEncodedSize];
+lengthOctets = ByteBuffer.allocate(RpcConstants.LENGTH_FIELD_LENGTH);
+lengthOctets.order(ByteOrder.BIG_ENDIAN);
+  }
+
+  @Override
+  public void handlerAdded(ChannelHandlerContext ctx) throws Exception {
+super.handlerAdded(ctx);
+logger.trace("Added " + RpcConstants.SASL_DECRYPTION_HANDLER + " 
handler");
+  }
+
+  @Override
+  public void handlerRemoved(ChannelHandlerContext ctx) throws Exception {
+super.handlerRemoved(ctx);
+logger.trace("Removed " + RpcConstants.SASL_DECRYPTION_HANDLER + " 
handler");
+  }
+
+  public void decode(ChannelHandlerContext ctx, ByteBuf msg, List 
out) throws IOException {
+
+if (!ctx.channel().isOpen()) {
+  logger.trace("Channel closed before decoding the message of {} 
bytes", msg.readableBytes());
+  msg.skipBytes(msg.readableBytes());
+  return;
+}
+
+try {
+  if(logger.isTraceEnabled()) {
+logger.trace("Trying to decrypt the encrypted message of size: {} 
with maxEncodedSize", msg.readableBytes());
+  }
+
+  final byte[] wrappedMsg;
+
+  // All the encrypted blocks are prefixed with it's length in network 
byte order (or BigEndian format). Netty's
+  // default Byte order of ByteBuf is Little Endian, so we cannot just 
do msg.getInt() as that will read the 4
+  // octets in little endian format.
+  //
+  // 

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15991402#comment-15991402
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r113835682
  
--- Diff: 
exec/rpc/src/main/java/org/apache/drill/exec/rpc/RemoteConnection.java ---
@@ -51,6 +51,12 @@
 
   SocketAddress getRemoteAddress();
 
+  void addSecurityHandlers();
+
+  void incConnectionCounter();
--- End diff --

Removed the methods from interface.


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15991403#comment-15991403
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r113808458
  
--- Diff: 
exec/rpc/src/main/java/org/apache/drill/exec/rpc/SaslDecryptionHandler.java ---
@@ -0,0 +1,162 @@
+/*
+ * 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.drill.exec.rpc;
+
+import io.netty.buffer.ByteBuf;
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.handler.codec.MessageToMessageDecoder;
+
+import org.apache.drill.exec.exception.OutOfMemoryException;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
+import java.util.List;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+/**
+ * Handler to Decrypt the input ByteBuf. It expects input to be in format 
where it has length of the bytes to
+ * decode in network order and actual encrypted bytes. The handler reads 
the length and then reads the
+ * required bytes to pass it to unwrap function for decryption. The 
decrypted buffer is copied to a new
+ * ByteBuf and added to out list.
+ * 
+ * Example:
+ * Input - [EBLN1, EB1, EBLN2, EB2] --> ByteBuf with repeated 
combination of encrypted byte length
+ * in network order (EBLNx) and encrypted bytes (EB)
+ * Output - [DB1] --> Decrypted ByteBuf of first chunk.(EB1)
+ * 
+ */
+class SaslDecryptionHandler extends MessageToMessageDecoder {
+
+  private static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(
+  SaslDecryptionHandler.class.getCanonicalName());
+
+  private final SaslCodec saslCodec;
+
+  private final int maxEncodedSize;
+
+  private final OutOfMemoryHandler outOfMemoryHandler;
+
+  private final byte[] encodedMsg;
+
+  private final ByteBuffer lengthOctets;
+
+  SaslDecryptionHandler(SaslCodec saslCodec, int maxEncodedSize, 
OutOfMemoryHandler oomHandler) {
+this.saslCodec = saslCodec;
+this.outOfMemoryHandler = oomHandler;
+this.maxEncodedSize = maxEncodedSize;
+
+// Allocate the byte array of maxEncodedSize to reuse for each encoded 
packet received on this connection
+// Maximum value of maxEncodedSize can be 16MB (i.e. OXFF)
+encodedMsg = new byte[maxEncodedSize];
+lengthOctets = ByteBuffer.allocate(RpcConstants.LENGTH_FIELD_LENGTH);
+lengthOctets.order(ByteOrder.BIG_ENDIAN);
+  }
+
+  @Override
+  public void handlerAdded(ChannelHandlerContext ctx) throws Exception {
+super.handlerAdded(ctx);
+logger.trace("Added " + RpcConstants.SASL_DECRYPTION_HANDLER + " 
handler");
+  }
+
+  @Override
+  public void handlerRemoved(ChannelHandlerContext ctx) throws Exception {
+super.handlerRemoved(ctx);
+logger.trace("Removed " + RpcConstants.SASL_DECRYPTION_HANDLER + " 
handler");
+  }
+
+  public void decode(ChannelHandlerContext ctx, ByteBuf msg, List 
out) throws IOException {
+
+if (!ctx.channel().isOpen()) {
+  logger.trace("Channel closed before decoding the message of {} 
bytes", msg.readableBytes());
+  msg.skipBytes(msg.readableBytes());
+  return;
+}
+
+try {
+  if(logger.isTraceEnabled()) {
+logger.trace("Trying to decrypt the encrypted message of size: {} 
with maxEncodedSize", msg.readableBytes());
+  }
+
+  final byte[] wrappedMsg;
+
+  // All the encrypted blocks are prefixed with it's length in network 
byte order (or BigEndian format). Netty's
+  // default Byte order of ByteBuf is Little Endian, so we cannot just 
do msg.getInt() as that will read the 4
+  // octets in little endian format.
+  //
+  // 

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15991391#comment-15991391
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r113376924
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/security/ServerAuthenticationHandler.java
 ---
@@ -251,25 +255,67 @@ void process(SaslResponseContext context) 
throws Exception {
   private static , T extends EnumLite>
   void handleSuccess(final SaslResponseContext context, final 
SaslMessage.Builder challenge,
  final SaslServer saslServer) throws IOException {
-context.connection.changeHandlerTo(context.requestHandler);
-context.connection.finalizeSaslSession();
-context.sender.send(new Response(context.saslResponseType, 
challenge.build()));
 
-// setup security layers here..
+final S connection = context.connection;
+connection.changeHandlerTo(context.requestHandler);
+connection.finalizeSaslSession();
+
+// Check the negotiated property before sending the response back to 
client
+try {
+  final String negotiatedQOP = 
saslServer.getNegotiatedProperty(Sasl.QOP).toString();
--- End diff --

Please see comment above.


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15991401#comment-15991401
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r113835699
  
--- Diff: 
exec/rpc/src/main/java/org/apache/drill/exec/rpc/RpcConstants.java ---
@@ -24,4 +24,29 @@ private RpcConstants(){}
 
   public static final boolean SOME_DEBUGGING = false;
   public static final boolean EXTRA_DEBUGGING = false;
+
+  // RPC Handler names
+  public static final String TIMEOUT_HANDLER = "timeout-handler";
+  public static final String PROTOCOL_DECODER = "protocol-decoder";
+  public static final String PROTOCOL_ENCODER = "protocol-encoder";
+  public static final String MESSAGE_DECODER = "message-decoder";
+  public static final String HANDSHAKE_HANDLER = "handshake-handler";
+  public static final String MESSAGE_HANDLER = "message-handler";
+  public static final String EXCEPTION_HANDLER = "exception-handler";
+  public static final String IDLE_STATE_HANDLER = "idle-state-handler";
+  public static final String SASL_DECRYPTION_HANDLER = 
"sasldecryption-handler";
--- End diff --

Fixed


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15991397#comment-15991397
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r113833928
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/security/AuthenticationOutcomeListener.java
 ---
@@ -143,16 +149,16 @@ public void interrupted(InterruptedException e) {
 completionListener.interrupted(e);
   }
 
-  private static class SaslChallengeContext {
+  private static class SaslChallengeContext {
--- End diff --

Fixed.


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15991393#comment-15991393
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r113378601
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/security/ServerAuthenticationHandler.java
 ---
@@ -251,25 +255,67 @@ void process(SaslResponseContext context) 
throws Exception {
   private static , T extends EnumLite>
   void handleSuccess(final SaslResponseContext context, final 
SaslMessage.Builder challenge,
  final SaslServer saslServer) throws IOException {
-context.connection.changeHandlerTo(context.requestHandler);
-context.connection.finalizeSaslSession();
-context.sender.send(new Response(context.saslResponseType, 
challenge.build()));
 
-// setup security layers here..
+final S connection = context.connection;
+connection.changeHandlerTo(context.requestHandler);
+connection.finalizeSaslSession();
+
+// Check the negotiated property before sending the response back to 
client
+try {
+  final String negotiatedQOP = 
saslServer.getNegotiatedProperty(Sasl.QOP).toString();
+  final String expectedQOP = (connection.isEncryptionEnabled())
+  ? SaslProperties.QualityOfProtection.PRIVACY.getSaslQop()
+  : SaslProperties.QualityOfProtection.AUTHENTICATION.getSaslQop();
+
+  if (!(negotiatedQOP.equals(expectedQOP))) {
+throw new SaslException(String.format("Mismatch in negotiated QOP 
value: %s and Expected QOP value: %s",
+negotiatedQOP, expectedQOP));
+  }
+
+  // Update the rawWrapSendSize with the negotiated rawSendSize since 
we cannot call encode with more than the
+  // negotiated size of buffer
+  if (connection.isEncryptionEnabled()) {
+final int negotiatedRawSendSize = Integer.parseInt(
+
saslServer.getNegotiatedProperty(Sasl.RAW_SEND_SIZE).toString());
+if (negotiatedRawSendSize <= 0) {
+  throw new SaslException(String.format("Negotiated rawSendSize: 
%d is invalid. Please check the configured " +
+  "value of encryption.sasl.max_wrapped_size. It might be 
configured to a very small value.",
+  negotiatedRawSendSize));
+}
+connection.setWrapSizeLimit(negotiatedRawSendSize);
--- End diff --

Good catch!. Previous logic of having a new EncryptionContext object with 
each connection was taking care of it. Didn't realized this while making the 
change. Will have a separate EncryptionContext object for each connection and 
initialize it with the passed object in constructor.


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15991390#comment-15991390
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r113378898
  
--- Diff: 
exec/rpc/src/main/java/org/apache/drill/exec/rpc/ChunkCreationHandler.java ---
@@ -0,0 +1,101 @@
+/*
+ * 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.drill.exec.rpc;
+
+
+import io.netty.buffer.ByteBuf;
+import io.netty.buffer.CompositeByteBuf;
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.handler.codec.MessageToMessageEncoder;
+
+import java.util.List;
+
+import static com.google.common.base.Preconditions.checkArgument;
+import static java.lang.Math.min;
+
+/**
+ * Handler that converts an input ByteBuf into chunk size ByteBuf's and 
add it to the
+ * CompositeByteBuf as individual components. This is done irrespective of 
chunk mode is
--- End diff --

Fixed.


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15991404#comment-15991404
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r113834772
  
--- Diff: 
exec/rpc/src/main/java/org/apache/drill/exec/rpc/EncryptionContext.java ---
@@ -0,0 +1,37 @@
+/*
+ * 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.drill.exec.rpc;
+
+public interface EncryptionContext {
--- End diff --

As discussed keeping as is. The main purpose of changing to a separate 
interface for EncryptionContext was to keep RemoteConnection interface as is 
which was the concern in previous comment. Doing getEncryptionOptions to return 
the instance of EncryptionContextImpl gives control to the caller to set and 
get upon the instance. Since instance is private to a connection it should only 
be allowed to change the state and caller will only have exposure to the 
functions as needed.  


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15991406#comment-15991406
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r113838105
  
--- Diff: 
exec/rpc/src/main/java/org/apache/drill/exec/rpc/SaslEncryptionHandler.java ---
@@ -0,0 +1,181 @@
+/*
+ * 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.drill.exec.rpc;
+
+import io.netty.buffer.ByteBuf;
+import io.netty.buffer.CompositeByteBuf;
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.handler.codec.MessageToMessageEncoder;
+
+import org.apache.drill.exec.exception.OutOfMemoryException;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
+import java.util.List;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+
+/**
+ * Handler to wrap the input Composite ByteBuf components separately and 
append the encrypted length for each
+ * component in the output ByteBuf. If there are multiple components in 
the input ByteBuf then each component will be
+ * encrypted individually and added to output ByteBuf with it's length 
prepended.
+ * 
+ * Example:
+ * Input ByteBuf  --> [B1,B2] - 2 component ByteBuf of 16K byte each.
+ * Output ByteBuf --> [[EBLN1, EB1], [EBLN2, EB2]] - List of ByteBuf's 
with each ByteBuf containing
+ *Encrypted Byte Length (EBLNx) in network order as 
per SASL RFC and Encrypted Bytes (EBx).
+ * 
+ */
+class SaslEncryptionHandler extends MessageToMessageEncoder {
+
+  private static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(
+  SaslEncryptionHandler.class.getCanonicalName());
+
+  private final SaslCodec saslCodec;
+
+  private final int maxRawWrapSize;
+
+  private byte[] origMsgBuffer;
+
+  private final ByteBuffer lengthOctets;
+
+  private final OutOfMemoryHandler outOfMemoryHandler;
+
+  /**
+   * We don't provide preference to allocator to use heap buffer instead 
of direct buffer.
+   * Drill uses it's own buffer allocator which doesn't support heap 
buffer allocation. We use
+   * Drill buffer allocator in the channel.
+   */
+  SaslEncryptionHandler(SaslCodec saslCodec, final int maxRawWrapSize, 
final OutOfMemoryHandler oomHandler) {
+this.saslCodec = saslCodec;
+this.maxRawWrapSize = maxRawWrapSize;
+this.outOfMemoryHandler = oomHandler;
+
+// The maximum size of the component will be maxRawWrapSize. Since 
this is maximum size we can allocate once
+// and reuse it for each component encode.
+origMsgBuffer = new byte[this.maxRawWrapSize];
+lengthOctets = ByteBuffer.allocate(RpcConstants.LENGTH_FIELD_LENGTH);
+lengthOctets.order(ByteOrder.BIG_ENDIAN);
+  }
+
+  @Override
+  public void handlerAdded(ChannelHandlerContext ctx) throws Exception {
+super.handlerAdded(ctx);
+logger.trace("Added " + RpcConstants.SASL_ENCRYPTION_HANDLER + " 
handler!");
+  }
+
+  @Override
+  public void handlerRemoved(ChannelHandlerContext ctx) throws Exception {
+super.handlerRemoved(ctx);
+logger.trace("Removed " + RpcConstants.SASL_ENCRYPTION_HANDLER + " 
handler");
+  }
+
+  public void encode(ChannelHandlerContext ctx, ByteBuf msg, List 
out) throws IOException {
+
+if (!ctx.channel().isOpen()) {
+  logger.debug("In " + RpcConstants.SASL_ENCRYPTION_HANDLER + " and 
channel is not open. " +
+  "So releasing msg memory before encryption.");
+  msg.release();
+  return;
+}
+
+try {
+  // If encryption is enabled then this handler will always get 
ByteBuf of type Composite ByteBuf
+  checkArgument(msg instanceof CompositeByteBuf);
+

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15991387#comment-15991387
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r113376608
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/security/AuthenticationOutcomeListener.java
 ---
@@ -243,4 +249,46 @@ public SaslMessage process(SaslChallengeContext 
context) throws Exception {
   }
 }
   }
+
+  private static void handleSuccess(SaslChallengeContext context) throws 
SaslException {
+final ClientConnection connection = context.connection;
+final SaslClient saslClient = connection.getSaslClient();
+
+try {
+  // Check if connection was marked for being secure then verify for 
negotiated QOP value for
+  // correctness.
+  final String negotiatedQOP = 
saslClient.getNegotiatedProperty(Sasl.QOP).toString();
--- End diff --

Null QOP is not auth. Instead when we pass QOP as null the mechanism will 
use the default QOP value for negotiation which is auth. So 
getNegotiatedProperty will always return a valid object.


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15991399#comment-15991399
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r113833301
  
--- Diff: 
exec/rpc/src/main/java/org/apache/drill/exec/rpc/SaslDecryptionHandler.java ---
@@ -0,0 +1,162 @@
+/*
+ * 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.drill.exec.rpc;
+
+import io.netty.buffer.ByteBuf;
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.handler.codec.MessageToMessageDecoder;
+
+import org.apache.drill.exec.exception.OutOfMemoryException;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
+import java.util.List;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+/**
+ * Handler to Decrypt the input ByteBuf. It expects input to be in format 
where it has length of the bytes to
+ * decode in network order and actual encrypted bytes. The handler reads 
the length and then reads the
+ * required bytes to pass it to unwrap function for decryption. The 
decrypted buffer is copied to a new
+ * ByteBuf and added to out list.
+ * 
+ * Example:
+ * Input - [EBLN1, EB1, EBLN2, EB2] --> ByteBuf with repeated 
combination of encrypted byte length
+ * in network order (EBLNx) and encrypted bytes (EB)
+ * Output - [DB1] --> Decrypted ByteBuf of first chunk.(EB1)
+ * 
+ */
+class SaslDecryptionHandler extends MessageToMessageDecoder {
+
+  private static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(
+  SaslDecryptionHandler.class.getCanonicalName());
+
+  private final SaslCodec saslCodec;
+
+  private final int maxEncodedSize;
+
+  private final OutOfMemoryHandler outOfMemoryHandler;
+
+  private final byte[] encodedMsg;
+
+  private final ByteBuffer lengthOctets;
+
+  SaslDecryptionHandler(SaslCodec saslCodec, int maxEncodedSize, 
OutOfMemoryHandler oomHandler) {
+this.saslCodec = saslCodec;
+this.outOfMemoryHandler = oomHandler;
+this.maxEncodedSize = maxEncodedSize;
+
+// Allocate the byte array of maxEncodedSize to reuse for each encoded 
packet received on this connection
+// Maximum value of maxEncodedSize can be 16MB (i.e. OXFF)
+encodedMsg = new byte[maxEncodedSize];
--- End diff --

Thanks for pointing out. The intent of having the pre-allocated buffer was 
to reuse the same heap buffer to copy the encoded bytes from DrillBuf to heap 
buffer rather than allocating new heap buffer all the times which can lead to 
GC thrashing. But 16MB being a too big of a buffer per connection, I did some 
perf test for the wrap function and it performs better with smaller chunks. So 
I will default the buffer size to 64K for now.  It's a configurable parameter 
which can be changed as needed.


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15991392#comment-15991392
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r113807219
  
--- Diff: 
exec/rpc/src/main/java/org/apache/drill/exec/rpc/EncryptionContext.java ---
@@ -0,0 +1,37 @@
+/*
+ * 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.drill.exec.rpc;
+
+public interface EncryptionContext {
+
+  boolean isEncryptionEnabled();
+
+  void setEncryption(boolean encryptionEnabled);
+
+  void setMaxWrappedSize(int maxWrappedChunkSize);
+
+  int getMaxWrappedSize();
+
+  void setWrapSizeLimit(int wrapSizeLimit);
--- End diff --

Same as before.


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15984250#comment-15984250
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r113375142
  
--- Diff: 
exec/rpc/src/main/java/org/apache/drill/exec/rpc/AbstractRemoteConnection.java 
---
@@ -224,4 +240,104 @@ public void close() {
 }
   }
 
+  /**
+   * Helps to add all the required security handler's after negotiation 
for encryption is completed.
+   * Handler's before encryption is negotiated are:
+   * 
+   *  PROTOCOL_DECODER {@link ProtobufLengthDecoder} 
+   *  MESSAGE_DECODER {@link RpcDecoder}  
+   *  PROTOCOL_ENCODER {@link RpcEncoder} 
+   *  HANDSHAKE_HANDLER {@link 
org.apache.drill.exec.rpc.BasicClient.ClientHandshakeHandler} OR
+   *{@link 
org.apache.drill.exec.rpc.BasicServer.ServerHandshakeHandler}  
+   *  optional - IDLE_STATE_HANDLER {@link 
org.apache.drill.exec.rpc.BasicClient.IdlePingHandler} OR
+   *   - TIMEOUT_HANDLER {@link 
org.apache.drill.exec.rpc.BasicServer.LoggingReadTimeoutHandler}  
+   *  MESSAGE_HANDLER {@link 
org.apache.drill.exec.rpc.RpcBus.InboundHandler} 
+   *  EXCEPTION_HANDLER {@link RpcExceptionHandler} 
+   * 
+   * Handler's after encryption is negotiated are:
+   * 
+   *  LENGTH_DECODER_HANDLER {@link LengthFieldBasedFrameDecoder}
+   *  SASL_DECRYPTION_HANDLER {@link SaslDecryptionHandler}
+   *  PROTOCOL_DECODER {@link ProtobufLengthDecoder}
+   *  MESSAGE_DECODER {@link RpcDecoder}
+   *  SASL_ENCRYPTION_HANDLER {@link SaslEncryptionHandler}
+   *  CHUNK_CREATION_HANDLER {@link ChunkCreationHandler}
+   *  PROTOCOL_ENCODER {@link RpcEncoder}
+   *  HANDSHAKE_HANDLER {@link 
org.apache.drill.exec.rpc.BasicClient.ClientHandshakeHandler} OR
+   *{@link 
org.apache.drill.exec.rpc.BasicServer.ServerHandshakeHandler}
+   *  optional - IDLE_STATE_HANDLER {@link 
org.apache.drill.exec.rpc.BasicClient.IdlePingHandler} OR
+   *   - TIMEOUT_HANDLER {@link 
org.apache.drill.exec.rpc.BasicServer.LoggingReadTimeoutHandler}
+   *  MESSAGE_HANDLER {@link 
org.apache.drill.exec.rpc.RpcBus.InboundHandler}
+   *  EXCEPTION_HANDLER {@link RpcExceptionHandler}
+   * 
+   * 
+   *  If encryption is enabled ChunkCreationHandler is always added to 
divide the Rpc message into chunks of
--- End diff --

This is required since Encoded Message size cannot be bigger than 16MB, 
this handler will take care of that. Keeping a separate handler for chunking 
purpose also helps to keep the encryption and chunking logic separate. As far 
as performance degradation is concerned I don't think there should be any 
notable difference. 


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15984072#comment-15984072
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r113348583
  
--- Diff: 
exec/rpc/src/main/java/org/apache/drill/exec/rpc/SaslEncryptionHandler.java ---
@@ -0,0 +1,181 @@
+/*
+ * 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.drill.exec.rpc;
+
+import io.netty.buffer.ByteBuf;
+import io.netty.buffer.CompositeByteBuf;
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.handler.codec.MessageToMessageEncoder;
+
+import org.apache.drill.exec.exception.OutOfMemoryException;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
+import java.util.List;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+
+/**
+ * Handler to wrap the input Composite ByteBuf components separately and 
append the encrypted length for each
+ * component in the output ByteBuf. If there are multiple components in 
the input ByteBuf then each component will be
+ * encrypted individually and added to output ByteBuf with it's length 
prepended.
+ * 
+ * Example:
+ * Input ByteBuf  --> [B1,B2] - 2 component ByteBuf of 16K byte each.
+ * Output ByteBuf --> [[EBLN1, EB1], [EBLN2, EB2]] - List of ByteBuf's 
with each ByteBuf containing
+ *Encrypted Byte Length (EBLNx) in network order as 
per SASL RFC and Encrypted Bytes (EBx).
+ * 
+ */
+class SaslEncryptionHandler extends MessageToMessageEncoder {
+
+  private static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(
+  SaslEncryptionHandler.class.getCanonicalName());
+
+  private final SaslCodec saslCodec;
+
+  private final int maxRawWrapSize;
+
+  private byte[] origMsgBuffer;
+
+  private final ByteBuffer lengthOctets;
+
+  private final OutOfMemoryHandler outOfMemoryHandler;
+
+  /**
+   * We don't provide preference to allocator to use heap buffer instead 
of direct buffer.
+   * Drill uses it's own buffer allocator which doesn't support heap 
buffer allocation. We use
+   * Drill buffer allocator in the channel.
+   */
+  SaslEncryptionHandler(SaslCodec saslCodec, final int maxRawWrapSize, 
final OutOfMemoryHandler oomHandler) {
+this.saslCodec = saslCodec;
+this.maxRawWrapSize = maxRawWrapSize;
+this.outOfMemoryHandler = oomHandler;
+
+// The maximum size of the component will be maxRawWrapSize. Since 
this is maximum size we can allocate once
+// and reuse it for each component encode.
+origMsgBuffer = new byte[this.maxRawWrapSize];
+lengthOctets = ByteBuffer.allocate(RpcConstants.LENGTH_FIELD_LENGTH);
+lengthOctets.order(ByteOrder.BIG_ENDIAN);
+  }
+
+  @Override
+  public void handlerAdded(ChannelHandlerContext ctx) throws Exception {
+super.handlerAdded(ctx);
+logger.trace("Added " + RpcConstants.SASL_ENCRYPTION_HANDLER + " 
handler!");
+  }
+
+  @Override
+  public void handlerRemoved(ChannelHandlerContext ctx) throws Exception {
+super.handlerRemoved(ctx);
+logger.trace("Removed " + RpcConstants.SASL_ENCRYPTION_HANDLER + " 
handler");
+  }
+
+  public void encode(ChannelHandlerContext ctx, ByteBuf msg, List 
out) throws IOException {
+
+if (!ctx.channel().isOpen()) {
+  logger.debug("In " + RpcConstants.SASL_ENCRYPTION_HANDLER + " and 
channel is not open. " +
+  "So releasing msg memory before encryption.");
+  msg.release();
+  return;
+}
+
+try {
+  // If encryption is enabled then this handler will always get 
ByteBuf of type Composite ByteBuf
+  checkArgument(msg instanceof 

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15984054#comment-15984054
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r113324923
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/security/ServerAuthenticationHandler.java
 ---
@@ -251,25 +255,67 @@ void process(SaslResponseContext context) 
throws Exception {
   private static , T extends EnumLite>
   void handleSuccess(final SaslResponseContext context, final 
SaslMessage.Builder challenge,
  final SaslServer saslServer) throws IOException {
-context.connection.changeHandlerTo(context.requestHandler);
-context.connection.finalizeSaslSession();
-context.sender.send(new Response(context.saslResponseType, 
challenge.build()));
 
-// setup security layers here..
+final S connection = context.connection;
+connection.changeHandlerTo(context.requestHandler);
+connection.finalizeSaslSession();
+
+// Check the negotiated property before sending the response back to 
client
+try {
+  final String negotiatedQOP = 
saslServer.getNegotiatedProperty(Sasl.QOP).toString();
--- End diff --

Could `.toString()` throw NPE, since null QOP is same as "auth"?


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15984047#comment-15984047
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r113324280
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/security/ServerAuthenticationHandler.java
 ---
@@ -251,25 +255,67 @@ void process(SaslResponseContext context) 
throws Exception {
   private static , T extends EnumLite>
   void handleSuccess(final SaslResponseContext context, final 
SaslMessage.Builder challenge,
  final SaslServer saslServer) throws IOException {
-context.connection.changeHandlerTo(context.requestHandler);
-context.connection.finalizeSaslSession();
-context.sender.send(new Response(context.saslResponseType, 
challenge.build()));
 
-// setup security layers here..
+final S connection = context.connection;
+connection.changeHandlerTo(context.requestHandler);
+connection.finalizeSaslSession();
+
+// Check the negotiated property before sending the response back to 
client
+try {
+  final String negotiatedQOP = 
saslServer.getNegotiatedProperty(Sasl.QOP).toString();
+  final String expectedQOP = (connection.isEncryptionEnabled())
+  ? SaslProperties.QualityOfProtection.PRIVACY.getSaslQop()
+  : SaslProperties.QualityOfProtection.AUTHENTICATION.getSaslQop();
+
+  if (!(negotiatedQOP.equals(expectedQOP))) {
+throw new SaslException(String.format("Mismatch in negotiated QOP 
value: %s and Expected QOP value: %s",
+negotiatedQOP, expectedQOP));
+  }
+
+  // Update the rawWrapSendSize with the negotiated rawSendSize since 
we cannot call encode with more than the
+  // negotiated size of buffer
+  if (connection.isEncryptionEnabled()) {
+final int negotiatedRawSendSize = Integer.parseInt(
+
saslServer.getNegotiatedProperty(Sasl.RAW_SEND_SIZE).toString());
+if (negotiatedRawSendSize <= 0) {
+  throw new SaslException(String.format("Negotiated rawSendSize: 
%d is invalid. Please check the configured " +
+  "value of encryption.sasl.max_wrapped_size. It might be 
configured to a very small value.",
+  negotiatedRawSendSize));
+}
+connection.setWrapSizeLimit(negotiatedRawSendSize);
--- End diff --

On the server side, there is only one EncryptionContext per connection 
type. This will have unintended consequences when there are multiple 
connections!

Since rawSendSize is negotiated, EncryptionContext objects need to live and 
die with a connection. Correct?


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15984052#comment-15984052
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r112789119
  
--- Diff: 
exec/rpc/src/main/java/org/apache/drill/exec/rpc/AbstractRemoteConnection.java 
---
@@ -224,4 +240,104 @@ public void close() {
 }
   }
 
+  /**
+   * Helps to add all the required security handler's after negotiation 
for encryption is completed.
+   * Handler's before encryption is negotiated are:
+   * 
+   *  PROTOCOL_DECODER {@link ProtobufLengthDecoder} 
+   *  MESSAGE_DECODER {@link RpcDecoder}  
+   *  PROTOCOL_ENCODER {@link RpcEncoder} 
+   *  HANDSHAKE_HANDLER {@link 
org.apache.drill.exec.rpc.BasicClient.ClientHandshakeHandler} OR
+   *{@link 
org.apache.drill.exec.rpc.BasicServer.ServerHandshakeHandler}  
+   *  optional - IDLE_STATE_HANDLER {@link 
org.apache.drill.exec.rpc.BasicClient.IdlePingHandler} OR
+   *   - TIMEOUT_HANDLER {@link 
org.apache.drill.exec.rpc.BasicServer.LoggingReadTimeoutHandler}  
+   *  MESSAGE_HANDLER {@link 
org.apache.drill.exec.rpc.RpcBus.InboundHandler} 
+   *  EXCEPTION_HANDLER {@link RpcExceptionHandler} 
+   * 
+   * Handler's after encryption is negotiated are:
+   * 
+   *  LENGTH_DECODER_HANDLER {@link LengthFieldBasedFrameDecoder}
+   *  SASL_DECRYPTION_HANDLER {@link SaslDecryptionHandler}
+   *  PROTOCOL_DECODER {@link ProtobufLengthDecoder}
+   *  MESSAGE_DECODER {@link RpcDecoder}
+   *  SASL_ENCRYPTION_HANDLER {@link SaslEncryptionHandler}
+   *  CHUNK_CREATION_HANDLER {@link ChunkCreationHandler}
+   *  PROTOCOL_ENCODER {@link RpcEncoder}
+   *  HANDSHAKE_HANDLER {@link 
org.apache.drill.exec.rpc.BasicClient.ClientHandshakeHandler} OR
+   *{@link 
org.apache.drill.exec.rpc.BasicServer.ServerHandshakeHandler}
+   *  optional - IDLE_STATE_HANDLER {@link 
org.apache.drill.exec.rpc.BasicClient.IdlePingHandler} OR
+   *   - TIMEOUT_HANDLER {@link 
org.apache.drill.exec.rpc.BasicServer.LoggingReadTimeoutHandler}
+   *  MESSAGE_HANDLER {@link 
org.apache.drill.exec.rpc.RpcBus.InboundHandler}
+   *  EXCEPTION_HANDLER {@link RpcExceptionHandler}
+   * 
+   * 
+   *  If encryption is enabled ChunkCreationHandler is always added to 
divide the Rpc message into chunks of
--- End diff --

But does always adding the ChunckCreationHandler have performance 
implications?


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15984066#comment-15984066
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r113349330
  
--- Diff: 
exec/rpc/src/main/java/org/apache/drill/exec/rpc/SaslEncryptionHandler.java ---
@@ -0,0 +1,181 @@
+/*
+ * 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.drill.exec.rpc;
+
+import io.netty.buffer.ByteBuf;
+import io.netty.buffer.CompositeByteBuf;
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.handler.codec.MessageToMessageEncoder;
+
+import org.apache.drill.exec.exception.OutOfMemoryException;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
+import java.util.List;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+
+/**
+ * Handler to wrap the input Composite ByteBuf components separately and 
append the encrypted length for each
+ * component in the output ByteBuf. If there are multiple components in 
the input ByteBuf then each component will be
+ * encrypted individually and added to output ByteBuf with it's length 
prepended.
+ * 
+ * Example:
+ * Input ByteBuf  --> [B1,B2] - 2 component ByteBuf of 16K byte each.
+ * Output ByteBuf --> [[EBLN1, EB1], [EBLN2, EB2]] - List of ByteBuf's 
with each ByteBuf containing
+ *Encrypted Byte Length (EBLNx) in network order as 
per SASL RFC and Encrypted Bytes (EBx).
+ * 
+ */
+class SaslEncryptionHandler extends MessageToMessageEncoder {
+
+  private static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(
+  SaslEncryptionHandler.class.getCanonicalName());
+
+  private final SaslCodec saslCodec;
+
+  private final int maxRawWrapSize;
+
+  private byte[] origMsgBuffer;
+
+  private final ByteBuffer lengthOctets;
+
+  private final OutOfMemoryHandler outOfMemoryHandler;
+
+  /**
+   * We don't provide preference to allocator to use heap buffer instead 
of direct buffer.
+   * Drill uses it's own buffer allocator which doesn't support heap 
buffer allocation. We use
+   * Drill buffer allocator in the channel.
+   */
+  SaslEncryptionHandler(SaslCodec saslCodec, final int maxRawWrapSize, 
final OutOfMemoryHandler oomHandler) {
+this.saslCodec = saslCodec;
+this.maxRawWrapSize = maxRawWrapSize;
+this.outOfMemoryHandler = oomHandler;
+
+// The maximum size of the component will be maxRawWrapSize. Since 
this is maximum size we can allocate once
+// and reuse it for each component encode.
+origMsgBuffer = new byte[this.maxRawWrapSize];
+lengthOctets = ByteBuffer.allocate(RpcConstants.LENGTH_FIELD_LENGTH);
+lengthOctets.order(ByteOrder.BIG_ENDIAN);
+  }
+
+  @Override
+  public void handlerAdded(ChannelHandlerContext ctx) throws Exception {
+super.handlerAdded(ctx);
+logger.trace("Added " + RpcConstants.SASL_ENCRYPTION_HANDLER + " 
handler!");
+  }
+
+  @Override
+  public void handlerRemoved(ChannelHandlerContext ctx) throws Exception {
+super.handlerRemoved(ctx);
+logger.trace("Removed " + RpcConstants.SASL_ENCRYPTION_HANDLER + " 
handler");
+  }
+
+  public void encode(ChannelHandlerContext ctx, ByteBuf msg, List 
out) throws IOException {
+
+if (!ctx.channel().isOpen()) {
+  logger.debug("In " + RpcConstants.SASL_ENCRYPTION_HANDLER + " and 
channel is not open. " +
+  "So releasing msg memory before encryption.");
+  msg.release();
+  return;
+}
+
+try {
+  // If encryption is enabled then this handler will always get 
ByteBuf of type Composite ByteBuf
+  checkArgument(msg instanceof 

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15984061#comment-15984061
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r113347107
  
--- Diff: 
exec/rpc/src/main/java/org/apache/drill/exec/rpc/SaslEncryptionHandler.java ---
@@ -0,0 +1,181 @@
+/*
+ * 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.drill.exec.rpc;
+
+import io.netty.buffer.ByteBuf;
+import io.netty.buffer.CompositeByteBuf;
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.handler.codec.MessageToMessageEncoder;
+
+import org.apache.drill.exec.exception.OutOfMemoryException;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
+import java.util.List;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+
+/**
+ * Handler to wrap the input Composite ByteBuf components separately and 
append the encrypted length for each
+ * component in the output ByteBuf. If there are multiple components in 
the input ByteBuf then each component will be
+ * encrypted individually and added to output ByteBuf with it's length 
prepended.
+ * 
+ * Example:
+ * Input ByteBuf  --> [B1,B2] - 2 component ByteBuf of 16K byte each.
+ * Output ByteBuf --> [[EBLN1, EB1], [EBLN2, EB2]] - List of ByteBuf's 
with each ByteBuf containing
+ *Encrypted Byte Length (EBLNx) in network order as 
per SASL RFC and Encrypted Bytes (EBx).
+ * 
+ */
+class SaslEncryptionHandler extends MessageToMessageEncoder {
+
+  private static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(
+  SaslEncryptionHandler.class.getCanonicalName());
+
+  private final SaslCodec saslCodec;
+
+  private final int maxRawWrapSize;
+
+  private byte[] origMsgBuffer;
+
+  private final ByteBuffer lengthOctets;
+
+  private final OutOfMemoryHandler outOfMemoryHandler;
+
+  /**
+   * We don't provide preference to allocator to use heap buffer instead 
of direct buffer.
+   * Drill uses it's own buffer allocator which doesn't support heap 
buffer allocation. We use
+   * Drill buffer allocator in the channel.
+   */
+  SaslEncryptionHandler(SaslCodec saslCodec, final int maxRawWrapSize, 
final OutOfMemoryHandler oomHandler) {
+this.saslCodec = saslCodec;
+this.maxRawWrapSize = maxRawWrapSize;
+this.outOfMemoryHandler = oomHandler;
+
+// The maximum size of the component will be maxRawWrapSize. Since 
this is maximum size we can allocate once
+// and reuse it for each component encode.
+origMsgBuffer = new byte[this.maxRawWrapSize];
+lengthOctets = ByteBuffer.allocate(RpcConstants.LENGTH_FIELD_LENGTH);
+lengthOctets.order(ByteOrder.BIG_ENDIAN);
+  }
+
+  @Override
+  public void handlerAdded(ChannelHandlerContext ctx) throws Exception {
+super.handlerAdded(ctx);
+logger.trace("Added " + RpcConstants.SASL_ENCRYPTION_HANDLER + " 
handler!");
+  }
+
+  @Override
+  public void handlerRemoved(ChannelHandlerContext ctx) throws Exception {
+super.handlerRemoved(ctx);
+logger.trace("Removed " + RpcConstants.SASL_ENCRYPTION_HANDLER + " 
handler");
+  }
+
+  public void encode(ChannelHandlerContext ctx, ByteBuf msg, List 
out) throws IOException {
+
+if (!ctx.channel().isOpen()) {
+  logger.debug("In " + RpcConstants.SASL_ENCRYPTION_HANDLER + " and 
channel is not open. " +
+  "So releasing msg memory before encryption.");
+  msg.release();
+  return;
+}
+
+try {
+  // If encryption is enabled then this handler will always get 
ByteBuf of type Composite ByteBuf
+  checkArgument(msg instanceof 

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15984053#comment-15984053
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r112788779
  
--- Diff: 
exec/rpc/src/main/java/org/apache/drill/exec/rpc/RemoteConnection.java ---
@@ -51,6 +51,12 @@
 
   SocketAddress getRemoteAddress();
 
+  void addSecurityHandlers();
+
+  void incConnectionCounter();
--- End diff --

Remove this increment and decrement counters from the interface; these 
methods are invoked on implementations. 

Maybe empty protected methods in `AbstractRemoteConnection` with impls in 
concrete classes?


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15984076#comment-15984076
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r113347217
  
--- Diff: 
exec/rpc/src/main/java/org/apache/drill/exec/rpc/SaslEncryptionHandler.java ---
@@ -0,0 +1,181 @@
+/*
+ * 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.drill.exec.rpc;
+
+import io.netty.buffer.ByteBuf;
+import io.netty.buffer.CompositeByteBuf;
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.handler.codec.MessageToMessageEncoder;
+
+import org.apache.drill.exec.exception.OutOfMemoryException;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
+import java.util.List;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+
+/**
+ * Handler to wrap the input Composite ByteBuf components separately and 
append the encrypted length for each
+ * component in the output ByteBuf. If there are multiple components in 
the input ByteBuf then each component will be
+ * encrypted individually and added to output ByteBuf with it's length 
prepended.
+ * 
+ * Example:
+ * Input ByteBuf  --> [B1,B2] - 2 component ByteBuf of 16K byte each.
+ * Output ByteBuf --> [[EBLN1, EB1], [EBLN2, EB2]] - List of ByteBuf's 
with each ByteBuf containing
+ *Encrypted Byte Length (EBLNx) in network order as 
per SASL RFC and Encrypted Bytes (EBx).
+ * 
+ */
+class SaslEncryptionHandler extends MessageToMessageEncoder {
+
+  private static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(
+  SaslEncryptionHandler.class.getCanonicalName());
+
+  private final SaslCodec saslCodec;
+
+  private final int maxRawWrapSize;
+
+  private byte[] origMsgBuffer;
+
+  private final ByteBuffer lengthOctets;
+
+  private final OutOfMemoryHandler outOfMemoryHandler;
+
+  /**
+   * We don't provide preference to allocator to use heap buffer instead 
of direct buffer.
+   * Drill uses it's own buffer allocator which doesn't support heap 
buffer allocation. We use
+   * Drill buffer allocator in the channel.
+   */
+  SaslEncryptionHandler(SaslCodec saslCodec, final int maxRawWrapSize, 
final OutOfMemoryHandler oomHandler) {
+this.saslCodec = saslCodec;
+this.maxRawWrapSize = maxRawWrapSize;
+this.outOfMemoryHandler = oomHandler;
+
+// The maximum size of the component will be maxRawWrapSize. Since 
this is maximum size we can allocate once
+// and reuse it for each component encode.
+origMsgBuffer = new byte[this.maxRawWrapSize];
+lengthOctets = ByteBuffer.allocate(RpcConstants.LENGTH_FIELD_LENGTH);
+lengthOctets.order(ByteOrder.BIG_ENDIAN);
+  }
+
+  @Override
+  public void handlerAdded(ChannelHandlerContext ctx) throws Exception {
+super.handlerAdded(ctx);
+logger.trace("Added " + RpcConstants.SASL_ENCRYPTION_HANDLER + " 
handler!");
+  }
+
+  @Override
+  public void handlerRemoved(ChannelHandlerContext ctx) throws Exception {
+super.handlerRemoved(ctx);
+logger.trace("Removed " + RpcConstants.SASL_ENCRYPTION_HANDLER + " 
handler");
+  }
+
+  public void encode(ChannelHandlerContext ctx, ByteBuf msg, List 
out) throws IOException {
+
+if (!ctx.channel().isOpen()) {
+  logger.debug("In " + RpcConstants.SASL_ENCRYPTION_HANDLER + " and 
channel is not open. " +
+  "So releasing msg memory before encryption.");
+  msg.release();
+  return;
+}
+
+try {
+  // If encryption is enabled then this handler will always get 
ByteBuf of type Composite ByteBuf
+  checkArgument(msg instanceof 

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15984065#comment-15984065
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r113351684
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/security/AuthenticationOutcomeListener.java
 ---
@@ -243,4 +249,46 @@ public SaslMessage process(SaslChallengeContext 
context) throws Exception {
   }
 }
   }
+
+  private static void handleSuccess(SaslChallengeContext context) throws 
SaslException {
+final ClientConnection connection = context.connection;
+final SaslClient saslClient = connection.getSaslClient();
+
+try {
+  // Check if connection was marked for being secure then verify for 
negotiated QOP value for
+  // correctness.
+  final String negotiatedQOP = 
saslClient.getNegotiatedProperty(Sasl.QOP).toString();
+  final String expectedQOP = connection.isEncryptionEnabled()
+  ? SaslProperties.QualityOfProtection.PRIVACY.getSaslQop()
+  : SaslProperties.QualityOfProtection.AUTHENTICATION.getSaslQop();
+
+  if (!(negotiatedQOP.equals(expectedQOP))) {
+throw new SaslException(String.format("Mismatch in negotiated QOP 
value: %s and Expected QOP value: %s",
+negotiatedQOP, expectedQOP));
+  }
+
+  // Update the rawWrapChunkSize with the negotiated buffer size since 
we cannot call encode with more than
+  // negotiated size of buffer.
+  if (connection.isEncryptionEnabled()) {
+final int negotiatedRawSendSize = Integer.parseInt(
+
saslClient.getNegotiatedProperty(Sasl.RAW_SEND_SIZE).toString());
+if (negotiatedRawSendSize <= 0) {
+  throw new SaslException(String.format("Negotiated rawSendSize: 
%d is invalid. Please check the configured " +
+  "value of sasl.encryption.encodesize. It might be configured 
to a very small value.",
+  negotiatedRawSendSize));
+}
+connection.setWrapSizeLimit(negotiatedRawSendSize);
--- End diff --

Please fix naming (or maybe I am easily confused).

wrapSizeLimit and negotiatedRawSendSize, contrast with maxRawWrapSize in 
SaslEncryptionHandler. All of which are actually `maxSendBufferSize` per 
documentation in Sasl.RAW_SEND_SIZE.


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15984070#comment-15984070
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r113340308
  
--- Diff: 
exec/rpc/src/main/java/org/apache/drill/exec/rpc/EncryptionContext.java ---
@@ -0,0 +1,37 @@
+/*
+ * 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.drill.exec.rpc;
+
+public interface EncryptionContext {
+
+  boolean isEncryptionEnabled();
+
+  void setEncryption(boolean encryptionEnabled);
+
+  void setMaxWrappedSize(int maxWrappedChunkSize);
+
+  int getMaxWrappedSize();
+
+  void setWrapSizeLimit(int wrapSizeLimit);
--- End diff --

+ How about `setMaxSendBufferSize(int size)` as documented in 
`Sasl.RAW_SEND_SIZE`?
+ Add doc pointing to `Sasl.RAW_SEND_SIZE`


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15984062#comment-15984062
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r113346921
  
--- Diff: 
exec/rpc/src/main/java/org/apache/drill/exec/rpc/SaslEncryptionHandler.java ---
@@ -0,0 +1,181 @@
+/*
+ * 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.drill.exec.rpc;
+
+import io.netty.buffer.ByteBuf;
+import io.netty.buffer.CompositeByteBuf;
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.handler.codec.MessageToMessageEncoder;
+
+import org.apache.drill.exec.exception.OutOfMemoryException;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
+import java.util.List;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+
+/**
+ * Handler to wrap the input Composite ByteBuf components separately and 
append the encrypted length for each
+ * component in the output ByteBuf. If there are multiple components in 
the input ByteBuf then each component will be
+ * encrypted individually and added to output ByteBuf with it's length 
prepended.
+ * 
+ * Example:
+ * Input ByteBuf  --> [B1,B2] - 2 component ByteBuf of 16K byte each.
+ * Output ByteBuf --> [[EBLN1, EB1], [EBLN2, EB2]] - List of ByteBuf's 
with each ByteBuf containing
+ *Encrypted Byte Length (EBLNx) in network order as 
per SASL RFC and Encrypted Bytes (EBx).
+ * 
+ */
+class SaslEncryptionHandler extends MessageToMessageEncoder {
+
+  private static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(
+  SaslEncryptionHandler.class.getCanonicalName());
+
+  private final SaslCodec saslCodec;
+
+  private final int maxRawWrapSize;
+
+  private byte[] origMsgBuffer;
+
+  private final ByteBuffer lengthOctets;
+
+  private final OutOfMemoryHandler outOfMemoryHandler;
+
+  /**
+   * We don't provide preference to allocator to use heap buffer instead 
of direct buffer.
+   * Drill uses it's own buffer allocator which doesn't support heap 
buffer allocation. We use
+   * Drill buffer allocator in the channel.
+   */
+  SaslEncryptionHandler(SaslCodec saslCodec, final int maxRawWrapSize, 
final OutOfMemoryHandler oomHandler) {
+this.saslCodec = saslCodec;
+this.maxRawWrapSize = maxRawWrapSize;
+this.outOfMemoryHandler = oomHandler;
+
+// The maximum size of the component will be maxRawWrapSize. Since 
this is maximum size we can allocate once
+// and reuse it for each component encode.
+origMsgBuffer = new byte[this.maxRawWrapSize];
--- End diff --

As mentioned above, this increases memory consumption of idle connections. 
Any measure perf benefits?


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15984048#comment-15984048
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r113245537
  
--- Diff: 
exec/rpc/src/main/java/org/apache/drill/exec/rpc/EncryptionContext.java ---
@@ -0,0 +1,37 @@
+/*
+ * 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.drill.exec.rpc;
+
+public interface EncryptionContext {
--- End diff --

Maybe rename to `EncryptionOptions` or `EncryptionSettings`? This also fits 
well with two implementations based on lifecycle: configured encryption options 
(immutable, defined in config classes) and connection encryption options 
(negotiated).

And instead of connection interface implementing this interface, maybe have 
a `getEncryptionOptions` in the `RemoteConnection` interface. So the usage 
would be: `connection.getEncryptionOptions().isEnabled()` or 
`connection.getEncryptionOptions().getMaxWrappedSize()`.

This is a variant of what you had previously. Sorry for the back and forth 
on this.


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15984077#comment-15984077
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r113353494
  
--- Diff: 
exec/rpc/src/main/java/org/apache/drill/exec/rpc/SaslDecryptionHandler.java ---
@@ -0,0 +1,162 @@
+/*
+ * 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.drill.exec.rpc;
+
+import io.netty.buffer.ByteBuf;
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.handler.codec.MessageToMessageDecoder;
+
+import org.apache.drill.exec.exception.OutOfMemoryException;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
+import java.util.List;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+/**
+ * Handler to Decrypt the input ByteBuf. It expects input to be in format 
where it has length of the bytes to
+ * decode in network order and actual encrypted bytes. The handler reads 
the length and then reads the
+ * required bytes to pass it to unwrap function for decryption. The 
decrypted buffer is copied to a new
+ * ByteBuf and added to out list.
+ * 
+ * Example:
+ * Input - [EBLN1, EB1, EBLN2, EB2] --> ByteBuf with repeated 
combination of encrypted byte length
+ * in network order (EBLNx) and encrypted bytes (EB)
+ * Output - [DB1] --> Decrypted ByteBuf of first chunk.(EB1)
+ * 
+ */
+class SaslDecryptionHandler extends MessageToMessageDecoder {
+
+  private static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(
+  SaslDecryptionHandler.class.getCanonicalName());
+
+  private final SaslCodec saslCodec;
+
+  private final int maxEncodedSize;
+
+  private final OutOfMemoryHandler outOfMemoryHandler;
+
+  private final byte[] encodedMsg;
+
+  private final ByteBuffer lengthOctets;
+
+  SaslDecryptionHandler(SaslCodec saslCodec, int maxEncodedSize, 
OutOfMemoryHandler oomHandler) {
+this.saslCodec = saslCodec;
+this.outOfMemoryHandler = oomHandler;
+this.maxEncodedSize = maxEncodedSize;
+
+// Allocate the byte array of maxEncodedSize to reuse for each encoded 
packet received on this connection
+// Maximum value of maxEncodedSize can be 16MB (i.e. OXFF)
+encodedMsg = new byte[maxEncodedSize];
+lengthOctets = ByteBuffer.allocate(RpcConstants.LENGTH_FIELD_LENGTH);
+lengthOctets.order(ByteOrder.BIG_ENDIAN);
+  }
+
+  @Override
+  public void handlerAdded(ChannelHandlerContext ctx) throws Exception {
+super.handlerAdded(ctx);
+logger.trace("Added " + RpcConstants.SASL_DECRYPTION_HANDLER + " 
handler");
+  }
+
+  @Override
+  public void handlerRemoved(ChannelHandlerContext ctx) throws Exception {
+super.handlerRemoved(ctx);
+logger.trace("Removed " + RpcConstants.SASL_DECRYPTION_HANDLER + " 
handler");
+  }
+
+  public void decode(ChannelHandlerContext ctx, ByteBuf msg, List 
out) throws IOException {
+
+if (!ctx.channel().isOpen()) {
+  logger.trace("Channel closed before decoding the message of {} 
bytes", msg.readableBytes());
+  msg.skipBytes(msg.readableBytes());
+  return;
+}
+
+try {
+  if(logger.isTraceEnabled()) {
+logger.trace("Trying to decrypt the encrypted message of size: {} 
with maxEncodedSize", msg.readableBytes());
+  }
+
+  final byte[] wrappedMsg;
+
+  // All the encrypted blocks are prefixed with it's length in network 
byte order (or BigEndian format). Netty's
+  // default Byte order of ByteBuf is Little Endian, so we cannot just 
do msg.getInt() as that will read the 4
+  // octets in little endian format.
+  //
+   

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15984056#comment-15984056
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r113323104
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/control/ControllerImpl.java
 ---
@@ -24,13 +24,15 @@
 import org.apache.drill.exec.exception.DrillbitStartupException;
 import org.apache.drill.exec.memory.BufferAllocator;
 import org.apache.drill.exec.proto.CoordinationProtos.DrillbitEndpoint;
+import org.apache.drill.exec.rpc.RpcMetrics;
 import org.apache.drill.exec.server.BootStrapContext;
 import org.apache.drill.exec.work.batch.ControlMessageHandler;
 
 import com.google.common.collect.Lists;
 import com.google.protobuf.Message;
 import com.google.protobuf.MessageLite;
 import com.google.protobuf.Parser;
+import org.apache.hadoop.io.ReadaheadPool;
--- End diff --

??


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15984051#comment-15984051
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r113329031
  
--- Diff: 
exec/rpc/src/main/java/org/apache/drill/exec/rpc/EncryptionContext.java ---
@@ -0,0 +1,37 @@
+/*
+ * 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.drill.exec.rpc;
+
+public interface EncryptionContext {
+
+  boolean isEncryptionEnabled();
+
+  void setEncryption(boolean encryptionEnabled);
+
+  void setMaxWrappedSize(int maxWrappedChunkSize);
+
+  int getMaxWrappedSize();
+
+  void setWrapSizeLimit(int wrapSizeLimit);
+
+  int getWrapSizeLimit();
+
+  String getEncryptionCtxtString();
--- End diff --

Unnecessary since implementation can override `toString`?


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15984068#comment-15984068
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r113348380
  
--- Diff: 
exec/rpc/src/main/java/org/apache/drill/exec/rpc/SaslEncryptionHandler.java ---
@@ -0,0 +1,181 @@
+/*
+ * 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.drill.exec.rpc;
+
+import io.netty.buffer.ByteBuf;
+import io.netty.buffer.CompositeByteBuf;
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.handler.codec.MessageToMessageEncoder;
+
+import org.apache.drill.exec.exception.OutOfMemoryException;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
+import java.util.List;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+
+/**
+ * Handler to wrap the input Composite ByteBuf components separately and 
append the encrypted length for each
+ * component in the output ByteBuf. If there are multiple components in 
the input ByteBuf then each component will be
+ * encrypted individually and added to output ByteBuf with it's length 
prepended.
+ * 
+ * Example:
+ * Input ByteBuf  --> [B1,B2] - 2 component ByteBuf of 16K byte each.
+ * Output ByteBuf --> [[EBLN1, EB1], [EBLN2, EB2]] - List of ByteBuf's 
with each ByteBuf containing
+ *Encrypted Byte Length (EBLNx) in network order as 
per SASL RFC and Encrypted Bytes (EBx).
+ * 
+ */
+class SaslEncryptionHandler extends MessageToMessageEncoder {
+
+  private static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(
+  SaslEncryptionHandler.class.getCanonicalName());
+
+  private final SaslCodec saslCodec;
+
+  private final int maxRawWrapSize;
+
+  private byte[] origMsgBuffer;
+
+  private final ByteBuffer lengthOctets;
+
+  private final OutOfMemoryHandler outOfMemoryHandler;
+
+  /**
+   * We don't provide preference to allocator to use heap buffer instead 
of direct buffer.
+   * Drill uses it's own buffer allocator which doesn't support heap 
buffer allocation. We use
+   * Drill buffer allocator in the channel.
+   */
+  SaslEncryptionHandler(SaslCodec saslCodec, final int maxRawWrapSize, 
final OutOfMemoryHandler oomHandler) {
+this.saslCodec = saslCodec;
+this.maxRawWrapSize = maxRawWrapSize;
+this.outOfMemoryHandler = oomHandler;
+
+// The maximum size of the component will be maxRawWrapSize. Since 
this is maximum size we can allocate once
+// and reuse it for each component encode.
+origMsgBuffer = new byte[this.maxRawWrapSize];
+lengthOctets = ByteBuffer.allocate(RpcConstants.LENGTH_FIELD_LENGTH);
+lengthOctets.order(ByteOrder.BIG_ENDIAN);
+  }
+
+  @Override
+  public void handlerAdded(ChannelHandlerContext ctx) throws Exception {
+super.handlerAdded(ctx);
+logger.trace("Added " + RpcConstants.SASL_ENCRYPTION_HANDLER + " 
handler!");
+  }
+
+  @Override
+  public void handlerRemoved(ChannelHandlerContext ctx) throws Exception {
+super.handlerRemoved(ctx);
+logger.trace("Removed " + RpcConstants.SASL_ENCRYPTION_HANDLER + " 
handler");
+  }
+
+  public void encode(ChannelHandlerContext ctx, ByteBuf msg, List 
out) throws IOException {
+
+if (!ctx.channel().isOpen()) {
+  logger.debug("In " + RpcConstants.SASL_ENCRYPTION_HANDLER + " and 
channel is not open. " +
+  "So releasing msg memory before encryption.");
+  msg.release();
+  return;
+}
+
+try {
+  // If encryption is enabled then this handler will always get 
ByteBuf of type Composite ByteBuf
+  checkArgument(msg instanceof 

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15984064#comment-15984064
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r113346292
  
--- Diff: 
exec/rpc/src/main/java/org/apache/drill/exec/rpc/SaslDecryptionHandler.java ---
@@ -0,0 +1,162 @@
+/*
+ * 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.drill.exec.rpc;
+
+import io.netty.buffer.ByteBuf;
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.handler.codec.MessageToMessageDecoder;
+
+import org.apache.drill.exec.exception.OutOfMemoryException;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
+import java.util.List;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+/**
+ * Handler to Decrypt the input ByteBuf. It expects input to be in format 
where it has length of the bytes to
+ * decode in network order and actual encrypted bytes. The handler reads 
the length and then reads the
+ * required bytes to pass it to unwrap function for decryption. The 
decrypted buffer is copied to a new
+ * ByteBuf and added to out list.
+ * 
+ * Example:
+ * Input - [EBLN1, EB1, EBLN2, EB2] --> ByteBuf with repeated 
combination of encrypted byte length
+ * in network order (EBLNx) and encrypted bytes (EB)
+ * Output - [DB1] --> Decrypted ByteBuf of first chunk.(EB1)
+ * 
+ */
+class SaslDecryptionHandler extends MessageToMessageDecoder {
+
+  private static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(
+  SaslDecryptionHandler.class.getCanonicalName());
+
+  private final SaslCodec saslCodec;
+
+  private final int maxEncodedSize;
+
+  private final OutOfMemoryHandler outOfMemoryHandler;
+
+  private final byte[] encodedMsg;
+
+  private final ByteBuffer lengthOctets;
+
+  SaslDecryptionHandler(SaslCodec saslCodec, int maxEncodedSize, 
OutOfMemoryHandler oomHandler) {
+this.saslCodec = saslCodec;
+this.outOfMemoryHandler = oomHandler;
+this.maxEncodedSize = maxEncodedSize;
+
+// Allocate the byte array of maxEncodedSize to reuse for each encoded 
packet received on this connection
+// Maximum value of maxEncodedSize can be 16MB (i.e. OXFF)
+encodedMsg = new byte[maxEncodedSize];
+lengthOctets = ByteBuffer.allocate(RpcConstants.LENGTH_FIELD_LENGTH);
+lengthOctets.order(ByteOrder.BIG_ENDIAN);
+  }
+
+  @Override
+  public void handlerAdded(ChannelHandlerContext ctx) throws Exception {
+super.handlerAdded(ctx);
+logger.trace("Added " + RpcConstants.SASL_DECRYPTION_HANDLER + " 
handler");
+  }
+
+  @Override
+  public void handlerRemoved(ChannelHandlerContext ctx) throws Exception {
+super.handlerRemoved(ctx);
+logger.trace("Removed " + RpcConstants.SASL_DECRYPTION_HANDLER + " 
handler");
+  }
+
+  public void decode(ChannelHandlerContext ctx, ByteBuf msg, List 
out) throws IOException {
+
+if (!ctx.channel().isOpen()) {
+  logger.trace("Channel closed before decoding the message of {} 
bytes", msg.readableBytes());
+  msg.skipBytes(msg.readableBytes());
+  return;
+}
+
+try {
+  if(logger.isTraceEnabled()) {
+logger.trace("Trying to decrypt the encrypted message of size: {} 
with maxEncodedSize", msg.readableBytes());
+  }
+
+  final byte[] wrappedMsg;
+
+  // All the encrypted blocks are prefixed with it's length in network 
byte order (or BigEndian format). Netty's
+  // default Byte order of ByteBuf is Little Endian, so we cannot just 
do msg.getInt() as that will read the 4
+  // octets in little endian format.
+  //
+   

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15984050#comment-15984050
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r113260837
  
--- Diff: 
exec/rpc/src/main/java/org/apache/drill/exec/rpc/ChunkCreationHandler.java ---
@@ -0,0 +1,101 @@
+/*
+ * 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.drill.exec.rpc;
+
+
+import io.netty.buffer.ByteBuf;
+import io.netty.buffer.CompositeByteBuf;
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.handler.codec.MessageToMessageEncoder;
+
+import java.util.List;
+
+import static com.google.common.base.Preconditions.checkArgument;
+import static java.lang.Math.min;
+
+/**
+ * Handler that converts an input ByteBuf into chunk size ByteBuf's and 
add it to the
+ * CompositeByteBuf as individual components. This is done irrespective of 
chunk mode is
+ * enabled or not.
+ */
+class ChunkCreationHandler extends MessageToMessageEncoder {
+  private static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(
+  ChunkCreationHandler.class.getCanonicalName());
+
+  private final int chunkSize;
+
+  ChunkCreationHandler(int chunkSize) {
+checkArgument(chunkSize > 0);
+this.chunkSize = chunkSize;
+  }
+
+  @Override
+  public void handlerAdded(ChannelHandlerContext ctx) throws Exception {
+super.handlerAdded(ctx);
+logger.trace("Added " + RpcConstants.CHUNK_CREATION_HANDLER + " 
handler!");
+  }
+
+  @Override
+  public void handlerRemoved(ChannelHandlerContext ctx) throws Exception {
+super.handlerRemoved(ctx);
+logger.trace("Removed " + RpcConstants.CHUNK_CREATION_HANDLER + " 
handler");
+  }
+
+  @Override
+  protected void encode(ChannelHandlerContext ctx, ByteBuf msg, 
List out) throws Exception {
+
+if (RpcConstants.EXTRA_DEBUGGING) {
+  logger.debug("ChunkCreationHandler called with msg {} of size {} 
with chunkSize {}",
+  msg, msg.readableBytes(), chunkSize);
+}
+
+if (!ctx.channel().isOpen()) {
+  logger.debug("Channel closed, skipping encode inside {}.", 
RpcConstants.CHUNK_CREATION_HANDLER);
+  msg.release();
+  return;
+}
+
+ByteBuf chunkBuf;
+
+// Calculate the number of chunks based on configured chunk size and 
input msg size
+int numChunks = (int) Math.ceil((double) msg.readableBytes() / 
chunkSize);
+
+// Initialize a composite buffer to hold numChunks chunk.
+CompositeByteBuf cbb = new CompositeByteBuf(ctx.alloc(), true, 
numChunks);
--- End diff --

+ Use `ctx.alloc().compositeDirectBuffer(...)`?
+ final


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15984059#comment-15984059
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r113334641
  
--- Diff: 
exec/rpc/src/main/java/org/apache/drill/exec/rpc/SaslDecryptionHandler.java ---
@@ -0,0 +1,162 @@
+/*
+ * 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.drill.exec.rpc;
+
+import io.netty.buffer.ByteBuf;
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.handler.codec.MessageToMessageDecoder;
+
+import org.apache.drill.exec.exception.OutOfMemoryException;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
+import java.util.List;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+/**
+ * Handler to Decrypt the input ByteBuf. It expects input to be in format 
where it has length of the bytes to
+ * decode in network order and actual encrypted bytes. The handler reads 
the length and then reads the
+ * required bytes to pass it to unwrap function for decryption. The 
decrypted buffer is copied to a new
+ * ByteBuf and added to out list.
+ * 
+ * Example:
+ * Input - [EBLN1, EB1, EBLN2, EB2] --> ByteBuf with repeated 
combination of encrypted byte length
+ * in network order (EBLNx) and encrypted bytes (EB)
+ * Output - [DB1] --> Decrypted ByteBuf of first chunk.(EB1)
+ * 
+ */
+class SaslDecryptionHandler extends MessageToMessageDecoder {
+
+  private static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(
+  SaslDecryptionHandler.class.getCanonicalName());
+
+  private final SaslCodec saslCodec;
+
+  private final int maxEncodedSize;
+
+  private final OutOfMemoryHandler outOfMemoryHandler;
+
+  private final byte[] encodedMsg;
+
+  private final ByteBuffer lengthOctets;
+
+  SaslDecryptionHandler(SaslCodec saslCodec, int maxEncodedSize, 
OutOfMemoryHandler oomHandler) {
+this.saslCodec = saslCodec;
+this.outOfMemoryHandler = oomHandler;
+this.maxEncodedSize = maxEncodedSize;
+
+// Allocate the byte array of maxEncodedSize to reuse for each encoded 
packet received on this connection
+// Maximum value of maxEncodedSize can be 16MB (i.e. OXFF)
+encodedMsg = new byte[maxEncodedSize];
+lengthOctets = ByteBuffer.allocate(RpcConstants.LENGTH_FIELD_LENGTH);
+lengthOctets.order(ByteOrder.BIG_ENDIAN);
+  }
+
+  @Override
+  public void handlerAdded(ChannelHandlerContext ctx) throws Exception {
+super.handlerAdded(ctx);
+logger.trace("Added " + RpcConstants.SASL_DECRYPTION_HANDLER + " 
handler");
+  }
+
+  @Override
+  public void handlerRemoved(ChannelHandlerContext ctx) throws Exception {
+super.handlerRemoved(ctx);
+logger.trace("Removed " + RpcConstants.SASL_DECRYPTION_HANDLER + " 
handler");
+  }
+
+  public void decode(ChannelHandlerContext ctx, ByteBuf msg, List 
out) throws IOException {
+
+if (!ctx.channel().isOpen()) {
+  logger.trace("Channel closed before decoding the message of {} 
bytes", msg.readableBytes());
+  msg.skipBytes(msg.readableBytes());
+  return;
+}
+
+try {
+  if(logger.isTraceEnabled()) {
+logger.trace("Trying to decrypt the encrypted message of size: {} 
with maxEncodedSize", msg.readableBytes());
+  }
+
+  final byte[] wrappedMsg;
+
+  // All the encrypted blocks are prefixed with it's length in network 
byte order (or BigEndian format). Netty's
+  // default Byte order of ByteBuf is Little Endian, so we cannot just 
do msg.getInt() as that will read the 4
+  // octets in little endian format.
+  //
+   

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15984058#comment-15984058
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r113337341
  
--- Diff: 
exec/rpc/src/main/java/org/apache/drill/exec/rpc/SaslDecryptionHandler.java ---
@@ -0,0 +1,162 @@
+/*
+ * 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.drill.exec.rpc;
+
+import io.netty.buffer.ByteBuf;
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.handler.codec.MessageToMessageDecoder;
+
+import org.apache.drill.exec.exception.OutOfMemoryException;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
+import java.util.List;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+/**
+ * Handler to Decrypt the input ByteBuf. It expects input to be in format 
where it has length of the bytes to
+ * decode in network order and actual encrypted bytes. The handler reads 
the length and then reads the
+ * required bytes to pass it to unwrap function for decryption. The 
decrypted buffer is copied to a new
+ * ByteBuf and added to out list.
+ * 
+ * Example:
+ * Input - [EBLN1, EB1, EBLN2, EB2] --> ByteBuf with repeated 
combination of encrypted byte length
+ * in network order (EBLNx) and encrypted bytes (EB)
+ * Output - [DB1] --> Decrypted ByteBuf of first chunk.(EB1)
+ * 
+ */
+class SaslDecryptionHandler extends MessageToMessageDecoder {
+
+  private static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(
+  SaslDecryptionHandler.class.getCanonicalName());
+
+  private final SaslCodec saslCodec;
+
+  private final int maxEncodedSize;
+
+  private final OutOfMemoryHandler outOfMemoryHandler;
+
+  private final byte[] encodedMsg;
+
+  private final ByteBuffer lengthOctets;
+
+  SaslDecryptionHandler(SaslCodec saslCodec, int maxEncodedSize, 
OutOfMemoryHandler oomHandler) {
+this.saslCodec = saslCodec;
+this.outOfMemoryHandler = oomHandler;
+this.maxEncodedSize = maxEncodedSize;
+
+// Allocate the byte array of maxEncodedSize to reuse for each encoded 
packet received on this connection
+// Maximum value of maxEncodedSize can be 16MB (i.e. OXFF)
+encodedMsg = new byte[maxEncodedSize];
+lengthOctets = ByteBuffer.allocate(RpcConstants.LENGTH_FIELD_LENGTH);
+lengthOctets.order(ByteOrder.BIG_ENDIAN);
+  }
+
+  @Override
+  public void handlerAdded(ChannelHandlerContext ctx) throws Exception {
+super.handlerAdded(ctx);
+logger.trace("Added " + RpcConstants.SASL_DECRYPTION_HANDLER + " 
handler");
+  }
+
+  @Override
+  public void handlerRemoved(ChannelHandlerContext ctx) throws Exception {
+super.handlerRemoved(ctx);
+logger.trace("Removed " + RpcConstants.SASL_DECRYPTION_HANDLER + " 
handler");
+  }
+
+  public void decode(ChannelHandlerContext ctx, ByteBuf msg, List 
out) throws IOException {
+
+if (!ctx.channel().isOpen()) {
+  logger.trace("Channel closed before decoding the message of {} 
bytes", msg.readableBytes());
+  msg.skipBytes(msg.readableBytes());
+  return;
+}
+
+try {
+  if(logger.isTraceEnabled()) {
+logger.trace("Trying to decrypt the encrypted message of size: {} 
with maxEncodedSize", msg.readableBytes());
+  }
+
+  final byte[] wrappedMsg;
+
+  // All the encrypted blocks are prefixed with it's length in network 
byte order (or BigEndian format). Netty's
+  // default Byte order of ByteBuf is Little Endian, so we cannot just 
do msg.getInt() as that will read the 4
+  // octets in little endian format.
+  //
+   

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15984069#comment-15984069
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r113334954
  
--- Diff: 
exec/rpc/src/main/java/org/apache/drill/exec/rpc/SaslDecryptionHandler.java ---
@@ -0,0 +1,162 @@
+/*
+ * 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.drill.exec.rpc;
+
+import io.netty.buffer.ByteBuf;
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.handler.codec.MessageToMessageDecoder;
+
+import org.apache.drill.exec.exception.OutOfMemoryException;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
+import java.util.List;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+/**
+ * Handler to Decrypt the input ByteBuf. It expects input to be in format 
where it has length of the bytes to
+ * decode in network order and actual encrypted bytes. The handler reads 
the length and then reads the
+ * required bytes to pass it to unwrap function for decryption. The 
decrypted buffer is copied to a new
+ * ByteBuf and added to out list.
+ * 
+ * Example:
+ * Input - [EBLN1, EB1, EBLN2, EB2] --> ByteBuf with repeated 
combination of encrypted byte length
+ * in network order (EBLNx) and encrypted bytes (EB)
+ * Output - [DB1] --> Decrypted ByteBuf of first chunk.(EB1)
+ * 
+ */
+class SaslDecryptionHandler extends MessageToMessageDecoder {
+
+  private static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(
+  SaslDecryptionHandler.class.getCanonicalName());
+
+  private final SaslCodec saslCodec;
+
+  private final int maxEncodedSize;
+
+  private final OutOfMemoryHandler outOfMemoryHandler;
+
+  private final byte[] encodedMsg;
+
+  private final ByteBuffer lengthOctets;
+
+  SaslDecryptionHandler(SaslCodec saslCodec, int maxEncodedSize, 
OutOfMemoryHandler oomHandler) {
+this.saslCodec = saslCodec;
+this.outOfMemoryHandler = oomHandler;
+this.maxEncodedSize = maxEncodedSize;
+
+// Allocate the byte array of maxEncodedSize to reuse for each encoded 
packet received on this connection
+// Maximum value of maxEncodedSize can be 16MB (i.e. OXFF)
+encodedMsg = new byte[maxEncodedSize];
--- End diff --

Given this, every connection (idle or not) consumes at least 16 MB. Are 
there any measured perf benefits? If not, please remove this.


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15984060#comment-15984060
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r113346069
  
--- Diff: 
exec/rpc/src/main/java/org/apache/drill/exec/rpc/SaslDecryptionHandler.java ---
@@ -0,0 +1,162 @@
+/*
+ * 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.drill.exec.rpc;
+
+import io.netty.buffer.ByteBuf;
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.handler.codec.MessageToMessageDecoder;
+
+import org.apache.drill.exec.exception.OutOfMemoryException;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
+import java.util.List;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+/**
+ * Handler to Decrypt the input ByteBuf. It expects input to be in format 
where it has length of the bytes to
+ * decode in network order and actual encrypted bytes. The handler reads 
the length and then reads the
+ * required bytes to pass it to unwrap function for decryption. The 
decrypted buffer is copied to a new
+ * ByteBuf and added to out list.
+ * 
+ * Example:
+ * Input - [EBLN1, EB1, EBLN2, EB2] --> ByteBuf with repeated 
combination of encrypted byte length
+ * in network order (EBLNx) and encrypted bytes (EB)
+ * Output - [DB1] --> Decrypted ByteBuf of first chunk.(EB1)
+ * 
+ */
+class SaslDecryptionHandler extends MessageToMessageDecoder {
+
+  private static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(
+  SaslDecryptionHandler.class.getCanonicalName());
+
+  private final SaslCodec saslCodec;
+
+  private final int maxEncodedSize;
+
+  private final OutOfMemoryHandler outOfMemoryHandler;
+
+  private final byte[] encodedMsg;
+
+  private final ByteBuffer lengthOctets;
+
+  SaslDecryptionHandler(SaslCodec saslCodec, int maxEncodedSize, 
OutOfMemoryHandler oomHandler) {
+this.saslCodec = saslCodec;
+this.outOfMemoryHandler = oomHandler;
+this.maxEncodedSize = maxEncodedSize;
+
+// Allocate the byte array of maxEncodedSize to reuse for each encoded 
packet received on this connection
+// Maximum value of maxEncodedSize can be 16MB (i.e. OXFF)
+encodedMsg = new byte[maxEncodedSize];
+lengthOctets = ByteBuffer.allocate(RpcConstants.LENGTH_FIELD_LENGTH);
+lengthOctets.order(ByteOrder.BIG_ENDIAN);
+  }
+
+  @Override
+  public void handlerAdded(ChannelHandlerContext ctx) throws Exception {
+super.handlerAdded(ctx);
+logger.trace("Added " + RpcConstants.SASL_DECRYPTION_HANDLER + " 
handler");
+  }
+
+  @Override
+  public void handlerRemoved(ChannelHandlerContext ctx) throws Exception {
+super.handlerRemoved(ctx);
+logger.trace("Removed " + RpcConstants.SASL_DECRYPTION_HANDLER + " 
handler");
+  }
+
+  public void decode(ChannelHandlerContext ctx, ByteBuf msg, List 
out) throws IOException {
+
+if (!ctx.channel().isOpen()) {
+  logger.trace("Channel closed before decoding the message of {} 
bytes", msg.readableBytes());
+  msg.skipBytes(msg.readableBytes());
+  return;
+}
+
+try {
+  if(logger.isTraceEnabled()) {
+logger.trace("Trying to decrypt the encrypted message of size: {} 
with maxEncodedSize", msg.readableBytes());
+  }
+
+  final byte[] wrappedMsg;
+
+  // All the encrypted blocks are prefixed with it's length in network 
byte order (or BigEndian format). Netty's
+  // default Byte order of ByteBuf is Little Endian, so we cannot just 
do msg.getInt() as that will read the 4
+  // octets in little endian format.
+  //
+   

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15984071#comment-15984071
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r113353207
  
--- Diff: 
exec/rpc/src/main/java/org/apache/drill/exec/rpc/RpcConstants.java ---
@@ -24,4 +24,29 @@ private RpcConstants(){}
 
   public static final boolean SOME_DEBUGGING = false;
   public static final boolean EXTRA_DEBUGGING = false;
+
+  // RPC Handler names
+  public static final String TIMEOUT_HANDLER = "timeout-handler";
+  public static final String PROTOCOL_DECODER = "protocol-decoder";
+  public static final String PROTOCOL_ENCODER = "protocol-encoder";
+  public static final String MESSAGE_DECODER = "message-decoder";
+  public static final String HANDSHAKE_HANDLER = "handshake-handler";
+  public static final String MESSAGE_HANDLER = "message-handler";
+  public static final String EXCEPTION_HANDLER = "exception-handler";
+  public static final String IDLE_STATE_HANDLER = "idle-state-handler";
+  public static final String SASL_DECRYPTION_HANDLER = 
"sasldecryption-handler";
+  public static final String SASL_ENCRYPTION_HANDLER = 
"saslencryption-handler";
+  public static final String LENGTH_DECODER_HANDLER = "length-decoder";
+  public static final String CHUNK_CREATION_HANDLER = 
"chunkcreation-handler";
+
+
+
+  // SASL RFC /4422 allows only 3 octets to specify the length of 
maximum encoded buffer each side can receive.
+  // Hence the maximum buffer size is capped at 16Mb i.e. 0XFF bytes.
+  public static final int MAX_WRAPPED_SIZE = 0XFF;
+
+  public static final int LENGTH_FIELD_OFFSET = 0;
+  public static final int LENGTH_FIELD_LENGTH = 4;
--- End diff --

Better name for `LENGTH_FIELD_LENGTH`?


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15984063#comment-15984063
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r113340307
  
--- Diff: 
exec/rpc/src/main/java/org/apache/drill/exec/rpc/EncryptionContext.java ---
@@ -0,0 +1,37 @@
+/*
+ * 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.drill.exec.rpc;
+
+public interface EncryptionContext {
+
+  boolean isEncryptionEnabled();
+
+  void setEncryption(boolean encryptionEnabled);
+
+  void setMaxWrappedSize(int maxWrappedChunkSize);
--- End diff --

+ How about `setMaxReceiveBufferSize(int size)` as documented in 
`Sasl.MAX_BUFFER`?
+ Add doc pointing to `Sasl.MAX_BUFFER`


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15984073#comment-15984073
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r113348909
  
--- Diff: 
exec/rpc/src/main/java/org/apache/drill/exec/rpc/SaslEncryptionHandler.java ---
@@ -0,0 +1,181 @@
+/*
+ * 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.drill.exec.rpc;
+
+import io.netty.buffer.ByteBuf;
+import io.netty.buffer.CompositeByteBuf;
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.handler.codec.MessageToMessageEncoder;
+
+import org.apache.drill.exec.exception.OutOfMemoryException;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
+import java.util.List;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+
+/**
+ * Handler to wrap the input Composite ByteBuf components separately and 
append the encrypted length for each
+ * component in the output ByteBuf. If there are multiple components in 
the input ByteBuf then each component will be
+ * encrypted individually and added to output ByteBuf with it's length 
prepended.
+ * 
+ * Example:
+ * Input ByteBuf  --> [B1,B2] - 2 component ByteBuf of 16K byte each.
+ * Output ByteBuf --> [[EBLN1, EB1], [EBLN2, EB2]] - List of ByteBuf's 
with each ByteBuf containing
+ *Encrypted Byte Length (EBLNx) in network order as 
per SASL RFC and Encrypted Bytes (EBx).
+ * 
+ */
+class SaslEncryptionHandler extends MessageToMessageEncoder {
+
+  private static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(
+  SaslEncryptionHandler.class.getCanonicalName());
+
+  private final SaslCodec saslCodec;
+
+  private final int maxRawWrapSize;
+
+  private byte[] origMsgBuffer;
+
+  private final ByteBuffer lengthOctets;
+
+  private final OutOfMemoryHandler outOfMemoryHandler;
+
+  /**
+   * We don't provide preference to allocator to use heap buffer instead 
of direct buffer.
+   * Drill uses it's own buffer allocator which doesn't support heap 
buffer allocation. We use
+   * Drill buffer allocator in the channel.
+   */
+  SaslEncryptionHandler(SaslCodec saslCodec, final int maxRawWrapSize, 
final OutOfMemoryHandler oomHandler) {
+this.saslCodec = saslCodec;
+this.maxRawWrapSize = maxRawWrapSize;
+this.outOfMemoryHandler = oomHandler;
+
+// The maximum size of the component will be maxRawWrapSize. Since 
this is maximum size we can allocate once
+// and reuse it for each component encode.
+origMsgBuffer = new byte[this.maxRawWrapSize];
+lengthOctets = ByteBuffer.allocate(RpcConstants.LENGTH_FIELD_LENGTH);
+lengthOctets.order(ByteOrder.BIG_ENDIAN);
+  }
+
+  @Override
+  public void handlerAdded(ChannelHandlerContext ctx) throws Exception {
+super.handlerAdded(ctx);
+logger.trace("Added " + RpcConstants.SASL_ENCRYPTION_HANDLER + " 
handler!");
+  }
+
+  @Override
+  public void handlerRemoved(ChannelHandlerContext ctx) throws Exception {
+super.handlerRemoved(ctx);
+logger.trace("Removed " + RpcConstants.SASL_ENCRYPTION_HANDLER + " 
handler");
+  }
+
+  public void encode(ChannelHandlerContext ctx, ByteBuf msg, List 
out) throws IOException {
+
+if (!ctx.channel().isOpen()) {
+  logger.debug("In " + RpcConstants.SASL_ENCRYPTION_HANDLER + " and 
channel is not open. " +
+  "So releasing msg memory before encryption.");
+  msg.release();
+  return;
+}
+
+try {
+  // If encryption is enabled then this handler will always get 
ByteBuf of type Composite ByteBuf
+  checkArgument(msg instanceof 

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15984049#comment-15984049
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r113331955
  
--- Diff: exec/rpc/src/main/java/org/apache/drill/exec/rpc/RpcMetrics.java 
---
@@ -0,0 +1,33 @@
+/*
+ * 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.drill.exec.rpc;
+
+import org.apache.drill.exec.memory.BufferAllocator;
+
+/**
+ * Holder interface for all the metrics used in RPC layer
+ */
+public interface RpcMetrics {
--- End diff --

This interface, its hierarchy, and all the plumbing in the server and 
client classes seem unnecessary. Drill only needs one MetricsRegistry (held in 
DrillMetrics), and metrics can be directly modified through that class.

Maybe all rpc metrics can be in one utility class in the java-exec...rpc 
package?


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15984057#comment-15984057
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r113336202
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/security/AuthenticationOutcomeListener.java
 ---
@@ -143,16 +149,16 @@ public void interrupted(InterruptedException e) {
 completionListener.interrupted(e);
   }
 
-  private static class SaslChallengeContext {
+  private static class SaslChallengeContext {
--- End diff --

Fix raw usage warnings due to this change


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15984074#comment-15984074
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r113347315
  
--- Diff: 
exec/rpc/src/main/java/org/apache/drill/exec/rpc/SaslEncryptionHandler.java ---
@@ -0,0 +1,181 @@
+/*
+ * 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.drill.exec.rpc;
+
+import io.netty.buffer.ByteBuf;
+import io.netty.buffer.CompositeByteBuf;
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.handler.codec.MessageToMessageEncoder;
+
+import org.apache.drill.exec.exception.OutOfMemoryException;
+
+import java.io.IOException;
+import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
+import java.util.List;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+
+/**
+ * Handler to wrap the input Composite ByteBuf components separately and 
append the encrypted length for each
+ * component in the output ByteBuf. If there are multiple components in 
the input ByteBuf then each component will be
+ * encrypted individually and added to output ByteBuf with it's length 
prepended.
+ * 
+ * Example:
+ * Input ByteBuf  --> [B1,B2] - 2 component ByteBuf of 16K byte each.
+ * Output ByteBuf --> [[EBLN1, EB1], [EBLN2, EB2]] - List of ByteBuf's 
with each ByteBuf containing
+ *Encrypted Byte Length (EBLNx) in network order as 
per SASL RFC and Encrypted Bytes (EBx).
+ * 
+ */
+class SaslEncryptionHandler extends MessageToMessageEncoder {
+
+  private static final org.slf4j.Logger logger = 
org.slf4j.LoggerFactory.getLogger(
+  SaslEncryptionHandler.class.getCanonicalName());
+
+  private final SaslCodec saslCodec;
+
+  private final int maxRawWrapSize;
+
+  private byte[] origMsgBuffer;
+
+  private final ByteBuffer lengthOctets;
+
+  private final OutOfMemoryHandler outOfMemoryHandler;
+
+  /**
+   * We don't provide preference to allocator to use heap buffer instead 
of direct buffer.
+   * Drill uses it's own buffer allocator which doesn't support heap 
buffer allocation. We use
+   * Drill buffer allocator in the channel.
+   */
+  SaslEncryptionHandler(SaslCodec saslCodec, final int maxRawWrapSize, 
final OutOfMemoryHandler oomHandler) {
+this.saslCodec = saslCodec;
+this.maxRawWrapSize = maxRawWrapSize;
+this.outOfMemoryHandler = oomHandler;
+
+// The maximum size of the component will be maxRawWrapSize. Since 
this is maximum size we can allocate once
+// and reuse it for each component encode.
+origMsgBuffer = new byte[this.maxRawWrapSize];
+lengthOctets = ByteBuffer.allocate(RpcConstants.LENGTH_FIELD_LENGTH);
+lengthOctets.order(ByteOrder.BIG_ENDIAN);
+  }
+
+  @Override
+  public void handlerAdded(ChannelHandlerContext ctx) throws Exception {
+super.handlerAdded(ctx);
+logger.trace("Added " + RpcConstants.SASL_ENCRYPTION_HANDLER + " 
handler!");
+  }
+
+  @Override
+  public void handlerRemoved(ChannelHandlerContext ctx) throws Exception {
+super.handlerRemoved(ctx);
+logger.trace("Removed " + RpcConstants.SASL_ENCRYPTION_HANDLER + " 
handler");
+  }
+
+  public void encode(ChannelHandlerContext ctx, ByteBuf msg, List 
out) throws IOException {
+
+if (!ctx.channel().isOpen()) {
+  logger.debug("In " + RpcConstants.SASL_ENCRYPTION_HANDLER + " and 
channel is not open. " +
+  "So releasing msg memory before encryption.");
+  msg.release();
+  return;
+}
+
+try {
+  // If encryption is enabled then this handler will always get 
ByteBuf of type Composite ByteBuf
+  checkArgument(msg instanceof 

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15984055#comment-15984055
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sudheeshkatkam commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r113328102
  
--- Diff: 
exec/rpc/src/main/java/org/apache/drill/exec/rpc/ChunkCreationHandler.java ---
@@ -0,0 +1,101 @@
+/*
+ * 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.drill.exec.rpc;
+
+
+import io.netty.buffer.ByteBuf;
+import io.netty.buffer.CompositeByteBuf;
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.handler.codec.MessageToMessageEncoder;
+
+import java.util.List;
+
+import static com.google.common.base.Preconditions.checkArgument;
+import static java.lang.Math.min;
+
+/**
+ * Handler that converts an input ByteBuf into chunk size ByteBuf's and 
add it to the
+ * CompositeByteBuf as individual components. This is done irrespective of 
chunk mode is
--- End diff --

... **If encryption is enabled, this is added** irrespective of ...


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[Sorabh Hamirwasia (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15983672#comment-15983672
 ] 

Sorabh Hamirwasia commented on DRILL-4335:
--

Thanks!. I will add you and other reviewer's in the pull request for C++ 
changes.

> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[Laurent Goujon (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15983616#comment-15983616
 ] 

Laurent Goujon commented on DRILL-4335:
---

unfortunately, not extensively. I'll do it this week!

> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[Sorabh Hamirwasia (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15983598#comment-15983598
 ] 

Sorabh Hamirwasia commented on DRILL-4335:
--

[~laurentgo] - Hi Laurent, Did you get chance to look into the changes ? It 
would be great if you can help to review the pull request.

> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15970804#comment-15970804
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r111713955
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserConnectionConfig.java
 ---
@@ -34,32 +39,81 @@
 
   private final UserServerRequestHandler handler;
 
+  // Total number of external DrillClient connection's on this server.
+  private static final Counter secureUserConnections = 
DrillMetrics.getRegistry()
+.counter("drill.user.encrypted.connections");
+
+  private static final Counter insecureUserConnections = 
DrillMetrics.getRegistry()
+.counter("drill.user.unencrypted.connections");
+
   UserConnectionConfig(BufferAllocator allocator, BootStrapContext 
context, UserServerRequestHandler handler)
-  throws DrillbitStartupException {
+throws DrillbitStartupException {
 super(allocator, context);
 this.handler = handler;
 
-if 
(context.getConfig().getBoolean(ExecConstants.USER_AUTHENTICATION_ENABLED)) {
-  if (getAuthProvider().getAllFactoryNames().isEmpty()) {
+final DrillConfig config = context.getConfig();
+final AuthenticatorProvider authProvider = getAuthProvider();
+
+if (config.getBoolean(ExecConstants.USER_AUTHENTICATION_ENABLED)) {
+  if (authProvider.getAllFactoryNames().isEmpty()) {
 throw new DrillbitStartupException("Authentication enabled, but no 
mechanisms found. Please check " +
-"authentication configuration.");
+  "authentication configuration.");
   }
   authEnabled = true;
-  logger.info("Configured all user connections to require 
authentication using: {}",
-  getAuthProvider().getAllFactoryNames());
+
+  // Update encryption related parameters.
+  
encryptionContext.setEncryption(config.getBoolean(ExecConstants.USER_SASL_ENCRYPTION_ENABLED));
+
+  int maxEncodeSize = 
config.getInt(ExecConstants.USER_SASL_ENCRYPTION_ENCODESIZE);
+
+  if(maxEncodeSize > RpcConstants.MAX_WRAP_SIZE) {
+logger.warn("Setting user.sasl.encryption.encodesize to maximum 
allowed value of 16MB");
+maxEncodeSize = RpcConstants.MAX_WRAP_SIZE;
+  }
+  encryptionContext.setWrappedChunkSize(maxEncodeSize);
+
+  if (encryptionContext.isEncryptionEnabled() && 
authProvider.isOnlyPlainConfigured()) {
--- End diff --

Yes see other comments.


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15970801#comment-15970801
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r111676760
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserServer.java ---
@@ -335,8 +350,27 @@ public BitToUserHandshake 
getHandshakeResponse(UserToBitHandshake inbound) throw
 }
   }
 
-  // mention server's authentication capabilities
-  
respBuilder.addAllAuthenticationMechanisms(config.getAuthProvider().getAllFactoryNames());
+  // We are checking in UserConnectionConfig that if SASL 
encryption is enabled then mechanisms other
+  // than PLAIN are also configured otherwise throw exception
+  final Set configuredMech = 
config.getAuthProvider().getAllFactoryNames();
+
+  if (!config.isEncryptionEnabled()) {
+
+respBuilder.addAllAuthenticationMechanisms(configuredMech);
+  } else {
+final Set saslEncryptMech = new HashSet<>();
+
+for (String mechanism : configuredMech) {
+  if 
(!mechanism.equals(PlainFactory.SIMPLE_NAME.toLowerCase())) {
+saslEncryptMech.add(mechanism);
+  }
+}
+respBuilder.addAllAuthenticationMechanisms(saslEncryptMech);
+  }
+
+  // set the encrypted flag in handshake message. For older 
clients this field is optional so will be ignored
+  respBuilder.setEncrypted(connection.isEncrypted());
--- End diff --

if client is new which supports encryption and encryption is disabled on 
server side then server will set the `encrypted` flag to false so that client 
knows not to negotiate for encryption. Whereas wrapChunkSize parameter will be 
ignored.


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15970789#comment-15970789
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r111651753
  
--- Diff: 
exec/rpc/src/main/java/org/apache/drill/exec/rpc/SaslBackendWrapper.java ---
@@ -0,0 +1,33 @@
+/*
+ * 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.drill.exec.rpc;
+
+import javax.security.sasl.SaslException;
+
+/*
+ * The helper interface to wrap SaslClient and SaslServer instances for 
use in Security Handlers.
+ */
+public interface SaslBackendWrapper{
--- End diff --

Renamed to SaslCodecContext.


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15970785#comment-15970785
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r111646955
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/security/AuthenticationOutcomeListener.java
 ---
@@ -243,4 +247,43 @@ public SaslMessage process(SaslChallengeContext 
context) throws Exception {
   }
 }
   }
+
+  private static void handleSuccess(SaslChallengeContext context) throws 
SaslException {
+final ClientConnection connection = context.connection;
+final SaslClient saslClient = connection.getSaslClient();
+
+if (connection.isEncrypted()) {
+  try {
+// Check if connection was marked for being secure then verify for 
negotiated QOP value for
+// correctness.
+final String negotiatedQOP = 
saslClient.getNegotiatedProperty(Sasl.QOP).toString();
+assert 
(negotiatedQOP.equals(SaslProperties.QualityOfProtection.PRIVACY.getSaslQop()));
+
+// Update the rawWrapChunkSize with the negotiated buffer size 
since we cannot call encode with more than
+// negotiated size of buffer.
+final int negotiatedRawSendSize = Integer.parseInt(saslClient
+
.getNegotiatedProperty(SaslProperties.WRAP_RAW_SEND_SIZE)
+.toString());
+if(negotiatedRawSendSize <= 0) {
+  throw new SaslException(String.format("Negotiated rawSendSize: 
%d is invalid. Please check the configured " +
+  "value of sasl.encryption.encodesize. It might be configured 
to a very small value.",
+negotiatedRawSendSize));
+}
+connection.setRawWrapSendSize(negotiatedRawSendSize);
+connection.addSecurityHandlers();
+  } catch (Exception e) {
+throw new SaslException(String.format("Unexpected failure while 
retrieving negotiated property values (%s)",
+  e.getMessage()), e);
+  }
+} else {
+  // Encryption is not required hence we don't need to hold on to 
saslClient object.
+  if (saslClient != null) {
--- End diff --

Considering these objects are per connection and have security contexts, I 
would prefer to get rid of these objects in cases when only Authentication is 
used. 


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15970779#comment-15970779
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r111646250
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/BitConnectionConfig.java 
---
@@ -46,16 +47,40 @@ protected BitConnectionConfig(BufferAllocator 
allocator, BootStrapContext contex
 super(allocator, context);
 
 final DrillConfig config = context.getConfig();
+final AuthenticatorProvider authProvider = getAuthProvider();
+
 if (config.getBoolean(ExecConstants.BIT_AUTHENTICATION_ENABLED)) {
   this.authMechanismToUse = 
config.getString(ExecConstants.BIT_AUTHENTICATION_MECHANISM);
   try {
-getAuthProvider().getAuthenticatorFactory(authMechanismToUse);
+authProvider.getAuthenticatorFactory(authMechanismToUse);
   } catch (final SaslException e) {
 throw new DrillbitStartupException(String.format(
 "'%s' mechanism not found for bit-to-bit authentication. 
Please check authentication configuration.",
 authMechanismToUse));
   }
-  logger.info("Configured bit-to-bit connections to require 
authentication using: {}", authMechanismToUse);
+
+  // Update encryption related configurations
+  
encryptionContext.setEncryption(config.getBoolean(ExecConstants.BIT_SASL_ENCRYPTION_ENABLED));
+
+  int maxEncodeSize = 
config.getInt(ExecConstants.BIT_SASL_ENCRYPTION_ENCODESIZE);
+
+  if(maxEncodeSize > RpcConstants.MAX_WRAP_SIZE) {
+logger.warn("Setting bit.sasl.encryption.encodesize to maximum 
allowed value of 16MB");
+maxEncodeSize = RpcConstants.MAX_WRAP_SIZE;
+  }
+  encryptionContext.setWrappedChunkSize(maxEncodeSize);
+
+  if (encryptionContext.isEncryptionEnabled() && 
authProvider.isOnlyPlainConfigured()) {
+throw new DrillbitStartupException("Encryption is enabled but only 
PLAIN mechanism is configured." +
+  " Please check the security.bit configurations.");
+  }
+
+  logger.info("Configured bit-to-bit connections to require 
authentication using: {} with encryption: {}",
+authMechanismToUse, encryptionContext.getEncryptionString());
+
+} else if 
(config.getBoolean(ExecConstants.BIT_SASL_ENCRYPTION_ENABLED)) {
+  throw new DrillbitStartupException("Invalid security configuration. 
Encryption is enabled with authentication " +
--- End diff --

Fixed


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15970787#comment-15970787
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r111647100
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/security/ServerAuthenticationHandler.java
 ---
@@ -251,25 +256,62 @@ void process(SaslResponseContext context) 
throws Exception {
   private static , T extends EnumLite>
   void handleSuccess(final SaslResponseContext context, final 
SaslMessage.Builder challenge,
  final SaslServer saslServer) throws IOException {
-context.connection.changeHandlerTo(context.requestHandler);
-context.connection.finalizeSaslSession();
-context.sender.send(new Response(context.saslResponseType, 
challenge.build()));
 
-// setup security layers here..
+final S connection = context.connection;
+connection.changeHandlerTo(context.requestHandler);
+connection.finalizeSaslSession();
+context.sender.send(new Response(context.saslResponseType, 
challenge.build()));
 
 if (logger.isTraceEnabled()) {
-  logger.trace("Authenticated {} successfully using {} from {}", 
saslServer.getAuthorizationID(),
-  saslServer.getMechanismName(), context.remoteAddress);
+  logger.trace("Authenticated {} successfully using {} from {} with 
encryption context {}",
+saslServer.getAuthorizationID(), saslServer.getMechanismName(), 
connection.getRemoteAddress().toString(),
+connection.getEncryptionString());
+}
+
+if (connection.isEncrypted()) {
+  try {
+// Check if connection was marked for being secure then verify for 
negotiated QOP value for correctness.
+final String negotiatedQOP = 
saslServer.getNegotiatedProperty(Sasl.QOP).toString();
+assert 
(negotiatedQOP.equals(SaslProperties.QualityOfProtection.PRIVACY.getSaslQop()));
--- End diff --

Fixed.


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15970796#comment-15970796
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r111676528
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/security/SaslProperties.java
 ---
@@ -0,0 +1,72 @@
+/*
+ * 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.drill.exec.rpc.security;
+
+import javax.security.sasl.Sasl;
+import java.util.HashMap;
+import java.util.Map;
+
+public final class SaslProperties {
+
+  /**
+   * All supported Quality of Protection value which can be negotiated for
+   */
+  enum QualityOfProtection {
+AUTHENTICATION("auth"),
+INTEGRITY("auth-int"),
+PRIVACY("auth-conf");
+
+public final String saslQop;
+
+QualityOfProtection(String saslQop) {
+  this.saslQop = saslQop;
+}
+
+public String getSaslQop() {
+  return saslQop;
+}
+  }
+
+  static final String WRAP_RAW_SEND_SIZE = 
"javax.security.sasl.rawsendsize";
+
+  /**
+   * Get's the map of minimum set of SaslProperties required during 
negotiation process either for encryption
+   * or authentication
+   * @param encryptionEnabled - Flag to determine if property needed is 
for encryption or authentication
+   * @param wrappedChunkSize  - Configured wrappedChunkSize to negotiate 
for.
+   *Default is {@link 
org.apache.drill.exec.rpc.RpcConstants.MAX_WRAP_SIZE}
--- End diff --

Default value is set in configuration file drill-module.conf
max_wrapped_size : 16777215



> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15970774#comment-15970774
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r111645820
  
--- Diff: 
common/src/main/java/org/apache/drill/common/config/DrillProperties.java ---
@@ -59,6 +60,12 @@
 
   public static final String KEYTAB = "keytab";
 
+  public static final String ENCRYPTION = "encryption";
--- End diff --

Renamed to sasl_encrypt to be consistent with other usages.


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15970781#comment-15970781
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r111646493
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/security/AuthenticationOutcomeListener.java
 ---
@@ -243,4 +247,43 @@ public SaslMessage process(SaslChallengeContext 
context) throws Exception {
   }
 }
   }
+
+  private static void handleSuccess(SaslChallengeContext context) throws 
SaslException {
+final ClientConnection connection = context.connection;
+final SaslClient saslClient = connection.getSaslClient();
+
+if (connection.isEncrypted()) {
+  try {
+// Check if connection was marked for being secure then verify for 
negotiated QOP value for
+// correctness.
+final String negotiatedQOP = 
saslClient.getNegotiatedProperty(Sasl.QOP).toString();
--- End diff --

Should not return null since QOP property is negotiated both for 
authentication and encryption case.


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15970809#comment-15970809
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r111694626
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/security/ServerAuthenticationHandler.java
 ---
@@ -208,8 +207,12 @@ void process(SaslResponseContext context) throws 
Exception {
 
 handleSuccess(context, challenge, saslServer);
   } else {
-logger.info("Failed to authenticate client from {}", 
context.remoteAddress);
-throw new SaslException("Client allegedly succeeded 
authentication, but server did not. Suspicious?");
+final S connection = context.connection;
+logger.info("Failed to authenticate client from {} with encryption 
context:{}",
+  connection.getRemoteAddress().toString(),
--- End diff --

getEncryptionString is returning string literal so it will be cached by JVM 
String pool after the first time.


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15970806#comment-15970806
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r111710374
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserConnectionConfig.java
 ---
@@ -34,32 +39,81 @@
 
   private final UserServerRequestHandler handler;
 
+  // Total number of external DrillClient connection's on this server.
+  private static final Counter secureUserConnections = 
DrillMetrics.getRegistry()
+.counter("drill.user.encrypted.connections");
+
+  private static final Counter insecureUserConnections = 
DrillMetrics.getRegistry()
+.counter("drill.user.unencrypted.connections");
+
   UserConnectionConfig(BufferAllocator allocator, BootStrapContext 
context, UserServerRequestHandler handler)
-  throws DrillbitStartupException {
+throws DrillbitStartupException {
 super(allocator, context);
 this.handler = handler;
 
-if 
(context.getConfig().getBoolean(ExecConstants.USER_AUTHENTICATION_ENABLED)) {
-  if (getAuthProvider().getAllFactoryNames().isEmpty()) {
+final DrillConfig config = context.getConfig();
+final AuthenticatorProvider authProvider = getAuthProvider();
+
+if (config.getBoolean(ExecConstants.USER_AUTHENTICATION_ENABLED)) {
+  if (authProvider.getAllFactoryNames().isEmpty()) {
 throw new DrillbitStartupException("Authentication enabled, but no 
mechanisms found. Please check " +
-"authentication configuration.");
+  "authentication configuration.");
--- End diff --

Thanks for the info. I have set the IDE settings and made the formatting 
changes in other places as well.


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15970776#comment-15970776
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r111646441
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/security/AuthenticationOutcomeListener.java
 ---
@@ -120,19 +121,22 @@ public void success(SaslMessage value, ByteBuf 
buffer) {
   new SaslException("Server sent a corrupt message.")));
 } else {
   try {
-final SaslChallengeContext context = new 
SaslChallengeContext(value, connection.getSaslClient(), ugi);
-
+final SaslChallengeContext context = new 
SaslChallengeContext<>(value, ugi, connection);
 final SaslMessage saslResponse = processor.process(context);
 
 if (saslResponse != null) {
   client.send(new AuthenticationOutcomeListener<>(client, 
connection, saslRpcType, ugi, completionListener),
   connection, saslRpcType, saslResponse, SaslMessage.class,
-  true /** the connection will not be backed up at this point 
*/);
+  true /* the connection will not be backed up at this point 
*/);
 } else {
   // success
   completionListener.success(null, null);
+  logger.trace("Successfully authenticated to server using {} 
mechanism and encryption context: {}",
--- End diff --

trying to understand why this check is needed here ? Whereas other places 
we just end up using logger.trace as is ? E.g: above `logger.trace("Initiated 
SASL exchange")`


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15970800#comment-15970800
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r111651798
  
--- Diff: 
exec/rpc/src/main/java/org/apache/drill/exec/rpc/AbstractRemoteConnection.java 
---
@@ -224,4 +237,67 @@ public void close() {
 }
   }
 
+  /**
+   * Helps to add all the required security handler's after negotiation 
for encryption is completed.
+   * 
+   *  Handler's that are added are:
+   *  SaslDecryptionHandler
+   *  LengthFieldBasedFrameDecoder Handler
+   *  SaslEncryptionHandler
+   *  ChunkCreationHandler
+   * 
+   * 
+   *  If encryption is enabled ChunkCreationHandler is always added 
irrespective of chunkMode enabled or not.
+   *  This helps to make a generic encryption handler.
+   * 
+   */
+  @Override
+  public void addSecurityHandlers() {
+
+final ChannelPipeline channelPipeline = getChannel().pipeline();
+channelPipeline.addFirst("SaslDecryptionHandler", new 
SaslDecryptionHandler(saslBackend, getWrappedChunkSize(),
--- End diff --

Moved all the handlers names as part of RpcConstants and using that in 
BasicClient/BasicServer as well.


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15970782#comment-15970782
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r111646237
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/BitConnectionConfig.java 
---
@@ -46,16 +47,40 @@ protected BitConnectionConfig(BufferAllocator 
allocator, BootStrapContext contex
 super(allocator, context);
 
 final DrillConfig config = context.getConfig();
+final AuthenticatorProvider authProvider = getAuthProvider();
+
 if (config.getBoolean(ExecConstants.BIT_AUTHENTICATION_ENABLED)) {
   this.authMechanismToUse = 
config.getString(ExecConstants.BIT_AUTHENTICATION_MECHANISM);
   try {
-getAuthProvider().getAuthenticatorFactory(authMechanismToUse);
+authProvider.getAuthenticatorFactory(authMechanismToUse);
   } catch (final SaslException e) {
 throw new DrillbitStartupException(String.format(
 "'%s' mechanism not found for bit-to-bit authentication. 
Please check authentication configuration.",
 authMechanismToUse));
   }
-  logger.info("Configured bit-to-bit connections to require 
authentication using: {}", authMechanismToUse);
+
+  // Update encryption related configurations
+  
encryptionContext.setEncryption(config.getBoolean(ExecConstants.BIT_SASL_ENCRYPTION_ENABLED));
+
+  int maxEncodeSize = 
config.getInt(ExecConstants.BIT_SASL_ENCRYPTION_ENCODESIZE);
+
+  if(maxEncodeSize > RpcConstants.MAX_WRAP_SIZE) {
+logger.warn("Setting bit.sasl.encryption.encodesize to maximum 
allowed value of 16MB");
+maxEncodeSize = RpcConstants.MAX_WRAP_SIZE;
+  }
+  encryptionContext.setWrappedChunkSize(maxEncodeSize);
--- End diff --

Sorry about the confusion. I have renamed the variables, hope that helps.

maxEncodeSize -- maxWrappedSize. This configurable parameter defines the 
maximum size of wrapped buffer that wrap call can produce. It's negotiated as 
part of SASL handshake and has maximum limit of 16MB
rawSendSize -- wrapSizeLimit. This parameter (non-configurable) is the 
maximum plain buffer size which we can pass to wrap function so that wrapped 
buffer size doesn't exceed maxWrappedSize. This is obtained after SASL 
negotiation is completed.


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15970792#comment-15970792
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r111647207
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/security/kerberos/KerberosFactory.java
 ---
@@ -93,6 +94,7 @@ public UserGroupInformation createAndLoginUser(final 
Map properties)
   @Override
   public SaslServer createSaslServer(final UserGroupInformation ugi, final 
Map properties)
   throws SaslException {
+final String qopValue = properties.containsKey(Sasl.QOP) ? 
properties.get(Sasl.QOP).toString() : "null";
--- End diff --

Fixed.


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15970793#comment-15970793
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r111676624
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserServer.java ---
@@ -335,8 +350,27 @@ public BitToUserHandshake 
getHandshakeResponse(UserToBitHandshake inbound) throw
 }
   }
 
-  // mention server's authentication capabilities
-  
respBuilder.addAllAuthenticationMechanisms(config.getAuthProvider().getAllFactoryNames());
+  // We are checking in UserConnectionConfig that if SASL 
encryption is enabled then mechanisms other
+  // than PLAIN are also configured otherwise throw exception
+  final Set configuredMech = 
config.getAuthProvider().getAllFactoryNames();
+
+  if (!config.isEncryptionEnabled()) {
+
+respBuilder.addAllAuthenticationMechanisms(configuredMech);
+  } else {
--- End diff --

Since PLAIN doesn't support encryption and also we don't support older 
clients without encryption support to connect to cluster with encryption 
enabled, there was no case where client can use PLAIN mechanism. 

But considering other custom mechanism which might not support encryption 
as well. Have removed the special handling for PLAIN.


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15970803#comment-15970803
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r111694492
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/security/ClientAuthenticatorProvider.java
 ---
@@ -101,4 +101,12 @@ public void close() throws Exception {
 AutoCloseables.close(authFactories.values());
 authFactories.clear();
   }
+
+  /**
+   * By default Kerberos and Plain factories are provided by Drill.
+   */
+  @Override
+  public boolean isOnlyPlainConfigured() {
+return false;
--- End diff --

Removed the method.


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15970775#comment-15970775
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r111645872
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/BitConnectionConfig.java 
---
@@ -46,16 +47,40 @@ protected BitConnectionConfig(BufferAllocator 
allocator, BootStrapContext contex
 super(allocator, context);
 
 final DrillConfig config = context.getConfig();
+final AuthenticatorProvider authProvider = getAuthProvider();
+
 if (config.getBoolean(ExecConstants.BIT_AUTHENTICATION_ENABLED)) {
   this.authMechanismToUse = 
config.getString(ExecConstants.BIT_AUTHENTICATION_MECHANISM);
   try {
-getAuthProvider().getAuthenticatorFactory(authMechanismToUse);
+authProvider.getAuthenticatorFactory(authMechanismToUse);
   } catch (final SaslException e) {
 throw new DrillbitStartupException(String.format(
 "'%s' mechanism not found for bit-to-bit authentication. 
Please check authentication configuration.",
 authMechanismToUse));
   }
-  logger.info("Configured bit-to-bit connections to require 
authentication using: {}", authMechanismToUse);
+
+  // Update encryption related configurations
+  
encryptionContext.setEncryption(config.getBoolean(ExecConstants.BIT_SASL_ENCRYPTION_ENABLED));
+
+  int maxEncodeSize = 
config.getInt(ExecConstants.BIT_SASL_ENCRYPTION_ENCODESIZE);
+
+  if(maxEncodeSize > RpcConstants.MAX_WRAP_SIZE) {
--- End diff --

Fixed.


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

[jira] [Commented] (DRILL-4335) Apache Drill should support network encryption

[ASF GitHub Bot (JIRA)]

[ 
https://issues.apache.org/jira/browse/DRILL-4335?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15970783#comment-15970783
 ] 

ASF GitHub Bot commented on DRILL-4335:
---

Github user sohami commented on a diff in the pull request:

https://github.com/apache/drill/pull/773#discussion_r111646353
  
--- Diff: 
exec/java-exec/src/main/java/org/apache/drill/exec/rpc/ConnectionConfig.java ---
@@ -31,4 +31,11 @@
 
   AuthenticatorProvider getAuthProvider();
 
+  void incConnectionCounter(boolean secure);
--- End diff --

Created a separate interface RpcMetrics and corresponding metrics classes 
for different server types and moved the counters there. Also refactored 
existing rpc-memory-allocator metrics to be in new class. 

The new class can be used in future to include any new metrics specific to 
Rpc Layer


> Apache Drill should support network encryption
> --
>
> Key: DRILL-4335
> URL: https://issues.apache.org/jira/browse/DRILL-4335
> Project: Apache Drill
>  Issue Type: New Feature
>Reporter: Keys Botzum
>Assignee: Sorabh Hamirwasia
>  Labels: security
> Attachments: ApacheDrillEncryptionUsingSASLDesign.pdf
>
>
> This is clearly related to Drill-291 but wanted to make explicit that this 
> needs to include network level encryption and not just authentication. This 
> is particularly important for the client connection to Drill which will often 
> be sending passwords in the clear until there is encryption.



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)