Browse Source

ZOOKEEPER-4940: Enabling zookeeper.ssl.ocsp with JRE TLS provider errors out

add docs
add new property for tcnative OCSP setting
rename property
factor out the stapling handling code to a new method
use and honor OpenSSL.isOcspSupported()
Add more log messages
Remove comments about BoringSSL not supporting OCSP stapling
rearrange code to make patch smaller
add comment for clarification
remove new property
Reviewers: anmolnar
Author: stoty
Closes #2270 from stoty/ZOOKEEPER-4940
Istvan Toth 2 weeks ago
parent
commit
9d1d25cd75

+ 14 - 2
zookeeper-server/src/main/java/org/apache/zookeeper/common/ClientX509Util.java

@@ -19,6 +19,7 @@
 package org.apache.zookeeper.common;
 
 import io.netty.handler.ssl.DelegatingSslContext;
+import io.netty.handler.ssl.OpenSsl;
 import io.netty.handler.ssl.SslContext;
 import io.netty.handler.ssl.SslContextBuilder;
 import io.netty.handler.ssl.SslProvider;
@@ -79,7 +80,7 @@ public class ClientX509Util extends X509Util {
             sslContextBuilder.trustManager(tm);
         }
 
-        sslContextBuilder.enableOcsp(config.getBoolean(getSslOcspEnabledProperty()));
+        handleTcnativeOcspStapling(sslContextBuilder, config);
         String[] enabledProtocols = getEnabledProtocols(config);
         if (enabledProtocols != null) {
             sslContextBuilder.protocols(enabledProtocols);
@@ -123,7 +124,7 @@ public class ClientX509Util extends X509Util {
             sslContextBuilder.trustManager(trustManager);
         }
 
-        sslContextBuilder.enableOcsp(config.getBoolean(getSslOcspEnabledProperty()));
+        handleTcnativeOcspStapling(sslContextBuilder, config);
         String[] enabledProtocols = getEnabledProtocols(config);
         if (enabledProtocols != null) {
             sslContextBuilder.protocols(enabledProtocols);
@@ -144,6 +145,17 @@ public class ClientX509Util extends X509Util {
         }
     }
 
+    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());
+
+        if (tcnative && ocspEnabled && OpenSsl.isOcspSupported()) {
+            builder.enableOcsp(ocspEnabled);
+        }
+        return builder;
+    }
+
     private SslContext addHostnameVerification(SslContext sslContext, String clientOrServer) {
         return new DelegatingSslContext(sslContext) {
             @Override

+ 14 - 0
zookeeper-server/src/test/java/org/apache/zookeeper/common/X509UtilTest.java

@@ -740,6 +740,20 @@ public class X509UtilTest extends BaseX509ParameterizedTestCase {
         assertEquals(SSLContext.getDefault(), sslContext);
     }
 
+    @ParameterizedTest
+    @MethodSource("data")
+    public void testCreateSSLContext_ocspWithJreProvider(
+            X509KeyType caKeyType, X509KeyType certKeyType, String keyPassword, Integer paramIndex)
+            throws Exception {
+        init(caKeyType, certKeyType, keyPassword, paramIndex);
+        ZKConfig zkConfig = new ZKConfig();
+        try (ClientX509Util clientX509Util = new ClientX509Util();) {
+            zkConfig.setProperty(clientX509Util.getSslOcspEnabledProperty(), "true");
+            // Must not throw IllegalArgumentException
+            clientX509Util.createSSLContext(zkConfig);
+        }
+    }
+
     private static void forceClose(Socket s) {
         if (s == null || s.isClosed()) {
             return;