On Wed, 17 May 2023 08:06:56 GMT, Christoph Langer <clan...@openjdk.org> wrote:
>> With this PR we try to be better in loading certificates from the MacOS >> Keychain into a JDK Trust store. >> >> The current implementation after JDK-8278449 would only load/trust >> certificates from an identity (with private key available) and certificates >> that have explicit trust set in the user domain (as shown by security >> dump-trust-settings). This, however is not sufficient and does not match the >> MacOS system behavior, e.g. if you compare with tools like curl or Safari. >> >> This change does the following: >> 1. The native method that reads trust settings will call the API >> SecTrustSettingsCopyTrustSettings on a certificate for both, User and Admin >> domain. >> 2. No trust settings will be reported as "inputTrust" being null. If the >> certificate is trusted with no specific records, "inputTrust" will be an >> empty list. >> 3. The Java Method to add a certificate now checks for "self signed" >> certificate not only by checking whether it was signed with its own key but >> it must also not be a root certificate that can be used to sign other >> certificates. This is done by inspecting the key usage extension. >> 4. We now trust certificates that are either "real" self-signed certificates >> or certificates that have an explicit trust entry with no sub-records that >> would deny the certificate for any purpose. >> 5. The check for double aliases has been augmented by comparing whether the >> certificate to be added is the same as the one that is already present. This >> can happen if a certificate is contained in both, the user and the system >> keychain, for instance. >> >> I have added a test that verifies whether certificates that should be >> trusted from "security dump-trust-settings" are contained in the keystore >> and those that should be disallowed are absent. > > Christoph Langer has updated the pull request incrementally with one > additional commit since the last revision: > > Check return code of SecTrustSettingsCopyTrustSettings and address review > comments src/java.base/macosx/native/libosxsecurity/KeystoreImpl.m line 1: > 1: /* Regarding CFArrayGetValueAtIndex, when looking at other usages of the function in the codebase the result is NULL checked usually. Should we do this here too? I admit the old coding does not have it as well. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/13945#discussion_r1196455175
