This is an automated email from the ASF dual-hosted git repository. volodymyr pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/drill.git
The following commit(s) were added to refs/heads/master by this push: new 1b95c0a DRILL-7750: Drill fails to read KeyStore password from Credential provider (#2088) 1b95c0a is described below commit 1b95c0a8cfce23e11596353a821a5216fd1a983d Author: Bohdan Kazydub <38408110+kazyd...@users.noreply.github.com> AuthorDate: Sat Jun 20 15:51:14 2020 +0300 DRILL-7750: Drill fails to read KeyStore password from Credential provider (#2088) --- .../java/org/apache/drill/exec/ssl/SSLConfig.java | 32 +++++++++++++++++++-- .../apache/drill/exec/ssl/SSLConfigBuilder.java | 6 ++-- .../org/apache/drill/exec/ssl/SSLConfigClient.java | 33 ++++++++++++++++++---- .../org/apache/drill/exec/ssl/SSLConfigServer.java | 22 +++++++++++---- .../drill/exec/ssl/SSLCredentialsProvider.java | 22 ++++++++++----- 5 files changed, 91 insertions(+), 24 deletions(-) diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/ssl/SSLConfig.java b/exec/java-exec/src/main/java/org/apache/drill/exec/ssl/SSLConfig.java index c3ff0c5..e82bbdf 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/ssl/SSLConfig.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/ssl/SSLConfig.java @@ -22,18 +22,23 @@ import io.netty.handler.ssl.SslProvider; import io.netty.handler.ssl.util.InsecureTrustManagerFactory; import org.apache.drill.common.exceptions.DrillException; import org.apache.drill.exec.memory.BufferAllocator; +import org.apache.hadoop.conf.Configuration; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import javax.net.ssl.KeyManagerFactory; import javax.net.ssl.SSLContext; import javax.net.ssl.SSLEngine; import javax.net.ssl.TrustManagerFactory; import java.io.FileInputStream; +import java.io.IOException; import java.io.InputStream; import java.security.KeyStore; +import java.text.MessageFormat; public abstract class SSLConfig { - private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(SSLConfig.class); + private static final Logger logger = LoggerFactory.getLogger(SSLConfig.class); public static final String DEFAULT_SSL_PROVIDER = "JDK"; // JDK or OPENSSL public static final String DEFAULT_SSL_PROTOCOL = "TLSv1.2"; @@ -60,8 +65,7 @@ public abstract class SSLConfig { // copy of Hadoop's SSLFactory.Mode. Done so that we do not // need to include hadoop-common as a dependency in // jdbc-all-jar. - public enum Mode { CLIENT, SERVER }; - + public enum Mode { CLIENT, SERVER } public SSLConfig() { } @@ -230,6 +234,28 @@ public abstract class SSLConfig { return engine; } + abstract Configuration getHadoopConfig(); + + String getPassword(String hadoopName) { + String value = null; + if (getHadoopConfig() != null) { + try { + char[] password = getHadoopConfig().getPassword(hadoopName); + if (password != null) { + value = String.valueOf(password); + } + } catch (IOException e) { + logger.warn("Unable to obtain password {} from CredentialProvider API: {}", hadoopName, e.getMessage()); + // fallthrough + } + } + return value; + } + + String resolveHadoopPropertyName(String nameTemplate, Mode mode) { + return MessageFormat.format(nameTemplate, mode.toString().toLowerCase()); + } + @Override public String toString() { StringBuilder sb = new StringBuilder(); diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/ssl/SSLConfigBuilder.java b/exec/java-exec/src/main/java/org/apache/drill/exec/ssl/SSLConfigBuilder.java index 7571e6b..f11b04b 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/ssl/SSLConfigBuilder.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/ssl/SSLConfigBuilder.java @@ -42,12 +42,12 @@ public class SSLConfigBuilder { if (mode == SSLConfig.Mode.SERVER) { sslConfig = new SSLConfigServer(config, hadoopConfig); } else { - sslConfig = new SSLConfigClient(properties); + sslConfig = new SSLConfigClient(properties, hadoopConfig); } - if(validateKeyStore){ + if (validateKeyStore) { sslConfig.validateKeyStore(); } - if(initializeSSLContext){ + if (initializeSSLContext) { sslConfig.initContext(); } return sslConfig; diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/ssl/SSLConfigClient.java b/exec/java-exec/src/main/java/org/apache/drill/exec/ssl/SSLConfigClient.java index 417f5ae..3b4f12b 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/ssl/SSLConfigClient.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/ssl/SSLConfigClient.java @@ -23,6 +23,9 @@ import io.netty.handler.ssl.SslProvider; import org.apache.drill.common.config.DrillProperties; import org.apache.drill.common.exceptions.DrillException; import org.apache.drill.exec.memory.BufferAllocator; +import org.apache.hadoop.conf.Configuration; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import javax.net.ssl.SSLContext; import javax.net.ssl.SSLEngine; @@ -32,9 +35,10 @@ import java.util.Properties; public class SSLConfigClient extends SSLConfig { - private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(SSLConfigClient.class); + private static final Logger logger = LoggerFactory.getLogger(SSLConfigClient.class); private final Properties properties; + private final Configuration hadoopConfig; private final boolean userSslEnabled; private final String trustStoreType; private final String trustStorePath; @@ -46,19 +50,22 @@ public class SSLConfigClient extends SSLConfig { private final int handshakeTimeout; private final String provider; - private final String emptyString = new String(); + private final String emptyString = ""; - public SSLConfigClient(Properties properties) throws DrillException { + public SSLConfigClient(Properties properties, Configuration hadoopConfig) throws DrillException { this.properties = properties; + this.hadoopConfig = hadoopConfig; userSslEnabled = getBooleanProperty(DrillProperties.ENABLE_TLS); SSLCredentialsProvider credentialsProvider = SSLCredentialsProvider.getSSLCredentialsProvider( this::getStringProperty, - Mode.CLIENT, + this::getPasswordStringProperty, + getMode(), getBooleanProperty(DrillProperties.USE_MAPR_SSL_CONFIG) ); trustStoreType = credentialsProvider.getTrustStoreType(DrillProperties.TRUSTSTORE_TYPE, "JKS"); trustStorePath = credentialsProvider.getTrustStoreLocation(DrillProperties.TRUSTSTORE_PATH, ""); - trustStorePassword = credentialsProvider.getTrustStorePassword(DrillProperties.TRUSTSTORE_PASSWORD, ""); + trustStorePassword = credentialsProvider.getTrustStorePassword(DrillProperties.TRUSTSTORE_PASSWORD, + resolveHadoopPropertyName(HADOOP_SSL_TRUSTSTORE_PASSWORD_TPL_KEY, getMode())); disableHostVerification = getBooleanProperty(DrillProperties.DISABLE_HOST_VERIFICATION); disableCertificateVerification = getBooleanProperty(DrillProperties.DISABLE_CERT_VERIFICATION); useSystemTrustStore = getBooleanProperty(DrillProperties.USE_SYSTEM_TRUSTSTORE); @@ -96,6 +103,15 @@ public class SSLConfigClient extends SSLConfig { return value; } + private String getPasswordStringProperty(String name, String hadoopName) { + String value = getPassword(hadoopName); + + if (value == null) { + value = getStringProperty(name, ""); + } + return value; + } + private int getIntProperty(String name, int defaultValue) { int value = defaultValue; if (properties != null) { @@ -107,8 +123,8 @@ public class SSLConfigClient extends SSLConfig { return value; } + @Override public void validateKeyStore() throws DrillException { - } @Override @@ -279,8 +295,13 @@ public class SSLConfigClient extends SSLConfig { return useSystemTrustStore; } + @Override public boolean isSslValid() { return true; } + @Override + Configuration getHadoopConfig() { + return hadoopConfig; + } } diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/ssl/SSLConfigServer.java b/exec/java-exec/src/main/java/org/apache/drill/exec/ssl/SSLConfigServer.java index 04fb3b6..e184a91 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/ssl/SSLConfigServer.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/ssl/SSLConfigServer.java @@ -26,16 +26,17 @@ import org.apache.drill.common.exceptions.DrillException; import org.apache.drill.exec.ExecConstants; import org.apache.drill.exec.memory.BufferAllocator; import org.apache.hadoop.conf.Configuration; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import javax.net.ssl.KeyManagerFactory; import javax.net.ssl.SSLContext; import javax.net.ssl.SSLEngine; import javax.net.ssl.TrustManagerFactory; -import java.text.MessageFormat; public class SSLConfigServer extends SSLConfig { - private static final org.slf4j.Logger logger = org.slf4j.LoggerFactory.getLogger(SSLConfigServer.class); + private static final Logger logger = LoggerFactory.getLogger(SSLConfigServer.class); private final DrillConfig config; private final Configuration hadoopConfig; @@ -76,6 +77,7 @@ public class SSLConfigServer extends SSLConfig { config.hasPath(ExecConstants.USER_SSL_ENABLED) && config.getBoolean(ExecConstants.USER_SSL_ENABLED); SSLCredentialsProvider credentialsProvider = SSLCredentialsProvider.getSSLCredentialsProvider( this::getConfigParam, + this::getPasswordConfigParam, Mode.SERVER, config.getBoolean(ExecConstants.SSL_USE_MAPR_CONFIG)); trustStoreType = credentialsProvider.getTrustStoreType( @@ -106,6 +108,7 @@ public class SSLConfigServer extends SSLConfig { provider = config.getString(ExecConstants.SSL_PROVIDER); } + @Override public void validateKeyStore() throws DrillException { //HTTPS validates the keystore is not empty. User Server SSL context initialization also validates keystore, but // much more strictly. User Client context initialization does not validate keystore. @@ -226,11 +229,15 @@ public class SSLConfigServer extends SSLConfig { return value; } - private String resolveHadoopPropertyName(String nameTemplate, Mode mode) { - return MessageFormat.format(nameTemplate, mode.toString().toLowerCase()); - } + private String getPasswordConfigParam(String name, String hadoopName) { + String value = getPassword(hadoopName); + if (value == null) { + value = getConfigParam(name, hadoopName); + } + return value; + } @Override public boolean isUserSslEnabled() { @@ -322,8 +329,13 @@ public class SSLConfigServer extends SSLConfig { return false; // Client only, notsupported by the server } + @Override public boolean isSslValid() { return !keyStorePath.isEmpty() && !keyStorePassword.isEmpty(); } + @Override + Configuration getHadoopConfig() { + return hadoopConfig; + } } diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/ssl/SSLCredentialsProvider.java b/exec/java-exec/src/main/java/org/apache/drill/exec/ssl/SSLCredentialsProvider.java index fd536f6..e2d0ced 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/ssl/SSLCredentialsProvider.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/ssl/SSLCredentialsProvider.java @@ -43,14 +43,19 @@ abstract class SSLCredentialsProvider { * <li>String parameter2 - default value</li> * <li>returns the property value or default value</li> * </ul> + * @param getPasswordPropertyMethod the same as {@code getPropertyMethod} but used to + * retrieve sensible data, such as keystore password, + * using Hadoop's CredentialProvider API with fallback + * to standard means as is used for {@code getPropertyMethod} * @param mode CLIENT or SERVER * @param useMapRSSLConfig use a MapR credential provider * @return concrete implementation of SSLCredentialsProvider */ - static SSLCredentialsProvider getSSLCredentialsProvider(BiFunction<String, String, String> getPropertyMethod, SSLConfig.Mode mode, boolean useMapRSSLConfig) { + static SSLCredentialsProvider getSSLCredentialsProvider(BiFunction<String, String, String> getPropertyMethod, + BiFunction<String, String, String> getPasswordPropertyMethod, SSLConfig.Mode mode, boolean useMapRSSLConfig) { return useMapRSSLConfig ? getMaprCredentialsProvider(mode) - .orElseGet(() -> new SSLCredentialsProviderImpl(getPropertyMethod)) : - new SSLCredentialsProviderImpl(getPropertyMethod); + .orElseGet(() -> new SSLCredentialsProviderImpl(getPropertyMethod, getPasswordPropertyMethod)) : + new SSLCredentialsProviderImpl(getPropertyMethod, getPasswordPropertyMethod); } private static Optional<SSLCredentialsProvider> getMaprCredentialsProvider(SSLConfig.Mode mode) { @@ -95,9 +100,12 @@ abstract class SSLCredentialsProvider { private static class SSLCredentialsProviderImpl extends SSLCredentialsProvider { private final BiFunction<String, String, String> getPropertyMethod; + private final BiFunction<String, String, String> getPasswordPropertyMethod; - private SSLCredentialsProviderImpl(BiFunction<String, String, String> getPropertyMethod) { + private SSLCredentialsProviderImpl(BiFunction<String, String, String> getPropertyMethod, + BiFunction<String, String, String> getPasswordPropertyMethod) { this.getPropertyMethod = getPropertyMethod; + this.getPasswordPropertyMethod = getPasswordPropertyMethod; } @Override @@ -112,7 +120,7 @@ abstract class SSLCredentialsProvider { @Override String getTrustStorePassword(String propertyName, String defaultValue) { - return getPropertyMethod.apply(propertyName, defaultValue); + return getPasswordPropertyMethod.apply(propertyName, defaultValue); } @Override @@ -127,12 +135,12 @@ abstract class SSLCredentialsProvider { @Override String getKeyStorePassword(String propertyName, String defaultValue) { - return getPropertyMethod.apply(propertyName, defaultValue); + return getPasswordPropertyMethod.apply(propertyName, defaultValue); } @Override String getKeyPassword(String propertyName, String defaultValue) { - return getPropertyMethod.apply(propertyName, defaultValue); + return getPasswordPropertyMethod.apply(propertyName, defaultValue); } } }