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

[Alexey Bakhtin]

On Wed, 24 Jan 2024 15:41:11 GMT, Weijun Wang <weijun at openjdk.org> wrote:

>> 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.

Thank you. updated with the $ sign

> src/java.base/macosx/classes/apple/security/KeychainStore.java line 52:
> 
>> 50:  */
>> 51: 
>> 52: abstract class KeychainStore extends KeyStoreSpi {
> 
> Make it `abstract sealed`.

Good catch. Thank you. Added `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.

OK. Not much reason for this method. removed

> src/java.base/macosx/classes/apple/security/KeychainStore.java line 546:
> 
>> 544: 
>> 545:         synchronized(entries) {
>> 546: 
> 
> Useless.

Fixed

> 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`.

Fixed

> 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`.

Fixed

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16722#discussion_r1467006056
PR Review Comment: https://git.openjdk.org/jdk/pull/16722#discussion_r1467006192
PR Review Comment: https://git.openjdk.org/jdk/pull/16722#discussion_r1467006360
PR Review Comment: https://git.openjdk.org/jdk/pull/16722#discussion_r1467006453
PR Review Comment: https://git.openjdk.org/jdk/pull/16722#discussion_r1467006522
PR Review Comment: https://git.openjdk.org/jdk/pull/16722#discussion_r1467006582