On 03/11/2026 15:10, Rémy Maucherat via dev wrote:
On Tue, Nov 3, 2026 at 3:06 PM markt.apache.org via dev
<[email protected]> wrote:

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 224095d6f5 Review "Refactor certificate loading"
224095d6f5 is described below

commit 224095d6f52ac3afd76219d575221adad22021fe
Author: Mark Thomas <[email protected]>
AuthorDate: Tue Nov 3 14:05:04 2026 +0000

     Review "Refactor certificate loading"

     Skip the certificate chain file if the certificate file includes the
     chain.

I am not against the change or anything, but this reads: "If the
certificate file already contains the certificate chain, the chain is
taken from the certificate file (skipping the server certificate) and
the chain file is ignored.".
That looks a lot like smoothing out a configuration error at this
point. What if the certificate chain provided is nonsense or does not
match, and so on ?

Before this change the certifcate chain from the certificate file and the certificate chain from the chain file were both provided to the client for at least one of the TLS implementations.

httpd is moving away from certificateChainFile hence why I selected the certifcate file to take priority with certificateChainFile used as a backup of the certificate file didn't include the chain.
Note: CertificateLoader exists to take out the certificate handling
code out of OpenSSLContext, without functionality changes. This was
requested or suggested in the past, I don't really remember. This was
done also for reuse of the certificate loading, but you don't have to
care about it ;)

I'm neutral on that but the review flagged multiple behaviour changes that I was trying to resolve. At the same time I wanted to improve the consistency of behaviour across the TLS implementations.

Mark




Rémy

---
  build.xml                                          |  29 +++-
  .../apache/tomcat/util/net/LocalStrings.properties |   1 +
  .../tomcat/util/net/SSLHostConfigCertificate.java  |   6 +-
  java/org/apache/tomcat/util/net/SSLUtilBase.java   |  17 ++-
  .../util/net/openssl/LocalStrings.properties       |   1 +
  .../tomcat/util/net/openssl/OpenSSLContext.java    |  57 +++++++-
  .../util/net/openssl/panama/CertificateLoader.java |  77 ++++++++---
  .../net/openssl/panama/LocalStrings.properties     |   1 +
  .../tomcat/util/net/TestSSLHostConfigCompat.java   | 151 +++++++++++++++++++++
  test/org/apache/tomcat/util/net/TesterSupport.java | 125 +++++++++++++++++
  .../net/openssl/panama/TestCertificateLoader.java  | 139 +++++++++++++++++++
  webapps/docs/changelog.xml                         |   9 ++
  webapps/docs/config/http.xml                       |  14 ++
  webapps/docs/ssl-howto.xml                         |   6 +
  14 files changed, 604 insertions(+), 29 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/LocalStrings.properties 
b/java/org/apache/tomcat/util/net/LocalStrings.properties
index 6bc78573bd..96e7d980e4 100644
--- a/java/org/apache/tomcat/util/net/LocalStrings.properties
+++ b/java/org/apache/tomcat/util/net/LocalStrings.properties
@@ -172,6 +172,7 @@ sslImplementation.cnfe=Unable to create SSLImplementation 
for class [{0}]
  sslUtilBase.active=The [{0}] that are active are : [{1}]
  sslUtilBase.aliasIgnored=FIPS enabled so alias name [{0}] will be ignored. If 
there is more than one key in the key store, the key used will depend on the 
key store implementation
  sslUtilBase.alias_no_key_entry=Alias name [{0}] does not identify a key entry
+sslUtilBase.chainFileIgnored=The certificate chain file [{0}] is ignored 
because the certificate file [{1}] already contains the certificate chain
  sslUtilBase.fipsPemNotSupported=FIPS enabled so PEM format certificate files 
are not supported. Configure the certificate using a key store rather than the 
certificateFile attribute
  sslUtilBase.invalidTrustManagerClassName=The trustManagerClassName provided 
[{0}] does not implement javax.net.ssl.TrustManager
  sslUtilBase.keystore_load_failed=Failed to load keystore type [{0}] with path 
