Skip to content

Commit 0f531da

Browse files
RealCLangerRealCLanger
committed
8303465: KeyStore of type KeychainStore, provider Apple does not show all trusted certificates
Backport-of: ac41c03
1 parent a93cf5f commit 0f531da

File tree

3 files changed

+209
-48
lines changed

3 files changed

+209
-48
lines changed

‎src/java.base/macosx/classes/apple/security/KeychainStore.java‎

Lines changed: 33 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ static class TrustedCertEntry {
6969
Certificate cert;
7070
long certRef; // SecCertificateRef for this key
7171

72-
// Each KeyStore.TrustedCertificateEntry have 2 attributes:
72+
// Each KeyStore.TrustedCertificateEntry has 2 attributes:
7373
// 1. "trustSettings" -> trustSettings.toString()
7474
// 2. "2.16.840.1.113894.746875.1.1" -> trustedKeyUsageValue
7575
// The 1st one is mainly for debugging use. The 2nd one is similar
@@ -660,7 +660,6 @@ public void engineStore(OutputStream stream, char[] password)
660660
_releaseKeychainItemRef(((TrustedCertEntry)entry).certRef);
661661
}
662662
} else {
663-
Certificate certElem;
664663
KeyEntry keyEntry = (KeyEntry)entry;
665664

666665
if (keyEntry.chain != null) {
@@ -812,8 +811,26 @@ private void createTrustedCertEntry(String alias, List<String> inputTrust,
812811
tce.cert = cert;
813812
tce.certRef = keychainItemRef;
814813

814+
// Check whether a certificate with same alias already exists and is the same
815+
// If yes, we can return here - the existing entry must have the same
816+
// properties and trust settings
817+
if (entries.contains(alias.toLowerCase())) {
818+
int uniqueVal = 1;
819+
String originalAlias = alias;
820+
var co = entries.get(alias.toLowerCase());
821+
while (co != null) {
822+
if (co instanceof TrustedCertEntry tco) {
823+
if (tco.cert.equals(tce.cert)) {
824+
return;
825+
}
826+
}
827+
alias = originalAlias + " " + uniqueVal++;
828+
co = entries.get(alias.toLowerCase());
829+
}
830+
}
831+
815832
tce.trustSettings = new ArrayList<>();
816-
Map<String,String> tmpMap = new LinkedHashMap<>();
833+
Map<String, String> tmpMap = new LinkedHashMap<>();
817834
for (int i = 0; i < inputTrust.size(); i++) {
818835
if (inputTrust.get(i) == null) {
819836
tce.trustSettings.add(tmpMap);
@@ -836,9 +853,10 @@ private void createTrustedCertEntry(String alias, List<String> inputTrust,
836853
} catch (Exception e) {
837854
isSelfSigned = false;
838855
}
856+
839857
if (tce.trustSettings.isEmpty()) {
840858
if (isSelfSigned) {
841-
// If a self-signed certificate has an empty trust settings,
859+
// If a self-signed certificate has trust settings without specific entries,
842860
// trust it for all purposes
843861
tce.trustedKeyUsageValue = KnownOIDs.anyExtendedKeyUsage.value();
844862
} else {
@@ -851,11 +869,19 @@ private void createTrustedCertEntry(String alias, List<String> inputTrust,
851869
for (var oneTrust : tce.trustSettings) {
852870
var result = oneTrust.get("kSecTrustSettingsResult");
853871
// https://developer.apple.com/documentation/security/sectrustsettingsresult?language=objc
854-
// 1 = kSecTrustSettingsResultTrustRoot, 2 = kSecTrustSettingsResultTrustAsRoot
872+
// 1 = kSecTrustSettingsResultTrustRoot, 2 = kSecTrustSettingsResultTrustAsRoot,
873+
// 3 = kSecTrustSettingsResultDeny
855874
// If missing, a default value of kSecTrustSettingsResultTrustRoot is assumed
856-
// for self-signed certificates (see doc for SecTrustSettingsCopyTrustSettings).
875+
// (see doc for SecTrustSettingsCopyTrustSettings).
857876
// Note that the same SecPolicyOid can appear in multiple trust settings
858877
// for different kSecTrustSettingsAllowedError and/or kSecTrustSettingsPolicyString.
878+
879+
// If we find explicit distrust in some record, we ignore the certificate
880+
if ("3".equals(result)) {
881+
return;
882+
}
883+
884+
// Trust, if explicitly trusted or result is null and certificate is self signed
859885
if ((result == null && isSelfSigned)
860886
|| "1".equals(result) || "2".equals(result)) {
861887
// When no kSecTrustSettingsPolicy, it means everything
@@ -875,20 +901,13 @@ private void createTrustedCertEntry(String alias, List<String> inputTrust,
875901
tce.trustedKeyUsageValue = values.toString();
876902
}
877903
}
904+
878905
// Make a creation date.
879906
if (creationDate != 0)
880907
tce.date = new Date(creationDate);
881908
else
882909
tce.date = new Date();
883910

884-
int uniqueVal = 1;
885-
String originalAlias = alias;
886-
887-
while (entries.containsKey(alias.toLowerCase())) {
888-
alias = originalAlias + " " + uniqueVal;
889-
uniqueVal++;
890-
}
891-
892911
entries.put(alias.toLowerCase(), tce);
893912
} catch (Exception e) {
894913
// The certificate will be skipped.

‎src/java.base/macosx/native/libosxsecurity/KeystoreImpl.m‎

Lines changed: 57 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -381,6 +381,35 @@ static void addIdentitiesToKeystore(JNIEnv *env, jobject keyStore)
381381

382382
#define ADDNULL(list) (*env)->CallBooleanMethod(env, list, jm_listAdd, NULL)
383383

384+
385+
static void addTrustSettingsToInputTrust(JNIEnv *env, jmethodID jm_listAdd, CFArrayRef trustSettings, jobject inputTrust)
386+
{
387+
CFIndex count = CFArrayGetCount(trustSettings);
388+
for (int i = 0; i < count; i++) {
389+
CFDictionaryRef oneTrust = (CFDictionaryRef) CFArrayGetValueAtIndex(trustSettings, i);
390+
CFIndex size = CFDictionaryGetCount(oneTrust);
391+
const void * keys [size];
392+
const void * values [size];
393+
CFDictionaryGetKeysAndValues(oneTrust, keys, values);
394+
for (int j = 0; j < size; j++) {
395+
NSString* s = [NSString stringWithFormat:@"%@", keys[j]];
396+
ADD(inputTrust, s);
397+
s = [NSString stringWithFormat:@"%@", values[j]];
398+
ADD(inputTrust, s);
399+
}
400+
SecPolicyRef certPolicy;
401+
certPolicy = (SecPolicyRef)CFDictionaryGetValue(oneTrust, kSecTrustSettingsPolicy);
402+
if (certPolicy != NULL) {
403+
CFDictionaryRef policyDict = SecPolicyCopyProperties(certPolicy);
404+
ADD(inputTrust, @"SecPolicyOid");
405+
NSString* s = [NSString stringWithFormat:@"%@", CFDictionaryGetValue(policyDict, @"SecPolicyOid")];
406+
ADD(inputTrust, s);
407+
CFRelease(policyDict);
408+
}
409+
ADDNULL(inputTrust);
410+
}
411+
}
412+
384413
static void addCertificatesToKeystore(JNIEnv *env, jobject keyStore)
385414
{
386415
// Search the user keychain list for all X509 certificates.
@@ -435,46 +464,40 @@ static void addCertificatesToKeystore(JNIEnv *env, jobject keyStore)
435464
goto errOut;
436465
}
437466

438-
// Only add certificates with trusted settings
439-
CFArrayRef trustSettings;
440-
if (SecTrustSettingsCopyTrustSettings(certRef, kSecTrustSettingsDomainUser, &trustSettings)
441-
== errSecItemNotFound) {
442-
continue;
443-
}
444-
445467
// See KeychainStore::createTrustedCertEntry for content of inputTrust
446-
jobject inputTrust = (*env)->NewObject(env, jc_arrayListClass, jm_arrayListCons);
447-
if (inputTrust == NULL) {
468+
// We load trust settings from domains kSecTrustSettingsDomainUser and kSecTrustSettingsDomainAdmin
469+
// kSecTrustSettingsDomainSystem is ignored because it seems to only contain data for root certificates
470+
jobject inputTrust = NULL;
471+
CFArrayRef trustSettings = NULL;
472+
473+
// Load user trustSettings into inputTrust
474+
if (SecTrustSettingsCopyTrustSettings(certRef, kSecTrustSettingsDomainUser, &trustSettings) == errSecSuccess && trustSettings != NULL) {
475+
inputTrust = (*env)->NewObject(env, jc_arrayListClass, jm_arrayListCons);
476+
if (inputTrust == NULL) {
477+
CFRelease(trustSettings);
478+
goto errOut;
479+
}
480+
addTrustSettingsToInputTrust(env, jm_listAdd, trustSettings, inputTrust);
448481
CFRelease(trustSettings);
449-
goto errOut;
450482
}
451-
452-
// Dump everything inside trustSettings into inputTrust
453-
CFIndex count = CFArrayGetCount(trustSettings);
454-
for (int i = 0; i < count; i++) {
455-
CFDictionaryRef oneTrust = (CFDictionaryRef) CFArrayGetValueAtIndex(trustSettings, i);
456-
CFIndex size = CFDictionaryGetCount(oneTrust);
457-
const void * keys [size];
458-
const void * values [size];
459-
CFDictionaryGetKeysAndValues(oneTrust, keys, values);
460-
for (int j = 0; j < size; j++) {
461-
NSString* s = [NSString stringWithFormat:@"%@", keys[j]];
462-
ADD(inputTrust, s);
463-
s = [NSString stringWithFormat:@"%@", values[j]];
464-
ADD(inputTrust, s);
483+
// Load admin trustSettings into inputTrust
484+
trustSettings = NULL;
485+
if (SecTrustSettingsCopyTrustSettings(certRef, kSecTrustSettingsDomainAdmin, &trustSettings) == errSecSuccess && trustSettings != NULL) {
486+
if (inputTrust == NULL) {
487+
inputTrust = (*env)->NewObject(env, jc_arrayListClass, jm_arrayListCons);
465488
}
466-
SecPolicyRef certPolicy;
467-
certPolicy = (SecPolicyRef)CFDictionaryGetValue(oneTrust, kSecTrustSettingsPolicy);
468-
if (certPolicy != NULL) {
469-
CFDictionaryRef policyDict = SecPolicyCopyProperties(certPolicy);
470-
ADD(inputTrust, @"SecPolicyOid");
471-
NSString* s = [NSString stringWithFormat:@"%@", CFDictionaryGetValue(policyDict, @"SecPolicyOid")];
472-
ADD(inputTrust, s);
473-
CFRelease(policyDict);
489+
if (inputTrust == NULL) {
490+
CFRelease(trustSettings);
491+
goto errOut;
474492
}
475-
ADDNULL(inputTrust);
493+
addTrustSettingsToInputTrust(env, jm_listAdd, trustSettings, inputTrust);
494+
CFRelease(trustSettings);
495+
}
496+
497+
// Only add certificates with trust settings
498+
if (inputTrust == NULL) {
499+
continue;
476500
}
477-
CFRelease(trustSettings);
478501

479502
// Find the creation date.
480503
jlong creationDate = getModDateFromItem(env, theItem);

‎test/jdk/java/security/KeyStore/CheckMacOSKeyChainTrust.java‎

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
2+
/*
3+
* Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
4+
* Copyright (c) 2023 SAP SE. All rights reserved.
5+
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
6+
*
7+
* This code is free software; you can redistribute it and/or modify it
8+
* under the terms of the GNU General Public License version 2 only, as
9+
* published by the Free Software Foundation.
10+
*
11+
* This code is distributed in the hope that it will be useful, but WITHOUT
12+
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
13+
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
14+
* version 2 for more details (a copy is included in the LICENSE file that
15+
* accompanied this code).
16+
*
17+
* You should have received a copy of the GNU General Public License version
18+
* 2 along with this work; if not, write to the Free Software Foundation,
19+
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
20+
*
21+
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
22+
* or visit www.oracle.com if you need additional information or have any
23+
* questions.
24+
*/
25+
26+
import java.security.KeyStore;
27+
import java.util.HashSet;
28+
import java.util.Set;
29+
30+
import jdk.test.lib.process.OutputAnalyzer;
31+
import jdk.test.lib.process.ProcessTools;
32+
33+
/*
34+
* @test
35+
* @bug 8303465
36+
* @library /test/lib
37+
* @requires os.family == "mac"
38+
* @summary Check whether loading of certificates from MacOS Keychain correctly
39+
* honors trust settings
40+
*/
41+
public class CheckMacOSKeyChainTrust {
42+
private static Set<String> trusted = new HashSet<>();
43+
private static Set<String> distrusted = new HashSet<>();
44+
45+
public static void main(String[] args) throws Throwable {
46+
loadUser();
47+
loadAdmin();
48+
System.out.println("Trusted Certs: " + trusted);
49+
System.out.println("Distrusted Certs: " + distrusted);
50+
KeyStore ks = KeyStore.getInstance("KEYCHAINSTORE");
51+
ks.load(null, null);
52+
for (String alias : trusted) {
53+
if (!ks.containsAlias(alias)) {
54+
throw new RuntimeException("Not found: " + alias);
55+
}
56+
}
57+
for (String alias : distrusted) {
58+
if (ks.containsAlias(alias)) {
59+
throw new RuntimeException("Found: " + alias);
60+
}
61+
}
62+
}
63+
64+
private static void loadUser() throws Throwable {
65+
populate(ProcessTools.executeProcess("security", "dump-trust-settings"));
66+
}
67+
68+
private static void loadAdmin() throws Throwable {
69+
populate(ProcessTools.executeProcess("security", "dump-trust-settings", "-d"));
70+
}
71+
72+
private static void populate(OutputAnalyzer output) throws Throwable {
73+
if (output.getExitValue() != 0) {
74+
return; // No Trust Settings were found
75+
}
76+
String certName = null;
77+
boolean trustRootFound = false;
78+
boolean trustAsRootFound = false;
79+
boolean denyFound = false;
80+
boolean unspecifiedFound = false;
81+
for (String line : output.asLines()) {
82+
if (line.startsWith("Cert ")) {
83+
if (certName != null) {
84+
if (!denyFound &&
85+
!(unspecifiedFound && !(trustRootFound || trustAsRootFound)) &&
86+
!distrusted.contains(certName)) {
87+
trusted.add(certName);
88+
} else {
89+
distrusted.add(certName);
90+
trusted.remove(certName);
91+
}
92+
}
93+
certName = line.split(":", 2)[1].trim().toLowerCase();
94+
trustRootFound = false;
95+
trustAsRootFound = false;
96+
denyFound = false;
97+
unspecifiedFound = false;
98+
} else if (line.contains("kSecTrustSettingsResultTrustRoot")) {
99+
trustRootFound = true;
100+
} else if (line.contains("kSecTrustSettingsResultTrustAsRoot")) {
101+
trustAsRootFound = true;
102+
} else if (line.contains("kSecTrustSettingsResultDeny")) {
103+
denyFound = true;
104+
} else if (line.contains("kSecTrustSettingsResultUnspecified")) {
105+
unspecifiedFound = true;
106+
}
107+
}
108+
if (certName != null) {
109+
if (!denyFound &&
110+
!(unspecifiedFound && !(trustRootFound || trustAsRootFound)) &&
111+
!distrusted.contains(certName)) {
112+
trusted.add(certName);
113+
} else {
114+
distrusted.add(certName);
115+
trusted.remove(certName);
116+
}
117+
}
118+
}
119+
}

0 commit comments

Comments
 (0)