You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by mm...@apache.org on 2023/03/01 18:45:08 UTC
[pulsar] branch branch-2.8 updated: [improve] Simplify enabling Broker, WS Proxy hostname verification (#19674)
This is an automated email from the ASF dual-hosted git repository.
mmarshall pushed a commit to branch branch-2.8
in repository https://gitbox.apache.org/repos/asf/pulsar.git
The following commit(s) were added to refs/heads/branch-2.8 by this push:
new f8f28b120fa [improve] Simplify enabling Broker, WS Proxy hostname verification (#19674)
f8f28b120fa is described below
commit f8f28b120fa7f1ad34884041848946e9c833ca5a
Author: Michael Marshall <mm...@apache.org>
AuthorDate: Wed Mar 1 12:21:25 2023 -0600
[improve] Simplify enabling Broker, WS Proxy hostname verification (#19674)
When we merged #15818 in order to make the broker's client configurable, we did not add an explicit config for hostname verification. This PR adds that config to the broker and the websocket proxy. I chose the name `tlsHostnameVerificationEnabled` because that is what is already used in the proxy. It diverges from the function worker's config of `tlsEnableHostnameVerification`.
Before this PR, you would have enabled hostname verification by configuring `brokerClient_tlsHostnameVerificationEnable=true` in the broker and WS proxy configs. (Note that the variable name is slightly different because the `ClientConfiguration` does not have a `d` at the end of its name.
The remaining follow up work will be to update the `ClusterData` objects to configure hostname verification there to make it easier to configure hostname verification for remote clusters.
* Add `tlsHostnameVerificationEnabled` to the `broker.conf` and the `proxy.conf`
* Update all of the relevant locations that were previously only relying on `brokerClient_tlsHostnameVerificationEnable`
I added a single test to ensure that the `WebSocketProxyConfiguration` properly converts to the `ServiceConfiguration` object.
Otherwise, I verified that anywhere we are using `"brokerClient_"`, this PR also adds the right configuration.
This PR introduces a "new" configuration key, but not a new concept. All underlying behaviors are unchanged.
- [x] `doc-not-needed`
Docs are automatically updated by these changes.
(cherry picked from commit 6621fd3451a52bb7447ebeb845c83ac2f310b718)
(cherry picked from commit b8083b0e3f5a63c0fdaaf784cf2976dcff1fd68d)
(cherry picked from commit a3a242cc48e8d7454ee0c5c8fd872ae6f92ae4f7)
(cherry picked from commit c84c3b70837eead15265ddad036d16fd0351e157)
---
conf/broker.conf | 12 +++++
conf/websocket.conf | 3 ++
.../apache/pulsar/broker/ServiceConfiguration.java | 5 ++
.../org/apache/pulsar/broker/PulsarService.java | 6 ++-
.../pulsar/broker/namespace/NamespaceService.java | 3 +-
.../pulsar/broker/service/BrokerService.java | 18 ++++++--
.../apache/pulsar/compaction/CompactorTool.java | 1 +
.../apache/pulsar/websocket/WebSocketService.java | 1 +
.../service/WebSocketProxyConfiguration.java | 4 ++
.../websocket/WebSocketProxyConfigurationTest.java | 53 ++++++++++++++++++++++
10 files changed, 98 insertions(+), 8 deletions(-)
diff --git a/conf/broker.conf b/conf/broker.conf
index f485e854bc1..23fd8c9b66f 100644
--- a/conf/broker.conf
+++ b/conf/broker.conf
@@ -568,6 +568,9 @@ tlsTrustCertsFilePath=
# though the cert will not be used for client authentication.
tlsAllowInsecureConnection=false
+# Whether the hostname is validated when the broker creates a TLS connection with other brokers
+tlsHostnameVerificationEnabled=false
+
# Specify the tls protocols the broker will use to negotiate during TLS handshake
# (a comma-separated list of protocol names).
# Examples:- [TLSv1.3, TLSv1.2]
@@ -869,6 +872,15 @@ bookkeeperTLSCertificateFilePath=
# Path for the trusted TLS certificate file
bookkeeperTLSTrustCertsFilePath=
+<<<<<<< HEAD
+=======
+# Tls cert refresh duration at bookKeeper-client in seconds (0 to disable check)
+bookkeeperTlsCertFilesRefreshDurationSeconds=300
+
+# Whether the hostname is validated when the broker creates a TLS connection to a bookkeeper
+bookkeeper_tlsHostnameVerificationEnabled=false
+
+>>>>>>> c84c3b70837 ([improve] Simplify enabling Broker, WS Proxy hostname verification (#19674))
# Enable/disable disk weight based placement. Default is false
bookkeeperDiskWeightBasedPlacementEnabled=false
diff --git a/conf/websocket.conf b/conf/websocket.conf
index 57d545ae80e..3fe341b0282 100644
--- a/conf/websocket.conf
+++ b/conf/websocket.conf
@@ -92,6 +92,9 @@ brokerClientAuthenticationPlugin=
brokerClientAuthenticationParameters=
brokerClientTrustCertsFilePath=
+# Whether the hostname is validated when connecting to the broker.
+tlsHostnameVerificationEnabled=false
+
# You can add extra configuration options for the Pulsar Client
# by prefixing them with "brokerClient_". These configurations are applied after hard coded configuration
# and before the above brokerClient configurations named above.
diff --git a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java
index 1fe4d90532f..b277c7db690 100644
--- a/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java
+++ b/pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java
@@ -1104,6 +1104,11 @@ public class ServiceConfiguration implements PulsarConfiguration {
doc = "Accept untrusted TLS certificate from client"
)
private boolean tlsAllowInsecureConnection = false;
+ @FieldContext(
+ category = CATEGORY_TLS,
+ doc = "Whether the hostname is validated when the broker creates a TLS connection with other brokers"
+ )
+ private boolean tlsHostnameVerificationEnabled = false;
@FieldContext(
category = CATEGORY_TLS,
doc = "Specify the tls protocols the broker will use to negotiate during TLS Handshake.\n\n"
diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/PulsarService.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/PulsarService.java
index 74bec5e0bb3..b72c59859b4 100644
--- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/PulsarService.java
+++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/PulsarService.java
@@ -1372,6 +1372,7 @@ public class PulsarService implements AutoCloseable {
conf.setTlsCiphers(this.getConfiguration().getBrokerClientTlsCiphers());
conf.setTlsProtocols(this.getConfiguration().getBrokerClientTlsProtocols());
conf.setTlsAllowInsecureConnection(this.getConfiguration().isTlsAllowInsecureConnection());
+ conf.setTlsHostnameVerificationEnable(this.getConfiguration().isTlsHostnameVerificationEnabled());
if (this.getConfiguration().isBrokerClientTlsEnabledWithKeyStore()) {
conf.setUseKeyStoreTls(true);
conf.setTlsTrustStoreType(this.getConfiguration().getBrokerClientTlsTrustStoreType());
@@ -1434,7 +1435,8 @@ public class PulsarService implements AutoCloseable {
} else {
builder.tlsTrustCertsFilePath(conf.getBrokerClientTrustCertsFilePath());
}
- builder.allowTlsInsecureConnection(conf.isTlsAllowInsecureConnection());
+ builder.allowTlsInsecureConnection(conf.isTlsAllowInsecureConnection())
+ .enableTlsHostnameVerification(conf.isTlsHostnameVerificationEnabled());
}
// most of the admin request requires to make zk-call so, keep the max read-timeout based on
@@ -1675,7 +1677,7 @@ public class PulsarService implements AutoCloseable {
workerConfig.setTlsAllowInsecureConnection(brokerConfig.isTlsAllowInsecureConnection());
workerConfig.setTlsEnabled(brokerConfig.isTlsEnabled());
- workerConfig.setTlsEnableHostnameVerification(false);
+ workerConfig.setTlsEnableHostnameVerification(brokerConfig.isTlsHostnameVerificationEnabled());
workerConfig.setBrokerClientTrustCertsFilePath(brokerConfig.getTlsTrustCertsFilePath());
// client in worker will use this config to authenticate with broker
diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/namespace/NamespaceService.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/namespace/NamespaceService.java
index 0012c3bd825..1aa9f3a6bad 100644
--- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/namespace/NamespaceService.java
+++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/namespace/NamespaceService.java
@@ -1296,7 +1296,8 @@ public class NamespaceService implements AutoCloseable {
? cluster.getBrokerServiceUrlTls() : cluster.getServiceUrlTls())
.enableTls(true)
.tlsTrustCertsFilePath(pulsar.getConfiguration().getBrokerClientTrustCertsFilePath())
- .allowTlsInsecureConnection(pulsar.getConfiguration().isTlsAllowInsecureConnection());
+ .allowTlsInsecureConnection(pulsar.getConfiguration().isTlsAllowInsecureConnection())
+ .enableTlsHostnameVerification(pulsar.getConfiguration().isTlsHostnameVerificationEnabled());
} else {
clientBuilder.serviceUrl(isNotBlank(cluster.getBrokerServiceUrl())
? cluster.getBrokerServiceUrl() : cluster.getServiceUrl());
diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java
index 34b126d6104..401558b2408 100644
--- a/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java
+++ b/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/BrokerService.java
@@ -1150,7 +1150,9 @@ public class BrokerService implements Closeable, ZooKeeperCacheListener<Policies
configTlsSettings(clientBuilder, serviceUrlTls,
data.isBrokerClientTlsEnabledWithKeyStore(), data.isTlsAllowInsecureConnection(),
data.getBrokerClientTlsTrustStoreType(), data.getBrokerClientTlsTrustStore(),
- data.getBrokerClientTlsTrustStorePassword(), data.getBrokerClientTrustCertsFilePath());
+ data.getBrokerClientTlsTrustStorePassword(), data.getBrokerClientTrustCertsFilePath(),
+ pulsar.getConfiguration().isTlsHostnameVerificationEnabled()
+ );
} else if (pulsar.getConfiguration().isBrokerClientTlsEnabled()) {
configTlsSettings(clientBuilder, serviceUrlTls,
pulsar.getConfiguration().isBrokerClientTlsEnabledWithKeyStore(),
@@ -1158,7 +1160,9 @@ public class BrokerService implements Closeable, ZooKeeperCacheListener<Policies
pulsar.getConfiguration().getBrokerClientTlsTrustStoreType(),
pulsar.getConfiguration().getBrokerClientTlsTrustStore(),
pulsar.getConfiguration().getBrokerClientTlsTrustStorePassword(),
- pulsar.getConfiguration().getBrokerClientTrustCertsFilePath());
+ pulsar.getConfiguration().getBrokerClientTrustCertsFilePath(),
+ pulsar.getConfiguration().isTlsHostnameVerificationEnabled()
+ );
} else {
clientBuilder.serviceUrl(
isNotBlank(data.getBrokerServiceUrl()) ? data.getBrokerServiceUrl() : data.getServiceUrl());
@@ -1184,11 +1188,14 @@ public class BrokerService implements Closeable, ZooKeeperCacheListener<Policies
private void configTlsSettings(ClientBuilder clientBuilder, String serviceUrl,
boolean brokerClientTlsEnabledWithKeyStore, boolean isTlsAllowInsecureConnection,
String brokerClientTlsTrustStoreType, String brokerClientTlsTrustStore,
- String brokerClientTlsTrustStorePassword, String brokerClientTrustCertsFilePath) {
+ String brokerClientTlsTrustStorePassword, String brokerClientTrustCertsFilePath,
+ boolean isTlsHostnameVerificationEnabled) {
clientBuilder
.serviceUrl(serviceUrl)
.enableTls(true)
- .allowTlsInsecureConnection(isTlsAllowInsecureConnection);
+ .allowTlsInsecureConnection(isTlsAllowInsecureConnection)
+ .enableTlsHostnameVerification(isTlsHostnameVerificationEnabled);
+
if (brokerClientTlsEnabledWithKeyStore) {
clientBuilder.useKeyStoreTls(true)
.tlsTrustStoreType(brokerClientTlsTrustStoreType)
@@ -1226,7 +1233,8 @@ public class BrokerService implements Closeable, ZooKeeperCacheListener<Policies
conf.getBrokerClientAuthenticationParameters());
if (isTlsUrl) {
- builder.allowTlsInsecureConnection(conf.isTlsAllowInsecureConnection());
+ builder.allowTlsInsecureConnection(conf.isTlsAllowInsecureConnection())
+ .enableTlsHostnameVerification(conf.isTlsHostnameVerificationEnabled());
if (conf.isBrokerClientTlsEnabledWithKeyStore()) {
builder.useKeyStoreTls(true)
.tlsTrustStoreType(conf.getBrokerClientTlsTrustStoreType())
diff --git a/pulsar-broker/src/main/java/org/apache/pulsar/compaction/CompactorTool.java b/pulsar-broker/src/main/java/org/apache/pulsar/compaction/CompactorTool.java
index f4259d101b4..cfc73fec951 100644
--- a/pulsar-broker/src/main/java/org/apache/pulsar/compaction/CompactorTool.java
+++ b/pulsar-broker/src/main/java/org/apache/pulsar/compaction/CompactorTool.java
@@ -114,6 +114,7 @@ public class CompactorTool {
ServiceConfigurationUtils.getAppliedAdvertisedAddress(brokerConfig, true),
brokerConfig.getBrokerServicePortTls().get()))
.allowTlsInsecureConnection(brokerConfig.isTlsAllowInsecureConnection())
+ .enableTlsHostnameVerification(brokerConfig.isTlsHostnameVerificationEnabled())
.tlsTrustCertsFilePath(brokerConfig.getTlsCertificateFilePath());
} else {
diff --git a/pulsar-websocket/src/main/java/org/apache/pulsar/websocket/WebSocketService.java b/pulsar-websocket/src/main/java/org/apache/pulsar/websocket/WebSocketService.java
index acbf1db3dd9..d98302acc36 100644
--- a/pulsar-websocket/src/main/java/org/apache/pulsar/websocket/WebSocketService.java
+++ b/pulsar-websocket/src/main/java/org/apache/pulsar/websocket/WebSocketService.java
@@ -186,6 +186,7 @@ public class WebSocketService implements Closeable {
.statsInterval(0, TimeUnit.SECONDS) //
.enableTls(config.isTlsEnabled()) //
.allowTlsInsecureConnection(config.isTlsAllowInsecureConnection()) //
+ .enableTlsHostnameVerification(config.isTlsHostnameVerificationEnabled())
.tlsTrustCertsFilePath(config.getBrokerClientTrustCertsFilePath()) //
.ioThreads(config.getWebSocketNumIoThreads()) //
.connectionsPerBroker(config.getWebSocketConnectionsPerBroker());
diff --git a/pulsar-websocket/src/main/java/org/apache/pulsar/websocket/service/WebSocketProxyConfiguration.java b/pulsar-websocket/src/main/java/org/apache/pulsar/websocket/service/WebSocketProxyConfiguration.java
index 22a1384af5a..158cc154b86 100644
--- a/pulsar-websocket/src/main/java/org/apache/pulsar/websocket/service/WebSocketProxyConfiguration.java
+++ b/pulsar-websocket/src/main/java/org/apache/pulsar/websocket/service/WebSocketProxyConfiguration.java
@@ -98,6 +98,9 @@ public class WebSocketProxyConfiguration implements PulsarConfiguration {
// Path for the trusted TLS certificate file for outgoing connection to a server (broker)
private String brokerClientTrustCertsFilePath = "";
+ // Note: name matches the ServiceConfiguration name to ensure correct mapping
+ @FieldContext(doc = "Enable TLS hostname verification when connecting to broker")
+ private boolean tlsHostnameVerificationEnabled = false;
// Number of IO threads in Pulsar Client used in WebSocket proxy
private int webSocketNumIoThreads = Runtime.getRuntime().availableProcessors();
@@ -124,6 +127,7 @@ public class WebSocketProxyConfiguration implements PulsarConfiguration {
// Path for the trusted TLS certificate file
private String tlsTrustCertsFilePath = "";
// Accept untrusted TLS certificate from client
+ @FieldContext(doc = "Accept untrusted TLS certificate from client and broker")
private boolean tlsAllowInsecureConnection = false;
// Specify whether Client certificates are required for TLS
// Reject the Connection if the Client Certificate is not trusted.
diff --git a/pulsar-websocket/src/test/java/org/apache/pulsar/websocket/WebSocketProxyConfigurationTest.java b/pulsar-websocket/src/test/java/org/apache/pulsar/websocket/WebSocketProxyConfigurationTest.java
new file mode 100644
index 00000000000..3f8b1a3f533
--- /dev/null
+++ b/pulsar-websocket/src/test/java/org/apache/pulsar/websocket/WebSocketProxyConfigurationTest.java
@@ -0,0 +1,53 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.pulsar.websocket;
+
+
+import org.apache.pulsar.broker.ServiceConfiguration;
+import org.apache.pulsar.common.configuration.PulsarConfigurationLoader;
+import org.apache.pulsar.websocket.service.WebSocketProxyConfiguration;
+import org.testng.annotations.Test;
+import org.testng.Assert;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.OutputStreamWriter;
+import java.io.PrintWriter;
+
+import static org.testng.Assert.assertEquals;
+
+public class WebSocketProxyConfigurationTest {
+
+ @Test
+ public void testConfigurationConversionUsedByWebSocketProxyStarter() {
+ WebSocketProxyConfiguration config = new WebSocketProxyConfiguration();
+ // Use non-default values for testing
+ config.setTlsAllowInsecureConnection(true);
+ Assert.assertFalse(config.isTlsHostnameVerificationEnabled(), "Update me when default changes.");
+ config.setTlsHostnameVerificationEnabled(true);
+ ServiceConfiguration brokerConf = PulsarConfigurationLoader.convertFrom(config);
+ Assert.assertTrue(brokerConf.isTlsAllowInsecureConnection(),
+ "TlsAllowInsecureConnection should convert correctly");
+ Assert.assertTrue(brokerConf.isTlsHostnameVerificationEnabled(),
+ "TlsHostnameVerificationEnabled should convert correctly");
+ }
+}