Skip to content

Commit

Permalink
Corrected credential cleanup; Addressed PR comments
Browse files Browse the repository at this point in the history
  • Loading branch information
tkyc committed Oct 12, 2023
1 parent 51ebf4e commit 6556067
Show file tree
Hide file tree
Showing 7 changed files with 22 additions and 42 deletions.
2 changes: 1 addition & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ allprojects {

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

Expand Down
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@
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)
Kerberos - - - - - For tests using Kerberos authentication (excluded by default)
kerberos - - - - - For tests using Kerberos authentication (excluded by default)
reqExternalSetup - For tests requiring external setup (excluded by default)
clientCertAuth - - For tests requiring client certificate authentication
setup (excluded by default) - - - - - - - - - - - - - - - - - - - - - - -
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -241,9 +241,9 @@ byte[] generateClientContext(byte[] pin, boolean[] done) throws SQLServerExcepti

void releaseClientContext() {
try {
if (null != peerCredentials && !isUserCreatedCredential) {
if (null != peerCredentials && !isUserCreatedCredential && !useDefaultNativeGSSCredential) {
peerCredentials.dispose();
} else if (null != peerCredentials && isUserCreatedCredential) {
} else if (null != peerCredentials && (isUserCreatedCredential || useDefaultNativeGSSCredential)) {
peerCredentials = null;
}
if (null != peerContext) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@ public final class SQLServerColumnEncryptionCertificateStoreProvider extends SQL
static final private java.util.logging.Logger windowsCertificateStoreLogger = java.util.logging.Logger
.getLogger("com.microsoft.sqlserver.jdbc.SQLServerColumnEncryptionCertificateStoreProvider");

static boolean isWindows;

String name = "MSSQL_CERTIFICATE_STORE";

static final String LOCAL_MACHINE_DIRECTORY = "LocalMachine";
Expand All @@ -29,14 +27,6 @@ public final class SQLServerColumnEncryptionCertificateStoreProvider extends SQL

private static final Lock lock = new ReentrantLock();

static {
if (System.getProperty("os.name").toLowerCase(Locale.ENGLISH).startsWith("windows")) {
isWindows = true;
} else {
isWindows = false;
}
}

/**
* Constructs a SQLServerColumnEncryptionCertificateStoreProvider.
*/
Expand Down Expand Up @@ -67,7 +57,7 @@ public byte[] decryptColumnEncryptionKey(String masterKeyPath, String encryption
windowsCertificateStoreLogger.entering(SQLServerColumnEncryptionCertificateStoreProvider.class.getName(),
"decryptColumnEncryptionKey", "Decrypting Column Encryption Key.");
byte[] plainCek;
if (isWindows) {
if (SQLServerConnection.isWindows) {
plainCek = decryptColumnEncryptionKeyWindows(masterKeyPath, encryptionAlgorithm,
encryptedColumnEncryptionKey);
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -982,10 +982,10 @@ IdleConnectionResiliency getSessionRecovery() {

/** global system ColumnEncryptionKeyStoreProviders */
static Map<String, SQLServerColumnEncryptionKeyStoreProvider> globalSystemColumnEncryptionKeyStoreProviders = new HashMap<>();
static boolean isWindows;

static boolean isWindows = System.getProperty("os.name").toLowerCase(Locale.ENGLISH).startsWith("windows");

static {
isWindows = System.getProperty("os.name").toLowerCase(Locale.ENGLISH).startsWith("windows");
if (isWindows) {
SQLServerColumnEncryptionCertificateStoreProvider provider = new SQLServerColumnEncryptionCertificateStoreProvider();
globalSystemColumnEncryptionKeyStoreProviders.put(provider.getName(), provider);
Expand Down
38 changes: 14 additions & 24 deletions src/test/java/com/microsoft/sqlserver/jdbc/KerberosTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import java.util.HashMap;
import java.util.Map;

@Tag(Constants.kerberos)
@RunWith(JUnitPlatform.class)
public class KerberosTest extends AbstractTest {

Expand All @@ -27,73 +28,63 @@ public static void setupTests() throws Exception {
setConnection();
}

@Tag(Constants.Kerberos)
@Test
public void testUseDefaultJaasConfigConnectionStringPropertyTrue() throws Exception {
String connectionStringUseDefaultJaasConfig = connectionStringKerberos + ";useDefaultJaasConfig=true;";

// Initial connection should succeed with default JAAS config
try (SQLServerConnection conn = (SQLServerConnection) DriverManager.getConnection(connectionStringUseDefaultJaasConfig)) {
ResultSet rs = conn.createStatement().executeQuery(authSchemeQuery);
rs.next();
Assertions.assertEquals(kerberosAuth, rs.getString(1));
}
createKerberosConnection(connectionStringUseDefaultJaasConfig);

// Attempt to overwrite JAAS config. Since useDefaultJaasConfig=true, this should have no effect
// and subsequent connections should succeed.
overwriteJaasConfig();

// New connection should successfully connect and continue to use the default JAAS config.
try (SQLServerConnection conn = (SQLServerConnection) DriverManager.getConnection(connectionStringUseDefaultJaasConfig)) {
ResultSet rs = conn.createStatement().executeQuery(authSchemeQuery);
rs.next();
Assertions.assertEquals(kerberosAuth, rs.getString(1));
}
createKerberosConnection(connectionStringUseDefaultJaasConfig);
}

@Tag(Constants.Kerberos)
@Test
public void testUseDefaultJaasConfigConnectionStringPropertyFalse() throws Exception {

// useDefaultJaasConfig=false by default
// Initial connection should succeed with default JAAS config
try (SQLServerConnection conn = (SQLServerConnection) DriverManager.getConnection(connectionStringKerberos)) {
ResultSet rs = conn.createStatement().executeQuery(authSchemeQuery);
rs.next();
Assertions.assertEquals(kerberosAuth, rs.getString(1));
}
createKerberosConnection(connectionStringKerberos);

// Overwrite JAAS config. Since useDefaultJaasConfig=false, overwriting should succeed and have an effect.
// Subsequent connections will fail.
overwriteJaasConfig();

// New connection should fail as it is attempting to connect using an overwritten JAAS config.
try (SQLServerConnection conn = (SQLServerConnection) DriverManager.getConnection(connectionStringKerberos)) {
try {
createKerberosConnection(connectionStringKerberos);
Assertions.fail(TestResource.getResource("R_expectedExceptionNotThrown"));
} catch (SQLServerException e) {
Assertions.assertTrue(e.getMessage()
.contains(TestResource.getResource("R_noLoginModulesConfiguredForJdbcDriver")));
}
}

@Tag(Constants.Kerberos)
@Test
public void testUseDefaultNativeGSSCredential() throws Exception {
// This is a negative test. This test should fail as expected as the JVM arg "-Dsun.security.jgss.native=true"
// isn't provided.
String connectionString = connectionStringKerberos + ";useDefaultGSSCredential=true;";

try (SQLServerConnection conn = (SQLServerConnection) DriverManager.getConnection(connectionString)) {
try {
createKerberosConnection(connectionString);
Assertions.fail(TestResource.getResource("R_expectedExceptionNotThrown"));
} catch (SQLServerException e) {
Assertions.assertEquals(e.getCause().getMessage(), TestResource.getResource("R_kerberosNativeGSSFailure"));
}
}

@Tag(Constants.Kerberos)
@Test
public void testBasicKerberosAuth() throws Exception {
try (SQLServerConnection conn = (SQLServerConnection) DriverManager.getConnection(connectionStringKerberos)) {
createKerberosConnection(connectionStringKerberos);
}

private static void createKerberosConnection(String connectionString) throws Exception {
try (SQLServerConnection conn = (SQLServerConnection) DriverManager.getConnection(connectionString)) {
ResultSet rs = conn.createStatement().executeQuery(authSchemeQuery);
rs.next();
Assertions.assertEquals(kerberosAuth, rs.getString(1));
Expand All @@ -109,7 +100,7 @@ private static void overwriteJaasConfig() {
AppConfigurationEntry.LoginModuleControlFlag.REQUIRED,
new HashMap<>());
Map<String, AppConfigurationEntry[]> configurationEntries = new HashMap<>();
configurationEntries.put("KAFKA_CLIENT_CONTEXT_NAME",
configurationEntries.put("CLIENT_CONTEXT_NAME",
new AppConfigurationEntry[] { kafkaClientConfigurationEntry });
Configuration.setConfiguration(new InternalConfiguration(configurationEntries));
}
Expand All @@ -125,6 +116,5 @@ private static class InternalConfiguration extends Configuration {
public AppConfigurationEntry[] getAppConfigurationEntry(String name) {
return this.configurationEntries.get(name);
}

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,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 Kerberos = "kerberos";
public static final String kerberos = "kerberos";
public static final String MSI = "MSI";
public static final String reqExternalSetup = "reqExternalSetup";
public static final String clientCertAuth = "clientCertAuth";
Expand Down

0 comments on commit 6556067

Please sign in to comment.