+1 Default to enable the TLS hostname verification is important anywhere, which protected our application.
Should these changes be released in a minor version? For the Golang client, I think we should introduce a `NewDefaultConfig` method to create the config instead of directly new config. Thanks, Zixuan Michael Marshall <mmarsh...@apache.org> 于2023年6月3日周六 05:39写道： > > I put together PRs for the python and the C++ clients: > > https://github.com/apache/pulsar-client-python/pull/128 > https://github.com/apache/pulsar-client-cpp/pull/278 > > I am not sure the right way to change the default for the go client > because go uses the zero value for structs, and we have the variable > named "TLSValidateHostname". Would it perhaps make sense to ignore > that variable and instead use "TLSHostnameVerificationDisabled"? Then, > false would be the secure default. However, it introduces a double > negative, which might make it harder for users to understand. > > I plan to start the vote next week. > > Thanks, > Michael > > > On Wed, May 31, 2023 at 11:08 PM Michael Marshall <mmarsh...@apache.org> > wrote: > > > > Hi Pulsar Community, > > > > I am writing to start the discussion on PIP 273 to enable hostname > > verification by default. > > > > PR with PIP contents: https://github.com/apache/pulsar/pull/20453 > > > > I copy the content below (except for the associated svg of the pulsar > > network diagram). > > > > I look forward to your feedback. > > > > Thanks, > > Michael > > > > ---- > > > > # PIP 273: Enable hostname verification by default > > > > # Background knowledge > > > > When using TLS to secure a network connection from client to server, > > hostname verification is the step where the client verifies that the > > server's certificate contains the expected Subject Alternative Name or > > the Common Name. This step is essential for preventing man in the > > middle attacks where a malicious server presents a certificate that is > > cryptographically valid but was intended for a different hostname than > > the one the client is trying to connect to. > > > > [RFC 2818](https://datatracker.ietf.org/doc/html/rfc2818#section-3.1) > > provides additional details on the hostname verification process as a > > part of the TLS handshake and why a client must verify the identity of > > the server. > > > > It is very helpful to understand the network topology of a Pulsar > > Cluster when looking to enable hostname verification. Here is a > > diagram I created. It is possibly incomplete: > > > > (see PIP PR to see diagram) > > > > # Motivation > > > > The primary motivation is to improve the security of Pulsar by making > > it secure by default for hostname verification. > > > > Pulsar currently disables hostname verification by default, which led > > to some security issues in the past. For example, > > [CVE-2022-33682](https://github.com/apache/pulsar/wiki/CVE-2022-33682) > > happened because there was no way to enable hostname verification > > within the broker or the proxy. If hostname verification had been > > enabled by default, the lack of configurability would not have been a > > security concern but a usability concern. > > > > # Goals > > > > ## In Scope > > > > * Enable hostname verification by default for all clients in the > > Pulsar ecosystem. > > * Make it possible to configure hostname verification per > > geo-replicated cluster. > > * Consolidate the name of the hostname verification configuration > > option across all server components. > > > > ## Out of Scope > > > > * This PIP does not seek to enable TLS by default, which is relevant > > because hostname verification only takes affect when TLS is enabled. > > * Client configuration names will not be renamed because that would > > introduce binary incompatibility. > > * Tiered storage clients > > * ETCd client configuration -- it enables hostname verification by > > default already > > * Zookeeper client configuration -- it enables hostname verification > > by default already > > * Function worker's gRPC client to function instances -- TLS is not > > configurable for this connection > > * Function state storage to bookkeeper -- TLS is not configurable for > > this connection > > * Pulsar SQL worker's hostname verification configuration name, which > > is `pulsar.tls-hostname-verification-enable` > > > > # High Level Design > > > > * Enable hostname verification by default for all (official) clients > > in the Pulsar ecosystem. > > * Enable hostname verification by default for all server components in > > the Pulsar ecosystem. > > * Make it possible to configure hostname verification per > > geo-replicated cluster. > > * Consolidate names by using `tlsHostnameVerificationEnabled` for all > > server components. > > > > # Detailed Design > > > > ## Design & Implementation Details > > > > ### Impacted configurations > > > > * The official Pulsar clients will be impacted. > > * The broker's internal pulsar admin client, pulsar client, and > > replication clients > > * The broker's bookkeeper client and dlog bookkeeper client > > * Autorecovery's bookkeeper client > > * Bookkeeper's bookkeeper client > > * Proxy's pulsar client and pulsar admin client (used for brokers and > > function workers) > > * WebSocket Proxy's pulsar client > > * Function Worker's dlog bookkeeper client, pulsar client (used for > > broker), and pulsar admin client (used for broker and for other > > function workers) > > * Function instance pulsar client and pulsar admin client (including > > the function worker client used to download the function) > > > > ## Public-facing Changes > > > > All changes in this PIP are public facing. > > > > ### Public API > > > > ### Binary protocol > > > > N/A > > > > ### Configuration > > > > There are two name changes. For function worker config, I propose we > > replace `tlsEnableHostnameVerification` with > > `tlsHostnameVerificationEnabled` to match the other server components. > > For the client.conf config which is used by CLI tools, I propose we > > change `tlsEnableHostnameVerification` to > > `tlsHostnameVerificationEnabled`. > > > > ### CLI > > > > ### Metrics > > > > N/A > > > > # Monitoring > > > > N/A > > > > # Security Considerations > > > > This PIP will improve Pulsar's security by making it secure by default > > for hostname verification. > > > > # Backward & Forward Compatibility > > > > ## Revert > > > > Users and operators can opt out of these changes by setting the > > following configurations to `false`: > > > > | Component | Configuration | > > |----------------------------------|----------------------------------------------------------------------------------| > > | Pulsar Client | `tlsHostnameVerificationEnabled=false` or > > `enableTlsHostnameVerification(false)` | > > | Pulsar Admin Client | `tlsHostnameVerificationEnabled=false` or > > `enableTlsHostnameVerification(false)` | > > | Broker | `tlsHostnameVerificationEnabled=false` | > > | Broker's BK Client | `bookkeeper_tlsHostnameVerificationEnabled=false` | > > | Bookkeeper | `tlsHostnameVerificationEnabled=false` | > > | Autorecovery | `tlsHostnameVerificationEnabled=false` | > > | Proxy | `tlsHostnameVerificationEnabled=false` | > > | WebSocket Proxy | `tlsHostnameVerificationEnabled=false` | > > | Function Worker | `tlsHostnameVerificationEnabled: false` | > > | Function Worker's BK/DLog Client | > > `bookkeeper_tlsHostnameVerificationEnabled=false` | > > | Function Instance | TODO | > > > > ## Upgrade > > > > Ensure your certificates will correctly pass hostname verification. > > > > # Alternatives > > > > The primary alternative is to make no changes. > > > > # General Notes > > > > Here are the current draft PRs for implementing this PIP: > > > > * https://github.com/apache/pulsar/pull/20268 > > * https://github.com/apache/pulsar/pull/20269
