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

trustStorePassword is null when using applicationIntent=ReadOnly #1539

Closed
jvtrigueros opened this issue Mar 11, 2021 · 2 comments · Fixed by #1565
Closed

trustStorePassword is null when using applicationIntent=ReadOnly #1539

jvtrigueros opened this issue Mar 11, 2021 · 2 comments · Fixed by #1565
Labels
Bug A bug in the driver. A high priority item that one can expect to be addressed quickly.

Comments

@jvtrigueros
Copy link

Driver version

2.9.1

SQL Server version

Microsoft SQL Server 2016 (SP2-CU12) (KB4536648) - 13.0.5698.0 (X64)
\tFeb 15 2020 01:47:30
\tCopyright (c) Microsoft Corporation
\tEnterprise Edition: Core-based Licensing (64-bit) on Windows Server 2016 Datacenter 10.0 (Build 14393: )

Client Operating System

RHEL 7

JAVA/JVM version

AdoptOpenJDK 11

Table schema

N/A

Problem description

Using applicationIntent=ReadOnly and configuring FIPS mode using the documentation fails to connect specifically when using a custom truststore via the trustStore and trustStorePassword properties.

  1. Expected behaviour:

Successful connection to secondary replica when given the hostname of the primary replica.

  1. Actual behaviour:

Fails to connect to secondary replica because an SSL session cannot be created.

  1. Error message/stack trace:
                              com.zaxxer.hikari.HikariDataSource.<init>     HikariDataSource.java:   81
                               com.zaxxer.hikari.pool.HikariPool.<init>           HikariPool.java:  115
                        com.zaxxer.hikari.pool.HikariPool.checkFailFast           HikariPool.java:  561
                      com.zaxxer.hikari.pool.HikariPool.createPoolEntry           HikariPool.java:  476
                           com.zaxxer.hikari.pool.PoolBase.newPoolEntry             PoolBase.java:  206
                          com.zaxxer.hikari.pool.PoolBase.newConnection             PoolBase.java:  364
                  com.zaxxer.hikari.util.DriverDataSource.getConnection     DriverDataSource.java:  138
                   com.microsoft.sqlserver.jdbc.SQLServerDriver.connect      SQLServerDriver.java:  881
               com.microsoft.sqlserver.jdbc.SQLServerConnection.connect  SQLServerConnection.java: 1291
       com.microsoft.sqlserver.jdbc.SQLServerConnection.connectInternal  SQLServerConnection.java: 2265
                 com.microsoft.sqlserver.jdbc.SQLServerConnection.login  SQLServerConnection.java: 2418
         com.microsoft.sqlserver.jdbc.SQLServerConnection.connectHelper  SQLServerConnection.java: 2760
                      com.microsoft.sqlserver.jdbc.TDSChannel.enableSSL             IOBuffer.java: 1735
                                            java.security.KeyStore.load
org.bouncycastle.jcajce.provider.ProvBCFKS$BCFIPSKeyStoreSpi.engineLoad
 org.bouncycastle.jcajce.provider.ProvBCFKS$BCFIPSKeyStoreSpi.verifyMac
java.io.IOException: BCFKS KeyStore corrupted: MAC calculation failed.
  1. Any other details that can be helpful:

From looking at the TRACE logs and source:

  • When using ReadWrite, one SSL handshake is made and the connection is successful
  • When using ReadOnly, two SSL handshakes are made the first one succeeds while the second one fails.
  • I noticed that the second SSL handshake was done against the secondary replica rather than my initial hostname (this is what we expect)

I verified using keytool that the provided trustStorePassword can open the trust store, and we can see the JDBC driver has no problem doing so either

Taking a look at IOBuffer.java, I noticed that the variable that stores the trustStorePassword is being cleared after a successful SSL handshake.

                    try {
                        ks.load(is, (null == trustStorePassword) ? null : trustStorePassword.toCharArray());
                    } finally {
                        // We are done with the trustStorePassword (if set). Clear it for better security.
                        con.activeConnectionProperties
                                .remove(SQLServerDriverStringProperty.TRUST_STORE_PASSWORD.toString());

                        // We are also done with the trust store input stream.
                        if (null != is) {
                            try {
                                is.close();
                            } catch (IOException e) {
                                if (logger.isLoggable(Level.FINE))
                                    logger.fine(toString() + " Ignoring error closing trust material InputStream...");
                            }
                        }
                    }

I built a version of mssql-jdbc that removes those two lines and my application is able to connect without any issues.

Given the comment above those lines, it's probably not the best solution. My guess is that the con object is being re-used when it should be recreated, but I wasn't able to confirm this.

JDBC trace logs

error.log*
In line 6, you can see the initial connection to the primary replica succeed properly. The ca.jks truststore is loaded and unlocked using the provided password. Not sure how the logic shakes out, but between this and line 238 an SSL connection attempt is made against the secondary replica with the same truststore but now this fails because the password is null.

* removed our internal hostnames, ports, and IPs

Reproduction code

I don't have exact code, all you need is to attempt to create a connection with these properties applicationIntent=ReadOnly;trustStore=ca.jks;trustStorePassword=changeit against an HA cluster that has primary and secondary replicas.

It's not trivial to reproduce locally, I'm testing against our production environment.

@peterbae
Copy link
Contributor

Thanks @jvtrigueros for reporting this issue. This is probably caused by the driver removing the password from memory after a successful connection (a requirement of the driver), but it looks like the driver is doing this too early in this specific scenario. The team will look into this soon.

@jvtrigueros
Copy link
Author

@peterbae that's exactly what's happening, thank you for the quick reply!

@lilgreenbird lilgreenbird added the Bug A bug in the driver. A high priority item that one can expect to be addressed quickly. label Mar 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in the driver. A high priority item that one can expect to be addressed quickly.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants