From 87b7f13eace1a8e16d6185421956b4500617090b Mon Sep 17 00:00:00 2001 From: Christoph Langer Date: Wed, 14 Jun 2023 07:12:24 +0000 Subject: [PATCH 1/4] 8303465: KeyStore of type KeychainStore, provider Apple does not show all trusted certificates Reviewed-by: mbaesken Backport-of: ac41c030030c3d31815474c793ac9c420c47e22c --- .../classes/apple/security/KeychainStore.java | 48 ++++--- .../native/libosxsecurity/KeystoreImpl.m | 91 +++++++++----- .../KeyStore/CheckMacOSKeyChainTrust.java | 119 ++++++++++++++++++ 3 files changed, 210 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 74a531dae4d..e2647118b09 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 @@ 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 @@ -701,7 +701,6 @@ public void engineStore(OutputStream stream, char[] password) _releaseKeychainItemRef(((TrustedCertEntry)entry).certRef); } } else { - Certificate certElem; KeyEntry keyEntry = (KeyEntry)entry; if (keyEntry.chain != null) { @@ -853,8 +852,27 @@ 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) { + var tco = (TrustedCertEntry)co; + 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); @@ -877,9 +895,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 { @@ -892,11 +911,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 @@ -916,20 +943,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); + } + } + } +} From 94e06b233c310e193495e8822dde77272eea40ba Mon Sep 17 00:00:00 2001 From: J9 Build Date: Wed, 21 Jun 2023 14:31:11 +0000 Subject: [PATCH 2/4] Update OPENJDK_TAG to merged level jdk-11.0.20+7 Signed-off-by: J9 Build --- closed/openjdk-tag.gmk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/closed/openjdk-tag.gmk b/closed/openjdk-tag.gmk index d33fe7288ff..071211ea481 100644 --- a/closed/openjdk-tag.gmk +++ b/closed/openjdk-tag.gmk @@ -1 +1 @@ -OPENJDK_TAG := jdk-11.0.20+6 +OPENJDK_TAG := jdk-11.0.20+7 From 517688f0620ca12c6312cf0cbfd211ad1a2685f0 Mon Sep 17 00:00:00 2001 From: Daryl Maier Date: Wed, 21 Jun 2023 12:45:05 -0400 Subject: [PATCH 3/4] Enable CRIU support builds for AArch64 Signed-off-by: Daryl Maier --- closed/autoconf/custom-hook.m4 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/closed/autoconf/custom-hook.m4 b/closed/autoconf/custom-hook.m4 index c4a90ee38f3..2c3eed7ef0a 100644 --- a/closed/autoconf/custom-hook.m4 +++ b/closed/autoconf/custom-hook.m4 @@ -377,7 +377,7 @@ AC_DEFUN([OPENJ9_CONFIGURE_CRIU_SUPPORT], AC_MSG_RESULT([no (explicitly disabled)]) elif test "x$enable_criu_support" = x ; then case "$OPENJ9_PLATFORM_CODE" in - xa64|xz64) + xa64|xz64|xr64) AC_MSG_RESULT([yes (default)]) OPENJ9_ENABLE_CRIU_SUPPORT=true ;; From 14d638c74ccf35107e2fda18c9b360e8c319e718 Mon Sep 17 00:00:00 2001 From: Daryl Maier Date: Wed, 21 Jun 2023 19:05:42 -0400 Subject: [PATCH 4/4] Reorder CRIU platforms alphabetically Signed-off-by: Daryl Maier --- closed/autoconf/custom-hook.m4 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/closed/autoconf/custom-hook.m4 b/closed/autoconf/custom-hook.m4 index 2c3eed7ef0a..8fb67956b7a 100644 --- a/closed/autoconf/custom-hook.m4 +++ b/closed/autoconf/custom-hook.m4 @@ -377,7 +377,7 @@ AC_DEFUN([OPENJ9_CONFIGURE_CRIU_SUPPORT], AC_MSG_RESULT([no (explicitly disabled)]) elif test "x$enable_criu_support" = x ; then case "$OPENJ9_PLATFORM_CODE" in - xa64|xz64|xr64) + xa64|xr64|xz64) AC_MSG_RESULT([yes (default)]) OPENJ9_ENABLE_CRIU_SUPPORT=true ;;