Fix issue with truststore password being removed too early for XA tra…

…nsaction (#1133) Fix | Fix issue with truststore password being removed too early for XA transaction
microsoft · Sep 27, 2019 · 8e61500 · 8e61500
1 parent 2deb6fa
commit 8e61500
Show file tree

Hide file tree

Showing 8 changed files with 137 additions and 11 deletions.
diff --git a/azure-pipelines.yml b/azure-pipelines.yml
@@ -29,11 +29,16 @@ jobs:
       Get-Content .\JavaKeyStoreBase.txt | Set-Content -Encoding utf8 JavaKeyStore.txt
       Remove-Item –path .\JavaKeyStoreBase.txt
     displayName: 'PowerShell Script'
+  - task: DownloadSecureFile@1
+    name: pkcs12_truststore
+    displayName: 'Download PKCS12 truststore file'
+    inputs:
+      secureFile: 'pkcs12_truststore'
   - task: Maven@3
     displayName: 'Maven build jre12'
     inputs:
       mavenPomFile: 'pom.xml'
-      goals: 'clean -Dmssql_jdbc_test_connection_properties=jdbc:sqlserver://$(Target_SQL)$(server_domain);$(database);$(user);$(password); install -Pjre12 -DuserNTLM=$(userNTLM) -DpasswordNTLM=$(passwordNTLM) -DdomainNTLM=$(domainNTLM) -DexcludedGroups=$(Ex_Groups)'
+      goals: 'clean -Dmssql_jdbc_test_connection_properties=jdbc:sqlserver://$(Target_SQL)$(server_domain);$(database);$(user);$(password); install -Pjre12 -DuserNTLM=$(userNTLM) -DpasswordNTLM=$(passwordNTLM) -DdomainNTLM=$(domainNTLM) -DexcludedGroups=$(Ex_Groups) -Dpkcs12_truststore_password=$(pkcs12_truststore_password) -Dpkcs12_truststore=$(pkcs12_truststore.secureFilePath)'
       testResultsFiles: '**/TEST-*.xml'
       testRunTitle: 'Maven build jre12'
       javaHomeOption: Path
@@ -42,7 +47,7 @@ jobs:
     displayName: 'Maven build jre11'
     inputs:
       mavenPomFile: 'pom.xml'
-      goals: 'clean -Dmssql_jdbc_test_connection_properties=jdbc:sqlserver://$(Target_SQL)$(server_domain);$(database);$(user);$(password); install -Pjre11 -DuserNTLM=$(userNTLM) -DpasswordNTLM=$(passwordNTLM) -DdomainNTLM=$(domainNTLM) -DexcludedGroups=$(Ex_Groups)'
+      goals: 'clean -Dmssql_jdbc_test_connection_properties=jdbc:sqlserver://$(Target_SQL)$(server_domain);$(database);$(user);$(password); install -Pjre11 -DuserNTLM=$(userNTLM) -DpasswordNTLM=$(passwordNTLM) -DdomainNTLM=$(domainNTLM) -DexcludedGroups=$(Ex_Groups) -Dpkcs12_truststore_password=$(pkcs12_truststore_password) -Dpkcs12_truststore=$(pkcs12_truststore.secureFilePath)'
       testResultsFiles: '**/TEST-*.xml'
       testRunTitle: 'Maven build jre11'
       javaHomeOption: Path
@@ -51,7 +56,7 @@ jobs:
     displayName: 'Maven build jre8'
     inputs:
       mavenPomFile: 'pom.xml'
-      goals: 'clean -Dmssql_jdbc_test_connection_properties=jdbc:sqlserver://$(Target_SQL)$(server_domain);$(database);$(user);$(password); install -Pjre8 -DuserNTLM=$(userNTLM) -DpasswordNTLM=$(passwordNTLM) -DdomainNTLM=$(domainNTLM) -DexcludedGroups=$(Ex_Groups)'
+      goals: 'clean -Dmssql_jdbc_test_connection_properties=jdbc:sqlserver://$(Target_SQL)$(server_domain);$(database);$(user);$(password); install -Pjre8 -DuserNTLM=$(userNTLM) -DpasswordNTLM=$(passwordNTLM) -DdomainNTLM=$(domainNTLM) -DexcludedGroups=$(Ex_Groups) -Dpkcs12_truststore_password=$(pkcs12_truststore_password) -Dpkcs12_truststore=$(pkcs12_truststore.secureFilePath)'
       testResultsFiles: '**/TEST-*.xml'
       testRunTitle: 'Maven build jre8'
       javaHomeOption: Path

diff --git a/build.gradle b/build.gradle
@@ -31,7 +31,7 @@ allprojects {
 
 test {
     useJUnitPlatform {
-        excludeTags (hasProperty('excludedGroups') ? excludedGroups : 'xSQLv15','xGradle','NTLM')
+        excludeTags (hasProperty('excludedGroups') ? excludedGroups : 'xSQLv15','xGradle','reqExternalSetup','NTLM')
     }
 }
 
@@ -70,7 +70,7 @@ if(hasProperty('buildProfile') && buildProfile == "jre8") {
 	targetCompatibility = 1.8
 	test {
 		useJUnitPlatform {
-			excludeTags (hasProperty('excludedGroups') ? excludedGroups : 'xSQLv15','xGradle','NTLM','xJDBC42')
+			excludeTags (hasProperty('excludedGroups') ? excludedGroups : 'xSQLv15','xGradle','NTLM','reqExternalSetup','xJDBC42')
 		}
 	}
 }

diff --git a/pom.xml b/pom.xml
@@ -49,10 +49,11 @@
 			xAzureSQLDB - - - - For tests not compatible with Azure SQL Database - - 
 			xAzureSQLDW - - - - For tests not compatible with Azure Data Warehouse - 
 			xAzureSQLMI - - - - For tests not compatible with Azure SQL Managed Instance 
-			NTLM - - - - - - For tests using NTLM Authentication mode (excluded by default) 
+			NTLM  - - - - - - - For tests using NTLM Authentication mode (excluded by default) 
+			reqExternalSetup  - For tests requiring external setup (excluded by default)
 			- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - 
 			Default testing enabled with SQL Server 2019 (SQLv14) -->
-		<excludedGroups>xSQLv15, NTLM</excludedGroups>
+		<excludedGroups>xSQLv15, NTLM, reqExternalSetup</excludedGroups>
 
 		<!-- Driver Dependencies -->
 		<azure.keyvault.version>1.2.1</azure.keyvault.version>

diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerDataSource.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerDataSource.java
@@ -358,6 +358,10 @@ public void setTrustStorePassword(String trustStorePassword) {
                 trustStorePassword);
     }
 
+    String getTrustStorePassword() {
+        return getStringProperty(connectionProps, SQLServerDriverStringProperty.TRUST_STORE_PASSWORD.toString(), null);
+    }
+
     @Override
     public void setHostNameInCertificate(String hostName) {
         setStringProperty(connectionProps, SQLServerDriverStringProperty.HOSTNAME_IN_CERTIFICATE.toString(), hostName);
@@ -478,19 +482,18 @@ public boolean getSendTimeAsDatetime() {
         return getBooleanProperty(connectionProps, SQLServerDriverBooleanProperty.SEND_TIME_AS_DATETIME.toString(),
                 SQLServerDriverBooleanProperty.SEND_TIME_AS_DATETIME.getDefaultValue());
     }
-    
+
     @Override
     public void setUseFmtOnly(boolean useFmtOnly) {
-        setBooleanProperty(connectionProps, SQLServerDriverBooleanProperty.USE_FMT_ONLY.toString(),
-                useFmtOnly);
+        setBooleanProperty(connectionProps, SQLServerDriverBooleanProperty.USE_FMT_ONLY.toString(), useFmtOnly);
     }
 
     @Override
     public boolean getUseFmtOnly() {
         return getBooleanProperty(connectionProps, SQLServerDriverBooleanProperty.USE_FMT_ONLY.toString(),
                 SQLServerDriverBooleanProperty.USE_FMT_ONLY.getDefaultValue());
     }
-    
+
     /**
      * Sets whether string parameters are sent to the server in UNICODE format.
      * 

diff --git a/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerXAConnection.java b/src/main/java/com/microsoft/sqlserver/jdbc/SQLServerXAConnection.java
@@ -52,6 +52,22 @@ public final class SQLServerXAConnection extends SQLServerPooledConnection imple
             controlConnectionProperties.setProperty(SQLServerDriverStringProperty.PASSWORD.toString(), pwd);
         }
 
+        // Add truststore password property for creating the control connection. This will be removed again
+        String trustStorePassword = ds.getTrustStorePassword();
+        if (null == trustStorePassword) {
+            // trustStorePassword can either come from the connection string or added via
+            // SQLServerXADataSource::setTrustStorePassword.
+            // if trustStorePassword is null at this point, then check the connection string.
+            Properties urlProps = Util.parseUrl(ds.getURL(), xaLogger);
+            trustStorePassword = urlProps.getProperty(SQLServerDriverStringProperty.TRUST_STORE_PASSWORD.toString());
+        }
+
+        // if trustStorePassword is still null, it wasn't provided. Do not set the property as null to avoid NPE.
+        if (null != trustStorePassword) {
+            controlConnectionProperties.setProperty(SQLServerDriverStringProperty.TRUST_STORE_PASSWORD.toString(),
+                    trustStorePassword);
+        }
+
         if (xaLogger.isLoggable(Level.FINER))
             xaLogger.finer("Creating an internal control connection for" + toString());
         physicalControlConnection = null;

diff --git a/src/test/java/com/microsoft/sqlserver/jdbc/TestUtils.java b/src/test/java/com/microsoft/sqlserver/jdbc/TestUtils.java
@@ -10,7 +10,12 @@
 
 import java.io.ByteArrayInputStream;
 import java.io.CharArrayReader;
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
 import java.net.URI;
+import java.security.KeyStore;
+import java.security.cert.CertificateFactory;
 import java.sql.CallableStatement;
 import java.sql.Connection;
 import java.sql.PreparedStatement;
@@ -21,6 +26,7 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Calendar;
+import java.util.List;
 import java.util.Locale;
 import java.util.ResourceBundle;
 
@@ -827,4 +833,53 @@ public static String formatErrorMsg(String s) {
     public static String addOrOverrideProperty(String connectionString, String property, String value) {
         return connectionString + ";" + property + "=" + value + ";";
     }
+
+    /**
+     * Creates a truststore and returns the path of it.
+     * 
+     * @param certificates
+     *        String list of certificates
+     * @param tsName
+     *        name of truststore to create
+     * @param tsPwd
+     *        password of truststore to set
+     * @param ksType
+     *        type of Keystore e.g PKCS12 or JKS
+     * @return Path of truststore that was created
+     * @throws Exception
+     */
+    public static String createTrustStore(List<String> certificates, String tsName, String tsPwd,
+            String ksType) throws Exception {
+        return (new TrustStore(certificates, tsName, tsPwd, ksType)).getFileName();
+    }
+
+    private static class TrustStore {
+        private File trustStoreFile;
+
+        TrustStore(List<String> certificateNames, String tsName, String tsPwd, String ksType) throws Exception {
+            trustStoreFile = File.createTempFile(tsName, null, new File("."));
+            trustStoreFile.deleteOnExit();
+            KeyStore ks = KeyStore.getInstance(ksType);
+            ks.load(null, null);
+
+            for (String certificateName : certificateNames) {
+                ks.setCertificateEntry(certificateName, getCertificate(certificateName));
+            }
+
+            FileOutputStream os = new FileOutputStream(trustStoreFile);
+            ks.store(os, tsPwd.toCharArray());
+            os.flush();
+            os.close();
+        }
+
+        final String getFileName() throws Exception {
+            return trustStoreFile.getCanonicalPath();
+        }
+
+        private static java.security.cert.Certificate getCertificate(String certname) throws Exception {
+            FileInputStream is = new FileInputStream(certname);
+            CertificateFactory cf = CertificateFactory.getInstance("X.509");
+            return cf.generateCertificate(is);
+        }
+    }
 }
diff --git a/src/test/java/com/microsoft/sqlserver/jdbc/connection/XADataSourceTest.java b/src/test/java/com/microsoft/sqlserver/jdbc/connection/XADataSourceTest.java
@@ -0,0 +1,44 @@
+/*
+ * Microsoft JDBC Driver for SQL Server Copyright(c) Microsoft Corporation All rights reserved. This program is made
+ * available under the terms of the MIT License. See the LICENSE file in the project root for more information.
+ */
+
+package com.microsoft.sqlserver.jdbc.connection;
+
+import javax.sql.XAConnection;
+
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+import org.junit.platform.runner.JUnitPlatform;
+import org.junit.runner.RunWith;
+
+import com.microsoft.sqlserver.jdbc.SQLServerXADataSource;
+import com.microsoft.sqlserver.testframework.AbstractTest;
+import com.microsoft.sqlserver.testframework.Constants;
+
+
+@RunWith(JUnitPlatform.class)
+@Tag(Constants.reqExternalSetup)
+public class XADataSourceTest extends AbstractTest {
+    private static String connectionUrlSSL = connectionString + ";encrypt=true;trustServerCertificate=false;";
+
+    /**
+     * Tests XA connection with PKCS12 truststore that is password protected.
+     * 
+     * Only re-populate the truststore if need arises in the future.
+     * TestUtils.createTrustStore() can be used to create the truststore.
+     * 
+     * @throws Exception
+     */
+    @Test
+    public void testPKCS12() throws Exception {
+        SQLServerXADataSource ds = new SQLServerXADataSource();
+
+        String trustStore = System.getProperty("pkcs12_truststore");
+        String url = connectionUrlSSL + "trustStore=" + trustStore + ";";
+        ds.setURL(url);
+        ds.setTrustStorePassword(System.getProperty("pkcs12_truststore_password"));
+        XAConnection connection = ds.getXAConnection();
+        connection.close();
+    }
+}
diff --git a/src/test/java/com/microsoft/sqlserver/testframework/Constants.java b/src/test/java/com/microsoft/sqlserver/testframework/Constants.java
@@ -25,6 +25,7 @@ private Constants() {}
      * xAzureSQLDW - - - - For tests not compatible with Azure Data Warehouse
      * xAzureSQLMI - - - - For tests not compatible with Azure SQL Managed Instance
      * NTLM  - - - - - - - For NTLM tests
+     * reqExternalSetup  - For tests requiring external setup
      * </pre>
      */
     public static final String xJDBC42 = "xJDBC42";
@@ -36,6 +37,7 @@ private Constants() {}
     public static final String xAzureSQLDW = "xAzureSQLDW";
     public static final String xAzureSQLMI = "xAzureSQLMI";
     public static final String NTLM = "NTLM";
+    public static final String reqExternalSetup = "reqExternalSetup";
 
     public static final ThreadLocalRandom RANDOM = ThreadLocalRandom.current();
     public static final Logger LOGGER = Logger.getLogger("AbstractTest");