From b9a4cf394fc426f9259ee3d93c1d0c6bd07b20af Mon Sep 17 00:00:00 2001 From: Lari Hotari Date: Thu, 13 Aug 2026 03:19:05 +0300 Subject: [PATCH 1/7] [improve][misc] PIP-478: remove the superseded PIP-337 PulsarSslFactory stack MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The PIP-337 TLS infrastructure has no callers left. The core migration replaced it with the PulsarTlsFactory SPI and the common.util.tls helpers, and the configuration keys that once named DefaultPulsarSslFactory by class name (sslFactoryPlugin, brokerClientSslFactoryPlugin) are gone — they now sit on the removed-key tolerance lists, which PulsarConfigurationLoaderTest and ConfigurationDataUtilsTest already pin. Deleted, each verified unreachable from any production path: PulsarSslFactory, DefaultPulsarSslFactory, PulsarSslConfiguration, KeyManagerProxy, TrustManagerProxy, KeyStoreSSLContext, SSLContextValidatorEngine, and the four tests that exist only to exercise them. SSLContextValidatorEngine had no references at all, not even from its sibling. SecurityUtility.createAutoRefreshSslContextForClient goes with them: it was the sole production referent of KeyManagerProxy and TrustManagerProxy and had no callers of its own. The rest of SecurityUtility is removed in the next commit, once its last live consumer is migrated. Despite its name and package, SslContextTest is not a general Netty test — every case constructs a DefaultPulsarSslFactory with a PulsarSslConfiguration. Assisted-by: Claude Code (Opus 5) --- .../common/util/DefaultPulsarSslFactory.java | 373 ------------------ .../pulsar/common/util/KeyManagerProxy.java | 154 -------- .../common/util/PulsarSslConfiguration.java | 167 -------- .../pulsar/common/util/PulsarSslFactory.java | 106 ----- .../pulsar/common/util/SecurityUtility.java | 37 -- .../pulsar/common/util/TrustManagerProxy.java | 123 ------ .../util/keystoretls/KeyStoreSSLContext.java | 352 ----------------- .../SSLContextValidatorEngine.java | 166 -------- .../common/util/keystoretls/package-info.java | 23 -- .../util/DefaultPulsarSslFactoryTest.java | 282 ------------- .../common/util/KeyManagerProxyTest.java | 51 --- .../common/util/TrustManagerProxyTest.java | 53 --- .../common/util/netty/SslContextTest.java | 179 --------- 13 files changed, 2066 deletions(-) delete mode 100644 pulsar-common/src/main/java/org/apache/pulsar/common/util/DefaultPulsarSslFactory.java delete mode 100644 pulsar-common/src/main/java/org/apache/pulsar/common/util/KeyManagerProxy.java delete mode 100644 pulsar-common/src/main/java/org/apache/pulsar/common/util/PulsarSslConfiguration.java delete mode 100644 pulsar-common/src/main/java/org/apache/pulsar/common/util/PulsarSslFactory.java delete mode 100644 pulsar-common/src/main/java/org/apache/pulsar/common/util/TrustManagerProxy.java delete mode 100644 pulsar-common/src/main/java/org/apache/pulsar/common/util/keystoretls/KeyStoreSSLContext.java delete mode 100644 pulsar-common/src/main/java/org/apache/pulsar/common/util/keystoretls/SSLContextValidatorEngine.java delete mode 100644 pulsar-common/src/main/java/org/apache/pulsar/common/util/keystoretls/package-info.java delete mode 100644 pulsar-common/src/test/java/org/apache/pulsar/common/util/DefaultPulsarSslFactoryTest.java delete mode 100644 pulsar-common/src/test/java/org/apache/pulsar/common/util/KeyManagerProxyTest.java delete mode 100644 pulsar-common/src/test/java/org/apache/pulsar/common/util/TrustManagerProxyTest.java delete mode 100644 pulsar-common/src/test/java/org/apache/pulsar/common/util/netty/SslContextTest.java diff --git a/pulsar-common/src/main/java/org/apache/pulsar/common/util/DefaultPulsarSslFactory.java b/pulsar-common/src/main/java/org/apache/pulsar/common/util/DefaultPulsarSslFactory.java deleted file mode 100644 index b7902b4543fc5..0000000000000 --- a/pulsar-common/src/main/java/org/apache/pulsar/common/util/DefaultPulsarSslFactory.java +++ /dev/null @@ -1,373 +0,0 @@ -/* - * 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.pulsar.common.util; - -import io.netty.buffer.ByteBufAllocator; -import io.netty.handler.ssl.SslContext; -import io.netty.handler.ssl.SslProvider; -import java.io.IOException; -import java.security.GeneralSecurityException; -import java.util.concurrent.atomic.AtomicReference; -import javax.annotation.concurrent.NotThreadSafe; -import javax.net.ssl.SSLContext; -import javax.net.ssl.SSLEngine; -import javax.net.ssl.SSLParameters; -import org.apache.commons.lang3.StringUtils; -import org.apache.pulsar.client.api.AuthenticationDataProvider; -import org.apache.pulsar.client.api.KeyStoreParams; -import org.apache.pulsar.common.util.keystoretls.KeyStoreSSLContext; - -/** - * Default Implementation of {@link PulsarSslFactory}. This factory loads file based certificates to create SSLContext - * and SSL Engines. This class is not thread safe. It has been integrated into the pulsar code base as a single writer, - * multiple readers pattern. - */ -@NotThreadSafe -public class DefaultPulsarSslFactory implements PulsarSslFactory { - - private PulsarSslConfiguration config; - private final AtomicReference internalSslContext = new AtomicReference<>(); - private final AtomicReference internalNettySslContext = new AtomicReference<>(); - - protected FileModifiedTimeUpdater tlsKeyStore; - protected FileModifiedTimeUpdater tlsTrustStore; - protected FileModifiedTimeUpdater tlsTrustCertsFilePath; - protected FileModifiedTimeUpdater tlsCertificateFilePath; - protected FileModifiedTimeUpdater tlsKeyFilePath; - protected AuthenticationDataProvider authData; - protected boolean isTlsTrustStoreStreamProvided; - protected final String[] defaultSslEnabledProtocols = {"TLSv1.3", "TLSv1.2"}; - protected String tlsKeystoreType; - protected String tlsKeystorePath; - protected String tlsKeystorePassword; - - /** - * Initializes the DefaultPulsarSslFactory. - * - * @param config {@link PulsarSslConfiguration} object required for initialization. - * - */ - @Override - public void initialize(PulsarSslConfiguration config) { - this.config = config; - AuthenticationDataProvider authData = this.config.getAuthData(); - if (this.config.isTlsEnabledWithKeystore()) { - if (authData != null && authData.hasDataForTls()) { - KeyStoreParams authParams = authData.getTlsKeyStoreParams(); - if (authParams != null) { - this.tlsKeystoreType = authParams.getKeyStoreType(); - this.tlsKeystorePath = authParams.getKeyStorePath(); - this.tlsKeystorePassword = authParams.getKeyStorePassword(); - } - } - if (this.tlsKeystoreType == null) { - this.tlsKeystoreType = this.config.getTlsKeyStoreType(); - } - if (this.tlsKeystorePath == null) { - this.tlsKeystorePath = this.config.getTlsKeyStorePath(); - } - if (this.tlsKeystorePassword == null) { - this.tlsKeystorePassword = this.config.getTlsKeyStorePassword(); - } - this.tlsKeyStore = new FileModifiedTimeUpdater(this.tlsKeystorePath); - this.tlsTrustStore = new FileModifiedTimeUpdater(this.config.getTlsTrustStorePath()); - } else { - if (authData != null && authData.hasDataForTls()) { - if (authData.getTlsTrustStoreStream() != null) { - this.isTlsTrustStoreStreamProvided = true; - } else { - this.tlsTrustCertsFilePath = new FileModifiedTimeUpdater(this.config.getTlsTrustCertsFilePath()); - } - this.authData = authData; - } else { - this.tlsCertificateFilePath = new FileModifiedTimeUpdater(this.config.getTlsCertificateFilePath()); - this.tlsTrustCertsFilePath = new FileModifiedTimeUpdater(this.config.getTlsTrustCertsFilePath()); - this.tlsKeyFilePath = new FileModifiedTimeUpdater(this.config.getTlsKeyFilePath()); - } - } - } - - /** - * Creates a Client {@link SSLEngine} utilizing the peer hostname, peer port and {@link PulsarSslConfiguration} - * object provided during initialization. - * - * @param peerHost the name of the peer host - * @param peerPort the port number of the peer - * @return {@link SSLEngine} - */ - @Override - public SSLEngine createClientSslEngine(ByteBufAllocator buf, String peerHost, int peerPort) { - return createSSLEngine(buf, peerHost, peerPort, NetworkMode.CLIENT); - } - - /** - * Creates a Server {@link SSLEngine} utilizing the {@link PulsarSslConfiguration} object provided during - * initialization. - * - * @return {@link SSLEngine} - */ - @Override - public SSLEngine createServerSslEngine(ByteBufAllocator buf) { - return createSSLEngine(buf, "", 0, NetworkMode.SERVER); - } - - /** - * Returns a boolean value based on if the underlying certificate files have been modified since it was last read. - * - * @return {@code true} if the underlying certificates have been modified indicating that - * the SSL Context should be refreshed. - */ - @Override - public boolean needsUpdate() { - if (this.config.isTlsEnabledWithKeystore()) { - return (this.tlsKeyStore != null && this.tlsKeyStore.checkAndRefresh()) - || (this.tlsTrustStore != null && this.tlsTrustStore.checkAndRefresh()); - } else { - if (this.authData != null && this.authData.hasDataForTls()) { - return true; - } else { - return this.tlsTrustCertsFilePath.checkAndRefresh() || this.tlsCertificateFilePath.checkAndRefresh() - || this.tlsKeyFilePath.checkAndRefresh(); - } - } - } - - /** - * Creates a {@link SSLContext} object and saves it internally. - * - * @throws Exception If there were any issues generating the {@link SSLContext} - */ - @Override - public void createInternalSslContext() throws Exception { - if (this.config.isTlsEnabledWithKeystore()) { - this.internalSslContext.set(buildKeystoreSslContext(this.config.isServerMode())); - } else { - if (this.config.isHttps()) { - this.internalSslContext.set(buildSslContext()); - } else { - this.internalNettySslContext.set(buildNettySslContext()); - } - } - } - - - /** - * Get the internally stored {@link SSLContext}. - * - * @return {@link SSLContext} - * @throws RuntimeException if the {@link SSLContext} object has not yet been initialized. - */ - @Override - public SSLContext getInternalSslContext() { - if (this.internalSslContext.get() == null) { - throw new RuntimeException("Internal SSL context is not initialized. " - + "Please call createInternalSslContext() first."); - } - return this.internalSslContext.get(); - } - - /** - * Get the internally stored {@link SslContext}. - * - * @return {@link SslContext} - * @throws RuntimeException if the {@link SslContext} object has not yet been initialized. - */ - public SslContext getInternalNettySslContext() { - if (this.internalNettySslContext.get() == null) { - throw new RuntimeException("Internal SSL context is not initialized. " - + "Please call createInternalSslContext() first."); - } - return this.internalNettySslContext.get(); - } - - private SSLContext buildKeystoreSslContext(boolean isServerMode) throws GeneralSecurityException, IOException { - KeyStoreSSLContext keyStoreSSLContext; - if (isServerMode) { - keyStoreSSLContext = KeyStoreSSLContext.createServerKeyStoreSslContext(this.config.getTlsProvider(), - this.tlsKeystoreType, this.tlsKeyStore.getFileName(), - this.tlsKeystorePassword, this.config.isAllowInsecureConnection(), - this.config.getTlsTrustStoreType(), this.tlsTrustStore.getFileName(), - this.config.getTlsTrustStorePassword(), this.config.isRequireTrustedClientCertOnConnect(), - this.config.getTlsCiphers(), this.config.getTlsProtocols()); - } else { - keyStoreSSLContext = KeyStoreSSLContext.createClientKeyStoreSslContext(this.config.getTlsProvider(), - this.tlsKeystoreType, this.tlsKeyStore.getFileName(), - this.tlsKeystorePassword, this.config.isAllowInsecureConnection(), - this.config.getTlsTrustStoreType(), this.tlsTrustStore.getFileName(), - this.config.getTlsTrustStorePassword(), this.config.getTlsCiphers(), - this.config.getTlsProtocols()); - } - return keyStoreSSLContext.createSSLContext(); - } - - private SSLContext buildSslContext() throws GeneralSecurityException { - if (this.authData != null && this.authData.hasDataForTls()) { - if (this.isTlsTrustStoreStreamProvided) { - return SecurityUtility.createSslContext(this.config.isAllowInsecureConnection(), - SecurityUtility.loadCertificatesFromPemStream(this.authData.getTlsTrustStoreStream()), - this.authData.getTlsCertificates(), - this.authData.getTlsPrivateKey(), - this.config.getTlsProvider()); - } else { - if (this.authData.getTlsCertificates() != null) { - return SecurityUtility.createSslContext(this.config.isAllowInsecureConnection(), - SecurityUtility.loadCertificatesFromPemFile(this.tlsTrustCertsFilePath.getFileName()), - this.authData.getTlsCertificates(), - this.authData.getTlsPrivateKey(), - this.config.getTlsProvider()); - } else { - return SecurityUtility.createSslContext(this.config.isAllowInsecureConnection(), - this.tlsTrustCertsFilePath.getFileName(), - this.authData.getTlsCertificateFilePath(), - this.authData.getTlsPrivateKeyFilePath(), - this.config.getTlsProvider() - ); - } - } - } else { - return SecurityUtility.createSslContext(this.config.isAllowInsecureConnection(), - this.tlsTrustCertsFilePath.getFileName(), - this.tlsCertificateFilePath.getFileName(), - this.tlsKeyFilePath.getFileName(), - this.config.getTlsProvider()); - } - } - - private SslContext buildNettySslContext() throws GeneralSecurityException, IOException { - SslProvider sslProvider = null; - if (StringUtils.isNotBlank(this.config.getTlsProvider())) { - sslProvider = SslProvider.valueOf(this.config.getTlsProvider()); - } - if (this.authData != null && this.authData.hasDataForTls()) { - if (this.isTlsTrustStoreStreamProvided) { - return SecurityUtility.createNettySslContextForClient(sslProvider, - this.config.isAllowInsecureConnection(), - this.authData.getTlsTrustStoreStream(), - this.authData.getTlsCertificates(), - this.authData.getTlsPrivateKey(), - this.config.getTlsCiphers(), - this.config.getTlsProtocols()); - } else { - if (this.authData.getTlsCertificates() != null) { - return SecurityUtility.createNettySslContextForClient(sslProvider, - this.config.isAllowInsecureConnection(), - this.tlsTrustCertsFilePath.getFileName(), - this.authData.getTlsCertificates(), - this.authData.getTlsPrivateKey(), - this.config.getTlsCiphers(), - this.config.getTlsProtocols()); - } else { - return SecurityUtility.createNettySslContextForClient(sslProvider, - this.config.isAllowInsecureConnection(), - this.tlsTrustCertsFilePath.getFileName(), - this.authData.getTlsCertificateFilePath(), - this.authData.getTlsPrivateKeyFilePath(), - this.config.getTlsCiphers(), - this.config.getTlsProtocols()); - } - } - } else { - if (this.config.isServerMode()) { - return SecurityUtility.createNettySslContextForServer(sslProvider, - this.config.isAllowInsecureConnection(), - this.tlsTrustCertsFilePath.getFileName(), - this.tlsCertificateFilePath.getFileName(), - this.tlsKeyFilePath.getFileName(), - this.config.getTlsCiphers(), - this.config.getTlsProtocols(), - this.config.isRequireTrustedClientCertOnConnect()); - } else { - return SecurityUtility.createNettySslContextForClient(sslProvider, - this.config.isAllowInsecureConnection(), - this.tlsTrustCertsFilePath.getFileName(), - this.tlsCertificateFilePath.getFileName(), - this.tlsKeyFilePath.getFileName(), - this.config.getTlsCiphers(), - this.config.getTlsProtocols()); - } - } - } - - private SSLEngine createSSLEngine(ByteBufAllocator buf, String peerHost, int peerPort, NetworkMode mode) { - SSLEngine sslEngine; - SSLParameters sslParams; - SSLContext sslContext = this.internalSslContext.get(); - SslContext nettySslContext = this.internalNettySslContext.get(); - validateSslContext(sslContext, nettySslContext); - if (mode == NetworkMode.CLIENT) { - if (sslContext != null) { - sslEngine = sslContext.createSSLEngine(peerHost, peerPort); - } else { - sslEngine = nettySslContext.newEngine(buf, peerHost, peerPort); - } - sslEngine.setUseClientMode(true); - sslParams = sslEngine.getSSLParameters(); - } else { - if (sslContext != null) { - sslEngine = sslContext.createSSLEngine(); - } else { - sslEngine = nettySslContext.newEngine(buf); - } - sslEngine.setUseClientMode(false); - sslParams = sslEngine.getSSLParameters(); - if (this.config.isRequireTrustedClientCertOnConnect()) { - sslParams.setNeedClientAuth(true); - } else { - sslParams.setWantClientAuth(true); - } - } - // Netty 4.2 changed SslContext.newEngine(alloc, peerHost, peerPort) to default the endpoint - // identification algorithm to "HTTPS" (it was unset in 4.1). Pulsar applies TLS hostname - // verification separately via SecurityUtility.configureSSLHandler(), and only when - // tlsHostnameVerificationEnable is set. Clear it here so the engine does not perform unintended - // hostname verification (which would otherwise fail whenever the peer host is absent from the - // certificate SANs, e.g. internal "localhost" broker connections). - sslParams.setEndpointIdentificationAlgorithm(null); - if (this.config.getTlsProtocols() != null && !this.config.getTlsProtocols().isEmpty()) { - sslParams.setProtocols(this.config.getTlsProtocols().toArray(new String[0])); - } else { - sslParams.setProtocols(defaultSslEnabledProtocols); - } - if (this.config.getTlsCiphers() != null && !this.config.getTlsCiphers().isEmpty()) { - sslParams.setCipherSuites(this.config.getTlsCiphers().toArray(new String[0])); - } - sslEngine.setSSLParameters(sslParams); - return sslEngine; - } - - private void validateSslContext(SSLContext sslContext, SslContext nettySslContext) { - if (sslContext == null && nettySslContext == null) { - throw new RuntimeException("Internal SSL context is not initialized. " - + "Please call createInternalSslContext() first."); - } - } - - /** - * Clean any resources that may have been created. - * @throws Exception if any resources failed to be cleaned. - */ - @Override - public void close() throws Exception { - // noop - } - - private enum NetworkMode { - CLIENT, SERVER - } -} \ No newline at end of file diff --git a/pulsar-common/src/main/java/org/apache/pulsar/common/util/KeyManagerProxy.java b/pulsar-common/src/main/java/org/apache/pulsar/common/util/KeyManagerProxy.java deleted file mode 100644 index 4e2ac76b7cb1a..0000000000000 --- a/pulsar-common/src/main/java/org/apache/pulsar/common/util/KeyManagerProxy.java +++ /dev/null @@ -1,154 +0,0 @@ -/* - * 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.pulsar.common.util; - -import io.netty.handler.ssl.SslContext; -import java.io.FileInputStream; -import java.io.IOException; -import java.io.InputStream; -import java.net.Socket; -import java.security.KeyManagementException; -import java.security.KeyStore; -import java.security.KeyStoreException; -import java.security.NoSuchAlgorithmException; -import java.security.Principal; -import java.security.PrivateKey; -import java.security.UnrecoverableKeyException; -import java.security.cert.Certificate; -import java.security.cert.CertificateException; -import java.security.cert.CertificateFactory; -import java.security.cert.X509Certificate; -import java.util.List; -import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.TimeUnit; -import java.util.stream.Collectors; -import javax.net.ssl.KeyManagerFactory; -import javax.net.ssl.SSLEngine; -import javax.net.ssl.X509ExtendedKeyManager; -import lombok.CustomLog; - -/** - * This class wraps {@link X509ExtendedKeyManager} and gives opportunity to refresh key-manager with refreshed certs - * without changing {@link SslContext}. - */ -@CustomLog -public class KeyManagerProxy extends X509ExtendedKeyManager { - - private static final char[] KEYSTORE_PASSWORD = "secret".toCharArray(); - private volatile X509ExtendedKeyManager keyManager; - private FileModifiedTimeUpdater certFile, keyFile; - - public KeyManagerProxy(String certFilePath, String keyFilePath, int refreshDurationSec, - ScheduledExecutorService executor) { - this.certFile = new FileModifiedTimeUpdater(certFilePath); - this.keyFile = new FileModifiedTimeUpdater(keyFilePath); - try { - updateKeyManager(); - } catch (CertificateException e) { - log.warn().attr("certFile", certFile).exception(e).log("Failed to load cert"); - throw new IllegalArgumentException(e); - } catch (KeyStoreException e) { - log.warn().attr("keyFile", keyFile).exception(e).log("Failed to load key"); - throw new IllegalArgumentException(e); - } catch (NoSuchAlgorithmException | UnrecoverableKeyException e) { - log.warn().exception(e).log("Failed to update key Manager"); - throw new IllegalArgumentException(e); - } - executor.scheduleWithFixedDelay(() -> updateKeyManagerSafely(), refreshDurationSec, refreshDurationSec, - TimeUnit.SECONDS); - } - - private void updateKeyManagerSafely() { - try { - log.debug().attr("certFile", certFile.getFileName()).attr("keyFile", keyFile.getFileName()) - .log("refreshing key manager"); - updateKeyManager(); - } catch (Exception e) { - log.warn().attr("certFile", certFile.getFileName()).attr("keyFile", keyFile.getFileName()) - .exception(e).log("Failed to update key Manager"); - } - } - - private void updateKeyManager() - throws CertificateException, KeyStoreException, NoSuchAlgorithmException, UnrecoverableKeyException { - if (keyManager != null && !certFile.checkAndRefresh() && !keyFile.checkAndRefresh()) { - return; - } - - final KeyStore keyStore; - try (InputStream publicCertStream = new FileInputStream(certFile.getFileName())) { - final CertificateFactory cf = CertificateFactory.getInstance("X.509"); - final List certificateList = cf.generateCertificates(publicCertStream) - .stream().map(o -> (X509Certificate) o).collect(Collectors.toList()); - keyStore = KeyStore.getInstance("JKS"); - final String alias = certificateList.get(0).getSubjectX500Principal().getName(); - final PrivateKey privateKey = SecurityUtility.loadPrivateKeyFromPemFile(keyFile.getFileName()); - keyStore.load(null); - keyStore.setKeyEntry(alias, privateKey, KEYSTORE_PASSWORD, certificateList.toArray(new Certificate[0])); - } catch (IOException | KeyManagementException e) { - throw new IllegalArgumentException(e); - } - - final KeyManagerFactory keyManagerFactory = KeyManagerFactory - .getInstance(KeyManagerFactory.getDefaultAlgorithm()); - keyManagerFactory.init(keyStore, KEYSTORE_PASSWORD); - this.keyManager = (X509ExtendedKeyManager) keyManagerFactory.getKeyManagers()[0]; - } - - @Override - public String[] getClientAliases(String s, Principal[] principals) { - return keyManager.getClientAliases(s, principals); - } - - @Override - public String chooseClientAlias(String[] strings, Principal[] principals, Socket socket) { - return keyManager.chooseClientAlias(strings, principals, socket); - } - - @Override - public String[] getServerAliases(String s, Principal[] principals) { - return keyManager.getServerAliases(s, principals); - } - - @Override - public String chooseServerAlias(String s, Principal[] principals, Socket socket) { - return keyManager.chooseServerAlias(s, principals, socket); - } - - @Override - public X509Certificate[] getCertificateChain(String s) { - return keyManager.getCertificateChain(s); - } - - @Override - public PrivateKey getPrivateKey(String s) { - return keyManager.getPrivateKey(s); - } - - @Override - public String chooseEngineClientAlias(String[] keyType, Principal[] issuers, SSLEngine engine) { - return keyManager.chooseEngineClientAlias(keyType, issuers, engine); - } - - @Override - public String chooseEngineServerAlias(String keyType, Principal[] issuers, SSLEngine engine) { - return keyManager.chooseEngineServerAlias(keyType, issuers, engine); - } - -} diff --git a/pulsar-common/src/main/java/org/apache/pulsar/common/util/PulsarSslConfiguration.java b/pulsar-common/src/main/java/org/apache/pulsar/common/util/PulsarSslConfiguration.java deleted file mode 100644 index 86e566c12f049..0000000000000 --- a/pulsar-common/src/main/java/org/apache/pulsar/common/util/PulsarSslConfiguration.java +++ /dev/null @@ -1,167 +0,0 @@ -/* - * 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.pulsar.common.util; - -import io.swagger.v3.oas.annotations.media.Schema; -import java.io.Serializable; -import java.util.Set; -import lombok.Builder; -import lombok.Getter; -import lombok.ToString; -import org.apache.pulsar.client.api.AuthenticationDataProvider; - -/** - * Pulsar SSL Configuration Object to be used by all Pulsar Server and Client Components. - */ -@Builder -@Getter -@ToString -public class PulsarSslConfiguration implements Serializable, Cloneable { - - private static final long serialVersionUID = 1L; - - @Schema( - name = "tlsCiphers", - description = "TLS ciphers to be used", - requiredMode = Schema.RequiredMode.REQUIRED - ) - private Set tlsCiphers; - - @Schema( - name = "tlsProtocols", - description = "TLS protocols to be used", - requiredMode = Schema.RequiredMode.REQUIRED - ) - private Set tlsProtocols; - - @Schema( - name = "allowInsecureConnection", - description = "Insecure Connections are allowed", - requiredMode = Schema.RequiredMode.REQUIRED - ) - private boolean allowInsecureConnection; - - @Schema( - name = "requireTrustedClientCertOnConnect", - description = "Require trusted client certificate on connect", - requiredMode = Schema.RequiredMode.REQUIRED - ) - private boolean requireTrustedClientCertOnConnect; - - @Schema( - name = "authData", - description = "Authentication Data Provider utilized by the Client for identification" - ) - private AuthenticationDataProvider authData; - - @Schema( - name = "tlsCustomParams", - description = "Custom Parameters required by Pulsar SSL factory plugins" - ) - private String tlsCustomParams; - - @Schema( - name = "tlsProvider", - description = "TLS Provider to be used" - ) - private String tlsProvider; - - @Schema( - name = "tlsTrustStoreType", - description = "TLS Trust Store Type to be used" - ) - private String tlsTrustStoreType; - - @Schema( - name = "tlsTrustStorePath", - description = "TLS Trust Store Path" - ) - private String tlsTrustStorePath; - - @Schema( - name = "tlsTrustStorePassword", - description = "TLS Trust Store Password" - ) - private String tlsTrustStorePassword; - - @Schema( - name = "tlsTrustCertsFilePath", - description = "TLS Trust certificates file path" - ) - private String tlsTrustCertsFilePath; - - @Schema( - name = "tlsCertificateFilePath", - description = "Path for the TLS Certificate file" - ) - private String tlsCertificateFilePath; - - @Schema( - name = "tlsKeyFilePath", - description = "Path for TLS Private key file" - ) - private String tlsKeyFilePath; - - @Schema( - name = "tlsKeyStoreType", - description = "TLS Key Store Type to be used" - ) - private String tlsKeyStoreType; - - @Schema( - name = "tlsKeyStorePath", - description = "TLS Key Store Path" - ) - private String tlsKeyStorePath; - - @Schema( - name = "tlsKeyStorePassword", - description = "TLS Key Store Password" - ) - private String tlsKeyStorePassword; - - @Schema( - name = "isTlsEnabledWithKeystore", - description = "TLS configuration enabled with key store configs" - ) - private boolean tlsEnabledWithKeystore; - - @Schema( - name = "isServerMode", - description = "Is the SSL Configuration for a Server or Client", - requiredMode = Schema.RequiredMode.REQUIRED - ) - private boolean serverMode; - - @Schema( - name = "isHttps", - description = "Is the SSL Configuration for a Http client or Server" - ) - private boolean isHttps; - - @Override - public PulsarSslConfiguration clone() { - try { - return (PulsarSslConfiguration) super.clone(); - } catch (CloneNotSupportedException e) { - throw new RuntimeException("Failed to clone PulsarSslConfiguration", e); - } - } - -} \ No newline at end of file diff --git a/pulsar-common/src/main/java/org/apache/pulsar/common/util/PulsarSslFactory.java b/pulsar-common/src/main/java/org/apache/pulsar/common/util/PulsarSslFactory.java deleted file mode 100644 index bccbbbe5b2516..0000000000000 --- a/pulsar-common/src/main/java/org/apache/pulsar/common/util/PulsarSslFactory.java +++ /dev/null @@ -1,106 +0,0 @@ -/* - * 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.pulsar.common.util; - -import io.netty.buffer.ByteBufAllocator; -import io.netty.handler.ssl.SslContext; -import javax.net.ssl.SSLContext; -import javax.net.ssl.SSLEngine; - -/** - * Factory for generating SSL Context and SSL Engine using {@link PulsarSslConfiguration}. - */ -public interface PulsarSslFactory extends AutoCloseable { - - /** - * Initializes the PulsarSslFactory. - * @param config {@link PulsarSslConfiguration} object required for initialization - */ - void initialize(PulsarSslConfiguration config); - - /** - * Creates a Client {@link SSLEngine} utilizing {@link ByteBufAllocator} object, the peer hostname, peer port and - * {@link PulsarSslConfiguration} object provided during initialization. - * - * @param buf The ByteBufAllocator required for netty connections. This can be passed as {@code null} if utilized - * for web connections. - * @param peerHost the name of the peer host - * @param peerPort the port number of the peer - * @return {@link SSLEngine} - */ - SSLEngine createClientSslEngine(ByteBufAllocator buf, String peerHost, int peerPort); - - /** - * Creates a Server {@link SSLEngine} utilizing the {@link ByteBufAllocator} object and - * {@link PulsarSslConfiguration} object provided during initialization. - * - * @param buf The ByteBufAllocator required for netty connections. This can be passed as {@code null} if utilized - * for web connections. - * @return {@link SSLEngine} - */ - SSLEngine createServerSslEngine(ByteBufAllocator buf); - - /** - * Returns a boolean value indicating {@link SSLContext} or {@link SslContext} should be refreshed. - * - * @return {@code true} if {@link SSLContext} or {@link SslContext} should be refreshed. - */ - boolean needsUpdate(); - - /** - * Update the internal {@link SSLContext} or {@link SslContext}. - * @throws Exception if there are any issues generating the new {@link SSLContext} or {@link SslContext} - */ - default void update() throws Exception { - if (this.needsUpdate()) { - this.createInternalSslContext(); - } - } - - /** - * Creates the following: - * 1. {@link SslContext} if netty connections are being created for Non-Keystore based TLS configurations. - * 2. {@link SSLContext} if netty connections are being created for Keystore based TLS configurations. It will - * also create it for all web connections irrespective of it being Keystore or Non-Keystore based TLS - * configurations. - * - * @throws Exception if there are any issues creating the new {@link SSLContext} or {@link SslContext} - */ - void createInternalSslContext() throws Exception; - - /** - * Get the internally stored {@link SSLContext}. It will be used in the following scenarios: - * 1. Netty connection creations for keystore based TLS configurations - * 2. All Web connections - * - * @return {@link SSLContext} - * @throws RuntimeException if the {@link SSLContext} object has not yet been initialized. - */ - SSLContext getInternalSslContext(); - - /** - * Get the internally stored {@link SslContext}. It will be used to create Netty Connections for non-keystore based - * tls configurations. - * - * @return {@link SslContext} - * @throws RuntimeException if the {@link SslContext} object has not yet been initialized. - */ - SslContext getInternalNettySslContext(); - -} \ No newline at end of file diff --git a/pulsar-common/src/main/java/org/apache/pulsar/common/util/SecurityUtility.java b/pulsar-common/src/main/java/org/apache/pulsar/common/util/SecurityUtility.java index c8746adba7993..307f930d19c83 100644 --- a/pulsar-common/src/main/java/org/apache/pulsar/common/util/SecurityUtility.java +++ b/pulsar-common/src/main/java/org/apache/pulsar/common/util/SecurityUtility.java @@ -56,7 +56,6 @@ import java.util.Collection; import java.util.List; import java.util.Set; -import java.util.concurrent.ScheduledExecutorService; import javax.net.ssl.HostnameVerifier; import javax.net.ssl.KeyManager; import javax.net.ssl.KeyManagerFactory; @@ -231,42 +230,6 @@ public static SSLContext createSslContext(boolean allowInsecureConnection, Strin return createSslContext(allowInsecureConnection, trustCertificates, certificates, privateKey, providerName); } - /** - * Creates {@link SslContext} with capability to do auto-cert refresh. - * @param allowInsecureConnection - * @param trustCertsFilePath - * @param certFilePath - * @param keyFilePath - * @param sslContextAlgorithm - * @param refreshDurationSec - * @param executor - * @return - * @throws GeneralSecurityException - * @throws SSLException - * @throws FileNotFoundException - * @throws IOException - */ - public static SslContext createAutoRefreshSslContextForClient(SslProvider sslProvider, - boolean allowInsecureConnection, - String trustCertsFilePath, String certFilePath, - String keyFilePath, String sslContextAlgorithm, - int refreshDurationSec, - ScheduledExecutorService executor) - throws GeneralSecurityException, SSLException, FileNotFoundException, IOException { - KeyManagerProxy keyManager = new KeyManagerProxy(certFilePath, keyFilePath, refreshDurationSec, executor); - SslContextBuilder sslContexBuilder = SslContextBuilder.forClient().sslProvider(sslProvider); - sslContexBuilder.keyManager(keyManager); - if (allowInsecureConnection) { - sslContexBuilder.trustManager(InsecureTrustManagerFactory.INSTANCE); - } else { - if (StringUtils.isNotBlank(trustCertsFilePath)) { - TrustManagerProxy trustManager = - new TrustManagerProxy(trustCertsFilePath, refreshDurationSec, executor); - sslContexBuilder.trustManager(trustManager); - } - } - return sslContexBuilder.build(); - } public static SslContext createNettySslContextForClient(SslProvider sslProvider, boolean allowInsecureConnection, String trustCertsFilePath, diff --git a/pulsar-common/src/main/java/org/apache/pulsar/common/util/TrustManagerProxy.java b/pulsar-common/src/main/java/org/apache/pulsar/common/util/TrustManagerProxy.java deleted file mode 100644 index d913987a9e067..0000000000000 --- a/pulsar-common/src/main/java/org/apache/pulsar/common/util/TrustManagerProxy.java +++ /dev/null @@ -1,123 +0,0 @@ -/* - * 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.pulsar.common.util; - -import io.netty.handler.ssl.SslContext; -import java.io.IOException; -import java.net.Socket; -import java.security.KeyManagementException; -import java.security.KeyStore; -import java.security.KeyStoreException; -import java.security.NoSuchAlgorithmException; -import java.security.cert.CertificateException; -import java.security.cert.X509Certificate; -import java.util.concurrent.ScheduledExecutorService; -import java.util.concurrent.TimeUnit; -import javax.net.ssl.SSLEngine; -import javax.net.ssl.TrustManagerFactory; -import javax.net.ssl.X509ExtendedTrustManager; -import lombok.CustomLog; - -/** - * This class wraps {@link X509ExtendedTrustManager} and gives opportunity to refresh Trust-manager with refreshed certs - * without changing {@link SslContext}. - */ -@CustomLog -public class TrustManagerProxy extends X509ExtendedTrustManager { - - private volatile X509ExtendedTrustManager trustManager; - private final FileModifiedTimeUpdater certFile; - - public TrustManagerProxy(String caCertFile, int refreshDurationSec, ScheduledExecutorService executor) { - this.certFile = new FileModifiedTimeUpdater(caCertFile); - try { - updateTrustManager(); - } catch (KeyManagementException | IOException | CertificateException e) { - log.warn().attr("certFile", certFile).exceptionMessage(e).log("Failed to load cert"); - throw new IllegalArgumentException(e); - } catch (NoSuchAlgorithmException | KeyStoreException e) { - log.warn().exception(e).log("Failed to init trust-store"); - throw new IllegalArgumentException(e); - } - executor.scheduleWithFixedDelay(() -> updateTrustManagerSafely(), refreshDurationSec, refreshDurationSec, - TimeUnit.SECONDS); - } - - private void updateTrustManagerSafely() { - try { - updateTrustManager(); - } catch (Exception e) { - log.warn().attr("certFile", certFile.getFileName()).exception(e).log("Failed to init trust-store"); - } - } - - private void updateTrustManager() throws CertificateException, KeyStoreException, NoSuchAlgorithmException, - IOException, KeyManagementException { - KeyStore keyStore = KeyStore.getInstance(KeyStore.getDefaultType()); - keyStore.load(null); - X509Certificate[] certificates = SecurityUtility.loadCertificatesFromPemFile(certFile.getFileName()); - for (X509Certificate certificate : certificates) { - String alias = certificate.getSubjectX500Principal().getName(); - keyStore.setCertificateEntry(alias, certificate); - } - final TrustManagerFactory trustManagerFactory = TrustManagerFactory - .getInstance(TrustManagerFactory.getDefaultAlgorithm()); - trustManagerFactory.init(keyStore); - trustManager = (X509ExtendedTrustManager) trustManagerFactory.getTrustManagers()[0]; - } - - @Override - public void checkClientTrusted(X509Certificate[] x509Certificates, String s) throws CertificateException { - trustManager.checkClientTrusted(x509Certificates, s); - } - - @Override - public void checkServerTrusted(X509Certificate[] x509Certificates, String s) throws CertificateException { - trustManager.checkServerTrusted(x509Certificates, s); - } - - @Override - public X509Certificate[] getAcceptedIssuers() { - return trustManager.getAcceptedIssuers(); - } - - @Override - public void checkClientTrusted(X509Certificate[] chain, String authType, Socket socket) - throws CertificateException { - trustManager.checkClientTrusted(chain, authType, socket); - } - - @Override - public void checkClientTrusted(X509Certificate[] chain, String authType, SSLEngine engine) - throws CertificateException { - trustManager.checkClientTrusted(chain, authType, engine); - } - - @Override - public void checkServerTrusted(X509Certificate[] chain, String authType, Socket socket) - throws CertificateException { - trustManager.checkServerTrusted(chain, authType, socket); - } - - @Override - public void checkServerTrusted(X509Certificate[] chain, String authType, SSLEngine engine) - throws CertificateException { - trustManager.checkServerTrusted(chain, authType, engine); - } -} diff --git a/pulsar-common/src/main/java/org/apache/pulsar/common/util/keystoretls/KeyStoreSSLContext.java b/pulsar-common/src/main/java/org/apache/pulsar/common/util/keystoretls/KeyStoreSSLContext.java deleted file mode 100644 index 1317e354fa9ac..0000000000000 --- a/pulsar-common/src/main/java/org/apache/pulsar/common/util/keystoretls/KeyStoreSSLContext.java +++ /dev/null @@ -1,352 +0,0 @@ -/* - * 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.pulsar.common.util.keystoretls; - -import static org.apache.pulsar.common.util.SecurityUtility.getProvider; -import com.google.common.base.Strings; -import io.netty.handler.ssl.util.InsecureTrustManagerFactory; -import java.io.FileInputStream; -import java.io.IOException; -import java.security.GeneralSecurityException; -import java.security.KeyStore; -import java.security.Provider; -import java.security.SecureRandom; -import java.util.Arrays; -import java.util.HashSet; -import java.util.Set; -import javax.net.ssl.KeyManager; -import javax.net.ssl.KeyManagerFactory; -import javax.net.ssl.SSLContext; -import javax.net.ssl.SSLEngine; -import javax.net.ssl.TrustManager; -import javax.net.ssl.TrustManagerFactory; -import lombok.CustomLog; -import lombok.Getter; -import org.apache.pulsar.common.util.SecurityUtility; - -/** - * KeyStoreSSLContext that mainly wrap a SSLContext to provide SSL context for both webservice and netty. - */ -@CustomLog -public class KeyStoreSSLContext { - public static final String DEFAULT_KEYSTORE_TYPE = "JKS"; - public static final String DEFAULT_SSL_PROTOCOL = "TLS"; - public static final String DEFAULT_SSL_ENABLED_PROTOCOLS = "TLSv1.3,TLSv1.2"; - public static final String DEFAULT_SSL_KEYMANGER_ALGORITHM = KeyManagerFactory.getDefaultAlgorithm(); - public static final String DEFAULT_SSL_TRUSTMANAGER_ALGORITHM = TrustManagerFactory.getDefaultAlgorithm(); - - public static final Provider BC_PROVIDER = getProvider(); - - /** - * Connection Mode for TLS. - */ - public enum Mode { - CLIENT, - SERVER - } - - @Getter - private final Mode mode; - - private final String sslProviderString; - private final String keyStoreTypeString; - private final String keyStorePath; - private final String keyStorePassword; - private final boolean allowInsecureConnection; - private final String trustStoreTypeString; - private final String trustStorePath; - private final String trustStorePassword; - private final boolean needClientAuth; - private final Set ciphers; - private final Set protocols; - private SSLContext sslContext; - - private final String protocol = DEFAULT_SSL_PROTOCOL; - private final String kmfAlgorithm = DEFAULT_SSL_KEYMANGER_ALGORITHM; - private final String tmfAlgorithm = DEFAULT_SSL_TRUSTMANAGER_ALGORITHM; - - // only init vars, before using it, need to call createSSLContext to create ssl context. - public KeyStoreSSLContext(Mode mode, - String sslProviderString, - String keyStoreTypeString, - String keyStorePath, - String keyStorePassword, - boolean allowInsecureConnection, - String trustStoreTypeString, - String trustStorePath, - String trustStorePassword, - boolean requireTrustedClientCertOnConnect, - Set ciphers, - Set protocols) { - this.mode = mode; - this.sslProviderString = sslProviderString; - this.keyStoreTypeString = Strings.isNullOrEmpty(keyStoreTypeString) - ? DEFAULT_KEYSTORE_TYPE - : keyStoreTypeString; - this.keyStorePath = keyStorePath; - this.keyStorePassword = keyStorePassword; - this.trustStoreTypeString = Strings.isNullOrEmpty(trustStoreTypeString) - ? DEFAULT_KEYSTORE_TYPE - : trustStoreTypeString; - this.trustStorePath = trustStorePath; - if (trustStorePassword == null) { - this.trustStorePassword = ""; - } else { - this.trustStorePassword = trustStorePassword; - } - this.needClientAuth = requireTrustedClientCertOnConnect; - - if (protocols != null && protocols.size() > 0) { - this.protocols = protocols; - } else { - this.protocols = new HashSet<>(Arrays.asList(DEFAULT_SSL_ENABLED_PROTOCOLS.split("\\s*,\\s*"))); - } - - if (ciphers != null && ciphers.size() > 0) { - this.ciphers = ciphers; - } else { - this.ciphers = null; - } - - this.allowInsecureConnection = allowInsecureConnection; - } - - public SSLContext createSSLContext() throws GeneralSecurityException, IOException { - SSLContext sslContext; - - Provider provider = SecurityUtility.resolveProvider(sslProviderString); - if (provider != null) { - sslContext = SSLContext.getInstance(protocol, provider); - } else { - sslContext = SSLContext.getInstance(protocol); - } - - // key store - KeyManager[] keyManagers = null; - if (!Strings.isNullOrEmpty(keyStorePath)) { - KeyManagerFactory keyManagerFactory = KeyManagerFactory.getInstance(kmfAlgorithm); - KeyStore keyStore = KeyStore.getInstance(keyStoreTypeString); - char[] passwordChars = keyStorePassword.toCharArray(); - try (FileInputStream inputStream = new FileInputStream(keyStorePath)) { - keyStore.load(inputStream, passwordChars); - } - keyManagerFactory.init(keyStore, passwordChars); - keyManagers = keyManagerFactory.getKeyManagers(); - } - - // trust store - TrustManagerFactory trustManagerFactory = null; - if (this.allowInsecureConnection) { - trustManagerFactory = InsecureTrustManagerFactory.INSTANCE; - } else { - if (!Strings.isNullOrEmpty(trustStorePath)) { - trustManagerFactory = provider != null - ? TrustManagerFactory.getInstance(tmfAlgorithm, provider) - : TrustManagerFactory.getInstance(tmfAlgorithm); - KeyStore trustStore = KeyStore.getInstance(trustStoreTypeString); - char[] passwordChars = trustStorePassword.toCharArray(); - try (FileInputStream inputStream = new FileInputStream(trustStorePath)) { - trustStore.load(inputStream, passwordChars); - } - trustManagerFactory.init(trustStore); - } - } - - TrustManager[] trustManagers = null; - if (trustManagerFactory != null) { - trustManagers = trustManagerFactory.getTrustManagers(); - } - - // init - sslContext.init(keyManagers, trustManagers, new SecureRandom()); - this.sslContext = sslContext; - return sslContext; - } - - public SSLContext getSslContext() { - if (sslContext == null) { - throw new IllegalStateException("createSSLContext hasn't been called."); - } - return sslContext; - } - - public SSLEngine createSSLEngine() { - return configureSSLEngine(getSslContext().createSSLEngine()); - } - - public SSLEngine createSSLEngine(String peerHost, int peerPort) { - return configureSSLEngine(getSslContext().createSSLEngine(peerHost, peerPort)); - } - - private SSLEngine configureSSLEngine(SSLEngine sslEngine) { - sslEngine.setEnabledProtocols(protocols.toArray(new String[0])); - if (this.ciphers != null) { - sslEngine.setEnabledCipherSuites(this.ciphers.toArray(new String[0])); - } - - if (this.mode == Mode.SERVER) { - if (needClientAuth) { - sslEngine.setNeedClientAuth(true); - } else { - sslEngine.setWantClientAuth(true); - } - sslEngine.setUseClientMode(false); - } else { - sslEngine.setUseClientMode(true); - } - return sslEngine; - } - - public static KeyStoreSSLContext createClientKeyStoreSslContext(String sslProviderString, - String keyStoreTypeString, - String keyStorePath, - String keyStorePassword, - boolean allowInsecureConnection, - String trustStoreTypeString, - String trustStorePath, - String trustStorePassword, - Set ciphers, - Set protocols) - throws GeneralSecurityException, IOException { - KeyStoreSSLContext keyStoreSSLContext = new KeyStoreSSLContext(Mode.CLIENT, - sslProviderString, - keyStoreTypeString, - keyStorePath, - keyStorePassword, - allowInsecureConnection, - trustStoreTypeString, - trustStorePath, - trustStorePassword, - false, - ciphers, - protocols); - - keyStoreSSLContext.createSSLContext(); - return keyStoreSSLContext; - } - - - public static KeyStoreSSLContext createServerKeyStoreSslContext(String sslProviderString, - String keyStoreTypeString, - String keyStorePath, - String keyStorePassword, - boolean allowInsecureConnection, - String trustStoreTypeString, - String trustStorePath, - String trustStorePassword, - boolean requireTrustedClientCertOnConnect, - Set ciphers, - Set protocols) - throws GeneralSecurityException, IOException { - KeyStoreSSLContext keyStoreSSLContext = new KeyStoreSSLContext(Mode.SERVER, - sslProviderString, - keyStoreTypeString, - keyStorePath, - keyStorePassword, - allowInsecureConnection, - trustStoreTypeString, - trustStorePath, - trustStorePassword, - requireTrustedClientCertOnConnect, - ciphers, - protocols); - - keyStoreSSLContext.createSSLContext(); - return keyStoreSSLContext; - } - - // the web server only use this method to get SSLContext, it won't use this to configure engine - // no need ciphers and protocols - public static SSLContext createServerSslContext(String sslProviderString, - String keyStoreTypeString, - String keyStorePath, - String keyStorePassword, - boolean allowInsecureConnection, - String trustStoreTypeString, - String trustStorePath, - String trustStorePassword, - boolean requireTrustedClientCertOnConnect) - throws GeneralSecurityException, IOException { - - return createServerKeyStoreSslContext( - sslProviderString, - keyStoreTypeString, - keyStorePath, - keyStorePassword, - allowInsecureConnection, - trustStoreTypeString, - trustStorePath, - trustStorePassword, - requireTrustedClientCertOnConnect, - null, - null).getSslContext(); - } - - // for web client - public static SSLContext createClientSslContext(String sslProviderString, - String keyStoreTypeString, - String keyStorePath, - String keyStorePassword, - boolean allowInsecureConnection, - String trustStoreTypeString, - String trustStorePath, - String trustStorePassword, - Set ciphers, - Set protocol) - throws GeneralSecurityException, IOException { - KeyStoreSSLContext keyStoreSSLContext = new KeyStoreSSLContext(Mode.CLIENT, - sslProviderString, - keyStoreTypeString, - keyStorePath, - keyStorePassword, - allowInsecureConnection, - trustStoreTypeString, - trustStorePath, - trustStorePassword, - false, - ciphers, - protocol); - - return keyStoreSSLContext.createSSLContext(); - } - - // for web client - public static SSLContext createClientSslContext(String keyStoreTypeString, - String keyStorePath, - String keyStorePassword, - String trustStoreTypeString, - String trustStorePath, - String trustStorePassword) - throws GeneralSecurityException, IOException { - KeyStoreSSLContext keyStoreSSLContext = new KeyStoreSSLContext(Mode.CLIENT, - null, - keyStoreTypeString, - keyStorePath, - keyStorePassword, - false, - trustStoreTypeString, - trustStorePath, - trustStorePassword, - false, - null, - null); - - return keyStoreSSLContext.createSSLContext(); - } -} diff --git a/pulsar-common/src/main/java/org/apache/pulsar/common/util/keystoretls/SSLContextValidatorEngine.java b/pulsar-common/src/main/java/org/apache/pulsar/common/util/keystoretls/SSLContextValidatorEngine.java deleted file mode 100644 index a69a18c21d97f..0000000000000 --- a/pulsar-common/src/main/java/org/apache/pulsar/common/util/keystoretls/SSLContextValidatorEngine.java +++ /dev/null @@ -1,166 +0,0 @@ -/* - * 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.pulsar.common.util.keystoretls; - -import static javax.net.ssl.SSLEngineResult.HandshakeStatus.FINISHED; -import java.nio.ByteBuffer; -import java.util.Arrays; -import javax.net.ssl.SSLEngine; -import javax.net.ssl.SSLEngineResult; -import javax.net.ssl.SSLException; -import lombok.CustomLog; - -/** - * SSLContextValidatorEngine to validate 2 SSlContext. - */ -@CustomLog -public class SSLContextValidatorEngine { - @FunctionalInterface - public interface SSLEngineProvider { - SSLEngine createSSLEngine(String peerHost, int peerPort); - } - - private static final ByteBuffer EMPTY_BUF = ByteBuffer.allocate(0); - private final SSLEngine sslEngine; - private SSLEngineResult handshakeResult; - private ByteBuffer appBuffer; - private ByteBuffer netBuffer; - private boolean finished = false; - - /** - * Validates TLS handshake up to TLSv1.2. - * TLSv1.3 has a differences in TLS handshake as described in https://stackoverflow.com/a/62465859 - */ - public static void validate(SSLEngineProvider clientSslEngineSupplier, SSLEngineProvider serverSslEngineSupplier) - throws SSLException { - SSLContextValidatorEngine clientEngine = new SSLContextValidatorEngine(clientSslEngineSupplier); - if (Arrays.stream(clientEngine.sslEngine.getEnabledProtocols()).anyMatch(s -> s.equals("TLSv1.3"))) { - throw new IllegalStateException("This validator doesn't support TLSv1.3"); - } - SSLContextValidatorEngine serverEngine = new SSLContextValidatorEngine(serverSslEngineSupplier); - try { - clientEngine.beginHandshake(); - serverEngine.beginHandshake(); - while (!serverEngine.complete() || !clientEngine.complete()) { - clientEngine.handshake(serverEngine); - serverEngine.handshake(clientEngine); - } - } finally { - clientEngine.close(); - serverEngine.close(); - } - } - - private SSLContextValidatorEngine(SSLEngineProvider sslEngineSupplier) { - this.sslEngine = sslEngineSupplier.createSSLEngine("localhost", 0); - appBuffer = ByteBuffer.allocate(sslEngine.getSession().getApplicationBufferSize()); - netBuffer = ByteBuffer.allocate(sslEngine.getSession().getPacketBufferSize()); - } - - void beginHandshake() throws SSLException { - sslEngine.beginHandshake(); - } - - void handshake(SSLContextValidatorEngine peerEngine) throws SSLException { - SSLEngineResult.HandshakeStatus handshakeStatus = sslEngine.getHandshakeStatus(); - while (true) { - switch (handshakeStatus) { - case NEED_WRAP: - handshakeResult = sslEngine.wrap(EMPTY_BUF, netBuffer); - switch (handshakeResult.getStatus()) { - case OK: break; - case BUFFER_OVERFLOW: - netBuffer.compact(); - netBuffer = ensureCapacity(netBuffer, sslEngine.getSession().getPacketBufferSize()); - netBuffer.flip(); - break; - case BUFFER_UNDERFLOW: - case CLOSED: - default: - throw new SSLException("Unexpected handshake status: " + handshakeResult.getStatus()); - } - return; - case NEED_UNWRAP: - if (peerEngine.netBuffer.position() == 0) { - return; - } - peerEngine.netBuffer.flip(); // unwrap the data from peer - handshakeResult = sslEngine.unwrap(peerEngine.netBuffer, appBuffer); - peerEngine.netBuffer.compact(); - handshakeStatus = handshakeResult.getHandshakeStatus(); - switch (handshakeResult.getStatus()) { - case OK: break; - case BUFFER_OVERFLOW: - appBuffer = ensureCapacity(appBuffer, sslEngine.getSession().getApplicationBufferSize()); - break; - case BUFFER_UNDERFLOW: - netBuffer = ensureCapacity(netBuffer, sslEngine.getSession().getPacketBufferSize()); - break; - case CLOSED: - default: - throw new SSLException("Unexpected handshake status: " + handshakeResult.getStatus()); - } - break; - case NEED_TASK: - sslEngine.getDelegatedTask().run(); - handshakeStatus = sslEngine.getHandshakeStatus(); - break; - case FINISHED: - return; - case NOT_HANDSHAKING: - if (handshakeResult.getHandshakeStatus() != FINISHED) { - throw new SSLException("Did not finish handshake"); - } - finished = true; - return; - default: - throw new IllegalStateException("Unexpected handshake status " + handshakeStatus); - } - } - } - - boolean complete() { - return finished; - } - - void close() { - sslEngine.closeOutbound(); - try { - sslEngine.closeInbound(); - } catch (Exception e) { - // ignore - } - } - - /** - * Check if the given ByteBuffer capacity. - * @param existingBuffer ByteBuffer capacity to check - * @param newLength new length for the ByteBuffer. - * returns ByteBuffer - */ - public static ByteBuffer ensureCapacity(ByteBuffer existingBuffer, int newLength) { - if (newLength > existingBuffer.capacity()) { - ByteBuffer newBuffer = ByteBuffer.allocate(newLength); - existingBuffer.flip(); - newBuffer.put(existingBuffer); - return newBuffer; - } - return existingBuffer; - } -} diff --git a/pulsar-common/src/main/java/org/apache/pulsar/common/util/keystoretls/package-info.java b/pulsar-common/src/main/java/org/apache/pulsar/common/util/keystoretls/package-info.java deleted file mode 100644 index a9b27bb759501..0000000000000 --- a/pulsar-common/src/main/java/org/apache/pulsar/common/util/keystoretls/package-info.java +++ /dev/null @@ -1,23 +0,0 @@ -/* - * 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. - */ - -/** - * Helpers to work with events from the non-blocking I/O client-server framework. - */ -package org.apache.pulsar.common.util.keystoretls; diff --git a/pulsar-common/src/test/java/org/apache/pulsar/common/util/DefaultPulsarSslFactoryTest.java b/pulsar-common/src/test/java/org/apache/pulsar/common/util/DefaultPulsarSslFactoryTest.java deleted file mode 100644 index 010c57aef15f4..0000000000000 --- a/pulsar-common/src/test/java/org/apache/pulsar/common/util/DefaultPulsarSslFactoryTest.java +++ /dev/null @@ -1,282 +0,0 @@ -/* - * 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.pulsar.common.util; - -import static org.testng.Assert.assertEquals; -import static org.testng.Assert.assertNotNull; -import static org.testng.Assert.assertThrows; -import com.google.common.io.Resources; -import io.netty.buffer.ByteBufAllocator; -import io.netty.handler.ssl.OpenSslEngine; -import java.io.File; -import java.util.Arrays; -import java.util.HashSet; -import java.util.Set; -import javax.net.ssl.SSLEngine; -import org.testng.annotations.Test; - -public class DefaultPulsarSslFactoryTest { - - public static final String KEYSTORE_FILE_PATH = - getAbsolutePath("certificate-authority/jks/broker.keystore.jks"); - public static final String TRUSTSTORE_FILE_PATH = - getAbsolutePath("certificate-authority/jks/broker.truststore.jks"); - public static final String TRUSTSTORE_NO_PASSWORD_FILE_PATH = - getAbsolutePath("certificate-authority/jks/broker.truststore.nopassword.jks"); - public static final String KEYSTORE_PW = "111111"; - public static final String TRUSTSTORE_PW = "111111"; - public static final String KEYSTORE_TYPE = "JKS"; - - public static final String CA_CERT_FILE_PATH = - getAbsolutePath("certificate-authority/certs/ca.cert.pem"); - public static final String CERT_FILE_PATH = - getAbsolutePath("certificate-authority/server-keys/broker.cert.pem"); - public static final String KEY_FILE_PATH = - getAbsolutePath("certificate-authority/server-keys/broker.key-pk8.pem"); - - @Test - public void sslContextCreationUsingKeystoreTest() throws Exception { - PulsarSslConfiguration pulsarSslConfiguration = PulsarSslConfiguration.builder() - .tlsEnabledWithKeystore(true) - .tlsKeyStoreType(KEYSTORE_TYPE) - .tlsKeyStorePath(KEYSTORE_FILE_PATH) - .tlsKeyStorePassword(KEYSTORE_PW) - .tlsTrustStorePath(TRUSTSTORE_FILE_PATH) - .tlsTrustStorePassword(TRUSTSTORE_PW) - .build(); - PulsarSslFactory pulsarSslFactory = new DefaultPulsarSslFactory(); - pulsarSslFactory.initialize(pulsarSslConfiguration); - pulsarSslFactory.createInternalSslContext(); - assertNotNull(pulsarSslFactory.getInternalSslContext()); - assertThrows(RuntimeException.class, pulsarSslFactory::getInternalNettySslContext); - } - - @Test - public void sslContextCreationUsingPasswordLessTruststoreTest() throws Exception { - PulsarSslConfiguration pulsarSslConfiguration = PulsarSslConfiguration.builder() - .tlsEnabledWithKeystore(true) - .tlsKeyStoreType(KEYSTORE_TYPE) - .tlsKeyStorePath(KEYSTORE_FILE_PATH) - .tlsKeyStorePassword(KEYSTORE_PW) - .tlsTrustStorePath(TRUSTSTORE_NO_PASSWORD_FILE_PATH) - .build(); - PulsarSslFactory pulsarSslFactory = new DefaultPulsarSslFactory(); - pulsarSslFactory.initialize(pulsarSslConfiguration); - pulsarSslFactory.createInternalSslContext(); - assertNotNull(pulsarSslFactory.getInternalSslContext()); - assertThrows(RuntimeException.class, pulsarSslFactory::getInternalNettySslContext); - } - - @Test - public void sslContextCreationUsingTlsCertsTest() throws Exception { - PulsarSslConfiguration pulsarSslConfiguration = PulsarSslConfiguration.builder() - .tlsCertificateFilePath(CERT_FILE_PATH) - .tlsKeyFilePath(KEY_FILE_PATH) - .tlsTrustCertsFilePath(CA_CERT_FILE_PATH) - .build(); - PulsarSslFactory pulsarSslFactory = new DefaultPulsarSslFactory(); - pulsarSslFactory.initialize(pulsarSslConfiguration); - pulsarSslFactory.createInternalSslContext(); - assertNotNull(pulsarSslFactory.getInternalNettySslContext()); - assertThrows(RuntimeException.class, pulsarSslFactory::getInternalSslContext); - } - - @Test - public void sslContextCreationUsingOnlyCACertsTest() throws Exception { - PulsarSslConfiguration pulsarSslConfiguration = PulsarSslConfiguration.builder() - .tlsTrustCertsFilePath(CA_CERT_FILE_PATH) - .build(); - PulsarSslFactory pulsarSslFactory = new DefaultPulsarSslFactory(); - pulsarSslFactory.initialize(pulsarSslConfiguration); - pulsarSslFactory.createInternalSslContext(); - assertNotNull(pulsarSslFactory.getInternalNettySslContext()); - assertThrows(RuntimeException.class, pulsarSslFactory::getInternalSslContext); - } - - @Test - public void sslContextCreationForWebClientConnections() throws Exception { - PulsarSslConfiguration pulsarSslConfiguration = PulsarSslConfiguration.builder() - .tlsCertificateFilePath(CERT_FILE_PATH) - .tlsKeyFilePath(KEY_FILE_PATH) - .tlsTrustCertsFilePath(CA_CERT_FILE_PATH) - .isHttps(true) - .build(); - PulsarSslFactory pulsarSslFactory = new DefaultPulsarSslFactory(); - pulsarSslFactory.initialize(pulsarSslConfiguration); - pulsarSslFactory.createInternalSslContext(); - assertNotNull(pulsarSslFactory.getInternalSslContext()); - assertThrows(RuntimeException.class, pulsarSslFactory::getInternalNettySslContext); - } - - @Test - public void sslContextCreationForWebServerConnectionsTest() throws Exception { - PulsarSslConfiguration pulsarSslConfiguration = PulsarSslConfiguration.builder() - .tlsCertificateFilePath(CERT_FILE_PATH) - .tlsKeyFilePath(KEY_FILE_PATH) - .tlsTrustCertsFilePath(CA_CERT_FILE_PATH) - .isHttps(true) - .serverMode(true) - .build(); - PulsarSslFactory pulsarSslFactory = new DefaultPulsarSslFactory(); - pulsarSslFactory.initialize(pulsarSslConfiguration); - pulsarSslFactory.createInternalSslContext(); - assertNotNull(pulsarSslFactory.getInternalSslContext()); - assertThrows(RuntimeException.class, pulsarSslFactory::getInternalNettySslContext); - } - - @Test - public void sslEngineCreationWithEnabledProtocolsAndCiphersForOpenSSLTest() throws Exception { - Set ciphers = new HashSet<>(); - ciphers.add("TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256"); - ciphers.add("TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256"); - Set protocols = new HashSet<>(); - protocols.add("TLSv1.2"); - protocols.add("TLSv1"); - PulsarSslConfiguration pulsarSslConfiguration = PulsarSslConfiguration.builder() - .tlsCertificateFilePath(CERT_FILE_PATH) - .tlsKeyFilePath(KEY_FILE_PATH) - .tlsTrustCertsFilePath(CA_CERT_FILE_PATH) - .tlsCiphers(ciphers) - .tlsProtocols(protocols) - .serverMode(true) - .build(); - PulsarSslFactory pulsarSslFactory = new DefaultPulsarSslFactory(); - pulsarSslFactory.initialize(pulsarSslConfiguration); - pulsarSslFactory.createInternalSslContext(); - assertNotNull(pulsarSslFactory.getInternalNettySslContext()); - assertThrows(RuntimeException.class, pulsarSslFactory::getInternalSslContext); - SSLEngine sslEngine = pulsarSslFactory.createServerSslEngine(ByteBufAllocator.DEFAULT); - /* Adding SSLv2Hello protocol only during expected checks as Netty adds it as part of the - ReferenceCountedOpenSslEngine's setEnabledProtocols method. The reasoning is that OpenSSL currently has no - way to disable this protocol. - */ - protocols.add("SSLv2Hello"); - assertEquals(new HashSet<>(Arrays.asList(sslEngine.getEnabledProtocols())), protocols); - assertEquals(new HashSet<>(Arrays.asList(sslEngine.getEnabledCipherSuites())), ciphers); - assert(!sslEngine.getUseClientMode()); - } - - @Test - public void sslEngineCreationWithEnabledProtocolsAndCiphersForWebTest() throws Exception { - Set ciphers = new HashSet<>(); - ciphers.add("TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256"); - ciphers.add("TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256"); - Set protocols = new HashSet<>(); - protocols.add("TLSv1.2"); - protocols.add("TLSv1"); - PulsarSslConfiguration pulsarSslConfiguration = PulsarSslConfiguration.builder() - .tlsCertificateFilePath(CERT_FILE_PATH) - .tlsKeyFilePath(KEY_FILE_PATH) - .tlsTrustCertsFilePath(CA_CERT_FILE_PATH) - .tlsCiphers(ciphers) - .tlsProtocols(protocols) - .isHttps(true) - .serverMode(true) - .build(); - PulsarSslFactory pulsarSslFactory = new DefaultPulsarSslFactory(); - pulsarSslFactory.initialize(pulsarSslConfiguration); - pulsarSslFactory.createInternalSslContext(); - assertNotNull(pulsarSslFactory.getInternalSslContext()); - assertThrows(RuntimeException.class, pulsarSslFactory::getInternalNettySslContext); - SSLEngine sslEngine = pulsarSslFactory.createServerSslEngine(ByteBufAllocator.DEFAULT); - assertEquals(new HashSet<>(Arrays.asList(sslEngine.getEnabledProtocols())), protocols); - assertEquals(new HashSet<>(Arrays.asList(sslEngine.getEnabledCipherSuites())), ciphers); - assert(!sslEngine.getUseClientMode()); - } - - @Test - public void sslContextCreationAsOpenSslTlsProvider() throws Exception { - PulsarSslConfiguration pulsarSslConfiguration = PulsarSslConfiguration.builder() - .tlsProvider("OPENSSL") - .tlsCertificateFilePath(CERT_FILE_PATH) - .tlsKeyFilePath(KEY_FILE_PATH) - .tlsTrustCertsFilePath(CA_CERT_FILE_PATH) - .serverMode(true) - .build(); - PulsarSslFactory pulsarSslFactory = new DefaultPulsarSslFactory(); - pulsarSslFactory.initialize(pulsarSslConfiguration); - pulsarSslFactory.createInternalSslContext(); - assertNotNull(pulsarSslFactory.getInternalNettySslContext()); - assertThrows(RuntimeException.class, pulsarSslFactory::getInternalSslContext); - SSLEngine sslEngine = pulsarSslFactory.createServerSslEngine(ByteBufAllocator.DEFAULT); - assert(sslEngine instanceof OpenSslEngine); - } - - @Test - public void sslContextCreationAsJDKTlsProvider() throws Exception { - PulsarSslConfiguration pulsarSslConfiguration = PulsarSslConfiguration.builder() - .tlsProvider("JDK") - .tlsCertificateFilePath(CERT_FILE_PATH) - .tlsKeyFilePath(KEY_FILE_PATH) - .tlsTrustCertsFilePath(CA_CERT_FILE_PATH) - .serverMode(true) - .build(); - PulsarSslFactory pulsarSslFactory = new DefaultPulsarSslFactory(); - pulsarSslFactory.initialize(pulsarSslConfiguration); - pulsarSslFactory.createInternalSslContext(); - assertNotNull(pulsarSslFactory.getInternalNettySslContext()); - assertThrows(RuntimeException.class, pulsarSslFactory::getInternalSslContext); - SSLEngine sslEngine = pulsarSslFactory.createServerSslEngine(ByteBufAllocator.DEFAULT); - assert (!(sslEngine instanceof OpenSslEngine)); - } - - @Test - public void sslEngineMutualAuthEnabledTest() throws Exception { - PulsarSslConfiguration pulsarSslConfiguration = PulsarSslConfiguration.builder() - .tlsProvider("JDK") - .tlsCertificateFilePath(CERT_FILE_PATH) - .tlsKeyFilePath(KEY_FILE_PATH) - .tlsTrustCertsFilePath(CA_CERT_FILE_PATH) - .requireTrustedClientCertOnConnect(true) - .serverMode(true) - .build(); - PulsarSslFactory pulsarSslFactory = new DefaultPulsarSslFactory(); - pulsarSslFactory.initialize(pulsarSslConfiguration); - pulsarSslFactory.createInternalSslContext(); - assertNotNull(pulsarSslFactory.getInternalNettySslContext()); - assertThrows(RuntimeException.class, pulsarSslFactory::getInternalSslContext); - SSLEngine sslEngine = pulsarSslFactory.createServerSslEngine(ByteBufAllocator.DEFAULT); - assert(sslEngine.getNeedClientAuth()); - } - - @Test - public void sslEngineSniClientTest() throws Exception { - PulsarSslConfiguration pulsarSslConfiguration = PulsarSslConfiguration.builder() - .tlsCertificateFilePath(CERT_FILE_PATH) - .tlsKeyFilePath(KEY_FILE_PATH) - .tlsTrustCertsFilePath(CA_CERT_FILE_PATH) - .build(); - PulsarSslFactory pulsarSslFactory = new DefaultPulsarSslFactory(); - pulsarSslFactory.initialize(pulsarSslConfiguration); - pulsarSslFactory.createInternalSslContext(); - assertNotNull(pulsarSslFactory.getInternalNettySslContext()); - assertThrows(RuntimeException.class, pulsarSslFactory::getInternalSslContext); - SSLEngine sslEngine = pulsarSslFactory.createClientSslEngine(ByteBufAllocator.DEFAULT, "localhost", - 1234); - assertEquals(sslEngine.getPeerHost(), "localhost"); - assertEquals(sslEngine.getPeerPort(), 1234); - } - - - - private static String getAbsolutePath(String resourceName) { - return new File(Resources.getResource(resourceName).getPath()).getAbsolutePath(); - } - -} diff --git a/pulsar-common/src/test/java/org/apache/pulsar/common/util/KeyManagerProxyTest.java b/pulsar-common/src/test/java/org/apache/pulsar/common/util/KeyManagerProxyTest.java deleted file mode 100644 index 5542f0b22ac95..0000000000000 --- a/pulsar-common/src/test/java/org/apache/pulsar/common/util/KeyManagerProxyTest.java +++ /dev/null @@ -1,51 +0,0 @@ -/* - * 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.pulsar.common.util; - -import static org.testng.Assert.assertEquals; -import com.google.common.io.Resources; -import java.util.concurrent.Executors; -import java.util.concurrent.ScheduledExecutorService; -import lombok.Cleanup; -import org.testng.annotations.DataProvider; -import org.testng.annotations.Test; - -public class KeyManagerProxyTest { - - @DataProvider(name = "certDataProvider") - public static Object[][] caDataProvider() { - return new Object[][]{ - {"ca/multiple-ca.pem", 2}, - {"ca/single-ca.pem", 1} - }; - } - - @Test(dataProvider = "certDataProvider") - public void testLoadCert(String path, int certCount) { - final String certFilePath = Resources.getResource(path).getPath(); - // This key is not paired with certs, but this is not a problem as the key is not used in this test - final String keyFilePath = Resources.getResource("ssl/my-ca/client-key.pem").getPath(); - @Cleanup("shutdownNow") - final ScheduledExecutorService scheduledExecutor = Executors.newSingleThreadScheduledExecutor(); - - final KeyManagerProxy keyManager = new KeyManagerProxy(certFilePath, keyFilePath, 60, scheduledExecutor); - assertEquals(keyManager.getCertificateChain("cn=test1").length, certCount); - } -} diff --git a/pulsar-common/src/test/java/org/apache/pulsar/common/util/TrustManagerProxyTest.java b/pulsar-common/src/test/java/org/apache/pulsar/common/util/TrustManagerProxyTest.java deleted file mode 100644 index ab31740bd5f11..0000000000000 --- a/pulsar-common/src/test/java/org/apache/pulsar/common/util/TrustManagerProxyTest.java +++ /dev/null @@ -1,53 +0,0 @@ -/* - * 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.pulsar.common.util; - -import static org.testng.Assert.assertEquals; -import static org.testng.Assert.assertNotNull; -import com.google.common.io.Resources; -import java.security.cert.X509Certificate; -import java.util.Arrays; -import java.util.concurrent.Executors; -import java.util.concurrent.ScheduledExecutorService; -import lombok.Cleanup; -import org.testng.annotations.DataProvider; -import org.testng.annotations.Test; - -public class TrustManagerProxyTest { - @DataProvider(name = "caDataProvider") - public static Object[][] caDataProvider() { - return new Object[][]{ - {"ca/multiple-ca.pem", 2}, - {"ca/single-ca.pem", 1} - }; - } - - @Test(dataProvider = "caDataProvider") - public void testLoadCA(String path, int count) { - String caPath = Resources.getResource(path).getPath(); - - @Cleanup("shutdownNow") - ScheduledExecutorService scheduledExecutor = Executors.newSingleThreadScheduledExecutor(); - TrustManagerProxy trustManagerProxy = - new TrustManagerProxy(caPath, 120, scheduledExecutor); - X509Certificate[] x509Certificates = trustManagerProxy.getAcceptedIssuers(); - assertNotNull(x509Certificates); - assertEquals(Arrays.stream(x509Certificates).count(), count); - } -} diff --git a/pulsar-common/src/test/java/org/apache/pulsar/common/util/netty/SslContextTest.java b/pulsar-common/src/test/java/org/apache/pulsar/common/util/netty/SslContextTest.java deleted file mode 100644 index 8dc9b1de95aea..0000000000000 --- a/pulsar-common/src/test/java/org/apache/pulsar/common/util/netty/SslContextTest.java +++ /dev/null @@ -1,179 +0,0 @@ -/* - * 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.pulsar.common.util.netty; - -import static org.testng.Assert.assertThrows; -import com.google.common.io.Resources; -import io.netty.handler.ssl.SslProvider; -import java.util.HashSet; -import java.util.Set; -import javax.net.ssl.SSLException; -import org.apache.pulsar.client.api.AuthenticationDataProvider; -import org.apache.pulsar.client.api.KeyStoreParams; -import org.apache.pulsar.common.util.DefaultPulsarSslFactory; -import org.apache.pulsar.common.util.PulsarSslConfiguration; -import org.apache.pulsar.common.util.PulsarSslFactory; -import org.testng.annotations.DataProvider; -import org.testng.annotations.Test; - -public class SslContextTest { - static final String BROKER_KEY_STORE_PATH = - Resources.getResource("certificate-authority/jks/broker.keystore.jks").getPath(); - static final String BROKER_TRUST_STORE_PATH = - Resources.getResource("certificate-authority/jks/broker.truststore.jks").getPath(); - static final String KEY_STORE_TYPE = "JKS"; - static final String KEY_STORE_PASSWORD = "111111"; - - static final String CA_CERT_PATH = Resources.getResource("certificate-authority/certs/ca.cert.pem").getPath(); - static final String BROKER_CERT_PATH = - Resources.getResource("certificate-authority/server-keys/broker.cert.pem").getPath(); - static final String BROKER_KEY_PATH = - Resources.getResource("certificate-authority/server-keys/broker.key-pk8.pem").getPath(); - - @DataProvider(name = "caCertSslContextDataProvider") - public static Object[][] getSslContextDataProvider() { - Set ciphers = new HashSet<>(); - ciphers.add("TLS_DHE_RSA_WITH_AES_256_GCM_SHA384"); - ciphers.add("TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256"); - ciphers.add("TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256"); - ciphers.add("TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384"); - ciphers.add("TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384"); - - // Note: OPENSSL doesn't support these ciphers. - return new Object[][]{ - new Object[]{SslProvider.JDK, ciphers}, - new Object[]{SslProvider.JDK, null}, - - new Object[]{SslProvider.OPENSSL, ciphers}, - new Object[]{SslProvider.OPENSSL, null}, - - new Object[]{null, ciphers}, - new Object[]{null, null}, - }; - } - - @DataProvider(name = "cipherDataProvider") - public static Object[] getCipher() { - Set cipher = new HashSet<>(); - cipher.add("TLS_DHE_RSA_WITH_AES_256_GCM_SHA384"); - cipher.add("TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256"); - cipher.add("TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256"); - cipher.add("TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384"); - cipher.add("TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384"); - - return new Object[]{null, cipher}; - } - - @Test(dataProvider = "cipherDataProvider") - @SuppressWarnings("try") - public void testServerKeyStoreSSLContext(Set cipher) throws Exception { - PulsarSslConfiguration pulsarSslConfiguration = PulsarSslConfiguration.builder() - .tlsEnabledWithKeystore(true) - .tlsKeyStoreType(KEY_STORE_TYPE) - .tlsKeyStorePath(BROKER_KEY_STORE_PATH) - .tlsKeyStorePassword(KEY_STORE_PASSWORD) - .allowInsecureConnection(false) - .tlsTrustStoreType(KEY_STORE_TYPE) - .tlsTrustStorePath(BROKER_TRUST_STORE_PATH) - .tlsTrustStorePassword(KEY_STORE_PASSWORD) - .requireTrustedClientCertOnConnect(true) - .tlsCiphers(cipher) - .build(); - try (PulsarSslFactory pulsarSslFactory = new DefaultPulsarSslFactory()) { - pulsarSslFactory.initialize(pulsarSslConfiguration); - pulsarSslFactory.createInternalSslContext(); - } - } - - private static class ClientAuthenticationData implements AuthenticationDataProvider { - @Override - public KeyStoreParams getTlsKeyStoreParams() { - return null; - } - } - - @Test(dataProvider = "cipherDataProvider") - @SuppressWarnings("try") - public void testClientKeyStoreSSLContext(Set cipher) throws Exception { - PulsarSslConfiguration pulsarSslConfiguration = PulsarSslConfiguration.builder() - .allowInsecureConnection(false) - .tlsEnabledWithKeystore(true) - .tlsTrustStoreType(KEY_STORE_TYPE) - .tlsTrustStorePath(BROKER_TRUST_STORE_PATH) - .tlsTrustStorePassword(KEY_STORE_PASSWORD) - .tlsCiphers(cipher) - .authData(new ClientAuthenticationData()) - .build(); - try (PulsarSslFactory pulsarSslFactory = new DefaultPulsarSslFactory()) { - pulsarSslFactory.initialize(pulsarSslConfiguration); - pulsarSslFactory.createInternalSslContext(); - } - } - - @Test(dataProvider = "caCertSslContextDataProvider") - @SuppressWarnings("try") - public void testServerCaCertSslContextWithSslProvider(SslProvider sslProvider, Set ciphers) - throws Exception { - try (PulsarSslFactory pulsarSslFactory = new DefaultPulsarSslFactory()) { - PulsarSslConfiguration.PulsarSslConfigurationBuilder builder = PulsarSslConfiguration.builder() - .tlsTrustCertsFilePath(CA_CERT_PATH) - .tlsCertificateFilePath(BROKER_CERT_PATH) - .tlsKeyFilePath(BROKER_KEY_PATH) - .tlsCiphers(ciphers) - .requireTrustedClientCertOnConnect(true); - if (sslProvider != null) { - builder.tlsProvider(sslProvider.name()); - } - PulsarSslConfiguration pulsarSslConfiguration = builder.build(); - pulsarSslFactory.initialize(pulsarSslConfiguration); - - if (ciphers != null) { - if (sslProvider == null || sslProvider == SslProvider.OPENSSL) { - assertThrows(SSLException.class, pulsarSslFactory::createInternalSslContext); - return; - } - } - pulsarSslFactory.createInternalSslContext(); - } - } - - @Test(dataProvider = "caCertSslContextDataProvider") - @SuppressWarnings("try") - public void testClientCaCertSslContextWithSslProvider(SslProvider sslProvider, Set ciphers) - throws Exception { - try (PulsarSslFactory pulsarSslFactory = new DefaultPulsarSslFactory()) { - PulsarSslConfiguration.PulsarSslConfigurationBuilder builder = PulsarSslConfiguration.builder() - .allowInsecureConnection(true) - .tlsTrustCertsFilePath(CA_CERT_PATH) - .tlsCiphers(ciphers); - if (sslProvider != null) { - builder.tlsProvider(sslProvider.name()); - } - PulsarSslConfiguration pulsarSslConfiguration = builder.build(); - pulsarSslFactory.initialize(pulsarSslConfiguration); - if (ciphers != null) { - if (sslProvider == null || sslProvider == SslProvider.OPENSSL) { - assertThrows(SSLException.class, pulsarSslFactory::createInternalSslContext); - return; - } - } - pulsarSslFactory.createInternalSslContext(); - } - } -} From e5d5db437734321f8a9c704c88a18955524b0571 Mon Sep 17 00:00:00 2001 From: Lari Hotari Date: Thu, 13 Aug 2026 03:19:05 +0300 Subject: [PATCH 2/7] [improve][misc] PIP-478: resolve BouncyCastle via JcaProviders and remove SecurityUtility MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SecurityUtility had exactly one live consumer left after the PIP-337 stack went: message crypto's BouncyCastle provider lookup. Point it at the decomposed equivalent and delete the monolith. MessageCryptoBc now resolves the provider through JcaProviders.requireBouncyCastleProvider(), which performs the same FIPS-agnostic lookup — non-FIPS "BC" or FIPS "BCFIPS", whichever is on the classpath. The lookup stays inside the initialization-on-demand holder, so a resolution failure still surfaces at first asymmetric-crypto use rather than at class loading. One deliberate difference: an absent BouncyCastle now fails with an IllegalStateException naming the caller instead of a RuntimeException wrapping the classpath-load failure. Same failure point, better message. Conscrypt registration is unaffected. Both classes register it eagerly in their static initializers, so it still lands in the JCA provider list; what SecurityUtility additionally installed — a process-global CN-tolerant hostname verifier — is dealt with in the next commit, where the classes it depended on are removed. Assisted-by: Claude Code (Opus 5) --- pulsar-broker-common/build.gradle.kts | 2 +- .../build.gradle.kts | 2 +- .../client/impl/crypto/MessageCryptoBc.java | 9 +- pulsar-common/build.gradle.kts | 2 +- .../pulsar/common/util/SecurityUtility.java | 513 ------------------ 5 files changed, 8 insertions(+), 520 deletions(-) delete mode 100644 pulsar-common/src/main/java/org/apache/pulsar/common/util/SecurityUtility.java diff --git a/pulsar-broker-common/build.gradle.kts b/pulsar-broker-common/build.gradle.kts index b9d2b00d1dd9a..11c5f528493fc 100644 --- a/pulsar-broker-common/build.gradle.kts +++ b/pulsar-broker-common/build.gradle.kts @@ -46,7 +46,7 @@ dependencies { implementation(libs.jetty.compression.gzip) implementation(libs.jetty.ee10.servlet) - // Non-FIPS BouncyCastle provider for tests that exercise SecurityUtility (which loads + // Non-FIPS BouncyCastle provider for tests that exercise JcaProviders (which loads // org.bouncycastle.jce.provider.BouncyCastleProvider in a static initializer). This matches // the provider used in production. FIPS is covered separately by the bcfips-include-test // module; bc-fips must not be on a classpath that also has the non-FIPS provider because both diff --git a/pulsar-client-messagecrypto-bc/build.gradle.kts b/pulsar-client-messagecrypto-bc/build.gradle.kts index 553b714703acc..b0f60b59e94c1 100644 --- a/pulsar-client-messagecrypto-bc/build.gradle.kts +++ b/pulsar-client-messagecrypto-bc/build.gradle.kts @@ -27,7 +27,7 @@ dependencies { api(project(":pulsar-client-api")) // MessageCryptoBc uses BouncyCastle types directly: bcpkix for PEM parsing (PEMParser, // JcaPEMKeyConverter) and bcprov for the EC/IES key specs and ASN.1 types used in key handling. - // The JCA provider itself is resolved at runtime via SecurityUtility (pulsar-common), not here. + // The JCA provider itself is resolved at runtime via JcaProviders (pulsar-common), not here. implementation(libs.bcpkix.jdk18on) api(libs.bcprov.jdk18on) implementation(libs.guava) diff --git a/pulsar-client-messagecrypto-bc/src/main/java/org/apache/pulsar/client/impl/crypto/MessageCryptoBc.java b/pulsar-client-messagecrypto-bc/src/main/java/org/apache/pulsar/client/impl/crypto/MessageCryptoBc.java index 7b418cd67dcc5..eb325236a6647 100644 --- a/pulsar-client-messagecrypto-bc/src/main/java/org/apache/pulsar/client/impl/crypto/MessageCryptoBc.java +++ b/pulsar-client-messagecrypto-bc/src/main/java/org/apache/pulsar/client/impl/crypto/MessageCryptoBc.java @@ -64,7 +64,7 @@ import org.apache.pulsar.common.api.proto.EncryptionKeys; import org.apache.pulsar.common.api.proto.KeyValue; import org.apache.pulsar.common.api.proto.MessageMetadata; -import org.apache.pulsar.common.util.SecurityUtility; +import org.apache.pulsar.common.util.tls.JcaProviders; import org.bouncycastle.asn1.ASN1ObjectIdentifier; import org.bouncycastle.asn1.pkcs.PrivateKeyInfo; import org.bouncycastle.asn1.x509.SubjectPublicKeyInfo; @@ -93,15 +93,16 @@ public class MessageCryptoBc implements MessageCrypto KEY_FACTORY_ALGORITHMS = List.of("RSA", "EC"); - - // Security.getProvider("BC") / Security.getProvider("BCFIPS"). - // also used to get Factories. e.g. CertificateFactory.getInstance("X.509", "BCFIPS") - public static final String BC_FIPS = "BCFIPS"; - public static final String BC = "BC"; - - public static boolean isBCFIPS() { - return BC_PROVIDER.getClass().getCanonicalName().equals(BC_FIPS_PROVIDER_CLASS); - } - - /** - * Get Bouncy Castle provider, and call Security.addProvider(provider) if success. - * 1. try get from classpath. - * 2. try get from Nar. - */ - public static Provider getProvider() { - boolean isProviderInstalled = - Security.getProvider(BC) != null || Security.getProvider(BC_FIPS) != null; - - if (isProviderInstalled) { - Provider provider = Security.getProvider(BC) != null - ? Security.getProvider(BC) - : Security.getProvider(BC_FIPS); - log.debug().attr("provider", provider.getName()).log("Already instantiated Bouncy Castle provider"); - return provider; - } - - // Not installed, try load from class path - try { - return getBCProviderFromClassPath(); - } catch (Exception e) { - log.warn().exception(e) - .log("Not able to get Bouncy Castle provider for both FIPS and Non-FIPS from class path"); - throw new RuntimeException(e); - } - } - - private static Provider loadConscryptProvider() { - Class conscryptClazz; - - try { - conscryptClazz = Class.forName("org.conscrypt.Conscrypt"); - conscryptClazz.getMethod("checkAvailability").invoke(null); - } catch (Throwable e) { - if (e instanceof ClassNotFoundException) { - log.debug("Conscrypt isn't available in the classpath. Using JDK default security provider."); - } else if (e.getCause() instanceof UnsatisfiedLinkError) { - log.debug().attr("os", System.getProperty("os.name")).attr("arch", System.getProperty("os.arch")) - .log("Conscrypt isn't available. Using JDK default security provider"); - } else { - log.debug().attr("cause", e.getCause()).attr("reason", e.getMessage()) - .log("Conscrypt isn't available. Using JDK default security provider"); - } - return null; - } - - Provider provider; - try { - provider = (Provider) Class.forName(CONSCRYPT_PROVIDER_CLASS).getDeclaredConstructor().newInstance(); - } catch (ReflectiveOperationException e) { - log.debug().attr("class", CONSCRYPT_PROVIDER_CLASS).exception(e) - .log("Unable to get security provider"); - return null; - } - - // Configure Conscrypt's default hostname verifier to use Pulsar's TlsHostnameVerifier which - // is more relaxed than the Conscrypt HostnameVerifier checking for RFC 2818 conformity. - // - // Certificates used in Pulsar docs and examples aren't strictly RFC 2818 compliant since they use the - // deprecated way of specifying the hostname in the CN field of the subject DN of the certificate. - // RFC 2818 recommends the use of SAN (subjectAltName) extension for specifying the hostname in the dNSName - // field of the subjectAltName extension. - // - // Conscrypt's default HostnameVerifier has dropped support for the deprecated method of specifying the hostname - // in the CN field. Pulsar's TlsHostnameVerifier continues to support the CN field. - // - // more details of Conscrypt's hostname verification: - // https://github.com/google/conscrypt/blob/master/IMPLEMENTATION_NOTES.md#hostname-verification - // - // Setting the default is sufficient on its own since Conscrypt 2.6.0: TrustManagerImpl used to ignore - // the default verifier (https://github.com/google/conscrypt/issues/1015), which forced Pulsar to copy it - // onto every TrustManager instance, but https://github.com/google/conscrypt/pull/1060 made it fall back - // to the default, so that workaround has been removed. - try { - HostnameVerifier hostnameVerifier = new TlsHostnameVerifier(); - Object wrappedHostnameVerifier = conscryptClazz - .getMethod("wrapHostnameVerifier", - new Class[]{HostnameVerifier.class}).invoke(null, hostnameVerifier); - Method setDefaultHostnameVerifierMethod = - conscryptClazz - .getMethod("setDefaultHostnameVerifier", - new Class[]{Class.forName("org.conscrypt.ConscryptHostnameVerifier")}); - setDefaultHostnameVerifierMethod.invoke(null, wrappedHostnameVerifier); - } catch (Exception e) { - log.warn().exception(e).log("Unable to set default hostname verifier for Conscrypt"); - } - - Security.addProvider(provider); - log.debug().attr("provider", provider.getName()).attr("class", CONSCRYPT_PROVIDER_CLASS) - .log("Added security provider"); - return provider; - } - - /** - * Get Bouncy Castle provider from classpath, and call Security.addProvider. - * Throw Exception if failed. - */ - public static Provider getBCProviderFromClassPath() throws Exception { - Class clazz; - try { - // prefer non FIPS, for backward compatibility concern. - clazz = Class.forName(BC_NON_FIPS_PROVIDER_CLASS); - } catch (ClassNotFoundException cnf) { - log.warn().attr("nonFipsClass", BC_NON_FIPS_PROVIDER_CLASS).attr("fipsClass", BC_FIPS_PROVIDER_CLASS) - .log("Not able to get Bouncy Castle provider, try to get FIPS provider"); - // attempt to use the FIPS provider. - clazz = Class.forName(BC_FIPS_PROVIDER_CLASS); - } - - Provider provider = (Provider) clazz.getDeclaredConstructor().newInstance(); - Security.addProvider(provider); - log.debug().attr("provider", provider.getName()) - .log("Found and Instantiated Bouncy Castle provider in classpath"); - return provider; - } - - public static SSLContext createSslContext(boolean allowInsecureConnection, Certificate[] trustCertificates, - String providerName) - throws GeneralSecurityException { - return createSslContext(allowInsecureConnection, trustCertificates, null, null, providerName); - } - - public static SslContext createNettySslContextForClient(SslProvider sslProvider, boolean allowInsecureConnection, - String trustCertsFilePath, Set ciphers, - Set protocols) - throws GeneralSecurityException, SSLException, FileNotFoundException, IOException { - return createNettySslContextForClient(sslProvider, allowInsecureConnection, trustCertsFilePath, - (Certificate[]) null, - (PrivateKey) null, ciphers, protocols); - } - - public static SSLContext createSslContext(boolean allowInsecureConnection, String trustCertsFilePath, - String certFilePath, String keyFilePath, String providerName) throws GeneralSecurityException { - X509Certificate[] trustCertificates = loadCertificatesFromPemFile(trustCertsFilePath); - X509Certificate[] certificates = loadCertificatesFromPemFile(certFilePath); - PrivateKey privateKey = loadPrivateKeyFromPemFile(keyFilePath); - return createSslContext(allowInsecureConnection, trustCertificates, certificates, privateKey, providerName); - } - - - public static SslContext createNettySslContextForClient(SslProvider sslProvider, boolean allowInsecureConnection, - String trustCertsFilePath, - String certFilePath, String keyFilePath, - Set ciphers, - Set protocols) - throws GeneralSecurityException, SSLException, FileNotFoundException, IOException { - X509Certificate[] certificates = loadCertificatesFromPemFile(certFilePath); - PrivateKey privateKey = loadPrivateKeyFromPemFile(keyFilePath); - return createNettySslContextForClient(sslProvider, allowInsecureConnection, trustCertsFilePath, certificates, - privateKey, ciphers, protocols); - } - - public static SslContext createNettySslContextForClient(SslProvider sslProvider, boolean allowInsecureConnection, - String trustCertsFilePath, - Certificate[] certificates, PrivateKey privateKey, - Set ciphers, - Set protocols) - throws GeneralSecurityException, SSLException, FileNotFoundException, IOException { - - if (StringUtils.isNotBlank(trustCertsFilePath)) { - try (FileInputStream trustCertsStream = new FileInputStream(trustCertsFilePath)) { - return createNettySslContextForClient(sslProvider, allowInsecureConnection, trustCertsStream, - certificates, - privateKey, ciphers, protocols); - } - } else { - return createNettySslContextForClient(sslProvider, allowInsecureConnection, (InputStream) null, - certificates, - privateKey, ciphers, protocols); - } - } - - public static SslContext createNettySslContextForClient(SslProvider sslProvider, boolean allowInsecureConnection, - InputStream trustCertsStream, Certificate[] certificates, - PrivateKey privateKey, Set ciphers, - Set protocols) - throws GeneralSecurityException, SSLException, FileNotFoundException, IOException { - SslContextBuilder builder = SslContextBuilder.forClient().sslProvider(sslProvider); - setupTrustCerts(builder, allowInsecureConnection, trustCertsStream); - setupKeyManager(builder, privateKey, (X509Certificate[]) certificates); - setupCiphers(builder, ciphers); - setupProtocols(builder, protocols); - return builder.build(); - } - - public static SslContext createNettySslContextForServer(SslProvider sslProvider, boolean allowInsecureConnection, - String trustCertsFilePath, - String certFilePath, String keyFilePath, - Set ciphers, Set protocols, - boolean requireTrustedClientCertOnConnect) - throws GeneralSecurityException, SSLException, FileNotFoundException, IOException { - X509Certificate[] certificates = loadCertificatesFromPemFile(certFilePath); - PrivateKey privateKey = loadPrivateKeyFromPemFile(keyFilePath); - - SslContextBuilder builder = - SslContextBuilder.forServer(privateKey, certificates).sslProvider(sslProvider); - setupCiphers(builder, ciphers); - setupProtocols(builder, protocols); - if (StringUtils.isNotBlank(trustCertsFilePath)) { - try (FileInputStream trustCertsStream = new FileInputStream(trustCertsFilePath)) { - setupTrustCerts(builder, allowInsecureConnection, trustCertsStream); - } - } else { - setupTrustCerts(builder, allowInsecureConnection, null); - } - setupKeyManager(builder, privateKey, certificates); - setupClientAuthentication(builder, requireTrustedClientCertOnConnect); - return builder.build(); - } - - public static SSLContext createSslContext(boolean allowInsecureConnection, Certificate[] trustCertficates, - Certificate[] certificates, PrivateKey privateKey) - throws GeneralSecurityException { - return createSslContext(allowInsecureConnection, trustCertficates, certificates, privateKey, null); - } - - public static SSLContext createSslContext(boolean allowInsecureConnection, Certificate[] trustCertficates, - Certificate[] certificates, PrivateKey privateKey, String providerName) - throws GeneralSecurityException { - KeyStoreHolder ksh = new KeyStoreHolder(); - TrustManager[] trustManagers = null; - KeyManager[] keyManagers = null; - Provider provider = resolveProvider(providerName); - - trustManagers = setupTrustCerts(ksh, allowInsecureConnection, trustCertficates, provider); - keyManagers = setupKeyManager(ksh, privateKey, certificates); - - SSLContext sslCtx = provider != null ? SSLContext.getInstance("TLS", provider) - : SSLContext.getInstance("TLS"); - sslCtx.init(keyManagers, trustManagers, new SecureRandom()); - return sslCtx; - } - - private static KeyManager[] setupKeyManager(KeyStoreHolder ksh, PrivateKey privateKey, Certificate[] certificates) - throws KeyStoreException, NoSuchAlgorithmException, UnrecoverableKeyException { - KeyManager[] keyManagers = null; - if (certificates != null && privateKey != null) { - ksh.setPrivateKey("private", privateKey, certificates); - KeyManagerFactory kmf = KeyManagerFactory.getInstance(KeyManagerFactory.getDefaultAlgorithm()); - kmf.init(ksh.getKeyStore(), "".toCharArray()); - keyManagers = kmf.getKeyManagers(); - } - return keyManagers; - } - - private static TrustManager[] setupTrustCerts(KeyStoreHolder ksh, boolean allowInsecureConnection, - Certificate[] trustCertficates, Provider securityProvider) - throws NoSuchAlgorithmException, KeyStoreException { - TrustManager[] trustManagers; - if (allowInsecureConnection) { - trustManagers = InsecureTrustManagerFactory.INSTANCE.getTrustManagers(); - } else { - TrustManagerFactory tmf = securityProvider != null - ? TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm(), securityProvider) - : TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm()); - - if (trustCertficates == null || trustCertficates.length == 0) { - tmf.init((KeyStore) null); - } else { - for (int i = 0; i < trustCertficates.length; i++) { - ksh.setCertificate("trust" + i, trustCertficates[i]); - } - tmf.init(ksh.getKeyStore()); - } - - trustManagers = tmf.getTrustManagers(); - } - return trustManagers; - } - - public static X509Certificate[] loadCertificatesFromPemFile(String certFilePath) throws KeyManagementException { - X509Certificate[] certificates = null; - - if (certFilePath == null || certFilePath.isEmpty()) { - return certificates; - } - - try (FileInputStream input = new FileInputStream(certFilePath)) { - certificates = loadCertificatesFromPemStream(input); - } catch (GeneralSecurityException | IOException e) { - throw new KeyManagementException("Certificate loading error", e); - } - - return certificates; - } - - public static X509Certificate[] loadCertificatesFromPemStream(InputStream inStream) throws KeyManagementException { - if (inStream == null) { - return null; - } - CertificateFactory cf; - try { - if (inStream.markSupported()) { - inStream.reset(); - } - cf = CertificateFactory.getInstance("X.509"); - @SuppressWarnings("unchecked") // CertificateFactory.getInstance("X.509") returns X509Certificate instances - Collection collection = (Collection) cf.generateCertificates(inStream); - return collection.toArray(new X509Certificate[collection.size()]); - } catch (CertificateException | IOException e) { - throw new KeyManagementException("Certificate loading error", e); - } - } - - public static PrivateKey loadPrivateKeyFromPemFile(String keyFilePath) throws KeyManagementException { - if (keyFilePath == null || keyFilePath.isEmpty()) { - return null; - } - - PrivateKey privateKey; - - try (FileInputStream input = new FileInputStream(keyFilePath)) { - privateKey = loadPrivateKeyFromPemStream(input); - } catch (IOException e) { - throw new KeyManagementException("Private key loading error", e); - } - - return privateKey; - } - - public static PrivateKey loadPrivateKeyFromPemStream(InputStream inStream) throws KeyManagementException { - if (inStream == null) { - return null; - } - - PrivateKey privateKey; - - try (BufferedReader reader = new BufferedReader(new InputStreamReader(inStream, StandardCharsets.UTF_8))) { - if (inStream.markSupported()) { - inStream.reset(); - } - StringBuilder sb = new StringBuilder(); - String currentLine = null; - - // Jump to the first line after -----BEGIN [RSA] PRIVATE KEY----- - while ((currentLine = reader.readLine()) != null && !currentLine.startsWith("-----BEGIN")) { - reader.readLine(); - } - - // Stop (and skip) at the last line that has, say, -----END [RSA] PRIVATE KEY----- - while ((currentLine = reader.readLine()) != null && !currentLine.startsWith("-----END")) { - sb.append(currentLine); - } - final KeySpec keySpec = new PKCS8EncodedKeySpec(Base64.getDecoder().decode(sb.toString())); - final List failedAlgorithm = new ArrayList<>(KEY_FACTORY_ALGORITHMS.size()); - for (String algorithm : KEY_FACTORY_ALGORITHMS) { - try { - return KeyFactory.getInstance(algorithm).generatePrivate(keySpec); - } catch (InvalidKeySpecException | NoSuchAlgorithmException ex) { - failedAlgorithm.add(algorithm); - } - } - throw new KeyManagementException("The private key algorithm is not supported. attempted: " - + StringUtils.join(failedAlgorithm, ",")); - } catch (IOException e) { - throw new KeyManagementException("Private key loading error", e); - } - - } - - private static void setupTrustCerts(SslContextBuilder builder, boolean allowInsecureConnection, - InputStream trustCertsStream) throws IOException, FileNotFoundException { - if (allowInsecureConnection) { - builder.trustManager(InsecureTrustManagerFactory.INSTANCE); - } else { - if (trustCertsStream != null) { - builder.trustManager(trustCertsStream); - } else { - builder.trustManager((File) null); - } - } - } - - private static void setupKeyManager(SslContextBuilder builder, PrivateKey privateKey, - X509Certificate[] certificates) { - builder.keyManager(privateKey, certificates); - } - - private static void setupCiphers(SslContextBuilder builder, Set ciphers) { - if (ciphers != null && ciphers.size() > 0) { - builder.ciphers(ciphers); - } - } - - private static void setupProtocols(SslContextBuilder builder, Set protocols) { - if (protocols != null && protocols.size() > 0) { - builder.protocols(protocols.toArray(new String[protocols.size()])); - } - } - - private static void setupClientAuthentication(SslContextBuilder builder, - boolean requireTrustedClientCertOnConnect) { - if (requireTrustedClientCertOnConnect) { - builder.clientAuth(ClientAuth.REQUIRE); - } else { - builder.clientAuth(ClientAuth.OPTIONAL); - } - } - - public static void configureSSLHandler(SslHandler handler) { - SSLEngine sslEngine = handler.engine(); - SSLParameters sslParameters = sslEngine.getSSLParameters(); - sslParameters.setEndpointIdentificationAlgorithm("HTTPS"); - sslEngine.setSSLParameters(sslParameters); - } - - public static Provider resolveProvider(String providerName) throws NoSuchAlgorithmException { - Provider provider = null; - if (!StringUtils.isEmpty(providerName)) { - provider = Security.getProvider(providerName); - } - - if (provider == null) { - provider = SSLContext.getDefault().getProvider(); - } - - return provider; - } - -} From 13fb8956ef60d1596a95922b825b2cfd84aaeac7 Mon Sep 17 00:00:00 2001 From: Lari Hotari Date: Thu, 13 Aug 2026 03:19:06 +0300 Subject: [PATCH 3/7] [improve][misc] PIP-478: remove the deprecated CN-based hostname matching MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pulsar carried its own hostname verifier — TlsHostnameVerifier plus SubjectName, PublicSuffixMatcher, PublicSuffixList and DomainType — whose only referent was SecurityUtility, now gone. Delete them; verification is whatever the configured provider implements under the standard endpoint-identification algorithm "HTTPS". Be precise about the compatibility story, because the obvious statement of it is wrong. The standard algorithm is NOT SAN-only. Per RFC 6125 the JDK and OpenSSL/BoringSSL engines consult a certificate's CN when it carries no dNSName SAN, and ignore the CN once any is present — verified here with real JDK handshakes against this repo's own fixtures: the CN-only hn-verification/broker-cert.pem is accepted for its CN host and rejected for another, and the SAN-bearing broker.cert.pem is rejected for its CN host. So on the default engines a CN-only certificate is accepted exactly as before. What does change is Conscrypt-pinned clients. SecurityUtility's static initializer installed the CN-tolerant verifier as Conscrypt's process-global default; without it, Conscrypt applies its own RFC 2818 verification and rejects a server certificate with no subjectAltName. That reaches a deployment only if it names Conscrypt as its client-side JSSE provider and still uses CN-only server certificates. Conscrypt is never the client-side default — it is defaulted only for TlsPurpose.WEB, i.e. server listeners, which do not verify hostnames. Four documents asserted the SAN-only guarantee and are corrected rather than kept: pip-478.md's summary line, its hostname-matching bullet, its breaking-change note, and the common.tls package javadoc. Deliberately no new "CN-only must be rejected" test: it would fail today on the default engines, which is the point. Both in-repo SAN-less server certificates are negative fixtures on the default engine and keep passing for the same reason as before — AuthenticationTlsHostnameVerificationTest fails on CN mismatch, not on SAN absence. Assisted-by: Claude Code (Opus 5) --- pip/pip-478.md | 10 +- .../apache/pulsar/common/tls/DomainType.java | 30 -- .../pulsar/common/tls/PublicSuffixList.java | 58 ---- .../common/tls/PublicSuffixMatcher.java | 195 ----------- .../apache/pulsar/common/tls/SubjectName.java | 44 --- .../common/tls/TlsHostnameVerifier.java | 313 ------------------ .../pulsar/common/tls/package-info.java | 8 +- .../pulsar/common/util/tls/JcaProviders.java | 10 +- 8 files changed, 15 insertions(+), 653 deletions(-) delete mode 100644 pulsar-common/src/main/java/org/apache/pulsar/common/tls/DomainType.java delete mode 100644 pulsar-common/src/main/java/org/apache/pulsar/common/tls/PublicSuffixList.java delete mode 100644 pulsar-common/src/main/java/org/apache/pulsar/common/tls/PublicSuffixMatcher.java delete mode 100644 pulsar-common/src/main/java/org/apache/pulsar/common/tls/SubjectName.java delete mode 100644 pulsar-common/src/main/java/org/apache/pulsar/common/tls/TlsHostnameVerifier.java diff --git a/pip/pip-478.md b/pip/pip-478.md index 9419f67750f10..130a3c4bed350 100644 --- a/pip/pip-478.md +++ b/pip/pip-478.md @@ -1,6 +1,6 @@ # PIP-478: Asynchronous v5 client authentication SPI, Pulsar TLS factory SPI, and Pulsar HTTP client SPI -> **This PIP at a glance.** It introduces three SPIs — an async, capability-segregated **v5 authentication SPI** (replacing PIP-466's synchronous `Authentication` stub), a purpose-driven **`PulsarTlsFactory` SPI**, and a framework-managed **`PulsarHttpClient` SPI** — and, as a deliberate **breaking change**, **removes PIP-337's `PulsarSslFactory` entirely**. It also makes Pulsar 5.0 TLS **secure by default** (hostname verification on, SAN-only). Reviewer's map: [Motivation](#motivation) (five concrete v4 problems) → [High Level Design](#high-level-design) (the shapes and the reasoning) → [Detailed Design](#detailed-design) (full type listings) → [PIP-337 removal impact](#pip-337-removal-impact) (disposition of every removed config key / method) → [Resolved design decisions](#resolved-design-decisions) (options weighed). Security- and compatibility-focused reviewers should also read [Security Considerations](#security-considerations) and [Backward & Forward Compatibility](#backward--forward-compatibility). +> **This PIP at a glance.** It introduces three SPIs — an async, capability-segregated **v5 authentication SPI** (replacing PIP-466's synchronous `Authentication` stub), a purpose-driven **`PulsarTlsFactory` SPI**, and a framework-managed **`PulsarHttpClient` SPI** — and, as a deliberate **breaking change**, **removes PIP-337's `PulsarSslFactory` entirely**. It also makes Pulsar 5.0 TLS **secure by default** (hostname verification on). Reviewer's map: [Motivation](#motivation) (five concrete v4 problems) → [High Level Design](#high-level-design) (the shapes and the reasoning) → [Detailed Design](#detailed-design) (full type listings) → [PIP-337 removal impact](#pip-337-removal-impact) (disposition of every removed config key / method) → [Resolved design decisions](#resolved-design-decisions) (options weighed). Security- and compatibility-focused reviewers should also read [Security Considerations](#security-considerations) and [Backward & Forward Compatibility](#backward--forward-compatibility). # Background knowledge @@ -1242,7 +1242,11 @@ New OpenTelemetry instruments: # Security Considerations - **TLS is secure by default in 5.0.** Hostname verification is **ON by default** for both the v4 and v5 client *and* for every broker / proxy / geo-replication **outbound** (broker-as-client) connection — a peer whose certificate does not match its hostname/SAN is rejected. Concretely, PIP-478 flips these defaults from `false` to `true`: `tlsHostnameVerificationEnable` on the v4 `ClientConfigurationData` (which also flows into the `CLIENT_DEFAULT` `TlsPolicy`), `tlsHostnameVerificationEnabled` on the broker `ServiceConfiguration` (geo-replication / broker-to-broker lookup) and the proxy `ProxyConfiguration` (proxy→broker), the websocket `WebSocketProxyConfiguration`, and `tlsEnableHostnameVerification` on the functions-worker `WorkerConfig` (their broker-facing clients). The v5 `TlsPolicy` already defaulted secure (`enableHostnameVerification()` true). `allowInsecureConnection` stays **false** everywhere — untrusted server certificates are rejected — unchanged from 4.x. (The dev-only `TlsPolicy.insecure()` preset and the separate BookKeeper-client passthrough `bookkeeper_tlsHostnameVerificationEnabled` are intentionally left as-is.) Enabling any insecure setting is logged once at WARN level, on first use. -- **SAN-only hostname matching (RFC 6125).** Deprecated CN-field hostname matching is **removed**: the custom `TlsHostnameVerifier` and its CN machinery (`SubjectName`, `PublicSuffixMatcher` / `PublicSuffixList`, `DomainType`) are deleted, and both the JDK path (endpoint-identification algorithm `"HTTPS"`) and the Conscrypt / OpenSSL path now perform standard SAN-based (RFC 2818) verification only. A server certificate must carry the hostname in its SubjectAltName extension; a certificate that matches only via its CN field is no longer accepted. This affects **server-hostname** matching only — a *client* certificate's CN is still read as the role token by the broker's `AuthenticationProviderTls`, unchanged. +- **Pulsar's own CN-matching machinery is removed.** The custom `TlsHostnameVerifier` and its CN machinery (`SubjectName`, `PublicSuffixMatcher` / `PublicSuffixList`, `DomainType`) are deleted; hostname verification is whatever the configured provider implements under the standard endpoint-identification algorithm `"HTTPS"`. + + Be precise about what that does and does not change. The standard algorithm is **not** SAN-only: per RFC 6125 §6.4.4, the JDK and OpenSSL/BoringSSL engines consult the certificate's CN **when it carries no `dNSName` SAN**, and ignore the CN once any is present. So on the default engines a CN-only certificate is still accepted, before and after this change. What the removal does change is **Conscrypt-pinned clients**: `SecurityUtility`'s static initializer used to install the CN-tolerant `TlsHostnameVerifier` as Conscrypt's process-global default. With it gone, Conscrypt applies its own RFC 2818 verification and rejects a server certificate with no `subjectAltName`. A deployment is affected only if it explicitly names Conscrypt as its *client-side* JSSE provider and still uses CN-only server certificates; the remedy is to reissue those certificates with the hostname in the SAN. Conscrypt is never the client-side default — it is defaulted only for `TlsPurpose.WEB`, i.e. server listeners, which do not verify hostnames. + + This affects **server-hostname** matching only — a *client* certificate's CN is still read as the role token by the broker's `AuthenticationProviderTls`, unchanged. - **A configured truststore never silently widens trust.** On the **keystore** axis a configured `trustStorePath` whose store holds zero X.509 certificates **fails the load** instead of resolving to an empty trust list, which the context builders would treat as "not configured" and satisfy with the *platform default* trust store — silently trusting every public CA. This restores the 4.x `KeyStoreSSLContext` behavior (the `TrustManagerFactory` was initialized with the explicit store, so a zero-entry store rejected every peer), and because the rotation baseline is committed only after a successful load, an accidental rotation to an empty store keeps the last-good material, WARNs, and retries. The **PEM** axis deliberately keeps the 4.x behavior (an empty/zero-certificate `trustCertsFilePath` falls back to the platform trust store, as `SecurityUtility.setupTrustCerts` did) so existing deployments are not broken — but it now WARNs naming the file. - **A half-configured identity fails loudly — on both axes.** On the **PEM** axis a `certificateFilePath` without a `keyFilePath` yields no usable identity and was silently dropped, surfacing only as a handshake/authentication failure; the default factory now rejects it at load, naming both fields. The converse is asymmetric on purpose: a `keyFilePath` without a certificate is what 4.x tolerated, so it stays a WARN rather than a new startup failure. On the **keystore** axis the counterpart is a *configured* `keyStorePath` whose store yields no usable key entry (a private key with an X.509 chain) — that fails the load too. The 4.x bound is worth stating precisely, because the two sub-cases differ: a wrong `keyStorePassword` already threw `UnrecoverableKeyException` in 4.x and still does (parity, raised before this check), whereas a store holding *no* key entry initialized 4.x's `KeyManagerFactory` fine but with no aliases — a certain, undiagnosed handshake failure for a server purpose, and a silently identity-less client against a peer that does not request a certificate. Failing the load is therefore a deliberate tightening whose only casualties are deployments that already presented no identity, and setting `keyStorePath` is an unambiguous statement of intent to present one. Keep-last-good applies on both axes: the rotation baseline is committed only after a successful load, so an accidental rotation to a keyless store retains the last-good material, WARNs, and retries. All these checks live in the default factory's material load, not in `TlsPolicy.Builder`, so a custom `PulsarTlsFactory` building its own policies is not constrained by them. - **Default enabled protocol set.** When no protocols are configured, the framework pins **`{TLSv1.3, TLSv1.2}`** on the built contexts — the native Netty, the JDK-synthesis, and the Jetty include-protocols paths alike — preserving the floor the removed `DefaultPulsarSslFactory` forced rather than silently deferring to the JVM/provider default on upgrade. @@ -1276,7 +1280,7 @@ New OpenTelemetry instruments: ## Upgrade - Existing applications using `pulsar-client-api` (v4) need **no source changes** — except the rare application that called the removed PIP-337 `sslFactoryPlugin(...)` / `sslFactoryPluginParams(...)` builder methods, which no longer compile and must move to `tlsFactoryClassName(...)` / `tlsFactoryConfig(...)` (see below) — but 5.0 carries several **runtime behavior deltas** they should review before upgrading: - - **Hostname verification is on by default** (`tlsHostnameVerificationEnable` now defaults `true`) and **CN-based matching is removed** (a matching SAN is required) — a server certificate without a matching SAN now fails the TLS handshake where 4.x silently accepted it. See [Security Considerations](#security-considerations) and the breaking-change note under [Backward & Forward Compatibility](#backward--forward-compatibility). + - **Hostname verification is on by default** (`tlsHostnameVerificationEnable` now defaults `true`) — a server certificate that does not match the host now fails the handshake where 4.x silently accepted it. Pulsar's own CN-matching verifier is also removed, which tightens verification for **Conscrypt-pinned clients** specifically; on the default engines the standard algorithm still falls back to the CN for a certificate with no SAN. See [Security Considerations](#security-considerations) and the breaking-change note under [Backward & Forward Compatibility](#backward--forward-compatibility). - **TLS material loads eagerly at client build** (the fail-fast contract), not lazily at first connection as in v4 — a misconfigured cert/key/trust path now surfaces at `PulsarClient` build instead of on first connect. - **`sslProvider` provider-name now routes to `jsseProvider`**: the `OPENSSL` / `OPENSSL_REFCNT` literals still select the native Netty engine; a JSSE *provider name* (e.g. `Conscrypt`, `SunJSSE`) that the old code accepted is now carried to `TlsPolicy.jsseProvider` and builds the `SSLContext` on the JDK engine (**v4 parity restored**), failing loudly if the name resolves to no installed provider (see [v4 client `tls*` settings](#pip-337-removal-impact)). - **The new `jcaProvider` key is a no-op on upgrade.** `jcaProvider` (and `brokerClientJcaProvider`) is unset by default on every surface, and unset means every material `getInstance` call keeps its one-argument form — the JVM provider search order, i.e. the behaviour of every release before this PIP. No config file needs editing, and unlike `jsseProvider` **no legacy v4 value is ever routed onto this axis**, so nothing can start landing there by surprise. Only deployments that opt in see a change; for them the store-type interaction under [Configuration](#configuration) applies. diff --git a/pulsar-common/src/main/java/org/apache/pulsar/common/tls/DomainType.java b/pulsar-common/src/main/java/org/apache/pulsar/common/tls/DomainType.java deleted file mode 100644 index 28b1a623341eb..0000000000000 --- a/pulsar-common/src/main/java/org/apache/pulsar/common/tls/DomainType.java +++ /dev/null @@ -1,30 +0,0 @@ -/* - * 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.pulsar.common.tls; - -/** - * Domain types differentiated by Mozilla Public Suffix List. - * - * @since 4.5 - */ -public enum DomainType { - - UNKNOWN, ICANN, PRIVATE - -} diff --git a/pulsar-common/src/main/java/org/apache/pulsar/common/tls/PublicSuffixList.java b/pulsar-common/src/main/java/org/apache/pulsar/common/tls/PublicSuffixList.java deleted file mode 100644 index 6e77ebe448e08..0000000000000 --- a/pulsar-common/src/main/java/org/apache/pulsar/common/tls/PublicSuffixList.java +++ /dev/null @@ -1,58 +0,0 @@ -/* - * 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. - */ - -/* - * From Apache HTTP client - */ - -package org.apache.pulsar.common.tls; - -import java.util.Collections; -import java.util.List; -import lombok.Data; - -/** - * Public suffix is a set of DNS names or wildcards concatenated with dots. It represents the part of a domain name - * which is not under the control of the individual registrant - *

- * An up-to-date list of suffixes can be obtained from publicsuffix.org - * - * @since 4.4 - */ -@Data -public final class PublicSuffixList { - - private final DomainType type; - private final List rules; - private final List exceptions; - - /** - * @since 4.5 - */ - public PublicSuffixList(final DomainType type, final List rules, final List exceptions) { - this.type = type; - this.rules = Collections.unmodifiableList(rules); - this.exceptions = Collections - .unmodifiableList(exceptions != null ? exceptions : Collections. emptyList()); - } - - public PublicSuffixList(final List rules, final List exceptions) { - this(DomainType.UNKNOWN, rules, exceptions); - } -} diff --git a/pulsar-common/src/main/java/org/apache/pulsar/common/tls/PublicSuffixMatcher.java b/pulsar-common/src/main/java/org/apache/pulsar/common/tls/PublicSuffixMatcher.java deleted file mode 100644 index c4f3e3eb987f7..0000000000000 --- a/pulsar-common/src/main/java/org/apache/pulsar/common/tls/PublicSuffixMatcher.java +++ /dev/null @@ -1,195 +0,0 @@ -/* - * 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. - */ - -/* - * From Apache HTTP client - */ - -package org.apache.pulsar.common.tls; - -import java.net.IDN; -import java.util.Collection; -import java.util.List; -import java.util.Locale; -import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; - -/** - * Utility class that can test if DNS names match the content of the Public Suffix List. - *

- * An up-to-date list of suffixes can be obtained from - * publicsuffix.org - * - * @see org.apache.pulsar.common.tls.PublicSuffixList - * - * @since 4.4 - */ -public final class PublicSuffixMatcher { - - private final Map rules; - private final Map exceptions; - - public PublicSuffixMatcher(final Collection rules, final Collection exceptions) { - this(DomainType.UNKNOWN, rules, exceptions); - } - - /** - * @since 4.5 - */ - public PublicSuffixMatcher( - final DomainType domainType, final Collection rules, final Collection exceptions) { - this.rules = new ConcurrentHashMap(rules.size()); - for (final String rule: rules) { - this.rules.put(rule, domainType); - } - this.exceptions = new ConcurrentHashMap(); - if (exceptions != null) { - for (final String exception: exceptions) { - this.exceptions.put(exception, domainType); - } - } - } - - /** - * @since 4.5 - */ - public PublicSuffixMatcher(final Collection lists) { - this.rules = new ConcurrentHashMap(); - this.exceptions = new ConcurrentHashMap(); - for (final PublicSuffixList list: lists) { - final DomainType domainType = list.getType(); - final List rules = list.getRules(); - for (final String rule: rules) { - this.rules.put(rule, domainType); - } - final List exceptions = list.getExceptions(); - if (exceptions != null) { - for (final String exception: exceptions) { - this.exceptions.put(exception, domainType); - } - } - } - } - - private static boolean hasEntry( - final Map map, - final String rule, - final DomainType expectedType) { - if (map == null) { - return false; - } - final DomainType domainType = map.get(rule); - if (domainType == null) { - return false; - } else { - return expectedType == null || domainType.equals(expectedType); - } - } - - private boolean hasRule(final String rule, final DomainType expectedType) { - return hasEntry(this.rules, rule, expectedType); - } - - private boolean hasException(final String exception, final DomainType expectedType) { - return hasEntry(this.exceptions, exception, expectedType); - } - - /** - * Returns registrable part of the domain for the given domain name or {@code null} - * if given domain represents a public suffix. - * - * @param domain - * @return domain root - */ - public String getDomainRoot(final String domain) { - return getDomainRoot(domain, null); - } - - /** - * Returns registrable part of the domain for the given domain name or {@code null} - * if given domain represents a public suffix. - * - * @param domain - * @param expectedType expected domain type or {@code null} if any. - * @return domain root - * - * @since 4.5 - */ - public String getDomainRoot(final String domain, final DomainType expectedType) { - if (domain == null) { - return null; - } - if (domain.startsWith(".")) { - return null; - } - String domainName = null; - String segment = domain.toLowerCase(Locale.ROOT); - while (segment != null) { - - // An exception rule takes priority over any other matching rule. - if (hasException(IDN.toUnicode(segment), expectedType)) { - return segment; - } - - if (hasRule(IDN.toUnicode(segment), expectedType)) { - break; - } - - final int nextdot = segment.indexOf('.'); - final String nextSegment = nextdot != -1 ? segment.substring(nextdot + 1) : null; - - if (nextSegment != null) { - if (hasRule("*." + IDN.toUnicode(nextSegment), expectedType)) { - break; - } - } - if (nextdot != -1) { - domainName = segment; - } - segment = nextSegment; - } - return domainName; - } - - /** - * Tests whether the given domain matches any of entry from the public suffix list. - */ - public boolean matches(final String domain) { - return matches(domain, null); - } - - /** - * Tests whether the given domain matches any of entry from the public suffix list. - * - * @param domain - * @param expectedType expected domain type or {@code null} if any. - * @return {@code true} if the given domain matches any of the public suffixes. - * - * @since 4.5 - */ - public boolean matches(final String domain, final DomainType expectedType) { - if (domain == null) { - return false; - } - final String domainRoot = getDomainRoot( - domain.startsWith(".") ? domain.substring(1) : domain, expectedType); - return domainRoot == null; - } - -} diff --git a/pulsar-common/src/main/java/org/apache/pulsar/common/tls/SubjectName.java b/pulsar-common/src/main/java/org/apache/pulsar/common/tls/SubjectName.java deleted file mode 100644 index 74542707d9ea8..0000000000000 --- a/pulsar-common/src/main/java/org/apache/pulsar/common/tls/SubjectName.java +++ /dev/null @@ -1,44 +0,0 @@ -/* - * 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. - */ - -/* - * From Apache HTTP client - */ - -package org.apache.pulsar.common.tls; - -import lombok.Data; - -@Data -final class SubjectName { - - static final int DNS = 2; - static final int IP = 7; - - private final String value; - private final int type; - - static SubjectName newIP(final String value) { - return new SubjectName(value, IP); - } - - static SubjectName newDNS(final String value) { - return new SubjectName(value, DNS); - } -} diff --git a/pulsar-common/src/main/java/org/apache/pulsar/common/tls/TlsHostnameVerifier.java b/pulsar-common/src/main/java/org/apache/pulsar/common/tls/TlsHostnameVerifier.java deleted file mode 100644 index 2bbb9cb34f54c..0000000000000 --- a/pulsar-common/src/main/java/org/apache/pulsar/common/tls/TlsHostnameVerifier.java +++ /dev/null @@ -1,313 +0,0 @@ -/* - * 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. - */ - -/* - * From Apache HTTP client - */ - -package org.apache.pulsar.common.tls; - -import java.net.InetAddress; -import java.net.UnknownHostException; -import java.security.cert.Certificate; -import java.security.cert.CertificateParsingException; -import java.security.cert.X509Certificate; -import java.util.ArrayList; -import java.util.Collection; -import java.util.Collections; -import java.util.List; -import java.util.Locale; -import java.util.NoSuchElementException; -import javax.naming.InvalidNameException; -import javax.naming.NamingException; -import javax.naming.directory.Attribute; -import javax.naming.directory.Attributes; -import javax.naming.ldap.LdapName; -import javax.naming.ldap.Rdn; -import javax.net.ssl.HostnameVerifier; -import javax.net.ssl.SSLException; -import javax.net.ssl.SSLPeerUnverifiedException; -import javax.net.ssl.SSLSession; -import javax.security.auth.x500.X500Principal; -import lombok.CustomLog; - -@CustomLog -public class TlsHostnameVerifier implements HostnameVerifier { - - enum HostNameType { - - IPv4(7), IPv6(7), DNS(2); - - final int subjectType; - - HostNameType(final int subjectType) { - this.subjectType = subjectType; - } - } - - private final PublicSuffixMatcher publicSuffixMatcher; - - public TlsHostnameVerifier(final PublicSuffixMatcher publicSuffixMatcher) { - this.publicSuffixMatcher = publicSuffixMatcher; - } - - public TlsHostnameVerifier() { - this(null); - } - - @Override - public boolean verify(final String host, final SSLSession session) { - try { - final Certificate[] certs = session.getPeerCertificates(); - final X509Certificate x509 = (X509Certificate) certs[0]; - verify(host, x509); - return true; - } catch (final SSLException ex) { - log.debug().exception(ex).log(ex.getMessage()); - return false; - } - } - - public void verify( - final String host, final X509Certificate cert) throws SSLException { - final HostNameType hostType = determineHostFormat(host); - final List subjectAlts = getSubjectAltNames(cert); - if (subjectAlts != null && !subjectAlts.isEmpty()) { - switch (hostType) { - case IPv4: - matchIPAddress(host, subjectAlts); - break; - case IPv6: - matchIPv6Address(host, subjectAlts); - break; - default: - matchDNSName(host, subjectAlts, this.publicSuffixMatcher); - } - } else { - // CN matching has been deprecated by rfc2818 and can be used - // as fallback only when no subjectAlts are available - final X500Principal subjectPrincipal = cert.getSubjectX500Principal(); - final String cn = extractCN(subjectPrincipal.getName(X500Principal.RFC2253)); - if (cn == null) { - throw new SSLException("Certificate subject for <" + host + "> doesn't contain " - + "a common name and does not have alternative names"); - } - matchCN(host, cn, this.publicSuffixMatcher); - } - } - - static void matchIPAddress(final String host, final List subjectAlts) throws SSLException { - for (int i = 0; i < subjectAlts.size(); i++) { - final SubjectName subjectAlt = subjectAlts.get(i); - if (subjectAlt.getType() == SubjectName.IP) { - if (host.equals(subjectAlt.getValue())) { - return; - } - } - } - throw new SSLPeerUnverifiedException("Certificate for <" + host + "> doesn't match any " - + "of the subject alternative names: " + subjectAlts); - } - - static void matchIPv6Address(final String host, final List subjectAlts) throws SSLException { - final String normalisedHost = normaliseAddress(host); - for (int i = 0; i < subjectAlts.size(); i++) { - final SubjectName subjectAlt = subjectAlts.get(i); - if (subjectAlt.getType() == SubjectName.IP) { - final String normalizedSubjectAlt = normaliseAddress(subjectAlt.getValue()); - if (normalisedHost.equals(normalizedSubjectAlt)) { - return; - } - } - } - throw new SSLPeerUnverifiedException("Certificate for <" + host + "> doesn't match any " - + "of the subject alternative names: " + subjectAlts); - } - - static void matchDNSName(final String host, final List subjectAlts, - final PublicSuffixMatcher publicSuffixMatcher) throws SSLException { - final String normalizedHost = host.toLowerCase(Locale.ROOT); - for (int i = 0; i < subjectAlts.size(); i++) { - final SubjectName subjectAlt = subjectAlts.get(i); - if (subjectAlt.getType() == SubjectName.DNS) { - final String normalizedSubjectAlt = subjectAlt.getValue().toLowerCase(Locale.ROOT); - if (matchIdentityStrict(normalizedHost, normalizedSubjectAlt, publicSuffixMatcher)) { - return; - } - } - } - throw new SSLPeerUnverifiedException("Certificate for <" + host + "> doesn't match any " - + "of the subject alternative names: " + subjectAlts); - } - - static void matchCN(final String host, final String cn, - final PublicSuffixMatcher publicSuffixMatcher) throws SSLException { - final String normalizedHost = host.toLowerCase(Locale.ROOT); - final String normalizedCn = cn.toLowerCase(Locale.ROOT); - if (!matchIdentityStrict(normalizedHost, normalizedCn, publicSuffixMatcher)) { - throw new SSLPeerUnverifiedException("Certificate for <" + host + "> doesn't match " - + "common name of the certificate subject: " + cn); - } - } - - static boolean matchDomainRoot(final String host, final String domainRoot) { - if (domainRoot == null) { - return false; - } - return host.endsWith(domainRoot) && (host.length() == domainRoot.length() - || host.charAt(host.length() - domainRoot.length() - 1) == '.'); - } - - private static boolean matchIdentity(final String host, final String identity, - final PublicSuffixMatcher publicSuffixMatcher, - final boolean strict) { - if (publicSuffixMatcher != null && host.contains(".")) { - if (!matchDomainRoot(host, publicSuffixMatcher.getDomainRoot(identity, DomainType.ICANN))) { - return false; - } - } - - // RFC 2818, 3.1. Server Identity - // "...Names may contain the wildcard - // character * which is considered to match any single domain name - // component or component fragment..." - // Based on this statement presuming only singular wildcard is legal - final int asteriskIdx = identity.indexOf('*'); - if (asteriskIdx != -1) { - final String prefix = identity.substring(0, asteriskIdx); - final String suffix = identity.substring(asteriskIdx + 1); - if (!prefix.isEmpty() && !host.startsWith(prefix)) { - return false; - } - if (!suffix.isEmpty() && !host.endsWith(suffix)) { - return false; - } - // Additional sanity checks on content selected by wildcard can be done here - if (strict) { - final String remainder = host.substring( - prefix.length(), host.length() - suffix.length()); - return !remainder.contains("."); - } - return true; - } - return host.equalsIgnoreCase(identity); - } - - static boolean matchIdentity(final String host, final String identity, - final PublicSuffixMatcher publicSuffixMatcher) { - return matchIdentity(host, identity, publicSuffixMatcher, false); - } - - static boolean matchIdentity(final String host, final String identity) { - return matchIdentity(host, identity, null, false); - } - - static boolean matchIdentityStrict(final String host, final String identity, - final PublicSuffixMatcher publicSuffixMatcher) { - return matchIdentity(host, identity, publicSuffixMatcher, true); - } - - static boolean matchIdentityStrict(final String host, final String identity) { - return matchIdentity(host, identity, null, true); - } - - static String extractCN(final String subjectPrincipal) throws SSLException { - if (subjectPrincipal == null) { - return null; - } - try { - final LdapName subjectDN = new LdapName(subjectPrincipal); - final List rdns = subjectDN.getRdns(); - for (int i = rdns.size() - 1; i >= 0; i--) { - final Rdn rds = rdns.get(i); - final Attributes attributes = rds.toAttributes(); - final Attribute cn = attributes.get("cn"); - if (cn != null) { - try { - final Object value = cn.get(); - if (value != null) { - return value.toString(); - } - } catch (final NoSuchElementException ignore) { - // ignore exception - } catch (final NamingException ignore) { - // ignore exception - } - } - } - return null; - } catch (final InvalidNameException e) { - throw new SSLException(subjectPrincipal + " is not a valid X500 distinguished name"); - } - } - - static HostNameType determineHostFormat(final String host) { - if (InetAddressUtils.isIPv4Address(host)) { - return HostNameType.IPv4; - } - String s = host; - if (s.startsWith("[") && s.endsWith("]")) { - s = host.substring(1, host.length() - 1); - } - if (InetAddressUtils.isIPv6Address(s)) { - return HostNameType.IPv6; - } - return HostNameType.DNS; - } - - static List getSubjectAltNames(final X509Certificate cert) { - try { - final Collection> entries = cert.getSubjectAlternativeNames(); - if (entries == null) { - return Collections.emptyList(); - } - final List result = new ArrayList(); - for (final List entry : entries) { - final Integer type = entry.size() >= 2 ? (Integer) entry.get(0) : null; - if (type != null) { - final Object o = entry.get(1); - if (o instanceof String) { - result.add(new SubjectName((String) o, type)); - } else if (o instanceof byte[]) { - // TODO ASN.1 DER encoded form - } - } - } - return result; - } catch (final CertificateParsingException ignore) { - return Collections.emptyList(); - } - } - - /* - * Normalize IPv6 or DNS name. - */ - static String normaliseAddress(final String hostname) { - if (hostname == null) { - return hostname; - } - try { - final InetAddress inetAddress = InetAddress.getByName(hostname); - return inetAddress.getHostAddress(); - } catch (final UnknownHostException unexpected) { // Should not happen, because we check for IPv6 address above - return hostname; - } - } - -} diff --git a/pulsar-common/src/main/java/org/apache/pulsar/common/tls/package-info.java b/pulsar-common/src/main/java/org/apache/pulsar/common/tls/package-info.java index 204d21194ef24..3ce2bdb05f14b 100644 --- a/pulsar-common/src/main/java/org/apache/pulsar/common/tls/package-info.java +++ b/pulsar-common/src/main/java/org/apache/pulsar/common/tls/package-info.java @@ -25,9 +25,11 @@ * stacks. They carry no dependency beyond the JDK and slog. * *

Hostname verification is delegated to the JDK/provider standard endpoint identification - * ({@code endpointIdentificationAlgorithm = "HTTPS"}), i.e. SAN-based (RFC 2818) matching. The deprecated - * custom CN-based hostname verifier was removed in Pulsar 5.0 (PIP-478); certificates must carry the hostname - * in the SubjectAltName (SAN) extension. + * ({@code endpointIdentificationAlgorithm = "HTTPS"}). Pulsar's own CN-based hostname verifier was removed in + * 5.0 (PIP-478), so verification is whatever the configured provider implements. Note that the standard + * algorithm is not SAN-only: per RFC 6125 the JDK and OpenSSL engines consult the CN when a certificate + * carries no {@code dNSName} SAN, and ignore it once any is present. Conscrypt does not fall back to the CN; + * pinning it is what makes verification strictly SAN-based. * *

The purpose-driven TLS SPI ({@code PulsarTlsFactory} and companions) lives in its own focused module * under {@link org.apache.pulsar.tls}; the default {@code FileBasedTlsFactory} implementation lives in the diff --git a/pulsar-common/src/main/java/org/apache/pulsar/common/util/tls/JcaProviders.java b/pulsar-common/src/main/java/org/apache/pulsar/common/util/tls/JcaProviders.java index cffcc078a3df8..886768c7f5b8a 100644 --- a/pulsar-common/src/main/java/org/apache/pulsar/common/util/tls/JcaProviders.java +++ b/pulsar-common/src/main/java/org/apache/pulsar/common/util/tls/JcaProviders.java @@ -300,13 +300,9 @@ private static Provider loadConscryptProvider() { // verifier on its own (https://github.com/google/conscrypt/pull/1060 fixing issue 1015), so nothing // has to be propagated onto the individual trust managers. // - // CAVEAT while both TLS stacks coexist: Conscrypt.setDefaultHostnameVerifier is process-global, and - // SecurityUtility's static initializer (still the wired PIP-337 path) sets it to the CN-tolerant - // TlsHostnameVerifier. Whenever that class has been loaded, that relaxed verifier — not Conscrypt's - // own — is the default the trust managers fall back to. Nothing routes through this class yet so no - // deployment is affected today; the PR that turns SAN-only hostname verification on by default must - // neutralize that global (or land together with SecurityUtility's removal) for the SAN-only guarantee - // to hold on a Conscrypt-pinned deployment. + // Nothing overrides that default any more: the CN-tolerant TlsHostnameVerifier that SecurityUtility + // used to install process-wide was removed with it, so a Conscrypt-pinned client gets Conscrypt's + // own RFC 2818 verification. Security.addProvider(provider); log.debug().attr("provider", provider.getName()).attr("class", CONSCRYPT_PROVIDER_CLASS) From 004e2f11229b7c60ff4b5cfb894208b098763db7 Mon Sep 17 00:00:00 2001 From: Lari Hotari Date: Thu, 13 Aug 2026 03:19:06 +0300 Subject: [PATCH 4/7] [improve][misc] PIP-478: finish correcting the hostname-verification record MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review found that three of the four CN/SAN passages were corrected and the fourth was not — and the one missed is the breaking-change bullet under Backward & Forward Compatibility, which is exactly where the other two send compatibility readers. It still announced "SAN required" and "a CN-only certificate no longer suffices", contradicting the Security Considerations text two sections above. An operator following the document's own routing would have reissued fleet certificates for a break that does not exist on the default engines. The bullet now states what actually changes: hostname verification is on by default, a matching SAN always suffices, the CN is still consulted on the default engines for a certificate carrying no dNSName SAN, and Conscrypt-pinned clients are what tighten. The mechanism is stated precisely too. Netty's native OpenSSL/BoringSSL engine does not verify the hostname itself — it delegates to the Java trust manager — so the JDK's HostnameChecker and its CN fallback govern both default engines, not BoringSSL's own matching. Also swept the framing out of the code: the broker TLS support javadoc claimed PIP-478 turns on "SAN-only verification", and eleven proxy tests plus two certificate-generation scripts carried the same phrase, so the corrected record would have been contradicted by the next file a reader opened. Two mislabels found in the same review, pre-existing but adjacent: the trust-manager fallback is documented as being for "a provider that offers none (e.g. Conscrypt)", but real Conscrypt registers both PKIX factories — the fallback is for a third-party provider, and the test stub named "Conscrypt-shaped" models a provider that does not exist. Renamed and re-described. The same javadoc still advertised a Conscrypt hostname-verifier propagation workaround whose body was removed when Conscrypt 2.6.1 landed. Assisted-by: Claude Code (Opus 5) --- pip/pip-478.md | 4 ++-- .../apache/pulsar/broker/tls/TlsFactorySupport.java | 2 +- .../pulsar/client/impl/crypto/MessageCryptoBc.java | 4 ++-- .../org/apache/pulsar/common/tls/impl/TlsContexts.java | 4 ++-- .../apache/pulsar/common/util/tls/JdkSslContexts.java | 8 ++++---- .../pulsar/common/util/tls/JdkSslContextsTest.java | 10 ++++++---- .../proxy/server/ProxyKeyStoreTlsTransportTest.java | 2 +- .../proxy/server/ProxyKeyStoreTlsWithAuthTest.java | 2 +- .../proxy/server/ProxyKeyStoreTlsWithoutAuthTest.java | 2 +- .../apache/pulsar/proxy/server/ProxyMutualTlsTest.java | 2 +- .../proxy/server/ProxyServiceTlsStarterTest.java | 2 +- .../pulsar/proxy/server/ProxyTlsFactoryTest.java | 2 +- .../org/apache/pulsar/proxy/server/ProxyTlsTest.java | 2 +- .../proxy/server/ProxyWithAuthorizationTest.java | 4 ++-- .../proxy/server/ProxyWithoutServiceDiscoveryTest.java | 2 +- .../proxy/server/SslContextFallbackSynthesisTest.java | 2 +- .../ec/jks/key_store_generation.txt | 2 +- tests/certificate-authority/generate_keystore.sh | 2 +- 18 files changed, 30 insertions(+), 28 deletions(-) diff --git a/pip/pip-478.md b/pip/pip-478.md index 130a3c4bed350..1d72b216222f1 100644 --- a/pip/pip-478.md +++ b/pip/pip-478.md @@ -1244,7 +1244,7 @@ New OpenTelemetry instruments: - **TLS is secure by default in 5.0.** Hostname verification is **ON by default** for both the v4 and v5 client *and* for every broker / proxy / geo-replication **outbound** (broker-as-client) connection — a peer whose certificate does not match its hostname/SAN is rejected. Concretely, PIP-478 flips these defaults from `false` to `true`: `tlsHostnameVerificationEnable` on the v4 `ClientConfigurationData` (which also flows into the `CLIENT_DEFAULT` `TlsPolicy`), `tlsHostnameVerificationEnabled` on the broker `ServiceConfiguration` (geo-replication / broker-to-broker lookup) and the proxy `ProxyConfiguration` (proxy→broker), the websocket `WebSocketProxyConfiguration`, and `tlsEnableHostnameVerification` on the functions-worker `WorkerConfig` (their broker-facing clients). The v5 `TlsPolicy` already defaulted secure (`enableHostnameVerification()` true). `allowInsecureConnection` stays **false** everywhere — untrusted server certificates are rejected — unchanged from 4.x. (The dev-only `TlsPolicy.insecure()` preset and the separate BookKeeper-client passthrough `bookkeeper_tlsHostnameVerificationEnabled` are intentionally left as-is.) Enabling any insecure setting is logged once at WARN level, on first use. - **Pulsar's own CN-matching machinery is removed.** The custom `TlsHostnameVerifier` and its CN machinery (`SubjectName`, `PublicSuffixMatcher` / `PublicSuffixList`, `DomainType`) are deleted; hostname verification is whatever the configured provider implements under the standard endpoint-identification algorithm `"HTTPS"`. - Be precise about what that does and does not change. The standard algorithm is **not** SAN-only: per RFC 6125 §6.4.4, the JDK and OpenSSL/BoringSSL engines consult the certificate's CN **when it carries no `dNSName` SAN**, and ignore the CN once any is present. So on the default engines a CN-only certificate is still accepted, before and after this change. What the removal does change is **Conscrypt-pinned clients**: `SecurityUtility`'s static initializer used to install the CN-tolerant `TlsHostnameVerifier` as Conscrypt's process-global default. With it gone, Conscrypt applies its own RFC 2818 verification and rejects a server certificate with no `subjectAltName`. A deployment is affected only if it explicitly names Conscrypt as its *client-side* JSSE provider and still uses CN-only server certificates; the remedy is to reissue those certificates with the hostname in the SAN. Conscrypt is never the client-side default — it is defaulted only for `TlsPurpose.WEB`, i.e. server listeners, which do not verify hostnames. + Be precise about what that does and does not change. The standard algorithm is **not** SAN-only: per RFC 6125 §6.4.4, the CN is consulted **when a certificate carries no `dNSName` SAN**, and ignored once any is present. That holds on both default engines — Netty's native (OpenSSL/BoringSSL) engine does not verify the hostname itself, it delegates to the Java trust manager, so the JDK's `HostnameChecker` and its CN fallback apply there too. So on the default engines a CN-only certificate is still accepted, before and after this change. What the removal does change is **Conscrypt-pinned clients**: `SecurityUtility`'s static initializer used to install the CN-tolerant `TlsHostnameVerifier` as Conscrypt's process-global default. With it gone, Conscrypt applies its own RFC 2818 verification and rejects a server certificate with no `subjectAltName`. A deployment is affected only if it explicitly names Conscrypt as its *client-side* JSSE provider and still uses CN-only server certificates; the remedy is to reissue those certificates with the hostname in the SAN. Conscrypt is never the client-side default — it is defaulted only for `TlsPurpose.WEB`, i.e. server listeners, which do not verify hostnames. This affects **server-hostname** matching only — a *client* certificate's CN is still read as the role token by the broker's `AuthenticationProviderTls`, unchanged. - **A configured truststore never silently widens trust.** On the **keystore** axis a configured `trustStorePath` whose store holds zero X.509 certificates **fails the load** instead of resolving to an empty trust list, which the context builders would treat as "not configured" and satisfy with the *platform default* trust store — silently trusting every public CA. This restores the 4.x `KeyStoreSSLContext` behavior (the `TrustManagerFactory` was initialized with the explicit store, so a zero-entry store rejected every peer), and because the rotation baseline is committed only after a successful load, an accidental rotation to an empty store keeps the last-good material, WARNs, and retries. The **PEM** axis deliberately keeps the 4.x behavior (an empty/zero-certificate `trustCertsFilePath` falls back to the platform trust store, as `SecurityUtility.setupTrustCerts` did) so existing deployments are not broken — but it now WARNs naming the file. @@ -1257,7 +1257,7 @@ New OpenTelemetry instruments: # Backward & Forward Compatibility -- **BREAKING CHANGE — TLS hostname verification on by default + SAN required (5.0).** A v4 or v5 client — and any broker / proxy / geo-replication **outbound** connection — that connects to a server whose certificate lacks a SubjectAltName matching the target hostname now **fails the TLS handshake**, where 4.x (hostname verification off by default) silently accepted it. Deprecated CN-field matching is also removed, so a CN-only certificate no longer suffices even when its CN names the host. This is a deliberate secure-by-default hardening — it closes the long-standing "hostname verification off by default is MITM-prone" gap. **Remediation:** regenerate the affected server certificates so each carries a SAN matching the hostname clients connect to (see the pulsar-site *Transport Encryption (TLS)* guide, `security-tls-transport.md`); or, only where a certificate cannot yet be reissued and the risk is understood, restore the prior per-connection behavior by setting `tlsHostnameVerificationEnable=false` (v4/v5 client), `tlsHostnameVerificationEnabled=false` (broker / proxy / websocket), or `tlsEnableHostnameVerification=false` (functions-worker) — **not recommended**, as it re-opens the connection to man-in-the-middle attacks. `allowInsecureConnection` is independent of this and still defaults to `false` (untrusted server certificates are rejected). +- **BREAKING CHANGE — TLS hostname verification on by default (5.0).** A v4 or v5 client — and any broker / proxy / geo-replication **outbound** connection — that connects to a server whose certificate does not match the target hostname now **fails the TLS handshake**, where 4.x (hostname verification off by default) silently accepted it. Matching follows the provider's standard endpoint-identification algorithm: a SubjectAltName matching the hostname always suffices, and on the default engines the CN is still consulted for a certificate that carries no `dNSName` SAN. Pulsar's own CN-matching verifier is removed, which tightens verification for **Conscrypt-pinned clients** specifically — Conscrypt does not fall back to the CN, and the relaxed global verifier that used to make it do so is gone. This is a deliberate secure-by-default hardening — it closes the long-standing "hostname verification off by default is MITM-prone" gap. **Remediation:** regenerate the affected server certificates so each carries a SAN matching the hostname clients connect to — which is also what a Conscrypt-pinned client requires (see the pulsar-site *Transport Encryption (TLS)* guide, `security-tls-transport.md`); or, only where a certificate cannot yet be reissued and the risk is understood, restore the prior per-connection behavior by setting `tlsHostnameVerificationEnable=false` (v4/v5 client), `tlsHostnameVerificationEnabled=false` (broker / proxy / websocket), or `tlsEnableHostnameVerification=false` (functions-worker) — **not recommended**, as it re-opens the connection to man-in-the-middle attacks. `allowInsecureConnection` is independent of this and still defaults to `false` (untrusted server certificates are rejected). - The v5 client is published as a standalone artifact in 5.0.0-M1 (the existing `pulsar-client-v5` and `pulsar-client-api-v5` modules from PIP-466), **unshaded**. The v5 client will not ship a shaded distribution; applications that need shading to resolve a dependency conflict relocate it themselves in their own build, as they would any other dependency (see the plugin-packaging note above). - **BREAKING CHANGE — PIP-337 removed (5.0).** PIP-337 is removed from Pulsar in Pulsar 5.0 since retaining it in the code base causes an additional maintenance burden and most users don't use it. Concretely: the `sslFactoryPlugin` / `sslFactoryPluginParams` / `brokerClientSslFactoryPlugin` / `brokerClientSslFactoryPluginParams` **config-file keys are removed** and a stale non-default value is **rejected at broker/proxy startup** (and at client `loadConf`) by a removed-key validation; the v4 `ClientBuilder` / `PulsarAdminBuilder` `sslFactoryPlugin(...)` / `sslFactoryPluginParams(...)` methods are **removed** (a source-compatibility break — a compile error on upgrade); the `ClusterData.brokerClientSslFactoryPlugin` / `...Params` fields are **deprecated and ignored** rather than removed — they stay in the metadata schema so a cluster written by 4.x still round-trips — and their successors `ClusterData.brokerClientTlsFactoryClassName` / `brokerClientTlsFactoryConfig` are **added**; and the `pulsar-admin clusters` `--tls-factory-plugin` / `--tls-factory-plugin-params` CLI options (`CmdClusters`) are **replaced** by `--tls-factory-class-name` / `--tls-factory-config`, which write the successor fields, so per-cluster factory selection is retained. Existing PIP-337 users migrate to the new `PulsarTlsFactory` SPI, selected by `tlsFactoryClassName` / `tlsFactoryConfig` — on server config **and**, new in this PIP, on the v4 client and admin builders — the affected public keys/methods/CLI options and each one's disposition are inventoried in [PIP-337 removal impact](#pip-337-removal-impact), and a migration sketch is under Upgrade below. - **Security-relevant behavior changes (5.0), surfaced here so they aren't buried in the removal inventory:** diff --git a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/tls/TlsFactorySupport.java b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/tls/TlsFactorySupport.java index c63e78f819356..4d85b4505e1ff 100644 --- a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/tls/TlsFactorySupport.java +++ b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/tls/TlsFactorySupport.java @@ -293,7 +293,7 @@ public static SslProvider engineProvider(String providerString) { * is conditional, because a default that breaks a supported platform is not a usable default. * *

This applies to server listeners only. Client-side hostname verification is unaffected: a server - * does not verify hostnames, so pinning Conscrypt here cannot interact with the SAN-only verification + * does not verify hostnames, so pinning Conscrypt here cannot interact with the hostname verification * PIP-478 turns on. * * @param explicitJsseProvider the configured {@code jsseProvider} (may be null/blank) diff --git a/pulsar-client-messagecrypto-bc/src/main/java/org/apache/pulsar/client/impl/crypto/MessageCryptoBc.java b/pulsar-client-messagecrypto-bc/src/main/java/org/apache/pulsar/client/impl/crypto/MessageCryptoBc.java index eb325236a6647..7363cc3ecdb0e 100644 --- a/pulsar-client-messagecrypto-bc/src/main/java/org/apache/pulsar/client/impl/crypto/MessageCryptoBc.java +++ b/pulsar-client-messagecrypto-bc/src/main/java/org/apache/pulsar/client/impl/crypto/MessageCryptoBc.java @@ -95,8 +95,8 @@ public class MessageCryptoBc implements MessageCryptoExposed for the same reason as {@link #createKeyManagerFactory(PrivateKey, Certificate[], Provider, * Provider)}: {@code SslContextBuilder.trustManager(X509Certificate...)} would have Netty build the @@ -294,8 +293,9 @@ public static TrustManager[] createTrustManagers(Certificate[] trustCertificates return InsecureTrustManagerFactory.INSTANCE.getTrustManagers(); } // Same algorithm negotiation as the key-manager side: prefer the pinned provider's - // TrustManagerFactory (BCJSSE registers PKIX, the platform default), fall back to the platform - // factory for a provider that offers none (e.g. Conscrypt). + // TrustManagerFactory, falling back to the platform factory for a provider that registers none. + // Both providers Pulsar can pin do register one — BCJSSE and Conscrypt each offer PKIX — so the + // fallback exists for a third-party provider, not for either of them. TrustManagerFactory tmf; if (provider != null) { String algorithm = supportedAlgorithm(provider, "TrustManagerFactory", diff --git a/pulsar-common/src/test/java/org/apache/pulsar/common/util/tls/JdkSslContextsTest.java b/pulsar-common/src/test/java/org/apache/pulsar/common/util/tls/JdkSslContextsTest.java index c47c84c8eb313..74e54b003c7c0 100644 --- a/pulsar-common/src/test/java/org/apache/pulsar/common/util/tls/JdkSslContextsTest.java +++ b/pulsar-common/src/test/java/org/apache/pulsar/common/util/tls/JdkSslContextsTest.java @@ -99,14 +99,16 @@ public void algorithmNegotiationSelectsTheAliasedFallbackForBcjsseShapedProvider .isEqualTo("PKIX"); } - // A JSSE provider with no key/trust-manager services at all (Conscrypt) selects no provider algorithm; + // A JSSE provider registering no key/trust-manager services selects no provider algorithm; real + // Conscrypt is not such a provider (it registers PKIX for both), so this stub stands in for a + // third-party one; // the caller then falls back to the platform default factory while the SSLContext stays pinned. @Test public void algorithmNegotiationReturnsNullForProvidersWithoutTheService() { - Provider conscryptShaped = new Provider("Conscrypt-shaped", "1.0", "test stub") { }; - assertThat(JdkSslContexts.supportedAlgorithm(conscryptShaped, "KeyManagerFactory", "SunX509", "PKIX")) + Provider noFactoryProvider = new Provider("no-factory-provider", "1.0", "test stub") { }; + assertThat(JdkSslContexts.supportedAlgorithm(noFactoryProvider, "KeyManagerFactory", "SunX509", "PKIX")) .isNull(); - assertThat(JdkSslContexts.supportedAlgorithm(conscryptShaped, "TrustManagerFactory", "PKIX", "PKIX")) + assertThat(JdkSslContexts.supportedAlgorithm(noFactoryProvider, "TrustManagerFactory", "PKIX", "PKIX")) .isNull(); } diff --git a/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyKeyStoreTlsTransportTest.java b/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyKeyStoreTlsTransportTest.java index c72be85812a3c..4c9d106d0bc0a 100644 --- a/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyKeyStoreTlsTransportTest.java +++ b/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyKeyStoreTlsTransportTest.java @@ -70,7 +70,7 @@ protected void setup() throws Exception { proxyConfig.setWebServicePort(Optional.of(0)); proxyConfig.setWebServicePortTls(Optional.of(0)); // Advertise over loopback so the proxy service URL host (localhost) matches the keystore server - // certificate's SubjectAltName. TLS hostname verification is on by default (PIP-478, SAN-only), + // certificate's SubjectAltName. TLS hostname verification is on by default (PIP-478), // and the default advertised address resolves to the machine's canonical hostname, which is not // in the cert SAN. This keeps hostname verification genuinely enabled. (The broker already // advertises localhost via the base test config, so the proxy->broker TLS connection matches too.) diff --git a/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyKeyStoreTlsWithAuthTest.java b/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyKeyStoreTlsWithAuthTest.java index d081c64e167d3..8af625572cc9a 100644 --- a/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyKeyStoreTlsWithAuthTest.java +++ b/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyKeyStoreTlsWithAuthTest.java @@ -68,7 +68,7 @@ protected void setup() throws Exception { proxyConfig.setWebServicePort(Optional.of(0)); proxyConfig.setWebServicePortTls(Optional.of(0)); // Advertise over loopback so the proxy service URL host (localhost) matches the keystore server - // certificate's SubjectAltName. TLS hostname verification is on by default (PIP-478, SAN-only), + // certificate's SubjectAltName. TLS hostname verification is on by default (PIP-478), // and the default advertised address resolves to the machine's canonical hostname, which is not // in the cert SAN. This keeps hostname verification genuinely enabled. proxyConfig.setAdvertisedAddress("localhost"); diff --git a/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyKeyStoreTlsWithoutAuthTest.java b/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyKeyStoreTlsWithoutAuthTest.java index a5dbab28f3607..7cb67ed0ecf2e 100644 --- a/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyKeyStoreTlsWithoutAuthTest.java +++ b/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyKeyStoreTlsWithoutAuthTest.java @@ -64,7 +64,7 @@ protected void setup() throws Exception { proxyConfig.setWebServicePort(Optional.of(0)); proxyConfig.setWebServicePortTls(Optional.of(0)); // Advertise over loopback so the proxy service URL host (localhost) matches the keystore server - // certificate's SubjectAltName. TLS hostname verification is on by default (PIP-478, SAN-only), + // certificate's SubjectAltName. TLS hostname verification is on by default (PIP-478), // and the default advertised address resolves to the machine's canonical hostname, which is not // in the cert SAN. This keeps hostname verification genuinely enabled. proxyConfig.setAdvertisedAddress("localhost"); diff --git a/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyMutualTlsTest.java b/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyMutualTlsTest.java index 41f1cd832ca13..7fa9a34e54511 100644 --- a/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyMutualTlsTest.java +++ b/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyMutualTlsTest.java @@ -65,7 +65,7 @@ protected void setup() throws Exception { proxyConfig.setWebServicePortTls(Optional.of(0)); // Advertise over loopback so the proxy service URL host (localhost) matches the server // certificate's SubjectAltName (DNS:localhost, IP:127.0.0.1). TLS hostname verification is on by - // default (PIP-478, SAN-only), and the default advertised address resolves to the machine's + // default (PIP-478), and the default advertised address resolves to the machine's // canonical hostname/IP, which is not in the cert SAN. This keeps hostname verification enabled. proxyConfig.setAdvertisedAddress("localhost"); proxyConfig.setTlsCertificateFilePath(TLS_PROXY_CERT_FILE_PATH); diff --git a/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyServiceTlsStarterTest.java b/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyServiceTlsStarterTest.java index c50bca3e0cd84..87714f96ae680 100644 --- a/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyServiceTlsStarterTest.java +++ b/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyServiceTlsStarterTest.java @@ -78,7 +78,7 @@ protected void setup() throws Exception { serviceStarter.getConfig().setClusterName(configClusterName); // Advertise over loopback so the proxy service URL host (localhost) matches the proxy server // certificate's SubjectAltName (proxy.cert.pem carries DNS:localhost, IP:127.0.0.1). TLS hostname - // verification is on by default (PIP-478, SAN-only), and the default advertised address (loaded from + // verification is on by default (PIP-478), and the default advertised address (loaded from // conf/proxy.conf) resolves to the machine's canonical hostname/IP, which is not in the cert SAN. serviceStarter.getConfig().setAdvertisedAddress("localhost"); serviceStarter.start(); diff --git a/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyTlsFactoryTest.java b/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyTlsFactoryTest.java index 64f4b4f459c02..cdeea22b0a25d 100644 --- a/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyTlsFactoryTest.java +++ b/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyTlsFactoryTest.java @@ -61,7 +61,7 @@ protected void setup() throws Exception { proxyConfig.setWebServicePortTls(Optional.of(0)); // Advertise over loopback so the proxy service URL host (localhost) matches the proxy server // certificate's SubjectAltName (proxy.cert.pem carries DNS:localhost, IP:127.0.0.1). TLS hostname - // verification is on by default (PIP-478, SAN-only), and the default advertised address resolves + // verification is on by default (PIP-478), and the default advertised address resolves // to the machine's canonical hostname/IP, which is not in the cert SAN. Keeps verification enabled. proxyConfig.setAdvertisedAddress("localhost"); proxyConfig.setTlsEnabledWithBroker(false); diff --git a/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyTlsTest.java b/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyTlsTest.java index d406016141a27..887f3001d7463 100644 --- a/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyTlsTest.java +++ b/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyTlsTest.java @@ -59,7 +59,7 @@ protected void setup() throws Exception { proxyConfig.setWebServicePortTls(Optional.of(0)); // Advertise over loopback so the proxy service URL host (localhost) matches the proxy server // certificate's SubjectAltName (proxy.cert.pem carries DNS:localhost, IP:127.0.0.1). TLS hostname - // verification is on by default (PIP-478, SAN-only), and the default advertised address resolves + // verification is on by default (PIP-478), and the default advertised address resolves // to the machine's canonical hostname/IP, which is not in the cert SAN. Keeps verification enabled. proxyConfig.setAdvertisedAddress("localhost"); proxyConfig.setTlsEnabledWithBroker(false); diff --git a/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyWithAuthorizationTest.java b/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyWithAuthorizationTest.java index 23925a430e683..9fb634c0d8aa1 100644 --- a/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyWithAuthorizationTest.java +++ b/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyWithAuthorizationTest.java @@ -186,7 +186,7 @@ protected void doInitConf() throws Exception { conf.setProxyRoles(Collections.singleton("Proxy")); // Advertise over loopback so the broker service/web URLs the proxy connects to (localhost) match the // broker server certificate's SubjectAltName (tls/server-cert.pem carries DNS:localhost, IP:127.0.0.1). - // TLS hostname verification is on by default (PIP-478, SAN-only), and the default advertised address + // TLS hostname verification is on by default (PIP-478), and the default advertised address // resolves to the machine's canonical hostname, which is not in the cert SAN (fails on CI runners). conf.setAdvertisedAddress("localhost"); @@ -245,7 +245,7 @@ protected void setup() throws Exception { proxyConfig.setBrokerWebServiceURLTLS(pulsar.getWebServiceAddressTls()); // Advertise over loopback so the client-facing proxy service URL host (localhost) matches the proxy // server certificate's SubjectAltName (tls/server-cert.pem carries DNS:localhost, IP:127.0.0.1). TLS - // hostname verification is on by default (PIP-478, SAN-only); the default advertised address resolves + // hostname verification is on by default (PIP-478); the default advertised address resolves // to the machine's canonical hostname, which is not in the cert SAN (fails on CI runners). The two // dedicated hostname-verification tests still fail as intended because their server presents a no-SAN // certificate, which cannot match any host regardless of the advertised address. diff --git a/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyWithoutServiceDiscoveryTest.java b/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyWithoutServiceDiscoveryTest.java index 8995b100e3832..07a65fcce6e2f 100644 --- a/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyWithoutServiceDiscoveryTest.java +++ b/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyWithoutServiceDiscoveryTest.java @@ -107,7 +107,7 @@ protected void setup() throws Exception { proxyConfig.setWebServicePortTls(Optional.of(0)); // Advertise over loopback so the client->proxy service URL host (localhost) matches the proxy server // certificate's SubjectAltName (proxy.cert.pem carries DNS:localhost, IP:127.0.0.1). TLS hostname - // verification is on by default (PIP-478, SAN-only), and the default advertised address resolves to + // verification is on by default (PIP-478), and the default advertised address resolves to // the machine's canonical hostname/IP, which is not in the cert SAN. This keeps verification enabled. proxyConfig.setAdvertisedAddress("localhost"); proxyConfig.setTlsEnabledWithBroker(true); diff --git a/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/SslContextFallbackSynthesisTest.java b/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/SslContextFallbackSynthesisTest.java index c7434bd41d821..21423d61cf92b 100644 --- a/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/SslContextFallbackSynthesisTest.java +++ b/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/SslContextFallbackSynthesisTest.java @@ -95,7 +95,7 @@ protected void setup() throws Exception { proxyConfig.setBrokerProxyAllowedTargetPorts("*"); // Advertise over loopback so the client->proxy service URL host (localhost) matches the proxy server // certificate's SubjectAltName (proxy.cert.pem carries DNS:localhost, IP:127.0.0.1). The client - // verifies hostnames (on by default, PIP-478 SAN-only) and the default advertised address resolves + // verifies hostnames (on by default, PIP-478) and the default advertised address resolves // to the machine's canonical hostname/IP, which is not in the cert SAN. proxyConfig.setAdvertisedAddress("localhost"); proxyConfig.setTlsAllowInsecureConnection(true); diff --git a/tests/certificate-authority/ec/jks/key_store_generation.txt b/tests/certificate-authority/ec/jks/key_store_generation.txt index 378b5af898c96..08b7d63c71ce9 100644 --- a/tests/certificate-authority/ec/jks/key_store_generation.txt +++ b/tests/certificate-authority/ec/jks/key_store_generation.txt @@ -7,7 +7,7 @@ keytool -keystore ca.truststore.jks -alias ca -importcert -file ca.cert.pem -sto # Create server keystore -# NOTE (Pulsar 5.0 / PIP-478): TLS hostname verification is enabled by default and is SAN-only, so the server +# NOTE (Pulsar 5.0 / PIP-478): TLS hostname verification is enabled by default, so the server # certificate MUST carry a SubjectAltName that matches the connected host (e.g. DNS:localhost / IP:127.0.0.1). # The keystore hierarchy has its own CA (this directory's ca.cert.pem / ca.key.pem, trusted via # ca.truststore.jks) which is SEPARATE from the PEM hierarchy's CA in the parent directory, so the server diff --git a/tests/certificate-authority/generate_keystore.sh b/tests/certificate-authority/generate_keystore.sh index 62836a5b186ea..ad547c51e56c3 100755 --- a/tests/certificate-authority/generate_keystore.sh +++ b/tests/certificate-authority/generate_keystore.sh @@ -28,7 +28,7 @@ COMMON_PARAMS="-storetype JKS -storepass 111111 -keypass 111111 -noprompt" # generate keystore # The broker keystore is presented as a TLS *server* certificate. With TLS hostname verification -# enabled by default in 5.0 (PIP-478) and CN-based matching removed (SAN-only), the server cert must +# enabled by default in 5.0 (PIP-478), the server cert must # carry a SubjectAltName covering the connected host, otherwise every peer that now verifies hostnames # rejects it with "No subject alternative names present". Tests reach the broker/proxy over loopback # (advertised address pinned to localhost), so DNS:localhost + IP:127.0.0.1 is sufficient. The client From c4f505b900d6dcb56d45d69c91164ca10c620676 Mon Sep 17 00:00:00 2001 From: Lari Hotari Date: Fri, 14 Aug 2026 16:27:22 +0300 Subject: [PATCH 5/7] [improve][misc] PIP-478: correct the RFC citation and finish the hostname sweep MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ### Motivation Review of #26322 found the hostname-verification correction was itself inaccurate in two ways, and incomplete in a third. **The RFC citation is wrong.** RFC 2818 section 3.1 *mandates* the CN fallback ("Otherwise, the (most specific) Common Name field ... MUST be used"), and RFC 6125 section 6.4.4 still permits it as a last resort. So Conscrypt refusing a SAN-less certificate is *stricter* than RFC 2818, not "its own RFC 2818 verification". The phrase was inherited from the `SecurityUtility` comment this series deletes, which had it wrong too. **The sweep stopped short.** Three comments still asserted the retracted claim that CN matching no longer works at all, including in the suite the PR names as its evidence and in two operator-facing conf files. **Two fixture comments state the wrong reason for a passing test.** The no-SAN certificates fail because their CN does not match the advertised host, not because a SAN-less certificate can never match. A reader correcting the fixture by adding a SAN would be surprised; one who set `CN=localhost` would be more surprised. ### Modifications - `JcaProviders`: describe Conscrypt's default as SAN-only and stricter than both RFCs, rather than as an application of RFC 2818. - `pip-478.md`: same correction, with the mandating text quoted. - `ProxyWithAuthorizationTest` (2 comments) and `AuthenticationTlsHostnameVerificationTest` (3): state that the fixtures fail on both counts — no SAN to match, and a non-matching CN. - `conf/client.conf`, `conf/functions_worker.yml`: SANs are authoritative and the CN is consulted only for a certificate with no dNSName SAN; drop "CN matching is not supported". - `generate_keystore.sh`, `key_store_generation.txt`: the SAN makes the fixture portable across providers and endpoint forms; `CN=localhost` alone satisfies the default engines but not Conscrypt and never an IP literal. - Two build files: `JcaProviders` resolves BouncyCastle reflectively on first use, not in a static initializer — that was true of the deleted `SecurityUtility`, and is the very property this PR preserves. Comment-only; no behaviour change. Assisted-by: Claude Code (Opus 5) --- conf/client.conf | 5 +++-- conf/functions_worker.yml | 5 +++-- pip/pip-478.md | 2 +- pulsar-broker-common/build.gradle.kts | 4 ++-- ...AuthenticationTlsHostnameVerificationTest.java | 9 +++++---- pulsar-common/build.gradle.kts | 4 ++-- .../pulsar/common/util/tls/JcaProviders.java | 15 +++++++++------ .../proxy/server/ProxyWithAuthorizationTest.java | 13 +++++++------ .../ec/jks/key_store_generation.txt | 2 ++ tests/certificate-authority/generate_keystore.sh | 10 ++++++---- 10 files changed, 40 insertions(+), 29 deletions(-) diff --git a/conf/client.conf b/conf/client.conf index df44375cd2bd4..a7ef291dfb0fd 100644 --- a/conf/client.conf +++ b/conf/client.conf @@ -46,8 +46,9 @@ authParams= # authority. tlsAllowInsecureConnection=false -# Whether the server hostname is verified against the subject alternative names (SANs) of the -# certificate the server is using (RFC 2818/6125; CN matching is not supported). +# Whether the server hostname is verified against the certificate the server is using. The subject +# alternative names (SANs) are authoritative; the CN is consulted only when the certificate carries no +# dNSName SAN (RFC 2818/6125). A Conscrypt-pinned client does not fall back to the CN at all. # Enabled by default since Pulsar 5.0 (PIP-478). tlsEnableHostnameVerification=true diff --git a/conf/functions_worker.yml b/conf/functions_worker.yml index e6dfbc35255af..4ba8fffcac514 100644 --- a/conf/functions_worker.yml +++ b/conf/functions_worker.yml @@ -348,8 +348,9 @@ tlsKeyFilePath: tlsTrustCertsFilePath: # Accept untrusted TLS certificate from client tlsAllowInsecureConnection: false -# Whether the server hostname is verified against the subject alternative names (SANs) of the -# server certificate (RFC 2818/6125; CN matching is not supported). +# Whether the server hostname is verified against the server certificate. The subject alternative names +# (SANs) are authoritative; the CN is consulted only when the certificate carries no dNSName SAN +# (RFC 2818/6125). A Conscrypt-pinned client does not fall back to the CN at all. # Enabled by default since Pulsar 5.0 (PIP-478). tlsEnableHostnameVerification: true # Tls cert refresh duration in seconds. Set 0 to disable the background rotation check, so the diff --git a/pip/pip-478.md b/pip/pip-478.md index 1d72b216222f1..3603c457ca673 100644 --- a/pip/pip-478.md +++ b/pip/pip-478.md @@ -1244,7 +1244,7 @@ New OpenTelemetry instruments: - **TLS is secure by default in 5.0.** Hostname verification is **ON by default** for both the v4 and v5 client *and* for every broker / proxy / geo-replication **outbound** (broker-as-client) connection — a peer whose certificate does not match its hostname/SAN is rejected. Concretely, PIP-478 flips these defaults from `false` to `true`: `tlsHostnameVerificationEnable` on the v4 `ClientConfigurationData` (which also flows into the `CLIENT_DEFAULT` `TlsPolicy`), `tlsHostnameVerificationEnabled` on the broker `ServiceConfiguration` (geo-replication / broker-to-broker lookup) and the proxy `ProxyConfiguration` (proxy→broker), the websocket `WebSocketProxyConfiguration`, and `tlsEnableHostnameVerification` on the functions-worker `WorkerConfig` (their broker-facing clients). The v5 `TlsPolicy` already defaulted secure (`enableHostnameVerification()` true). `allowInsecureConnection` stays **false** everywhere — untrusted server certificates are rejected — unchanged from 4.x. (The dev-only `TlsPolicy.insecure()` preset and the separate BookKeeper-client passthrough `bookkeeper_tlsHostnameVerificationEnabled` are intentionally left as-is.) Enabling any insecure setting is logged once at WARN level, on first use. - **Pulsar's own CN-matching machinery is removed.** The custom `TlsHostnameVerifier` and its CN machinery (`SubjectName`, `PublicSuffixMatcher` / `PublicSuffixList`, `DomainType`) are deleted; hostname verification is whatever the configured provider implements under the standard endpoint-identification algorithm `"HTTPS"`. - Be precise about what that does and does not change. The standard algorithm is **not** SAN-only: per RFC 6125 §6.4.4, the CN is consulted **when a certificate carries no `dNSName` SAN**, and ignored once any is present. That holds on both default engines — Netty's native (OpenSSL/BoringSSL) engine does not verify the hostname itself, it delegates to the Java trust manager, so the JDK's `HostnameChecker` and its CN fallback apply there too. So on the default engines a CN-only certificate is still accepted, before and after this change. What the removal does change is **Conscrypt-pinned clients**: `SecurityUtility`'s static initializer used to install the CN-tolerant `TlsHostnameVerifier` as Conscrypt's process-global default. With it gone, Conscrypt applies its own RFC 2818 verification and rejects a server certificate with no `subjectAltName`. A deployment is affected only if it explicitly names Conscrypt as its *client-side* JSSE provider and still uses CN-only server certificates; the remedy is to reissue those certificates with the hostname in the SAN. Conscrypt is never the client-side default — it is defaulted only for `TlsPurpose.WEB`, i.e. server listeners, which do not verify hostnames. + Be precise about what that does and does not change. The standard algorithm is **not** SAN-only: per RFC 6125 §6.4.4, the CN is consulted **when a certificate carries no `dNSName` SAN**, and ignored once any is present. That holds on both default engines — Netty's native (OpenSSL/BoringSSL) engine does not verify the hostname itself, it delegates to the Java trust manager, so the JDK's `HostnameChecker` and its CN fallback apply there too. So on the default engines a CN-only certificate is still accepted, before and after this change. What the removal does change is **Conscrypt-pinned clients**: `SecurityUtility`'s static initializer used to install the CN-tolerant `TlsHostnameVerifier` as Conscrypt's process-global default. With it gone, Conscrypt applies its own SAN-only verification and rejects a server certificate with no `subjectAltName`. Note that this is *stricter* than RFC 2818, not an application of it: RFC 2818 §3.1 mandates the CN fallback ("Otherwise, the (most specific) Common Name field ... MUST be used"), and RFC 6125 §6.4.4 still permits it as a last resort. Conscrypt implements neither. A deployment is affected only if it explicitly names Conscrypt as its *client-side* JSSE provider and still uses CN-only server certificates; the remedy is to reissue those certificates with the hostname in the SAN. Conscrypt is never the client-side default — it is defaulted only for `TlsPurpose.WEB`, i.e. server listeners, which do not verify hostnames. This affects **server-hostname** matching only — a *client* certificate's CN is still read as the role token by the broker's `AuthenticationProviderTls`, unchanged. - **A configured truststore never silently widens trust.** On the **keystore** axis a configured `trustStorePath` whose store holds zero X.509 certificates **fails the load** instead of resolving to an empty trust list, which the context builders would treat as "not configured" and satisfy with the *platform default* trust store — silently trusting every public CA. This restores the 4.x `KeyStoreSSLContext` behavior (the `TrustManagerFactory` was initialized with the explicit store, so a zero-entry store rejected every peer), and because the rotation baseline is committed only after a successful load, an accidental rotation to an empty store keeps the last-good material, WARNs, and retries. The **PEM** axis deliberately keeps the 4.x behavior (an empty/zero-certificate `trustCertsFilePath` falls back to the platform trust store, as `SecurityUtility.setupTrustCerts` did) so existing deployments are not broken — but it now WARNs naming the file. diff --git a/pulsar-broker-common/build.gradle.kts b/pulsar-broker-common/build.gradle.kts index 11c5f528493fc..5c2d700607fe5 100644 --- a/pulsar-broker-common/build.gradle.kts +++ b/pulsar-broker-common/build.gradle.kts @@ -46,8 +46,8 @@ dependencies { implementation(libs.jetty.compression.gzip) implementation(libs.jetty.ee10.servlet) - // Non-FIPS BouncyCastle provider for tests that exercise JcaProviders (which loads - // org.bouncycastle.jce.provider.BouncyCastleProvider in a static initializer). This matches + // Non-FIPS BouncyCastle provider for tests that exercise JcaProviders (which resolves + // org.bouncycastle.jce.provider.BouncyCastleProvider reflectively, on first use). This matches // the provider used in production. FIPS is covered separately by the bcfips-include-test // module; bc-fips must not be on a classpath that also has the non-FIPS provider because both // jars define org.bouncycastle.* and the JVM rejects the mismatched signers. diff --git a/pulsar-broker/src/test/java/org/apache/pulsar/client/api/AuthenticationTlsHostnameVerificationTest.java b/pulsar-broker/src/test/java/org/apache/pulsar/client/api/AuthenticationTlsHostnameVerificationTest.java index 82db1c79c17e9..0dca46fe3133e 100644 --- a/pulsar-broker/src/test/java/org/apache/pulsar/client/api/AuthenticationTlsHostnameVerificationTest.java +++ b/pulsar-broker/src/test/java/org/apache/pulsar/client/api/AuthenticationTlsHostnameVerificationTest.java @@ -125,8 +125,9 @@ public Object[][] codecProvider() { * 1. Client tries to connect to broker with hostname="localhost" * 2. Broker sends an x509 certificate whose SubjectAltName does not match "localhost" * (in fact it has no SAN at all, only CN=broker.pulsar.apache.org) - * 3. Client performs SAN-based (RFC 2818) host-name verification, closes the connection - * and fails consumer creation + * 3. Client performs host-name verification: there is no SAN to match, and the CN the default + * engines fall back to is "broker.pulsar.apache.org", so it closes the connection and fails + * consumer creation * * * @throws Exception @@ -140,7 +141,7 @@ public void testTlsSyncProducerAndConsumerWithInvalidBrokerHost(boolean hostname this.hostnameVerificationEnabled = hostnameVerificationEnabled; clientTrustCertFilePath = TLS_MIM_TRUST_CERT_FILE_PATH; // setup broker cert which has no SAN matching the broker's hostname="localhost" - // (CN=broker.pulsar.apache.org, no subjectAltName): CN-based matching is no longer supported (PIP-478). + // (CN=broker.pulsar.apache.org, no subjectAltName), so it matches neither by SAN nor by CN fallback. // The broker itself uses this deliberately-mismatched cert, so its own internal client (e.g. the // system-topic reader for topic policies) cannot verify "localhost" against it; disable hostname // verification for the broker's outbound client here. This test exercises the end-user client's @@ -179,7 +180,7 @@ public void testTlsSyncProducerAndConsumerWithInvalidBrokerHost(boolean hostname * 

      * 1. Client tries to connect to broker with hostname="localhost"
      * 2. Broker sends an x509 certificate whose SubjectAltName includes DNS:localhost
-     * 3. Client performs SAN-based (RFC 2818) host-name verification and continues
+     * 3. Client performs host-name verification, matches DNS:localhost in the SAN, and continues
      *
* * @throws Exception diff --git a/pulsar-common/build.gradle.kts b/pulsar-common/build.gradle.kts index cb24eb41275ed..9978c8d456ce5 100644 --- a/pulsar-common/build.gradle.kts +++ b/pulsar-common/build.gradle.kts @@ -206,8 +206,8 @@ dependencies { // opentelemetry-api as `api` (the SPI exposes OpenTelemetry on its surface), so it reaches this // module's compile AND runtime classpaths through the api(project(":pulsar-tls-factory-api")) above. - // Non-FIPS BouncyCastle provider for tests that exercise JcaProviders (which loads - // org.bouncycastle.jce.provider.BouncyCastleProvider in a static initializer). This matches + // Non-FIPS BouncyCastle provider for tests that exercise JcaProviders (which resolves + // org.bouncycastle.jce.provider.BouncyCastleProvider reflectively, on first use). This matches // the provider used in production. FIPS is covered separately by the bcfips-include-test // module; bc-fips must not be on a classpath that also has the non-FIPS provider because both // jars define org.bouncycastle.* and the JVM rejects the mismatched signers. diff --git a/pulsar-common/src/main/java/org/apache/pulsar/common/util/tls/JcaProviders.java b/pulsar-common/src/main/java/org/apache/pulsar/common/util/tls/JcaProviders.java index 886768c7f5b8a..e5852136f776c 100644 --- a/pulsar-common/src/main/java/org/apache/pulsar/common/util/tls/JcaProviders.java +++ b/pulsar-common/src/main/java/org/apache/pulsar/common/util/tls/JcaProviders.java @@ -295,14 +295,17 @@ private static Provider loadConscryptProvider() { } // Unlike the PIP-337 loader this one installs NO custom hostname verifier, so Conscrypt keeps its - // built-in default: standard RFC 2818 (SAN-based) verification, matching PIP-478's removal of the - // deprecated CN-based matching. Since Conscrypt 2.6.0 a TrustManagerImpl falls back to the default - // verifier on its own (https://github.com/google/conscrypt/pull/1060 fixing issue 1015), so nothing - // has to be propagated onto the individual trust managers. + // built-in default: SAN-only verification, with no fallback to the CN. That is stricter than the + // fallback RFC 2818 section 3.1 mandates when a certificate carries no dNSName SAN, and stricter + // than the last-resort CN-ID check RFC 6125 section 6.4.4 still permits. Since Conscrypt 2.6.0 a + // TrustManagerImpl falls back to the default verifier on its own + // (https://github.com/google/conscrypt/pull/1060 fixing issue 1015), so nothing has to be + // propagated onto the individual trust managers. // // Nothing overrides that default any more: the CN-tolerant TlsHostnameVerifier that SecurityUtility - // used to install process-wide was removed with it, so a Conscrypt-pinned client gets Conscrypt's - // own RFC 2818 verification. + // used to install process-wide was removed with it. So a Conscrypt-pinned client rejects a CN-only + // server certificate that the JDK and OpenSSL engines still accept — the one deployment-visible + // change from removing Pulsar's own CN-matching verifier. Security.addProvider(provider); log.debug().attr("provider", provider.getName()).attr("class", CONSCRYPT_PROVIDER_CLASS) diff --git a/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyWithAuthorizationTest.java b/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyWithAuthorizationTest.java index 9fb634c0d8aa1..ba90173b01714 100644 --- a/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyWithAuthorizationTest.java +++ b/pulsar-proxy/src/test/java/org/apache/pulsar/proxy/server/ProxyWithAuthorizationTest.java @@ -73,10 +73,11 @@ public class ProxyWithAuthorizationTest extends ProducerConsumerBase { // The Proxy, Client, and SuperUser Client certs are signed by this CA private static final String TLS_TRUST_CERT_FILE_PATH = "./src/test/resources/authentication/tls/cacert.pem"; - // A valid cert (signed by the trusted CA) that has NO Subject Alternative Name. Used only by the two - // dedicated hostname-verification tests to prove that SAN-based (RFC 2818) hostname verification correctly - // rejects a connection whose server certificate does not match the host. CN-based matching is no longer - // supported (PIP-478, Pulsar 5.0), so this cert always fails hostname verification. + // A valid cert (signed by the trusted CA) that has NO Subject Alternative Name and CN=Broker. Used only + // by the two dedicated hostname-verification tests to prove that hostname verification correctly rejects + // a connection whose server certificate does not match the host. It fails on both counts: there is no SAN + // to match, and the CN that the JDK and OpenSSL engines fall back to for a SAN-less certificate is + // "Broker", not the advertised "localhost". private static final String TLS_NO_SUBJECT_CERT_FILE_PATH = "./src/test/resources/authentication/tls/ProxyWithAuthorizationTest/no-subject-alt-cert.pem"; private static final String TLS_NO_SUBJECT_KEY_FILE_PATH = @@ -247,8 +248,8 @@ protected void setup() throws Exception { // server certificate's SubjectAltName (tls/server-cert.pem carries DNS:localhost, IP:127.0.0.1). TLS // hostname verification is on by default (PIP-478); the default advertised address resolves // to the machine's canonical hostname, which is not in the cert SAN (fails on CI runners). The two - // dedicated hostname-verification tests still fail as intended because their server presents a no-SAN - // certificate, which cannot match any host regardless of the advertised address. + // dedicated hostname-verification tests still fail as intended: their server presents a certificate + // with no SAN and CN=Broker, which matches neither "localhost" nor the default advertised address. proxyConfig.setAdvertisedAddress("localhost"); proxyConfig.setClusterName(CLUSTER_NAME); diff --git a/tests/certificate-authority/ec/jks/key_store_generation.txt b/tests/certificate-authority/ec/jks/key_store_generation.txt index 08b7d63c71ce9..3b270a97e882c 100644 --- a/tests/certificate-authority/ec/jks/key_store_generation.txt +++ b/tests/certificate-authority/ec/jks/key_store_generation.txt @@ -9,6 +9,8 @@ keytool -keystore ca.truststore.jks -alias ca -importcert -file ca.cert.pem -sto # Create server keystore # NOTE (Pulsar 5.0 / PIP-478): TLS hostname verification is enabled by default, so the server # certificate MUST carry a SubjectAltName that matches the connected host (e.g. DNS:localhost / IP:127.0.0.1). +# The CN fallback the default engines apply to a SAN-less certificate does not rescue this fixture: its CN is +# "server", which matches none of the hosts below, and an IP literal is never matched against the CN anyway. # The keystore hierarchy has its own CA (this directory's ca.cert.pem / ca.key.pem, trusted via # ca.truststore.jks) which is SEPARATE from the PEM hierarchy's CA in the parent directory, so the server # certificate here must be signed by THIS directory's ca.key.pem. `keytool -certreq` drops extensions and diff --git a/tests/certificate-authority/generate_keystore.sh b/tests/certificate-authority/generate_keystore.sh index ad547c51e56c3..67900bbf77405 100755 --- a/tests/certificate-authority/generate_keystore.sh +++ b/tests/certificate-authority/generate_keystore.sh @@ -27,10 +27,12 @@ DAYS=36500 COMMON_PARAMS="-storetype JKS -storepass 111111 -keypass 111111 -noprompt" # generate keystore -# The broker keystore is presented as a TLS *server* certificate. With TLS hostname verification -# enabled by default in 5.0 (PIP-478), the server cert must -# carry a SubjectAltName covering the connected host, otherwise every peer that now verifies hostnames -# rejects it with "No subject alternative names present". Tests reach the broker/proxy over loopback +# The broker keystore is presented as a TLS *server* certificate, and TLS hostname verification is +# enabled by default in 5.0 (PIP-478), so it carries a SubjectAltName covering the connected host. +# CN=localhost on its own would satisfy the JDK and OpenSSL engines, which fall back to the CN for a +# certificate with no dNSName SAN — but not a Conscrypt-pinned peer, which does not fall back, and not a +# connection made to the IP literal, which is never matched against the CN. The SAN is what makes this +# fixture portable across providers and endpoint forms. Tests reach the broker/proxy over loopback # (advertised address pinned to localhost), so DNS:localhost + IP:127.0.0.1 is sufficient. The client # and proxy keystores below are only ever used as *client* identities (not hostname-verified), so they # intentionally keep no SAN. From 542bd1e0587e602aa4c515e05e5777f1c790a05f Mon Sep 17 00:00:00 2001 From: Lari Hotari Date: Fri, 14 Aug 2026 16:40:20 +0300 Subject: [PATCH 6/7] [improve][misc] PIP-478: qualify the CN fallback and correct two more fixture comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ### Motivation A local adversarial pass over the previous commit found three more instances of the defect that commit was fixing — including one in the correction itself. **The CN fallback is for hostnames only.** The corrected text said the CN is consulted "when a certificate carries no `dNSName` SAN", with no reference identity qualifier. The JDK's `HostnameChecker` dispatches an IP literal to `matchIP`, which consults `iPAddress` SANs and never the CN — so a SAN-less certificate has never worked for an IP connection, on any provider. The previous commit stated this correctly in `generate_keystore.sh` and omitted it from the three normative places, which is an inconsistency inside one commit. **Two OAuth2 test comments assert a fixture has no SAN when it does.** `broker.keystore.jks` carries `DNSName: localhost` and `IPAddress: 127.0.0.1`. It fails because it is self-signed and does not chain to `ca.cert.pem` — a trust failure, not a hostname one. Same origin as the conf files (#26282). **Two public javadocs now over-promise.** `ClientBuilder` and `PulsarAdminBuilder` document hostname verification as matching `hostname(CN/SAN)` per RFC 2818 §3.1. Both builders also expose `sslProvider(String)`, and this PR is what makes that promise false for a Conscrypt-pinned client: before it, `SecurityUtility` installed the CN-tolerant verifier process-globally, so the promise held there too. ### Modifications - `pip-478.md`, `JcaProviders`, `common.tls` `package-info`: the CN fallback applies only when connecting by hostname; an IP literal is matched against `iPAddress` SANs on every provider. - `OAuth2IdpTlsFrameworkClientTest`, `AdminOAuth2IdpTlsEndToEndTest`: state the real reason the shared keystore is unusable (self-signed, fails trust), not a SAN it actually has. - `ClientBuilder`, `PulsarAdminBuilder`: record that the CN is a fallback, that it applies only on the default engines and only for hostnames, and that a Conscrypt-pinned client never falls back to it. Comment/javadoc-only; no behaviour change. `quickCheck` and `sanityCheck` pass. Assisted-by: Claude Code (Opus 5) --- pip/pip-478.md | 2 +- .../apache/pulsar/client/admin/PulsarAdminBuilder.java | 5 +++++ .../admin/internal/AdminOAuth2IdpTlsEndToEndTest.java | 3 ++- .../java/org/apache/pulsar/client/api/ClientBuilder.java | 5 +++++ .../impl/auth/oauth2/OAuth2IdpTlsFrameworkClientTest.java | 7 ++++--- .../java/org/apache/pulsar/common/tls/package-info.java | 8 +++++--- .../org/apache/pulsar/common/util/tls/JcaProviders.java | 6 ++++-- 7 files changed, 26 insertions(+), 10 deletions(-) diff --git a/pip/pip-478.md b/pip/pip-478.md index 3603c457ca673..4f9ad8f0eed6c 100644 --- a/pip/pip-478.md +++ b/pip/pip-478.md @@ -1244,7 +1244,7 @@ New OpenTelemetry instruments: - **TLS is secure by default in 5.0.** Hostname verification is **ON by default** for both the v4 and v5 client *and* for every broker / proxy / geo-replication **outbound** (broker-as-client) connection — a peer whose certificate does not match its hostname/SAN is rejected. Concretely, PIP-478 flips these defaults from `false` to `true`: `tlsHostnameVerificationEnable` on the v4 `ClientConfigurationData` (which also flows into the `CLIENT_DEFAULT` `TlsPolicy`), `tlsHostnameVerificationEnabled` on the broker `ServiceConfiguration` (geo-replication / broker-to-broker lookup) and the proxy `ProxyConfiguration` (proxy→broker), the websocket `WebSocketProxyConfiguration`, and `tlsEnableHostnameVerification` on the functions-worker `WorkerConfig` (their broker-facing clients). The v5 `TlsPolicy` already defaulted secure (`enableHostnameVerification()` true). `allowInsecureConnection` stays **false** everywhere — untrusted server certificates are rejected — unchanged from 4.x. (The dev-only `TlsPolicy.insecure()` preset and the separate BookKeeper-client passthrough `bookkeeper_tlsHostnameVerificationEnabled` are intentionally left as-is.) Enabling any insecure setting is logged once at WARN level, on first use. - **Pulsar's own CN-matching machinery is removed.** The custom `TlsHostnameVerifier` and its CN machinery (`SubjectName`, `PublicSuffixMatcher` / `PublicSuffixList`, `DomainType`) are deleted; hostname verification is whatever the configured provider implements under the standard endpoint-identification algorithm `"HTTPS"`. - Be precise about what that does and does not change. The standard algorithm is **not** SAN-only: per RFC 6125 §6.4.4, the CN is consulted **when a certificate carries no `dNSName` SAN**, and ignored once any is present. That holds on both default engines — Netty's native (OpenSSL/BoringSSL) engine does not verify the hostname itself, it delegates to the Java trust manager, so the JDK's `HostnameChecker` and its CN fallback apply there too. So on the default engines a CN-only certificate is still accepted, before and after this change. What the removal does change is **Conscrypt-pinned clients**: `SecurityUtility`'s static initializer used to install the CN-tolerant `TlsHostnameVerifier` as Conscrypt's process-global default. With it gone, Conscrypt applies its own SAN-only verification and rejects a server certificate with no `subjectAltName`. Note that this is *stricter* than RFC 2818, not an application of it: RFC 2818 §3.1 mandates the CN fallback ("Otherwise, the (most specific) Common Name field ... MUST be used"), and RFC 6125 §6.4.4 still permits it as a last resort. Conscrypt implements neither. A deployment is affected only if it explicitly names Conscrypt as its *client-side* JSSE provider and still uses CN-only server certificates; the remedy is to reissue those certificates with the hostname in the SAN. Conscrypt is never the client-side default — it is defaulted only for `TlsPurpose.WEB`, i.e. server listeners, which do not verify hostnames. + Be precise about what that does and does not change. The standard algorithm is **not** SAN-only: per RFC 6125 §6.4.4, the CN is consulted **when the client connects by hostname and the certificate carries no `dNSName` SAN**, and ignored once any is present. The fallback is for hostnames only — a client connecting to an IP literal is matched against `iPAddress` SANs and never against the CN, so a SAN-less certificate has never worked for an IP connection, before or after this change. That holds on both default engines — Netty's native (OpenSSL/BoringSSL) engine does not verify the hostname itself, it delegates to the Java trust manager, so the JDK's `HostnameChecker` and its CN fallback apply there too. So on the default engines a CN-only certificate is still accepted, before and after this change. What the removal does change is **Conscrypt-pinned clients**: `SecurityUtility`'s static initializer used to install the CN-tolerant `TlsHostnameVerifier` as Conscrypt's process-global default. With it gone, Conscrypt applies its own SAN-only verification and rejects a server certificate with no `subjectAltName`. Note that this is *stricter* than RFC 2818, not an application of it: RFC 2818 §3.1 mandates the CN fallback ("Otherwise, the (most specific) Common Name field ... MUST be used"), and RFC 6125 §6.4.4 still permits it as a last resort. Conscrypt implements neither. A deployment is affected only if it explicitly names Conscrypt as its *client-side* JSSE provider and still uses CN-only server certificates; the remedy is to reissue those certificates with the hostname in the SAN. Conscrypt is never the client-side default — it is defaulted only for `TlsPurpose.WEB`, i.e. server listeners, which do not verify hostnames. This affects **server-hostname** matching only — a *client* certificate's CN is still read as the role token by the broker's `AuthenticationProviderTls`, unchanged. - **A configured truststore never silently widens trust.** On the **keystore** axis a configured `trustStorePath` whose store holds zero X.509 certificates **fails the load** instead of resolving to an empty trust list, which the context builders would treat as "not configured" and satisfy with the *platform default* trust store — silently trusting every public CA. This restores the 4.x `KeyStoreSSLContext` behavior (the `TrustManagerFactory` was initialized with the explicit store, so a zero-entry store rejected every peer), and because the rotation baseline is committed only after a successful load, an accidental rotation to an empty store keeps the last-good material, WARNs, and retries. The **PEM** axis deliberately keeps the 4.x behavior (an empty/zero-certificate `trustCertsFilePath` falls back to the platform trust store, as `SecurityUtility.setupTrustCerts` did) so existing deployments are not broken — but it now WARNs naming the file. diff --git a/pulsar-client-admin-api/src/main/java/org/apache/pulsar/client/admin/PulsarAdminBuilder.java b/pulsar-client-admin-api/src/main/java/org/apache/pulsar/client/admin/PulsarAdminBuilder.java index a9fb95a121110..8fd959238ebb9 100644 --- a/pulsar-client-admin-api/src/main/java/org/apache/pulsar/client/admin/PulsarAdminBuilder.java +++ b/pulsar-client-admin-api/src/main/java/org/apache/pulsar/client/admin/PulsarAdminBuilder.java @@ -205,6 +205,11 @@ PulsarAdminBuilder authentication(String authPluginClassName, MapThe CN is only a fallback, and only on the default engines: it is consulted when the client connects by + * hostname and the certificate carries no {@code dNSName} SAN, and ignored once any is present. A client that + * pins Conscrypt as its JSSE provider verifies against the SAN alone and never falls back to the CN (Pulsar + * 5.0, PIP-478). A connection to an IP literal is matched against {@code iPAddress} SANs, never the CN. + * * @see rfc2818 * * @param enableTlsHostnameVerification diff --git a/pulsar-client-admin/src/test/java/org/apache/pulsar/client/admin/internal/AdminOAuth2IdpTlsEndToEndTest.java b/pulsar-client-admin/src/test/java/org/apache/pulsar/client/admin/internal/AdminOAuth2IdpTlsEndToEndTest.java index 40d7b3e572da9..08473bdc5de58 100644 --- a/pulsar-client-admin/src/test/java/org/apache/pulsar/client/admin/internal/AdminOAuth2IdpTlsEndToEndTest.java +++ b/pulsar-client-admin/src/test/java/org/apache/pulsar/client/admin/internal/AdminOAuth2IdpTlsEndToEndTest.java @@ -69,7 +69,8 @@ public class AdminOAuth2IdpTlsEndToEndTest { // broker.cert.pem carries SAN:localhost and chains to ca.cert.pem, so both trust and hostname verification - // pass against the WireMock server (the shared broker.keystore.jks has no SAN). + // pass against the WireMock server (the shared broker.keystore.jks carries the same SAN but is self-signed, + // so it fails trust verification rather than hostname verification). private static final String BROKER_CERT = resource("certificate-authority/server-keys/broker.cert.pem"); private static final String BROKER_KEY = resource("certificate-authority/server-keys/broker.key-pk8.pem"); private static final String CA_CERT = resource("certificate-authority/certs/ca.cert.pem"); diff --git a/pulsar-client-api/src/main/java/org/apache/pulsar/client/api/ClientBuilder.java b/pulsar-client-api/src/main/java/org/apache/pulsar/client/api/ClientBuilder.java index 2966ca993bc49..478bb3919af1c 100644 --- a/pulsar-client-api/src/main/java/org/apache/pulsar/client/api/ClientBuilder.java +++ b/pulsar-client-api/src/main/java/org/apache/pulsar/client/api/ClientBuilder.java @@ -381,6 +381,11 @@ ClientBuilder authentication(String authPluginClassName, Map aut * certificate and matches provided hostname(CN/SAN) with expected broker's host name. It follows RFC 2818, 3.1. * Server Identity hostname verification. * + *

The CN is only a fallback, and only on the default engines: it is consulted when the client connects by + * hostname and the certificate carries no {@code dNSName} SAN, and ignored once any is present. A client that + * pins Conscrypt as its JSSE provider verifies against the SAN alone and never falls back to the CN (Pulsar + * 5.0, PIP-478). A connection to an IP literal is matched against {@code iPAddress} SANs, never the CN. + * * @see RFC 818 * * @param enableTlsHostnameVerification whether to enable TLS hostname verification diff --git a/pulsar-client/src/test/java/org/apache/pulsar/client/impl/auth/oauth2/OAuth2IdpTlsFrameworkClientTest.java b/pulsar-client/src/test/java/org/apache/pulsar/client/impl/auth/oauth2/OAuth2IdpTlsFrameworkClientTest.java index 510ab639371d7..42f36f67988a3 100644 --- a/pulsar-client/src/test/java/org/apache/pulsar/client/impl/auth/oauth2/OAuth2IdpTlsFrameworkClientTest.java +++ b/pulsar-client/src/test/java/org/apache/pulsar/client/impl/auth/oauth2/OAuth2IdpTlsFrameworkClientTest.java @@ -69,9 +69,10 @@ */ public class OAuth2IdpTlsFrameworkClientTest { - // The shared self-signed broker.keystore.jks carries no SAN, so it fails modern hostname verification (which - // the folded CLIENT_OAUTH2 policy keeps on, matching v4). Build the WireMock server keystore from - // broker.cert.pem instead — it has SAN:localhost and chains to ca.cert.pem — so both trust and hostname pass. + // The shared broker.keystore.jks is self-signed (issuer == subject), so it does not chain to ca.cert.pem and + // fails trust verification — its SAN is fine (DNS:localhost, IP:127.0.0.1). Build the WireMock server + // keystore from broker.cert.pem instead — same SAN, but chained to ca.cert.pem — so both trust and hostname + // verification pass (the folded CLIENT_OAUTH2 policy keeps hostname verification on, matching v4). private static final String BROKER_CERT = resource("certificate-authority/server-keys/broker.cert.pem"); private static final String BROKER_KEY = resource("certificate-authority/server-keys/broker.key-pk8.pem"); private static final String CA_CERT = resource("certificate-authority/certs/ca.cert.pem"); diff --git a/pulsar-common/src/main/java/org/apache/pulsar/common/tls/package-info.java b/pulsar-common/src/main/java/org/apache/pulsar/common/tls/package-info.java index 3ce2bdb05f14b..d5a29b4423db3 100644 --- a/pulsar-common/src/main/java/org/apache/pulsar/common/tls/package-info.java +++ b/pulsar-common/src/main/java/org/apache/pulsar/common/tls/package-info.java @@ -27,9 +27,11 @@ *

Hostname verification is delegated to the JDK/provider standard endpoint identification * ({@code endpointIdentificationAlgorithm = "HTTPS"}). Pulsar's own CN-based hostname verifier was removed in * 5.0 (PIP-478), so verification is whatever the configured provider implements. Note that the standard - * algorithm is not SAN-only: per RFC 6125 the JDK and OpenSSL engines consult the CN when a certificate - * carries no {@code dNSName} SAN, and ignore it once any is present. Conscrypt does not fall back to the CN; - * pinning it is what makes verification strictly SAN-based. + * algorithm is not SAN-only: per RFC 6125 the JDK and OpenSSL engines consult the CN when the client connects + * by hostname and the certificate carries no {@code dNSName} SAN, and ignore it once any is present. The + * fallback is for hostnames only — a client connecting to an IP literal is matched against {@code iPAddress} + * SANs and never against the CN. Conscrypt does not fall back to the CN at all; pinning it is what makes + * verification strictly SAN-based. * *

The purpose-driven TLS SPI ({@code PulsarTlsFactory} and companions) lives in its own focused module * under {@link org.apache.pulsar.tls}; the default {@code FileBasedTlsFactory} implementation lives in the diff --git a/pulsar-common/src/main/java/org/apache/pulsar/common/util/tls/JcaProviders.java b/pulsar-common/src/main/java/org/apache/pulsar/common/util/tls/JcaProviders.java index e5852136f776c..352f475f92bad 100644 --- a/pulsar-common/src/main/java/org/apache/pulsar/common/util/tls/JcaProviders.java +++ b/pulsar-common/src/main/java/org/apache/pulsar/common/util/tls/JcaProviders.java @@ -296,8 +296,10 @@ private static Provider loadConscryptProvider() { // Unlike the PIP-337 loader this one installs NO custom hostname verifier, so Conscrypt keeps its // built-in default: SAN-only verification, with no fallback to the CN. That is stricter than the - // fallback RFC 2818 section 3.1 mandates when a certificate carries no dNSName SAN, and stricter - // than the last-resort CN-ID check RFC 6125 section 6.4.4 still permits. Since Conscrypt 2.6.0 a + // fallback RFC 2818 section 3.1 mandates when a hostname is matched against a certificate carrying + // no dNSName SAN, and stricter than the last-resort CN-ID check RFC 6125 section 6.4.4 still + // permits. (Neither fallback ever applied to an IP literal, which is matched against iPAddress + // SANs only, so pinning Conscrypt changes nothing there.) Since Conscrypt 2.6.0 a // TrustManagerImpl falls back to the default verifier on its own // (https://github.com/google/conscrypt/pull/1060 fixing issue 1015), so nothing has to be // propagated onto the individual trust managers. From be570cff9145ecf3ae43594689e4be65c74d9e99 Mon Sep 17 00:00:00 2001 From: Lari Hotari Date: Fri, 14 Aug 2026 16:44:03 +0300 Subject: [PATCH 7/7] [improve][misc] PIP-478: cite the current hostname-verification standard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ### Motivation A third review pass over the two preceding commits found the corrected RFC record stops one RFC short, and misattributes one condition. **RFC 6125 is obsoleted.** The text said Conscrypt's SAN-only verification is stricter than "the last-resort CN-ID check RFC 6125 §6.4.4 still permits". RFC 9525 (2023) obsoletes RFC 6125 and forbids the check outright — "The Common Name RDN MUST NOT be used to identify a service". So Conscrypt is not extra-standard at all; it implements the current rule, and the default engines are the lenient ones. Framing it the other way invites a future maintainer to cite RFC 6125 as the last word and reopen a closed question. **The fallback condition is RFC 2818's, not RFC 6125's.** The text attributed "the CN is consulted when a certificate carries no dNSName SAN" to RFC 6125 §6.4.4, whose condition is stricter — it permits the CN check only when the certificate presents no DNS-ID, SRV-ID or URI-ID at all. The JDK's `HostnameChecker` implements the looser RFC 2818 condition (it falls back on a certificate carrying only a URI SAN, which RFC 6125 forbids), so RFC 2818 is the accurate citation for the behaviour Pulsar actually gets. ### Modifications - `pip-478.md`, `JcaProviders`, `common.tls` `package-info`: cite RFC 9525 as the current rule and RFC 2818 §3.1 for the fallback condition the JDK implements; note explicitly that the default engines are the lenient party. - `ClientBuilder`: fix the pre-existing `@see` link text, which read "RFC 818". Comment/javadoc-only; no behaviour change. Assisted-by: Claude Code (Opus 5) --- conf/client.conf | 2 +- conf/functions_worker.yml | 2 +- pip/pip-478.md | 2 +- .../java/org/apache/pulsar/client/api/ClientBuilder.java | 2 +- .../java/org/apache/pulsar/common/tls/package-info.java | 5 +++-- .../org/apache/pulsar/common/util/tls/JcaProviders.java | 6 ++++-- 6 files changed, 11 insertions(+), 8 deletions(-) diff --git a/conf/client.conf b/conf/client.conf index a7ef291dfb0fd..493e5a734839d 100644 --- a/conf/client.conf +++ b/conf/client.conf @@ -48,7 +48,7 @@ tlsAllowInsecureConnection=false # Whether the server hostname is verified against the certificate the server is using. The subject # alternative names (SANs) are authoritative; the CN is consulted only when the certificate carries no -# dNSName SAN (RFC 2818/6125). A Conscrypt-pinned client does not fall back to the CN at all. +# dNSName SAN (RFC 2818 section 3.1). A Conscrypt-pinned client does not fall back to the CN at all. # Enabled by default since Pulsar 5.0 (PIP-478). tlsEnableHostnameVerification=true diff --git a/conf/functions_worker.yml b/conf/functions_worker.yml index 4ba8fffcac514..6d92c939f744d 100644 --- a/conf/functions_worker.yml +++ b/conf/functions_worker.yml @@ -350,7 +350,7 @@ tlsTrustCertsFilePath: tlsAllowInsecureConnection: false # Whether the server hostname is verified against the server certificate. The subject alternative names # (SANs) are authoritative; the CN is consulted only when the certificate carries no dNSName SAN -# (RFC 2818/6125). A Conscrypt-pinned client does not fall back to the CN at all. +# (RFC 2818 section 3.1). A Conscrypt-pinned client does not fall back to the CN at all. # Enabled by default since Pulsar 5.0 (PIP-478). tlsEnableHostnameVerification: true # Tls cert refresh duration in seconds. Set 0 to disable the background rotation check, so the diff --git a/pip/pip-478.md b/pip/pip-478.md index 4f9ad8f0eed6c..bacce78e4c15c 100644 --- a/pip/pip-478.md +++ b/pip/pip-478.md @@ -1244,7 +1244,7 @@ New OpenTelemetry instruments: - **TLS is secure by default in 5.0.** Hostname verification is **ON by default** for both the v4 and v5 client *and* for every broker / proxy / geo-replication **outbound** (broker-as-client) connection — a peer whose certificate does not match its hostname/SAN is rejected. Concretely, PIP-478 flips these defaults from `false` to `true`: `tlsHostnameVerificationEnable` on the v4 `ClientConfigurationData` (which also flows into the `CLIENT_DEFAULT` `TlsPolicy`), `tlsHostnameVerificationEnabled` on the broker `ServiceConfiguration` (geo-replication / broker-to-broker lookup) and the proxy `ProxyConfiguration` (proxy→broker), the websocket `WebSocketProxyConfiguration`, and `tlsEnableHostnameVerification` on the functions-worker `WorkerConfig` (their broker-facing clients). The v5 `TlsPolicy` already defaulted secure (`enableHostnameVerification()` true). `allowInsecureConnection` stays **false** everywhere — untrusted server certificates are rejected — unchanged from 4.x. (The dev-only `TlsPolicy.insecure()` preset and the separate BookKeeper-client passthrough `bookkeeper_tlsHostnameVerificationEnabled` are intentionally left as-is.) Enabling any insecure setting is logged once at WARN level, on first use. - **Pulsar's own CN-matching machinery is removed.** The custom `TlsHostnameVerifier` and its CN machinery (`SubjectName`, `PublicSuffixMatcher` / `PublicSuffixList`, `DomainType`) are deleted; hostname verification is whatever the configured provider implements under the standard endpoint-identification algorithm `"HTTPS"`. - Be precise about what that does and does not change. The standard algorithm is **not** SAN-only: per RFC 6125 §6.4.4, the CN is consulted **when the client connects by hostname and the certificate carries no `dNSName` SAN**, and ignored once any is present. The fallback is for hostnames only — a client connecting to an IP literal is matched against `iPAddress` SANs and never against the CN, so a SAN-less certificate has never worked for an IP connection, before or after this change. That holds on both default engines — Netty's native (OpenSSL/BoringSSL) engine does not verify the hostname itself, it delegates to the Java trust manager, so the JDK's `HostnameChecker` and its CN fallback apply there too. So on the default engines a CN-only certificate is still accepted, before and after this change. What the removal does change is **Conscrypt-pinned clients**: `SecurityUtility`'s static initializer used to install the CN-tolerant `TlsHostnameVerifier` as Conscrypt's process-global default. With it gone, Conscrypt applies its own SAN-only verification and rejects a server certificate with no `subjectAltName`. Note that this is *stricter* than RFC 2818, not an application of it: RFC 2818 §3.1 mandates the CN fallback ("Otherwise, the (most specific) Common Name field ... MUST be used"), and RFC 6125 §6.4.4 still permits it as a last resort. Conscrypt implements neither. A deployment is affected only if it explicitly names Conscrypt as its *client-side* JSSE provider and still uses CN-only server certificates; the remedy is to reissue those certificates with the hostname in the SAN. Conscrypt is never the client-side default — it is defaulted only for `TlsPurpose.WEB`, i.e. server listeners, which do not verify hostnames. + Be precise about what that does and does not change. The standard algorithm is **not** SAN-only: per RFC 2818 §3.1, and as the JDK's `HostnameChecker` implements it, the CN is consulted **when the client connects by hostname and the certificate carries no `dNSName` SAN**, and ignored once any is present. (RFC 6125 §6.4.4's condition is stricter than that — it permits the CN check only when the certificate presents *no* DNS-ID, SRV-ID or URI-ID at all — so the behaviour Pulsar actually gets is RFC 2818's, not RFC 6125's.) The fallback is for hostnames only — a client connecting to an IP literal is matched against `iPAddress` SANs and never against the CN, so a SAN-less certificate has never worked for an IP connection, before or after this change. That holds on both default engines — Netty's native (OpenSSL/BoringSSL) engine does not verify the hostname itself, it delegates to the Java trust manager, so the JDK's `HostnameChecker` and its CN fallback apply there too. So on the default engines a CN-only certificate is still accepted, before and after this change. What the removal does change is **Conscrypt-pinned clients**: `SecurityUtility`'s static initializer used to install the CN-tolerant `TlsHostnameVerifier` as Conscrypt's process-global default. With it gone, Conscrypt applies its own SAN-only verification and rejects a server certificate with no `subjectAltName`. Note that this is *stricter* than RFC 2818, not an application of it: RFC 2818 §3.1 mandates the CN fallback ("Otherwise, the (most specific) Common Name field ... MUST be used"), and RFC 6125 §6.4.4 permitted it as a last resort. Conscrypt implements neither — and that is not a Conscrypt eccentricity but the *current* standard: RFC 9525 (2023) obsoletes RFC 6125 and forbids the CN check outright ("The Common Name RDN MUST NOT be used to identify a service"). So the default engines are the lenient ones here, and what Pulsar's CN-only certificates have always relied on is behaviour the IETF has since retired. A deployment is affected only if it explicitly names Conscrypt as its *client-side* JSSE provider and still uses CN-only server certificates; the remedy is to reissue those certificates with the hostname in the SAN. Conscrypt is never the client-side default — it is defaulted only for `TlsPurpose.WEB`, i.e. server listeners, which do not verify hostnames. This affects **server-hostname** matching only — a *client* certificate's CN is still read as the role token by the broker's `AuthenticationProviderTls`, unchanged. - **A configured truststore never silently widens trust.** On the **keystore** axis a configured `trustStorePath` whose store holds zero X.509 certificates **fails the load** instead of resolving to an empty trust list, which the context builders would treat as "not configured" and satisfy with the *platform default* trust store — silently trusting every public CA. This restores the 4.x `KeyStoreSSLContext` behavior (the `TrustManagerFactory` was initialized with the explicit store, so a zero-entry store rejected every peer), and because the rotation baseline is committed only after a successful load, an accidental rotation to an empty store keeps the last-good material, WARNs, and retries. The **PEM** axis deliberately keeps the 4.x behavior (an empty/zero-certificate `trustCertsFilePath` falls back to the platform trust store, as `SecurityUtility.setupTrustCerts` did) so existing deployments are not broken — but it now WARNs naming the file. diff --git a/pulsar-client-api/src/main/java/org/apache/pulsar/client/api/ClientBuilder.java b/pulsar-client-api/src/main/java/org/apache/pulsar/client/api/ClientBuilder.java index 478bb3919af1c..7ef5ca1c1dd84 100644 --- a/pulsar-client-api/src/main/java/org/apache/pulsar/client/api/ClientBuilder.java +++ b/pulsar-client-api/src/main/java/org/apache/pulsar/client/api/ClientBuilder.java @@ -386,7 +386,7 @@ ClientBuilder authentication(String authPluginClassName, Map aut * pins Conscrypt as its JSSE provider verifies against the SAN alone and never falls back to the CN (Pulsar * 5.0, PIP-478). A connection to an IP literal is matched against {@code iPAddress} SANs, never the CN. * - * @see RFC 818 + * @see RFC 2818 * * @param enableTlsHostnameVerification whether to enable TLS hostname verification * @return the client builder instance diff --git a/pulsar-common/src/main/java/org/apache/pulsar/common/tls/package-info.java b/pulsar-common/src/main/java/org/apache/pulsar/common/tls/package-info.java index d5a29b4423db3..8d1120c1a2d7d 100644 --- a/pulsar-common/src/main/java/org/apache/pulsar/common/tls/package-info.java +++ b/pulsar-common/src/main/java/org/apache/pulsar/common/tls/package-info.java @@ -27,8 +27,9 @@ *

Hostname verification is delegated to the JDK/provider standard endpoint identification * ({@code endpointIdentificationAlgorithm = "HTTPS"}). Pulsar's own CN-based hostname verifier was removed in * 5.0 (PIP-478), so verification is whatever the configured provider implements. Note that the standard - * algorithm is not SAN-only: per RFC 6125 the JDK and OpenSSL engines consult the CN when the client connects - * by hostname and the certificate carries no {@code dNSName} SAN, and ignore it once any is present. The + * algorithm is not SAN-only: per RFC 2818 §3.1, the JDK and OpenSSL engines consult the CN when the client + * connects by hostname and the certificate carries no {@code dNSName} SAN, and ignore it once any is + * present — behaviour RFC 9525 has since retired, which is why Conscrypt does not implement it. The * fallback is for hostnames only — a client connecting to an IP literal is matched against {@code iPAddress} * SANs and never against the CN. Conscrypt does not fall back to the CN at all; pinning it is what makes * verification strictly SAN-based. diff --git a/pulsar-common/src/main/java/org/apache/pulsar/common/util/tls/JcaProviders.java b/pulsar-common/src/main/java/org/apache/pulsar/common/util/tls/JcaProviders.java index 352f475f92bad..907dd235d8ce7 100644 --- a/pulsar-common/src/main/java/org/apache/pulsar/common/util/tls/JcaProviders.java +++ b/pulsar-common/src/main/java/org/apache/pulsar/common/util/tls/JcaProviders.java @@ -297,8 +297,10 @@ private static Provider loadConscryptProvider() { // Unlike the PIP-337 loader this one installs NO custom hostname verifier, so Conscrypt keeps its // built-in default: SAN-only verification, with no fallback to the CN. That is stricter than the // fallback RFC 2818 section 3.1 mandates when a hostname is matched against a certificate carrying - // no dNSName SAN, and stricter than the last-resort CN-ID check RFC 6125 section 6.4.4 still - // permits. (Neither fallback ever applied to an IP literal, which is matched against iPAddress + // no dNSName SAN, and stricter than the last-resort CN-ID check RFC 6125 section 6.4.4 permitted — + // but it is what RFC 9525 (which obsoletes RFC 6125) now requires: "The Common Name RDN MUST NOT be + // used to identify a service". So it is the default engines that are lenient, not Conscrypt that is + // eccentric. (Neither fallback ever applied to an IP literal, which is matched against iPAddress // SANs only, so pinning Conscrypt changes nothing there.) Since Conscrypt 2.6.0 a // TrustManagerImpl falls back to the default verifier on its own // (https://github.com/google/conscrypt/pull/1060 fixing issue 1015), so nothing has to be