This is an automated email from the ASF dual-hosted git repository.
anmolnar pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/zookeeper.git
The following commit(s) were added to refs/heads/master by this push:
new bbaaf0b17c ZOOKEEPER-4992: Avoid overriding same-subject certs in PEM
trust store
bbaaf0b17c is described below
commit bbaaf0b17caac173c319742817a69f3242ce182a
Author: Tero Saarni <[email protected]>
AuthorDate: Thu Apr 23 02:16:54 2026 +0300
ZOOKEEPER-4992: Avoid overriding same-subject certs in PEM trust store
Reviewers: anmolnar
Author: tsaarni
Closes #2336 from tsaarni/fix-zookeeper-4992
---
.../java/org/apache/zookeeper/util/PemReader.java | 10 +++++--
.../apache/zookeeper/common/PEMFileLoaderTest.java | 7 +++--
.../apache/zookeeper/common/X509TestContext.java | 32 ++++++++++++++--------
.../org/apache/zookeeper/util/PemReaderTest.java | 3 +-
4 files changed, 34 insertions(+), 18 deletions(-)
diff --git
a/zookeeper-server/src/main/java/org/apache/zookeeper/util/PemReader.java
b/zookeeper-server/src/main/java/org/apache/zookeeper/util/PemReader.java
index dd5a157640..20e4adc6ea 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/util/PemReader.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/util/PemReader.java
@@ -49,7 +49,6 @@
import javax.crypto.SecretKey;
import javax.crypto.SecretKeyFactory;
import javax.crypto.spec.PBEKeySpec;
-import javax.security.auth.x500.X500Principal;
/**
* Note: this class is copied from io.airlift.security.pem.PemReader (see
@@ -93,8 +92,13 @@ public static KeyStore loadTrustStore(File
certificateChainFile) throws IOExcept
List<X509Certificate> certificateChain =
readCertificateChain(certificateChainFile);
for (X509Certificate certificate : certificateChain) {
- X500Principal principal = certificate.getSubjectX500Principal();
- keyStore.setCertificateEntry(principal.getName("RFC2253"),
certificate);
+ String subject =
certificate.getSubjectX500Principal().getName("RFC2253");
+ String alias = subject;
+ // Append a suffix alias-1, alias-2 ... if the same alias
(subject) already exists.
+ for (int i = 1; keyStore.containsAlias(alias); i++) {
+ alias = subject + "-" + i;
+ }
+ keyStore.setCertificateEntry(alias, certificate);
}
return keyStore;
}
diff --git
a/zookeeper-server/src/test/java/org/apache/zookeeper/common/PEMFileLoaderTest.java
b/zookeeper-server/src/test/java/org/apache/zookeeper/common/PEMFileLoaderTest.java
index ee9a68eb7a..8abe383a06 100644
---
a/zookeeper-server/src/test/java/org/apache/zookeeper/common/PEMFileLoaderTest.java
+++
b/zookeeper-server/src/test/java/org/apache/zookeeper/common/PEMFileLoaderTest.java
@@ -20,6 +20,7 @@
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
import java.io.IOException;
import java.security.KeyStore;
import java.security.KeyStoreException;
@@ -108,7 +109,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 +119,9 @@ public void testLoadTrustStore(
.setTrustStorePassword(x509TestContext.getTrustStorePassword())
.build()
.loadTrustStore();
- assertEquals(1, ts.size());
+ assertEquals(2, ts.size());
+
assertTrue(ts.containsAlias("cn=org.apache.zookeeper.common.x509testcontext
root ca"));
+
assertTrue(ts.containsAlias("cn=org.apache.zookeeper.common.x509testcontext
root ca-1"));
}
@ParameterizedTest
diff --git
a/zookeeper-server/src/test/java/org/apache/zookeeper/common/X509TestContext.java
b/zookeeper-server/src/test/java/org/apache/zookeeper/common/X509TestContext.java
index 2bb759e639..80632e45c0 100644
---
a/zookeeper-server/src/test/java/org/apache/zookeeper/common/X509TestContext.java
+++
b/zookeeper-server/src/test/java/org/apache/zookeeper/common/X509TestContext.java
@@ -29,6 +29,7 @@
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 +49,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 +100,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 (e.g. reissued cert with different key type).
+ 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 +147,8 @@ public long getTrustStoreCertExpirationMillis() {
return trustStoreCertExpirationMillis;
}
- public X509Certificate getTrustStoreCertificate() {
- return trustStoreCertificate;
+ public List<X509Certificate> getTrustStoreCertificates() {
+ return trustStoreCertificates;
}
public String getTrustStorePassword() {
@@ -159,7 +167,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 +185,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 +196,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 +213,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 +230,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) {
diff --git
a/zookeeper-server/src/test/java/org/apache/zookeeper/util/PemReaderTest.java
b/zookeeper-server/src/test/java/org/apache/zookeeper/util/PemReaderTest.java
index d584c28da9..30d9241e7a 100644
---
a/zookeeper-server/src/test/java/org/apache/zookeeper/util/PemReaderTest.java
+++
b/zookeeper-server/src/test/java/org/apache/zookeeper/util/PemReaderTest.java
@@ -135,8 +135,7 @@ 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(x509TestContext.getTrustStoreCertificates(), certs);
}
}
