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

Fips #97

Merged
merged 12 commits into from
Jan 9, 2017
Merged

Fips #97

merged 12 commits into from
Jan 9, 2017

Conversation

v-nisidh
Copy link
Contributor

@v-nisidh v-nisidh commented Jan 5, 2017

Add FIPS Support.

@@ -194,6 +194,7 @@ static String getResource(String key)
{"R_packetSizePropertyDescription", "The network packet size used to communicate with SQL Server."},
{"R_encryptPropertyDescription", "Determines if Secure Sockets Layer (SSL) encryption should be used between the client and the server."},
{"R_trustServerCertificatePropertyDescription", "Determines if the driver should validate the SQL Server Secure Sockets Layer (SSL) certificate."},
{"R_trustStoreTypePropertyDescription", "Type of trust store type like JKS / PKCS12 or nay FIPS implementation KeyStore Type. etc."},
Copy link
Member

Choose a reason for hiding this comment

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

Are these the allowable types or are there others? http://docs.oracle.com/javase/8/docs/technotes/guides/security/StandardNames.html#KeyStore

Perhaps we should simply say 'Type of trust store, such as JKS or PKCS12.'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

BCFIPS supports both PKCS12 & BCFKS.

Ref: https://www.bouncycastle.org/fips/BCFipsDescription-20150501.pdf [2.4.8 FIPS Compliant Key Store]

* @return {@link Boolean} if provided char sequence is null or empty / blank
* @since 6.1.2
*/
public static boolean isEmpty(final CharSequence charSequence) {
Copy link
Member

Choose a reason for hiding this comment

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

Is it worth it to add an entire class for the isEmpty implementation - how much use do we expect to get out of this in the future?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In future we can add more functions if we are going to use this. As per need basis. This is kind of common utilities. In future, we can add functions like add padding, masking etc. I am seeing this class as place holder for all common / global functions related to String operations.

} else {
//If FIPS is not enabled issue warning and fall-back to non-FIPS mode.
StringBuilder sb = new StringBuilder();
sb.append("Could not switch to FIPS mode due to above settings: ");
Copy link
Member

Choose a reason for hiding this comment

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

We cannot build strings directly in the implementation - it needs to be localizable to languages other than English. Can we add a parameterized string resource and use that (you can see what is done for the error messages, for example)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will add localized warning message in coming sprint.


assertTrue(isFIPS("SunJSSE"),"FIPS Should be enabled");

// assertTrue(isFIPS("BCFIPS"),"FIPS Should be enabled");
Copy link
Member

Choose a reason for hiding this comment

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

What's this one for (is it needed still)?

}

/**
* It will test
Copy link
Member

Choose a reason for hiding this comment

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

Finish comment :)

# Resolve Conflicts:
#	src/main/java/com/microsoft/sqlserver/jdbc/SQLServerDriver.java
#	src/test/java/com/microsoft/sqlserver/testframework/AbstractTest.java
#	src/test/java/com/microsoft/sqlserver/testframework/PrepUtil.java
Cosmetic changes like add comments & remove deprecated method etc.
@v-nisidh v-nisidh merged commit fd8244a into microsoft:dev Jan 9, 2017
@ulvii ulvii mentioned this pull request Aug 12, 2017
@lilgreenbird lilgreenbird added this to Closed/Merged PRs in MSSQL JDBC Apr 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
MSSQL JDBC
  
Closed/Merged PRs
Development

Successfully merging this pull request may close these issues.

None yet

2 participants