stoty commented on code in PR #2270:
URL: https://github.com/apache/zookeeper/pull/2270#discussion_r2212148314


##########
zookeeper-server/src/main/java/org/apache/zookeeper/common/ClientX509Util.java:
##########
@@ -79,7 +79,11 @@ public SslContext createNettySslContextForClient(ZKConfig 
config)
             sslContextBuilder.trustManager(tm);
         }
 
-        
sslContextBuilder.enableOcsp(config.getBoolean(getSslOcspEnabledProperty()));
+        SslProvider sslProvider = getSslProvider(config);
+        sslContextBuilder.sslProvider(sslProvider);
+        if (sslProvider == SslProvider.OPENSSL || sslProvider == 
SslProvider.OPENSSL_REFCNT) {

Review Comment:
   OpenSsl.isAvailable() is completely irrelevant. It only tells if the OpenSSL 
provider is present on the classpath (perhaps with some sanity checks).
   We COULD make a different check for it, but Netty will handle this and error 
out anyway, so I don't see any added value.
   
   **The actual error that we want to fix is the error that we get when netty 
is configured to use the JRE PROVIDER and 
   we're calling enableOCSP().**
   
   OpenSsl.isOcspSupported() is actually broken:
   
   `    sslCtx = SSLContext.make(SSL.SSL_PROTOCOL_TLSV1_2, SSL.SSL_MODE_SERVER);
                   SSLContext.enableOcsp(sslCtx, false);
                   supportsOcsp = true;
   `
   
   As tcnative will simply ignore enableOcsp() if the native library does not 
support it, that call will return true for BoringSSL, so it only really tells 
us wheter calling enableOcsp() would cause the SSLContextContext creation to 
fail.
   Additionally, all enableOcsp() does is set a member in the builder, the 
actual check only happens at build time.
   
   While this won't do anything useful for now, we can add a check for this in 
case tcnative behaviour changes in the future and some relevant check is added 
in enableOcsp()
   



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscr...@zookeeper.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to