Skip to content

Commit

Permalink
8303465: KeyStore of type KeychainStore, provider Apple does not show…
Browse files Browse the repository at this point in the history
… all trusted certificates

Reviewed-by: mbaesken, weijun
  • Loading branch information
RealCLanger committed Jun 5, 2023
1 parent 11fb5b2 commit ac41c03
Show file tree
Hide file tree
Showing 3 changed files with 209 additions and 48 deletions.
47 changes: 33 additions & 14 deletions src/java.base/macosx/classes/apple/security/KeychainStore.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -652,7 +652,6 @@ public void engineStore(OutputStream stream, char[] password)
_releaseKeychainItemRef(((TrustedCertEntry)entry).certRef);
}
} else {
Certificate certElem;
KeyEntry keyEntry = (KeyEntry)entry;

if (keyEntry.chain != null) {
Expand Down Expand Up @@ -804,8 +803,26 @@ private void createTrustedCertEntry(String alias, List<String> 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<String,String> tmpMap = new LinkedHashMap<>();
Map<String, String> tmpMap = new LinkedHashMap<>();
for (int i = 0; i < inputTrust.size(); i++) {
if (inputTrust.get(i) == null) {
tce.trustSettings.add(tmpMap);
Expand All @@ -828,9 +845,10 @@ private void createTrustedCertEntry(String alias, List<String> 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 {
Expand All @@ -843,11 +861,19 @@ private void createTrustedCertEntry(String alias, List<String> 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
Expand All @@ -867,20 +893,13 @@ private void createTrustedCertEntry(String alias, List<String> 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.
Expand Down
91 changes: 57 additions & 34 deletions src/java.base/macosx/native/libosxsecurity/KeystoreImpl.m
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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);
Expand Down
119 changes: 119 additions & 0 deletions test/jdk/java/security/KeyStore/CheckMacOSKeyChainTrust.java
Original file line number Diff line number Diff line change
@@ -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<String> trusted = new HashSet<>();
private static Set<String> 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);
}
}
}
}

7 comments on commit ac41c03

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RealCLanger
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/backport jdk20u

@openjdk
Copy link

@openjdk openjdk bot commented on ac41c03 Jun 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RealCLanger Could not automatically backport ac41c030 to openjdk/jdk20u due to conflicts in the following files:

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

Please fetch the appropriate branch/commit and manually resolve these conflicts by using the following commands in your personal fork of openjdk/jdk20u. Note: these commands are just some suggestions and you can use other equivalent commands you know.

# Fetch the up-to-date version of the target branch
$ git fetch --no-tags https://git.openjdk.org/jdk20u.git master:master

# Check out the target branch and create your own branch to backport
$ git checkout master
$ git checkout -b RealCLanger-backport-ac41c030

# Fetch the commit you want to backport
$ git fetch --no-tags https://git.openjdk.org/jdk.git ac41c030030c3d31815474c793ac9c420c47e22c

# Backport the commit
$ git cherry-pick --no-commit ac41c030030c3d31815474c793ac9c420c47e22c
# Resolve conflicts now

# Commit the files you have modified
$ git add files/with/resolved/conflicts
$ git commit -m 'Backport ac41c030030c3d31815474c793ac9c420c47e22c'

Once you have resolved the conflicts as explained above continue with creating a pull request towards the openjdk/jdk20u with the title Backport ac41c030030c3d31815474c793ac9c420c47e22c.

@RealCLanger
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/backport jdk17u

@openjdk
Copy link

@openjdk openjdk bot commented on ac41c03 Jun 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RealCLanger the backport was successfully created on the branch RealCLanger-backport-ac41c030 in my personal fork of openjdk/jdk17u. To create a pull request with this backport targeting openjdk/jdk17u:master, just click the following link:

➡️ Create pull request

The title of the pull request is automatically filled in correctly and below you find a suggestion for the pull request body:

Hi all,

This pull request contains a backport of commit ac41c030 from the openjdk/jdk repository.

The commit being backported was authored by Christoph Langer on 5 Jun 2023 and was reviewed by Matthias Baesken and Weijun Wang.

Thanks!

If you need to update the source branch of the pull then run the following commands in a local clone of your personal fork of openjdk/jdk17u:

$ git fetch https://github.com/openjdk-bots/jdk17u.git RealCLanger-backport-ac41c030:RealCLanger-backport-ac41c030
$ git checkout RealCLanger-backport-ac41c030
# make changes
$ git add paths/to/changed/files
$ git commit --message 'Describe additional changes made'
$ git push https://github.com/openjdk-bots/jdk17u.git RealCLanger-backport-ac41c030

@RealCLanger
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/backport jdk11u

@openjdk
Copy link

@openjdk openjdk bot commented on ac41c03 Jun 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@RealCLanger the backport was successfully created on the branch RealCLanger-backport-ac41c030 in my personal fork of openjdk/jdk11u. To create a pull request with this backport targeting openjdk/jdk11u:master, just click the following link:

➡️ Create pull request

The title of the pull request is automatically filled in correctly and below you find a suggestion for the pull request body:

Hi all,

This pull request contains a backport of commit ac41c030 from the openjdk/jdk repository.

The commit being backported was authored by Christoph Langer on 5 Jun 2023 and was reviewed by Matthias Baesken and Weijun Wang.

Thanks!

If you need to update the source branch of the pull then run the following commands in a local clone of your personal fork of openjdk/jdk11u:

$ git fetch https://github.com/openjdk-bots/jdk11u.git RealCLanger-backport-ac41c030:RealCLanger-backport-ac41c030
$ git checkout RealCLanger-backport-ac41c030
# make changes
$ git add paths/to/changed/files
$ git commit --message 'Describe additional changes made'
$ git push https://github.com/openjdk-bots/jdk11u.git RealCLanger-backport-ac41c030

Please sign in to comment.