From 12c8fb54ff4b39c3cfdb107d96e53c53b6ecc814 Mon Sep 17 00:00:00 2001 From: Madhav Sainanee Date: Sat, 17 May 2025 11:10:38 +0000 Subject: [PATCH 01/11] Add support for SSL client certificate authentication - Implemented client certificate authentication via keystore configuration parameters. - Updated `ConfiguratorUtils` to handle loading and managing keystores. - Enhanced unit tests to cover keystore loading and client certificate scenarios. --- NEXT_CHANGELOG.md | 1 + .../impl/common/ConfiguratorUtils.java | 175 +++++++++++++++++- .../impl/common/ConfiguratorUtilsTest.java | 156 ++++++++++++++-- 3 files changed, 311 insertions(+), 21 deletions(-) diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index 43cce78527..a932e6421f 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -6,6 +6,7 @@ - Support for fetching tables and views across all catalogs using SHOW TABLES FROM/IN ALL CATALOGS in the SQL Exec API. - Support for Token Exchange in OAuth flows where in third party tokens are exchanged for InHouse tokens. - Added support for polling of statementStatus and sqlState for async SQL execution. +- Added support for SSL client certificate authentication via keystore configuration parameters: SSLKeyStore, SSLKeyStorePwd, SSLKeyStoreType, and SSLKeyStoreProvider. ### Updated - diff --git a/src/main/java/com/databricks/jdbc/dbclient/impl/common/ConfiguratorUtils.java b/src/main/java/com/databricks/jdbc/dbclient/impl/common/ConfiguratorUtils.java index a88c8fda78..c7be031cfe 100644 --- a/src/main/java/com/databricks/jdbc/dbclient/impl/common/ConfiguratorUtils.java +++ b/src/main/java/com/databricks/jdbc/dbclient/impl/common/ConfiguratorUtils.java @@ -16,6 +16,7 @@ import java.security.cert.*; import java.util.Arrays; import java.util.Collections; +import java.util.Enumeration; import java.util.Set; import java.util.stream.Collectors; import javax.net.ssl.*; @@ -242,6 +243,15 @@ private static Registry createRegistryWithJdkDefaultTru return null; } + /** + * Creates a socket factory registry from trust anchors and client keystore if available. + * + * @param trustAnchors The trust anchors for server certificate validation. + * @param connectionContext The connection context for configuration. + * @param sourceDescription A description of the trust store source for logging. + * @return A registry of connection socket factories. + * @throws DatabricksHttpException If there is an error during setup. + */ private static Registry createRegistryFromTrustAnchors( Set trustAnchors, IDatabricksConnectionContext connectionContext, @@ -254,15 +264,35 @@ private static Registry createRegistryFromTrustAnchors( } try { + // Create trust managers for server certificate validation TrustManager[] trustManagers = createTrustManagers( trustAnchors, connectionContext.checkCertificateRevocation(), connectionContext.acceptUndeterminedCertificateRevocation()); - return createSocketFactoryRegistry(trustManagers); + // Load client certificate keystore if available + KeyStore keyStore = loadKeystoreOrNull(connectionContext); + + if (keyStore != null) { + LOGGER.info("Client certificate authentication enabled"); + + // Get password for the keystore + char[] keyStorePassword = null; + if (connectionContext.getSSLKeyStorePassword() != null) { + keyStorePassword = connectionContext.getSSLKeyStorePassword().toCharArray(); + } + + // Create key managers for client certificate authentication + javax.net.ssl.KeyManager[] keyManagers = createKeyManagers(keyStore, keyStorePassword); + + return createSocketFactoryRegistry(trustManagers, keyManagers); + } else { + LOGGER.info("No client keystore configured, server certificate validation only"); + return createSocketFactoryRegistry(trustManagers); + } } catch (Exception e) { - handleError("Error setting up trust managers for " + sourceDescription, e); + handleError("Error setting up SSL socket factory for " + sourceDescription, e); } return null; } @@ -277,9 +307,55 @@ private static Registry createRegistryFromTrustAnchors( */ private static Registry createSocketFactoryRegistry( TrustManager[] trustManagers) throws NoSuchAlgorithmException, KeyManagementException { + return createSocketFactoryRegistry(trustManagers, null); + } + + /** + * Creates key managers from the provided key store. + * + * @param keyStore The KeyStore containing client certificates and private keys. + * @param keyStorePassword The password for the key store. + * @return An array of key managers, or null if the key store is null. + * @throws NoSuchAlgorithmException If the algorithm for the key manager factory is not available. + * @throws KeyStoreException If there is an error accessing the key store. + * @throws DatabricksHttpException If there is an error creating the key managers. + */ + private static javax.net.ssl.KeyManager[] createKeyManagers( + KeyStore keyStore, char[] keyStorePassword) + throws NoSuchAlgorithmException, KeyStoreException, DatabricksHttpException { + if (keyStore == null) { + return null; + } + + try { + KeyManagerFactory kmf = + KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm()); + kmf.init(keyStore, keyStorePassword); + LOGGER.info("Successfully initialized key managers for client certificate authentication"); + return kmf.getKeyManagers(); + } catch (UnrecoverableKeyException e) { + String errorMessage = "Failed to initialize key managers: " + e.getMessage(); + LOGGER.error(errorMessage); + throw new DatabricksHttpException( + errorMessage, e, DatabricksDriverErrorCode.SSL_HANDSHAKE_ERROR); + } + } + + /** + * Creates a socket factory registry with the provided trust managers and key managers. + * + * @param trustManagers The trust managers to use. + * @param keyManagers The key managers to use for client authentication. + * @return A registry of connection socket factories. + * @throws NoSuchAlgorithmException If there is an error during SSL context creation. + * @throws KeyManagementException If there is an error during SSL context creation. + */ + private static Registry createSocketFactoryRegistry( + TrustManager[] trustManagers, javax.net.ssl.KeyManager[] keyManagers) + throws NoSuchAlgorithmException, KeyManagementException { SSLContext sslContext = SSLContext.getInstance(DatabricksJdbcConstants.TLS); - sslContext.init(null, trustManagers, null); + sslContext.init(keyManagers, trustManagers, new SecureRandom()); SSLConnectionSocketFactory sslSocketFactory = new SSLConnectionSocketFactory(sslContext); return RegistryBuilder.create() @@ -388,6 +464,99 @@ public static KeyStore loadTruststoreOrNull(IDatabricksConnectionContext connect } } + /** + * Loads a key store from the path specified in the connection context. The key store contains the + * client's private key and certificate for client authentication. + * + * @param connectionContext The connection context containing key store configuration. + * @return The loaded KeyStore or null if no key store was specified or it could not be loaded. + * @throws DatabricksHttpException If there is an error during loading. + */ + public static KeyStore loadKeystoreOrNull(IDatabricksConnectionContext connectionContext) + throws DatabricksHttpException { + String keyStorePath = connectionContext.getSSLKeyStore(); + if (keyStorePath == null) { + return null; + } + + // If the specified file doesn't exist, throw a specific error + File keyStoreFile = new File(keyStorePath); + if (!keyStoreFile.exists()) { + String errorMessage = "Specified key store file does not exist: " + keyStorePath; + LOGGER.error(errorMessage); + throw new DatabricksHttpException( + errorMessage, DatabricksDriverErrorCode.SSL_HANDSHAKE_ERROR); + } + + // Check if keystore password is provided, which is required for accessing private keys + String keyStorePassword = connectionContext.getSSLKeyStorePassword(); + if (keyStorePassword == null) { + String errorMessage = + "Key store password is required when a key store is specified: " + keyStorePath; + LOGGER.error(errorMessage); + throw new DatabricksHttpException( + errorMessage, DatabricksDriverErrorCode.SSL_HANDSHAKE_ERROR); + } + char[] password = keyStorePassword.toCharArray(); + + String keyStoreType = connectionContext.getSSLKeyStoreType(); + String keyStoreProvider = connectionContext.getSSLKeyStoreProvider(); + + try { + LOGGER.info("Loading key store as type: " + keyStoreType); + + // Create KeyStore instance, with provider if specified + KeyStore keyStore; + if (keyStoreProvider != null && !keyStoreProvider.isEmpty()) { + LOGGER.info("Using key store provider: " + keyStoreProvider); + keyStore = KeyStore.getInstance(keyStoreType, keyStoreProvider); + } else { + keyStore = KeyStore.getInstance(keyStoreType); + } + + // Load the KeyStore with password + try (FileInputStream keyStoreStream = new FileInputStream(keyStorePath)) { + keyStore.load(keyStoreStream, password); + } + + // Verify that the keystore contains at least one private key entry + boolean hasKeyEntry = false; + try { + for (Enumeration aliases = keyStore.aliases(); aliases.hasMoreElements(); ) { + String alias = aliases.nextElement(); + if (keyStore.isKeyEntry(alias)) { + hasKeyEntry = true; + LOGGER.debug("Found key entry with alias: " + alias); + break; + } + } + } catch (KeyStoreException e) { + // Log but don't fail - we'll still return the keystore + LOGGER.warn("Unable to verify key entries in keystore: " + e.getMessage()); + } + + if (!hasKeyEntry) { + LOGGER.warn( + "Key store does not contain any private key entries. " + + "Client authentication may fail."); + } + + LOGGER.info("Successfully loaded key store: " + keyStorePath); + return keyStore; + } catch (Exception e) { + String errorMessage = + "Failed to load key store: " + + keyStorePath + + " with type " + + keyStoreType + + ": " + + e.getMessage(); + LOGGER.error(errorMessage); + throw new DatabricksHttpException( + errorMessage, e, DatabricksDriverErrorCode.SSL_HANDSHAKE_ERROR); + } + } + /** * Extracts trust anchors from a KeyStore. * diff --git a/src/test/java/com/databricks/jdbc/dbclient/impl/common/ConfiguratorUtilsTest.java b/src/test/java/com/databricks/jdbc/dbclient/impl/common/ConfiguratorUtilsTest.java index 30d10d7429..7da0862c83 100644 --- a/src/test/java/com/databricks/jdbc/dbclient/impl/common/ConfiguratorUtilsTest.java +++ b/src/test/java/com/databricks/jdbc/dbclient/impl/common/ConfiguratorUtilsTest.java @@ -52,36 +52,55 @@ public class ConfiguratorUtilsTest { BASE_TRUST_STORE_PATH + "empty-truststore.jks"; private static final String DUMMY_TRUST_STORE_PATH = BASE_TRUST_STORE_PATH + "dummy-truststore.jks"; + private static final String EMPTY_KEY_STORE_PATH = BASE_TRUST_STORE_PATH + "empty-keystore.jks"; + private static final String DUMMY_KEY_STORE_PATH = BASE_TRUST_STORE_PATH + "dummy-keystore.jks"; private static final String CERTIFICATE_CN = "MinimalCertificate"; private static final String TRUST_STORE_TYPE = "PKCS12"; private static final String TRUST_STORE_PASSWORD = "changeit"; + private static final String KEY_STORE_TYPE = "PKCS12"; + private static final String KEY_STORE_PASSWORD = "changeit"; @BeforeAll static void setup() throws Exception { - createEmptyTrustStore(); - createDummyTrustStore(); + createEmptyStore(EMPTY_TRUST_STORE_PATH, TRUST_STORE_TYPE, TRUST_STORE_PASSWORD); + createEmptyStore(EMPTY_KEY_STORE_PATH, KEY_STORE_TYPE, KEY_STORE_PASSWORD); + createDummyStore( + DUMMY_TRUST_STORE_PATH, TRUST_STORE_TYPE, TRUST_STORE_PASSWORD, "dummy-cert", false); + createDummyStore(DUMMY_KEY_STORE_PATH, KEY_STORE_TYPE, KEY_STORE_PASSWORD, "client-cert", true); } - private static void createEmptyTrustStore() + /** + * Creates an empty keystore/truststore file. + * + * @param filePath The path where the store will be saved + * @param storeType The type of store (e.g., "PKCS12", "JKS") + * @param password The password for the store + */ + private static void createEmptyStore(String filePath, String storeType, String password) throws KeyStoreException, CertificateException, IOException, NoSuchAlgorithmException { - String password = TRUST_STORE_PASSWORD; - // Create an empty JKS keystore - KeyStore keyStore = KeyStore.getInstance(TRUST_STORE_TYPE); + KeyStore keyStore = KeyStore.getInstance(storeType); keyStore.load(null, password.toCharArray()); // Save the empty keystore to a file - try (FileOutputStream fos = new FileOutputStream(EMPTY_TRUST_STORE_PATH)) { + try (FileOutputStream fos = new FileOutputStream(filePath)) { keyStore.store(fos, password.toCharArray()); } } - private static void createDummyTrustStore() throws Exception { - String trustStorePassword = TRUST_STORE_PASSWORD; // Password for the trust store - String alias = "dummy-cert"; // Alias for the dummy certificate - - // Create an empty JKS keystore - KeyStore keyStore = KeyStore.getInstance(TRUST_STORE_TYPE); - keyStore.load(null, trustStorePassword.toCharArray()); + /** + * Creates a keystore/truststore with a test certificate. + * + * @param filePath The path where the store will be saved + * @param storeType The type of store (e.g., "PKCS12", "JKS") + * @param password The password for the store + * @param alias The alias for the certificate entry + * @param isKeyStore Whether this is a keystore (with private key) or truststore (cert only) + */ + private static void createDummyStore( + String filePath, String storeType, String password, String alias, boolean isKeyStore) + throws Exception { + KeyStore keyStore = KeyStore.getInstance(storeType); + keyStore.load(null, password.toCharArray()); // Generate a key pair (public and private keys) KeyPairGenerator keyPairGen = KeyPairGenerator.getInstance("RSA"); @@ -91,12 +110,18 @@ private static void createDummyTrustStore() throws Exception { // Create a self-signed certificate X509Certificate certificate = generateBarebonesCertificate(keyPair); - // Add the certificate to the keystore - keyStore.setCertificateEntry(alias, certificate); + // For keystore, add the private key with certificate chain + // For truststore, add just the certificate + if (isKeyStore) { + keyStore.setKeyEntry( + alias, keyPair.getPrivate(), password.toCharArray(), new X509Certificate[] {certificate}); + } else { + keyStore.setCertificateEntry(alias, certificate); + } // Save the keystore to a file - try (FileOutputStream fos = new FileOutputStream(DUMMY_TRUST_STORE_PATH)) { - keyStore.store(fos, trustStorePassword.toCharArray()); + try (FileOutputStream fos = new FileOutputStream(filePath)) { + keyStore.store(fos, password.toCharArray()); } } @@ -135,6 +160,16 @@ static void cleanup() { } catch (IOException e) { LOGGER.info("Failed to delete dummy trust store file: " + e.getMessage()); } + try { + Files.delete(Path.of(EMPTY_KEY_STORE_PATH)); + } catch (IOException e) { + LOGGER.info("Failed to delete empty key store file: " + e.getMessage()); + } + try { + Files.delete(Path.of(DUMMY_KEY_STORE_PATH)); + } catch (IOException e) { + LOGGER.info("Failed to delete dummy key store file: " + e.getMessage()); + } } @Test @@ -457,4 +492,89 @@ void testCreateSocketFactoryRegistry() throws Exception { assertInstanceOf( PlainConnectionSocketFactory.class, registry.lookup(DatabricksJdbcConstants.HTTP)); } + + @Test + void testLoadKeystoreOrNull() throws DatabricksHttpException { + when(mockContext.getSSLKeyStorePassword()).thenReturn(KEY_STORE_PASSWORD); + when(mockContext.getSSLKeyStoreType()).thenReturn(KEY_STORE_TYPE); + when(mockContext.getSSLKeyStore()).thenReturn(DUMMY_KEY_STORE_PATH); + + KeyStore keyStore = ConfiguratorUtils.loadKeystoreOrNull(mockContext); + assertNotNull(keyStore, "Keystore should be loaded successfully"); + + try { + assertTrue( + keyStore.containsAlias("client-cert"), "Keystore should contain the client-cert alias"); + assertTrue(keyStore.isKeyEntry("client-cert"), "Alias should be a key entry"); + } catch (KeyStoreException e) { + fail("Exception checking keystore: " + e.getMessage()); + } + } + + @Test + void testNonExistentKeyStore() { + when(mockContext.getSSLKeyStore()).thenReturn("non-existent-keystore.jks"); + assertThrows( + DatabricksHttpException.class, + () -> ConfiguratorUtils.loadKeystoreOrNull(mockContext), + "Should throw an exception for non-existent keystore"); + } + + @Test + void testEmptyKeyStore() throws DatabricksHttpException { + when(mockContext.getSSLKeyStorePassword()).thenReturn(KEY_STORE_PASSWORD); + when(mockContext.getSSLKeyStoreType()).thenReturn(KEY_STORE_TYPE); + when(mockContext.getSSLKeyStore()).thenReturn(EMPTY_KEY_STORE_PATH); + + KeyStore keyStore = ConfiguratorUtils.loadKeystoreOrNull(mockContext); + assertNotNull(keyStore, "Empty keystore should load successfully"); + + // Verify the keystore has no key entries + try { + boolean hasKeyEntry = false; + for (String alias : Collections.list(keyStore.aliases())) { + if (keyStore.isKeyEntry(alias)) { + hasKeyEntry = true; + break; + } + } + assertFalse(hasKeyEntry, "Empty keystore should not have key entries"); + } catch (KeyStoreException e) { + fail("Exception checking empty keystore: " + e.getMessage()); + } + } + + @Test + void testClientCertificateAuthentication() throws DatabricksHttpException { + // Set up the mock context for both trust store and key store + when(mockContext.getSSLTrustStorePassword()).thenReturn(TRUST_STORE_PASSWORD); + when(mockContext.getSSLTrustStoreType()).thenReturn(TRUST_STORE_TYPE); + when(mockContext.getSSLTrustStore()).thenReturn(DUMMY_TRUST_STORE_PATH); + when(mockContext.getSSLKeyStorePassword()).thenReturn(KEY_STORE_PASSWORD); + when(mockContext.getSSLKeyStoreType()).thenReturn(KEY_STORE_TYPE); + when(mockContext.getSSLKeyStore()).thenReturn(DUMMY_KEY_STORE_PATH); + when(mockContext.checkCertificateRevocation()).thenReturn(false); + + // Create registry with both trust store and key store configured + Registry registry = + ConfiguratorUtils.createConnectionSocketFactoryRegistry(mockContext); + + // Verify registry was created successfully + assertNotNull(registry, "Registry should be created successfully with client certificate"); + assertInstanceOf( + SSLConnectionSocketFactory.class, registry.lookup(DatabricksJdbcConstants.HTTPS)); + assertInstanceOf( + PlainConnectionSocketFactory.class, registry.lookup(DatabricksJdbcConstants.HTTP)); + } + + @Test + void testMissingKeyStorePassword() { + when(mockContext.getSSLKeyStore()).thenReturn(DUMMY_KEY_STORE_PATH); + when(mockContext.getSSLKeyStorePassword()).thenReturn(null); + + assertThrows( + DatabricksHttpException.class, + () -> ConfiguratorUtils.loadKeystoreOrNull(mockContext), + "Should throw an exception when key store password is missing"); + } } From dbc1144e38e042093594d32c127cecb43b1001ac Mon Sep 17 00:00:00 2001 From: Madhav Sainanee Date: Mon, 19 May 2025 12:42:31 +0000 Subject: [PATCH 02/11] - Removed redundant null check for the keystore in createKeyManagers method. - Simplified key entry verification by replacing Enumeration with a stream-based approach for better readability and performance. --- .../impl/common/ConfiguratorUtils.java | 28 +++++++++---------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/main/java/com/databricks/jdbc/dbclient/impl/common/ConfiguratorUtils.java b/src/main/java/com/databricks/jdbc/dbclient/impl/common/ConfiguratorUtils.java index c7be031cfe..346c0a8ab4 100644 --- a/src/main/java/com/databricks/jdbc/dbclient/impl/common/ConfiguratorUtils.java +++ b/src/main/java/com/databricks/jdbc/dbclient/impl/common/ConfiguratorUtils.java @@ -16,7 +16,6 @@ import java.security.cert.*; import java.util.Arrays; import java.util.Collections; -import java.util.Enumeration; import java.util.Set; import java.util.stream.Collectors; import javax.net.ssl.*; @@ -323,10 +322,6 @@ private static Registry createSocketFactoryRegistry( private static javax.net.ssl.KeyManager[] createKeyManagers( KeyStore keyStore, char[] keyStorePassword) throws NoSuchAlgorithmException, KeyStoreException, DatabricksHttpException { - if (keyStore == null) { - return null; - } - try { KeyManagerFactory kmf = KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm()); @@ -522,16 +517,21 @@ public static KeyStore loadKeystoreOrNull(IDatabricksConnectionContext connectio // Verify that the keystore contains at least one private key entry boolean hasKeyEntry = false; try { - for (Enumeration aliases = keyStore.aliases(); aliases.hasMoreElements(); ) { - String alias = aliases.nextElement(); - if (keyStore.isKeyEntry(alias)) { - hasKeyEntry = true; - LOGGER.debug("Found key entry with alias: " + alias); - break; - } - } + hasKeyEntry = + Collections.list(keyStore.aliases()).stream() + .anyMatch( + alias -> { + try { + boolean isKey = keyStore.isKeyEntry(alias); + if (isKey) { + LOGGER.debug("Found key entry with alias: " + alias); + } + return isKey; + } catch (KeyStoreException e) { + return false; + } + }); } catch (KeyStoreException e) { - // Log but don't fail - we'll still return the keystore LOGGER.warn("Unable to verify key entries in keystore: " + e.getMessage()); } From cb5b1e3f274db4e1bad481662caae04dd9e0dc42 Mon Sep 17 00:00:00 2001 From: Madhav Sainanee Date: Thu, 29 May 2025 07:03:08 +0000 Subject: [PATCH 03/11] addresses comments --- .../impl/common/ConfiguratorUtils.java | 43 +++++++++++++++++-- 1 file changed, 39 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/databricks/jdbc/dbclient/impl/common/ConfiguratorUtils.java b/src/main/java/com/databricks/jdbc/dbclient/impl/common/ConfiguratorUtils.java index 346c0a8ab4..73d93f2159 100644 --- a/src/main/java/com/databricks/jdbc/dbclient/impl/common/ConfiguratorUtils.java +++ b/src/main/java/com/databricks/jdbc/dbclient/impl/common/ConfiguratorUtils.java @@ -26,7 +26,39 @@ import org.apache.http.conn.ssl.SSLConnectionSocketFactory; import org.apache.http.impl.conn.PoolingHttpClientConnectionManager; -/** This class contains the utility functions for configuring a client. */ +/** + * Utility class for configuring SSL/TLS for Databricks JDBC connections. + * + *

