Skip to content
Closed
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 3 additions & 9 deletions src/java.base/macosx/classes/apple/security/KeychainStore.java
Original file line number Diff line number Diff line change
Expand Up @@ -878,15 +878,9 @@ private void createTrustedCertEntry(String alias, List<String> inputTrust,
}

if (tce.trustSettings.isEmpty()) {
if (isSelfSigned) {
// If a self-signed certificate has trust settings without specific entries,
// trust it for all purposes
tce.trustedKeyUsageValue = KnownOIDs.anyExtendedKeyUsage.value();
} else {
// Otherwise, return immediately. The certificate is not
// added into entries.
return;
}
// If there is no trust settings then the certificate was verified against other trusted certificates already
// or it is self-signed
tce.trustedKeyUsageValue = KnownOIDs.anyExtendedKeyUsage.value();
} else {
List<String> values = new ArrayList<>();
for (var oneTrust : tce.trustSettings) {
Expand Down
37 changes: 35 additions & 2 deletions src/java.base/macosx/native/libosxsecurity/KeystoreImpl.m
Original file line number Diff line number Diff line change
Expand Up @@ -458,6 +458,34 @@ static bool createTrustedCertEntry(JNIEnv *env, jobject keyStore,
return true;
}

static bool validateCertificate(SecCertificateRef certRef) {
SecTrustRef secTrust = NULL;
CFMutableArrayRef subjCerts = CFArrayCreateMutable(NULL, 1, &kCFTypeArrayCallBacks);
CFArraySetValueAtIndex(subjCerts, 0, certRef);

SecPolicyRef policy = SecPolicyCreateBasicX509();
OSStatus ortn = SecTrustCreateWithCertificates(subjCerts, policy, &secTrust);
bool result = false;
if (ortn) {
/* should never happen */
cssmPerror("SecTrustCreateWithCertificates", ortn);
goto errOut;
}

result = SecTrustEvaluateWithError(secTrust, NULL);
errOut:
if (policy) {
CFRelease(policy);
}
if (secTrust) {
CFRelease(secTrust);
}
if (subjCerts) {
CFRelease(subjCerts);
}
return result;
}

static void addCertificatesToKeystore(JNIEnv *env, jobject keyStore,
jmethodID jm_createTrustedCertEntry,
jclass jc_arrayListClass,
Expand Down Expand Up @@ -492,9 +520,14 @@ static void addCertificatesToKeystore(JNIEnv *env, jobject keyStore,
goto errOut;
}

// Only add certificates with trust settings
// If no trust settings we need to verify the certificate first
if (inputTrust == NULL) {
continue;
bool valid = validateCertificate(certRef);
if (valid) {
inputTrust = (*env)->NewObject(env, jc_arrayListClass, jm_arrayListCons);
} else {
continue;
}
}

// Create java object for certificate with trust settings
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,170 @@
/*
* Copyright (c) 2025, Oracle and/or its affiliates. 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.nio.file.Path;
import java.security.KeyStore;
import java.security.KeyStoreException;
import java.security.cert.X509Certificate;
import java.util.Iterator;
import java.util.List;
import java.util.Spliterator;
import java.util.Spliterators;
import java.util.stream.Collectors;
import java.util.stream.StreamSupport;

import jdk.test.lib.process.ProcessTools;

import static org.junit.jupiter.api.Assertions.fail;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

/*
* @test
Copy link
Author

Choose a reason for hiding this comment

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

@alexeybakhtin quick question on how this should be marked as manual.

I see all tests in https://github.com/openjdk/jdk/blob/master/test/jdk/TEST.groups#L256-L259 are manual ones.
Is this test automatically included in that?

Or should it be added here?
https://github.com/openjdk/jdk/blob/master/test/jdk/TEST.groups#L657

Copy link
Contributor

Choose a reason for hiding this comment

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

The test should be marked as @run junit/manual and added to the jdk_security_manual_interactive part of the TEST.groups

Copy link
Author

@timja timja Jan 7, 2025

Choose a reason for hiding this comment

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

Any idea how I can run the test after making those changes?

The test just gets skipped with:

CONF=release make test TEST=test/jdk/java/security/KeyStore/CheckMacOSKeyChainIntermediateCATrust.java

I've checked through:
https://openjdk.org/jtreg/faq.html#how-do-i-run-only-tests-requiring-manual-intervention

and I saw somewhere mention of passing -manual but I can't get that to work with make test

Copy link
Author

@timja timja Jan 7, 2025

Choose a reason for hiding this comment

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

I was able to run it by setting up jtreg and avoiding the make test task.

jtreg.sh -manual test/jdk/java/security/KeyStore/CheckMacOSKeyChainIntermediateCATrust.java

Although it seems it ran anyway without passing manual:

jtreg.sh test/jdk/java/security/KeyStore/CheckMacOSKeyChainIntermediateCATrust.java

Test results: passed: 1

* @bug 8347067
* @library /test/lib
* @requires os.family == "mac"
* @summary Check whether loading of certificates from MacOS Keychain correctly
* loads intermediate CA certificates
* @run junit CheckMacOSKeyChainIntermediateCATrust
*/
public class CheckMacOSKeyChainIntermediateCATrust {

private static final String DIR = System.getProperty("test.src", ".");

@Test
public void test() throws Throwable {
KeyStore ks = KeyStore.getInstance("KeychainStore");
ks.load(null, null);

Iterator<String> iterator = ks.aliases().asIterator();
List<X509Certificate> certificates = StreamSupport.stream(
Spliterators.spliteratorUnknownSize(iterator, Spliterator.ORDERED), false)
.sorted()
.map(alias -> {
try {
return (X509Certificate) ks.getCertificate(alias);
} catch (KeyStoreException e) {
throw new RuntimeException(e);
}
})
.collect(Collectors.toList());

System.out.println("Verifying expected certificates are trusted");

String rootCASubjectName = "CN=Example CA,O=Example,C=US";
assertThat(containsSubjectName(certificates, rootCASubjectName), "Root CA not found " + rootCASubjectName, certificates);

String intermediateCASubjectName = "CN=Example Intermediate CA,O=Example,C=US";
assertThat(containsSubjectName(certificates, intermediateCASubjectName), "Intermediate CA not found " + intermediateCASubjectName, certificates);
}

@BeforeEach
public void setup() {
System.out.println("Adding certificates to key chain");
addCertificatesToKeyChain();
}

@AfterEach
public void cleanup() {
System.out.println("Cleaning up");
deleteCertificatesFromKeyChain();
}

private static void addCertificatesToKeyChain() {
String loginKeyChain = getLoginKeyChain();

Path caPath = Path.of("%s/%s".formatted(DIR, "test-ca.pem"));
List<String> args = List.of(
"/usr/bin/security",
"add-trusted-cert",
"-k", loginKeyChain,
caPath.toString()
);
executeProcess(args);

caPath = Path.of("%s/%s".formatted(DIR, "test-intermediate-ca.pem"));
args = List.of(
"/usr/bin/security",
"add-certificates",
"-k", loginKeyChain,
caPath.toString()
);
executeProcess(args);

}

private static String getLoginKeyChain() {
return Path.of(System.getProperty("user.home"), "Library/Keychains/login.keychain-db").toString();
}

private static void executeProcess(List<String> params) {
System.out.println("Command line: " + params);
try {
int exitStatus = ProcessTools.executeProcess(params.toArray(new String[0])).getExitValue();
if (exitStatus != 0) {
fail("Process started with: " + params + " failed");
}
} catch (Throwable e) {
fail(e.getMessage());
}
}

private static void deleteCertificatesFromKeyChain() {
executeProcess(
List.of(
"/usr/bin/security",
"delete-certificate",
"-c", "Example CA",
"-t"
)
);

executeProcess(
List.of(
"/usr/bin/security",
"delete-certificate",
"-c", "Example Intermediate CA",
"-t"
)
);
}

private static boolean containsSubjectName(List<X509Certificate> certificates, String subjectName) {
return certificates.stream()
.map(cert -> cert.getSubjectX500Principal().getName())
.anyMatch(name -> name.contains(subjectName));
}

private static List<String> getSubjects(List<X509Certificate> certificates) {
return certificates.stream()
.map(cert -> cert.getSubjectX500Principal().getName())
.toList();
}

private static void assertThat(boolean expected, String message, List<X509Certificate> certificates) {
if (!expected) {
throw new AssertionError(message + ", subjects: " + getSubjects(certificates));
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
#!/usr/bin/env bash

set -ex

cd "$(dirname "$0")"

openssl genrsa -out root.key 2048
openssl req -x509 -sha256 -nodes -extensions v3_ca -key root.key -subj "/C=US/O=Example/CN=Example CA" -days 3650 -out test-ca.pem

openssl genrsa -out intermediate.key 2048
openssl req -new -sha256 -nodes -key intermediate.key \
-subj "/C=US/O=Example/CN=Example Intermediate CA" -out test-intermediate-ca.csr

openssl x509 -req \
-extensions v3_ca \
-extfile openssl.cnf \
-in test-intermediate-ca.csr \
-CA test-ca.pem \
-CAkey root.key \
-CAcreateserial \
-out test-intermediate-ca.pem \
-days 3650 \
-sha256

rm -f root.key test-intermediate-ca.csr intermediate.key test-ca.srl
17 changes: 17 additions & 0 deletions test/jdk/java/security/KeyStore/openssl.cnf
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
[ v3_ca ]


# Extensions for a typical CA


# PKIX recommendation.

subjectKeyIdentifier=hash

authorityKeyIdentifier=keyid:always,issuer:always

# This is what PKIX recommends but some broken software chokes on critical
# extensions.
#basicConstraints = critical,CA:true
# So we do this instead.
basicConstraints = CA:true
20 changes: 20 additions & 0 deletions test/jdk/java/security/KeyStore/test-ca.pem
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
-----BEGIN CERTIFICATE-----
MIIDSTCCAjGgAwIBAgIUHYTHHRJWyWjKpg5H2acbXj0aDeUwDQYJKoZIhvcNAQEL
BQAwNDELMAkGA1UEBhMCVVMxEDAOBgNVBAoMB0V4YW1wbGUxEzARBgNVBAMMCkV4
YW1wbGUgQ0EwHhcNMjUwMTA3MTUxMzExWhcNMzUwMTA1MTUxMzExWjA0MQswCQYD
VQQGEwJVUzEQMA4GA1UECgwHRXhhbXBsZTETMBEGA1UEAwwKRXhhbXBsZSBDQTCC
ASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBANQtNFDwJ8KayH0hFHCioT18
aMToU8SNBT+ogKT7rQhWkqC4aBVS+H3JSN1vG/RN+qWFS/DrTpUb5tVY9mYH5s4p
PqnXXN1EIoOgn+2vYB7sMY/MyAzn3MynDNHE0QYKdxK3H06BrmTgDiDcpYEexRex
B/3p9daJrU9L/EK4l41Vk7jOqu3wq39ECvAdMpt1eIg02nUS1EIxLmfoFRSHHtRf
rRPAU84BdGCYMWuBfxcXqMn4PumfkS3AoG6ul277FUNsvjlmEoQeotuXwCz1ELyy
U3uHnSFItPs/69Bg2FxAeig9zbAnYO9eCYPrhME452tROnOqhm0LjZjR8R4Lbl0C
AwEAAaNTMFEwHQYDVR0OBBYEFGt4wbms5GC1yWwh2TytpEuHpMReMB8GA1UdIwQY
MBaAFGt4wbms5GC1yWwh2TytpEuHpMReMA8GA1UdEwEB/wQFMAMBAf8wDQYJKoZI
hvcNAQELBQADggEBAAaq5JrVfinF53fBQKwBZYXv7Afr/H04GYVhXqAtl1n65Maj
si4GRBDSyhMWUrwHFNH4SIGHu4LZU2aROXIXrJl3qFXFl3WfdyVZTNjssgVcKFxl
Uav+pISUiIpAZPV55tFkE/2gmpkLUyhaiUQvql4XTZDSU8mTcWovzMVusXQtfo+L
O848Fspo30BvPEUUt1BOhqKwWbHV/2WJ4vYJPt6jFGnDMZHdILCp/DOXBxHS0pxd
0Tofrfum0MUbtScuzjM/otMduwjSalrDhkNLsLxLueXTf6dtnE9CLxaWXhWlAJ70
0mGFdwvYtLEbLTJKtqqq+lafAt5Af69VoqM1jjo=
-----END CERTIFICATE-----
22 changes: 22 additions & 0 deletions test/jdk/java/security/KeyStore/test-intermediate-ca.pem
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
-----BEGIN CERTIFICATE-----
MIIDpTCCAo2gAwIBAgIUBdWVSNEFs3LuTagptBgmihEZrVowDQYJKoZIhvcNAQEL
BQAwNDELMAkGA1UEBhMCVVMxEDAOBgNVBAoMB0V4YW1wbGUxEzARBgNVBAMMCkV4
YW1wbGUgQ0EwHhcNMjUwMTA3MTUxMzExWhcNMzUwMTA1MTUxMzExWjBBMQswCQYD
VQQGEwJVUzEQMA4GA1UECgwHRXhhbXBsZTEgMB4GA1UEAwwXRXhhbXBsZSBJbnRl
cm1lZGlhdGUgQ0EwggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQCXK7MQ
/LfR/zm8fV677S2NCsBVdizTsE5ugYuXGNlrbF26pcyJhK8N/T2l2HAz2UszzRB2
ZpqyE21SpepYATvG9M3moPEkFiojmhS+3mPhRhgTAxAIYA+hQ+8ics6C2zCvl2vw
5PpHdbZiL+2K/j+DtZxfmJoG0HYWMHeqlbA3smEkNOfOS9rc0kGpu3Q4mCLO+kmb
IofGf9ASsuyWH9GgxCcmUlu6UTRvt57LlTcaz3mncq6V++UDZHOmyTa6q9GIgTUc
sTfcS0aMXfBvsw6eZhlTcWvkXQaJRVb6UeGJwx/Kq715XphWJ9wXc4pT/f7nHinc
utdXjnayO+o3Vk85AgMBAAGjgaEwgZ4wHQYDVR0OBBYEFGlsam1VG6UErlvVa1+p
40CuawniMG8GA1UdIwRoMGaAFGt4wbms5GC1yWwh2TytpEuHpMReoTikNjA0MQsw
CQYDVQQGEwJVUzEQMA4GA1UECgwHRXhhbXBsZTETMBEGA1UEAwwKRXhhbXBsZSBD
QYIUHYTHHRJWyWjKpg5H2acbXj0aDeUwDAYDVR0TBAUwAwEB/zANBgkqhkiG9w0B
AQsFAAOCAQEAS9n0MIie2TCo+5Wbur7RrX4aOMrKooZwSYxyvb4c+U6zNYBZmkwI
Pqk/LYNU6FMCKCg/PT4kcgGAu+UeYb6K2Llwyej4Tjy/8bCOXgrPTzP4f0COg2wi
y1SlMSEolhslJV46HlZHtrASpE8mj7mh3RRTAkn86gCEd7A/CdQgMrTiwVjOGrza
B5/UGJpVUvm7W5H1UXunYqFrVaMIN0zWfaj4lRgzLAkZ7ldLdBA7mIbc2/C9JSvX
dCezRkjJpIHbXjojtDek1vDy/UopyQRYpz2CPu62o8iM9Iw6M7SzF6d8ud9rzeZ1
zwtxTCJqohwP3t132oxoYwEyYpF+Xcrvhg==
-----END CERTIFICATE-----