apache / apache/pulsar

[fix][misc] PIP-478: webServiceTls{Ciphers,Protocols} and the proxy lookup leg's brokerClientTls{Ciphers,Protocols} are not applied

Open
#26,386 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Java
Stars
15.3k
Forks
3.8k
Avg merge
1d 14h
Merged PRs (30d)
160

Description

### Motivation

Two components accept a TLS cipher/protocol restriction that never reaches the connection it names.

**1. The broker's web listener ignores `webServiceTlsCiphers` / `webServiceTlsProtocols`.**

`WebService.createTlsFactoryWebServer` passes the *binary-listener* keys to Jetty:

```java
JettyTlsFactory.createReloadingServerFactory(this.tlsFactory, TlsPurpose.WEB, pulsar.getExecutor(),
config.getWebServiceTlsProvider(), config.isTlsRequireTrustedClientCertOnConnect(),
config.isTlsAllowInsecureConnection(), config.getTlsCiphers(), config.getTlsProtocols());
```

The three sibling components pass the web keys at the same call site — `pulsar-proxy` `WebServer`,
`pulsar-websocket` `ProxyServer` and functions `WorkerServer` all pass
`config.getWebServiceTlsCiphers(), config.getWebServiceTlsProtocols()`. Those arguments are what reach
the engine via `JettyTlsFactory.applyServerConfig` → `setIncludeCipherSuites` / `setIncludeProtocols`.

The composed policy cannot compensate. `DefaultBrokerTlsFactory.webPolicy` does resolve
`firstNonEmpty(getWebServiceTlsProtocols(), getTlsProtocols())`, but the `WEB` purpose is only acquired
as `SSLContext` and `SSLParameters`, and `FileBasedTlsFactory.isSupported` is

```java
return instanceClass == SslContext.class || instanceClass == SSLContext.class;
```

so the `SSLParameters` companion resolves to `Optional.empty()`, the baseline is null, and the policy's
protocol/cipher lists reach nothing. That half of `webPolicy` is dead code.

`DefaultBrokerTlsFactory`'s class javadoc nonetheless states the opposite:

> The web listener has its own provider/protocol/cipher keys, and they take precedence over the
> binary-listener ones when set — webServiceTlsProvider, webServiceTlsProtocols, webServiceTlsCiphers

Both keys are shipped in `conf/broker.conf`.

**2. The proxy's binary lookup leg ignores `brokerClientTlsCiphers` / `brokerClientTlsProtocols`.**

The proxy has three outbound legs. The direct binary leg and the admin HTTP leg build from
`ProxyTlsFactories.brokerClientPolicy(config)`, which sets `.protocols(...)` and `.ciphers(...)` from
those keys. The lookup leg gets its TLS from `ProxyConnection.createClientConfiguration`, which sets the
trust/cert/key paths, the keystore fields, the insecure and hostname-verification flags, the three
provider pins and the factory selection — but never calls `setTlsCiphers` or `setTlsProtocols`.
`ClientTlsFactorySupport.clientDefaultPolicy` then reads the untouched `new TreeSet<>()` defaults.

`ProxyConfiguration.brokerClientTlsCiphers` is a real field whose `@FieldContext` says it is "used by the
Pulsar proxy to authenticate with Pulsar brokers". The sibling component does propagate it —
`PulsarService` does `conf.setTlsCiphers(getConfiguration().getBrokerClientTlsCiphers())` for the broker's
own outbound client.

### Impact

An operator who narrows the negotiated protocol or cipher set for these connections gets the restriction
applied to some of them and silently not to others: on the broker, the REST/admin listener keeps
negotiating from the framework default floor while the binary listener is restricted; on the proxy, the
partitioned-metadata / get-topics / get-schema lookups negotiate unrestricted while the data and admin
connections are restricted. The same `broker.conf` applied to a proxy or worker *does* restrict, so the
difference is invisible in a mixed deployment and in any config-driven review.

### Note on history

Both are pre-existing rather than introduced by PIP-478 (`git show bb4768d4902~1` shows the same wiring
in `WebService` and in `ProxyConnection.createClientConfiguration`). What PIP-478 added is the
`webPolicy` resolution and the javadoc asserting a precedence the code does not implement, and the
provider pins next to the missing cipher/protocol pair on the proxy leg.

### Suggested fix

- `WebService`: pass `config.getWebServiceTlsCiphers()` / `getWebServiceTlsProtocols()`, falling back to
the binary keys when empty (matching `webPolicy`'s `firstNonEmpty`); or, if the binary-key behaviour
must be preserved, delete the dead resolution from `webPolicy` and correct its javadoc.
- `ProxyConnection.createClientConfiguration`: add `setTlsCiphers` / `setTlsProtocols` from the
`brokerClientTls*` keys, mirroring `PulsarService`.
- Pin both with assertions on the resulting configuration.

---

Found in a final-state review of the PIP-478 series (#26321).

Contributor guide

Open the contributing guide

Research direction

Start with WebService.createTlsFactoryWebServer and ProxyConnection.createClientConfiguration, then trace JettyTlsFactory.applyServerConfig and the existing PulsarService propagation. Compare the broker and proxy TLS configuration paths and inspect nearby tests or assertions. Done means both named connection paths receive the configured cipher and protocol restrictions, with assertions covering the resulting configuration.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
backend, security
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
52/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.