This is an automated email from the ASF dual-hosted git repository.
andor 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 770804bef ZOOKEEPER-4955: Fix intererence with jvm wide ssl properties
for ssl.crl and ssl.ocsp
770804bef is described below
commit 770804bef861bbfc9e150b63774f8557f1f8d995
Author: Kezhu Wang <[email protected]>
AuthorDate: Thu Aug 14 08:24:38 2025 +0800
ZOOKEEPER-4955: Fix intererence with jvm wide ssl properties for ssl.crl
and ssl.ocsp
ZOOKEEPER-4955: Fix intererence with jvm wide ssl properties for ssl.crl
and ssl.ocsp
Refs: ZOOKEEPER-4955, apache/zookeeper#2289, stoty/zookeeper#2
Add more comments about PKIXRevocationChecker options
Rename test methods to reflect where revocation happens
Refactor tests to reflect that revocation checking affects is only enforced
in enabled side
Fix missing fallback property for ssl.ocsp
Reviewers: anmolnar, stoty
Author: kezhuw
Closes #2292 from kezhuw/ZOOKEEPER-4955-fix-interference-with-jvm-properties
---
.../src/main/resources/markdown/zookeeperAdmin.md | 8 +-
.../apache/zookeeper/common/ClientX509Util.java | 7 +-
.../java/org/apache/zookeeper/common/X509Util.java | 45 +-
.../server/auth/X509AuthenticationProvider.java | 5 +-
.../apache/zookeeper/common/X509TestContext.java | 4 +-
.../apache/zookeeper/common/X509TestHelpers.java | 42 +-
.../org/apache/zookeeper/common/X509UtilTest.java | 49 --
.../zookeeper/server/ClientSSLRevocationTest.java | 610 +++++++++++++++++++++
.../zookeeper/server/quorum/QuorumSSLTest.java | 1 +
9 files changed, 703 insertions(+), 68 deletions(-)
diff --git a/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md
b/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md
index a4fccf1f9..5bac829cf 100644
--- a/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md
+++ b/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md
@@ -1768,13 +1768,17 @@ and [SASL authentication for
ZooKeeper](https://cwiki.apache.org/confluence/disp
(Java system properties: **zookeeper.ssl.crl** and
**zookeeper.ssl.quorum.crl**)
**New in 3.5.5:**
Specifies whether Certificate Revocation List is enabled in client and
quorum TLS protocols.
- Default: false
+ Default: jvm property "com.sun.net.ssl.checkRevocation" since 3.10.0,
false otherwise
* *ssl.ocsp* and *ssl.quorum.ocsp* :
(Java system properties: **zookeeper.ssl.ocsp** and
**zookeeper.ssl.quorum.ocsp**)
**New in 3.5.5:**
Specifies whether Online Certificate Status Protocol is enabled in client
and quorum TLS protocols.
- Default: false
+ **Changed in 3.10.0:**
+ Before 3.10.0, *ssl.ocsp* and *ssl.quorum.ocsp* implies *ssl.crl* and
*ssl.quorum.crl* correspondingly.
+ After 3.10.0, one has to setup both *ssl.crl* and *ssl.ocsp* (or
*ssl.quorum.crl* and *ssl.quorum.ocsp*)
+ to enable OCSP. This is consistent with jdk's method of [Setting up a Java
Client to use Client-Driven
OCSP](https://docs.oracle.com/javase/8/docs/technotes/guides/security/jsse/ocsp.html#setting-up-a-java-client-to-use-client-driven-ocsp).
+ Default: jvm security property "ocsp.enable" since 3.10.0, false otherwise
* *ssl.clientAuth* and *ssl.quorum.clientAuth* :
(Java system properties: **zookeeper.ssl.clientAuth** and
**zookeeper.ssl.quorum.clientAuth**)
diff --git
a/zookeeper-server/src/main/java/org/apache/zookeeper/common/ClientX509Util.java
b/zookeeper-server/src/main/java/org/apache/zookeeper/common/ClientX509Util.java
index 6034b9e94..16d7f27a5 100644
---
a/zookeeper-server/src/main/java/org/apache/zookeeper/common/ClientX509Util.java
+++
b/zookeeper-server/src/main/java/org/apache/zookeeper/common/ClientX509Util.java
@@ -23,6 +23,7 @@
import io.netty.handler.ssl.SslContext;
import io.netty.handler.ssl.SslContextBuilder;
import io.netty.handler.ssl.SslProvider;
+import java.security.Security;
import java.util.Arrays;
import javax.net.ssl.KeyManager;
import javax.net.ssl.SSLEngine;
@@ -148,7 +149,7 @@ public SslContext createNettySslContextForServer(ZKConfig
config, KeyManager key
private SslContextBuilder handleTcnativeOcspStapling(SslContextBuilder
builder, ZKConfig config) {
SslProvider sslProvider = getSslProvider(config);
boolean tcnative = sslProvider == SslProvider.OPENSSL || sslProvider
== SslProvider.OPENSSL_REFCNT;
- boolean ocspEnabled = config.getBoolean(getSslOcspEnabledProperty());
+ boolean ocspEnabled = config.getBoolean(getSslOcspEnabledProperty(),
Boolean.parseBoolean(Security.getProperty("ocsp.enable")));
if (tcnative && ocspEnabled && OpenSsl.isOcspSupported()) {
builder.enableOcsp(ocspEnabled);
@@ -201,8 +202,8 @@ private TrustManager getTrustManager(ZKConfig config)
throws X509Exception.Trust
getSslTruststorePasswdPathProperty());
String trustStoreType =
config.getProperty(getSslTruststoreTypeProperty());
- boolean sslCrlEnabled = config.getBoolean(getSslCrlEnabledProperty());
- boolean sslOcspEnabled =
config.getBoolean(getSslOcspEnabledProperty());
+ boolean sslCrlEnabled = config.getBoolean(getSslCrlEnabledProperty(),
Boolean.getBoolean("com.sun.net.ssl.checkRevocation"));
+ boolean sslOcspEnabled =
config.getBoolean(getSslOcspEnabledProperty(),
Boolean.parseBoolean(Security.getProperty("ocsp.enable")));
boolean sslServerHostnameVerificationEnabled =
isServerHostnameVerificationEnabled(config);
boolean sslClientHostnameVerificationEnabled =
isClientHostnameVerificationEnabled(config);
diff --git
a/zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java
b/zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java
index 153a826ba..2584fecd8 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java
@@ -31,11 +31,15 @@
import java.security.KeyStore;
import java.security.NoSuchAlgorithmException;
import java.security.Security;
+import java.security.cert.CertPathValidator;
import java.security.cert.PKIXBuilderParameters;
+import java.security.cert.PKIXRevocationChecker;
import java.security.cert.X509CertSelector;
import java.util.ArrayList;
import java.util.Arrays;
+import java.util.HashSet;
import java.util.List;
+import java.util.Set;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Supplier;
import javax.net.ssl.CertPathTrustManagerParameters;
@@ -392,8 +396,9 @@ public SSLContextAndOptions
createSSLContextAndOptionsFromConfig(ZKConfig config
String trustStorePasswordProp =
getPasswordFromConfigPropertyOrFile(config, sslTruststorePasswdProperty,
sslTruststorePasswdPathProperty);
String trustStoreTypeProp =
config.getProperty(sslTruststoreTypeProperty);
- boolean sslCrlEnabled = config.getBoolean(this.sslCrlEnabledProperty);
- boolean sslOcspEnabled =
config.getBoolean(this.sslOcspEnabledProperty);
+ boolean sslCrlEnabled = config.getBoolean(this.sslCrlEnabledProperty,
Boolean.getBoolean("com.sun.net.ssl.checkRevocation"));
+ boolean sslOcspEnabled =
config.getBoolean(this.sslOcspEnabledProperty,
Boolean.parseBoolean(Security.getProperty("ocsp.enable")));
+
boolean sslServerHostnameVerificationEnabled =
isServerHostnameVerificationEnabled(config);
boolean sslClientHostnameVerificationEnabled =
isClientHostnameVerificationEnabled(config);
boolean fipsMode = getFipsMode(config);
@@ -548,13 +553,37 @@ public static X509TrustManager createTrustManager(
try {
KeyStore ts = loadTrustStore(trustStoreLocation,
trustStorePassword, trustStoreTypeProp);
PKIXBuilderParameters pbParams = new PKIXBuilderParameters(ts, new
X509CertSelector());
- if (crlEnabled || ocspEnabled) {
- pbParams.setRevocationEnabled(true);
- System.setProperty("com.sun.net.ssl.checkRevocation", "true");
- System.setProperty("com.sun.security.enableCRLDP", "true");
- if (ocspEnabled) {
- Security.setProperty("ocsp.enable", "true");
+ if (crlEnabled) {
+ // See [RevocationChecker][1] for details. Basically, we are
mimicking the legacy path,
+ // which relies significantly on jvm wide properties[2], as
that is the path we are routing
+ // before (i.e. no explicit `PKIXRevocationChecker`).
+ //
+ // By reading but not writing these properties, we conform to
but not interfere with what
+ // admin set while still keep backward compatibility.
+ // 1. Default "zookeeper.ssl.crl" to jvm property
"com.sun.net.ssl.checkRevocation" if it is unset.
+ // 2. Default "zookeeper.ssl.ocsp" to jvm security property
"ocsp.enable" if it is unset.
+ // 3. Set `Option.ONLY_END_ENTITY` for jvm security property
"com.sun.security.onlyCheckRevocationOfEECert".
+ // 4. Don't set "com.sun.security.enableCRLDP" as it is always
enabled in no legacy path.
+ //
+ // See also:
+ // *
https://docs.oracle.com/javase/8/docs/technotes/guides/security/jsse/ocsp.html
+ // *
https://docs.oracle.com/javase/8/docs/technotes/guides/security/jsse/JSSERefGuide.html
+ // *
https://docs.oracle.com/javase/8/docs/technotes/guides/security/certpath/CertPathProgGuide.html#PKIXRevocationChecker
+ //
+ // [1]:
https://github.com/openjdk/jdk/blob/jdk-11%2B28/src/java.base/share/classes/sun/security/provider/certpath/RevocationChecker.java#L124
+ // [2]:
https://github.com/openjdk/jdk/blob/jdk-11%2B28/src/java.base/share/classes/sun/security/provider/certpath/RevocationChecker.java#L179
+ Set<PKIXRevocationChecker.Option> options = new HashSet<>();
+ if (!ocspEnabled) {
+ options.add(PKIXRevocationChecker.Option.NO_FALLBACK);
+ options.add(PKIXRevocationChecker.Option.PREFER_CRLS);
+ }
+ if
(Boolean.parseBoolean(Security.getProperty("com.sun.security.onlyCheckRevocationOfEECert")))
{
+ options.add(PKIXRevocationChecker.Option.ONLY_END_ENTITY);
}
+
+ PKIXRevocationChecker revocationChecker =
(PKIXRevocationChecker)
CertPathValidator.getInstance("PKIX").getRevocationChecker();
+ revocationChecker.setOptions(options);
+ pbParams.addCertPathChecker(revocationChecker);
} else {
pbParams.setRevocationEnabled(false);
}
diff --git
a/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/X509AuthenticationProvider.java
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/X509AuthenticationProvider.java
index 4ea925320..8286c061c 100644
---
a/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/X509AuthenticationProvider.java
+++
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/auth/X509AuthenticationProvider.java
@@ -18,6 +18,7 @@
package org.apache.zookeeper.server.auth;
+import java.security.Security;
import java.security.cert.Certificate;
import java.security.cert.CertificateException;
import java.security.cert.X509Certificate;
@@ -85,8 +86,8 @@ public X509AuthenticationProvider() throws X509Exception {
x509Util.getSslKeystorePasswdPathProperty());
String keyStoreTypeProp =
config.getProperty(x509Util.getSslKeystoreTypeProperty());
- boolean crlEnabled =
Boolean.parseBoolean(config.getProperty(x509Util.getSslCrlEnabledProperty()));
- boolean ocspEnabled =
Boolean.parseBoolean(config.getProperty(x509Util.getSslOcspEnabledProperty()));
+ boolean crlEnabled =
config.getBoolean(x509Util.getSslCrlEnabledProperty(),
Boolean.getBoolean("com.sun.net.ssl.checkRevocation"));
+ boolean ocspEnabled =
config.getBoolean(x509Util.getSslOcspEnabledProperty(),
Boolean.parseBoolean(Security.getProperty("ocsp.enable")));
boolean hostnameVerificationEnabled =
Boolean.parseBoolean(config.getProperty(x509Util.getSslHostnameVerificationEnabledProperty()));
X509KeyManager km = null;
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 f672bf468..2bb759e63 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
@@ -80,7 +80,7 @@ public class X509TestContext {
* @throws GeneralSecurityException
* @throws OperatorCreationException
*/
- private X509TestContext(File tempDir, KeyPair trustStoreKeyPair, long
trustStoreCertExpirationMillis, String trustStorePassword, KeyPair
keyStoreKeyPair, long keyStoreCertExpirationMillis, String keyStorePassword,
Boolean hostnameVerification) throws IOException, GeneralSecurityException,
OperatorCreationException {
+ private X509TestContext(File tempDir, KeyPair trustStoreKeyPair, long
trustStoreCertExpirationMillis, String trustStorePassword, KeyPair
keyStoreKeyPair, long keyStoreCertExpirationMillis, String keyStorePassword,
Boolean hostnameVerification) throws Exception {
if (Security.getProvider(BouncyCastleProvider.PROVIDER_NAME) == null) {
throw new IllegalStateException("BC Security provider was not
found");
}
@@ -425,7 +425,7 @@ public Builder() {
* @throws GeneralSecurityException
* @throws OperatorCreationException
*/
- public X509TestContext build() throws IOException,
GeneralSecurityException, OperatorCreationException {
+ public X509TestContext build() throws Exception {
KeyPair trustStoreKeyPair =
X509TestHelpers.generateKeyPair(trustStoreKeyType);
KeyPair keyStoreKeyPair =
X509TestHelpers.generateKeyPair(keyStoreKeyType);
return new X509TestContext(tempDir, trustStoreKeyPair,
trustStoreCertExpirationMillis, trustStorePassword, keyStoreKeyPair,
keyStoreCertExpirationMillis, keyStorePassword, hostnameVerification);
diff --git
a/zookeeper-server/src/test/java/org/apache/zookeeper/common/X509TestHelpers.java
b/zookeeper-server/src/test/java/org/apache/zookeeper/common/X509TestHelpers.java
index b9f2f6db9..89907e078 100644
---
a/zookeeper-server/src/test/java/org/apache/zookeeper/common/X509TestHelpers.java
+++
b/zookeeper-server/src/test/java/org/apache/zookeeper/common/X509TestHelpers.java
@@ -36,11 +36,14 @@
import java.security.cert.X509Certificate;
import java.security.spec.ECGenParameterSpec;
import java.security.spec.RSAKeyGenParameterSpec;
+import java.time.Duration;
import java.util.Date;
import org.bouncycastle.asn1.DERIA5String;
import org.bouncycastle.asn1.DEROctetString;
import org.bouncycastle.asn1.pkcs.PKCSObjectIdentifiers;
import org.bouncycastle.asn1.x500.X500Name;
+import org.bouncycastle.asn1.x500.X500NameBuilder;
+import org.bouncycastle.asn1.x500.style.BCStyle;
import org.bouncycastle.asn1.x509.AlgorithmIdentifier;
import org.bouncycastle.asn1.x509.BasicConstraints;
import org.bouncycastle.asn1.x509.ExtendedKeyUsage;
@@ -53,6 +56,7 @@
import org.bouncycastle.cert.X509CertificateHolder;
import org.bouncycastle.cert.X509v3CertificateBuilder;
import org.bouncycastle.cert.jcajce.JcaX509CertificateConverter;
+import org.bouncycastle.cert.jcajce.JcaX509v3CertificateBuilder;
import org.bouncycastle.crypto.params.AsymmetricKeyParameter;
import org.bouncycastle.crypto.util.PrivateKeyFactory;
import org.bouncycastle.jce.provider.BouncyCastleProvider;
@@ -85,6 +89,27 @@ public class X509TestHelpers {
// Per RFC 5280 section 4.1.2.2, X509 certificates can use up to 20 bytes
== 160 bits for serial numbers.
private static final int SERIAL_NUMBER_MAX_BITS = 20 * Byte.SIZE;
+ public static X509Certificate newSelfSignedCert(String name, KeyPair
keyPair) throws IOException, OperatorCreationException,
GeneralSecurityException {
+ X500NameBuilder caNameBuilder = new X500NameBuilder(BCStyle.INSTANCE);
+ caNameBuilder.addRDN(BCStyle.CN, name);
+ return newSelfSignedCACert(caNameBuilder.build(), keyPair,
Duration.ofDays(1).toMillis());
+ }
+
+ @FunctionalInterface
+ public interface CertificateCustomization {
+ void customize(X509v3CertificateBuilder builder) throws Exception;
+ }
+
+ public static X509Certificate newCert(X509Certificate caCert, KeyPair
caKeyPair, String name, PublicKey certPublicKey, CertificateCustomization
customization) throws Exception {
+ X500NameBuilder nameBuilder = new X500NameBuilder(BCStyle.INSTANCE);
+ nameBuilder.addRDN(BCStyle.CN, name);
+ return newCert(caCert, caKeyPair, nameBuilder.build(), certPublicKey,
Duration.ofDays(1).toMillis(), customization);
+ }
+
+ public static X509Certificate newCert(X509Certificate caCert, KeyPair
caKeyPair, String name, PublicKey certPublicKey) throws Exception {
+ return newCert(caCert, caKeyPair, name, certPublicKey, null);
+ }
+
/**
* Uses the private key of the given key pair to create a self-signed CA
certificate with the public half of the
* key pair and the given subject and expiration. The issuer of the new
cert will be equal to the subject.
@@ -102,6 +127,11 @@ public class X509TestHelpers {
*/
public static X509Certificate newSelfSignedCACert(
X500Name subject, KeyPair keyPair, long expirationMillis) throws
IOException, OperatorCreationException, GeneralSecurityException {
+ return newSelfSignedCACert(subject, keyPair, expirationMillis, null);
+ }
+
+ public static X509Certificate newSelfSignedCACert(
+ X500Name subject, KeyPair keyPair, long expirationMillis,
CertificateCustomization customization) throws IOException,
OperatorCreationException, GeneralSecurityException {
Date now = new Date();
X509v3CertificateBuilder builder = initCertBuilder(subject, // for
self-signed certs, issuer == subject
now, new
Date(now.getTime()
@@ -129,7 +159,12 @@ now, new Date(now.getTime()
* @throws GeneralSecurityException
*/
public static X509Certificate newCert(
- X509Certificate caCert, KeyPair caKeyPair, X500Name certSubject,
PublicKey certPublicKey, long expirationMillis) throws IOException,
OperatorCreationException, GeneralSecurityException {
+ X509Certificate caCert, KeyPair caKeyPair, X500Name certSubject,
PublicKey certPublicKey, long expirationMillis) throws Exception {
+ return newCert(caCert, caKeyPair, certSubject, certPublicKey,
expirationMillis, null);
+ }
+
+ public static X509Certificate newCert(
+ X509Certificate caCert, KeyPair caKeyPair, X500Name certSubject,
PublicKey certPublicKey, long expirationMillis, CertificateCustomization
customization) throws Exception {
if (!caKeyPair.getPublic().equals(caCert.getPublicKey())) {
throw new IllegalArgumentException("CA private key does not match
the public key in the CA cert");
}
@@ -143,6 +178,9 @@ public static X509Certificate newCert(
builder.addExtension(Extension.extendedKeyUsage, true, new
ExtendedKeyUsage(new KeyPurposeId[]{KeyPurposeId.id_kp_serverAuth,
KeyPurposeId.id_kp_clientAuth}));
builder.addExtension(Extension.subjectAlternativeName, false,
getLocalhostSubjectAltNames());
+ if (customization != null) {
+ customization.customize(builder);
+ }
return buildAndSignCertificate(caKeyPair.getPrivate(), builder);
}
@@ -172,7 +210,7 @@ private static GeneralNames getLocalhostSubjectAltNames()
throws UnknownHostExce
*/
private static X509v3CertificateBuilder initCertBuilder(
X500Name issuer, Date notBefore, Date notAfter, X500Name subject,
PublicKey subjectPublicKey) {
- return new X509v3CertificateBuilder(issuer, new
BigInteger(SERIAL_NUMBER_MAX_BITS, PRNG), notBefore, notAfter, subject,
SubjectPublicKeyInfo.getInstance(subjectPublicKey.getEncoded()));
+ return new JcaX509v3CertificateBuilder(issuer, new
BigInteger(SERIAL_NUMBER_MAX_BITS, PRNG), notBefore, notAfter, subject,
SubjectPublicKeyInfo.getInstance(subjectPublicKey.getEncoded()));
}
/**
diff --git
a/zookeeper-server/src/test/java/org/apache/zookeeper/common/X509UtilTest.java
b/zookeeper-server/src/test/java/org/apache/zookeeper/common/X509UtilTest.java
index dd803c49f..ff054c1d6 100644
---
a/zookeeper-server/src/test/java/org/apache/zookeeper/common/X509UtilTest.java
+++
b/zookeeper-server/src/test/java/org/apache/zookeeper/common/X509UtilTest.java
@@ -20,7 +20,6 @@
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import io.netty.buffer.UnpooledByteBufAllocator;
@@ -32,7 +31,6 @@
import java.net.Socket;
import java.nio.file.Path;
import java.security.NoSuchAlgorithmException;
-import java.security.Security;
import java.util.Arrays;
import java.util.List;
import java.util.concurrent.Callable;
@@ -90,10 +88,6 @@ public void cleanUp() {
System.clearProperty(x509Util.getCipherSuitesProperty());
System.clearProperty(x509Util.getSslProtocolProperty());
System.clearProperty(x509Util.getSslHandshakeDetectionTimeoutMillisProperty());
- System.clearProperty("com.sun.net.ssl.checkRevocation");
- System.clearProperty("com.sun.security.enableCRLDP");
- Security.setProperty("ocsp.enable", Boolean.FALSE.toString());
- Security.setProperty("com.sun.security.enableCRLDP",
Boolean.FALSE.toString());
System.clearProperty(ServerCnxnFactory.ZOOKEEPER_SERVER_CNXN_FACTORY);
System.clearProperty(ZKClientConfig.ZOOKEEPER_CLIENT_CNXN_SOCKET);
x509Util.close();
@@ -227,49 +221,6 @@ public void testCreateSSLContextWithCustomCipherSuites(
assertArrayEquals(customCipherSuites,
sslSocket.getEnabledCipherSuites());
}
- // It would be great to test the value of
PKIXBuilderParameters#setRevocationEnabled but it does not appear to be
- // possible
- @ParameterizedTest
- @MethodSource("data")
- @Timeout(value = 5)
- public void testCRLEnabled(
- X509KeyType caKeyType, X509KeyType certKeyType, String
keyPassword, Integer paramIndex)
- throws Exception {
- init(caKeyType, certKeyType, keyPassword, paramIndex);
- System.setProperty(x509Util.getSslCrlEnabledProperty(), "true");
- x509Util.getDefaultSSLContext();
-
assertTrue(Boolean.valueOf(System.getProperty("com.sun.net.ssl.checkRevocation")));
-
assertTrue(Boolean.valueOf(System.getProperty("com.sun.security.enableCRLDP")));
- assertFalse(Boolean.valueOf(Security.getProperty("ocsp.enable")));
- }
-
- @ParameterizedTest
- @MethodSource("data")
- @Timeout(value = 5)
- public void testCRLDisabled(
- X509KeyType caKeyType, X509KeyType certKeyType, String
keyPassword, Integer paramIndex)
- throws Exception {
- init(caKeyType, certKeyType, keyPassword, paramIndex);
- x509Util.getDefaultSSLContext();
-
assertFalse(Boolean.valueOf(System.getProperty("com.sun.net.ssl.checkRevocation")));
-
assertFalse(Boolean.valueOf(System.getProperty("com.sun.security.enableCRLDP")));
- assertFalse(Boolean.valueOf(Security.getProperty("ocsp.enable")));
- }
-
- @ParameterizedTest
- @MethodSource("data")
- @Timeout(value = 5)
- public void testOCSPEnabled(
- X509KeyType caKeyType, X509KeyType certKeyType, String
keyPassword, Integer paramIndex)
- throws Exception {
- init(caKeyType, certKeyType, keyPassword, paramIndex);
- System.setProperty(x509Util.getSslOcspEnabledProperty(), "true");
- x509Util.getDefaultSSLContext();
-
assertTrue(Boolean.valueOf(System.getProperty("com.sun.net.ssl.checkRevocation")));
-
assertTrue(Boolean.valueOf(System.getProperty("com.sun.security.enableCRLDP")));
- assertTrue(Boolean.valueOf(Security.getProperty("ocsp.enable")));
- }
-
@ParameterizedTest
@MethodSource("data")
@Timeout(value = 5)
diff --git
a/zookeeper-server/src/test/java/org/apache/zookeeper/server/ClientSSLRevocationTest.java
b/zookeeper-server/src/test/java/org/apache/zookeeper/server/ClientSSLRevocationTest.java
new file mode 100644
index 000000000..6e1fd2708
--- /dev/null
+++
b/zookeeper-server/src/test/java/org/apache/zookeeper/server/ClientSSLRevocationTest.java
@@ -0,0 +1,610 @@
+/*
+ * 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.zookeeper.server;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import com.sun.net.httpserver.Headers;
+import com.sun.net.httpserver.HttpHandler;
+import com.sun.net.httpserver.HttpServer;
+import java.io.FileWriter;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStream;
+import java.math.BigInteger;
+import java.net.InetSocketAddress;
+import java.net.URLDecoder;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.StandardCopyOption;
+import java.security.KeyPair;
+import java.security.Security;
+import java.security.cert.X509Certificate;
+import java.time.Instant;
+import java.util.Arrays;
+import java.util.Base64;
+import java.util.Calendar;
+import java.util.Collections;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Properties;
+import java.util.UUID;
+import java.util.concurrent.atomic.AtomicLong;
+import java.util.stream.Collectors;
+import org.apache.zookeeper.client.ZKClientConfig;
+import org.apache.zookeeper.common.X509TestHelpers;
+import org.apache.zookeeper.server.embedded.ExitHandler;
+import org.apache.zookeeper.server.embedded.ZooKeeperServerEmbedded;
+import org.apache.zookeeper.test.ClientBase;
+import org.bouncycastle.asn1.ASN1GeneralizedTime;
+import org.bouncycastle.asn1.ocsp.OCSPResponse;
+import org.bouncycastle.asn1.ocsp.OCSPResponseStatus;
+import org.bouncycastle.asn1.ocsp.RevokedInfo;
+import org.bouncycastle.asn1.x509.AuthorityInformationAccess;
+import org.bouncycastle.asn1.x509.CRLDistPoint;
+import org.bouncycastle.asn1.x509.CRLNumber;
+import org.bouncycastle.asn1.x509.CRLReason;
+import org.bouncycastle.asn1.x509.DistributionPoint;
+import org.bouncycastle.asn1.x509.DistributionPointName;
+import org.bouncycastle.asn1.x509.Extension;
+import org.bouncycastle.asn1.x509.GeneralName;
+import org.bouncycastle.asn1.x509.GeneralNames;
+import org.bouncycastle.asn1.x509.X509ObjectIdentifiers;
+import org.bouncycastle.cert.X509CRLHolder;
+import org.bouncycastle.cert.X509CertificateHolder;
+import org.bouncycastle.cert.X509v2CRLBuilder;
+import org.bouncycastle.cert.jcajce.JcaX509CertificateHolder;
+import org.bouncycastle.cert.jcajce.JcaX509ExtensionUtils;
+import org.bouncycastle.cert.jcajce.JcaX509v2CRLBuilder;
+import org.bouncycastle.cert.ocsp.BasicOCSPResp;
+import org.bouncycastle.cert.ocsp.BasicOCSPRespBuilder;
+import org.bouncycastle.cert.ocsp.CertificateID;
+import org.bouncycastle.cert.ocsp.CertificateStatus;
+import org.bouncycastle.cert.ocsp.OCSPReq;
+import org.bouncycastle.cert.ocsp.OCSPResp;
+import org.bouncycastle.cert.ocsp.OCSPRespBuilder;
+import org.bouncycastle.cert.ocsp.Req;
+import org.bouncycastle.cert.ocsp.RevokedStatus;
+import org.bouncycastle.cert.ocsp.jcajce.JcaBasicOCSPRespBuilder;
+import org.bouncycastle.cert.ocsp.jcajce.JcaCertificateID;
+import org.bouncycastle.jce.provider.BouncyCastleProvider;
+import org.bouncycastle.openssl.MiscPEMGenerator;
+import org.bouncycastle.operator.ContentSigner;
+import org.bouncycastle.operator.DigestCalculator;
+import org.bouncycastle.operator.jcajce.JcaContentSignerBuilder;
+import org.bouncycastle.operator.jcajce.JcaDigestCalculatorProviderBuilder;
+import org.bouncycastle.util.io.pem.PemWriter;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class ClientSSLRevocationTest {
+ private static final Logger LOG =
LoggerFactory.getLogger(ClientSSLRevocationTest.class);
+
+ static {
+ Security.addProvider(new BouncyCastleProvider());
+ }
+
+ private static class OCSPHandler implements HttpHandler {
+ private final Ca ca;
+
+ public OCSPHandler(Ca ca) {
+ this.ca = ca;
+ }
+
+ @Override
+ public void handle(com.sun.net.httpserver.HttpExchange httpExchange)
throws IOException {
+ byte[] responseBytes;
+ try {
+ String uri = httpExchange.getRequestURI().toString();
+ LOG.info("OCSP request: {} {}",
httpExchange.getRequestMethod(), uri);
+ httpExchange.getRequestHeaders().entrySet().forEach((e) -> {
+ LOG.info("OCSP request header: {} {}", e.getKey(),
e.getValue());
+ });
+ InputStream request = httpExchange.getRequestBody();
+ byte[] requestBytes = new byte[10000];
+ int len = request.read(requestBytes);
+ LOG.info("OCSP request size {}", len);
+
+ if (len < 0) {
+ String removedUriEncoding =
URLDecoder.decode(uri.substring(1), "utf-8");
+ LOG.info("OCSP request from URI no encoding {}",
removedUriEncoding);
+ requestBytes =
Base64.getDecoder().decode(removedUriEncoding);
+ }
+ OCSPReq ocspRequest = new OCSPReq(requestBytes);
+ Req[] requestList = ocspRequest.getRequestList();
+ LOG.info("requestList {}", Arrays.toString(requestList));
+
+ DigestCalculator digestCalculator = new
JcaDigestCalculatorProviderBuilder().build().get(CertificateID.HASH_SHA1);
+
+ Map<CertificateID, RevokedInfo> revokedCerts =
ca.ocspRevokedCerts.entrySet().stream().collect(Collectors.toMap(entry -> {
+ try {
+ return new JcaCertificateID(digestCalculator,
ca.cert, entry.getKey().getSerialNumber());
+ } catch (Exception ex) {
+ throw new RuntimeException(ex);
+ }
+ }, Map.Entry::getValue));
+
+ BasicOCSPRespBuilder responseBuilder = new
JcaBasicOCSPRespBuilder(ca.key.getPublic(), digestCalculator);
+ for (Req req : requestList) {
+ CertificateID certId = req.getCertID();
+ CertificateStatus certificateStatus =
CertificateStatus.GOOD;
+ RevokedInfo revokedInfo = revokedCerts.get(certId);
+ if (revokedInfo != null) {
+ certificateStatus = new RevokedStatus(revokedInfo);
+ }
+ responseBuilder.addResponse(certId, certificateStatus,
null);
+ }
+
+ X509CertificateHolder[] chain = new
X509CertificateHolder[]{new JcaX509CertificateHolder(ca.cert)};
+ ContentSigner signer = new
JcaContentSignerBuilder("SHA1withRSA").setProvider("BC").build(ca.key.getPrivate());
+ BasicOCSPResp ocspResponse = responseBuilder.build(signer,
chain, Calendar.getInstance().getTime());
+ LOG.info("response {}", ocspResponse);
+ responseBytes = new
OCSPRespBuilder().build(OCSPRespBuilder.SUCCESSFUL, ocspResponse).getEncoded();
+ LOG.error("OCSP server response OK");
+ } catch (Throwable exception) {
+ LOG.error("Internal OCSP server error", exception);
+ responseBytes = new OCSPResp(new OCSPResponse(new
OCSPResponseStatus(OCSPRespBuilder.INTERNAL_ERROR), null)).getEncoded();
+ }
+
+ Headers rh = httpExchange.getResponseHeaders();
+ rh.set("Content-Type", "application/ocsp-response");
+ httpExchange.sendResponseHeaders(200, responseBytes.length);
+
+ OutputStream os = httpExchange.getResponseBody();
+ os.write(responseBytes);
+ os.close();
+ }
+
+ }
+
+ private static class PemFile {
+ private final Path file;
+ private final String password;
+
+ public PemFile(Path file, String password) {
+ this.file = file;
+ this.password = password;
+ }
+ }
+
+ private static class CertWithCrl extends Cert {
+ public final Path crl;
+
+ public CertWithCrl(String name, KeyPair key, X509Certificate cert,
Path crl) {
+ super(name, key, cert, crl.getParent());
+ this.crl = crl;
+ }
+ }
+
+ private static class Cert {
+ public final String name;
+ public final KeyPair key;
+ public final X509Certificate cert;
+ public final Path dir;
+
+ public Cert(String name, KeyPair key, X509Certificate cert, Path dir) {
+ this.name = name;
+ this.key = key;
+ this.cert = cert;
+ this.dir = dir;
+ }
+
+ public PemFile writePem() throws Exception {
+ String password = UUID.randomUUID().toString();
+ String pem = X509TestHelpers.pemEncodeCertAndPrivateKey(cert,
key.getPrivate(), password);
+ Path file = Files.createTempFile(dir, name, ".pem");
+ Files.write(file, pem.getBytes());
+ return new PemFile(file, password);
+ }
+ }
+
+ private static class Ca implements AutoCloseable {
+ private final String name;
+ private final KeyPair key;
+ private final X509Certificate cert;
+ private final Path dir;
+ private final Map<X509Certificate, RevokedInfo> crlRevokedCerts =
Collections.synchronizedMap(new HashMap<>());
+ private final Map<X509Certificate, RevokedInfo> ocspRevokedCerts =
Collections.synchronizedMap(new HashMap<>());
+ private final HttpServer ocspServer;
+ private final AtomicLong crlNumber = new AtomicLong(1);
+
+ private Ca(String name, KeyPair key, X509Certificate cert, Path dir,
HttpServer ocspServer) throws Exception {
+ this.name = name;
+ this.key = key;
+ this.cert = cert;
+ this.dir = dir;
+ this.ocspServer = ocspServer;
+ }
+
+ public PemFile writePem() throws Exception {
+ String pem = X509TestHelpers.pemEncodeX509Certificate(cert);
+ Path file = Files.createTempFile(dir, name, ".pem");
+ Files.write(file, pem.getBytes());
+ return new PemFile(file, "");
+ }
+
+ // Check result of crldp could be cached, so use per-cert crl file.
+ public void flush_crl(Path crl) throws Exception {
+ Instant now = Instant.now();
+
+ X509v2CRLBuilder builder = new
JcaX509v2CRLBuilder(cert.getIssuerX500Principal(), Date.from(now));
+ builder.setNextUpdate(Date.from(now.plusSeconds(2)));
+
+ builder.addExtension(Extension.authorityKeyIdentifier, false, new
JcaX509ExtensionUtils().createAuthorityKeyIdentifier(this.cert));
+ builder.addExtension(Extension.cRLNumber, false, new
CRLNumber(BigInteger.valueOf(crlNumber.getAndAdd(1L))));
+
+ for (Map.Entry<X509Certificate, RevokedInfo> entry :
crlRevokedCerts.entrySet()) {
+ builder.addCRLEntry(entry.getKey().getSerialNumber(),
entry.getValue().getRevocationTime().getDate(), CRLReason.cACompromise);
+ }
+
+ ContentSigner contentSigner = new
JcaContentSignerBuilder("SHA256WithRSAEncryption").build(this.key.getPrivate());
+ X509CRLHolder crlHolder = builder.build(contentSigner);
+
+ Path tmpFile = Files.createTempFile(dir, "crldp-", ".pem.tmp");
+ PemWriter pemWriter = new PemWriter(new
FileWriter(tmpFile.toFile()));
+ pemWriter.writeObject(new MiscPEMGenerator(crlHolder));
+ pemWriter.flush();
+ pemWriter.close();
+
+ Files.move(tmpFile, crl, StandardCopyOption.REPLACE_EXISTING,
StandardCopyOption.ATOMIC_MOVE);
+ }
+
+ public void revoke_through_crldp(CertWithCrl cert) throws Exception {
+ Date now = new Date();
+ RevokedInfo revokedInfo = new RevokedInfo(new
ASN1GeneralizedTime(now), CRLReason.lookup(CRLReason.cACompromise));
+ this.crlRevokedCerts.put(cert.cert, revokedInfo);
+ flush_crl(cert.crl);
+ }
+
+ public void revoke_through_ocsp(X509Certificate cert) throws Exception
{
+ Date now = new Date();
+ RevokedInfo revokedInfo = new RevokedInfo(new
ASN1GeneralizedTime(now), CRLReason.lookup(CRLReason.cACompromise));
+ this.ocspRevokedCerts.put(cert, revokedInfo);
+ }
+
+ public Cert sign(String name) throws Exception {
+ KeyPair key = X509TestHelpers.generateRSAKeyPair();
+ X509Certificate cert = X509TestHelpers.newCert(this.cert,
this.key, name, key.getPublic());
+ return new Cert(name, key, cert, dir);
+ }
+
+ public CertWithCrl sign_with_crl(String name) throws Exception {
+ KeyPair key = X509TestHelpers.generateRSAKeyPair();
+ Path crl = Files.createTempFile(dir, String.format("%s-crldp-",
name), ".pem");
+ X509Certificate cert = X509TestHelpers.newCert(this.cert,
this.key, name, key.getPublic(), builder -> {
+ DistributionPointName distPointOne = new DistributionPointName(
+ new GeneralNames(new
GeneralName(GeneralName.uniformResourceIdentifier, "file://" + crl)));
+ builder.addExtension(
+ Extension.cRLDistributionPoints,
+ false,
+ new CRLDistPoint(new DistributionPoint[]{new
DistributionPoint(distPointOne, null, null)}));
+
+ });
+ flush_crl(crl);
+ return new CertWithCrl(name, key, cert, crl);
+ }
+
+ public Cert sign_with_ocsp(String name) throws Exception {
+ KeyPair key = X509TestHelpers.generateRSAKeyPair();
+ X509Certificate cert = X509TestHelpers.newCert(this.cert,
this.key, name, key.getPublic(), builder -> {
+ String addr = "http://127.0.0.1:"; +
ocspServer.getAddress().getPort();
+ builder.addExtension(
+ Extension.authorityInfoAccess,
+ false,
+ new AuthorityInformationAccess(
+ X509ObjectIdentifiers.ocspAccessMethod,
+ new
GeneralName(GeneralName.uniformResourceIdentifier, addr)));
+ });
+ return new Cert(name, key, cert, dir);
+ }
+
+ public static Ca create(Path dir) throws Exception {
+ return create(dir, "CA");
+ }
+
+ public static Ca create(Path dir, String name) throws Exception {
+ KeyPair caKey = X509TestHelpers.generateRSAKeyPair();
+ X509Certificate caCert = X509TestHelpers.newSelfSignedCert(name,
caKey);
+ HttpServer ocspServer = HttpServer.create(new
InetSocketAddress("127.0.0.1", 0), 0);
+ Ca ca = new Ca(name, caKey, caCert, dir, ocspServer);
+ ca.ocspServer.createContext("/", new OCSPHandler(ca));
+ ca.ocspServer.start();
+ return ca;
+ }
+
+ @Override
+ public void close() throws Exception {
+ ocspServer.stop(0);
+ }
+ }
+
+ @AfterEach
+ public void cleanup() throws Exception {
+ Security.setProperty("ocsp.enable", "false");
+ System.clearProperty("com.sun.net.ssl.checkRevocation");
+ System.clearProperty("zookeeper.ssl.crl");
+ System.clearProperty("zookeeper.ssl.ocsp");
+ }
+
+ @Test
+ public void testRevocationDisabled(@TempDir Path tmpDir) throws Exception {
+ // given: crl not enabled
+ try (Ca ca = Ca.create(tmpDir)) {
+ PemFile caPem = ca.writePem();
+
+ Cert serverCert = ca.sign_with_ocsp("server");
+ final Properties config = getServerConfig(caPem, serverCert);
+ // given: revoked server cert
+ ca.revoke_through_ocsp(serverCert.cert);
+ try (ZooKeeperServerEmbedded server = ZooKeeperServerEmbedded
+ .builder()
+ .baseDir(Files.createTempDirectory(tmpDir, "server.data"))
+ .configuration(config)
+ .exitHandler(ExitHandler.LOG_ONLY)
+ .build()) {
+ server.start();
+
+ CertWithCrl client1Cert = ca.sign_with_crl("client1");
+ ca.revoke_through_crldp(client1Cert);
+
+
assertTrue(ClientBase.waitForServerUp(server.getConnectionString(), 60000));
+
+ // when: connect with revoked cert.
+ // then: connected
+
assertTrue(ClientBase.waitForServerUp(server.getSecureConnectionString(), 6000,
true, getZKClientConfig(caPem, client1Cert)));
+ }
+ }
+ }
+
+ @ParameterizedTest
+ @ValueSource(booleans = {true, false})
+ public void testRevocationInClientUsingCrldp(boolean clientRevoked,
@TempDir Path tmpDir) throws Exception {
+ try (Ca ca = Ca.create(tmpDir)) {
+ PemFile caPem = ca.writePem();
+ // given: server cert with crldp
+ CertWithCrl server1Cert = ca.sign_with_crl("server1");
+ try (ZooKeeperServerEmbedded server = ZooKeeperServerEmbedded
+ .builder()
+ .baseDir(Files.createTempDirectory(tmpDir, "server.data"))
+ .configuration(getServerConfig(caPem, server1Cert))
+ .exitHandler(ExitHandler.LOG_ONLY)
+ .build()) {
+ server.start();
+
+
assertTrue(ClientBase.waitForServerUp(server.getConnectionString(), 60000));
+
+ CertWithCrl clientCert = ca.sign_with_crl("client1");
+ if (clientRevoked) {
+ // crl in server side is disabled, so it does not matter
whether
+ // client cert is revoked or not.
+ ca.revoke_through_crldp(clientCert);
+ }
+
+ // then: ssl authentication succeed when crl is disabled
+ ZKClientConfig clientConfig = getZKClientConfig(caPem,
clientCert);
+
assertTrue(ClientBase.waitForServerUp(server.getSecureConnectionString(), 6000,
true, clientConfig));
+
+ // when: valid server cert
+ // then: ssl authentication succeed when crl is enabled
+ clientConfig.setProperty("zookeeper.ssl.crl", "true");
+
assertTrue(ClientBase.waitForServerUp(server.getSecureConnectionString(), 6000,
true, clientConfig));
+ }
+
+ // crldp check is not realtime, so we have to start a new server
with revoked cert
+
+ // given: revoked server cert with crldp
+ CertWithCrl server2Cert = ca.sign_with_crl("server2");
+ ca.revoke_through_crldp(server2Cert);
+ try (ZooKeeperServerEmbedded server = ZooKeeperServerEmbedded
+ .builder()
+ .baseDir(Files.createTempDirectory(tmpDir, "server2.data"))
+ .configuration(getServerConfig(caPem, server2Cert))
+ .exitHandler(ExitHandler.LOG_ONLY)
+ .build()) {
+ server.start();
+
+
assertTrue(ClientBase.waitForServerUp(server.getConnectionString(), 60000));
+
+ CertWithCrl clientCert = ca.sign_with_crl("client1");
+ if (clientRevoked) {
+ // crl in server side is disabled, so it does not matter
whether
+ // client cert is revoked or not.
+ ca.revoke_through_crldp(clientCert);
+ }
+
+ // then: ssl authentication succeed when crl is disabled
+ ZKClientConfig clientConfig = getZKClientConfig(caPem,
clientCert);
+
assertTrue(ClientBase.waitForServerUp(server.getSecureConnectionString(), 6000,
true, clientConfig));
+
+ // then: ssl authentication failed when crl is enabled
+ clientConfig.setProperty("zookeeper.ssl.crl", "true");
+
assertFalse(ClientBase.waitForServerUp(server.getSecureConnectionString(),
6000, true, clientConfig));
+ }
+ }
+ }
+
+ @ParameterizedTest
+ @ValueSource(booleans = {true, false})
+ public void testRevocationInClientUsingOCSP(boolean clientRevoked,
@TempDir Path tmpDir) throws Exception {
+ try (Ca ca = Ca.create(tmpDir)) {
+ PemFile caPem = ca.writePem();
+ // given: server cert with ocsp
+ Cert serverCert = ca.sign_with_ocsp("server1");
+ try (ZooKeeperServerEmbedded server = ZooKeeperServerEmbedded
+ .builder()
+ .baseDir(Files.createTempDirectory(tmpDir, "server.data"))
+ .configuration(getServerConfig(caPem, serverCert))
+ .exitHandler(ExitHandler.LOG_ONLY)
+ .build()) {
+ server.start();
+
+
assertTrue(ClientBase.waitForServerUp(server.getConnectionString(), 60000));
+
+ Cert clientCert = ca.sign_with_ocsp("client");
+ if (clientRevoked) {
+ // crl in server side is disabled, so it does not matter
whether
+ // client cert is revoked or not.
+ ca.revoke_through_ocsp(clientCert.cert);
+ }
+
+ ZKClientConfig clientConfig = getZKClientConfig(caPem,
clientCert);
+
+ // when: connect to serve with valid cert
+ // then: connected
+ //
+ // we can't config crl using jvm properties as server will
access them also
+ // see: https://issues.apache.org/jira/browse/ZOOKEEPER-4875
+ clientConfig.setProperty("zookeeper.ssl.crl", "true");
+ clientConfig.setProperty("zookeeper.ssl.ocsp", "true");
+
assertTrue(ClientBase.waitForServerUp(server.getSecureConnectionString(), 6000,
true, clientConfig));
+
+ // when: server cert get revoked
+ ca.revoke_through_ocsp(serverCert.cert);
+
+ // then: ssl authentication failed when crl is enabled
+
assertFalse(ClientBase.waitForServerUp(server.getSecureConnectionString(),
6000, true, clientConfig));
+
+ // then: ssl authentication succeed when crl is disabled
+ clientConfig.setProperty("zookeeper.ssl.crl", "false");
+
assertTrue(ClientBase.waitForServerUp(server.getSecureConnectionString(), 6000,
true, clientConfig));
+ }
+ }
+ }
+
+
+ @ParameterizedTest
+ @ValueSource(booleans = {true, false})
+ public void testRevocationInServerUsingCrldp(boolean serverRevoked,
@TempDir Path tmpDir) throws Exception {
+ try (Ca ca = Ca.create(tmpDir)) {
+ PemFile caPem = ca.writePem();
+ // given: server with crl enabled
+ System.setProperty("zookeeper.ssl.crl", "true");
+ CertWithCrl serverCert = ca.sign_with_crl("server1");
+ if (serverRevoked) {
+ // crl in client side will be disabled, so it does not matter
whether
+ // server cert is revoked or not.
+ ca.revoke_through_crldp(serverCert);
+ }
+ try (ZooKeeperServerEmbedded server = ZooKeeperServerEmbedded
+ .builder()
+ .baseDir(Files.createTempDirectory(tmpDir, "server.data"))
+ .configuration(getServerConfig(caPem, serverCert))
+ .exitHandler(ExitHandler.LOG_ONLY)
+ .build()) {
+ server.start();
+
+
assertTrue(ClientBase.waitForServerUp(server.getConnectionString(), 60000));
+
+ // when: valid client cert with crldp
+ // then: ssl authentication failed when crl is enabled
+ Cert client1Cert = ca.sign_with_crl("client1");
+ ZKClientConfig client1Config = getZKClientConfig(caPem,
client1Cert);
+ client1Config.setProperty("zookeeper.ssl.crl", "false");
+
assertTrue(ClientBase.waitForServerUp(server.getSecureConnectionString(), 6000,
true, client1Config));
+
+ CertWithCrl client2Cert = ca.sign_with_crl("client2");
+ ca.revoke_through_crldp(client2Cert);
+
+ // when: revoked client cert with crldp
+ // then: ssl authentication failed when crl is enabled
+ ZKClientConfig client2Config = getZKClientConfig(caPem,
client2Cert);
+ client2Config.setProperty("zookeeper.ssl.crl", "false");
+
assertFalse(ClientBase.waitForServerUp(server.getSecureConnectionString(),
6000, true, client2Config));
+ }
+ }
+ }
+
+ @ParameterizedTest
+ @ValueSource(booleans = {true, false})
+ public void testRevocationInServerUsingOCSP(boolean serverRevoked,
@TempDir Path tmpDir) throws Exception {
+ try (Ca ca = Ca.create(tmpDir)) {
+ PemFile caPem = ca.writePem();
+ // given: server with crl and ocsp enabled
+ System.setProperty("com.sun.net.ssl.checkRevocation", "true");
+ System.setProperty("zookeeper.ssl.ocsp", "true");
+ Cert serverCert = ca.sign("server1");
+ if (serverRevoked) {
+ // crl in client side will be disabled, so it does not matter
whether
+ // server cert is revoked or not.
+ ca.revoke_through_ocsp(serverCert.cert);
+ }
+ try (ZooKeeperServerEmbedded server = ZooKeeperServerEmbedded
+ .builder()
+ .baseDir(Files.createTempDirectory(tmpDir, "server.data"))
+ .configuration(getServerConfig(caPem, serverCert))
+ .exitHandler(ExitHandler.LOG_ONLY)
+ .build()) {
+ server.start();
+
+
assertTrue(ClientBase.waitForServerUp(server.getConnectionString(), 60000));
+
+ // when: valid client cert with crldp
+ // then: ssl authentication failed when crl is enabled
+ Cert client1Cert = ca.sign_with_ocsp("client1");
+ ZKClientConfig client1Config = getZKClientConfig(caPem,
client1Cert);
+ client1Config.setProperty("zookeeper.ssl.crl", "false");
+
assertTrue(ClientBase.waitForServerUp(server.getSecureConnectionString(), 6000,
true, client1Config));
+
+ // ocsp is realtime, so we can reuse this client.
+ ca.revoke_through_ocsp(client1Cert.cert);
+
assertFalse(ClientBase.waitForServerUp(server.getSecureConnectionString(),
6000, true, client1Config));
+ }
+ }
+ }
+
+ private Properties getServerConfig(PemFile ca, Cert identity) throws
Exception {
+ final Properties config = new Properties();
+ config.put("clientPort", "0");
+ config.put("secureClientPort", "0");
+ config.put("host", "localhost");
+ config.put("ticktime", "4000");
+
+ PemFile serverPem = identity.writePem();
+
+ // TLS config fields
+ //config.put("ssl.clientAuth", "need");
+ config.put("ssl.keyStore.location", serverPem.file.toString());
+ config.put("ssl.keyStore.password", serverPem.password);
+ config.put("ssl.trustStore.location", ca.file.toString());
+
+ // Netty is required for TLS
+ config.put("serverCnxnFactory",
org.apache.zookeeper.server.NettyServerCnxnFactory.class.getName());
+ config.put("4lw.commands.whitelist", "*");
+ return config;
+ }
+
+ private ZKClientConfig getZKClientConfig(PemFile ca, Cert cert) throws
Exception {
+ PemFile pemFile = cert.writePem();
+
+ ZKClientConfig config = new ZKClientConfig();
+ config.setProperty("zookeeper.client.secure", "true");
+ config.setProperty("zookeeper.ssl.keyStore.password",
pemFile.password);
+ config.setProperty("zookeeper.ssl.keyStore.location",
pemFile.file.toString());
+ config.setProperty("zookeeper.ssl.trustStore.location",
ca.file.toString());
+ // only netty supports TLS
+ config.setProperty("zookeeper.clientCnxnSocket",
org.apache.zookeeper.ClientCnxnSocketNetty.class.getName());
+ return config;
+ }
+}
diff --git
a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumSSLTest.java
b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumSSLTest.java
index e2e1227e9..c843efd00 100644
---
a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumSSLTest.java
+++
b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumSSLTest.java
@@ -901,6 +901,7 @@ public void testOCSP(boolean fipsEnabled) throws Exception {
assertTrue(ClientBase.waitForServerDown("127.0.0.1:" +
clientPortQp3, CONNECTION_TIMEOUT));
setSSLSystemProperties();
+ System.setProperty(quorumX509Util.getSslCrlEnabledProperty(),
"true");
System.setProperty(quorumX509Util.getSslOcspEnabledProperty(),
"true");
X509Certificate validCertificate = buildEndEntityCert(
