RFR: 8347067: Load certificates without explicit trust settings in KeyChainStore

[Alexey Bakhtin]

On Fri, 3 Jan 2025 11:28:01 GMT, Tim Jacomb <duke at openjdk.org> wrote:

> ## The change
> 
> Without this change intermediate certificates that don't have explicit trust settings are ignored not added to the truststore.
> 
> 
> 
> ## Reproducer
> 
> See https://github.com/timja/openjdk-intermediate-ca-reproducer
> 
> Without this change the reproducer fails, and with this change it succeeds.
> 
> ## Example failing architecture
> 
> Root CA -> Intermediate 1 -> Intermediate 2 -> Leaf
> 
> Where:
> * All certs are in admin domain kSecTrustSettingsDomainAdmin
> * Root CA is marked as always trust
> * Intermediate 1 and 2 are Unspecified
> 
> Previously Root CA would be found but intermediate 1 and 2 would be skipped when verifying trust settings.
> 
> ## Background reading
> 
> ### Rust
> see also Rust Lib that is used throughout Rust ecosystem for this: 
> https://github.com/rustls/rustls-native-certs/blob/efe7b1d77bf6080851486535664d1dc7ef0dea68/src/macos.rs#L39-L58
> 
> e.g. in Deno `https://github.com/denoland/deno/pull/11491` where I've verified it is correctly implemented and works in my setup
> 
> ## Python
> 
> I also looked at the Python implementation for inspiration as well (which also works on my system): https://github.com/sethmlarson/truststore/blob/main/src/truststore/_macos.py

test/jdk/java/security/KeyStore/CheckMacOSKeyChainIntermediateCATrust.java line 48:

> 46:  * @summary Check whether loading of certificates from macOS Keychain correctly
> 47:  *          loads intermediate CA certificates
> 48:  * @run junit/manual CheckMacOSKeyChainIntermediateCATrust

I think you can add negative tests also. E.g. add Root CA without trust settings

test/jdk/java/security/KeyStore/CheckMacOSKeyChainIntermediateCATrust.java line 123:

> 121:         System.out.println("Command line: " + params);
> 122:         try {
> 123:             int exitStatus = ProcessTools.executeProcess(params.toArray(new String[0])).getExitValue();

shouldHaveExitValue(0) can be used

test/jdk/java/security/KeyStore/CheckMacOSKeyChainIntermediateCATrust.java line 166:

> 164:     private static void assertThat(boolean expected, String message, List<X509Certificate> certificates) {
> 165:         if (!expected) {
> 166:             throw new AssertionError(message + ", subjects: " + getSubjects(certificates));

I do not like printing all KeyChain certificates on the failure. It could be sensitive information.

If you do not collect all certificates, the test could be simplified - without Stream API

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/22911#discussion_r1929425287
PR Review Comment: https://git.openjdk.org/jdk/pull/22911#discussion_r1929426230
PR Review Comment: https://git.openjdk.org/jdk/pull/22911#discussion_r1929424938