Skip to content

Fips#97

Merged
v-nisidh merged 12 commits intomicrosoft:devfrom
v-nisidh:fips
Jan 9, 2017
Merged

Fips#97
v-nisidh merged 12 commits intomicrosoft:devfrom
v-nisidh:fips

Conversation

@v-nisidh
Copy link
Copy Markdown
Contributor

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

Add FIPS Support.

{"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
Copy Markdown

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
Copy Markdown
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
Copy Markdown

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
Copy Markdown
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
Copy Markdown

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
Copy Markdown
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
Copy Markdown

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
Copy Markdown

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants