Skip to content

Removing connection property - fipsProvider #460

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.

Sign up for GitHub

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

Merged
ulvii merged 5 commits into from
Aug 31, 2017
Merged

Removing connection property - fipsProvider #460

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
d97ea4d
Removing fipsProvider connection property
ulvii Aug 25, 2017
a5794e9
Minor fix
ulvii Aug 25, 2017
9403a5f
Merge branch 'dev' into FIPSFix
ulvii Aug 31, 2017
376a06d
Update IOBuffer.java
ulvii Aug 31, 2017
6b26648
update FipsTest
ulvii Aug 31, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 10 additions & 33 deletions src/main/java/com/microsoft/sqlserver/jdbc/IOBuffer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1579,7 +1579,6 @@ void enableSSL(String host,

boolean isFips = false;
String trustStoreType = null;
String fipsProvider = null;
String sslProtocol = null;

// If anything in here fails, terminate the connection and throw an exception
Expand All @@ -1598,12 +1597,11 @@ void enableSSL(String host,
trustStoreType = SQLServerDriverStringProperty.TRUST_STORE_TYPE.getDefaultValue();
}

fipsProvider = con.activeConnectionProperties.getProperty(SQLServerDriverStringProperty.FIPS_PROVIDER.toString());
isFips = Boolean.valueOf(con.activeConnectionProperties.getProperty(SQLServerDriverBooleanProperty.FIPS.toString()));
sslProtocol = con.activeConnectionProperties.getProperty(SQLServerDriverStringProperty.SSL_PROTOCOL.toString());

if (isFips) {
validateFips(fipsProvider, trustStoreType, trustStoreFileName);
validateFips(trustStoreType, trustStoreFileName);
}

assert TDS.ENCRYPT_OFF == con.getRequestedEncryptionLevel() || // Login only SSL
Expand Down Expand Up @@ -1649,12 +1647,8 @@ void enableSSL(String host,
if (logger.isLoggable(Level.FINEST))
logger.finest(toString() + " Finding key store interface");

if (isFips) {
ks = KeyStore.getInstance(trustStoreType, fipsProvider);
}
else {
ks = KeyStore.getInstance(trustStoreType);
}

ks = KeyStore.getInstance(trustStoreType);
ksProvider = ks.getProvider();

// Next, load up the trust store file from the specified location.
Expand Down Expand Up @@ -1828,57 +1822,40 @@ void enableSSL(String host,
* Valid FIPS settings:
* <LI>Encrypt should be true
* <LI>trustServerCertificate should be false
* <LI>if certificate is not installed FIPSProvider & TrustStoreType should be present.
* <LI>if certificate is not installed TrustStoreType should be present.
*
* @param fipsProvider
* FIPS Provider
* @param trustStoreType
* @param trustStoreFileName
* @throws SQLServerException
* @since 6.1.4
*/
private void validateFips(final String fipsProvider,
final String trustStoreType,
private void validateFips(final String trustStoreType,
final String trustStoreFileName) throws SQLServerException {
boolean isValid = false;
boolean isEncryptOn;
boolean isValidTrustStoreType;
boolean isValidTrustStore;
boolean isTrustServerCertificate;
boolean isValidFipsProvider;

String strError = SQLServerException.getErrString("R_invalidFipsConfig");

isEncryptOn = (TDS.ENCRYPT_ON == con.getRequestedEncryptionLevel());

// Here different FIPS provider supports different KeyStore type along with different JVM Implementation.
isValidFipsProvider = !StringUtils.isEmpty(fipsProvider);
isValidTrustStoreType = !StringUtils.isEmpty(trustStoreType);
isValidTrustStore = !StringUtils.isEmpty(trustStoreFileName);
isTrustServerCertificate = con.trustServerCertificate();

if (isEncryptOn && !isTrustServerCertificate) {
if (logger.isLoggable(Level.FINER))
logger.finer(toString() + " Found parameters are encrypt is true & trustServerCertificate false");

if (isEncryptOn && !isTrustServerCertificate) {
isValid = true;

if (isValidTrustStore) {
// In case of valid trust store we need to check fipsProvider and TrustStoreType.
if (!isValidFipsProvider || !isValidTrustStoreType) {
isValid = false;
strError = SQLServerException.getErrString("R_invalidFipsProviderConfig");

// In case of valid trust store we need to check TrustStoreType.
if (!isValidTrustStoreType) {
isValid = false;
if (logger.isLoggable(Level.FINER))
logger.finer(toString() + " FIPS provider & TrustStoreType should pass with TrustStore.");
logger.finer(toString() + "TrustStoreType is requiered alongside with TrustStore.");

@sehrope sehrope Aug 30, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/requiered/required/

}
if (logger.isLoggable(Level.FINER))
logger.finer(toString() + " Found FIPS parameters seems to be valid.");
}
}
else {
strError = SQLServerException.getErrString("R_invalidFipsEncryptConfig");
}

if (!isValid) {
throw new SQLServerException(strError, null, 0, null);
Expand Down
8 changes: 0 additions & 8 deletions src/main/java/com/microsoft/sqlserver/jdbc/SQLServerDataSource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -585,14 +585,6 @@ public boolean getFIPS() {
return getBooleanProperty(connectionProps, SQLServerDriverBooleanProperty.FIPS.toString(),
SQLServerDriverBooleanProperty.FIPS.getDefaultValue());
}

public void setFIPSProvider(String fipsProvider) {
setStringProperty(connectionProps, SQLServerDriverStringProperty.FIPS_PROVIDER.toString(), fipsProvider);
}

public String getFIPSProvider() {
return getStringProperty(connectionProps, SQLServerDriverStringProperty.FIPS_PROVIDER.toString(), null);
}

public void setSSLProtocol(String sslProtocol) {
setStringProperty(connectionProps, SQLServerDriverStringProperty.SSL_PROTOCOL.toString(), sslProtocol);
Expand Down
2 changes: 0 additions & 2 deletions src/main/java/com/microsoft/sqlserver/jdbc/SQLServerDriver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,6 @@ enum SQLServerDriverStringProperty
KEY_STORE_AUTHENTICATION ("keyStoreAuthentication", ""),
KEY_STORE_SECRET ("keyStoreSecret", ""),
KEY_STORE_LOCATION ("keyStoreLocation", ""),
FIPS_PROVIDER ("fipsProvider", ""),
SSL_PROTOCOL ("sslProtocol", SSLProtocol.TLS.toString()),
;

Expand Down Expand Up @@ -418,7 +417,6 @@ public final class SQLServerDriver implements java.sql.Driver {
new SQLServerDriverPropertyInfo(SQLServerDriverBooleanProperty.XOPEN_STATES.toString(), Boolean.toString(SQLServerDriverBooleanProperty.XOPEN_STATES.getDefaultValue()), false, TRUE_FALSE),
new SQLServerDriverPropertyInfo(SQLServerDriverStringProperty.AUTHENTICATION_SCHEME.toString(), SQLServerDriverStringProperty.AUTHENTICATION_SCHEME.getDefaultValue(), false, new String[] {AuthenticationScheme.javaKerberos.toString(),AuthenticationScheme.nativeAuthentication.toString()}),
new SQLServerDriverPropertyInfo(SQLServerDriverStringProperty.AUTHENTICATION.toString(), SQLServerDriverStringProperty.AUTHENTICATION.getDefaultValue(), false, new String[] {SqlAuthentication.NotSpecified.toString(),SqlAuthentication.SqlPassword.toString(),SqlAuthentication.ActiveDirectoryPassword.toString(),SqlAuthentication.ActiveDirectoryIntegrated.toString()}),
new SQLServerDriverPropertyInfo(SQLServerDriverStringProperty.FIPS_PROVIDER.toString(), SQLServerDriverStringProperty.FIPS_PROVIDER.getDefaultValue(), false, null),
new SQLServerDriverPropertyInfo(SQLServerDriverIntProperty.SOCKET_TIMEOUT.toString(), Integer.toString(SQLServerDriverIntProperty.SOCKET_TIMEOUT.getDefaultValue()), false, null),
new SQLServerDriverPropertyInfo(SQLServerDriverBooleanProperty.FIPS.toString(), Boolean.toString(SQLServerDriverBooleanProperty.FIPS.getDefaultValue()), false, TRUE_FALSE),
new SQLServerDriverPropertyInfo(SQLServerDriverBooleanProperty.ENABLE_PREPARE_ON_FIRST_PREPARED_STATEMENT.toString(), Boolean.toString(SQLServerDriverBooleanProperty.ENABLE_PREPARE_ON_FIRST_PREPARED_STATEMENT.getDefaultValue()), false,TRUE_FALSE),
Expand Down
7 changes: 2 additions & 5 deletions src/main/java/com/microsoft/sqlserver/jdbc/SQLServerResource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,6 @@ protected Object[][] getContents() {
{"R_keyStoreAuthenticationPropertyDescription", "The name that identifies a key store."},
{"R_keyStoreSecretPropertyDescription", "The authentication secret or information needed to locate the secret."},
{"R_keyStoreLocationPropertyDescription", "The key store location."},
{"R_fipsProviderPropertyDescription", "FIPS Provider."},
{"R_keyStoreAuthenticationNotSet", "\"keyStoreAuthentication\" connection string keyword must be specified, if \"{0}\" is specified."},
{"R_keyStoreSecretOrLocationNotSet", "Both \"keyStoreSecret\" and \"keyStoreLocation\" must be set, if \"keyStoreAuthentication=JavaKeyStorePassword\" has been specified in the connection string."},
{"R_certificateStoreInvalidKeyword", "Cannot set \"keyStoreSecret\", if \"keyStoreAuthentication=CertificateStore\" has been specified in the connection string."},
Expand All @@ -375,10 +374,8 @@ protected Object[][] getContents() {
{"R_TVPnotWorkWithSetObjectResultSet" , "setObject() with ResultSet is not supported for Table-Valued Parameter. Please use setStructured()."},
{"R_invalidQueryTimeout", "The queryTimeout {0} is not valid."},
{"R_invalidSocketTimeout", "The socketTimeout {0} is not valid."},
{"R_fipsPropertyDescription", "Determines if enable FIPS compliant SSL connection between the client and the server."},
{"R_invalidFipsConfig", "Could not enable FIPS."},
{"R_invalidFipsEncryptConfig", "Could not enable FIPS due to either encrypt is not true or using trusted certificate settings."},
{"R_invalidFipsProviderConfig", "Could not enable FIPS due to invalid FIPSProvider or TrustStoreType."},
{"R_fipsPropertyDescription", "Determines if FIPS mode is enabled."},
{"R_invalidFipsConfig", "Unable to verify FIPS mode settings."},
{"R_serverPreparedStatementDiscardThreshold", "The serverPreparedStatementDiscardThreshold {0} is not valid."},
{"R_statementPoolingCacheSize", "The statementPoolingCacheSize {0} is not valid."},
{"R_kerberosLoginFailedForUsername", "Cannot login with Kerberos principal {0}, check your credentials. {1}"},
Expand Down
53 changes: 4 additions & 49 deletions src/test/java/com/microsoft/sqlserver/jdbc/fips/FipsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,7 @@ public void fipsTrustServerCertificateTest() throws Exception {
}
catch (SQLServerException e) {
Assertions.assertTrue(
e.getMessage().contains("Could not enable FIPS due to either encrypt is not true or using trusted certificate settings."),
"Should create exception for invalid TrustServerCertificate value");
e.getMessage().contains("Unable to verify FIPS mode settings."));

@cheenamalhotra cheenamalhotra Aug 31, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any specific reason for removing "Should create exception for invalid TrustServerCertificate value" from line 56?

}
}

Expand All @@ -72,31 +71,11 @@ public void fipsEncryptTest() throws Exception {
}
catch (SQLServerException e) {
Assertions.assertTrue(
e.getMessage().contains("Could not enable FIPS due to either encrypt is not true or using trusted certificate settings."),
e.getMessage().contains("Unable to verify FIPS mode settings."),

@cheenamalhotra cheenamalhotra Aug 31, 2017 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would suggest getting these same String error messages from SQLServerResource.java in test cases, such that your code becomes:
e.getMessage().contains(SQLServerResource.getResource("R_invalidFipsConfig"), "Some..").
It would remove all possibilities of errors and add maintainability.

@ulvii ulvii Aug 31, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getResource() is not a public API

@cheenamalhotra cheenamalhotra Aug 31, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah.. I missed that, in that case we need a custom resource bundle for our test framework. I have created issue #466 to track this change.

"Should create exception for invalid encrypt value");
}
}

/**
* Test after removing FIPS PROVIDER
*
* @throws Exception
*/
@Test
public void fipsProviderTest() throws Exception {
try {
Properties props = buildConnectionProperties();
props.remove("fipsProvider");
props.setProperty("trustStore", "/SOME_PATH");
Connection con = PrepUtil.getConnection(connectionString, props);
Assertions.fail("It should fail as we are not passing appropriate params");
}
catch (SQLServerException e) {
Assertions.assertTrue(e.getMessage().contains("Could not enable FIPS due to invalid FIPSProvider or TrustStoreType"),
"Should create exception for invalid FIPSProvider");
}
}

/**
* Test after removing fips, encrypt & trustStore it should work appropriately.
*
Expand Down Expand Up @@ -124,7 +103,6 @@ public void fipsDataSourcePropertyTest() throws Exception {
SQLServerDataSource ds = new SQLServerDataSource();
setDataSourceProperties(ds);
ds.setFIPS(false);
ds.setFIPSProvider("");
ds.setEncrypt(false);
ds.setTrustStoreType("JKS");
Connection con = ds.getConnection();
Expand All @@ -148,32 +126,11 @@ public void fipsDatSourceEncrypt() {
}
catch (SQLServerException e) {
Assertions.assertTrue(
e.getMessage().contains("Could not enable FIPS due to either encrypt is not true or using trusted certificate settings."),
e.getMessage().contains("Unable to verify FIPS mode settings."),
"Should create exception for invalid encrypt value");
}
}

/**
* Test after removing FIPS PROVIDER
*
* @throws Exception
*/
@Test
public void fipsDataSourceProviderTest() throws Exception {
try {
SQLServerDataSource ds = new SQLServerDataSource();
setDataSourceProperties(ds);
ds.setFIPSProvider("");
ds.setTrustStore("/SOME_PATH");
Connection con = ds.getConnection();
Assertions.fail("It should fail as we are not passing appropriate params");
}
catch (SQLServerException e) {
Assertions.assertTrue(e.getMessage().contains("Could not enable FIPS due to invalid FIPSProvider or TrustStoreType"),
"Should create exception for invalid FIPSProvider");
}
}

/**
* Test after setting TrustServerCertificate as true.
*
Expand All @@ -190,7 +147,7 @@ public void fipsDataSourceTrustServerCertificateTest() throws Exception {
}
catch (SQLServerException e) {
Assertions.assertTrue(
e.getMessage().contains("Could not enable FIPS due to either encrypt is not true or using trusted certificate settings."),
e.getMessage().contains("Unable to verify FIPS mode settings."),
"Should create exception for invalid TrustServerCertificate value");
}
}
Expand All @@ -216,7 +173,6 @@ private void setDataSourceProperties(SQLServerDataSource ds) {
ds.setTrustServerCertificate(false);
ds.setIntegratedSecurity(false);
ds.setTrustStoreType("PKCS12");
ds.setFIPSProvider("BCFIPS");
}

/**
Expand All @@ -235,7 +191,6 @@ private Properties buildConnectionProperties() {

// For New Code
connectionProps.setProperty("trustStoreType", "PKCS12");
connectionProps.setProperty("fipsProvider", "BCFIPS");
connectionProps.setProperty("fips", "true");

return connectionProps;
Expand Down