Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issue with truststore password being removed too early for XA transaction #1133

Merged
merged 16 commits into from
Sep 27, 2019
11 changes: 8 additions & 3 deletions azure-pipelines.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ allprojects {

test {
useJUnitPlatform {
excludeTags (hasProperty('excludedGroups') ? excludedGroups : 'xSQLv15','xGradle','NTLM')
excludeTags (hasProperty('excludedGroups') ? excludedGroups : 'xSQLv15','xGradle','reqExternalSetup','NTLM')
}
}

Expand Down Expand Up @@ -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')
}
}
}
Expand Down
5 changes: 3 additions & 2 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
ulvii marked this conversation as resolved.
Show resolved Hide resolved
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) {
peterbae marked this conversation as resolved.
Show resolved Hide resolved
controlConnectionProperties.setProperty(SQLServerDriverStringProperty.TRUST_STORE_PASSWORD.toString(),
trustStorePassword);
}

if (xaLogger.isLoggable(Level.FINER))
xaLogger.finer("Creating an internal control connection for" + toString());
physicalControlConnection = null;
Expand Down
55 changes: 55 additions & 0 deletions src/test/java/com/microsoft/sqlserver/jdbc/TestUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -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);
}
}
}
Original file line number Diff line number Diff line change
@@ -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();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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");
Expand Down