xiangfu0 commented on code in PR #19828:
URL: https://github.com/apache/pinot/pull/19828#discussion_r4276513524
##########
pinot-tools/src/main/java/org/apache/pinot/tools/admin/PinotAdministrator.java:
##########
@@ -94,6 +95,7 @@ public class PinotAdministrator {
private static final Map<String, Command> SUBCOMMAND_MAP = new HashMap<>();
static {
+ ZooKeeperSslContextSupplier.initialize();
Review Comment:
Fixed in d6aac4d3d7a0ffb8c1a975d2263d09e646677c06. One guard in
PinotToolLauncher's existing static bootstrap runs before its registered
command instances are constructed, covering the separate launcher and embedded
access to those commands. PerfBenchmarkDriver's class bootstrap also covers
constructors/startComponents/run and its public static
waitForExternalViewUpdate utility, including existing-cluster mode that skips
starting Pinot roles. Process TLS bootstrap properties must be set before
loading these entry points.
Maven wrapper/JDK 25 validation succeeds across the 53-project tools source
closure. Eight existing TLS/Helix cases pass with zero failures/errors/skips,
and scoped Spotless/license/Checkstyle checks pass. Independent
compiled-bytecode inspection confirms both initialization calls precede their
client paths. No tools/performance workload was executed and no benchmark tests
were added.
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/tls/ZooKeeperSslContextSupplier.java:
##########
@@ -0,0 +1,64 @@
+/**
+ * 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.pinot.common.utils.tls;
+
+import java.util.Properties;
+import java.util.function.Supplier;
+import javax.net.ssl.SSLContext;
+import org.apache.zookeeper.common.ClientX509Util;
+import org.apache.zookeeper.common.X509Exception.SSLContextException;
+import org.apache.zookeeper.common.ZKConfig;
+
+
+/// Preserves disabled ZooKeeper hostname verification with Netty 4.2 using
ZooKeeper's JDK context supplier hook.
+///
+/// Pinot's Helix clients use process-wide ZooKeeper TLS bootstrap properties.
Call [#initialize()] before creating
+/// these clients; existing custom suppliers retain ownership of their context
and provider. The vendor's no-argument
+/// supplier cannot consume arbitrary per-client TLS overrides, so it is
intended for these bootstrap-owned clients.
+/// Each context uses ZooKeeper's configured stores, protocol, revocation and
FIPS policy, without caching key material.
+public class ZooKeeperSslContextSupplier implements Supplier<SSLContext> {
+ /// Installs the JDK adapter only when bootstrap configuration explicitly
disables hostname verification.
+ /// Enabled verification and custom suppliers keep their existing
construction paths. TLS bootstrap configuration
+ /// must be supplied before client creation; it cannot be obtained from the
cluster being connected to.
+ public static void initialize() {
+ Properties properties = System.getProperties();
+ synchronized (properties) {
+ try (ClientX509Util util = new ClientX509Util()) {
+ ZKConfig config = new ZKConfig();
+ if (util.isServerHostnameVerificationEnabled(config)
+ || config.getProperty(util.getSslContextSupplierClassProperty())
!= null) {
+ return;
+ }
+ // ZooKeeper 3.9.6 supports supplied SSLContext instances only with
its JDK provider.
+ properties.setProperty(util.getSslProviderProperty(), "JDK");
Review Comment:
Fixed in d6aac4d3d7a0ffb8c1a975d2263d09e646677c06. Before installing the
owned JDK supplier, initialize() converts configured OpenSSL cipher aliases to
JSSE names using Netty 4.2.19's public CipherSuiteConverter.toJava(). List
order and unmappable tokens are retained; no suites are silently dropped. The
enabled-verification/custom-supplier returns precede normalization, preserving
their provider and cipher ownership.
The existing hostname-policy/FIPS cases now begin from explicit OPENSSL and
OPENSSL_REFCNT configurations in the disabled-policy rows. Both real
trusted-mismatched-peer handshakes succeed and negotiate
TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256. Enabled-policy cases still reject the
certificate identity, and the custom-supplier case preserves its original
OPENSSL provider and alias. Removing only conversion reproduces exactly two
Unsupported CipherSuite: ECDHE-RSA-AES128-GCM-SHA256 errors; restoring it
passes all eight selected TLS/Helix cases with zero failures/errors/skips.
Scoped quality and the 53-project source build pass. No test cases or
benchmarks were added. Name translation does not add OpenSSL-only algorithms to
JDK TLS.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
