RFR: 8320362: Load anchor certificates from Keychain keystore [v3]

Weijun Wang weijun at openjdk.org
Wed Jan 24 16:01:38 UTC 2024


On Thu, 4 Jan 2024 02:24:56 GMT, Alexey Bakhtin <abakhtin at openjdk.org> wrote:

>> Please review the proposed fix.
>> 
>> The patch loads system root certificates from the MacOS Keychain with TrustSettings.
>> It allows to build a trusted certificate path using the MacOS Keychain store only.
>
> Alexey Bakhtin has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Add KeychainStore-ROOT keystore for root certificates

src/java.base/macosx/classes/apple/security/AppleProvider.java line 89:

> 87:                            "KeychainStore", "apple.security.KeychainStore.USER"));
> 88:                 putService(new ProviderService(p, "KeyStore",
> 89:                            "KeychainStore-ROOT", "apple.security.KeychainStore.ROOT"));

I think the correct class names should be `KeychainStore$USER` and `KeychainStore$ROOT`, although they might be used.

src/java.base/macosx/classes/apple/security/KeychainStore.java line 45:

> 43: 
> 44: /**
> 45:  * This class provides the keystores implementation referred to as

Or maybe "keystore implementations"? If yes, then use "They use ... their...".

src/java.base/macosx/classes/apple/security/KeychainStore.java line 52:

> 50:  */
> 51: 
> 52: abstract class KeychainStore extends KeyStoreSpi {

Make it `abstract sealed`.

src/java.base/macosx/classes/apple/security/KeychainStore.java line 216:

> 214:      */
> 215:     private String getName()
> 216:     {

Put the closing brace to the previous line. Actually, probably not worth creating a new method.

src/java.base/macosx/classes/apple/security/KeychainStore.java line 546:

> 544: 
> 545:         synchronized(entries) {
> 546: 

Useless.

src/java.base/macosx/native/libosxsecurity/KeystoreImpl.m line 478:

> 476:                 // Load user trustSettings into inputTrust
> 477:                 if (SecTrustSettingsCopyTrustSettings(certRef, domain, &trustSettings) == errSecSuccess && trustSettings != NULL) {
> 478:                     if(inputTrust == NULL) {

Add a space after `if`.

src/java.base/macosx/native/libosxsecurity/KeystoreImpl.m line 544:

> 542: 
> 543:     // Read Trust Anchors
> 544:     if(SecTrustCopyAnchorCertificates(&currAnchors) == errSecSuccess) {

Add a space after `if`.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16722#discussion_r1465113070
PR Review Comment: https://git.openjdk.org/jdk/pull/16722#discussion_r1465114949
PR Review Comment: https://git.openjdk.org/jdk/pull/16722#discussion_r1465124742
PR Review Comment: https://git.openjdk.org/jdk/pull/16722#discussion_r1465130743
PR Review Comment: https://git.openjdk.org/jdk/pull/16722#discussion_r1465132076
PR Review Comment: https://git.openjdk.org/jdk/pull/16722#discussion_r1465140314
PR Review Comment: https://git.openjdk.org/jdk/pull/16722#discussion_r1465150456



More information about the security-dev mailing list