SSL/TLS Configuration Flow: + * + *

1. getBaseConnectionManager(IDatabricksConnectionContext connectionContext): - Entry point for + * HTTP client SSL configuration. - Determines if a custom trust store (SSLTrustStore), system trust + * store, or default JDK trust store should be used based on connectionContext parameters. - Handles + * test and self-signed certificate scenarios via allowSelfSignedCerts() and isJDBCTestEnv(). + * + *

2. createConnectionSocketFactoryRegistry(IDatabricksConnectionContext connectionContext): - + * Chooses between createRegistryWithCustomTrustStore and + * createRegistryWithSystemOrDefaultTrustStore based on the presence of SSLTrustStore in the + * connection context. + * + *

3. Trust Store Handling: - loadTruststoreOrNull(): Loads the trust store from the path + * specified by connectionContext.getSSLTrustStore(). If the path is null, a debug log is emitted + * and null is returned. - If the trust store cannot be loaded or contains no trust anchors, an + * error is logged and a DatabricksHttpException is thrown. + * + *

4. Key Store Handling: - loadKeystoreOrNull(): Loads the client keystore from the path + * specified by connectionContext.getSSLKeyStore(). If the path is null, a debug log is emitted and + * null is returned. - If the keystore is present, it is used for client certificate authentication + * (mutual TLS). If not, a debug log is emitted and only server certificate validation is performed. + * + *

5. Socket Factory Registry Construction: - createRegistryFromTrustAnchors(): Builds the + * registry using trust anchors and, if available, key managers from the keystore. - Handles both + * one-way (server) and two-way (mutual) TLS authentication. + * + *

