This is an automated email from the ASF dual-hosted git repository.
markt-asf pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/main by this push:
new 1b164484b9 Review "Refactor certificate loading"
1b164484b9 is described below
commit 1b164484b95fc732c2c8e1fe8124857295c5ef96
Author: Mark Thomas <[email protected]>
AuthorDate: Tue Nov 3 11:34:54 2026 +0000
Review "Refactor certificate loading"
Do not ignore certificate chains in PKCS#12 files
---
build.xml | 29 +++++++-
.../util/net/openssl/panama/CertificateLoader.java | 66 +++++++++++++-----
.../tomcat/util/net/TestSSLHostConfigCompat.java | 61 ++++++++++++++++
test/org/apache/tomcat/util/net/TesterSupport.java | 37 ++++++++++
.../net/openssl/panama/TestCertificateLoader.java | 81 ++++++++++++++++++++++
5 files changed, 255 insertions(+), 19 deletions(-)
diff --git a/build.xml b/build.xml
index 0138bb3247..aa1b5f87ef 100644
--- a/build.xml
+++ b/build.xml
@@ -218,6 +218,11 @@
<!-- set above.
-->
<property name="test.jvmarg.egd" value="-Dignore" />
+ <!-- Flag for building FFM and associated tests -->
+ <condition property="has-ffm">
+ <javaversion atleast="${release.java.version}" />
+ </condition>
+
<!-- Location of OpenSSL binary (file name, not directory) -->
<!-- The OpenSSL tests cases be disabled by specifying an invalid path here
-->
<property name="test.openssl.path" value="" />
@@ -1053,9 +1058,6 @@
<exclude name="org/apache/tomcat/util/openssl/**"/>
</javac>
<!-- Compile internal server components that use FFM -->
- <condition property="has-ffm">
- <javaversion atleast="${release.java.version}" />
- </condition>
<javac srcdir="java" destdir="${tomcat.classes}"
debug="${compile.debug}"
deprecation="${compile.deprecation}"
@@ -1947,6 +1949,24 @@
<!-- Exclude these by default as Gump can't run them because the -->
<!-- classes on which they depend are not in any output JAR. -->
<exclude name="org/apache/tomcat/buildutil/**"
unless="test.includeBuildutil"/>
+ <!-- Exclude FFM tests as they are compiled below -->
+ <exclude name="org/apache/tomcat/util/net/openssl/panama/**"/>
+ <exclude name="org/apache/tomcat/util/openssl/**"/>
+ </javac>
+ <!-- Compile tests that use FFM -->
+ <javac srcdir="test" destdir="${test.classes}"
+ debug="${compile.debug}"
+ deprecation="${compile.deprecation}"
+ release="${release.java.version}"
+ encoding="ISO-8859-1"
+ includeAntRuntime="true"
+ if:set="has-ffm" >
+ <!-- Uncomment this to show unchecked warnings:
+ <compilerarg value="-Xlint:unchecked"/>
+ -->
+ <classpath refid="tomcat.test.classpath" />
+ <include name="org/apache/tomcat/util/net/openssl/panama/**"/>
+ <include name="org/apache/tomcat/util/openssl/**"/>
</javac>
<copy todir="${test.classes}">
<fileset dir="test">
@@ -2291,6 +2311,9 @@
<exclude name="**/integration/**"
unless="test.includeIntegration" />
<!-- Exclude tests that require large heaps -->
<exclude name="**/*LargeHeap.java"
unless="${test.includeLargeHeap}" />
+ <!-- Exclude FFM tests unless FFM has been compiled -->
+ <exclude name="org/apache/tomcat/util/net/openssl/panama/**"
unless="has-ffm"/>
+ <exclude name="org/apache/tomcat/util/openssl/**"
unless="has-ffm"/>
</fileset>
</batchtest>
</junit>
diff --git
a/java/org/apache/tomcat/util/net/openssl/panama/CertificateLoader.java
b/java/org/apache/tomcat/util/net/openssl/panama/CertificateLoader.java
index 0dc416c8d1..de27351e1b 100644
--- a/java/org/apache/tomcat/util/net/openssl/panama/CertificateLoader.java
+++ b/java/org/apache/tomcat/util/net/openssl/panama/CertificateLoader.java
@@ -371,19 +371,9 @@ public final class CertificateLoader {
return null;
}
- if (chainFile != null) {
- byte[] chainBytes = readConfigFileBytes(chainFile);
- if (chainBytes == null) {
-
log.warn(sm.getString("certificateLoader.certificateLoadFailed", hostName,
chainFile));
- } else {
- // Append the separately configured chain to the intermediates
- // (if any) read from the certificate file.
- MemorySegment[] extraChain =
readX509ChainFromBytes(chainBytes);
- chain = Arrays.copyOf(chain, chain.length + extraChain.length);
- System.arraycopy(extraChain, 0, chain, chain.length -
extraChain.length,
- extraChain.length);
- }
- }
+ // Append the separately configured chain to the intermediates (if any)
+ // read from the certificate file.
+ chain = appendChainFile(chainFile, hostName, chain);
return new CertificateEntry(leaf, key, chain);
}
@@ -419,8 +409,15 @@ public final class CertificateLoader {
KeyStore keyStore = KeyStore.getInstance("PKCS12");
keyStore.load(new ByteArrayInputStream(fileBytes),
(keyPassToUse != null) ? keyPassToUse.toCharArray() :
null);
- return loadJavaKeyStoreKey(keyStore,
certificate.getCertificateKeyAlias(), keyPassToUse,
- certFile, hostName, arena);
+ CertificateEntry entry = loadJavaKeyStoreKey(keyStore,
certificate.getCertificateKeyAlias(),
+ keyPassToUse, certFile, hostName, arena);
+ if (entry != null && certificate.getCertificateChainFile() !=
null) {
+ // Append the separately configured chain to the intermediates
+ // (if any) held by the bundle.
+ entry = new CertificateEntry(entry.leaf, entry.key,
+ appendChainFile(certificate.getCertificateChainFile(),
hostName, entry.chain));
+ }
+ return entry;
} catch (IOException e) {
// Covers a wrong password: the JDK fails the MAC check
log.warn(sm.getString("certificateLoader.certificateLoadFailed",
hostName,
@@ -606,6 +603,32 @@ public final class CertificateLoader {
// ------------------------------------- File and native reading helpers
+ /**
+ * Reads the certificates of the configured chain file and appends them to
the given chain. A chain file that
+ * cannot be read is logged and ignored.
+ *
+ * @param chainFile The location of the chain file, or {@code null} if
none is configured
+ * @param hostName The host name the certificate belongs to, used for
logging
+ * @param chain The certificates (owned by the caller) to append to
+ *
+ * @return the chain with the certificates of the chain file appended,
which owns all the native objects
+ */
+ private static MemorySegment[] appendChainFile(String chainFile, String
hostName, MemorySegment[] chain) {
+ if (chainFile == null) {
+ return chain;
+ }
+ byte[] chainBytes = readConfigFileBytes(chainFile);
+ if (chainBytes == null) {
+ log.warn(sm.getString("certificateLoader.certificateLoadFailed",
hostName, chainFile));
+ return chain;
+ }
+ MemorySegment[] extraChain = readX509ChainFromBytes(chainBytes);
+ MemorySegment[] result = Arrays.copyOf(chain, chain.length +
extraChain.length);
+ System.arraycopy(extraChain, 0, result, chain.length,
extraChain.length);
+ return result;
+ }
+
+
/**
* An OpenSSL ENGINE needs a real file. Resolves a configured location,
relative to the Catalina base if
* necessary, to the path of an existing file.
@@ -941,8 +964,19 @@ public final class CertificateLoader {
KeyStore keyStore = KeyStore.getInstance("PKCS12");
keyStore.load(new ByteArrayInputStream(fileBytes),
(password != null) ? password.toCharArray() : null);
- return getCertificateChainKeyStore(keyStore,
+ X509Certificate[] bundleChain = getCertificateChainKeyStore(keyStore,
certificate.getCertificateKeyAlias());
+ String chainFile = certificate.getCertificateChainFile();
+ if (bundleChain == null || chainFile == null) {
+ return bundleChain;
+ }
+ // A separately configured chain file is appended to the chain
+ List<X509Certificate> chain = new
ArrayList<>(Arrays.asList(bundleChain));
+ byte[] chainBytes = readConfigFileBytes(chainFile);
+ if (chainBytes != null) {
+ addX509Certificates(chain,
CertificateFactory.getInstance("X.509"), chainBytes);
+ }
+ return chain.toArray(new X509Certificate[0]);
}
diff --git a/test/org/apache/tomcat/util/net/TestSSLHostConfigCompat.java
b/test/org/apache/tomcat/util/net/TestSSLHostConfigCompat.java
index c223dc3af9..cedb4b0206 100644
--- a/test/org/apache/tomcat/util/net/TestSSLHostConfigCompat.java
+++ b/test/org/apache/tomcat/util/net/TestSSLHostConfigCompat.java
@@ -17,10 +17,17 @@
package org.apache.tomcat.util.net;
import java.io.File;
+import java.security.cert.Certificate;
+import java.security.cert.X509Certificate;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.SSLSocket;
+import javax.net.ssl.TrustManager;
+import javax.net.ssl.X509TrustManager;
+
import org.junit.Assert;
import org.junit.Assume;
import org.junit.Test;
@@ -261,6 +268,60 @@ public class TestSSLHostConfigCompat extends
TomcatBaseTest {
}
+ /*
+ * The certificates of the chain file must be sent after the certificate
of a PKCS#12 bundle just as they are for a
+ * PEM certificate. The bundle only holds the leaf certificate so the CA
certificate can only come from the chain
+ * file. This is a feature of the OpenSSL FFM implementation which is the
only one that reads PKCS#12 bundles
+ * through the certificate file.
+ */
+ @Test
+ public void testHostRSAWithPkcs12AndChainFile() throws Exception {
+ Assume.assumeTrue("Only needs to be run once", storeType ==
StoreType.PEM);
+ Assume.assumeTrue("Only applies to the OpenSSL FFM implementation",
+
"org.apache.tomcat.util.net.openssl.panama.OpenSSLImplementation".equals(sslImplementationName));
+
+ SSLHostConfig sslHostConfig = getSSLHostConfig();
+ SSLHostConfigCertificate certificate = new
SSLHostConfigCertificate(sslHostConfig, Type.RSA);
+
certificate.setCertificateFile(TesterSupport.createLocalhostRsaLeafOnlyPkcs12().getAbsolutePath());
+ certificate.setCertificateKeyPassword(TesterSupport.JKS_PASS);
+
certificate.setCertificateChainFile(getPath(TesterSupport.CA_CERT_PEM));
+ sslHostConfig.addCertificate(certificate);
+
+ Tomcat tomcat = getTomcatInstance();
+ tomcat.start();
+
+ // Trust everything, the test is about what the server sends
+ SSLContext clientContext = SSLContext.getInstance("TLS");
+ clientContext.init(null, new TrustManager[] { new X509TrustManager() {
+ @Override
+ public void checkClientTrusted(X509Certificate[] chain, String
authType) {
+ // NO-OP
+ }
+
+ @Override
+ public void checkServerTrusted(X509Certificate[] chain, String
authType) {
+ // NO-OP
+ }
+
+ @Override
+ public X509Certificate[] getAcceptedIssuers() {
+ return new X509Certificate[0];
+ }
+ } }, null);
+
+ try (SSLSocket socket = (SSLSocket)
clientContext.getSocketFactory().createSocket("localhost", getPort())) {
+ socket.setSoTimeout(5000);
+ socket.startHandshake();
+ Certificate[] peerCertificates =
socket.getSession().getPeerCertificates();
+
+ Assert.assertEquals("The chain file was not sent", 2,
peerCertificates.length);
+ X509Certificate leaf = (X509Certificate) peerCertificates[0];
+ X509Certificate ca = (X509Certificate) peerCertificates[1];
+ Assert.assertEquals(leaf.getIssuerX500Principal(),
ca.getSubjectX500Principal());
+ }
+ }
+
+
private void configureHostRSA() {
SSLHostConfig sslHostConfig = getSSLHostConfig();
switch (storeType) {
diff --git a/test/org/apache/tomcat/util/net/TesterSupport.java
b/test/org/apache/tomcat/util/net/TesterSupport.java
index 3592567592..9681bb62ca 100644
--- a/test/org/apache/tomcat/util/net/TesterSupport.java
+++ b/test/org/apache/tomcat/util/net/TesterSupport.java
@@ -18,8 +18,10 @@ package org.apache.tomcat.util.net;
import java.io.File;
import java.io.FileInputStream;
+import java.io.FileOutputStream;
import java.io.IOException;
import java.io.InputStream;
+import java.io.OutputStream;
import java.net.InetAddress;
import java.net.Socket;
import java.net.UnknownHostException;
@@ -292,6 +294,41 @@ public final class TesterSupport {
return ks;
}
+ /**
+ * Creates a PKCS#12 bundle (the file name ends with {@code .pkcs12}) that
holds the localhost RSA private key and
+ * only its own certificate, i.e. without the certificate of the CA that
issued it. The bundle is protected with
+ * {@link #JKS_PASS}.
+ *
+ * @return the bundle, which is deleted when the JVM exits
+ *
+ * @throws Exception if the bundle could not be created
+ */
+ public static File createLocalhostRsaLeafOnlyPkcs12() throws Exception {
+ KeyStore jks = getKeyStore(LOCALHOST_RSA_JKS);
+ String alias = null;
+ Enumeration<String> aliases = jks.aliases();
+ while (alias == null && aliases.hasMoreElements()) {
+ String candidate = aliases.nextElement();
+ if (jks.isKeyEntry(candidate)) {
+ alias = candidate;
+ }
+ }
+ Assert.assertNotNull(alias);
+ PrivateKey key = (PrivateKey) jks.getKey(alias,
JKS_PASS.toCharArray());
+ Certificate leaf = jks.getCertificateChain(alias)[0];
+
+ KeyStore pkcs12 = KeyStore.getInstance("PKCS12");
+ pkcs12.load(null, null);
+ pkcs12.setKeyEntry("localhost", key, JKS_PASS.toCharArray(), new
Certificate[] { leaf });
+
+ File file = File.createTempFile("tomcat-test-", ".pkcs12");
+ file.deleteOnExit();
+ try (OutputStream os = new FileOutputStream(file)) {
+ pkcs12.store(os, JKS_PASS.toCharArray());
+ }
+ return file;
+ }
+
protected static boolean isClientRenegotiationSupported(Tomcat tomcat) {
// Disabled for Tomcat Native (part of response to CVE-2009-3555)
// Only JRE provided JSSE implementation supports this
diff --git
a/test/org/apache/tomcat/util/net/openssl/panama/TestCertificateLoader.java
b/test/org/apache/tomcat/util/net/openssl/panama/TestCertificateLoader.java
new file mode 100644
index 0000000000..62ba3f6a04
--- /dev/null
+++ b/test/org/apache/tomcat/util/net/openssl/panama/TestCertificateLoader.java
@@ -0,0 +1,81 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.tomcat.util.net.openssl.panama;
+
+import java.io.File;
+import java.security.cert.X509Certificate;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+import org.apache.tomcat.util.net.SSLHostConfig;
+import org.apache.tomcat.util.net.SSLHostConfigCertificate;
+import org.apache.tomcat.util.net.SSLHostConfigCertificate.Type;
+import org.apache.tomcat.util.net.TesterSupport;
+
+/*
+ * Only the parts of CertificateLoader that do not need the native OpenSSL
library are tested here. Loading into
+ * native objects is covered by the tests that start a connector that uses the
OpenSSL FFM implementation.
+ */
+public class TestCertificateLoader {
+
+ private static SSLHostConfigCertificate createPkcs12Certificate(String
chainFile) throws Exception {
+ SSLHostConfig sslHostConfig = new SSLHostConfig();
+ SSLHostConfigCertificate certificate = new
SSLHostConfigCertificate(sslHostConfig, Type.RSA);
+
certificate.setCertificateFile(TesterSupport.createLocalhostRsaLeafOnlyPkcs12().getAbsolutePath());
+ certificate.setCertificateKeyPassword(TesterSupport.JKS_PASS);
+ if (chainFile != null) {
+ certificate.setCertificateChainFile(new
File(chainFile).getAbsolutePath());
+ }
+ return certificate;
+ }
+
+
+ @Test
+ public void testGetCertificateChainPkcs12() throws Exception {
+ X509Certificate[] chain =
CertificateLoader.getCertificateChain(createPkcs12Certificate(null));
+
+ // The bundle only holds the leaf certificate
+ Assert.assertNotNull(chain);
+ Assert.assertEquals(1, chain.length);
+ }
+
+
+ /*
+ * A separately configured chain file is appended to the chain of the
bundle, as it is for a PEM certificate.
+ */
+ @Test
+ public void testGetCertificateChainPkcs12WithChainFile() throws Exception {
+ X509Certificate[] chain =
+
CertificateLoader.getCertificateChain(createPkcs12Certificate(TesterSupport.CA_CERT_PEM));
+
+ Assert.assertNotNull(chain);
+ Assert.assertEquals(2, chain.length);
+ Assert.assertEquals(chain[0].getIssuerX500Principal(),
chain[1].getSubjectX500Principal());
+ }
+
+
+ @Test
+ public void testGetCertificateChainPkcs12WithMissingChainFile() throws
Exception {
+ X509Certificate[] chain = CertificateLoader.getCertificateChain(
+
createPkcs12Certificate("test/org/apache/tomcat/util/net/does-not-exist.pem"));
+
+ // A chain file that cannot be read is ignored, as it is when loading
the certificate
+ Assert.assertNotNull(chain);
+ Assert.assertEquals(1, chain.length);
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