[{1}] due to [{2}]
diff --git a/java/org/apache/tomcat/util/net/SSLHostConfigCertificate.java 
b/java/org/apache/tomcat/util/net/SSLHostConfigCertificate.java
index edd52cda0f..04eafc632e 100644
--- a/java/org/apache/tomcat/util/net/SSLHostConfigCertificate.java
+++ b/java/org/apache/tomcat/util/net/SSLHostConfigCertificate.java
@@ -478,7 +478,9 @@ public class SSLHostConfigCertificate implements 
Serializable {
      // OpenSSL

      /**
-     * Sets the path to the certificate chain file used by OpenSSL.
+     * Sets the path to the certificate chain file. The chain file is only 
used if the certificate file
+     * (a PEM file or a PKCS#12 bundle) contains the server certificate alone. 
If the certificate file already contains
+     * the certificate chain, the chain file is ignored.
       *
       * @param certificateChainFile the path to the certificate chain file
       */
@@ -489,7 +491,7 @@ public class SSLHostConfigCertificate implements 
Serializable {


      /**
-     * Returns the path to the certificate chain file used by OpenSSL.
+     * Returns the path to the certificate chain file.
       *
       * @return the certificate chain file path
       */
diff --git a/java/org/apache/tomcat/util/net/SSLUtilBase.java 
b/java/org/apache/tomcat/util/net/SSLUtilBase.java
index 68ac2ecbb8..7b1d5e5c92 100644
--- a/java/org/apache/tomcat/util/net/SSLUtilBase.java
+++ b/java/org/apache/tomcat/util/net/SSLUtilBase.java
@@ -375,12 +375,23 @@ public abstract class SSLUtilBase implements SSLUtil {
              PEMFile privateKeyFile =
                      new PEMFile(certificate.getCertificateKeyFile() != null ? 
certificate.getCertificateKeyFile() :
                              certificate.getCertificateFile(), keyPass, 
keyPassFile, null);
-            PEMFile certificateFile = new 
PEMFile(certificate.getCertificateFile());
+            // The certificate file may also hold the private key, which may 
be encrypted, so the key password is
+            // needed to read the file.
+            PEMFile certificateFile = new 
PEMFile(certificate.getCertificateFile(), keyPass, keyPassFile, null);

              Collection<Certificate> chain = new 
ArrayList<>(certificateFile.getCertificates());
              if (certificate.getCertificateChainFile() != null) {
-                PEMFile certificateChainFile = new 
PEMFile(certificate.getCertificateChainFile());
-                chain.addAll(certificateChainFile.getCertificates());
+                if (chain.size() > 1) {
+                    // The chain file is only used if the certificate file 
holds the server certificate alone. If the
+                    // certificate file already contains the certificate 
chain, the chain file is ignored.
+                    if (log.isDebugEnabled()) {
+                        log.debug(sm.getString("sslUtilBase.chainFileIgnored", 
certificate.getCertificateChainFile(),
+                                certificate.getCertificateFile()));
+                    }
+                } else {
+                    PEMFile certificateChainFile = new 
PEMFile(certificate.getCertificateChainFile());
+                    chain.addAll(certificateChainFile.getCertificates());
+                }
              }

              if (keyAlias == null) {
diff --git a/java/org/apache/tomcat/util/net/openssl/LocalStrings.properties 
b/java/org/apache/tomcat/util/net/openssl/LocalStrings.properties
index 2978501885..6bf1dee206 100644
--- a/java/org/apache/tomcat/util/net/openssl/LocalStrings.properties
+++ b/java/org/apache/tomcat/util/net/openssl/LocalStrings.properties
@@ -40,6 +40,7 @@ openssl.X509FactoryError=Error getting X509 factory instance
  openssl.addedClientCaCert=Added client CA cert: [{0}]
  openssl.applyConf=Applying OpenSSLConfCmd to SSL context
  openssl.certificateVerificationFailed=Certificate verification failed
+openssl.chainFileIgnored=The certificate chain file [{0}] is ignored because 
the certificate file [{1}] already contains the certificate chain
  openssl.checkConf=Checking OpenSSLConf
  openssl.doubleInit=SSL context already initialized, ignoring
  openssl.errApplyConf=Could not apply OpenSSLConf to SSL context
diff --git a/java/org/apache/tomcat/util/net/openssl/OpenSSLContext.java 
b/java/org/apache/tomcat/util/net/openssl/OpenSSLContext.java
index 619fc78b4c..d886a650a6 100644
--- a/java/org/apache/tomcat/util/net/openssl/OpenSSLContext.java
+++ b/java/org/apache/tomcat/util/net/openssl/OpenSSLContext.java
@@ -18,6 +18,7 @@ package org.apache.tomcat.util.net.openssl;

  import java.io.BufferedReader;
  import java.io.FileInputStream;
+import java.io.IOException;
  import java.io.InputStreamReader;
  import java.lang.ref.Cleaner;
  import java.lang.ref.Cleaner.Cleanable;
@@ -33,6 +34,7 @@ import java.util.Arrays;
  import java.util.Base64;
  import java.util.Iterator;
  import java.util.List;
+import java.util.Locale;
  import java.util.Set;
  import java.util.concurrent.locks.Lock;

@@ -550,6 +552,42 @@ public class OpenSSLContext implements 
org.apache.tomcat.util.net.SSLContext {
      }


+    /**
+     * Determines if a PEM certificate file holds more than the server 
certificate, i.e. it also holds the certificate
+     * chain. The certificates are counted without decoding the file. The file 
may also hold a private key, which may
+     * be encrypted, and the password for that key is not needed to find out 
if there is a certificate chain.
+     *
+     * @param certificateFile The certificate file
+     *
+     * @return {@code true} if the file holds the server certificate and at 
least one more certificate
+     */
+    private static boolean hasCertificateChain(String certificateFile) {
+        if (certificateFile.toLowerCase(Locale.ENGLISH).endsWith(".pkcs12")) {
+            // Not a PEM file. Leave the handling of the bundle to the native 
library.
+            return false;
+        }
+        int certificates = 0;
+        try (BufferedReader reader = new BufferedReader(
+                new InputStreamReader(new FileInputStream(certificateFile), 
StandardCharsets.ISO_8859_1))) {
+            String line;
+            while ((line = reader.readLine()) != null) {
+                // The types of PEM object that OpenSSL reads as a certificate
+                if (line.startsWith("-----BEGIN CERTIFICATE-----") ||
+                        line.startsWith("-----BEGIN X509 CERTIFICATE-----") ||
+                        line.startsWith("-----BEGIN TRUSTED 
CERTIFICATE-----")) {
+                    certificates++;
+                    if (certificates > 1) {
+                        return true;
+                    }
+                }
+            }
+        } catch (IOException e) {
+            // Leave any problems with the file to the native library.
+        }
+        return false;
+    }
+
+
      /**
       * Adds a certificate to this SSL context.
       *
@@ -571,12 +609,23 @@ public class OpenSSLContext implements 
org.apache.tomcat.util.net.SSLContext {
              } else {
                  passwordToUse = certificate.getCertificateKeyPassword();
              }
-            SSLContext.setCertificate(state.ctx, 
SSLHostConfig.adjustRelativePath(certificate.getCertificateFile()),
+            String certificateFile = 
SSLHostConfig.adjustRelativePath(certificate.getCertificateFile());
+            SSLContext.setCertificate(state.ctx, certificateFile,
                      
SSLHostConfig.adjustRelativePath(certificate.getCertificateKeyFile()), 
passwordToUse,
                      getCertificateIndex(certificate));
-            // Set certificate chain file
-            SSLContext.setCertificateChainFile(state.ctx,
-                    
SSLHostConfig.adjustRelativePath(certificate.getCertificateChainFile()), false);
+            // Set the certificate chain. The chain file is only used if the 
certificate file holds the server
+            // certificate alone. If the certificate file already contains the 
certificate chain, the chain is taken
+            // from the certificate file (skipping the server certificate) and 
the chain file is ignored.
+            if (hasCertificateChain(certificateFile)) {
+                if (log.isDebugEnabled() && 
certificate.getCertificateChainFile() != null) {
+                    log.debug(sm.getString("openssl.chainFileIgnored", 
certificate.getCertificateChainFile(),
+                            certificate.getCertificateFile()));
+                }
+                SSLContext.setCertificateChainFile(state.ctx, certificateFile, 
true);
+            } else {
+                SSLContext.setCertificateChainFile(state.ctx,
+                        
SSLHostConfig.adjustRelativePath(certificate.getCertificateChainFile()), false);
+            }
          } else {
              String alias = certificate.getCertificateKeyAlias();
              X509KeyManager x509KeyManager = 
certificate.getCertificateKeyManager();
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..15edc39b65 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,10 @@ 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);
-            }
-        }
+        // The configured chain file is only used if the certificate file holds
+        // the leaf certificate alone. If the certificate file already contains
+        // the intermediate certificates, the chain file is ignored.
+        chain = appendChainFile(certificate.getCertificateChainFile(), 
hostName, certFile, chain);

          return new CertificateEntry(leaf, key, chain);
      }
@@ -419,8 +410,16 @@ 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) {
+                // The configured chain file is only used if the bundle holds
+                // the leaf certificate alone. If the bundle already contains
+                // the intermediate certificates, the chain file is ignored.
+                entry = new CertificateEntry(entry.leaf, entry.key, 
appendChainFile(
+                        certificate.getCertificateChainFile(), hostName, 
certFile, 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 +605,38 @@ public final class CertificateLoader {

      // ------------------------------------- File and native reading helpers

+    /**
+     * Applies the configured certificate chain file. The chain file is only 
used if the certificate source (a PEM
+     * certificate file or a PKCS#12 bundle) holds the leaf certificate alone. 
If the source already provides
+     * intermediate certificates, the chain file is ignored. 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 source     The certificate source, used for logging
+     * @param chain      The intermediate certificates (owned by the caller) 
read from the certificate source
+     *
+     * @return the chain to use, which owns all the native objects
+     */
+    private static MemorySegment[] appendChainFile(String chainFile, String 
hostName, String source,
+            MemorySegment[] chain) {
+        if (chainFile == null) {
+            return chain;
+        }
+        if (chain.length > 0) {
+            if (log.isDebugEnabled()) {
+                log.debug(sm.getString("certificateLoader.chainFileIgnored", 
hostName, chainFile, source));
+            }
+            return chain;
+        }
+        byte[] chainBytes = readConfigFileBytes(chainFile);
+        if (chainBytes == null) {
+            log.warn(sm.getString("certificateLoader.certificateLoadFailed", 
hostName, chainFile));
+            return chain;
+        }
+        return readX509ChainFromBytes(chainBytes);
+    }
+
+
      /**
       * 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.
@@ -893,8 +924,9 @@ public final class CertificateLoader {
          CertificateFactory factory = CertificateFactory.getInstance("X.509");
          addX509Certificates(chain, factory, fileBytes);

+        // Only used if the certificate file holds the leaf certificate alone
          String chainFile = certificate.getCertificateChainFile();
-        if (chainFile != null) {
+        if (chainFile != null && chain.size() <= 1) {
              byte[] chainBytes = readConfigFileBytes(chainFile);
              if (chainBytes != null) {
                  addX509Certificates(chain, factory, chainBytes);
@@ -941,8 +973,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());
+        // Only used if the bundle holds the leaf certificate alone
+        String chainFile = certificate.getCertificateChainFile();
+        if (bundleChain == null || bundleChain.length > 1 || chainFile == 
null) {
+            return bundleChain;
+        }
+        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/java/org/apache/tomcat/util/net/openssl/panama/LocalStrings.properties 
b/java/org/apache/tomcat/util/net/openssl/panama/LocalStrings.properties
index 77a6c27697..ea2ebdab5e 100644
--- a/java/org/apache/tomcat/util/net/openssl/panama/LocalStrings.properties
+++ b/java/org/apache/tomcat/util/net/openssl/panama/LocalStrings.properties
@@ -17,6 +17,7 @@ aprListener.usingFIPSProvider=Using OpenSSL with the FIPS 
provider as the defaul

  certificateLoader.badPassword=Bad PKCS#12 password
  certificateLoader.certificateLoadFailed=Certificate for host [{0}] could not 
be loaded: {1}
+certificateLoader.chainFileIgnored=Certificate for host [{0}]: the certificate 
chain file [{1}] is ignored because [{2}] already contains the certificate chain
  certificateLoader.chainReadError=Error reading the certificate chain file: {0}
  certificateLoader.noKeyEntry=No usable key entry in key store
  certificateLoader.pskFindSessionError=Error in the pre-shared key 
find-session callback; the offered identity is rejected
diff --git a/test/org/apache/tomcat/util/net/TestSSLHostConfigCompat.java 
b/test/org/apache/tomcat/util/net/TestSSLHostConfigCompat.java
index c223dc3af9..f01252d52b 100644
--- a/test/org/apache/tomcat/util/net/TestSSLHostConfigCompat.java
+++ b/test/org/apache/tomcat/util/net/TestSSLHostConfigCompat.java
@@ -17,10 +17,20 @@
  package org.apache.tomcat.util.net;

  import java.io.File;
+import java.io.FileInputStream;
+import java.io.InputStream;
+import java.security.cert.Certificate;
+import java.security.cert.CertificateFactory;
+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 +271,147 @@ public class TestSSLHostConfigCompat extends 
TomcatBaseTest {
      }


+    /*
+     * The certificate chain file is only used if the certificate file (a PEM 
file or a PKCS#12 bundle) holds the
+     * server certificate alone. If it already holds the certificate chain, 
the chain file is ignored.
+     *
+     * The chain file used in these tests holds the certificate of a different 
CA to the one in the certificate chain
+     * so it is possible to tell which of the two the server sends after the 
server certificate.
+     *
+     * This applies to PEM files with all of the TLS implementations. Only the 
OpenSSL FFM implementation reads PKCS#12
+     * bundles through the certificate file.
+     */
+    @Test
+    public void testHostRSAPemLeafOnlyWithChainFile() throws Exception {
+        configureHostRSAChainFileTest(new 
File(TesterSupport.LOCALHOST_RSA_CERT_PEM), false);
+        // The chain file is used
+        assertServerChain(TesterSupport.LOCALHOST_EC_CERT_PEM);
+    }
+
+
+    @Test
+    public void testHostRSAPemWithChainAndChainFile() throws Exception {
+        
configureHostRSAChainFileTest(TesterSupport.createLocalhostRsaCertAndCaPem(), 
false);
+        // The chain file is ignored
+        assertServerChain(TesterSupport.CA_CERT_PEM);
+    }
+
+
+    @Test
+    public void testHostRSAPkcs12LeafOnlyWithChainFile() throws Exception {
+        
configureHostRSAChainFileTest(TesterSupport.createLocalhostRsaPkcs12(false), 
true);
+        // The chain file is used
+        assertServerChain(TesterSupport.LOCALHOST_EC_CERT_PEM);
+    }
+
+
+    @Test
+    public void testHostRSAPkcs12WithChainAndChainFile() throws Exception {
+        
configureHostRSAChainFileTest(TesterSupport.createLocalhostRsaPkcs12(true), 
true);
+        // The chain file is ignored
+        assertServerChain(TesterSupport.CA_CERT_PEM);
+    }
+
+
+    /*
+     * The certificate file may also hold the private key and that key may be 
encrypted. That must not affect whether
+     * the certificate file is found to hold the certificate chain. The 
certificate file is the certificate, the
+     * certificate of a CA and the encrypted key.
+     */
+    @Test
+    public void testHostRSAPemWithChainAndEncryptedKeyAndChainFile() throws 
Exception {
+        doTestHostRSAPemWithChainAndEncryptedKeyAndChainFile(false);
+    }
+
+
+    @Test
+    public void 
testHostRSAPemWithChainAndEncryptedKeyAndChainFileWithPasswordFile() throws 
Exception {
+        doTestHostRSAPemWithChainAndEncryptedKeyAndChainFile(true);
+    }
+
+
+    private void doTestHostRSAPemWithChainAndEncryptedKeyAndChainFile(boolean 
passwordFile) throws Exception {
+        Assume.assumeTrue("Only needs to be run once for each implementation", 
storeType == StoreType.PEM);
+
+        SSLHostConfig sslHostConfig = getSSLHostConfig();
+        SSLHostConfigCertificate certificate = new 
SSLHostConfigCertificate(sslHostConfig, Type.RSA);
+        certificate.setCertificateFile(
+                
TesterSupport.createLocalhostRsaCertCaAndEncryptedKeyPem(TesterSupport.JKS_PASS).getAbsolutePath());
+        if (passwordFile) {
+            
certificate.setCertificateKeyPasswordFile(getPath(TesterSupport.JKS_PASS_FILE));
+        } else {
+            certificate.setCertificateKeyPassword(TesterSupport.JKS_PASS);
+        }
+        
certificate.setCertificateChainFile(getPath(TesterSupport.LOCALHOST_EC_CERT_PEM));
+        sslHostConfig.addCertificate(certificate);
+
+        getTomcatInstance().start();
+
+        // The chain file is ignored
+        assertServerChain(TesterSupport.CA_CERT_PEM);
+    }
+
+
+    private void configureHostRSAChainFileTest(File certificateFile, boolean 
pkcs12) throws Exception {
+        Assume.assumeTrue("Only needs to be run once for each implementation", 
storeType == StoreType.PEM);
+        if (pkcs12) {
+            Assume.assumeTrue("Only the OpenSSL FFM implementation reads PKCS#12 
bundles via the certificate file",
+                    
"org.apache.tomcat.util.net.openssl.panama.OpenSSLImplementation".equals(sslImplementationName));
+        }
+
+        SSLHostConfig sslHostConfig = getSSLHostConfig();
+        SSLHostConfigCertificate certificate = new 
SSLHostConfigCertificate(sslHostConfig, Type.RSA);
+        certificate.setCertificateFile(certificateFile.getAbsolutePath());
+        if (!pkcs12) {
+            
certificate.setCertificateKeyFile(getPath(TesterSupport.LOCALHOST_RSA_KEY_PEM));
+        }
+        certificate.setCertificateKeyPassword(TesterSupport.JKS_PASS);
+        
certificate.setCertificateChainFile(getPath(TesterSupport.LOCALHOST_EC_CERT_PEM));
+        sslHostConfig.addCertificate(certificate);
+
+        getTomcatInstance().start();
+    }
+
+
+    /*
+     * Checks the server sends the server certificate followed by the 
certificate in the given file and nothing else.
+     */
+    private void assertServerChain(String expectedChainCertificate) throws 
Exception {
+        X509Certificate expected;
+        try (InputStream is = new FileInputStream(expectedChainCertificate)) {
+            expected = (X509Certificate) 
CertificateFactory.getInstance("X.509").generateCertificate(is);
+        }
+
+        // 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(2, peerCertificates.length);
+            Assert.assertEquals(expected, peerCertificates[1]);
+        }
+    }
+
+
      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..674bef0e7e 100644
--- a/test/org/apache/tomcat/util/net/TesterSupport.java
+++ b/test/org/apache/tomcat/util/net/TesterSupport.java
@@ -18,11 +18,15 @@ 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;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.Files;
  import java.security.KeyStore;
  import java.security.KeyStoreException;
  import java.security.NoSuchAlgorithmException;
@@ -31,16 +35,21 @@ import java.security.PrivateKey;
  import java.security.cert.CertPathValidator;
  import java.security.cert.Certificate;
  import java.security.cert.CertificateException;
+import java.security.cert.CertificateFactory;
  import java.security.cert.PKIXBuilderParameters;
  import java.security.cert.PKIXRevocationChecker;
  import java.security.cert.TrustAnchor;
  import java.security.cert.X509CertSelector;
  import java.security.cert.X509Certificate;
+import java.util.Base64;
  import java.util.EnumSet;
  import java.util.Enumeration;
  import java.util.HashSet;
  import java.util.Set;

+import javax.crypto.Cipher;
+import javax.crypto.SecretKeyFactory;
+import javax.crypto.spec.PBEKeySpec;
  import javax.net.ssl.CertPathTrustManagerParameters;
  import javax.net.ssl.KeyManager;
  import javax.net.ssl.KeyManagerFactory;
@@ -72,6 +81,7 @@ import org.apache.catalina.core.OpenSSLLifecycleListener;
  import org.apache.catalina.core.StandardServer;
  import org.apache.catalina.startup.TesterMapRealm;
  import org.apache.catalina.startup.Tomcat;
+import org.apache.tomcat.util.buf.Asn1Writer;
  import org.apache.tomcat.util.descriptor.web.LoginConfig;
  import org.apache.tomcat.util.descriptor.web.SecurityCollection;
  import org.apache.tomcat.util.descriptor.web.SecurityConstraint;
@@ -292,6 +302,121 @@ 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
+     * its certificate. The bundle is protected with {@link #JKS_PASS}.
+     *
+     * @param includeCa If {@code true} the certificate of the CA that issued 
the certificate is included in the
+     *                      bundle so it holds the certificate chain, 
otherwise the bundle only holds the certificate
+     *
+     * @return the bundle, which is deleted when the JVM exits
+     *
+     * @throws Exception if the bundle could not be created
+     */
+    public static File createLocalhostRsaPkcs12(boolean includeCa) 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];
+
+        Certificate[] chain;
+        if (includeCa) {
+            Certificate ca;
+            try (InputStream is = new FileInputStream(CA_CERT_PEM)) {
+                ca = 
CertificateFactory.getInstance("X.509").generateCertificate(is);
+            }
+            chain = new Certificate[] { leaf, ca };
+        } else {
+            chain = new Certificate[] { leaf };
+        }
+
+        KeyStore pkcs12 = KeyStore.getInstance("PKCS12");
+        pkcs12.load(null, null);
+        pkcs12.setKeyEntry("localhost", key, JKS_PASS.toCharArray(), chain);
+
+        File file = File.createTempFile("tomcat-test-", ".pkcs12");
+        file.deleteOnExit();
+        try (OutputStream os = new FileOutputStream(file)) {
+            pkcs12.store(os, JKS_PASS.toCharArray());
+        }
+        return file;
+    }
+
+
+    /**
+     * Creates a PEM file that holds the localhost RSA certificate followed by 
the certificate of the CA that issued
+     * it, i.e. the certificate and its certificate chain.
+     *
+     * @return the file, which is deleted when the JVM exits
+     *
+     * @throws Exception if the file could not be created
+     */
+    public static File createLocalhostRsaCertAndCaPem() throws Exception {
+        File file = File.createTempFile("tomcat-test-", ".pem");
+        file.deleteOnExit();
+        try (OutputStream os = new FileOutputStream(file)) {
+            os.write(Files.readAllBytes(new 
File(LOCALHOST_RSA_CERT_PEM).toPath()));
+            os.write('\n');
+            os.write(Files.readAllBytes(new File(CA_CERT_PEM).toPath()));
+        }
+        return file;
+    }
+
+    /**
+     * Creates a PEM file that holds the localhost RSA certificate, followed 
by the certificate of the CA that issued
+     * it (i.e. the certificate chain), followed by the localhost RSA private 
key encrypted with the given password.
+     *
+     * @param password The password to encrypt the private key with
+     *
+     * @return the file, which is deleted when the JVM exits
+     *
+     * @throws Exception if the file could not be created
+     */
+    public static File createLocalhostRsaCertCaAndEncryptedKeyPem(String 
password) throws Exception {
+        // The private key file holds an unencrypted PKCS#8 key
+        StringBuilder base64 = new StringBuilder();
+        for (String line : Files.readAllLines(new 
File(LOCALHOST_RSA_KEY_PEM).toPath())) {
+            if (!line.startsWith("-----")) {
+                base64.append(line.trim());
+            }
+        }
+        byte[] pkcs8 = Base64.getDecoder().decode(base64.toString());
+
+        String algorithm = "PBEWithHmacSHA256AndAES_128";
+        Cipher cipher = Cipher.getInstance(algorithm);
+        cipher.init(Cipher.ENCRYPT_MODE,
+                SecretKeyFactory.getInstance(algorithm).generateSecret(new 
PBEKeySpec(password.toCharArray())));
+        byte[] encrypted = cipher.doFinal(pkcs8);
+        // EncryptedPrivateKeyInfo with the PBES2 algorithm identifier 
(1.2.840.113549.1.5.13) and the parameters from
+        // the cipher. The JRE class used for this can't be used as it doesn't 
recognise the algorithm name.
+        byte[] pbes2Oid = new byte[] { 0x06, 0x09, 0x2A, (byte) 0x86, 0x48, 
(byte) 0x86, (byte) 0xF7, 0x0D, 0x01, 0x05,
+                0x0D };
+        byte[] algorithmIdentifier = Asn1Writer.writeSequence(pbes2Oid, 
cipher.getParameters().getEncoded());
+        byte[] encryptedKeyInfo =
+                Asn1Writer.writeSequence(algorithmIdentifier, 
Asn1Writer.writeOctetString(encrypted));
+
+        File file = File.createTempFile("tomcat-test-", ".pem");
+        file.deleteOnExit();
+        try (OutputStream os = new FileOutputStream(file)) {
+            os.write(Files.readAllBytes(new 
File(LOCALHOST_RSA_CERT_PEM).toPath()));
+            os.write('\n');
+            os.write(Files.readAllBytes(new File(CA_CERT_PEM).toPath()));
+            os.write('\n');
+            os.write("-----BEGIN ENCRYPTED PRIVATE 
KEY-----\n".getBytes(StandardCharsets.US_ASCII));
+            os.write(Base64.getMimeEncoder(64, new byte[] { '\n' 
}).encode(encryptedKeyInfo));
+            os.write("\n-----END ENCRYPTED PRIVATE 
KEY-----\n".getBytes(StandardCharsets.US_ASCII));
+        }
+        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..3b5ac8dd57
--- /dev/null
+++ b/test/org/apache/tomcat/util/net/openssl/panama/TestCertificateLoader.java
@@ -0,0 +1,139 @@
+/*
+ * 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.
+ *
+ * The certificate chain file is only used if the certificate file (a PEM file 
or a PKCS#12 bundle) holds the server
+ * certificate alone. If it already holds the certificate chain, the chain 
file is ignored. The CA certificate is used
+ * for the chain file in all of these tests so it would be in the chain twice 
if the chain file was not ignored.
+ */
+public class TestCertificateLoader {
+
+    private static SSLHostConfigCertificate createCertificate(File 
certificateFile, String chainFile) {
+        SSLHostConfig sslHostConfig = new SSLHostConfig();
+        SSLHostConfigCertificate certificate = new 
SSLHostConfigCertificate(sslHostConfig, Type.RSA);
+        certificate.setCertificateFile(certificateFile.getAbsolutePath());
+        certificate.setCertificateKeyPassword(TesterSupport.JKS_PASS);
+        if (chainFile != null) {
+            certificate.setCertificateChainFile(new 
File(chainFile).getAbsolutePath());
+        }
+        return certificate;
+    }
+
+
+    private static void assertChainOfLeafAndCa(X509Certificate[] chain) {
+        Assert.assertNotNull(chain);
+        Assert.assertEquals(2, chain.length);
+        Assert.assertEquals(chain[0].getIssuerX500Principal(), 
chain[1].getSubjectX500Principal());
+    }
+
+
+    @Test
+    public void testPemLeafOnly() {
+        X509Certificate[] chain = CertificateLoader.getCertificateChain(
+                createCertificate(new 
File(TesterSupport.LOCALHOST_RSA_CERT_PEM), null));
+
+        Assert.assertNotNull(chain);
+        Assert.assertEquals(1, chain.length);
+    }
+
+
+    @Test
+    public void testPemLeafOnlyWithChainFile() {
+        X509Certificate[] chain = CertificateLoader.getCertificateChain(
+                createCertificate(new 
File(TesterSupport.LOCALHOST_RSA_CERT_PEM), TesterSupport.CA_CERT_PEM));
+
+        // The chain file is used
+        assertChainOfLeafAndCa(chain);
+    }
+
+
+    @Test
+    public void testPemWithChainAndChainFile() throws Exception {
+        X509Certificate[] chain = CertificateLoader.getCertificateChain(
+                
createCertificate(TesterSupport.createLocalhostRsaCertAndCaPem(), 
TesterSupport.CA_CERT_PEM));
+
+        // The chain file is ignored
+        assertChainOfLeafAndCa(chain);
+    }
+
+
+    @Test
+    public void testPemLeafOnlyWithMissingChainFile() {
+        X509Certificate[] chain = 
CertificateLoader.getCertificateChain(createCertificate(
+                new File(TesterSupport.LOCALHOST_RSA_CERT_PEM), 
"test/org/apache/tomcat/util/net/missing.pem"));
+
+        // A chain file that cannot be read is ignored
+        Assert.assertNotNull(chain);
+        Assert.assertEquals(1, chain.length);
+    }
+
+
+    @Test
+    public void testPkcs12LeafOnly() throws Exception {
+        X509Certificate[] chain = CertificateLoader.getCertificateChain(
+                
createCertificate(TesterSupport.createLocalhostRsaPkcs12(false), null));
+
+        Assert.assertNotNull(chain);
+        Assert.assertEquals(1, chain.length);
+    }
+
+
+    @Test
+    public void testPkcs12LeafOnlyWithChainFile() throws Exception {
+        X509Certificate[] chain = CertificateLoader.getCertificateChain(
+                
createCertificate(TesterSupport.createLocalhostRsaPkcs12(false), 
TesterSupport.CA_CERT_PEM));
+
+        // The chain file is used
+        assertChainOfLeafAndCa(chain);
+    }
+
+
+    @Test
+    public void testPkcs12WithChainAndChainFile() throws Exception {
+        X509Certificate[] chain = CertificateLoader.getCertificateChain(
+                
createCertificate(TesterSupport.createLocalhostRsaPkcs12(true), 
TesterSupport.CA_CERT_PEM));
+
+        // The chain file is ignored
+        assertChainOfLeafAndCa(chain);
+    }
+
+
+    @Test
+    public void testPkcs12LeafOnlyWithMissingChainFile() throws Exception {
+        X509Certificate[] chain = 
CertificateLoader.getCertificateChain(createCertificate(
+                TesterSupport.createLocalhostRsaPkcs12(false), 
"test/org/apache/tomcat/util/net/missing.pem"));
+
+        // A chain file that cannot be read is ignored
+        Assert.assertNotNull(chain);
+        Assert.assertEquals(1, chain.length);
+    }
+}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 81f6f39356..7c0def85ae 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -495,6 +495,15 @@
    </subsection>
    <subsection name="Coyote">
      <changelog>
+      <fix>
+        Use the <code>certificateChainFile</code> only if the PEM certificate
+        file (all TLS implementations) or the PKCS#12 bundle (OpenSSL FFM
+        implementation) contains the server certificate alone. If it already
+        contains the certificate chain, the <code>certificateChainFile</code>
+        is ignored. The OpenSSL FFM implementation now sends the intermediate
+        certificates of a PEM certificate file or a PKCS#12 bundle to the
+        client. (markt)
+      </fix>
        <update>
          Change the default value of the <code>cookiesWithoutEquals</code>
          attribute of the <code>Rfc6265CookieProcessor</code> from
diff --git a/webapps/docs/config/http.xml b/webapps/docs/config/http.xml
index c87099b587..f040ecf048 100644
--- a/webapps/docs/config/http.xml
+++ b/webapps/docs/config/http.xml
@@ -1646,6 +1646,16 @@
        generated by <code>openssl dhparam</code> and <code>openssl 
ecparam</code>,
        respectively. The output of the respective OpenSSL command can simply
        be concatenated to the certificate file.</p>
+      <p>The file may also contain the certificate chain, that is the
+      intermediate certificates, after the server certificate. If it does, the
+      <strong>certificateChainFile</strong> is ignored.</p>
+      <p>When using the OpenSSL FFM implementation, a file with a name that 
ends
+      with <code>.pkcs12</code> is read as a PKCS#12 bundle containing the
+      server key and certificate. The password of the bundle is the
+      <strong>certificateKeyPassword</strong> or, if that is not set, the
+      <strong>certificateKeystorePassword</strong> (or the equivalent password
+      files). The bundle may also contain the certificate chain. If it does, 
the
+      <strong>certificateChainFile</strong> is ignored.</p>
        <p>This attribute is required unless
        <strong>certificateKeystoreFile</strong> is specified.</p>
      </attribute>
@@ -1657,6 +1667,10 @@
        <code>$CATALINA_BASE</code>.</p>
        <p>The certificate chain used for Tomcat should not include the server
        certificate as its first element.</p>
+      <p>The chain file is only used if the
+      <strong>certificateFile</strong> (a PEM file or a PKCS#12 bundle) 
contains
+      the server certificate alone. If the <strong>certificateFile</strong>
+      already contains the certificate chain, the chain file is ignored.</p>
        <p>Note that when using more than one certificate for different types,
        they all must use the same certificate chain.</p>
      </attribute>
diff --git a/webapps/docs/ssl-howto.xml b/webapps/docs/ssl-howto.xml
index 8eee9f1210..147e4559be 100644
--- a/webapps/docs/ssl-howto.xml
+++ b/webapps/docs/ssl-howto.xml
@@ -350,6 +350,12 @@ so it looks something like this:</p>
          />
    </SSLHostConfig>
  </Connector>]]></source>
+<p>The <code>certificateChainFile</code> is only used if the
+<code>certificateFile</code> contains the server certificate alone. If the
+<code>certificateFile</code> already contains the intermediate certificates
+after the server certificate, the <code>certificateChainFile</code> is ignored.
+The same applies when the OpenSSL FFM implementation reads the certificate from
+a PKCS#12 bundle.</p>


  <p>The configuration options and information on which attributes


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to