diff --git a/data/files/keystore.jks b/data/files/keystore.jks index 469d8a5..8f35af0 100644 Binary files a/data/files/keystore.jks and b/data/files/keystore.jks differ diff --git a/data/files/keystore_exampledotcom.jks b/data/files/keystore_exampledotcom.jks new file mode 100644 index 0000000..1d33aad Binary files /dev/null and b/data/files/keystore_exampledotcom.jks differ diff --git a/data/files/truststore.jks b/data/files/truststore.jks index 9c5d703..03dd2a3 100644 Binary files a/data/files/truststore.jks and b/data/files/truststore.jks differ diff --git a/itests/hive-unit/src/main/java/org/apache/hive/jdbc/miniHS2/MiniHS2.java b/itests/hive-unit/src/main/java/org/apache/hive/jdbc/miniHS2/MiniHS2.java index 9ab5566..de1ce76 100644 --- a/itests/hive-unit/src/main/java/org/apache/hive/jdbc/miniHS2/MiniHS2.java +++ b/itests/hive-unit/src/main/java/org/apache/hive/jdbc/miniHS2/MiniHS2.java @@ -182,6 +182,8 @@ public boolean isUseMiniKdc() { private MiniHS2(HiveConf hiveConf, MiniClusterType miniClusterType, boolean useMiniKdc, String serverPrincipal, String serverKeytab, boolean isMetastoreRemote, boolean usePortsFromConf, String authType, boolean isHA) throws Exception { + // Always use localhost for hostname as some tests like SSL CN validation ones + // are tied to localhost being present in the certificate name super(hiveConf, "localhost", (usePortsFromConf ? hiveConf.getIntVar(HiveConf.ConfVars.HIVE_SERVER2_THRIFT_PORT) : MetaStoreUtils.findFreePort()), (usePortsFromConf ? hiveConf.getIntVar(HiveConf.ConfVars.HIVE_SERVER2_THRIFT_HTTP_PORT) : MetaStoreUtils.findFreePort())); @@ -382,29 +384,38 @@ public String getJdbcURL(String dbName, String sessionConfExt) throws Exception * @return * @throws Exception */ - public String getJdbcURL(String dbName, String sessionConfExt, String hiveConfExt) throws Exception { + public String getJdbcURL(String dbName, String sessionConfExt, String hiveConfExt) + throws Exception { sessionConfExt = (sessionConfExt == null ? "" : sessionConfExt); hiveConfExt = (hiveConfExt == null ? "" : hiveConfExt); - String krbConfig = ""; + // Strip the leading ";" if provided + // (this is the assumption with which we're going to start configuring sessionConfExt) + if (sessionConfExt.startsWith(";")) { + sessionConfExt = sessionConfExt.substring(1); + } if (isUseMiniKdc()) { - krbConfig = "principal=" + serverPrincipal; + sessionConfExt = "principal=" + serverPrincipal + ";" + sessionConfExt; } if (isHttpTransportMode()) { - sessionConfExt = "transportMode=http;httpPath=cliservice;" + sessionConfExt; + sessionConfExt = "transportMode=http;httpPath=cliservice" + ";" + sessionConfExt; } String baseJdbcURL; if (isDynamicServiceDiscovery()) { - String serviceDiscoveryConfig = + sessionConfExt = "serviceDiscoveryMode=zooKeeper;zooKeeperNamespace=" - + getServerConf().getVar(HiveConf.ConfVars.HIVE_SERVER2_ZOOKEEPER_NAMESPACE) + ";"; - baseJdbcURL = getZKBaseJdbcURL() + dbName + ";" + serviceDiscoveryConfig; + + getServerConf().getVar(HiveConf.ConfVars.HIVE_SERVER2_ZOOKEEPER_NAMESPACE) + ";" + + sessionConfExt; + baseJdbcURL = getZKBaseJdbcURL(); + } else { + baseJdbcURL = getBaseJdbcURL(); } - else { - baseJdbcURL = getBaseJdbcURL() + dbName + ";"; + + baseJdbcURL = baseJdbcURL + dbName; + if (!sessionConfExt.isEmpty()) { + baseJdbcURL = baseJdbcURL + ";" + sessionConfExt; } - baseJdbcURL = baseJdbcURL + krbConfig + ";" + sessionConfExt; - if (!hiveConfExt.trim().equals("")) { - baseJdbcURL = "?" + hiveConfExt; + if ((hiveConfExt != null) && (!hiveConfExt.trim().isEmpty())) { + baseJdbcURL = baseJdbcURL + "?" + hiveConfExt; } return baseJdbcURL; } diff --git a/itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestSSL.java b/itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestSSL.java index ea9acba..2f4db0d 100644 --- a/itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestSSL.java +++ b/itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestSSL.java @@ -18,6 +18,7 @@ package org.apache.hive.jdbc; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import java.io.File; @@ -47,9 +48,10 @@ public class TestSSL { private static final Logger LOG = LoggerFactory.getLogger(TestSSL.class); - private static final String KEY_STORE_NAME = "keystore.jks"; + private static final String LOCALHOST_KEY_STORE_NAME = "keystore.jks"; + private static final String EXAMPLEDOTCOM_KEY_STORE_NAME = "keystore_exampledotcom.jks"; private static final String TRUST_STORE_NAME = "truststore.jks"; - private static final String KEY_STORE_PASSWORD = "HiveJdbc"; + private static final String KEY_STORE_TRUST_STORE_PASSWORD = "HiveJdbc"; private static final String JAVA_TRUST_STORE_PROP = "javax.net.ssl.trustStore"; private static final String JAVA_TRUST_STORE_PASS_PROP = "javax.net.ssl.trustStorePassword"; private static final String HS2_BINARY_MODE = "binary"; @@ -62,9 +64,9 @@ private Connection hs2Conn = null; private String dataFileDir = conf.get("test.data.files"); private Map confOverlay; - private final String SSL_CONN_PARAMS = ";ssl=true;sslTrustStore=" + private final String SSL_CONN_PARAMS = "ssl=true;sslTrustStore=" + URLEncoder.encode(dataFileDir + File.separator + TRUST_STORE_NAME) + ";trustStorePassword=" - + KEY_STORE_PASSWORD; + + KEY_STORE_TRUST_STORE_PASSWORD; @BeforeClass public static void beforeTest() throws Exception { @@ -127,7 +129,7 @@ public void testSSLVersion() throws Exception { // make SSL connection hs2Conn = DriverManager.getConnection(miniHS2.getJdbcURL() + ";ssl=true;sslTrustStore=" + dataFileDir - + File.separator + TRUST_STORE_NAME + ";trustStorePassword=" + KEY_STORE_PASSWORD, + + File.separator + TRUST_STORE_NAME + ";trustStorePassword=" + KEY_STORE_TRUST_STORE_PASSWORD, System.getProperty("user.name"), "bar"); hs2Conn.close(); Assert.assertEquals("Expected exit code of 1", 1, execCommand("openssl s_client -connect " @@ -144,7 +146,7 @@ public void testSSLVersion() throws Exception { hs2Conn = DriverManager.getConnection(miniHS2.getJdbcURL() + ";ssl=true;sslTrustStore=" + dataFileDir + File.separator + TRUST_STORE_NAME + ";trustStorePassword=" - + KEY_STORE_PASSWORD, System.getProperty("user.name"), "bar"); + + KEY_STORE_TRUST_STORE_PASSWORD, System.getProperty("user.name"), "bar"); Assert.fail("Expected SQLException during connect"); } catch (SQLException e) { LOG.info("Expected exception: " + e, e); @@ -181,7 +183,7 @@ public void testInvalidConfig() throws Exception { } System.setProperty(JAVA_TRUST_STORE_PROP, dataFileDir + File.separator + TRUST_STORE_NAME ); - System.setProperty(JAVA_TRUST_STORE_PASS_PROP, KEY_STORE_PASSWORD); + System.setProperty(JAVA_TRUST_STORE_PASS_PROP, KEY_STORE_TRUST_STORE_PASSWORD); try { hs2Conn = DriverManager.getConnection(miniHS2.getJdbcURL() + ";ssl=true", System.getProperty("user.name"), "bar"); @@ -291,7 +293,7 @@ public void testSSLConnectionWithProperty() throws Exception { miniHS2.start(confOverlay); System.setProperty(JAVA_TRUST_STORE_PROP, dataFileDir + File.separator + TRUST_STORE_NAME ); - System.setProperty(JAVA_TRUST_STORE_PASS_PROP, KEY_STORE_PASSWORD); + System.setProperty(JAVA_TRUST_STORE_PASS_PROP, KEY_STORE_TRUST_STORE_PASSWORD); // make SSL connection hs2Conn = DriverManager.getConnection(miniHS2.getJdbcURL() + ";ssl=true", System.getProperty("user.name"), "bar"); @@ -375,6 +377,55 @@ public void testSSLFetchHttp() throws Exception { hs2Conn.close(); } + /*** + * Test a new connection when server sends a certificate with wrong CN + * (sends a certificate for www.example.com instead of localhost) + * Opening a new connection with this wrong certificate should fail + * @throws Exception + */ + @Test + public void testConnectionWrongCertCN() throws Exception { + // This call sets the default ssl params including the correct keystore in the server config + setSslConfOverlay(confOverlay); + // Replace default keystore with keystore for www.example.com + confOverlay.put(ConfVars.HIVE_SERVER2_SSL_KEYSTORE_PATH.varname, dataFileDir + File.separator + + EXAMPLEDOTCOM_KEY_STORE_NAME); + // Binary (TCP) mode + setBinaryConfOverlay(confOverlay); + miniHS2.start(confOverlay); + try { + hs2Conn = + DriverManager.getConnection(miniHS2.getJdbcURL("default", SSL_CONN_PARAMS), + System.getProperty("user.name"), "bar"); + fail("SSL connection, with the server providing wrong certifcate (with CN www.example.com, " + + "instead of localhost), should fail"); + } catch (SQLException e) { + // Expected error: should throw java.security.cert.CertificateException + assertEquals("08S01", e.getSQLState().trim()); + assertTrue(e.toString().contains("java.security.cert.CertificateException")); + } + + miniHS2.stop(); + + // Http mode + setHttpConfOverlay(confOverlay); + miniHS2.start(confOverlay); + try { + hs2Conn = + DriverManager.getConnection(miniHS2.getJdbcURL("default", SSL_CONN_PARAMS), + System.getProperty("user.name"), "bar"); + fail("SSL connection, with the server providing wrong certifcate (with CN www.example.com, " + + "instead of localhost), should fail"); + } catch (SQLException e) { + // Expected error: should throw javax.net.ssl.SSLPeerUnverifiedException + assertEquals("08S01", e.getSQLState().trim()); + assertTrue(e.toString().contains("javax.net.ssl.SSLPeerUnverifiedException")); + } + // Revert to default keystore path + confOverlay.put(ConfVars.HIVE_SERVER2_SSL_KEYSTORE_PATH.varname, dataFileDir + File.separator + + LOCALHOST_KEY_STORE_NAME); + } + private void setupTestTableWithData(String tableName, Path dataFilePath, Connection hs2Conn) throws Exception { Statement stmt = hs2Conn.createStatement(); @@ -393,9 +444,9 @@ private void setupTestTableWithData(String tableName, Path dataFilePath, private void setSslConfOverlay(Map confOverlay) { confOverlay.put(ConfVars.HIVE_SERVER2_USE_SSL.varname, "true"); confOverlay.put(ConfVars.HIVE_SERVER2_SSL_KEYSTORE_PATH.varname, - dataFileDir + File.separator + KEY_STORE_NAME); + dataFileDir + File.separator + LOCALHOST_KEY_STORE_NAME); confOverlay.put(ConfVars.HIVE_SERVER2_SSL_KEYSTORE_PASSWORD.varname, - KEY_STORE_PASSWORD); + KEY_STORE_TRUST_STORE_PASSWORD); } private void clearSslConfOverlay(Map confOverlay) { diff --git a/jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java b/jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java index 40ad3b2..0b0db43 100644 --- a/jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java +++ b/jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java @@ -44,13 +44,15 @@ import org.apache.http.config.Registry; import org.apache.http.config.RegistryBuilder; import org.apache.http.conn.socket.ConnectionSocketFactory; -import org.apache.http.conn.ssl.SSLSocketFactory; +import org.apache.http.conn.ssl.DefaultHostnameVerifier; +import org.apache.http.conn.ssl.SSLConnectionSocketFactory; import org.apache.http.impl.client.BasicCookieStore; import org.apache.http.impl.client.CloseableHttpClient; import org.apache.http.impl.client.HttpClientBuilder; import org.apache.http.impl.client.HttpClients; import org.apache.http.impl.conn.BasicHttpClientConnectionManager; import org.apache.http.protocol.HttpContext; +import org.apache.http.ssl.SSLContexts; import org.apache.thrift.TException; import org.apache.thrift.protocol.TBinaryProtocol; import org.apache.thrift.transport.THttpClient; @@ -324,30 +326,29 @@ private CloseableHttpClient getHttpClient(Boolean useSsl) throws SQLException { if (isCookieEnabled) { // Create a http client with a retry mechanism when the server returns a status code of 401. httpClientBuilder = - HttpClients.custom().setServiceUnavailableRetryStrategy( - new ServiceUnavailableRetryStrategy() { - - @Override - public boolean retryRequest( - final HttpResponse response, - final int executionCount, - final HttpContext context) { - int statusCode = response.getStatusLine().getStatusCode(); - boolean ret = statusCode == 401 && executionCount <= 1; - - // Set the context attribute to true which will be interpreted by the request interceptor - if (ret) { - context.setAttribute(Utils.HIVE_SERVER2_RETRY_KEY, Utils.HIVE_SERVER2_RETRY_TRUE); - } - return ret; - } - - @Override - public long getRetryInterval() { - // Immediate retry - return 0; - } - }); + HttpClients.custom().setServiceUnavailableRetryStrategy( + new ServiceUnavailableRetryStrategy() { + @Override + public boolean retryRequest(final HttpResponse response, final int executionCount, + final HttpContext context) { + int statusCode = response.getStatusLine().getStatusCode(); + boolean ret = statusCode == 401 && executionCount <= 1; + + // Set the context attribute to true which will be interpreted by the request + // interceptor + if (ret) { + context.setAttribute(Utils.HIVE_SERVER2_RETRY_KEY, + Utils.HIVE_SERVER2_RETRY_TRUE); + } + return ret; + } + + @Override + public long getRetryInterval() { + // Immediate retry + return 0; + } + }); } else { httpClientBuilder = HttpClientBuilder.create(); } @@ -360,47 +361,37 @@ public long getRetryInterval() { String sslTrustStorePassword = sessConfMap.get( JdbcConnectionParams.SSL_TRUST_STORE_PASSWORD); KeyStore sslTrustStore; - SSLSocketFactory socketFactory; - + SSLConnectionSocketFactory socketFactory; + SSLContext sslContext; /** - * The code within the try block throws: - * 1. SSLInitializationException - * 2. KeyStoreException - * 3. IOException - * 4. NoSuchAlgorithmException - * 5. CertificateException - * 6. KeyManagementException - * 7. UnrecoverableKeyException - * We don't want the client to retry on any of these, hence we catch all - * and throw a SQLException. + * The code within the try block throws: SSLInitializationException, KeyStoreException, + * IOException, NoSuchAlgorithmException, CertificateException, KeyManagementException & + * UnrecoverableKeyException. We don't want the client to retry on any of these, + * hence we catch all and throw a SQLException. */ try { - if (useTwoWaySSL != null && - useTwoWaySSL.equalsIgnoreCase(JdbcConnectionParams.TRUE)) { + if (useTwoWaySSL != null && useTwoWaySSL.equalsIgnoreCase(JdbcConnectionParams.TRUE)) { socketFactory = getTwoWaySSLSocketFactory(); } else if (sslTrustStorePath == null || sslTrustStorePath.isEmpty()) { // Create a default socket factory based on standard JSSE trust material - socketFactory = SSLSocketFactory.getSocketFactory(); + socketFactory = SSLConnectionSocketFactory.getSocketFactory(); } else { // Pick trust store config from the given path sslTrustStore = KeyStore.getInstance(JdbcConnectionParams.SSL_TRUST_STORE_TYPE); try (FileInputStream fis = new FileInputStream(sslTrustStorePath)) { sslTrustStore.load(fis, sslTrustStorePassword.toCharArray()); } - socketFactory = new SSLSocketFactory(sslTrustStore); + sslContext = SSLContexts.custom().loadTrustMaterial(sslTrustStore, null).build(); + socketFactory = + new SSLConnectionSocketFactory(sslContext, new DefaultHostnameVerifier(null)); } - socketFactory.setHostnameVerifier(SSLSocketFactory.ALLOW_ALL_HOSTNAME_VERIFIER); - final Registry registry = - RegistryBuilder.create() - .register("https", socketFactory) - .build(); - + RegistryBuilder. create().register("https", socketFactory) + .build(); httpClientBuilder.setConnectionManager(new BasicHttpClientConnectionManager(registry)); - } - catch (Exception e) { - String msg = "Could not create an https connection to " + - jdbcUriString + ". " + e.getMessage(); + } catch (Exception e) { + String msg = + "Could not create an https connection to " + jdbcUriString + ". " + e.getMessage(); throw new SQLException(msg, " 08S01", e); } } @@ -502,8 +493,8 @@ private TTransport createBinaryTransport() throws SQLException, TTransportExcept return transport; } - SSLSocketFactory getTwoWaySSLSocketFactory() throws SQLException { - SSLSocketFactory socketFactory = null; + SSLConnectionSocketFactory getTwoWaySSLSocketFactory() throws SQLException { + SSLConnectionSocketFactory socketFactory = null; try { KeyManagerFactory keyManagerFactory = KeyManagerFactory.getInstance( @@ -540,7 +531,7 @@ SSLSocketFactory getTwoWaySSLSocketFactory() throws SQLException { SSLContext context = SSLContext.getInstance("TLS"); context.init(keyManagerFactory.getKeyManagers(), trustManagerFactory.getTrustManagers(), new SecureRandom()); - socketFactory = new SSLSocketFactory(context); + socketFactory = new SSLConnectionSocketFactory(context); } catch (Exception e) { throw new SQLException("Error while initializing 2 way ssl socket factory ", e); } diff --git a/service/src/java/org/apache/hive/service/auth/HiveAuthFactory.java b/service/src/java/org/apache/hive/service/auth/HiveAuthFactory.java index 8af9d0a..ab8806c 100644 --- a/service/src/java/org/apache/hive/service/auth/HiveAuthFactory.java +++ b/service/src/java/org/apache/hive/service/auth/HiveAuthFactory.java @@ -29,7 +29,9 @@ import java.util.List; import java.util.Map; +import javax.net.ssl.SSLParameters; import javax.net.ssl.SSLServerSocket; +import javax.net.ssl.SSLSocket; import javax.security.auth.login.LoginException; import javax.security.sasl.AuthenticationException; import javax.security.sasl.Sasl; @@ -258,7 +260,9 @@ public static TTransport getSocketTransport(String host, int port, int loginTime public static TTransport getSSLSocket(String host, int port, int loginTimeout) throws TTransportException { - return TSSLTransportFactory.getClientSocket(host, port, loginTimeout); + // The underlying SSLSocket object is bound to host:port with the given SO_TIMEOUT + TSocket tSSLSocket = TSSLTransportFactory.getClientSocket(host, port, loginTimeout); + return getSSLSocketWithHttps(tSSLSocket); } public static TTransport getSSLSocket(String host, int port, int loginTimeout, @@ -267,7 +271,20 @@ public static TTransport getSSLSocket(String host, int port, int loginTimeout, new TSSLTransportFactory.TSSLTransportParameters(); params.setTrustStore(trustStorePath, trustStorePassWord); params.requireClientAuth(true); - return TSSLTransportFactory.getClientSocket(host, port, loginTimeout, params); + // The underlying SSLSocket object is bound to host:port with the given SO_TIMEOUT and + // SSLContext created with the given params + TSocket tSSLSocket = TSSLTransportFactory.getClientSocket(host, port, loginTimeout, params); + return getSSLSocketWithHttps(tSSLSocket); + } + + // Using endpoint identification algorithm as HTTPS enables us to do + // CNAMEs/subjectAltName verification + private static TSocket getSSLSocketWithHttps(TSocket tSSLSocket) throws TTransportException { + SSLSocket sslSocket = (SSLSocket) tSSLSocket.getSocket(); + SSLParameters sslParams = sslSocket.getSSLParameters(); + sslParams.setEndpointIdentificationAlgorithm("HTTPS"); + sslSocket.setSSLParameters(sslParams); + return new TSocket(sslSocket); } public static TServerSocket getServerSocket(String hiveHost, int portNum)