Key Parameters: - SSLTrustStore, SSLTrustStorePwd, SSLTrustStoreType: Custom trust store + * configuration - SSLKeyStore, SSLKeyStorePwd, SSLKeyStoreType: Client keystore for mutual TLS - + * AllowSelfSignedCerts, UseSystemTrustStore: Control trust strategy + */ public class ConfiguratorUtils { private static final JdbcLogger LOGGER = JdbcLoggerFactory.getLogger(ConfiguratorUtils.class); @@ -111,6 +143,7 @@ private static Registry createRegistryWithCustomTrustSt if (trustStore == null) { String errorMessage = "Specified trust store could not be loaded: " + connectionContext.getSSLTrustStore(); + LOGGER.debug("Trust store load failed: " + connectionContext.getSSLTrustStore()); handleError(errorMessage, new IOException(errorMessage)); } @@ -287,7 +320,7 @@ private static Registry createRegistryFromTrustAnchors( return createSocketFactoryRegistry(trustManagers, keyManagers); } else { - LOGGER.info("No client keystore configured, server certificate validation only"); + LOGGER.debug("No keystore path specified in connection url"); return createSocketFactoryRegistry(trustManagers); } } catch (Exception e) { @@ -364,8 +397,8 @@ private static Registry createSocketFactoryRegistry( * * @param trustAnchors The trust anchors to use. * @param checkCertificateRevocation Whether to check certificate revocation. - * @param acceptUndeterminedCertificateRevocation Whether to accept undetermined revocation - * status. + * @param acceptUndeterminedCertificateRevocation Whether to accept undetermined certificate + * revocation status. * @return An array of trust managers. * @throws NoSuchAlgorithmException If there is an error during trust manager creation. * @throws InvalidAlgorithmParameterException If there is an error during trust manager creation. @@ -420,6 +453,7 @@ public static KeyStore loadTruststoreOrNull(IDatabricksConnectionContext connect throws DatabricksHttpException { String trustStorePath = connectionContext.getSSLTrustStore(); if (trustStorePath == null) { + LOGGER.debug("No truststore path specified in connection url"); return null; } @@ -471,6 +505,7 @@ public static KeyStore loadKeystoreOrNull(IDatabricksConnectionContext connectio throws DatabricksHttpException { String keyStorePath = connectionContext.getSSLKeyStore(); if (keyStorePath == null) { + LOGGER.debug("No keystore path specified in connection context"); return null; } From 1243a12476669c4f6cc5ee2b284c613040ce6739 Mon Sep 17 00:00:00 2001 From: Madhav Sainanee Date: Mon, 9 Jun 2025 11:15:46 +0530 Subject: [PATCH 04/11] address comments --- .github/workflows/sslTesting.yml | 42 ++++ .../impl/common/ConfiguratorUtils.java | 55 ++--- .../com/databricks/client/jdbc/SSLTest.java | 216 ++++++++++++++++-- 3 files changed, 254 insertions(+), 59 deletions(-) diff --git a/.github/workflows/sslTesting.yml b/.github/workflows/sslTesting.yml index 5e02b54392..2165b07e83 100644 --- a/.github/workflows/sslTesting.yml +++ b/.github/workflows/sslTesting.yml @@ -48,6 +48,8 @@ jobs: HTTPS_PROXY_URL: "https://localhost:3129" TRUSTSTORE_PATH: "/tmp/ssl-certs/test-truststore.jks" TRUSTSTORE_PASSWORD: "changeit" + KEYSTORE_PATH: "/tmp/ssl-certs/test-keystore.jks" + KEYSTORE_PASSWORD: "changeit" run: | echo "DATABRICKS_TOKEN=${DATABRICKS_TOKEN}" >> $GITHUB_ENV echo "DATABRICKS_HOST=${DATABRICKS_HOST}" >> $GITHUB_ENV @@ -56,6 +58,8 @@ jobs: echo "HTTPS_PROXY_URL=${HTTPS_PROXY_URL}" >> $GITHUB_ENV echo "TRUSTSTORE_PATH=${TRUSTSTORE_PATH}" >> $GITHUB_ENV echo "TRUSTSTORE_PASSWORD=${TRUSTSTORE_PASSWORD}" >> $GITHUB_ENV + echo "KEYSTORE_PATH=${KEYSTORE_PATH}" >> $GITHUB_ENV + echo "KEYSTORE_PASSWORD=${KEYSTORE_PASSWORD}" >> $GITHUB_ENV - name: Install Squid and SSL Tools run: | @@ -120,6 +124,7 @@ jobs: # Copy to appropriate locations sudo cp squid.pem /etc/squid/ + sudo cp intermediateCA.crt /tmp/ssl-certs/ sudo chown proxy:proxy /etc/squid/squid.pem # Extract the Databricks workspace certificate @@ -142,6 +147,40 @@ jobs: -keystore test-truststore.jks -storepass changeit chmod 644 test-truststore.jks + + # Generate Client Certificate for mutual TLS testing + openssl genrsa -out client.key 2048 + openssl req -new -key client.key -out client.csr \ + -subj "/C=US/ST=California/L=San Francisco/O=Databricks Test/OU=Testing/CN=test-client" + + # Create extension file for client certificate + cat > client_ext.cnf << EOF + [ v3_req ] + basicConstraints = CA:FALSE + keyUsage = digitalSignature, keyEncipherment + extendedKeyUsage = clientAuth + EOF + + # Sign client certificate with Intermediate CA + openssl x509 -req -in client.csr -CA intermediateCA.crt -CAkey intermediateCA.key \ + -CAcreateserial -out client.crt -days 365 -sha256 \ + -extfile client_ext.cnf -extensions v3_req + + # Create PKCS12 keystore from client certificate and key + openssl pkcs12 -export -inkey client.key -in client.crt -certfile intermediateCA.crt \ + -out client.p12 -passout pass:changeit + + # Convert PKCS12 to JKS keystore + keytool -importkeystore -noprompt \ + -srckeystore client.p12 -srcstoretype PKCS12 -srcstorepass changeit \ + -destkeystore test-keystore.jks -deststoretype JKS -deststorepass changeit + + chmod 644 test-keystore.jks + + # Verify keystore contents + echo "=== Keystore contents ===" + keytool -list -keystore test-keystore.jks -storepass changeit + echo "=========================" - name: Configure Squid with Standard SSL run: | @@ -154,6 +193,9 @@ jobs: # Plain HTTPS port with certificate https_port 3129 tls-cert=/etc/squid/squid.pem + # HTTPS port with client certificate requirement (mutual TLS) + https_port 3130 tls-cert=/etc/squid/squid.pem clientca=/tmp/ssl-certs/intermediateCA.crt + # Access Control - very permissive for testing http_access allow all always_direct allow all diff --git a/src/main/java/com/databricks/jdbc/dbclient/impl/common/ConfiguratorUtils.java b/src/main/java/com/databricks/jdbc/dbclient/impl/common/ConfiguratorUtils.java index 73d93f2159..58b4680954 100644 --- a/src/main/java/com/databricks/jdbc/dbclient/impl/common/ConfiguratorUtils.java +++ b/src/main/java/com/databricks/jdbc/dbclient/impl/common/ConfiguratorUtils.java @@ -316,7 +316,7 @@ private static Registry createRegistryFromTrustAnchors( } // Create key managers for client certificate authentication - javax.net.ssl.KeyManager[] keyManagers = createKeyManagers(keyStore, keyStorePassword); + KeyManager[] keyManagers = createKeyManagers(keyStore, keyStorePassword); return createSocketFactoryRegistry(trustManagers, keyManagers); } else { @@ -352,8 +352,7 @@ private static Registry createSocketFactoryRegistry( * @throws KeyStoreException If there is an error accessing the key store. * @throws DatabricksHttpException If there is an error creating the key managers. */ - private static javax.net.ssl.KeyManager[] createKeyManagers( - KeyStore keyStore, char[] keyStorePassword) + private static KeyManager[] createKeyManagers(KeyStore keyStore, char[] keyStorePassword) throws NoSuchAlgorithmException, KeyStoreException, DatabricksHttpException { try { KeyManagerFactory kmf = @@ -379,7 +378,7 @@ private static javax.net.ssl.KeyManager[] createKeyManagers( * @throws KeyManagementException If there is an error during SSL context creation. */ private static Registry createSocketFactoryRegistry( - TrustManager[] trustManagers, javax.net.ssl.KeyManager[] keyManagers) + TrustManager[] trustManagers, KeyManager[] keyManagers) throws NoSuchAlgorithmException, KeyManagementException { SSLContext sslContext = SSLContext.getInstance(DatabricksJdbcConstants.TLS); @@ -549,46 +548,20 @@ public static KeyStore loadKeystoreOrNull(IDatabricksConnectionContext connectio keyStore.load(keyStoreStream, password); } - // Verify that the keystore contains at least one private key entry - boolean hasKeyEntry = false; - try { - hasKeyEntry = - Collections.list(keyStore.aliases()).stream() - .anyMatch( - alias -> { - try { - boolean isKey = keyStore.isKeyEntry(alias); - if (isKey) { - LOGGER.debug("Found key entry with alias: " + alias); - } - return isKey; - } catch (KeyStoreException e) { - return false; - } - }); - } catch (KeyStoreException e) { - LOGGER.warn("Unable to verify key entries in keystore: " + e.getMessage()); - } - - if (!hasKeyEntry) { - LOGGER.warn( - "Key store does not contain any private key entries. " - + "Client authentication may fail."); - } - - LOGGER.info("Successfully loaded key store: " + keyStorePath); + LOGGER.debug("Successfully loaded key store: " + keyStorePath); return keyStore; } catch (Exception e) { - String errorMessage = - "Failed to load key store: " - + keyStorePath - + " with type " - + keyStoreType - + ": " - + e.getMessage(); - LOGGER.error(errorMessage); + StringBuilder errorMessage = + new StringBuilder() + .append("Failed to load key store: ") + .append(keyStorePath) + .append(" with type ") + .append(keyStoreType) + .append(": ") + .append(e.getMessage()); + LOGGER.error(errorMessage.toString(), e); throw new DatabricksHttpException( - errorMessage, e, DatabricksDriverErrorCode.SSL_HANDSHAKE_ERROR); + errorMessage.toString(), e, DatabricksDriverErrorCode.SSL_HANDSHAKE_ERROR); } } diff --git a/src/test/java/com/databricks/client/jdbc/SSLTest.java b/src/test/java/com/databricks/client/jdbc/SSLTest.java index 2c9e7d8c84..fe2d13c552 100644 --- a/src/test/java/com/databricks/client/jdbc/SSLTest.java +++ b/src/test/java/com/databricks/client/jdbc/SSLTest.java @@ -23,6 +23,8 @@ public class SSLTest { private static String httpsProxyUrl; private static String trustStorePath; private static String trustStorePassword; + private static String keyStorePath; + private static String keyStorePassword; @BeforeAll public static void setupEnv() { @@ -33,6 +35,8 @@ public static void setupEnv() { httpsProxyUrl = System.getenv("HTTPS_PROXY_URL"); trustStorePath = System.getenv("TRUSTSTORE_PATH"); trustStorePassword = System.getenv("TRUSTSTORE_PASSWORD"); + keyStorePath = System.getenv("KEYSTORE_PATH"); + keyStorePassword = System.getenv("KEYSTORE_PASSWORD"); } private String buildJdbcUrl( @@ -41,7 +45,8 @@ private String buildJdbcUrl( boolean useHttpsProxy, boolean allowSelfSignedCerts, boolean useSystemTrustStore, - boolean useCustomTrustStore) { + boolean useCustomTrustStore, + boolean useKeyStore) { String defaultProxyHost = "localhost"; String defaultProxyPort = "3128"; @@ -116,6 +121,15 @@ private String buildJdbcUrl( } } + if (useKeyStore && keyStorePath != null && !keyStorePath.isEmpty()) { + sb.append("SSLKeyStore=").append(keyStorePath).append(";"); + + if (keyStorePassword != null && !keyStorePassword.isEmpty()) { + sb.append("SSLKeyStorePwd=").append(keyStorePassword).append(";"); + sb.append("SSLKeyStoreType=").append("JKS").append(";"); + } + } + sb.append("ssl=1;"); return sb.toString(); } @@ -136,7 +150,7 @@ private void verifyConnect(String jdbcUrl) throws Exception { public void testDirectConnectionDefaultSSL() { LOGGER.info("Scenario: Direct connection with default SSL settings"); for (boolean thrift : new boolean[] {true, false}) { - String url = buildJdbcUrl(thrift, false, false, false, false, false); + String url = buildJdbcUrl(thrift, false, false, false, false, false, false); try { verifyConnect(url); } catch (Exception e) { @@ -149,7 +163,7 @@ public void testDirectConnectionDefaultSSL() { public void testHttpProxyDefaultSSL() { LOGGER.info("Scenario: HTTP Proxy with default SSL settings"); for (boolean thrift : new boolean[] {true, false}) { - String url = buildJdbcUrl(thrift, true, false, false, false, false); + String url = buildJdbcUrl(thrift, true, false, false, false, false, false); try { verifyConnect(url); } catch (Exception e) { @@ -186,7 +200,7 @@ public void testWithAllowSelfSigned() { for (boolean thrift : new boolean[] {true, false}) { // Test 1: Connection with empty trust store - should fail - String url1 = buildJdbcUrl(thrift, true, false, false, false, false); + String url1 = buildJdbcUrl(thrift, true, false, false, false, false, false); url1 += ";LogLevel=TRACE;"; url1 += "SSLTrustStore=" + emptyTrustStore.getAbsolutePath() + ";"; url1 += "SSLTrustStorePwd=changeit;"; @@ -204,7 +218,7 @@ public void testWithAllowSelfSigned() { // Test 2: Non-existent trust store - should fail with clear error String nonExistentPath = "/path/to/nonexistent"; - String url2 = buildJdbcUrl(thrift, true, false, false, false, false); + String url2 = buildJdbcUrl(thrift, true, false, false, false, false, false); url2 += ";LogLevel=TRACE;"; url2 += "SSLTrustStore=" + nonExistentPath + ";"; @@ -230,7 +244,7 @@ public void testWithAllowSelfSigned() { // Test 3: With self-signed certs allowed - should succeed System.setProperty("javax.net.ssl.trustStore", emptyTrustStore.getAbsolutePath()); - String url3 = buildJdbcUrl(thrift, true, false, true, false, false); + String url3 = buildJdbcUrl(thrift, true, false, true, false, false, false); url3 += ";LogLevel=TRACE;"; try { @@ -272,7 +286,7 @@ public void testWithAllowSelfSigned() { public void testWithSystemTrustStore() { LOGGER.info("Scenario: Testing with UseSystemTrustStore=1"); for (boolean thrift : new boolean[] {true, false}) { - String url = buildJdbcUrl(thrift, true, false, false, true, false); + String url = buildJdbcUrl(thrift, true, false, false, true, false, false); try { verifyConnect(url); } catch (Exception e) { @@ -292,7 +306,7 @@ public void testDirectConnectionSystemTrustStoreFallback() { System.clearProperty("javax.net.ssl.trustStore"); for (boolean thrift : new boolean[] {true, false}) { - String url = buildJdbcUrl(thrift, false, false, false, true, false); + String url = buildJdbcUrl(thrift, false, false, false, true, false, false); try { verifyConnect(url); } catch (Exception e) { @@ -321,7 +335,7 @@ public void testIgnoreSystemPropertyWhenUseSystemTrustStoreDisabled() { System.setProperty("javax.net.ssl.trustStore", "/path/that/does/not/exist.jks"); for (boolean thrift : new boolean[] {true, false}) { - String url = buildJdbcUrl(thrift, false, false, false, false, false); + String url = buildJdbcUrl(thrift, false, false, false, false, false, false); try { verifyConnect(url); } catch (Exception e) { @@ -383,7 +397,7 @@ public void testWithCustomTrustStore() { } for (boolean thrift : new boolean[] {true, false}) { - String url = buildJdbcUrl(thrift, true, false, false, false, true); + String url = buildJdbcUrl(thrift, true, false, false, false, true, false); url += ";LogLevel=TRACE;"; try { @@ -394,7 +408,7 @@ public void testWithCustomTrustStore() { LOGGER.info( "Connection failed with custom trust store, trying with AllowSelfSignedCerts=1: " + e.getMessage()); - String fallbackUrl = buildJdbcUrl(thrift, true, false, true, false, false); + String fallbackUrl = buildJdbcUrl(thrift, true, false, true, false, false, false); fallbackUrl += ";LogLevel=TRACE;"; try { verifyConnect(fallbackUrl); @@ -408,7 +422,7 @@ public void testWithCustomTrustStore() { LOGGER.info("Custom trust store test setup failed: " + e.getMessage()); // Instead of failing the test, try with AllowSelfSignedCerts=1 for (boolean thrift : new boolean[] {true}) { - String fallbackUrl = buildJdbcUrl(thrift, true, false, true, false, false); + String fallbackUrl = buildJdbcUrl(thrift, true, false, true, false, false, false); fallbackUrl += ";LogLevel=TRACE;"; try { verifyConnect(fallbackUrl); @@ -451,13 +465,13 @@ public void testWithSystemProperties() { // Use AllowSelfSignedCerts as fallback mechanism for (boolean thrift : new boolean[] {true, false}) { try { - String url = buildJdbcUrl(thrift, false, false, false, false, false); + String url = buildJdbcUrl(thrift, false, false, false, false, false, false); verifyConnect(url); } catch (Exception e) { LOGGER.info( "Connection with system properties failed, trying with AllowSelfSignedCerts=1: " + e.getMessage()); - String fallbackUrl = buildJdbcUrl(thrift, false, false, true, false, false); + String fallbackUrl = buildJdbcUrl(thrift, false, false, true, false, false, false); try { verifyConnect(fallbackUrl); LOGGER.info("Successfully connected with AllowSelfSignedCerts=1 fallback"); @@ -507,7 +521,7 @@ public void testEmptyTrustStore() { for (boolean thrift : new boolean[] {true, false}) { - String url = buildJdbcUrl(thrift, false, false, false, false, false); + String url = buildJdbcUrl(thrift, false, false, false, false, false, false); url += ";SSLTrustStore=" + emptyTrustStore.getAbsolutePath() + ";"; url += "SSLTrustStorePwd=changeit;"; url += "SSLTrustStoreType=JKS;"; @@ -537,7 +551,7 @@ public void testNonExistentTrustStore() { String nonExistentPath = "/path/to/nonexistent/truststore.jks"; for (boolean thrift : new boolean[] {true, false}) { - String url = buildJdbcUrl(thrift, false, false, false, false, false); + String url = buildJdbcUrl(thrift, false, false, false, false, false, false); url += ";SSLTrustStore=" + nonExistentPath + ";"; try { @@ -563,7 +577,7 @@ public void testNoCustomTrustStoreWithUseSystemTrustStoreFalse() { // This test simply verifies that when UseSystemTrustStore=false and no custom trust store // is provided, the connection still works (falls back to JDK default trust store) for (boolean thrift : new boolean[] {true, false}) { - String url = buildJdbcUrl(thrift, false, false, false, false, false); + String url = buildJdbcUrl(thrift, false, false, false, false, false, false); try { LOGGER.info( @@ -610,7 +624,7 @@ public void testCustomTrustStorePrecedence() { for (boolean thrift : new boolean[] {true}) { // Use both UseSystemTrustStore=true and a custom trust store // The custom trust store should take precedence - String url = buildJdbcUrl(thrift, false, false, false, true, true); + String url = buildJdbcUrl(thrift, false, false, false, true, true, false); try { LOGGER.info("\n==== Testing custom trust store precedence ===="); @@ -635,4 +649,170 @@ public void testCustomTrustStorePrecedence() { } } } + + @Test + public void testWithClientCertificateKeyStore() { + LOGGER.info("Scenario: Testing with client certificate authentication (mutual TLS)"); + + // Skip if we don't have a valid keystore + if (keyStorePath == null || keyStorePath.isEmpty()) { + LOGGER.info("Skipping client certificate test - no keystore path provided"); + return; + } + + File keyStoreFile = new File(keyStorePath); + if (!keyStoreFile.exists() || !keyStoreFile.canRead()) { + LOGGER.info( + "Skipping client certificate test - keystore does not exist or is not readable: " + + keyStorePath); + return; + } + + try { + // Validate keystore content first + KeyStore ks = KeyStore.getInstance("JKS"); + try (java.io.FileInputStream fis = new java.io.FileInputStream(keyStorePath)) { + ks.load(fis, keyStorePassword.toCharArray()); + int entriesCount = java.util.Collections.list(ks.aliases()).size(); + + LOGGER.info("Keystore contains " + entriesCount + " entries"); + assertTrue(entriesCount > 0, "Keystore must contain at least one entry"); + + // Check if at least one entry is a private key entry + boolean hasKeyEntry = false; + for (String alias : java.util.Collections.list(ks.aliases())) { + if (ks.isKeyEntry(alias)) { + hasKeyEntry = true; + LOGGER.info("Found private key entry: " + alias); + break; + } + } + assertTrue(hasKeyEntry, "Keystore must contain at least one private key entry"); + } + + for (boolean thrift : new boolean[] {true, false}) { + // Test with both custom trust store and keystore for mutual TLS + String url = buildJdbcUrl(thrift, true, false, false, false, true, true); + url += ";LogLevel=TRACE;"; + + try { + LOGGER.info("Testing client certificate authentication (mutual TLS)"); + verifyConnect(url); + LOGGER.info("Client certificate authentication succeeded"); + } catch (Exception e) { + LOGGER.info( + "Client certificate authentication failed, this may be expected if the server doesn't require client certs: " + + e.getMessage()); + // Try with just the keystore (no custom trust store) + String fallbackUrl = buildJdbcUrl(thrift, true, false, false, false, false, true); + fallbackUrl += ";LogLevel=TRACE;"; + try { + verifyConnect(fallbackUrl); + LOGGER.info("Client certificate authentication succeeded with default trust store"); + } catch (Exception e2) { + LOGGER.info( + "Client certificate authentication failed with both approaches: " + + e2.getMessage()); + // This is not necessarily a test failure since the server might not be configured for + // mutual TLS + } + } + } + } catch (Exception e) { + LOGGER.info("Client certificate test setup failed: " + e.getMessage()); + } + } + + @Test + public void testNonExistentKeyStore() { + LOGGER.info("Scenario: Testing with non-existent keystore"); + + String nonExistentPath = "/path/to/nonexistent/keystore.jks"; + for (boolean thrift : new boolean[] {true, false}) { + + String url = buildJdbcUrl(thrift, false, false, false, false, false, false); + url += ";SSLKeyStore=" + nonExistentPath + ";"; + url += ";SSLKeyStorePwd=changeit;"; + + try { + verifyConnect(url); + fail("Connection with non-existent keystore should have failed"); + } catch (Exception e) { + LOGGER.info("Connection correctly failed with non-existent keystore: " + e.getMessage()); + assertTrue( + e.getMessage().contains("key store") || e.getMessage().contains("keystore"), + "Error message should mention keystore issues"); + } + } + } + + @Test + public void testKeyStoreWithoutPassword() { + LOGGER.info("Scenario: Testing keystore without password"); + + // Skip if we don't have a valid keystore + if (keyStorePath == null || keyStorePath.isEmpty()) { + LOGGER.info("Skipping keystore without password test - no keystore path provided"); + return; + } + + File keyStoreFile = new File(keyStorePath); + if (!keyStoreFile.exists()) { + LOGGER.info("Skipping keystore without password test - keystore does not exist"); + return; + } + + for (boolean thrift : new boolean[] {true, false}) { + String url = buildJdbcUrl(thrift, false, false, false, false, false, false); + url += ";SSLKeyStore=" + keyStorePath + ";"; + // Intentionally omit SSLKeyStorePwd + + try { + verifyConnect(url); + fail("Connection with keystore but no password should have failed"); + } catch (Exception e) { + LOGGER.info("Connection correctly failed with keystore but no password: " + e.getMessage()); + assertTrue( + e.getMessage().contains("password") || e.getMessage().contains("required"), + "Error message should mention password requirement"); + } + } + } + + @Test + public void testEmptyKeyStore() { + LOGGER.info("Scenario: Testing with empty keystore"); + + try { + // Create an empty keystore file + java.io.File emptyKeyStore = java.io.File.createTempFile("empty-test-key", ".jks"); + emptyKeyStore.deleteOnExit(); + + // Initialize an empty keystore + java.security.KeyStore ks = java.security.KeyStore.getInstance("JKS"); + ks.load(null, "changeit".toCharArray()); + try (java.io.FileOutputStream fos = new java.io.FileOutputStream(emptyKeyStore)) { + ks.store(fos, "changeit".toCharArray()); + } + + for (boolean thrift : new boolean[] {true, false}) { + String url = buildJdbcUrl(thrift, false, false, false, false, false, false); + url += ";SSLKeyStore=" + emptyKeyStore.getAbsolutePath() + ";"; + url += "SSLKeyStorePwd=changeit;"; + url += "SSLKeyStoreType=JKS;"; + + try { + // This may succeed or fail depending on whether the server requires client certs + verifyConnect(url); + LOGGER.info( + "Connection with empty keystore succeeded (server doesn't require client certs)"); + } catch (Exception e) { + LOGGER.info("Connection with empty keystore failed: " + e.getMessage()); + // This is acceptable - empty keystore means no client certificate for mutual TLS + } + } + } catch (Exception e) { + fail("Test setup failed: " + e.getMessage()); + } + } } From b183c354010b2c6bb2d17a287801b8a2b17cf1a8 Mon Sep 17 00:00:00 2001 From: Madhav Sainanee Date: Mon, 9 Jun 2025 11:17:21 +0530 Subject: [PATCH 05/11] fix next changelog --- NEXT_CHANGELOG.md | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index a932e6421f..020f1514e9 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -3,17 +3,15 @@ ## [Unreleased] ### Added -- Support for fetching tables and views across all catalogs using SHOW TABLES FROM/IN ALL CATALOGS in the SQL Exec API. -- Support for Token Exchange in OAuth flows where in third party tokens are exchanged for InHouse tokens. -- Added support for polling of statementStatus and sqlState for async SQL execution. +- Added support for DoD (.mil) domains +- Support to fetch metadata in PreparedStatement for SELECT queries before executing the query. - Added support for SSL client certificate authentication via keystore configuration parameters: SSLKeyStore, SSLKeyStorePwd, SSLKeyStoreType, and SSLKeyStoreProvider. ### Updated - ### Fixed -- Fix: unsupported data types in `setObject(int,Object,int targetSqlType)` method in PreparedStatement -- Fix: Added explicit null check for Arrow value vector when the value is empty, and Arrow null checking is disabled. +- --- *Note: When making changes, please add your change under the appropriate section with a brief description.* \ No newline at end of file From 042fa1ff781718e47eec539a4882850832cfabbb Mon Sep 17 00:00:00 2001 From: Madhav Sainanee Date: Mon, 9 Jun 2025 11:24:15 +0530 Subject: [PATCH 06/11] Remove unnecessary copy command for intermediateCA.crt in sslTesting workflow --- .github/workflows/sslTesting.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.github/workflows/sslTesting.yml b/.github/workflows/sslTesting.yml index 2165b07e83..5f97f2d005 100644 --- a/.github/workflows/sslTesting.yml +++ b/.github/workflows/sslTesting.yml @@ -124,7 +124,6 @@ jobs: # Copy to appropriate locations sudo cp squid.pem /etc/squid/ - sudo cp intermediateCA.crt /tmp/ssl-certs/ sudo chown proxy:proxy /etc/squid/squid.pem # Extract the Databricks workspace certificate From 393cc52755aa1409b15d3fee156cecc0d7424ca9 Mon Sep 17 00:00:00 2001 From: Madhav Sainanee Date: Mon, 9 Jun 2025 11:32:04 +0530 Subject: [PATCH 07/11] Update sslTesting workflow to correct client certificate configuration for mutual TLS --- .github/workflows/sslTesting.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/sslTesting.yml b/.github/workflows/sslTesting.yml index 5f97f2d005..647bc39260 100644 --- a/.github/workflows/sslTesting.yml +++ b/.github/workflows/sslTesting.yml @@ -193,7 +193,7 @@ jobs: https_port 3129 tls-cert=/etc/squid/squid.pem # HTTPS port with client certificate requirement (mutual TLS) - https_port 3130 tls-cert=/etc/squid/squid.pem clientca=/tmp/ssl-certs/intermediateCA.crt + https_port 3130 tls-cert=/etc/squid/squid.pem tls-clientca=/tmp/ssl-certs/intermediateCA.crt # Access Control - very permissive for testing http_access allow all From cbce7a54277d55e182346a50589c484baa2672c2 Mon Sep 17 00:00:00 2001 From: Madhav Sainanee Date: Mon, 9 Jun 2025 11:39:13 +0530 Subject: [PATCH 08/11] Update sslTesting workflow to include intermediate CA certificate and remove obsolete DNS directive --- .github/workflows/sslTesting.yml | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/.github/workflows/sslTesting.yml b/.github/workflows/sslTesting.yml index 647bc39260..3097213f38 100644 --- a/.github/workflows/sslTesting.yml +++ b/.github/workflows/sslTesting.yml @@ -124,7 +124,9 @@ jobs: # Copy to appropriate locations sudo cp squid.pem /etc/squid/ + sudo cp intermediateCA.crt /etc/squid/ sudo chown proxy:proxy /etc/squid/squid.pem + sudo chown proxy:proxy /etc/squid/intermediateCA.crt # Extract the Databricks workspace certificate echo -n | openssl s_client -connect ${DATABRICKS_HOST}:443 -showcerts 2>/dev/null | \ @@ -193,14 +195,13 @@ jobs: https_port 3129 tls-cert=/etc/squid/squid.pem # HTTPS port with client certificate requirement (mutual TLS) - https_port 3130 tls-cert=/etc/squid/squid.pem tls-clientca=/tmp/ssl-certs/intermediateCA.crt + https_port 3130 tls-cert=/etc/squid/squid.pem tls-clientca=/etc/squid/intermediateCA.crt # Access Control - very permissive for testing http_access allow all always_direct allow all - # Avoid DNS issues in test environment - dns_v4_first on + # Skip DNS v4 first (obsolete directive removed) # Disable caching for testing cache deny all From c511d9fe29ca252800b0e55c19b4d0ac1e022c18 Mon Sep 17 00:00:00 2001 From: Madhav Sainanee Date: Mon, 9 Jun 2025 12:13:36 +0530 Subject: [PATCH 09/11] revert to main --- .github/workflows/sslTesting.yml | 46 +--- .../com/databricks/client/jdbc/SSLTest.java | 216 ++---------------- 2 files changed, 20 insertions(+), 242 deletions(-) diff --git a/.github/workflows/sslTesting.yml b/.github/workflows/sslTesting.yml index 3097213f38..5e02b54392 100644 --- a/.github/workflows/sslTesting.yml +++ b/.github/workflows/sslTesting.yml @@ -48,8 +48,6 @@ jobs: HTTPS_PROXY_URL: "https://localhost:3129" TRUSTSTORE_PATH: "/tmp/ssl-certs/test-truststore.jks" TRUSTSTORE_PASSWORD: "changeit" - KEYSTORE_PATH: "/tmp/ssl-certs/test-keystore.jks" - KEYSTORE_PASSWORD: "changeit" run: | echo "DATABRICKS_TOKEN=${DATABRICKS_TOKEN}" >> $GITHUB_ENV echo "DATABRICKS_HOST=${DATABRICKS_HOST}" >> $GITHUB_ENV @@ -58,8 +56,6 @@ jobs: echo "HTTPS_PROXY_URL=${HTTPS_PROXY_URL}" >> $GITHUB_ENV echo "TRUSTSTORE_PATH=${TRUSTSTORE_PATH}" >> $GITHUB_ENV echo "TRUSTSTORE_PASSWORD=${TRUSTSTORE_PASSWORD}" >> $GITHUB_ENV - echo "KEYSTORE_PATH=${KEYSTORE_PATH}" >> $GITHUB_ENV - echo "KEYSTORE_PASSWORD=${KEYSTORE_PASSWORD}" >> $GITHUB_ENV - name: Install Squid and SSL Tools run: | @@ -124,9 +120,7 @@ jobs: # Copy to appropriate locations sudo cp squid.pem /etc/squid/ - sudo cp intermediateCA.crt /etc/squid/ sudo chown proxy:proxy /etc/squid/squid.pem - sudo chown proxy:proxy /etc/squid/intermediateCA.crt # Extract the Databricks workspace certificate echo -n | openssl s_client -connect ${DATABRICKS_HOST}:443 -showcerts 2>/dev/null | \ @@ -148,40 +142,6 @@ jobs: -keystore test-truststore.jks -storepass changeit chmod 644 test-truststore.jks - - # Generate Client Certificate for mutual TLS testing - openssl genrsa -out client.key 2048 - openssl req -new -key client.key -out client.csr \ - -subj "/C=US/ST=California/L=San Francisco/O=Databricks Test/OU=Testing/CN=test-client" - - # Create extension file for client certificate - cat > client_ext.cnf << EOF - [ v3_req ] - basicConstraints = CA:FALSE - keyUsage = digitalSignature, keyEncipherment - extendedKeyUsage = clientAuth - EOF - - # Sign client certificate with Intermediate CA - openssl x509 -req -in client.csr -CA intermediateCA.crt -CAkey intermediateCA.key \ - -CAcreateserial -out client.crt -days 365 -sha256 \ - -extfile client_ext.cnf -extensions v3_req - - # Create PKCS12 keystore from client certificate and key - openssl pkcs12 -export -inkey client.key -in client.crt -certfile intermediateCA.crt \ - -out client.p12 -passout pass:changeit - - # Convert PKCS12 to JKS keystore - keytool -importkeystore -noprompt \ - -srckeystore client.p12 -srcstoretype PKCS12 -srcstorepass changeit \ - -destkeystore test-keystore.jks -deststoretype JKS -deststorepass changeit - - chmod 644 test-keystore.jks - - # Verify keystore contents - echo "=== Keystore contents ===" - keytool -list -keystore test-keystore.jks -storepass changeit - echo "=========================" - name: Configure Squid with Standard SSL run: | @@ -194,14 +154,12 @@ jobs: # Plain HTTPS port with certificate https_port 3129 tls-cert=/etc/squid/squid.pem - # HTTPS port with client certificate requirement (mutual TLS) - https_port 3130 tls-cert=/etc/squid/squid.pem tls-clientca=/etc/squid/intermediateCA.crt - # Access Control - very permissive for testing http_access allow all always_direct allow all - # Skip DNS v4 first (obsolete directive removed) + # Avoid DNS issues in test environment + dns_v4_first on # Disable caching for testing cache deny all diff --git a/src/test/java/com/databricks/client/jdbc/SSLTest.java b/src/test/java/com/databricks/client/jdbc/SSLTest.java index fe2d13c552..2c9e7d8c84 100644 --- a/src/test/java/com/databricks/client/jdbc/SSLTest.java +++ b/src/test/java/com/databricks/client/jdbc/SSLTest.java @@ -23,8 +23,6 @@ public class SSLTest { private static String httpsProxyUrl; private static String trustStorePath; private static String trustStorePassword; - private static String keyStorePath; - private static String keyStorePassword; @BeforeAll public static void setupEnv() { @@ -35,8 +33,6 @@ public static void setupEnv() { httpsProxyUrl = System.getenv("HTTPS_PROXY_URL"); trustStorePath = System.getenv("TRUSTSTORE_PATH"); trustStorePassword = System.getenv("TRUSTSTORE_PASSWORD"); - keyStorePath = System.getenv("KEYSTORE_PATH"); - keyStorePassword = System.getenv("KEYSTORE_PASSWORD"); } private String buildJdbcUrl( @@ -45,8 +41,7 @@ private String buildJdbcUrl( boolean useHttpsProxy, boolean allowSelfSignedCerts, boolean useSystemTrustStore, - boolean useCustomTrustStore, - boolean useKeyStore) { + boolean useCustomTrustStore) { String defaultProxyHost = "localhost"; String defaultProxyPort = "3128"; @@ -121,15 +116,6 @@ private String buildJdbcUrl( } } - if (useKeyStore && keyStorePath != null && !keyStorePath.isEmpty()) { - sb.append("SSLKeyStore=").append(keyStorePath).append(";"); - - if (keyStorePassword != null && !keyStorePassword.isEmpty()) { - sb.append("SSLKeyStorePwd=").append(keyStorePassword).append(";"); - sb.append("SSLKeyStoreType=").append("JKS").append(";"); - } - } - sb.append("ssl=1;"); return sb.toString(); } @@ -150,7 +136,7 @@ private void verifyConnect(String jdbcUrl) throws Exception { public void testDirectConnectionDefaultSSL() { LOGGER.info("Scenario: Direct connection with default SSL settings"); for (boolean thrift : new boolean[] {true, false}) { - String url = buildJdbcUrl(thrift, false, false, false, false, false, false); + String url = buildJdbcUrl(thrift, false, false, false, false, false); try { verifyConnect(url); } catch (Exception e) { @@ -163,7 +149,7 @@ public void testDirectConnectionDefaultSSL() { public void testHttpProxyDefaultSSL() { LOGGER.info("Scenario: HTTP Proxy with default SSL settings"); for (boolean thrift : new boolean[] {true, false}) { - String url = buildJdbcUrl(thrift, true, false, false, false, false, false); + String url = buildJdbcUrl(thrift, true, false, false, false, false); try { verifyConnect(url); } catch (Exception e) { @@ -200,7 +186,7 @@ public void testWithAllowSelfSigned() { for (boolean thrift : new boolean[] {true, false}) { // Test 1: Connection with empty trust store - should fail - String url1 = buildJdbcUrl(thrift, true, false, false, false, false, false); + String url1 = buildJdbcUrl(thrift, true, false, false, false, false); url1 += ";LogLevel=TRACE;"; url1 += "SSLTrustStore=" + emptyTrustStore.getAbsolutePath() + ";"; url1 += "SSLTrustStorePwd=changeit;"; @@ -218,7 +204,7 @@ public void testWithAllowSelfSigned() { // Test 2: Non-existent trust store - should fail with clear error String nonExistentPath = "/path/to/nonexistent"; - String url2 = buildJdbcUrl(thrift, true, false, false, false, false, false); + String url2 = buildJdbcUrl(thrift, true, false, false, false, false); url2 += ";LogLevel=TRACE;"; url2 += "SSLTrustStore=" + nonExistentPath + ";"; @@ -244,7 +230,7 @@ public void testWithAllowSelfSigned() { // Test 3: With self-signed certs allowed - should succeed System.setProperty("javax.net.ssl.trustStore", emptyTrustStore.getAbsolutePath()); - String url3 = buildJdbcUrl(thrift, true, false, true, false, false, false); + String url3 = buildJdbcUrl(thrift, true, false, true, false, false); url3 += ";LogLevel=TRACE;"; try { @@ -286,7 +272,7 @@ public void testWithAllowSelfSigned() { public void testWithSystemTrustStore() { LOGGER.info("Scenario: Testing with UseSystemTrustStore=1"); for (boolean thrift : new boolean[] {true, false}) { - String url = buildJdbcUrl(thrift, true, false, false, true, false, false); + String url = buildJdbcUrl(thrift, true, false, false, true, false); try { verifyConnect(url); } catch (Exception e) { @@ -306,7 +292,7 @@ public void testDirectConnectionSystemTrustStoreFallback() { System.clearProperty("javax.net.ssl.trustStore"); for (boolean thrift : new boolean[] {true, false}) { - String url = buildJdbcUrl(thrift, false, false, false, true, false, false); + String url = buildJdbcUrl(thrift, false, false, false, true, false); try { verifyConnect(url); } catch (Exception e) { @@ -335,7 +321,7 @@ public void testIgnoreSystemPropertyWhenUseSystemTrustStoreDisabled() { System.setProperty("javax.net.ssl.trustStore", "/path/that/does/not/exist.jks"); for (boolean thrift : new boolean[] {true, false}) { - String url = buildJdbcUrl(thrift, false, false, false, false, false, false); + String url = buildJdbcUrl(thrift, false, false, false, false, false); try { verifyConnect(url); } catch (Exception e) { @@ -397,7 +383,7 @@ public void testWithCustomTrustStore() { } for (boolean thrift : new boolean[] {true, false}) { - String url = buildJdbcUrl(thrift, true, false, false, false, true, false); + String url = buildJdbcUrl(thrift, true, false, false, false, true); url += ";LogLevel=TRACE;"; try { @@ -408,7 +394,7 @@ public void testWithCustomTrustStore() { LOGGER.info( "Connection failed with custom trust store, trying with AllowSelfSignedCerts=1: " + e.getMessage()); - String fallbackUrl = buildJdbcUrl(thrift, true, false, true, false, false, false); + String fallbackUrl = buildJdbcUrl(thrift, true, false, true, false, false); fallbackUrl += ";LogLevel=TRACE;"; try { verifyConnect(fallbackUrl); @@ -422,7 +408,7 @@ public void testWithCustomTrustStore() { LOGGER.info("Custom trust store test setup failed: " + e.getMessage()); // Instead of failing the test, try with AllowSelfSignedCerts=1 for (boolean thrift : new boolean[] {true}) { - String fallbackUrl = buildJdbcUrl(thrift, true, false, true, false, false, false); + String fallbackUrl = buildJdbcUrl(thrift, true, false, true, false, false); fallbackUrl += ";LogLevel=TRACE;"; try { verifyConnect(fallbackUrl); @@ -465,13 +451,13 @@ public void testWithSystemProperties() { // Use AllowSelfSignedCerts as fallback mechanism for (boolean thrift : new boolean[] {true, false}) { try { - String url = buildJdbcUrl(thrift, false, false, false, false, false, false); + String url = buildJdbcUrl(thrift, false, false, false, false, false); verifyConnect(url); } catch (Exception e) { LOGGER.info( "Connection with system properties failed, trying with AllowSelfSignedCerts=1: " + e.getMessage()); - String fallbackUrl = buildJdbcUrl(thrift, false, false, true, false, false, false); + String fallbackUrl = buildJdbcUrl(thrift, false, false, true, false, false); try { verifyConnect(fallbackUrl); LOGGER.info("Successfully connected with AllowSelfSignedCerts=1 fallback"); @@ -521,7 +507,7 @@ public void testEmptyTrustStore() { for (boolean thrift : new boolean[] {true, false}) { - String url = buildJdbcUrl(thrift, false, false, false, false, false, false); + String url = buildJdbcUrl(thrift, false, false, false, false, false); url += ";SSLTrustStore=" + emptyTrustStore.getAbsolutePath() + ";"; url += "SSLTrustStorePwd=changeit;"; url += "SSLTrustStoreType=JKS;"; @@ -551,7 +537,7 @@ public void testNonExistentTrustStore() { String nonExistentPath = "/path/to/nonexistent/truststore.jks"; for (boolean thrift : new boolean[] {true, false}) { - String url = buildJdbcUrl(thrift, false, false, false, false, false, false); + String url = buildJdbcUrl(thrift, false, false, false, false, false); url += ";SSLTrustStore=" + nonExistentPath + ";"; try { @@ -577,7 +563,7 @@ public void testNoCustomTrustStoreWithUseSystemTrustStoreFalse() { // This test simply verifies that when UseSystemTrustStore=false and no custom trust store // is provided, the connection still works (falls back to JDK default trust store) for (boolean thrift : new boolean[] {true, false}) { - String url = buildJdbcUrl(thrift, false, false, false, false, false, false); + String url = buildJdbcUrl(thrift, false, false, false, false, false); try { LOGGER.info( @@ -624,7 +610,7 @@ public void testCustomTrustStorePrecedence() { for (boolean thrift : new boolean[] {true}) { // Use both UseSystemTrustStore=true and a custom trust store // The custom trust store should take precedence - String url = buildJdbcUrl(thrift, false, false, false, true, true, false); + String url = buildJdbcUrl(thrift, false, false, false, true, true); try { LOGGER.info("\n==== Testing custom trust store precedence ===="); @@ -649,170 +635,4 @@ public void testCustomTrustStorePrecedence() { } } } - - @Test - public void testWithClientCertificateKeyStore() { - LOGGER.info("Scenario: Testing with client certificate authentication (mutual TLS)"); - - // Skip if we don't have a valid keystore - if (keyStorePath == null || keyStorePath.isEmpty()) { - LOGGER.info("Skipping client certificate test - no keystore path provided"); - return; - } - - File keyStoreFile = new File(keyStorePath); - if (!keyStoreFile.exists() || !keyStoreFile.canRead()) { - LOGGER.info( - "Skipping client certificate test - keystore does not exist or is not readable: " - + keyStorePath); - return; - } - - try { - // Validate keystore content first - KeyStore ks = KeyStore.getInstance("JKS"); - try (java.io.FileInputStream fis = new java.io.FileInputStream(keyStorePath)) { - ks.load(fis, keyStorePassword.toCharArray()); - int entriesCount = java.util.Collections.list(ks.aliases()).size(); - - LOGGER.info("Keystore contains " + entriesCount + " entries"); - assertTrue(entriesCount > 0, "Keystore must contain at least one entry"); - - // Check if at least one entry is a private key entry - boolean hasKeyEntry = false; - for (String alias : java.util.Collections.list(ks.aliases())) { - if (ks.isKeyEntry(alias)) { - hasKeyEntry = true; - LOGGER.info("Found private key entry: " + alias); - break; - } - } - assertTrue(hasKeyEntry, "Keystore must contain at least one private key entry"); - } - - for (boolean thrift : new boolean[] {true, false}) { - // Test with both custom trust store and keystore for mutual TLS - String url = buildJdbcUrl(thrift, true, false, false, false, true, true); - url += ";LogLevel=TRACE;"; - - try { - LOGGER.info("Testing client certificate authentication (mutual TLS)"); - verifyConnect(url); - LOGGER.info("Client certificate authentication succeeded"); - } catch (Exception e) { - LOGGER.info( - "Client certificate authentication failed, this may be expected if the server doesn't require client certs: " - + e.getMessage()); - // Try with just the keystore (no custom trust store) - String fallbackUrl = buildJdbcUrl(thrift, true, false, false, false, false, true); - fallbackUrl += ";LogLevel=TRACE;"; - try { - verifyConnect(fallbackUrl); - LOGGER.info("Client certificate authentication succeeded with default trust store"); - } catch (Exception e2) { - LOGGER.info( - "Client certificate authentication failed with both approaches: " - + e2.getMessage()); - // This is not necessarily a test failure since the server might not be configured for - // mutual TLS - } - } - } - } catch (Exception e) { - LOGGER.info("Client certificate test setup failed: " + e.getMessage()); - } - } - - @Test - public void testNonExistentKeyStore() { - LOGGER.info("Scenario: Testing with non-existent keystore"); - - String nonExistentPath = "/path/to/nonexistent/keystore.jks"; - for (boolean thrift : new boolean[] {true, false}) { - - String url = buildJdbcUrl(thrift, false, false, false, false, false, false); - url += ";SSLKeyStore=" + nonExistentPath + ";"; - url += ";SSLKeyStorePwd=changeit;"; - - try { - verifyConnect(url); - fail("Connection with non-existent keystore should have failed"); - } catch (Exception e) { - LOGGER.info("Connection correctly failed with non-existent keystore: " + e.getMessage()); - assertTrue( - e.getMessage().contains("key store") || e.getMessage().contains("keystore"), - "Error message should mention keystore issues"); - } - } - } - - @Test - public void testKeyStoreWithoutPassword() { - LOGGER.info("Scenario: Testing keystore without password"); - - // Skip if we don't have a valid keystore - if (keyStorePath == null || keyStorePath.isEmpty()) { - LOGGER.info("Skipping keystore without password test - no keystore path provided"); - return; - } - - File keyStoreFile = new File(keyStorePath); - if (!keyStoreFile.exists()) { - LOGGER.info("Skipping keystore without password test - keystore does not exist"); - return; - } - - for (boolean thrift : new boolean[] {true, false}) { - String url = buildJdbcUrl(thrift, false, false, false, false, false, false); - url += ";SSLKeyStore=" + keyStorePath + ";"; - // Intentionally omit SSLKeyStorePwd - - try { - verifyConnect(url); - fail("Connection with keystore but no password should have failed"); - } catch (Exception e) { - LOGGER.info("Connection correctly failed with keystore but no password: " + e.getMessage()); - assertTrue( - e.getMessage().contains("password") || e.getMessage().contains("required"), - "Error message should mention password requirement"); - } - } - } - - @Test - public void testEmptyKeyStore() { - LOGGER.info("Scenario: Testing with empty keystore"); - - try { - // Create an empty keystore file - java.io.File emptyKeyStore = java.io.File.createTempFile("empty-test-key", ".jks"); - emptyKeyStore.deleteOnExit(); - - // Initialize an empty keystore - java.security.KeyStore ks = java.security.KeyStore.getInstance("JKS"); - ks.load(null, "changeit".toCharArray()); - try (java.io.FileOutputStream fos = new java.io.FileOutputStream(emptyKeyStore)) { - ks.store(fos, "changeit".toCharArray()); - } - - for (boolean thrift : new boolean[] {true, false}) { - String url = buildJdbcUrl(thrift, false, false, false, false, false, false); - url += ";SSLKeyStore=" + emptyKeyStore.getAbsolutePath() + ";"; - url += "SSLKeyStorePwd=changeit;"; - url += "SSLKeyStoreType=JKS;"; - - try { - // This may succeed or fail depending on whether the server requires client certs - verifyConnect(url); - LOGGER.info( - "Connection with empty keystore succeeded (server doesn't require client certs)"); - } catch (Exception e) { - LOGGER.info("Connection with empty keystore failed: " + e.getMessage()); - // This is acceptable - empty keystore means no client certificate for mutual TLS - } - } - } catch (Exception e) { - fail("Test setup failed: " + e.getMessage()); - } - } } From e3ad6376e665c8c4fc8843d4882cff65ff870e4e Mon Sep 17 00:00:00 2001 From: Madhav Sainanee Date: Mon, 9 Jun 2025 13:04:31 +0530 Subject: [PATCH 10/11] Refactor HTTP exception handling to use DatabricksSSLException for SSL/TLS errors. Update relevant classes and tests to replace DatabricksHttpException with DatabricksSSLException, ensuring proper error handling during SSL configuration and handshake processes. --- .../DatabricksClientConfiguratorManager.java | 4 +- .../impl/common/ClientConfigurator.java | 6 +- .../impl/common/ConfiguratorUtils.java | 81 +++++++++---------- .../impl/http/DatabricksHttpClient.java | 3 +- .../exception/DatabricksSSLException.java | 24 ++++++ .../auth/SSLConnectionParametersTest.java | 16 ++-- .../impl/common/ClientConfiguratorTest.java | 26 +++--- .../impl/common/ConfiguratorUtilsTest.java | 40 ++++----- 8 files changed, 113 insertions(+), 87 deletions(-) create mode 100644 src/main/java/com/databricks/jdbc/exception/DatabricksSSLException.java diff --git a/src/main/java/com/databricks/jdbc/common/DatabricksClientConfiguratorManager.java b/src/main/java/com/databricks/jdbc/common/DatabricksClientConfiguratorManager.java index 98fefc8fc3..9c2275ad93 100644 --- a/src/main/java/com/databricks/jdbc/common/DatabricksClientConfiguratorManager.java +++ b/src/main/java/com/databricks/jdbc/common/DatabricksClientConfiguratorManager.java @@ -3,7 +3,7 @@ import com.databricks.jdbc.api.internal.IDatabricksConnectionContext; import com.databricks.jdbc.dbclient.impl.common.ClientConfigurator; import com.databricks.jdbc.exception.DatabricksDriverException; -import com.databricks.jdbc.exception.DatabricksHttpException; +import com.databricks.jdbc.exception.DatabricksSSLException; import com.databricks.jdbc.log.JdbcLogger; import com.databricks.jdbc.log.JdbcLoggerFactory; import com.databricks.jdbc.model.telemetry.enums.DatabricksDriverErrorCode; @@ -28,7 +28,7 @@ public ClientConfigurator getConfigurator(IDatabricksConnectionContext context) k -> { try { return new ClientConfigurator(context); - } catch (DatabricksHttpException e) { + } catch (DatabricksSSLException e) { String message = String.format("client configurator failed due to HTTP error: %s", e.getMessage()); LOGGER.error(e, message); diff --git a/src/main/java/com/databricks/jdbc/dbclient/impl/common/ClientConfigurator.java b/src/main/java/com/databricks/jdbc/dbclient/impl/common/ClientConfigurator.java index 68854c950c..95e47b43b1 100644 --- a/src/main/java/com/databricks/jdbc/dbclient/impl/common/ClientConfigurator.java +++ b/src/main/java/com/databricks/jdbc/dbclient/impl/common/ClientConfigurator.java @@ -9,8 +9,8 @@ import com.databricks.jdbc.common.DatabricksJdbcConstants; import com.databricks.jdbc.common.util.DatabricksAuthUtil; import com.databricks.jdbc.common.util.DriverUtil; -import com.databricks.jdbc.exception.DatabricksHttpException; import com.databricks.jdbc.exception.DatabricksParsingException; +import com.databricks.jdbc.exception.DatabricksSSLException; import com.databricks.jdbc.log.JdbcLogger; import com.databricks.jdbc.log.JdbcLoggerFactory; import com.databricks.jdbc.model.telemetry.enums.DatabricksDriverErrorCode; @@ -43,7 +43,7 @@ public class ClientConfigurator { private DatabricksConfig databricksConfig; public ClientConfigurator(IDatabricksConnectionContext connectionContext) - throws DatabricksHttpException { + throws DatabricksSSLException { this.connectionContext = connectionContext; this.databricksConfig = new DatabricksConfig(); CommonsHttpClient.Builder httpClientBuilder = new CommonsHttpClient.Builder(); @@ -109,7 +109,7 @@ private static String createUniqueIdentifier(String host, String clientId, List< * @param httpClientBuilder The builder to which the SSL configuration should be added. */ void setupConnectionManager(CommonsHttpClient.Builder httpClientBuilder) - throws DatabricksHttpException { + throws DatabricksSSLException { PoolingHttpClientConnectionManager connManager = ConfiguratorUtils.getBaseConnectionManager(connectionContext); // Default value is 100 which is consistent with the value in the SDK diff --git a/src/main/java/com/databricks/jdbc/dbclient/impl/common/ConfiguratorUtils.java b/src/main/java/com/databricks/jdbc/dbclient/impl/common/ConfiguratorUtils.java index 58b4680954..251c385fbe 100644 --- a/src/main/java/com/databricks/jdbc/dbclient/impl/common/ConfiguratorUtils.java +++ b/src/main/java/com/databricks/jdbc/dbclient/impl/common/ConfiguratorUtils.java @@ -5,7 +5,7 @@ import com.databricks.jdbc.api.internal.IDatabricksConnectionContext; import com.databricks.jdbc.common.DatabricksJdbcConstants; import com.databricks.jdbc.common.util.SocketFactoryUtil; -import com.databricks.jdbc.exception.DatabricksHttpException; +import com.databricks.jdbc.exception.DatabricksSSLException; import com.databricks.jdbc.log.JdbcLogger; import com.databricks.jdbc.log.JdbcLoggerFactory; import com.databricks.jdbc.model.telemetry.enums.DatabricksDriverErrorCode; @@ -44,7 +44,7 @@ *

3. Trust Store Handling: - loadTruststoreOrNull(): Loads the trust store from the path * specified by connectionContext.getSSLTrustStore(). If the path is null, a debug log is emitted * and null is returned. - If the trust store cannot be loaded or contains no trust anchors, an - * error is logged and a DatabricksHttpException is thrown. + * error is logged and a DatabricksSSLException is thrown. * *

4. Key Store Handling: - loadKeystoreOrNull(): Loads the client keystore from the path * specified by connectionContext.getSSLKeyStore(). If the path is null, a debug log is emitted and @@ -76,10 +76,10 @@ private static boolean isJDBCTestEnv() { * * @param connectionContext The connection context to use for configuration. * @return A configured PoolingHttpClientConnectionManager. - * @throws DatabricksHttpException If there is an error during configuration. + * @throws DatabricksSSLException If there is an error during configuration. */ public static PoolingHttpClientConnectionManager getBaseConnectionManager( - IDatabricksConnectionContext connectionContext) throws DatabricksHttpException { + IDatabricksConnectionContext connectionContext) throws DatabricksSSLException { if (connectionContext.getSSLTrustStore() == null && connectionContext.checkCertificateRevocation() @@ -115,10 +115,10 @@ public static PoolingHttpClientConnectionManager getBaseConnectionManager( * * @param connectionContext The connection context to use for configuration. * @return A configured Registry of ConnectionSocketFactory. - * @throws DatabricksHttpException If there is an error during configuration. + * @throws DatabricksSSLException If there is an error during configuration. */ public static Registry createConnectionSocketFactoryRegistry( - IDatabricksConnectionContext connectionContext) throws DatabricksHttpException { + IDatabricksConnectionContext connectionContext) throws DatabricksSSLException { // First check if a custom trust store is specified if (connectionContext.getSSLTrustStore() != null) { @@ -133,10 +133,10 @@ public static Registry createConnectionSocketFactoryReg * * @param connectionContext The connection context containing the trust store information. * @return A registry of connection socket factories. - * @throws DatabricksHttpException If there is an error setting up the trust store. + * @throws DatabricksSSLException If there is an error setting up the trust store. */ private static Registry createRegistryWithCustomTrustStore( - IDatabricksConnectionContext connectionContext) throws DatabricksHttpException { + IDatabricksConnectionContext connectionContext) throws DatabricksSSLException { try { KeyStore trustStore = loadTruststoreOrNull(connectionContext); @@ -173,10 +173,10 @@ private static Registry createRegistryWithCustomTrustSt * * @param connectionContext The connection context for configuration. * @return A registry of connection socket factories. - * @throws DatabricksHttpException If there is an error during setup. + * @throws DatabricksSSLException If there is an error during setup. */ private static Registry createRegistryWithSystemOrDefaultTrustStore( - IDatabricksConnectionContext connectionContext) throws DatabricksHttpException { + IDatabricksConnectionContext connectionContext) throws DatabricksSSLException { // Check if we should use the system property trust store based on useSystemTrustStore String sysTrustStore = null; @@ -201,11 +201,11 @@ private static Registry createRegistryWithSystemOrDefau * @param connectionContext The connection context for configuration. * @param sysTrustStore The path to the system property trust store. * @return A registry of connection socket factories. - * @throws DatabricksHttpException If there is an error during setup. + * @throws DatabricksSSLException If there is an error during setup. */ private static Registry createRegistryWithSystemPropertyTrustStore( IDatabricksConnectionContext connectionContext, String sysTrustStore) - throws DatabricksHttpException { + throws DatabricksSSLException { try { LOGGER.info( @@ -237,7 +237,7 @@ private static Registry createRegistryWithSystemPropert Set trustAnchors = getTrustAnchorsFromTrustStore(trustStore); return createRegistryFromTrustAnchors( trustAnchors, connectionContext, "system property trust store: " + sysTrustStore); - } catch (DatabricksHttpException + } catch (DatabricksSSLException | KeyStoreException | NoSuchAlgorithmException | CertificateException @@ -252,10 +252,10 @@ private static Registry createRegistryWithSystemPropert * * @param connectionContext The connection context for configuration. * @return A registry of connection socket factories. - * @throws DatabricksHttpException If there is an error during setup. + * @throws DatabricksSSLException If there is an error during setup. */ private static Registry createRegistryWithJdkDefaultTrustStore( - IDatabricksConnectionContext connectionContext) throws DatabricksHttpException { + IDatabricksConnectionContext connectionContext) throws DatabricksSSLException { try { if (connectionContext.useSystemTrustStore()) { @@ -269,7 +269,7 @@ private static Registry createRegistryWithJdkDefaultTru Set systemTrustAnchors = getTrustAnchorsFromTrustStore(null); return createRegistryFromTrustAnchors( systemTrustAnchors, connectionContext, "JDK default trust store (cacerts)"); - } catch (DatabricksHttpException e) { + } catch (DatabricksSSLException e) { handleError("Error while setting up JDK default trust store", e); } return null; @@ -282,15 +282,15 @@ private static Registry createRegistryWithJdkDefaultTru * @param connectionContext The connection context for configuration. * @param sourceDescription A description of the trust store source for logging. * @return A registry of connection socket factories. - * @throws DatabricksHttpException If there is an error during setup. + * @throws DatabricksSSLException If there is an error during setup. */ private static Registry createRegistryFromTrustAnchors( Set trustAnchors, IDatabricksConnectionContext connectionContext, String sourceDescription) - throws DatabricksHttpException { + throws DatabricksSSLException { if (trustAnchors == null || trustAnchors.isEmpty()) { - throw new DatabricksHttpException( + throw new DatabricksSSLException( sourceDescription + " contains no trust anchors", DatabricksDriverErrorCode.SSL_HANDSHAKE_ERROR); } @@ -350,10 +350,10 @@ private static Registry createSocketFactoryRegistry( * @return An array of key managers, or null if the key store is null. * @throws NoSuchAlgorithmException If the algorithm for the key manager factory is not available. * @throws KeyStoreException If there is an error accessing the key store. - * @throws DatabricksHttpException If there is an error creating the key managers. + * @throws DatabricksSSLException If there is an error creating the key managers. */ private static KeyManager[] createKeyManagers(KeyStore keyStore, char[] keyStorePassword) - throws NoSuchAlgorithmException, KeyStoreException, DatabricksHttpException { + throws NoSuchAlgorithmException, KeyStoreException, DatabricksSSLException { try { KeyManagerFactory kmf = KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm()); @@ -363,7 +363,7 @@ private static KeyManager[] createKeyManagers(KeyStore keyStore, char[] keyStore } catch (UnrecoverableKeyException e) { String errorMessage = "Failed to initialize key managers: " + e.getMessage(); LOGGER.error(errorMessage); - throw new DatabricksHttpException( + throw new DatabricksSSLException( errorMessage, e, DatabricksDriverErrorCode.SSL_HANDSHAKE_ERROR); } } @@ -406,7 +406,7 @@ private static TrustManager[] createTrustManagers( Set trustAnchors, boolean checkCertificateRevocation, boolean acceptUndeterminedCertificateRevocation) - throws NoSuchAlgorithmException, InvalidAlgorithmParameterException, DatabricksHttpException { + throws NoSuchAlgorithmException, InvalidAlgorithmParameterException, DatabricksSSLException { // Always use the custom trust manager with trust anchors CertPathTrustManagerParameters trustManagerParams = @@ -446,10 +446,10 @@ private static X509TrustManager findX509TrustManager(TrustManager[] trustManager * * @param connectionContext The connection context containing trust store configuration. * @return The loaded KeyStore or null if it could not be loaded. - * @throws DatabricksHttpException If there is an error during loading. + * @throws DatabricksSSLException If there is an error during loading. */ public static KeyStore loadTruststoreOrNull(IDatabricksConnectionContext connectionContext) - throws DatabricksHttpException { + throws DatabricksSSLException { String trustStorePath = connectionContext.getSSLTrustStore(); if (trustStorePath == null) { LOGGER.debug("No truststore path specified in connection url"); @@ -461,8 +461,7 @@ public static KeyStore loadTruststoreOrNull(IDatabricksConnectionContext connect if (!trustStoreFile.exists()) { String errorMessage = "Specified trust store file does not exist: " + trustStorePath; LOGGER.error(errorMessage); - throw new DatabricksHttpException( - errorMessage, DatabricksDriverErrorCode.SSL_HANDSHAKE_ERROR); + throw new DatabricksSSLException(errorMessage, DatabricksDriverErrorCode.SSL_HANDSHAKE_ERROR); } char[] password = null; @@ -487,7 +486,7 @@ public static KeyStore loadTruststoreOrNull(IDatabricksConnectionContext connect + ": " + e.getMessage(); LOGGER.error(errorMessage); - throw new DatabricksHttpException( + throw new DatabricksSSLException( errorMessage, e, DatabricksDriverErrorCode.SSL_HANDSHAKE_ERROR); } } @@ -498,10 +497,10 @@ public static KeyStore loadTruststoreOrNull(IDatabricksConnectionContext connect * * @param connectionContext The connection context containing key store configuration. * @return The loaded KeyStore or null if no key store was specified or it could not be loaded. - * @throws DatabricksHttpException If there is an error during loading. + * @throws DatabricksSSLException If there is an error during loading. */ public static KeyStore loadKeystoreOrNull(IDatabricksConnectionContext connectionContext) - throws DatabricksHttpException { + throws DatabricksSSLException { String keyStorePath = connectionContext.getSSLKeyStore(); if (keyStorePath == null) { LOGGER.debug("No keystore path specified in connection context"); @@ -513,8 +512,7 @@ public static KeyStore loadKeystoreOrNull(IDatabricksConnectionContext connectio if (!keyStoreFile.exists()) { String errorMessage = "Specified key store file does not exist: " + keyStorePath; LOGGER.error(errorMessage); - throw new DatabricksHttpException( - errorMessage, DatabricksDriverErrorCode.SSL_HANDSHAKE_ERROR); + throw new DatabricksSSLException(errorMessage, DatabricksDriverErrorCode.SSL_HANDSHAKE_ERROR); } // Check if keystore password is provided, which is required for accessing private keys @@ -523,8 +521,7 @@ public static KeyStore loadKeystoreOrNull(IDatabricksConnectionContext connectio String errorMessage = "Key store password is required when a key store is specified: " + keyStorePath; LOGGER.error(errorMessage); - throw new DatabricksHttpException( - errorMessage, DatabricksDriverErrorCode.SSL_HANDSHAKE_ERROR); + throw new DatabricksSSLException(errorMessage, DatabricksDriverErrorCode.SSL_HANDSHAKE_ERROR); } char[] password = keyStorePassword.toCharArray(); @@ -560,7 +557,7 @@ public static KeyStore loadKeystoreOrNull(IDatabricksConnectionContext connectio .append(": ") .append(e.getMessage()); LOGGER.error(errorMessage.toString(), e); - throw new DatabricksHttpException( + throw new DatabricksSSLException( errorMessage.toString(), e, DatabricksDriverErrorCode.SSL_HANDSHAKE_ERROR); } } @@ -570,10 +567,10 @@ public static KeyStore loadKeystoreOrNull(IDatabricksConnectionContext connectio * * @param trustStore The KeyStore from which to extract trust anchors. * @return A Set of TrustAnchor objects extracted from the KeyStore. - * @throws DatabricksHttpException If there is an error during extraction. + * @throws DatabricksSSLException If there is an error during extraction. */ public static Set getTrustAnchorsFromTrustStore(KeyStore trustStore) - throws DatabricksHttpException { + throws DatabricksSSLException { try { TrustManagerFactory trustManagerFactory = TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm()); @@ -606,13 +603,13 @@ public static Set getTrustAnchorsFromTrustStore(KeyStore trustStore * @param acceptUndeterminedCertificateRevocation Whether to accept undetermined certificate * revocation status. * @return The trust manager parameters based on the input parameters. - * @throws DatabricksHttpException If there is an error during configuration. + * @throws DatabricksSSLException If there is an error during configuration. */ public static CertPathTrustManagerParameters buildTrustManagerParameters( Set trustAnchors, boolean checkCertificateRevocation, boolean acceptUndeterminedCertificateRevocation) - throws DatabricksHttpException { + throws DatabricksSSLException { try { PKIXBuilderParameters pkixBuilderParameters = new PKIXBuilderParameters(trustAnchors, new X509CertSelector()); @@ -650,11 +647,11 @@ public static CertPathTrustManagerParameters buildTrustManagerParameters( * * @param errorMessage The error message to log. * @param e The exception to log and throw. - * @throws DatabricksHttpException The wrapped exception. + * @throws DatabricksSSLException The wrapped exception. */ - private static void handleError(String errorMessage, Exception e) throws DatabricksHttpException { + private static void handleError(String errorMessage, Exception e) throws DatabricksSSLException { LOGGER.error(errorMessage, e); - throw new DatabricksHttpException( + throw new DatabricksSSLException( errorMessage, e, DatabricksDriverErrorCode.SSL_HANDSHAKE_ERROR); } } diff --git a/src/main/java/com/databricks/jdbc/dbclient/impl/http/DatabricksHttpClient.java b/src/main/java/com/databricks/jdbc/dbclient/impl/http/DatabricksHttpClient.java index da854eb1ad..0addf55e07 100644 --- a/src/main/java/com/databricks/jdbc/dbclient/impl/http/DatabricksHttpClient.java +++ b/src/main/java/com/databricks/jdbc/dbclient/impl/http/DatabricksHttpClient.java @@ -14,6 +14,7 @@ import com.databricks.jdbc.exception.DatabricksDriverException; import com.databricks.jdbc.exception.DatabricksHttpException; import com.databricks.jdbc.exception.DatabricksRetryHandlerException; +import com.databricks.jdbc.exception.DatabricksSSLException; import com.databricks.jdbc.log.JdbcLogger; import com.databricks.jdbc.log.JdbcLoggerFactory; import com.databricks.jdbc.model.telemetry.enums.DatabricksDriverErrorCode; @@ -135,7 +136,7 @@ private PoolingHttpClientConnectionManager initializeConnectionManager( connectionManager.setMaxTotal(DEFAULT_MAX_HTTP_CONNECTIONS); connectionManager.setDefaultMaxPerRoute(DEFAULT_MAX_HTTP_CONNECTIONS_PER_ROUTE); return connectionManager; - } catch (DatabricksHttpException e) { + } catch (DatabricksSSLException e) { LOGGER.error("Failed to initialize HTTP connection manager", e); // Currently only SSL Handshake failure causes this exception. throw new DatabricksDriverException( diff --git a/src/main/java/com/databricks/jdbc/exception/DatabricksSSLException.java b/src/main/java/com/databricks/jdbc/exception/DatabricksSSLException.java new file mode 100644 index 0000000000..d315bb45e6 --- /dev/null +++ b/src/main/java/com/databricks/jdbc/exception/DatabricksSSLException.java @@ -0,0 +1,24 @@ +package com.databricks.jdbc.exception; + +import com.databricks.jdbc.model.telemetry.enums.DatabricksDriverErrorCode; + +/** Exception class to handle SSL/TLS configuration and handshake errors. */ +public class DatabricksSSLException extends DatabricksSQLException { + + public DatabricksSSLException( + String message, Throwable cause, DatabricksDriverErrorCode sqlCode) { + super(message, cause, sqlCode); + } + + public DatabricksSSLException(String message, DatabricksDriverErrorCode internalCode) { + super(message, null, internalCode.toString()); + } + + public DatabricksSSLException(String message, String sqlState) { + super(message, null, sqlState); + } + + public DatabricksSSLException(String message, Throwable throwable, String sqlState) { + super(message, throwable, sqlState); + } +} diff --git a/src/test/java/com/databricks/jdbc/auth/SSLConnectionParametersTest.java b/src/test/java/com/databricks/jdbc/auth/SSLConnectionParametersTest.java index ec1470b111..9b764d30ac 100644 --- a/src/test/java/com/databricks/jdbc/auth/SSLConnectionParametersTest.java +++ b/src/test/java/com/databricks/jdbc/auth/SSLConnectionParametersTest.java @@ -7,8 +7,8 @@ import com.databricks.jdbc.api.internal.IDatabricksConnectionContext; import com.databricks.jdbc.common.util.SocketFactoryUtil; import com.databricks.jdbc.dbclient.impl.common.ConfiguratorUtils; -import com.databricks.jdbc.exception.DatabricksHttpException; import com.databricks.jdbc.exception.DatabricksSQLException; +import com.databricks.jdbc.exception.DatabricksSSLException; import java.security.cert.X509Certificate; import java.util.Properties; import javax.net.ssl.X509TrustManager; @@ -32,7 +32,7 @@ public void setUp() { } @Test - public void testGetBaseConnectionManagerWithDefaultSettings() throws DatabricksHttpException { + public void testGetBaseConnectionManagerWithDefaultSettings() throws DatabricksSSLException { when(mockContext.allowSelfSignedCerts()).thenReturn(false); when(mockContext.useSystemTrustStore()).thenReturn(false); when(mockContext.getSSLTrustStore()).thenReturn(null); @@ -46,7 +46,7 @@ public void testGetBaseConnectionManagerWithDefaultSettings() throws DatabricksH } @Test - public void testGetBaseConnectionManagerWithSelfSignedCerts() throws DatabricksHttpException { + public void testGetBaseConnectionManagerWithSelfSignedCerts() throws DatabricksSSLException { when(mockContext.allowSelfSignedCerts()).thenReturn(true); PoolingHttpClientConnectionManager manager = @@ -68,7 +68,7 @@ public void testGetBaseConnectionManagerWithCustomTrustStore() { try { ConfiguratorUtils.getBaseConnectionManager(mockContext); fail("Should throw exception for non-existent trust store"); - } catch (DatabricksHttpException e) { + } catch (DatabricksSSLException e) { assertTrue( e.getMessage() .contains("Error while setting up custom trust store: /path/to/truststore.jks"), @@ -88,8 +88,12 @@ public void testGetTrustAllSocketFactoryRegistry() { @Test public void testGetConnectionSocketFactoryRegistryWithSelfSignedCerts() - throws DatabricksHttpException { - when(mockContext.allowSelfSignedCerts()).thenReturn(true); + throws DatabricksSSLException { + when(mockContext.allowSelfSignedCerts()).thenReturn(false); + when(mockContext.useSystemTrustStore()).thenReturn(false); + when(mockContext.getSSLTrustStore()).thenReturn(null); + when(mockContext.checkCertificateRevocation()).thenReturn(false); + when(mockContext.acceptUndeterminedCertificateRevocation()).thenReturn(false); Registry registry = ConfiguratorUtils.createConnectionSocketFactoryRegistry(mockContext); diff --git a/src/test/java/com/databricks/jdbc/dbclient/impl/common/ClientConfiguratorTest.java b/src/test/java/com/databricks/jdbc/dbclient/impl/common/ClientConfiguratorTest.java index a091e37ca7..bd3657ab28 100644 --- a/src/test/java/com/databricks/jdbc/dbclient/impl/common/ClientConfiguratorTest.java +++ b/src/test/java/com/databricks/jdbc/dbclient/impl/common/ClientConfiguratorTest.java @@ -13,9 +13,9 @@ import com.databricks.jdbc.common.AuthFlow; import com.databricks.jdbc.common.AuthMech; import com.databricks.jdbc.common.DatabricksJdbcConstants; -import com.databricks.jdbc.exception.DatabricksHttpException; import com.databricks.jdbc.exception.DatabricksParsingException; import com.databricks.jdbc.exception.DatabricksSQLException; +import com.databricks.jdbc.exception.DatabricksSSLException; import com.databricks.sdk.WorkspaceClient; import com.databricks.sdk.core.DatabricksConfig; import com.databricks.sdk.core.DatabricksException; @@ -40,7 +40,7 @@ public class ClientConfiguratorTest { @Test void getWorkspaceClient_PAT_AuthenticatesWithAccessToken() - throws DatabricksParsingException, DatabricksHttpException { + throws DatabricksParsingException, DatabricksSSLException { when(mockContext.getAuthMech()).thenReturn(AuthMech.PAT); when(mockContext.getHostUrl()).thenReturn("https://pat.databricks.com"); when(mockContext.getToken()).thenReturn("pat-token"); @@ -58,7 +58,7 @@ void getWorkspaceClient_PAT_AuthenticatesWithAccessToken() @Test void getWorkspaceClient_OAuthWithTokenPassthrough_AuthenticatesCorrectly() - throws DatabricksParsingException, DatabricksHttpException { + throws DatabricksParsingException, DatabricksSSLException { when(mockContext.getAuthMech()).thenReturn(AuthMech.OAUTH); when(mockContext.getAuthFlow()).thenReturn(AuthFlow.TOKEN_PASSTHROUGH); when(mockContext.getHostUrl()).thenReturn("https://oauth-token.databricks.com"); @@ -77,7 +77,7 @@ void getWorkspaceClient_OAuthWithTokenPassthrough_AuthenticatesCorrectly() @Test void getWorkspaceClient_OAuthWithClientCredentials_AuthenticatesCorrectly() - throws DatabricksParsingException, DatabricksHttpException { + throws DatabricksParsingException, DatabricksSSLException { when(mockContext.getAuthMech()).thenReturn(AuthMech.OAUTH); when(mockContext.getAuthFlow()).thenReturn(AuthFlow.CLIENT_CREDENTIALS); when(mockContext.getHostForOAuth()).thenReturn("https://oauth-client.databricks.com"); @@ -98,7 +98,7 @@ void getWorkspaceClient_OAuthWithClientCredentials_AuthenticatesCorrectly() @Test void getWorkspaceClient_OAuthWithClientCredentials_AuthenticatesCorrectlyGCP() - throws DatabricksParsingException, DatabricksHttpException { + throws DatabricksParsingException, DatabricksSSLException { when(mockContext.getAuthMech()).thenReturn(AuthMech.OAUTH); when(mockContext.getAuthFlow()).thenReturn(AuthFlow.CLIENT_CREDENTIALS); when(mockContext.getHostForOAuth()).thenReturn("https://oauth-client.databricks.com"); @@ -118,7 +118,7 @@ void getWorkspaceClient_OAuthWithClientCredentials_AuthenticatesCorrectlyGCP() @Test void getWorkspaceClient_OAuthWithClientCredentials_AuthenticatesCorrectlyWithJWT() - throws DatabricksParsingException, DatabricksHttpException { + throws DatabricksParsingException, DatabricksSSLException { when(mockContext.getConnectionUuid()).thenReturn("connection-uuid"); when(mockContext.getAuthMech()).thenReturn(AuthMech.OAUTH); when(mockContext.getAuthFlow()).thenReturn(AuthFlow.CLIENT_CREDENTIALS); @@ -169,7 +169,7 @@ void testM2MWithJWT() throws DatabricksSQLException { @Test void getWorkspaceClient_OAuthWithBrowserBasedAuthentication_AuthenticatesCorrectly() - throws DatabricksParsingException, DatabricksHttpException { + throws DatabricksParsingException, DatabricksSSLException { when(mockContext.getAuthMech()).thenReturn(AuthMech.OAUTH); when(mockContext.getAuthFlow()).thenReturn(AuthFlow.BROWSER_BASED_AUTHENTICATION); when(mockContext.getHostForOAuth()).thenReturn("https://oauth-browser.databricks.com"); @@ -194,7 +194,7 @@ void getWorkspaceClient_OAuthWithBrowserBasedAuthentication_AuthenticatesCorrect @Test void getWorkspaceClient_OAuthWithBrowserBasedAuthentication_WithDiscoveryURL_AuthenticatesCorrectly() - throws DatabricksParsingException, IOException, DatabricksHttpException { + throws DatabricksParsingException, IOException, DatabricksSSLException { when(mockContext.getAuthMech()).thenReturn(AuthMech.OAUTH); when(mockContext.getAuthFlow()).thenReturn(AuthFlow.BROWSER_BASED_AUTHENTICATION); when(mockContext.getHostForOAuth()).thenReturn("https://oauth-browser.databricks.com"); @@ -220,7 +220,7 @@ void getWorkspaceClient_OAuthWithBrowserBasedAuthentication_AuthenticatesCorrect } @Test - void testNonOauth() throws DatabricksHttpException { + void testNonOauth() throws DatabricksSSLException { when(mockContext.getAuthMech()).thenReturn(AuthMech.OTHER); when(mockContext.getHttpConnectionPoolSize()).thenReturn(100); configurator = new ClientConfigurator(mockContext); @@ -250,7 +250,7 @@ void testNonProxyHostsFormatConversion() { } @Test - void testSetupProxyConfig() throws DatabricksHttpException { + void testSetupProxyConfig() throws DatabricksSSLException { when(mockContext.getAuthMech()).thenReturn(AuthMech.PAT); when(mockContext.getUseProxy()).thenReturn(true); when(mockContext.getProxyHost()).thenReturn("proxy.host.com"); @@ -281,7 +281,7 @@ void testSetupProxyConfig() throws DatabricksHttpException { @Test void setupM2MConfig_WithAzureTenantId_ConfiguresCorrectly() - throws DatabricksParsingException, DatabricksHttpException { + throws DatabricksParsingException, DatabricksSSLException { when(mockContext.getAuthMech()).thenReturn(AuthMech.OAUTH); when(mockContext.getAuthFlow()).thenReturn(AuthFlow.CLIENT_CREDENTIALS); when(mockContext.getHostForOAuth()).thenReturn("https://azure-oauth.databricks.com"); @@ -445,7 +445,7 @@ void getWorkspaceClient_OAuthWithBrowserBasedAuthentication_SetsCustomRedirectUr @Test void testSetupU2MConfig_WithTokenCache() - throws DatabricksParsingException, DatabricksHttpException { + throws DatabricksParsingException, DatabricksSSLException { when(mockContext.getAuthMech()).thenReturn(AuthMech.OAUTH); when(mockContext.getAuthFlow()).thenReturn(AuthFlow.BROWSER_BASED_AUTHENTICATION); when(mockContext.getHostForOAuth()).thenReturn("https://oauth-browser.databricks.com"); @@ -493,7 +493,7 @@ void testSetupU2MConfig_WithTokenCacheNoPassphrase() throws DatabricksParsingExc @Test void testSetupU2MConfig_WithoutTokenCache() - throws DatabricksParsingException, DatabricksHttpException { + throws DatabricksParsingException, DatabricksSSLException { when(mockContext.getAuthMech()).thenReturn(AuthMech.OAUTH); when(mockContext.getAuthFlow()).thenReturn(AuthFlow.BROWSER_BASED_AUTHENTICATION); when(mockContext.getHostForOAuth()).thenReturn("https://oauth-browser.databricks.com"); diff --git a/src/test/java/com/databricks/jdbc/dbclient/impl/common/ConfiguratorUtilsTest.java b/src/test/java/com/databricks/jdbc/dbclient/impl/common/ConfiguratorUtilsTest.java index 7da0862c83..8e96bf5004 100644 --- a/src/test/java/com/databricks/jdbc/dbclient/impl/common/ConfiguratorUtilsTest.java +++ b/src/test/java/com/databricks/jdbc/dbclient/impl/common/ConfiguratorUtilsTest.java @@ -5,7 +5,7 @@ import com.databricks.jdbc.api.internal.IDatabricksConnectionContext; import com.databricks.jdbc.common.DatabricksJdbcConstants; -import com.databricks.jdbc.exception.DatabricksHttpException; +import com.databricks.jdbc.exception.DatabricksSSLException; import com.databricks.jdbc.log.JdbcLogger; import com.databricks.jdbc.log.JdbcLoggerFactory; import java.io.FileOutputStream; @@ -173,12 +173,12 @@ static void cleanup() { } @Test - void testGetConnectionSocketFactoryRegistry() throws DatabricksHttpException { + void testGetConnectionSocketFactoryRegistry() throws DatabricksSSLException { when(mockContext.getSSLTrustStorePassword()).thenReturn(TRUST_STORE_PASSWORD); when(mockContext.getSSLTrustStoreType()).thenReturn(TRUST_STORE_TYPE); when(mockContext.getSSLTrustStore()).thenReturn(EMPTY_TRUST_STORE_PATH); assertThrows( - DatabricksHttpException.class, + DatabricksSSLException.class, () -> ConfiguratorUtils.createConnectionSocketFactoryRegistry(mockContext), "the trustAnchors parameter must be non-empty"); @@ -192,7 +192,7 @@ void testGetConnectionSocketFactoryRegistry() throws DatabricksHttpException { } @Test - void testGetTrustAnchorsFromTrustStore() throws DatabricksHttpException { + void testGetTrustAnchorsFromTrustStore() throws DatabricksSSLException { when(mockContext.getSSLTrustStorePassword()).thenReturn(TRUST_STORE_PASSWORD); when(mockContext.getSSLTrustStoreType()).thenReturn(TRUST_STORE_TYPE); when(mockContext.getSSLTrustStore()).thenReturn(DUMMY_TRUST_STORE_PATH); @@ -205,7 +205,7 @@ void testGetTrustAnchorsFromTrustStore() throws DatabricksHttpException { @Test void testGetBaseConnectionManager_NoSSLTrustStoreAndRevocationCheckEnabled() - throws DatabricksHttpException { + throws DatabricksSSLException { // Define behavior for mock context when(mockContext.getSSLTrustStore()).thenReturn(null); when(mockContext.checkCertificateRevocation()).thenReturn(true); @@ -224,7 +224,7 @@ void testGetBaseConnectionManager_NoSSLTrustStoreAndRevocationCheckEnabled() } @Test - void testGetBaseConnectionManager_WithSSLTrustStore() throws DatabricksHttpException { + void testGetBaseConnectionManager_WithSSLTrustStore() throws DatabricksSSLException { try (MockedStatic configuratorUtils = mockStatic(ConfiguratorUtils.class)) { configuratorUtils .when(() -> ConfiguratorUtils.getBaseConnectionManager(mockContext)) @@ -246,7 +246,7 @@ void testGetBaseConnectionManager_WithSSLTrustStore() throws DatabricksHttpExcep } @Test - void testUseSystemTrustStoreFalse_NoCustomTrustStore() throws DatabricksHttpException { + void testUseSystemTrustStoreFalse_NoCustomTrustStore() throws DatabricksSSLException { // Scenario: useSystemTrustStore=false and no custom trust store provided // Should use JDK default trust store and ignore system property @@ -268,7 +268,7 @@ void testUseSystemTrustStoreFalse_NoCustomTrustStore() throws DatabricksHttpExce } @Test - void testAllowSelfSignedCerts() throws DatabricksHttpException { + void testAllowSelfSignedCerts() throws DatabricksSSLException { // Scenario: allowSelfSignedCerts=true // Should use trust-all socket factory @@ -281,7 +281,7 @@ void testAllowSelfSignedCerts() throws DatabricksHttpException { } @Test - void testCustomTrustStore_WithRevocationChecking() throws DatabricksHttpException { + void testCustomTrustStore_WithRevocationChecking() throws DatabricksSSLException { // Scenario: Custom trust store with certificate revocation checking when(mockContext.getSSLTrustStore()).thenReturn(DUMMY_TRUST_STORE_PATH); @@ -299,7 +299,7 @@ void testCustomTrustStore_WithRevocationChecking() throws DatabricksHttpExceptio } @Test - void testCreateRegistryWithSystemPropertyTrustStore() throws DatabricksHttpException { + void testCreateRegistryWithSystemPropertyTrustStore() throws DatabricksSSLException { // Save original system properties to restore later String originalTrustStore = System.getProperty("javax.net.ssl.trustStore"); String originalPassword = System.getProperty("javax.net.ssl.trustStorePassword"); @@ -345,7 +345,7 @@ void testCreateRegistryWithSystemPropertyTrustStore() throws DatabricksHttpExcep @Test void testCreateRegistryWithSystemPropertyTrustStore_WithRevocationChecking() - throws DatabricksHttpException { + throws DatabricksSSLException { // Save original system properties to restore later String originalTrustStore = System.getProperty("javax.net.ssl.trustStore"); String originalPassword = System.getProperty("javax.net.ssl.trustStorePassword"); @@ -398,9 +398,9 @@ void testNonExistentTrustStore() { String nonExistentPath = "/path/to/nonexistent/truststore.jks"; when(mockContextLocal.getSSLTrustStore()).thenReturn(nonExistentPath); - DatabricksHttpException exception = + DatabricksSSLException exception = assertThrows( - DatabricksHttpException.class, + DatabricksSSLException.class, () -> ConfiguratorUtils.loadTruststoreOrNull(mockContextLocal)); assertTrue( @@ -461,9 +461,9 @@ void testEmptyTrustAnchorsException() { // Test the behavior when trust anchors are empty Set emptyTrustAnchors = Collections.emptySet(); - DatabricksHttpException exception = + DatabricksSSLException exception = assertThrows( - DatabricksHttpException.class, + DatabricksSSLException.class, () -> ConfiguratorUtils.buildTrustManagerParameters(emptyTrustAnchors, true, false)); assertTrue( @@ -494,7 +494,7 @@ void testCreateSocketFactoryRegistry() throws Exception { } @Test - void testLoadKeystoreOrNull() throws DatabricksHttpException { + void testLoadKeystoreOrNull() throws DatabricksSSLException { when(mockContext.getSSLKeyStorePassword()).thenReturn(KEY_STORE_PASSWORD); when(mockContext.getSSLKeyStoreType()).thenReturn(KEY_STORE_TYPE); when(mockContext.getSSLKeyStore()).thenReturn(DUMMY_KEY_STORE_PATH); @@ -515,13 +515,13 @@ void testLoadKeystoreOrNull() throws DatabricksHttpException { void testNonExistentKeyStore() { when(mockContext.getSSLKeyStore()).thenReturn("non-existent-keystore.jks"); assertThrows( - DatabricksHttpException.class, + DatabricksSSLException.class, () -> ConfiguratorUtils.loadKeystoreOrNull(mockContext), "Should throw an exception for non-existent keystore"); } @Test - void testEmptyKeyStore() throws DatabricksHttpException { + void testEmptyKeyStore() throws DatabricksSSLException { when(mockContext.getSSLKeyStorePassword()).thenReturn(KEY_STORE_PASSWORD); when(mockContext.getSSLKeyStoreType()).thenReturn(KEY_STORE_TYPE); when(mockContext.getSSLKeyStore()).thenReturn(EMPTY_KEY_STORE_PATH); @@ -545,7 +545,7 @@ void testEmptyKeyStore() throws DatabricksHttpException { } @Test - void testClientCertificateAuthentication() throws DatabricksHttpException { + void testClientCertificateAuthentication() throws DatabricksSSLException { // Set up the mock context for both trust store and key store when(mockContext.getSSLTrustStorePassword()).thenReturn(TRUST_STORE_PASSWORD); when(mockContext.getSSLTrustStoreType()).thenReturn(TRUST_STORE_TYPE); @@ -573,7 +573,7 @@ void testMissingKeyStorePassword() { when(mockContext.getSSLKeyStorePassword()).thenReturn(null); assertThrows( - DatabricksHttpException.class, + DatabricksSSLException.class, () -> ConfiguratorUtils.loadKeystoreOrNull(mockContext), "Should throw an exception when key store password is missing"); } From ea15c6a990349a288e337151a49c16f1253d8edc Mon Sep 17 00:00:00 2001 From: Madhav Sainanee Date: Mon, 9 Jun 2025 13:05:36 +0530 Subject: [PATCH 11/11] Update error message in DatabricksClientConfiguratorManager to reflect SSL error handling instead of HTTP error. This change enhances clarity in logging and exception management related to SSL configuration failures. --- .../jdbc/common/DatabricksClientConfiguratorManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/com/databricks/jdbc/common/DatabricksClientConfiguratorManager.java b/src/main/java/com/databricks/jdbc/common/DatabricksClientConfiguratorManager.java index 9c2275ad93..534aea2d45 100644 --- a/src/main/java/com/databricks/jdbc/common/DatabricksClientConfiguratorManager.java +++ b/src/main/java/com/databricks/jdbc/common/DatabricksClientConfiguratorManager.java @@ -30,7 +30,7 @@ public ClientConfigurator getConfigurator(IDatabricksConnectionContext context) return new ClientConfigurator(context); } catch (DatabricksSSLException e) { String message = - String.format("client configurator failed due to HTTP error: %s", e.getMessage()); + String.format("client configurator failed due to SSL error: %s", e.getMessage()); LOGGER.error(e, message); throw new DatabricksDriverException(message, DatabricksDriverErrorCode.AUTH_ERROR); }