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

zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md

@@ -1776,6 +1776,13 @@ and [SASL authentication for ZooKeeper](https://cwiki.apache.org/confluence/disp
     Specifies whether Online Certificate Status Protocol is enabled in client and quorum TLS protocols.
     Default: false
 
+* *ssl.tcnative.ocsp.stapling* and *ssl.quorum.tcnative.ocsp.stapling* :
+    (Java system properties: **zookeeper.ssl.tcnative.ocsp.stapling** and **zookeeper.ssl.quorum.tcnative.ocsp.stapling**)
+    **New in 3.10.0:**
+    Specifies whether OCSP stapling is requested by the client.
+    This options has no side effects on JVM global system properties.
+    Default: if the option is not set, or set to the value "default" then the library default is used.
+
 * *ssl.clientAuth* and *ssl.quorum.clientAuth* :
     (Java system properties: **zookeeper.ssl.clientAuth** and **zookeeper.ssl.quorum.clientAuth**)
     **Added in 3.5.5, but broken until 3.5.7:**

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 SslContext createNettySslContextForClient(ZKConfig config)
             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 SslContext createNettySslContextForServer(ZKConfig config, KeyManager key
             sslContextBuilder.trustManager(trustManager);
         }
 
-        sslContextBuilder.enableOcsp(config.getBoolean(getSslOcspEnabledProperty()));
+        handleTcnativeOcspStapling(sslContextBuilder, config);
         String[] enabledProtocols = getEnabledProtocols(config);
         if (enabledProtocols != null) {
             sslContextBuilder.protocols(enabledProtocols);
@@ -144,6 +145,33 @@ 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());
+        TriState tcnativeOcspStapling = config.getTristate(getSslTcnativeOcspStaplingEnabledProperty());
+
+        if (tcnative && ocspEnabled && tcnativeOcspStapling.isDefault() && OpenSsl.isOcspSupported()) {
+            // Maintain old behaviour (mostly, we also check for OpenSsl.isOcspSupported())
+            builder.enableOcsp(ocspEnabled);
+        } else if (tcnativeOcspStapling.isTrue()) {
+            if (!tcnative) {
+                // Don't override the explicit setting, let it error out
+                LOG.error("Trying to enable OpenSSL OCSP stapling for non-OpenSSL TLS provider. "
+                        + "This is going to fail. Please fix the TLS configuration");
+            } else if (!OpenSsl.isOcspSupported()) {
+                LOG.warn("Trying to enable OpenSSL OCSP stapling for OpenSSL provider {} which does not support it. "
+                        + "This is either going to be ignored or fail.", OpenSsl.versionString());
+            }
+            builder.enableOcsp(true);
+        } else if (tcnativeOcspStapling.isFalse()) {
+            // Disabling OCSP for the builder is always safe.
+            // This is the same as the builder default, effectively a noop.
+            builder.enableOcsp(false);
+        }
+        return builder;
+    }
+
     private SslContext addHostnameVerification(SslContext sslContext, String clientOrServer) {
         return new DelegatingSslContext(sslContext) {
             @Override

zookeeper-server/src/main/java/org/apache/zookeeper/common/TriState.java

@@ -0,0 +1,53 @@
+/*
+ * 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.common;
+
+/**
+ * For storing configuration parameters where want to distinguish a default case
+ * in addition to true and false.
+ */
+public enum TriState {
+    True, False, Default;
+
+    public static TriState parse(String value) {
+        if (value == null || value.equalsIgnoreCase("default")) {
+            return TriState.Default;
+        } else if (value.equalsIgnoreCase("true")) {
+            return TriState.True;
+        } else {
+            return TriState.False;
+        }
+    }
+
+    public boolean isTrue() {
+        return this == TriState.True;
+    }
+
+    public boolean isFalse() {
+        return this == TriState.False;
+    }
+
+    public boolean isDefault() {
+        return this == TriState.Default;
+    }
+
+    public boolean isNotDefault() {
+        return this != TriState.Default;
+    }
+}

zookeeper-server/src/main/java/org/apache/zookeeper/common/X509Util.java

@@ -164,6 +164,7 @@ public io.netty.handler.ssl.ClientAuth toNettyClientAuth() {
     private final String sslClientHostnameVerificationEnabledProperty = getConfigPrefix() + "clientHostnameVerification";
     private final String sslCrlEnabledProperty = getConfigPrefix() + "crl";
     private final String sslOcspEnabledProperty = getConfigPrefix() + "ocsp";
+    private final String sslTcnativeOcspStaplingEnabledProperty = getConfigPrefix() + ".tcnative.ocsp.stapling";
     private final String sslClientAuthProperty = getConfigPrefix() + "clientAuth";
     private final String sslHandshakeDetectionTimeoutMillisProperty = getConfigPrefix() + "handshakeDetectionTimeoutMillis";
 
@@ -248,6 +249,10 @@ public String getSslOcspEnabledProperty() {
         return sslOcspEnabledProperty;
     }
 
+    public String getSslTcnativeOcspStaplingEnabledProperty() {
+        return sslTcnativeOcspStaplingEnabledProperty;
+    }
+
     public String getSslClientAuthProperty() {
         return sslClientAuthProperty;
     }

zookeeper-server/src/main/java/org/apache/zookeeper/common/ZKConfig.java

@@ -131,6 +131,7 @@ private void putSSLProperties(X509Util x509Util) {
         properties.put(x509Util.getSslHostnameVerificationEnabledProperty(), System.getProperty(x509Util.getSslHostnameVerificationEnabledProperty()));
         properties.put(x509Util.getSslCrlEnabledProperty(), System.getProperty(x509Util.getSslCrlEnabledProperty()));
         properties.put(x509Util.getSslOcspEnabledProperty(), System.getProperty(x509Util.getSslOcspEnabledProperty()));
+        properties.put(x509Util.getSslTcnativeOcspStaplingEnabledProperty(), System.getProperty(x509Util.getSslTcnativeOcspStaplingEnabledProperty()));
         properties.put(x509Util.getSslClientAuthProperty(), System.getProperty(x509Util.getSslClientAuthProperty()));
         properties.put(x509Util.getSslHandshakeDetectionTimeoutMillisProperty(), System.getProperty(x509Util.getSslHandshakeDetectionTimeoutMillisProperty()));
         properties.put(x509Util.getFipsModeProperty(), System.getProperty(x509Util.getFipsModeProperty()));
@@ -284,4 +285,15 @@ public int getInt(String key, int defaultValue) {
         return defaultValue;
     }
 
+    /**
+     * Returns {@code TriState.True} if and only if the property named by the argument
+     * exists and is equal to the string {@code "true"}.
+     * Returns {@code TriState.Default} if and only if the property named by the argument
+     * does not exist or is equal to the string {@code "default"}.
+     * Returns {@code TriState.False} otherwise.
+     */
+    public TriState getTristate(String key) {
+        String propertyValue = getProperty(key);
+        return TriState.parse(propertyValue);
+    }
 }

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

@@ -740,6 +740,20 @@ public void testCreateSSLContext_validCustomSSLContextClass(
         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;