HIVE-27308: Avoid exposing client keystore and truststore passwords in the JDBC URL #4282
Conversation
kgyrtkirk
added
tests pending
tests unstable
tests passed
and removed
tests pending
tests unstable
labels
VenuReddy2103
changed the title
|
@saihemanth-cloudera @dengzhhu653 Could you please help review this PR |
| @@ -1008,6 +1016,10 @@ SSLConnectionSocketFactory getTwoWaySSLSocketFactory() throws SQLException { | |||
| JdbcConnectionParams.SUNJSSE_ALGORITHM_STRING); | |||
| String keyStorePath = sessConfMap.get(JdbcConnectionParams.SSL_KEY_STORE); | |||
| String keyStorePassword = sessConfMap.get(JdbcConnectionParams.SSL_KEY_STORE_PASSWORD); | |||
| if (keyStorePassword == null) { | |||
| keyStorePassword = Utils.getPasswordFromCredentialProvider( | |||
There was a problem hiding this comment.
Seems like we load all credentials from the same creds file, is this by design?
And we can add some comment how to use the key to generate the local creds, looks like we use JdbcConnectionParams.SSL_KEY_STORE_PASSWORD as the key.
There was a problem hiding this comment.
Seems like we load all credentials from the same creds file, is this by design?
And we can add some comment how to use the
keyto generate the local creds, looks like we useJdbcConnectionParams.SSL_KEY_STORE_PASSWORDas the key.
Yes. We store all the passwords in the same file. Will document about the generation and usage in cwiki https://cwiki.apache.org/confluence/display/Hive/HiveServer2+Clients
There was a problem hiding this comment.
Can we move all these three types of passwords to a different API? This API can take in the
String keyas a parameter and return a string password. You can then call the API at lines #900, #1018, and #1037.
Done. Added an API in Utils.java that takes map and key as arguments. First tries to find the password from map. If not found, then gets it from credential provider.
| public static String getPasswordFromCredentialProvider(String providerPath, String key) { | ||
| try { | ||
| if (providerPath != null) { | ||
| Configuration conf = new Configuration(); |
There was a problem hiding this comment.
Instead of creating a new config, can we reuse the setHiveConfs() L#326-#328?
There was a problem hiding this comment.
Used org.apache.hadoop.conf.Configuration object because Configuration.getPassword resolve the key as an alias through the CredentialProvider API internally. So we can avoid using the CredentialProvider APIs to fetch password explicitly in our code.
|
Kudos, SonarCloud Quality Gate passed!
|
…n the JDBC URL (apache#4282) (Venu Reddy, reviewed by Zhihua Deng and Sai Hemanth Gantasala)
…n the JDBC URL (apache#4282) (Venu Reddy, reviewed by Zhihua Deng and Sai Hemanth Gantasala)
What changes were proposed in this pull request?
Added a new property
storePasswordPathto JDBC URL that point to the local JCE keystore file storing the password aliases. When an existing password property is present in URL, ignores to fetch that particular alias from local jceks(i.e., giving preference to existing password property). And if password property is not present in URL, fetches the password from local jceks file specified instorePasswordPathproperty. Hive JDBC can obtains the passwords with Configuration.getPassword API to read the password from jceks file.JDBC URL would look like -
beeline -u "jdbc:hive2://kvr-host:10001/default;retries=5;ssl=true;sslTrustStore=/tmp/truststore.jks;transportMode=http;httpPath=cliservice;twoWay=true;sslKeyStore=/tmp/keystore.jks;storePasswordPath=localjceks://file/tmp/client_creds.jceks"Why are the changes needed?
At present, we may have
trustStorePassword, keyStorePassword, zooKeeperTruststorePassword, zooKeeperKeystorePasswordpasswords in the JDBC URL. Exposing these passwords in URL can be a security concern. We can hide all these passwords from JDBC URL when we protect these passwords in a local JCEKS keystore file and pass the JCEKS file path to URL instead.Does this PR introduce any user-facing change?
Optional
storePasswordPathproperty is supported in JDBC URL. ExistingtrustStorePassword, keyStorePassword, zooKeeperTruststorePassword, zooKeeperKeystorePasswordproperties continue to exist and are supported in JDBC URL without any change in their behavior. When both password(s) and storePasswordPath properties are present in URL, password(s) property is preferred.storePasswordPathproperty is effective only when password(s) property is not in JDBC URL.How was this patch tested?
Tested manually