JDK-8129988 introduces a new behavior when reading the javax.net.ssl.trustStore property.

[Volker Simonis]

Hi,

I'd like to resurrect this old discussion which seems to have got lost.

David has analyzed and described the behavioral differences introduced
by JDK-8219988 around the handling of the "javax.net.ssl.trustStore"
property pretty well in his initial mail (see below).

The main difference is that before JDK-8219988, if
"javax.net.ssl.trustStore" was specified but did not exist, an empty
KeyStore was used. After "javax.net.ssl.trustStore", if
"javax.net.ssl.trustStore" was specified but did not exist, the system
will fall back to the default "lib/security/cacerts" store (for more
details see below).

The only documentation of the "javax.net.ssl.trustStore" property I
could find is Oracle's "Java Secure Socket Extension (JSSE) Reference
Guide" which clearly describes the pre-JDK-8219988 behavior:

"If the javax.net.ssl.trustStore property is defined but the specified
file does not exist, then a default TrustManager using an empty
keystore is created."

for jdk8 [1] as well as for 9 [2] (which introduced JDK-8219988), 11
[3] and 16 [4]

Because the new behavior has been around since jdk 9 I tend to
recommend that we change the documentation of post jdk-8 releases to
match the implementation. However, for jdk-8 this is a breaking,
backwards incompatible change so I think it would be reasonable to
change the implementation back to conform to the documentation and the
old behavior.

What do others think? Are there any reasons why this behavioral
changes have been made in the first place?

Thank you and best regards,
Volker

[1] https://docs.oracle.com/javase/8/docs/technotes/guides/security/jsse/JSSERefGuide.html#CustomizingStores
[2] https://docs.oracle.com/javase/9/security/java-secure-socket-extension-jsse-reference-guide.htm#JSSEC-GUID-7932AB21-2FED-402E-A806-3088402BAEA6
[3] https://docs.oracle.com/en/java/javase/11/security/java-secure-socket-extension-jsse-reference-guide.html#GUID-32CF3420-56E8-4BC5-8D3B-1F6B4692A290
[4] https://docs.oracle.com/en/java/javase/11/security/java-secure-socket-extension-jsse-reference-guide.html#GUID-32CF3420-56E8-4BC5-8D3B-1F6B4692A290

On Mon, Aug 12, 2019 at 4:10 PM Andrew John Hughes
<gnu.andrew at redhat.com> wrote:
>
> Forwarding to security-dev as this was backported from later JDK versions:
>
> On 09/08/2019 20:52, Alvarez, David wrote:
> > Hello,
> >
> > We have detected that JDK-8219988 [1], that has been included in OpenJDK 8u222
> > included a non-documented change in the behavior of the
> > javax.net.ssl.trustStore property. In previous versions, should this property
> > point to a non-existent file, an empty KeyStore would be used instead. [2]
> >
> > In newer versions, if the value of the property contains an invalid URL, the
> > code will instead fall back and use the lib/security/cacerts file. [3]
> >
> > However, there are two things about that change that caught our attention:
> > - Whenever there is no javax.net.ssl.trustStore property set, the code will
> > first look for lib/security/jssecacerts. If that file does not exist, then it
> > will look for lib/security/cacerts. However, when the property is set to an
> > invalid file, the fallback mechanism jumps directly to lib/security/cacerts,
> > never checking lib/security/jssecacerts.
> >
> > - This fallback mechanism for invalid javax.net.ssl.trustStore values reuses the
> > value of javax.net.ssl.trustStorePassword. If that property is set in
> > conjunction with an invalid value in javax.net.ssl.trustStore the specified
> > password will be used when attempting to read the lib/security/cacerts store.
> > It seems unclear why this path of action is chosen here.
> >
> > If the lib/security/cacerts has no password (and as far as I'm aware, that is
> > the case in the majority of OpenJDK distributions, if not all), the operation
> > will raise an exception. This exception mentions that 'Password verification
> > failed', hiding the underlying cause (the property pointing to a bad store).[4]
> >
> > Although the new behavior isn't necessarily wrong, I think there is room for
> > Improvement. Suggestions:
> >
> > - Make sure lib/security/jssecacerts is checked during the fallback process.
> > - Ignore the value of javax.net.ssl.trustStorePassword when we fallback to use
> > the bundled jssecacerts or cacerts file.
> > - Change the exception message to avoid confusion.
> >
> > I would like to have your opinion on this.
> >
> > Thanks,
> >
> > David
> >
> > --
> > [1] https://bugs.openjdk.java.net/browse/JDK-8129988
> > [2] http://hg.openjdk.java.net/jdk8u/jdk8u/jdk/file/ac2ef877d3e8/src/share/classes/sun/security/ssl/TrustManagerFactoryImpl.java#l132
> > [3] http://hg.openjdk.java.net/jdk8u/jdk8u/jdk/file/2a9bea6e5e03/src/share/classes/sun/security/ssl/TrustStoreManager.java#l128
> > [4] Here is a copy of the exception:
> > Caused by: java.io.IOException: Keystore was tampered with, or password was incorrect
> >     at sun.security.provider.JavaKeyStore.engineLoad(JavaKeyStore.java:785)
> >     at sun.security.provider.JavaKeyStore$JKS.engineLoad(JavaKeyStore.java:56)
> >     at sun.security.provider.KeyStoreDelegator.engineLoad(KeyStoreDelegator.java:224)
> >     at sun.security.provider.JavaKeyStore$DualFormatJKS.engineLoad(JavaKeyStore.java:70)
> >     at java.security.KeyStore.load(KeyStore.java:1445)
> >     at sun.security.ssl.TrustStoreManager$TrustAnchorManager.loadKeyStore(TrustStoreManager.java:367)
> >     at sun.security.ssl.TrustStoreManager$TrustAnchorManager.getTrustedCerts(TrustStoreManager.java:315)
> >     at sun.security.ssl.TrustStoreManager.getTrustedCerts(TrustStoreManager.java:59)
> >     at sun.security.ssl.TrustManagerFactoryImpl.engineInit(TrustManagerFactoryImpl.java:51)
> > Caused by: java.security.UnrecoverableKeyException: Password verification failed
> >     at sun.security.provider.JavaKeyStore.engineLoad(JavaKeyStore.java:783)
> >
> --
> Andrew :)
>
> Senior Free Java Software Engineer
> Red Hat, Inc. (http://www.redhat.com)
>
> PGP Key: ed25519/0xCFDA0F9B35964222 (hkp://keys.gnupg.net)
> Fingerprint = 5132 579D D154 0ED2 3E04  C5A0 CFDA 0F9B 3596 4222
> https://keybase.io/gnu_andrew
>