diff options
3 files changed, 377 insertions, 10 deletions
diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserClient.java b/exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserClient.java index 99614bdfa..fb7881290 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserClient.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/rpc/user/UserClient.java @@ -202,7 +202,8 @@ public class UserClient .checkedGet(sslConfig.getHandshakeTimeout(), TimeUnit.MILLISECONDS); } catch (TimeoutException e) { String msg = new StringBuilder().append( - "Connecting to the server timed out. This is sometimes due to a mismatch in the SSL configuration between client and server. [ Exception: ") + "Connecting to the server timed out. This is sometimes due to a mismatch in the SSL configuration between" + + " client and server. [ Exception: ") .append(e.getMessage()).append("]").toString(); throw new NonTransientRpcException(msg); } @@ -210,15 +211,8 @@ public class UserClient connect(hsBuilder.build(), endpoint).checkedGet(); } - // Check if client needs encryption and server is not configured for encryption. - final boolean clientNeedsEncryption = properties.containsKey(DrillProperties.SASL_ENCRYPT) && Boolean - .parseBoolean(properties.getProperty(DrillProperties.SASL_ENCRYPT)); - - if (clientNeedsEncryption && !connection.isEncryptionEnabled()) { - throw new NonTransientRpcException( - "Client needs encrypted connection but server is not configured for " - + "encryption. Please check connection parameter or contact your administrator"); - } + // Validate if both client and server are compatible in their security requirements for the connection + validateSaslCompatibility(properties); if (serverAuthMechanisms != null) { try { @@ -229,6 +223,61 @@ public class UserClient } } + /** + * Validate that security requirements from client and Drillbit side is compatible. For example: It verifies if one + * side needs authentication / encryption then other side is also configured to support that security properties. + * @param properties - DrillClient connection parameters + * @throws NonTransientRpcException - When DrillClient security requirements doesn't match Drillbit side of security + * configurations. + */ + private void validateSaslCompatibility(DrillProperties properties) throws NonTransientRpcException { + + final boolean clientNeedsEncryption = properties.containsKey(DrillProperties.SASL_ENCRYPT) + && Boolean.parseBoolean(properties.getProperty(DrillProperties.SASL_ENCRYPT)); + + // Check if client needs encryption and server is not configured for encryption. + if (clientNeedsEncryption && !connection.isEncryptionEnabled()) { + throw new NonTransientRpcException( + "Client needs encrypted connection but server is not configured for encryption." + + " Please contact your administrator. [Warn: It may be due to wrong config or a security attack in" + + " progress.]"); + } + + // Check if client needs encryption and server doesn't support any security mechanisms. + if (clientNeedsEncryption && serverAuthMechanisms == null) { + throw new NonTransientRpcException( + "Client needs encrypted connection but server doesn't support any security mechanisms." + + " Please contact your administrator. [Warn: It may be due to wrong config or a security attack in" + + " progress.]"); + } + + // Check if client needs authentication and server doesn't support any security mechanisms. + if (clientNeedsAuth(properties) && serverAuthMechanisms == null) { + throw new NonTransientRpcException( + "Client needs authentication but server doesn't support any security mechanisms." + + " Please contact your administrator. [Warn: It may be due to wrong config or a security attack in" + + " progress.]"); + } + } + + /** + * Determine if client needs authenticated connection or not. It checks for following as an indication of + * requiring authentication from client side: + * 1) Any auth mechanism is provided in connection properties with DrillProperties.AUTH_MECHANISM parameter. + * 2) A service principal is provided in connection properties in which case we treat AUTH_MECHANISM as Kerberos + * 3) Username and Password is provided in connection properties in which case we treat AUTH_MECHANISM as Plain + * @param props - DrillClient connection parameters + * @return - true - If any of above 3 checks is successful. + * - false - If all the above 3 checks failed. + */ + private boolean clientNeedsAuth(DrillProperties props) { + + return !Strings.isNullOrEmpty(props.getProperty(DrillProperties.AUTH_MECHANISM)) || + !Strings.isNullOrEmpty(props.getProperty(DrillProperties.SERVICE_PRINCIPAL)) || + (props.containsKey(DrillProperties.USER) && !Strings.isNullOrEmpty(props.getProperty(DrillProperties.PASSWORD))); + + } + private CheckedFuture<Void, RpcException> connect(final UserToBitHandshake handshake, final DrillbitEndpoint endpoint) { final SettableFuture<Void> connectionSettable = SettableFuture.create(); diff --git a/exec/java-exec/src/test/java/org/apache/drill/exec/rpc/user/security/TestUserBitSaslCompatibility.java b/exec/java-exec/src/test/java/org/apache/drill/exec/rpc/user/security/TestUserBitSaslCompatibility.java new file mode 100644 index 000000000..bbb957ddb --- /dev/null +++ b/exec/java-exec/src/test/java/org/apache/drill/exec/rpc/user/security/TestUserBitSaslCompatibility.java @@ -0,0 +1,317 @@ +/* + * 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.user.security; + + +import com.google.common.collect.Lists; +import com.typesafe.config.ConfigValueFactory; +import org.apache.drill.BaseTestQuery; +import org.apache.drill.common.config.DrillConfig; +import org.apache.drill.common.config.DrillProperties; +import org.apache.drill.exec.ExecConstants; +import org.apache.drill.exec.rpc.NonTransientRpcException; +import org.apache.drill.exec.rpc.user.security.testing.UserAuthenticatorTestImpl; +import org.junit.BeforeClass; +import org.junit.Test; + +import javax.security.sasl.SaslException; +import java.util.Properties; + +import static junit.framework.TestCase.assertTrue; +import static junit.framework.TestCase.fail; + +/** + * Helps to test different scenarios based on security configuration on client and Drillbit side with respect to SASL + * and specifically using PLAIN mechanism + */ +public class TestUserBitSaslCompatibility extends BaseTestQuery { + //private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(TestUserClient.class); + + @BeforeClass + public static void setup() { + final DrillConfig newConfig = new DrillConfig(DrillConfig.create(cloneDefaultTestConfigProperties()) + .withValue(ExecConstants.USER_AUTHENTICATION_ENABLED, + ConfigValueFactory.fromAnyRef(true)) + .withValue(ExecConstants.USER_AUTHENTICATOR_IMPL, + ConfigValueFactory.fromAnyRef(UserAuthenticatorTestImpl.TYPE)) + .withValue(ExecConstants.AUTHENTICATION_MECHANISMS, + ConfigValueFactory.fromIterable(Lists.newArrayList("plain"))) + .withValue(ExecConstants.USER_ENCRYPTION_SASL_ENABLED, + ConfigValueFactory.fromAnyRef(false)), + false); + + final Properties connectionProps = new Properties(); + connectionProps.setProperty(DrillProperties.USER, "anonymous"); + connectionProps.setProperty(DrillProperties.PASSWORD, "anything works!"); + + updateTestCluster(1, newConfig, connectionProps); + } + + /** + * Test showing failure before SASL handshake when Drillbit is not configured for authentication whereas client + * explicitly requested for authentication. + * @throws Exception + */ + @Test + public void testDisableDrillbitAuth_EnableClientAuth() throws Exception { + + final DrillConfig newConfig = new DrillConfig(DrillConfig.create(cloneDefaultTestConfigProperties()) + .withValue(ExecConstants.USER_AUTHENTICATION_ENABLED, + ConfigValueFactory.fromAnyRef(false)), + false); + + final Properties connectionProps = new Properties(); + connectionProps.setProperty(DrillProperties.USER, "anonymous"); + connectionProps.setProperty(DrillProperties.PASSWORD, "anything works!"); + + try { + updateTestCluster(1, newConfig, connectionProps); + fail(); + } catch (Exception ex) { + assertTrue(ex.getCause() instanceof NonTransientRpcException); + assertTrue(!(ex.getCause().getCause() instanceof SaslException)); + } + } + + /** + * Test showing failure before SASL handshake when Drillbit is not configured for authentication whereas client + * explicitly requested for encrypted connection. + * @throws Exception + */ + @Test + public void testDisableDrillbitAuth_EnableClientEncryption() throws Exception { + final DrillConfig newConfig = new DrillConfig(DrillConfig.create(cloneDefaultTestConfigProperties()) + .withValue(ExecConstants.USER_AUTHENTICATION_ENABLED, + ConfigValueFactory.fromAnyRef(false)), + false); + + final Properties connectionProps = new Properties(); + connectionProps.setProperty(DrillProperties.USER, "anonymous"); + connectionProps.setProperty(DrillProperties.PASSWORD, "anything works!"); + connectionProps.setProperty(DrillProperties.SASL_ENCRYPT, "true"); + + try { + updateTestCluster(1, newConfig, connectionProps); + fail(); + } catch (Exception ex) { + assertTrue(ex.getCause() instanceof NonTransientRpcException); + assertTrue(!(ex.getCause().getCause() instanceof SaslException)); + } + } + + /** + * Test showing failure before SASL handshake when Drillbit is not configured for encryption whereas client explicitly + * requested for encrypted connection. + * @throws Exception + */ + @Test + public void testDisableDrillbitEncryption_EnableClientEncryption() throws Exception { + final DrillConfig newConfig = new DrillConfig(DrillConfig.create(cloneDefaultTestConfigProperties()) + .withValue(ExecConstants.USER_AUTHENTICATION_ENABLED, + ConfigValueFactory.fromAnyRef(true)) + .withValue(ExecConstants.USER_AUTHENTICATOR_IMPL, + ConfigValueFactory.fromAnyRef(UserAuthenticatorTestImpl.TYPE)) + .withValue(ExecConstants.AUTHENTICATION_MECHANISMS, + ConfigValueFactory.fromIterable(Lists.newArrayList("plain"))) + .withValue(ExecConstants.USER_ENCRYPTION_SASL_ENABLED, + ConfigValueFactory.fromAnyRef(false)), + false); + + final Properties connectionProps = new Properties(); + connectionProps.setProperty(DrillProperties.USER, "anonymous"); + connectionProps.setProperty(DrillProperties.PASSWORD, "anything works!"); + connectionProps.setProperty(DrillProperties.SASL_ENCRYPT, "true"); + + try { + updateTestCluster(1, newConfig, connectionProps); + fail(); + } catch (Exception ex) { + assertTrue(ex.getCause() instanceof NonTransientRpcException); + assertTrue(!(ex.getCause().getCause() instanceof SaslException)); + } + } + + /** + * Test showing failure in SASL handshake when Drillbit is configured for authentication only whereas client doesn't + * provide any security properties like username/password in this case. + * @throws Exception + */ + @Test + public void testEnableDrillbitAuth_DisableClientAuth() throws Exception { + final DrillConfig newConfig = new DrillConfig(DrillConfig.create(cloneDefaultTestConfigProperties()) + .withValue(ExecConstants.USER_AUTHENTICATION_ENABLED, + ConfigValueFactory.fromAnyRef(true)) + .withValue(ExecConstants.USER_AUTHENTICATOR_IMPL, + ConfigValueFactory.fromAnyRef(UserAuthenticatorTestImpl.TYPE)) + .withValue(ExecConstants.AUTHENTICATION_MECHANISMS, + ConfigValueFactory.fromIterable(Lists.newArrayList("plain"))) + .withValue(ExecConstants.USER_ENCRYPTION_SASL_ENABLED, + ConfigValueFactory.fromAnyRef(false)), + false); + + final Properties connectionProps = new Properties(); + + try { + updateTestCluster(1, newConfig, connectionProps); + fail(); + } catch (Exception ex) { + assertTrue(ex.getCause() instanceof NonTransientRpcException); + assertTrue(ex.getCause().getCause() instanceof SaslException); + } + } + + /** + * Test showing failure in SASL handshake when Drillbit is configured for encryption whereas client doesn't provide any + * security properties like username/password in this case. + * @throws Exception + */ + @Test + public void testEnableDrillbitEncryption_DisableClientAuth() throws Exception { + final DrillConfig newConfig = new DrillConfig(DrillConfig.create(cloneDefaultTestConfigProperties()) + .withValue(ExecConstants.USER_AUTHENTICATION_ENABLED, + ConfigValueFactory.fromAnyRef(true)) + .withValue(ExecConstants.USER_AUTHENTICATOR_IMPL, + ConfigValueFactory.fromAnyRef(UserAuthenticatorTestImpl.TYPE)) + .withValue(ExecConstants.AUTHENTICATION_MECHANISMS, + ConfigValueFactory.fromIterable(Lists.newArrayList("plain"))) + .withValue(ExecConstants.USER_ENCRYPTION_SASL_ENABLED, + ConfigValueFactory.fromAnyRef(true)), + false); + + final Properties connectionProps = new Properties(); + + try { + updateTestCluster(1, newConfig, connectionProps); + fail(); + } catch (Exception ex) { + assertTrue(ex.getCause() instanceof NonTransientRpcException); + assertTrue(ex.getCause().getCause() instanceof SaslException); + } + } + + /** + * Test showing successful SASL handshake when both Drillbit and client side authentication is enabled using PLAIN + * mechanism. + * @throws Exception + */ + @Test + public void testEnableDrillbitClientAuth() throws Exception { + final DrillConfig newConfig = new DrillConfig(DrillConfig.create(cloneDefaultTestConfigProperties()) + .withValue(ExecConstants.USER_AUTHENTICATION_ENABLED, + ConfigValueFactory.fromAnyRef(true)) + .withValue(ExecConstants.USER_AUTHENTICATOR_IMPL, + ConfigValueFactory.fromAnyRef(UserAuthenticatorTestImpl.TYPE)) + .withValue(ExecConstants.AUTHENTICATION_MECHANISMS, + ConfigValueFactory.fromIterable(Lists.newArrayList("plain"))) + .withValue(ExecConstants.USER_ENCRYPTION_SASL_ENABLED, + ConfigValueFactory.fromAnyRef(false)), + false); + + final Properties connectionProps = new Properties(); + connectionProps.setProperty(DrillProperties.USER, "anonymous"); + connectionProps.setProperty(DrillProperties.PASSWORD, "anything works!"); + + try { + updateTestCluster(1, newConfig, connectionProps); + } catch (Exception ex) { + fail(); + } + } + + /** + * Below test shows the failure in Sasl layer with client and Drillbit side encryption enabled using PLAIN + * mechanism. This is expected since PLAIN mechanism doesn't support encryption using SASL. Whereas same test + * setup using Kerberos or any other mechanism with encryption support will result in successful SASL handshake. + * @throws Exception + */ + @Test + public void testEnableDrillbitClientEncryption_UsingPlain() throws Exception { + final DrillConfig newConfig = new DrillConfig(DrillConfig.create(cloneDefaultTestConfigProperties()) + .withValue(ExecConstants.USER_AUTHENTICATION_ENABLED, + ConfigValueFactory.fromAnyRef(true)) + .withValue(ExecConstants.USER_AUTHENTICATOR_IMPL, + ConfigValueFactory.fromAnyRef(UserAuthenticatorTestImpl.TYPE)) + .withValue(ExecConstants.AUTHENTICATION_MECHANISMS, + ConfigValueFactory.fromIterable(Lists.newArrayList("plain"))) + .withValue(ExecConstants.USER_ENCRYPTION_SASL_ENABLED, + ConfigValueFactory.fromAnyRef(true)), + false); + + final Properties connectionProps = new Properties(); + connectionProps.setProperty(DrillProperties.USER, "anonymous"); + connectionProps.setProperty(DrillProperties.PASSWORD, "anything works!"); + connectionProps.setProperty(DrillProperties.SASL_ENCRYPT, "true"); + + try { + updateTestCluster(1, newConfig, connectionProps); + fail(); + } catch (Exception ex) { + assertTrue(ex.getCause() instanceof NonTransientRpcException); + assertTrue(ex.getCause().getCause() instanceof SaslException); + } + } + + /** + * Test showing successful handshake when authentication is disabled on Drillbit side and client also + * doesn't provide any security properties in connection URL. + * @throws Exception + */ + @Test + public void testDisableDrillbitClientAuth() throws Exception { + final DrillConfig newConfig = new DrillConfig(DrillConfig.create(cloneDefaultTestConfigProperties()) + .withValue(ExecConstants.USER_AUTHENTICATION_ENABLED, + ConfigValueFactory.fromAnyRef(false)), + false); + + final Properties connectionProps = new Properties(); + + try { + updateTestCluster(1, newConfig, connectionProps); + } catch (Exception ex) { + fail(); + } + } + + /** + * Test showing successful handshake when authentication is disabled but impersonation is enabled on Drillbit side + * and client only provides USERNAME as a security property in connection URL. + * @throws Exception + */ + @Test + public void testEnableDrillbitImpersonation_DisableClientAuth() throws Exception { + final DrillConfig newConfig = new DrillConfig(DrillConfig.create(cloneDefaultTestConfigProperties()) + .withValue(ExecConstants.USER_AUTHENTICATION_ENABLED, + ConfigValueFactory.fromAnyRef(false)) + .withValue(ExecConstants.IMPERSONATION_ENABLED, + ConfigValueFactory.fromAnyRef(true)) + .withValue(ExecConstants.IMPERSONATION_MAX_CHAINED_USER_HOPS, + ConfigValueFactory.fromAnyRef(3)), + false); + + final Properties connectionProps = new Properties(); + connectionProps.setProperty(DrillProperties.USER, "anonymous"); + + try { + updateTestCluster(1, newConfig, connectionProps); + } catch (Exception ex) { + fail(); + } + } + +} + diff --git a/exec/rpc/src/main/java/org/apache/drill/exec/rpc/BasicClient.java b/exec/rpc/src/main/java/org/apache/drill/exec/rpc/BasicClient.java index 0d80df6da..0f4ef1be7 100644 --- a/exec/rpc/src/main/java/org/apache/drill/exec/rpc/BasicClient.java +++ b/exec/rpc/src/main/java/org/apache/drill/exec/rpc/BasicClient.java @@ -254,6 +254,7 @@ public abstract class BasicClient<T extends EnumLite, CC extends ClientConnectio if (connection != null) { connection.close(); + connection = null; } } } |