ZOOKEEPER-4992: Avoid overriding same-subject certs in PEM trust store

Signed-off-by: Tero Saarni <tero.saarni@est.tech>

Author: tsaarni

zookeeper-server/src/main/java/org/apache/zookeeper/util/PemReader.java

@@ -92,9 +92,10 @@ public static KeyStore loadTrustStore(File certificateChainFile) throws IOExcept
         keyStore.load(null, null);
 
         List<X509Certificate> certificateChain = readCertificateChain(certificateChainFile);
+        int i = 1;
         for (X509Certificate certificate : certificateChain) {
             X500Principal principal = certificate.getSubjectX500Principal();
-            keyStore.setCertificateEntry(principal.getName("RFC2253"), certificate);
+            keyStore.setCertificateEntry(principal.getName("RFC2253") + "-" + i++, certificate);
         }
         return keyStore;
     }

zookeeper-server/src/test/java/org/apache/zookeeper/common/PEMFileLoaderTest.java

@@ -108,7 +108,7 @@ public void testLoadKeyStoreWithWrongFileType(
 
     @ParameterizedTest
     @MethodSource("data")
-    public void testLoadTrustStore(
+    public void testLoadTrustStoreFromPemBundle(
             X509KeyType caKeyType, X509KeyType certKeyType, String keyPassword, Integer paramIndex)
             throws Exception {
         init(caKeyType, certKeyType, keyPassword, paramIndex);
@@ -118,7 +118,7 @@ public void testLoadTrustStore(
             .setTrustStorePassword(x509TestContext.getTrustStorePassword())
             .build()
             .loadTrustStore();
-        assertEquals(1, ts.size());
+        assertEquals(2, ts.size());
     }
 
     @ParameterizedTest

zookeeper-server/src/test/java/org/apache/zookeeper/common/X509TestContext.java

@@ -29,6 +29,8 @@
 import java.security.Security;
 import java.security.cert.X509Certificate;
 import java.util.Arrays;
+import java.util.List;
+
 import org.apache.commons.io.FileUtils;
 import org.bouncycastle.asn1.x500.X500NameBuilder;
 import org.bouncycastle.asn1.x500.style.BCStyle;
@@ -48,7 +50,7 @@ public class X509TestContext {
     private final X509KeyType trustStoreKeyType;
     private final KeyPair trustStoreKeyPair;
     private final long trustStoreCertExpirationMillis;
-    private final X509Certificate trustStoreCertificate;
+    private final List<X509Certificate> trustStoreCertificates;
     private final String trustStorePassword;
     private File trustStoreJksFile;
     private File trustStorePemFile;
@@ -99,11 +101,18 @@ private X509TestContext(File tempDir, KeyPair trustStoreKeyPair, long trustStore
 
         X500NameBuilder caNameBuilder = new X500NameBuilder(BCStyle.INSTANCE);
         caNameBuilder.addRDN(BCStyle.CN, MethodHandles.lookup().lookupClass().getCanonicalName() + " Root CA");
-        trustStoreCertificate = X509TestHelpers.newSelfSignedCACert(caNameBuilder.build(), trustStoreKeyPair, trustStoreCertExpirationMillis);
+        // Create two CA certs to test multiple certs in PEM bundles.
+        // Use same subject name to simulate multiple CA certs from the same CA.
+        trustStoreCertificates = Arrays.asList(
+                X509TestHelpers.newSelfSignedCACert(caNameBuilder.build(), trustStoreKeyPair,
+                        trustStoreCertExpirationMillis),
+                X509TestHelpers.newSelfSignedCACert(caNameBuilder.build(), trustStoreKeyPair,
+                        trustStoreCertExpirationMillis)
+        );
 
         X500NameBuilder nameBuilder = new X500NameBuilder(BCStyle.INSTANCE);
         nameBuilder.addRDN(BCStyle.CN, MethodHandles.lookup().lookupClass().getCanonicalName() + " Zookeeper Test");
-        keyStoreCertificate = X509TestHelpers.newCert(trustStoreCertificate, trustStoreKeyPair, nameBuilder.build(), keyStoreKeyPair.getPublic(), keyStoreCertExpirationMillis);
+        keyStoreCertificate = X509TestHelpers.newCert(trustStoreCertificates.get(0), trustStoreKeyPair, nameBuilder.build(), keyStoreKeyPair.getPublic(), keyStoreCertExpirationMillis);
         trustStorePkcs12File = trustStorePemFile = trustStoreJksFile = null;
         keyStorePkcs12File = keyStorePemFile = keyStoreJksFile = null;
 
@@ -139,8 +148,8 @@ public long getTrustStoreCertExpirationMillis() {
         return trustStoreCertExpirationMillis;
     }
 
-    public X509Certificate getTrustStoreCertificate() {
-        return trustStoreCertificate;
+    public X509Certificate getTrustStoreCertificates() {
+        return trustStoreCertificates.get(0);
     }
 
     public String getTrustStorePassword() {
@@ -159,7 +168,7 @@ public File getTrustStoreFile(KeyStoreFileType storeFileType) throws IOException
         case JKS:
             return getTrustStoreJksFile();
         case PEM:
-            return getTrustStorePemFile();
+            return getTrustStorePemBundleFile();
         case PKCS12:
             return getTrustStorePkcs12File();
         case BCFKS:
@@ -177,7 +186,7 @@ private File getTrustStoreJksFile() throws IOException {
             File trustStoreJksFile = File.createTempFile(TRUST_STORE_PREFIX, KeyStoreFileType.JKS.getDefaultFileExtension(), tempDir);
             trustStoreJksFile.deleteOnExit();
             try (final FileOutputStream trustStoreOutputStream = new FileOutputStream(trustStoreJksFile)) {
-                byte[] bytes = X509TestHelpers.certToJavaTrustStoreBytes(trustStoreCertificate, trustStorePassword);
+                byte[] bytes = X509TestHelpers.certToJavaTrustStoreBytes(trustStoreCertificates.get(0), trustStorePassword);
                 trustStoreOutputStream.write(bytes);
                 trustStoreOutputStream.flush();
             } catch (GeneralSecurityException e) {
@@ -188,11 +197,13 @@ private File getTrustStoreJksFile() throws IOException {
         return trustStoreJksFile;
     }
 
-    private File getTrustStorePemFile() throws IOException {
+    private File getTrustStorePemBundleFile() throws IOException {
         if (trustStorePemFile == null) {
             File trustStorePemFile = File.createTempFile(TRUST_STORE_PREFIX, KeyStoreFileType.PEM.getDefaultFileExtension(), tempDir);
             trustStorePemFile.deleteOnExit();
-            FileUtils.writeStringToFile(trustStorePemFile, X509TestHelpers.pemEncodeX509Certificate(trustStoreCertificate), StandardCharsets.US_ASCII, false);
+            for (X509Certificate cert : trustStoreCertificates) {
+                FileUtils.writeStringToFile(trustStorePemFile, X509TestHelpers.pemEncodeX509Certificate(cert), StandardCharsets.US_ASCII, true);
+            }
             this.trustStorePemFile = trustStorePemFile;
         }
         return trustStorePemFile;
@@ -203,7 +214,7 @@ private File getTrustStorePkcs12File() throws IOException {
             File trustStorePkcs12File = File.createTempFile(TRUST_STORE_PREFIX, KeyStoreFileType.PKCS12.getDefaultFileExtension(), tempDir);
             trustStorePkcs12File.deleteOnExit();
             try (final FileOutputStream trustStoreOutputStream = new FileOutputStream(trustStorePkcs12File)) {
-                byte[] bytes = X509TestHelpers.certToPKCS12TrustStoreBytes(trustStoreCertificate, trustStorePassword);
+                byte[] bytes = X509TestHelpers.certToPKCS12TrustStoreBytes(trustStoreCertificates.get(0), trustStorePassword);
                 trustStoreOutputStream.write(bytes);
                 trustStoreOutputStream.flush();
             } catch (GeneralSecurityException e) {
@@ -220,7 +231,7 @@ private File getTrustStoreBcfksFile() throws IOException {
                 TRUST_STORE_PREFIX, KeyStoreFileType.BCFKS.getDefaultFileExtension(), tempDir);
             trustStoreBcfksFile.deleteOnExit();
             try (final FileOutputStream trustStoreOutputStream = new FileOutputStream(trustStoreBcfksFile)) {
-                byte[] bytes = X509TestHelpers.certToBCFKSTrustStoreBytes(trustStoreCertificate, trustStorePassword);
+                byte[] bytes = X509TestHelpers.certToBCFKSTrustStoreBytes(trustStoreCertificates.get(0), trustStorePassword);
                 trustStoreOutputStream.write(bytes);
                 trustStoreOutputStream.flush();
             } catch (GeneralSecurityException e) {

zookeeper-server/src/test/java/org/apache/zookeeper/util/PemReaderTest.java

@@ -135,8 +135,8 @@ public void testLoadCertificateFromTrustStore(
             throws Exception {
         init(caKeyType, certKeyType, keyPassword, paramIndex);
         List<X509Certificate> certs = PemReader.readCertificateChain(x509TestContext.getTrustStoreFile(KeyStoreFileType.PEM));
-        assertEquals(1, certs.size());
-        assertEquals(x509TestContext.getTrustStoreCertificate(), certs.get(0));
+        assertEquals(2, certs.size());
+        assertEquals(x509TestContext.getTrustStoreCertificates(), certs.get(0));
     }
 
 }