From 0f531dacb878efe77912688216b0f39a769259a3 Mon Sep 17 00:00:00 2001 From: Christoph Langer Date: Tue, 13 Jun 2023 13:43:57 +0000 Subject: [PATCH] 8303465: KeyStore of type KeychainStore, provider Apple does not show all trusted certificates Backport-of: ac41c030030c3d31815474c793ac9c420c47e22c --- .../classes/apple/security/KeychainStore.java | 47 ++++--- .../native/libosxsecurity/KeystoreImpl.m | 91 +++++++++----- .../KeyStore/CheckMacOSKeyChainTrust.java | 119 ++++++++++++++++++ 3 files changed, 209 insertions(+), 48 deletions(-) create mode 100644 test/jdk/java/security/KeyStore/CheckMacOSKeyChainTrust.java diff --git a/src/java.base/macosx/classes/apple/security/KeychainStore.java b/src/java.base/macosx/classes/apple/security/KeychainStore.java index e4a77e61cca..7cb280035e3 100644 --- a/src/java.base/macosx/classes/apple/security/KeychainStore.java +++ b/src/java.base/macosx/classes/apple/security/KeychainStore.java @@ -69,7 +69,7 @@ static class TrustedCertEntry { Certificate cert; long certRef; // SecCertificateRef for this key - // Each KeyStore.TrustedCertificateEntry have 2 attributes: + // Each KeyStore.TrustedCertificateEntry has 2 attributes: // 1. "trustSettings" -> trustSettings.toString() // 2. "2.16.840.1.113894.746875.1.1" -> trustedKeyUsageValue // The 1st one is mainly for debugging use. The 2nd one is similar @@ -660,7 +660,6 @@ public void engineStore(OutputStream stream, char[] password) _releaseKeychainItemRef(((TrustedCertEntry)entry).certRef); } } else { - Certificate certElem; KeyEntry keyEntry = (KeyEntry)entry; if (keyEntry.chain != null) { @@ -812,8 +811,26 @@ private void createTrustedCertEntry(String alias, List inputTrust, tce.cert = cert; tce.certRef = keychainItemRef; + // Check whether a certificate with same alias already exists and is the same + // If yes, we can return here - the existing entry must have the same + // properties and trust settings + if (entries.contains(alias.toLowerCase())) { + int uniqueVal = 1; + String originalAlias = alias; + var co = entries.get(alias.toLowerCase()); + while (co != null) { + if (co instanceof TrustedCertEntry tco) { + if (tco.cert.equals(tce.cert)) { + return; + } + } + alias = originalAlias + " " + uniqueVal++; + co = entries.get(alias.toLowerCase()); + } + } + tce.trustSettings = new ArrayList<>(); - Map tmpMap = new LinkedHashMap<>(); + Map tmpMap = new LinkedHashMap<>(); for (int i = 0; i < inputTrust.size(); i++) { if (inputTrust.get(i) == null) { tce.trustSettings.add(tmpMap); @@ -836,9 +853,10 @@ private void createTrustedCertEntry(String alias, List inputTrust, } catch (Exception e) { isSelfSigned = false; } + if (tce.trustSettings.isEmpty()) { if (isSelfSigned) { - // If a self-signed certificate has an empty trust settings, + // If a self-signed certificate has trust settings without specific entries, // trust it for all purposes tce.trustedKeyUsageValue = KnownOIDs.anyExtendedKeyUsage.value(); } else { @@ -851,11 +869,19 @@ private void createTrustedCertEntry(String alias, List inputTrust, for (var oneTrust : tce.trustSettings) { var result = oneTrust.get("kSecTrustSettingsResult"); // https://developer.apple.com/documentation/security/sectrustsettingsresult?language=objc - // 1 = kSecTrustSettingsResultTrustRoot, 2 = kSecTrustSettingsResultTrustAsRoot + // 1 = kSecTrustSettingsResultTrustRoot, 2 = kSecTrustSettingsResultTrustAsRoot, + // 3 = kSecTrustSettingsResultDeny // If missing, a default value of kSecTrustSettingsResultTrustRoot is assumed - // for self-signed certificates (see doc for SecTrustSettingsCopyTrustSettings). + // (see doc for SecTrustSettingsCopyTrustSettings). // Note that the same SecPolicyOid can appear in multiple trust settings // for different kSecTrustSettingsAllowedError and/or kSecTrustSettingsPolicyString. + + // If we find explicit distrust in some record, we ignore the certificate + if ("3".equals(result)) { + return; + } + + // Trust, if explicitly trusted or result is null and certificate is self signed if ((result == null && isSelfSigned) || "1".equals(result) || "2".equals(result)) { // When no kSecTrustSettingsPolicy, it means everything @@ -875,20 +901,13 @@ private void createTrustedCertEntry(String alias, List inputTrust, tce.trustedKeyUsageValue = values.toString(); } } + // Make a creation date. if (creationDate != 0) tce.date = new Date(creationDate); else tce.date = new Date(); - int uniqueVal = 1; - String originalAlias = alias; - - while (entries.containsKey(alias.toLowerCase())) { - alias = originalAlias + " " + uniqueVal; - uniqueVal++; - } - entries.put(alias.toLowerCase(), tce); } catch (Exception e) { // The certificate will be skipped. diff --git a/src/java.base/macosx/native/libosxsecurity/KeystoreImpl.m b/src/java.base/macosx/native/libosxsecurity/KeystoreImpl.m index 38362709d27..b4e19a27995 100644 --- a/src/java.base/macosx/native/libosxsecurity/KeystoreImpl.m +++ b/src/java.base/macosx/native/libosxsecurity/KeystoreImpl.m @@ -381,6 +381,35 @@ static void addIdentitiesToKeystore(JNIEnv *env, jobject keyStore) #define ADDNULL(list) (*env)->CallBooleanMethod(env, list, jm_listAdd, NULL) + +static void addTrustSettingsToInputTrust(JNIEnv *env, jmethodID jm_listAdd, CFArrayRef trustSettings, jobject inputTrust) +{ + CFIndex count = CFArrayGetCount(trustSettings); + for (int i = 0; i < count; i++) { + CFDictionaryRef oneTrust = (CFDictionaryRef) CFArrayGetValueAtIndex(trustSettings, i); + CFIndex size = CFDictionaryGetCount(oneTrust); + const void * keys [size]; + const void * values [size]; + CFDictionaryGetKeysAndValues(oneTrust, keys, values); + for (int j = 0; j < size; j++) { + NSString* s = [NSString stringWithFormat:@"%@", keys[j]]; + ADD(inputTrust, s); + s = [NSString stringWithFormat:@"%@", values[j]]; + ADD(inputTrust, s); + } + SecPolicyRef certPolicy; + certPolicy = (SecPolicyRef)CFDictionaryGetValue(oneTrust, kSecTrustSettingsPolicy); + if (certPolicy != NULL) { + CFDictionaryRef policyDict = SecPolicyCopyProperties(certPolicy); + ADD(inputTrust, @"SecPolicyOid"); + NSString* s = [NSString stringWithFormat:@"%@", CFDictionaryGetValue(policyDict, @"SecPolicyOid")]; + ADD(inputTrust, s); + CFRelease(policyDict); + } + ADDNULL(inputTrust); + } +} + static void addCertificatesToKeystore(JNIEnv *env, jobject keyStore) { // Search the user keychain list for all X509 certificates. @@ -435,46 +464,40 @@ static void addCertificatesToKeystore(JNIEnv *env, jobject keyStore) goto errOut; } - // Only add certificates with trusted settings - CFArrayRef trustSettings; - if (SecTrustSettingsCopyTrustSettings(certRef, kSecTrustSettingsDomainUser, &trustSettings) - == errSecItemNotFound) { - continue; - } - // See KeychainStore::createTrustedCertEntry for content of inputTrust - jobject inputTrust = (*env)->NewObject(env, jc_arrayListClass, jm_arrayListCons); - if (inputTrust == NULL) { + // We load trust settings from domains kSecTrustSettingsDomainUser and kSecTrustSettingsDomainAdmin + // kSecTrustSettingsDomainSystem is ignored because it seems to only contain data for root certificates + jobject inputTrust = NULL; + CFArrayRef trustSettings = NULL; + + // Load user trustSettings into inputTrust + if (SecTrustSettingsCopyTrustSettings(certRef, kSecTrustSettingsDomainUser, &trustSettings) == errSecSuccess && trustSettings != NULL) { + inputTrust = (*env)->NewObject(env, jc_arrayListClass, jm_arrayListCons); + if (inputTrust == NULL) { + CFRelease(trustSettings); + goto errOut; + } + addTrustSettingsToInputTrust(env, jm_listAdd, trustSettings, inputTrust); CFRelease(trustSettings); - goto errOut; } - - // Dump everything inside trustSettings into inputTrust - CFIndex count = CFArrayGetCount(trustSettings); - for (int i = 0; i < count; i++) { - CFDictionaryRef oneTrust = (CFDictionaryRef) CFArrayGetValueAtIndex(trustSettings, i); - CFIndex size = CFDictionaryGetCount(oneTrust); - const void * keys [size]; - const void * values [size]; - CFDictionaryGetKeysAndValues(oneTrust, keys, values); - for (int j = 0; j < size; j++) { - NSString* s = [NSString stringWithFormat:@"%@", keys[j]]; - ADD(inputTrust, s); - s = [NSString stringWithFormat:@"%@", values[j]]; - ADD(inputTrust, s); + // Load admin trustSettings into inputTrust + trustSettings = NULL; + if (SecTrustSettingsCopyTrustSettings(certRef, kSecTrustSettingsDomainAdmin, &trustSettings) == errSecSuccess && trustSettings != NULL) { + if (inputTrust == NULL) { + inputTrust = (*env)->NewObject(env, jc_arrayListClass, jm_arrayListCons); } - SecPolicyRef certPolicy; - certPolicy = (SecPolicyRef)CFDictionaryGetValue(oneTrust, kSecTrustSettingsPolicy); - if (certPolicy != NULL) { - CFDictionaryRef policyDict = SecPolicyCopyProperties(certPolicy); - ADD(inputTrust, @"SecPolicyOid"); - NSString* s = [NSString stringWithFormat:@"%@", CFDictionaryGetValue(policyDict, @"SecPolicyOid")]; - ADD(inputTrust, s); - CFRelease(policyDict); + if (inputTrust == NULL) { + CFRelease(trustSettings); + goto errOut; } - ADDNULL(inputTrust); + addTrustSettingsToInputTrust(env, jm_listAdd, trustSettings, inputTrust); + CFRelease(trustSettings); + } + + // Only add certificates with trust settings + if (inputTrust == NULL) { + continue; } - CFRelease(trustSettings); // Find the creation date. jlong creationDate = getModDateFromItem(env, theItem); diff --git a/test/jdk/java/security/KeyStore/CheckMacOSKeyChainTrust.java b/test/jdk/java/security/KeyStore/CheckMacOSKeyChainTrust.java new file mode 100644 index 00000000000..edad6210195 --- /dev/null +++ b/test/jdk/java/security/KeyStore/CheckMacOSKeyChainTrust.java @@ -0,0 +1,119 @@ + +/* + * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2023 SAP SE. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +import java.security.KeyStore; +import java.util.HashSet; +import java.util.Set; + +import jdk.test.lib.process.OutputAnalyzer; +import jdk.test.lib.process.ProcessTools; + +/* + * @test + * @bug 8303465 + * @library /test/lib + * @requires os.family == "mac" + * @summary Check whether loading of certificates from MacOS Keychain correctly + * honors trust settings + */ +public class CheckMacOSKeyChainTrust { + private static Set trusted = new HashSet<>(); + private static Set distrusted = new HashSet<>(); + + public static void main(String[] args) throws Throwable { + loadUser(); + loadAdmin(); + System.out.println("Trusted Certs: " + trusted); + System.out.println("Distrusted Certs: " + distrusted); + KeyStore ks = KeyStore.getInstance("KEYCHAINSTORE"); + ks.load(null, null); + for (String alias : trusted) { + if (!ks.containsAlias(alias)) { + throw new RuntimeException("Not found: " + alias); + } + } + for (String alias : distrusted) { + if (ks.containsAlias(alias)) { + throw new RuntimeException("Found: " + alias); + } + } + } + + private static void loadUser() throws Throwable { + populate(ProcessTools.executeProcess("security", "dump-trust-settings")); + } + + private static void loadAdmin() throws Throwable { + populate(ProcessTools.executeProcess("security", "dump-trust-settings", "-d")); + } + + private static void populate(OutputAnalyzer output) throws Throwable { + if (output.getExitValue() != 0) { + return; // No Trust Settings were found + } + String certName = null; + boolean trustRootFound = false; + boolean trustAsRootFound = false; + boolean denyFound = false; + boolean unspecifiedFound = false; + for (String line : output.asLines()) { + if (line.startsWith("Cert ")) { + if (certName != null) { + if (!denyFound && + !(unspecifiedFound && !(trustRootFound || trustAsRootFound)) && + !distrusted.contains(certName)) { + trusted.add(certName); + } else { + distrusted.add(certName); + trusted.remove(certName); + } + } + certName = line.split(":", 2)[1].trim().toLowerCase(); + trustRootFound = false; + trustAsRootFound = false; + denyFound = false; + unspecifiedFound = false; + } else if (line.contains("kSecTrustSettingsResultTrustRoot")) { + trustRootFound = true; + } else if (line.contains("kSecTrustSettingsResultTrustAsRoot")) { + trustAsRootFound = true; + } else if (line.contains("kSecTrustSettingsResultDeny")) { + denyFound = true; + } else if (line.contains("kSecTrustSettingsResultUnspecified")) { + unspecifiedFound = true; + } + } + if (certName != null) { + if (!denyFound && + !(unspecifiedFound && !(trustRootFound || trustAsRootFound)) && + !distrusted.contains(certName)) { + trusted.add(certName); + } else { + distrusted.add(certName); + trusted.remove(certName); + } + } + } +}