Removing connection properties: fipsProvider and fips #433
Conversation
|
@ulvii, |
|
Hi @v-nisidh/ @nsidhaye, please feel free to leave feedback. |
Codecov Report
@@ Coverage Diff @@
## dev #433 +/- ##
============================================
- Coverage 45.95% 45.92% -0.04%
+ Complexity 2200 2197 -3
============================================
Files 108 108
Lines 25210 25167 -43
Branches 4164 4150 -14
============================================
- Hits 11585 11557 -28
- Misses 11702 11705 +3
+ Partials 1923 1905 -18
Continue to review full report at Codecov.
|
1 similar comment
Codecov Report
@@ Coverage Diff @@
## dev #433 +/- ##
============================================
- Coverage 45.95% 45.92% -0.04%
+ Complexity 2200 2197 -3
============================================
Files 108 108
Lines 25210 25167 -43
Branches 4164 4150 -14
============================================
- Hits 11585 11557 -28
- Misses 11702 11705 +3
+ Partials 1923 1905 -18
Continue to review full report at Codecov.
|
| else { | ||
| tm = new TrustManager[] {new HostNameOverrideX509TrustManager(this, (X509TrustManager) tm[0], host)}; | ||
| } | ||
| // if the host name in cert provided use it or use the host name |
There was a problem hiding this comment.
In case of FIPS our driver should delegate responsibility of validating certificate to JVM.
In this case we are using our own TrustManager i.e. HostNameOverrideX509TrustManager which will throw following exception :
FIPS mode: only SunJSSE TrustManagers may be used
If I am correctly remember in order to remove error FIPS mode: only SunJSSE TrustManagers may be used we reviewed our FIPS implementation and added fips attribute.
If you want to get rid of fips attribute then you have to come up with function which will detect if JVM is in FIPS mode or not. You can find logic in our test cases as well as on internet. And then in case of configured FIPS one should not used custom created TrustManagers.
I will suggest instead of merging back in master / dev branch come up with FIPS or FIPSRnD branch and test with different FIPS implementation. If users are comfortable with changes; then merge back in main stream.
Remember to test with at-least following FIPS implementations:
- BouncyCastle
- IBM
- OpenJDK 9 (Not Tested yet)
There was a problem hiding this comment.
Thank you for pointing this out. I am able to reproduce the exception, will investigate and get back to you.
| String trustStoreType = null; | ||
| String fipsProvider = null; |
There was a problem hiding this comment.
Removing fipsProvider will not directly impact. But think of following cases:
You can configure BouncyCastle FIPS provider in two ways :
- By configuration files (java.security)
- Run time by passing FIPS provider .
I added fipsProvider for 2nd case.
Also on Oracle site mentioned following:
Configuring SunJSSE for FIPS Mode
SunJSSE is configured in FIPS mode by associating it with an appropriate FIPS 140 certified cryptographic provider that supplies the implementations for all cryptographic algorithms required by SunJSSE. This can be done in one of the following ways:
-
edit the file ${java.home}/lib/security/java.security and modify the line that lists com.sun.net.ssl.internal.ssl.Provider to list the provider name of the FIPS 140 certified cryptographic provider. For example if the name of the cryptographic provider is SunPKCS11-NSS, change the line from
security.provider.4=com.sun.net.ssl.internal.ssl.Provider`
to
security.provider.4=com.sun.net.ssl.internal.ssl.Provider SunPKCS11-NSS
The class for the provider of the given name must also be listed as a security provider in the java.security file.
-
at runtime, call the constructor of the SunJSSE provider that takes a java.security.Provider object as a parameter. For example, if the variable cryptoProvider is a reference to the cryptographic provider, call new com.sun.net.ssl.internal.ssl.Provider(cryptoProvider).
-
at runtime, call the constructor of the SunJSSE provider that takes a String object as a parameter. For example if the cryptographic provider is called SunPKCS11-NSS call new com.sun.net.ssl.internal.ssl.Provider("SunPKCS11-NSS"). A provider with the specified name must be one of the configured security providers.
Within a given Java process, SunJSSE can be used either in FIPS mode or in default mode, but not both at the same time. Once SunJSSE has been initialized, it is not possible to change the mode. This means that if one of the runtime configuration options is used (option 2 or 3), the configuration must take place before any SSL/TLS operation.
Ref: Oracle FIPS
There was a problem hiding this comment.
Hi @nsidhaye ,
Thank you so much for detailed comments. We appreciate the time and effort.
As I mentioned above, if fipsProvider is not specified,KeyStore.getInstance() traverses the list of registered security Providers - Security.getProviders() and returns the matching KeyStore object.
Example, in case of option 3, user would make sure BouncyCastleFipsProvider is installed before creating a connection.
Security.addProvider(new org.bouncycastle.jcajce.provider.BouncyCastleFipsProvider());
Security.addProvider(new com.sun.net.ssl.internal.ssl.Provider ("BCFIPS"));
ds.setTrustStoreType("BCFKS");
connection = (SQLServerConnection) ds.getConnection();
When the drivers calls KeyStore.getInstance(trustStoreType) before TLS Handshake, BCFIPS will already be in the list of installed providers.
Please let me know if I am missing something.
There was a problem hiding this comment.
After further investigation, I figured out the root cause of the issue,
Provider IBMJCEFIPS does not contain a keystore. When an application calls ds.setFIPSProvider("IBMJCEFIPS"); , the driver throws an exception, because there are no matching keystore types for IBMJCEFIPS. The users should use non-IBMJCEFIPS keystores to store the keys. See this page. This means, the provider that contains keystore is not necessarily the same as the provider that contains FIPS-complaint algorithms. Below is a sample app that would work for IBM users with the current implementation.
ds.setTrustStoreType("JCEKS");
ds.setFIPS(true);
ds.setFIPSProvider("IBMJCE"); //Not IBMJCEFIPS
connection = (SQLServerConnection) ds.getConnection();
This way we are not introducing any breaking changes, but the connection property fipsProvider has a confusing name.
There was a problem hiding this comment.
@ulvi :
In this case as FIPSProvider is non-FIPS it's not fip connection. One can test it by FipsEnvTest#isFips
ds.setTrustStoreType("JCEKS");
ds.setFIPS(true);
ds.setFIPSProvider("IBMJCE"); //Not IBMJCEFIPS
connection = (SQLServerConnection) ds.getConnection();
For one of user I forked out SSLProtocol which having 2 fixes:
- Configurable SSL Protocol (Which you already worked on)
- Made FIPSProvider optional as IBMFIPS does not like FIPSProvider while calling
Keystore.getInstance(...). (Need to test what happens in JDK 9 & Bouncy Castle for configurable configuration)
Unfortunately, we did not finalize on solution with user, but this worked on user environment with appropriate SSL certificate.
If you use my SSLProtocol fork and use following code for IBM FIPS.
//ds.setTrustStoreType("JCEKS"); (As per my understanding IBM works well with JKS too in FIPS)
ds.setFIPS(true);
//ds.setFIPSProvider("IBMJCE"); For IBM FIPS one should not set FIPSProvider.
connection = (SQLServerConnection) ds.getConnection();
Possibly @v-xiangs might have some information. Feel free to call me.
There was a problem hiding this comment.
Hi @nsidhaye ,
Thank you for the comments. I am definitely open to a discussion over a call. Please email me your contact details at v-ulibra@microsoft.com.
In this case as FIPSProvider is non-FIPS it's not fip connection.
fipsProvider connection property is only used to load the keys. Users will still have to create the keys using a FIPS-approved provider. Non FIPS provider keystores can be used to store those keys and this will not affect FIPS compliant mode. Please, take a look at this article., especially the last section.
One can test it by FipsEnvTest#isFips
Of course this test will fail if you pass in non-FIPS provider. That test just checks whether a provider is FIPS compliant or not, it does not check if FIPS mode is enabled. As I mentioned above, non FIPS provider is used to only store the keys.
If you use my SSLProtocol fork and use following code for IBM FIPS.
Let me explain what happens when you don't set fipsProvider property. In this case, since you did not specify the provider, the driver will traverse the registered providers and find the matching keystore. So if you set keystoreType to JKS, the matching provider that contains this Keystore is a non-FIPS provider. You can put a breakpoint at this line and check the provider name.
Made FIPSProvider optional as IBMFIPS does not like FIPSProvider while calling Keystore.getInstance(...)
Keystore.getInstance(storeType, "IBMJCEFIPS") does not work, because IBMJCEFIPS does not contain any keystores. That's why we need to use non-FIPS provider to store the keys. If the given provider contains the specified storeType, Keystore.getInstance(storeType, provider) is perfectly valid. See this page.
Thanks again for valuable comments and talk to you soon :)
|
Closing this PR and creating a new one which removes |
This is a breaking change and should not be merged. Creating this PR early to allow discussions.
fipsProviderCurrently, this property is only used to specify the name of a provider which guarantees that the implementation of the type requested is from the named provider. Note that, the specified provider must still be registered in the security provider list.
If
fipsProvideris not specified,KeyStore.getInstancetraverses the list of registered security Providers, starting with the most preferred Provider.fips.Once FIPS mode is enabled for JAVA (either from JAVA options or java.security), all the driver needs to know is
trustStoreType. There is no need to setfipsto true, because the driver does not really enable FIPS.To sum up, after proposed changes, user will have to enable FIPS mode for JAVA and pass
trustStoreTypeto driver.