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 7b3d39addd Revert "Review "Refactor certificate loading""
7b3d39addd is described below
commit 7b3d39addd7d24e4e6034830182fd26de335da37
Author: Mark Thomas <[email protected]>
AuthorDate: Tue Nov 3 11:38:11 2026 +0000
Revert "Review "Refactor certificate loading""
This reverts commit 1b164484b95fc732c2c8e1fe8124857295c5ef96.
---
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, 19 insertions(+), 255 deletions(-)
diff --git a/build.xml b/build.xml
index aa1b5f87ef..0138bb3247 100644
--- a/build.xml
+++ b/build.xml
@@ -218,11 +218,6 @@
<!-- 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="" />
@@ -1058,6 +1053,9 @@
<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}"
@@ -1949,24 +1947,6 @@
<!-- 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">
@@ -2311,9 +2291,6 @@
<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 de27351e1b..0dc416c8d1 100644
--- a/java/org/apache/tomcat/util/net/openssl/panama/CertificateLoader.java
+++ b/java/org/apache/tomcat/util/net/openssl/panama/CertificateLoader.java
@@ -371,9 +371,19 @@ public final class CertificateLoader {
return null;
}
- // Append the separately configured chain to the intermediates (if any)
- // read from the certificate file.
- chain = appendChainFile(chainFile, hostName, chain);
+ 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);
+ }
+ }
return new CertificateEntry(leaf, key, chain);
}
@@ -409,15 +419,8 @@ public final class CertificateLoader {
KeyStore keyStore = KeyStore.getInstance("PKCS12");
keyStore.load(new ByteArrayInputStream(fileBytes),
(keyPassToUse != null) ? keyPassToUse.toCharArray() :
null);
- 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;
+ return loadJavaKeyStoreKey(keyStore,
certificate.getCertificateKeyAlias(), keyPassToUse,
+ certFile, hostName, arena);
} catch (IOException e) {
// Covers a wrong password: the JDK fails the MAC check
log.warn(sm.getString("certificateLoader.certificateLoadFailed",
hostName,
@@ -603,32 +606,6 @@ 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.
@@ -964,19 +941,8 @@ public final class CertificateLoader {
KeyStore keyStore = KeyStore.getInstance("PKCS12");
keyStore.load(new ByteArrayInputStream(fileBytes),
(password != null) ? password.toCharArray() : null);
- X509Certificate[] bundleChain = getCertificateChainKeyStore(keyStore,
+ return 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 cedb4b0206..c223dc3af9 100644
--- a/test/org/apache/tomcat/util/net/TestSSLHostConfigCompat.java
+++ b/test/org/apache/tomcat/util/net/TestSSLHostConfigCompat.java
@@ -17,17 +17,10 @@
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;
@@ -268,60 +261,6 @@ 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 9681bb62ca..3592567592 100644
--- a/test/org/apache/tomcat/util/net/TesterSupport.java
+++ b/test/org/apache/tomcat/util/net/TesterSupport.java
@@ -18,10 +18,8 @@ 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;
@@ -294,41 +292,6 @@ 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
deleted file mode 100644
index 62ba3f6a04..0000000000
--- a/test/org/apache/tomcat/util/net/openssl/panama/TestCertificateLoader.java
+++ /dev/null
@@ -1,81 +0,0 @@
-/*
- * 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]
