Skip to content

Commit ac41c03

Browse files
RealCLangerRealCLanger
committed
8303465: KeyStore of type KeychainStore, provider Apple does not show all trusted certificates
Reviewed-by: mbaesken, weijun
1 parent 11fb5b2 commit ac41c03

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
@@ -652,7 +652,6 @@ public void engineStore(OutputStream stream, char[] password)
652652
_releaseKeychainItemRef(((TrustedCertEntry)entry).certRef);
653653
}
654654
} else {
655-
Certificate certElem;
656655
KeyEntry keyEntry = (KeyEntry)entry;
657656

658657
if (keyEntry.chain != null) {
@@ -804,8 +803,26 @@ private void createTrustedCertEntry(String alias, List<String> inputTrust,
804803
tce.cert = cert;
805804
tce.certRef = keychainItemRef;
806805

806+
// Check whether a certificate with same alias already exists and is the same
807+
// If yes, we can return here - the existing entry must have the same
808+
// properties and trust settings
809+
if (entries.contains(alias.toLowerCase())) {
810+
int uniqueVal = 1;
811+
String originalAlias = alias;
812+
var co = entries.get(alias.toLowerCase());
813+
while (co != null) {
814+
if (co instanceof TrustedCertEntry tco) {
815+
if (tco.cert.equals(tce.cert)) {
816+
return;
817+
}
818+
}
819+
alias = originalAlias + " " + uniqueVal++;
820+
co = entries.get(alias.toLowerCase());
821+
}
822+
}
823+
807824
tce.trustSettings = new ArrayList<>();
808-
Map<String,String> tmpMap = new LinkedHashMap<>();
825+
Map<String, String> tmpMap = new LinkedHashMap<>();
809826
for (int i = 0; i < inputTrust.size(); i++) {
810827
if (inputTrust.get(i) == null) {
811828
tce.trustSettings.add(tmpMap);
@@ -828,9 +845,10 @@ private void createTrustedCertEntry(String alias, List<String> inputTrust,
828845
} catch (Exception e) {
829846
isSelfSigned = false;
830847
}
848+
831849
if (tce.trustSettings.isEmpty()) {
832850
if (isSelfSigned) {
833-
// If a self-signed certificate has an empty trust settings,
851+
// If a self-signed certificate has trust settings without specific entries,
834852
// trust it for all purposes
835853
tce.trustedKeyUsageValue = KnownOIDs.anyExtendedKeyUsage.value();
836854
} else {
@@ -843,11 +861,19 @@ private void createTrustedCertEntry(String alias, List<String> inputTrust,
843861
for (var oneTrust : tce.trustSettings) {
844862
var result = oneTrust.get("kSecTrustSettingsResult");
845863
// https://developer.apple.com/documentation/security/sectrustsettingsresult?language=objc
846-
// 1 = kSecTrustSettingsResultTrustRoot, 2 = kSecTrustSettingsResultTrustAsRoot
864+
// 1 = kSecTrustSettingsResultTrustRoot, 2 = kSecTrustSettingsResultTrustAsRoot,
865+
// 3 = kSecTrustSettingsResultDeny
847866
// If missing, a default value of kSecTrustSettingsResultTrustRoot is assumed
848-
// for self-signed certificates (see doc for SecTrustSettingsCopyTrustSettings).
867+
// (see doc for SecTrustSettingsCopyTrustSettings).
849868
// Note that the same SecPolicyOid can appear in multiple trust settings
850869
// for different kSecTrustSettingsAllowedError and/or kSecTrustSettingsPolicyString.
870+
871+
// If we find explicit distrust in some record, we ignore the certificate
872+
if ("3".equals(result)) {
873+
return;
874+
}
875+
876+
// Trust, if explicitly trusted or result is null and certificate is self signed
851877
if ((result == null && isSelfSigned)
852878
|| "1".equals(result) || "2".equals(result)) {
853879
// When no kSecTrustSettingsPolicy, it means everything
@@ -867,20 +893,13 @@ private void createTrustedCertEntry(String alias, List<String> inputTrust,
867893
tce.trustedKeyUsageValue = values.toString();
868894
}
869895
}
896+
870897
// Make a creation date.
871898
if (creationDate != 0)
872899
tce.date = new Date(creationDate);
873900
else
874901
tce.date = new Date();
875902

876-
int uniqueVal = 1;
877-
String originalAlias = alias;
878-
879-
while (entries.containsKey(alias.toLowerCase())) {
880-
alias = originalAlias + " " + uniqueVal;
881-
uniqueVal++;
882-
}
883-
884903
entries.put(alias.toLowerCase(), tce);
885904
} catch (Exception e) {
886905
// 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)