On Thu, 4 Jan 2024 02:24:56 GMT, Alexey Bakhtin <abakh...@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
