stoty commented on code in PR #2270:
URL: https://github.com/apache/zookeeper/pull/2270#discussion_r2214120127
##########
zookeeper-server/src/main/java/org/apache/zookeeper/common/ClientX509Util.java:
##########
@@ -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.
Review Comment:
Yes, it would fix the original issue.
However we want to phase out the exisitng OCSP setting (at least for the
client), as it relies on setting a JVM global system property. If we phase that
out, there will still be a need to somehow set this property, and that's what
the new property is for.
If you prefer, we can remove the new property from this patch and it back in
a follow-up patch that deals with the JVM globals.
--
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
