On Tue, 11 Jul 2023 18:09:26 GMT, Craig Andrews <d...@openjdk.org> wrote:
> When loading the default JVM trust store, if the JVM trust store contains an
> invalid certificate, the exception contains insufficient information to
> determine which certificate is invalid, making it very difficult to fix the
> problem.
>
> To reproduce the issue:
> 1. Modify the default JVM trust store to contain invalid information. A very
> easy way to do this on openjdk / red hat systems is to edit
> /etc/pki/ca-trust/extracted/java/cacerts and add garbage text to the file.
> 2. Run this code:
>
> TrustManagerFactory =
> TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm());
> // initializing the trust store with a null KeyStore will load the default
> JVM trust store
> tmf.init((KeyStore) null);
>
>
> This stack trace results:
>
> Caused by: java.security.KeyStoreException: problem accessing trust store
> at
> java.base/sun.security.ssl.TrustManagerFactoryImpl.engineInit(TrustManagerFactoryImpl.java:73)
> at
> java.base/javax.net.ssl.TrustManagerFactory.init(TrustManagerFactory.java:282)
> ... 81 common frames omitted
> Caused by: java.io.IOException: toDerInputStream rejects tag type 97
> at
> java.base/sun.security.util.DerValue.toDerInputStream(DerValue.java:1155)
> at
> java.base/sun.security.pkcs12.PKCS12KeyStore.engineLoad(PKCS12KeyStore.java:2013)
> at
> java.base/sun.security.util.KeyStoreDelegator.engineLoad(KeyStoreDelegator.java:221)
> at java.base/java.security.KeyStore.load(KeyStore.java:1473)
> at
> java.base/sun.security.ssl.TrustStoreManager$TrustAnchorManager.loadKeyStore(TrustStoreManager.java:390)
> at
> java.base/sun.security.ssl.TrustStoreManager$TrustAnchorManager.getTrustedCerts(TrustStoreManager.java:336)
> at
> java.base/sun.security.ssl.TrustStoreManager.getTrustedCerts(TrustStoreManager.java:57)
> at
> java.base/sun.security.ssl.TrustManagerFactoryImpl.engineInit(TrustManagerFactoryImpl.java:49)
> ... 83 common frames omitted
>
>
> Throwing an exception with a more detailed error message facilitates
> debugging and ultimately fixing such problems.
> Excellent point, thank you for your thoughtful review and for linking the
> documentation :)
>
> It would be really nice to give the user some information to help them along
> without requiring a JVM argument, though... What if we just included the file
> name (but not the path) in the exception message, something like:
>
> ```java
> throw new KeyStoreException("Failed to load key store with file name: " +
> descriptor.storeFile.getName(), e);
> ```
>
> Would that be acceptable? Or is there something else we could do to provide a
> little more helpful of a message?
It would be more acceptable, but it also may be more confusing if for example
"cacerts" was emitted, and there were more than one truststore with a filename
of "cacerts" in the system.
There are really only a couple of different ways a truststore can be set. See
https://docs.oracle.com/en/java/javase/20/security/java-secure-socket-extension-jsse-reference-guide.html#GUID-32CF3420-56E8-4BC5-8D3B-1F6B4692A290
for more info.
I think in most cases, you should be able to figure out which truststore has
the problem. Can you explain or give a scenario where that would be difficult
to determine? However, what might be acceptable to me is an exception that
gives a hint as to what truststore is being used, w/o giving away the full
path. For example, one of the following, depending on how it was configured:
"Problem accessing truststore specified in javax.net.ssl.trustStore system
property"
"Problem accessing truststore with file name:
${java.home}/lib/security/jssecacerts"
"Problem accessing truststore with file name:
${java.home}/lib/security/cacerts"
This change would probably require enhancing TrustStoreManager and
TrustStoreDescriptor to cache additional info about what type of truststore was
used. But it is not really clear to me that the effort is justified.
If you are able to re-run the application with debugging, you can obtain this
information. Also, with JFR you can get a dump of the system properties used by
an application, which also would be helpful if it is setting the
javax.net.ssl.trustStore system property.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/14834#issuecomment-1655789320
