[feat][misc] PIP-478: default file-based TLS factory implementation

[lhotari]

PIP: #25890 (pip/pip-478.md)

Motivation

This is the second implementation PR for PIP-478, following #26222 which landed the two SPI modules (pulsar-tls-factory-api, pulsar-http-client-api). It adds the default implementation of the PulsarTlsFactory SPI and the JDK-level TLS primitives it is built from.

The PR is additive: the PIP-337 stack (PulsarSslFactory, DefaultPulsarSslFactory, JettySslContextFactory, SecurityUtility, KeyStoreSSLContext) still exists and remains the wired implementation, and the new code is exercised only by its own unit tests. There is no behavioural change to any existing module. Wiring the new factory into the client and the server components, and removing the superseded PIP-337 classes, are later PRs in the series.

Modifications

pulsar-common — org.apache.pulsar.common.tls.impl, the default factory:

• FileBasedTlsFactory + FileBasedTlsFactorySettings: the default PulsarTlsFactory, built from PEM or keystore file material. It owns the purpose→source registry, purpose resolution (direct map lookup, then the role's terminal rule — client purposes resolve to the system default, server purposes fail the request), and the reload fan-out to subscribers.
• TlsMaterialSource / MaterialSource / AuthProvidedMaterialSource / TlsMaterial: per-purpose material load, watch and cache. Value equality on the loaded material suppresses spurious rebuilds when files are touched but unchanged, a failed reload keeps the last good material, and contexts are rebuilt, never mutated in place, so a native OpenSSL engine never observes a torn context.
• TlsContexts / TlsContextAcquisition / SynthesizedEngineSslContext / TlsSynthesisSpec: SSL context assembly, per-acquisition read of the current context, and SSLParameters companion synthesis.
• TlsKeyStoreLoader, TlsFactoryProbe, TlsReloadMetrics, package-info.

pulsar-common — org.apache.pulsar.common.util.tls, the JDK-level primitives:

• PemReader (PEM parsing), JdkSslContexts (JDK SSLContext assembly), JcaProviders (provider resolution), JcaKeyStores (KeyStore creation through a pinned provider). Extracted from the SecurityUtility grab-bag.
• Together these implement both provider axes carried by TlsPolicy: jsseProvider pins the SSLContext / KeyManagerFactory / TrustManagerFactory, and jcaProvider pins the KeyStore / CertificateFactory / KeyFactory engines that parse the material. Both are unset by default, which is exactly today's behaviour.
• KeyStoreHolder gains an optional pinned JCA provider and, on that path only, a random per-instance entry password (a FIPS provider in approved-only mode rejects an empty password-based-KDF password). With no pinned provider its behaviour is unchanged.

pulsar-broker-common:

• broker/tls/DefaultBrokerTlsFactory + TlsFactorySupport: the default broker-side factory, composed from the existing base tls* fields of ServiceConfiguration. No new configuration keys are added in this PR — those arrive with the migration PR.
• jetty/tls/JettyTlsFactory: the framework-side Jetty reload driver built on the new factory.

Engine selection. An unset tlsProvider selects Netty's OPENSSL_REFCNT when a netty-tcnative binary is available for the platform, else the JDK engine. This preserves the historical default — the PIP-337 path passes a null provider and SslContextBuilder then picks the native engine — while keeping the new factory off finalization: OpenSslContext frees its native SSL_CTX from a finalize(), which is deprecated for removal (JEP 421) and can already be disabled with --finalization=disabled. Both variants expose real reference counting; this factory owns and deterministically releases every context it builds, so the finalizer only ever masked a bug. An explicitly configured engine literal is honoured verbatim.

tlsProvider and webServiceTlsProvider are now documented as the distinct axes they are — a Netty engine versus a Jetty JSSE (SSLContext) provider — in ServiceConfiguration, ProxyConfiguration, broker.conf, standalone.conf and proxy.conf. No default values change.

Relationship to FIPS and to PIP-489

The main focus of this PR is the default TLS factory above. It does, however, lay the groundwork for a FIPS-compliant TLS transport, which is the subject of PIP-489 (discussion).

PIP-489's own Goals section states that it delivers a FIPS-capable TLS transport by building on PIP-478 rather than defining a parallel TLS configuration path. Because PIP-478 replaces the PIP-337 PulsarSslFactory / PulsarSslConfiguration surface outright in 5.0, parts of the PIP-489 design that are written against that surface — the per-listener .tlsProvider(...) wiring gaps in PulsarChannelInitializer, WebService, the WebSocket proxy and the function worker — become obsolete on master: those call sites do not survive the migration, and the equivalent capability is expressed through TlsPolicy and the factory instead. The remaining PIP-489 scope — the BC/BC-FIPS packaging story, the non-approved algorithms in the crypto and authentication paths, FIPS documentation, and the distribution variant — is unaffected by this PR.

The FIPS-relevant pieces here are:

• Both provider axes are implemented, including the key/trust-manager algorithm negotiation a FIPS JSSE provider needs (BCJSSE registers KeyManagerFactory.X.509 with a PKIX alias but not the JDK-default SunX509), and pinned in-memory carrier keystores so parsed key material stays inside the pinned module.
• Pulsar had no BouncyCastle JSSE dependency at all (see pulsar-site#974). bctls is added to the version catalog, and JcaProviders registers BCJSSE on demand when it is the pinned jsseProvider: an operator-registered instance always wins, otherwise it is constructed from the classpath — in FIPS mode bound to the FIPS JCA provider when that is the BouncyCastle artifact present, and in the default mode otherwise. bctls ships no META-INF/services/java.security.Provider entry and its FIPS mode is a constructor argument while it registers under the plain name BCJSSE either way, so neither ServiceLoader discovery nor default construction can produce the right provider — hence the explicit registration.
• BouncyCastle is no longer mandatory: it is resolved lazily and reported as an Optional, so a deployment without it on the classpath still resolves named providers.

Deliberately not in this PR: bctls is a test dependency only, matching how bcprov is already scoped in pulsar-common (loaded reflectively in main). Which BouncyCastle artifacts land on the Pulsar classpath — selecting the non-FIPS or the FIPS jars via an environment variable in the bin/pulsar script, since the two cannot coexist on one classpath — will come in a later PR, together with the distribution and LICENSE wiring that implies.

Where the rest of PIP-478 goes

Subsequent implementation PRs in this series will cover the migration itself, and with it the TLS-transport essence of PIP-489:

1. Core migration — the asynchronous v5 client authentication engine and its v4 compatibility bridges, plus wiring the TLS factory into every server component (broker binary listener and web service, proxy, WebSocket proxy, function worker) and the client. This introduces the new configuration surface (tlsFactoryClassName, jsseProvider / jcaProvider and their broker-client counterparts) and makes the new SPI the TLS path in use — which is what turns the engine and provider selection above into something an operator can actually configure end to end.
2. Authentication plugin migration — the Athenz and SASL plugins onto the v5 asynchronous SPI.
3. PIP-337 removal — deleting PulsarSslFactory, SecurityUtility, KeyStoreSSLContext and the deprecated CN-based hostname matching, leaving the new SPI as the only TLS path.

Verifying this change

This change added tests and can be verified as follows:

• FileBasedTlsFactoryTest, TlsMaterialSourceTest, AuthProvidedMaterialFoldTest — context building, purpose resolution, rotation with keep-last-good, and rebuild-not-mutate semantics, exercised with real in-memory TLS handshakes (no broker, no sockets), including the OpenSSL engine path and a reference-count-reaches-zero assertion (there is no finalizer to fall back on with OPENSSL_REFCNT).
• SslParametersSynthesisTest, TlsReloadMetricsTest — SSLParameters companion synthesis and the pulsar.tls.reload metric.
• JcaProviderPinningTest, JcaProvidersTest, JdkSslContextsTest — the two provider axes: name resolution and ordering, key/trust-manager algorithm negotiation, material loading through a pinned JCA provider, that the Netty builders route their carrier keystore and manager factories through the pins rather than letting Netty build them via the JVM search order, and BCJSSE on-demand registration (including that it supplies SSLContext.TLS and TrustManagerFactory.PKIX but not SunX509).
• PemReaderTest — PEM preamble parsing across leading-line parities.
• TlsFactorySupportTest, JettyTlsFactoryTest — the broker-side scaffolding and the Jetty reload driver.
• ./gradlew :pulsar-common:test :pulsar-broker-common:test — 1261 tests, 0 failures.
• ./gradlew assemble rat spotlessCheck checkstyleMain checkstyleTest checkBinaryLicense passes.

Does this pull request potentially affect one of the following parts:

If the box was checked, please highlight the changes

• Dependencies (add or upgrade a dependency) — adds org.bouncycastle:bctls-jdk18on to the version catalog and as a test dependency of pulsar-common. Nothing new ships in the distribution.
• The public API — adds the org.apache.pulsar.common.tls.impl and org.apache.pulsar.common.util.tls packages, and the org.apache.pulsar.broker.tls / org.apache.pulsar.jetty.tls classes. All additive and unreferenced by existing code in this PR. KeyStoreHolder gains one method and one constructor overload; its existing behaviour is unchanged.
• The schema
• The default values of configurations
• The threading model
• The binary protocol
• The REST endpoints
• The admin CLI options
• The metrics
• Anything that affects deployment

Documentation

• doc-required
• doc-not-needed — the new implementation is not wired to any user-facing surface yet, and the PIP-478 design document covers the design. The tlsProvider / webServiceTlsProvider comments in the shipped conf/*.conf files are updated here to describe the two provider axes accurately; no website documentation change is needed.
• doc
• doc-complete

Matching PR in forked repository

PR in forked repository: lhotari#248 (Personal CI — full matrix green)

_{This PR was prepared with the assistance of Claude Code (Opus 5); the change was reviewed and is submitted by a human contributor who takes responsibility for it, per the ASF Generative Tooling guidance.}

[david-streamlio]

Detailed comments inline. The summary — and the one issue I'd call major — are in the conversation comment on this PR, so I won't repeat them here.

Short version: the change is additive and reads well, and almost all of my concerns are about what the migration PR inherits rather than defects in this one. Nothing blocking on its own.

[lhotari]

Thanks — this is a careful review, and it found two things that two prior multi-model review passes on this branch missed: the coordinator's whenComplete running inline under the source monitor (the default factory never returns an incomplete SSLParameters future), and the isStarted() TOCTOU that drops a rotation silently.

Every comment has an inline reply. 17 of the 19 are addressed in 15 commits, one per comment where that made sense.

Behaviour changes

• WEB purpose honours webServiceTlsProvider / webServiceTlsProtocols / webServiceTlsCiphers, falling back to the binary-listener keys — a3d135b
• Jetty reload moved off the delivery thread onto a supplied executor — a4832cd
• Jetty apply became an unconditional reload(...), closing the start race — a4832cd
• Synchronous builders document the blocking-executor constraint and unwrap CompletionException — a4832cd
• Material poll runs on the blocking executor, not the framework scheduler — 49ef9eb
• Re-entrant acquisition fails loudly instead of deadlocking — 373d462
• Hostname verification pinned off on server-role policies — 984fd9d
• Rotation detection uses size and file identity, not mtime alone — 065b304
• Insecure-mode warn set no longer retains policies (and their passwords) — 0344337

Hygiene, docs and tests

• Redundant compileOnly opentelemetry-api and its incorrect comment deleted — 21a9afb
• JcaProviders resolution javadoc corrected and completed — ccc2240
• Named class-initialization call replaces the empty if — 7316728
• SubscriptionHandle.dispose matches OneShotHandle's guard — 01c848e
• Factory-wide client-auth documented as deliberate PIP-337 parity — 0aa644b
• The three fail-closed material tightenings documented for the upgrade notes — 9d3ea3c
• BouncyCastle key value-equality pinned by a test — a9e50fe
• Test-provider visibility rationale recorded — 50d9109

Not code changes here, deliberately

• The auth-overlay re-parse: an auth-data identity check would miss plugin-internal rotation, so the workable form is mtime-stats of the plugin's file paths. Better landed in the PR that decides the acquisition pattern, where it can be measured rather than guessed.
• The Conscrypt global-verifier constraint: getting a tracking issue linked from PIP-478, as you suggested.

One I am pushing back on

opentelemetry-api is on the runtime classpath transitively via pulsar-tls-factory-api's api declaration, so the NoClassDefFoundError cannot occur. But the compileOnly line that prompted the question was redundant and its comment was wrong, so both are deleted — details in that thread.

Two points on the webServiceTls* comment describe master's existing behaviour rather than a regression this series introduces; evidence in that thread, along with an error of ours in the PIP text that your comment flushed out.

Verification. 1270 tests, 0 failures across pulsar-common and pulsar-broker-common, plus assemble rat spotlessCheck checkstyleMain checkstyleTest checkBinaryLicense. Full CI matrix running.

One ask: the review body mentions a summary and "the one issue I'd call major" in a conversation comment, but I don't see one on the PR — could you re-post it? I don't want to answer around it.

[david-streamlio]

Thanks for the thorough turnaround — I re-read the delta (15 files, +550/-86) against the prior base and most of it lands cleanly. Verified as correct: the poll dispatch with the pollInFlight flag, the FileStamp record (mtime + size + fileKey() from one stat, which catches the write-to-temp-and-rename rotation that cert-manager and friends use), the AtomicBoolean symmetry on SubscriptionHandle.disposed, bounding INSECURE_WARNED by hashCode so TlsPolicy objects (and their plaintext passwords) aren't pinned for the JVM lifetime, webPolicy() restoring WEB precedence for webServiceTls*, enableHostnameVerification(false) pinned on the server roles, and the unconditional target.reload() — I decompiled Jetty 12.1.10 and confirmed reload() and doStart() do share the same AutoLock, so the TOCTOU fix is sound.

Two of the fixes don't do what they say, though, and in both cases the accompanying test cannot fail. I mutation-tested both rather than argue from reading:

1. (major) the re-entrancy guard is evaluated only after the hop to blockingExecutor, so it's a no-op under any real executor — the original deadlock is unchanged. Running the shipped test verbatim against a real single-threaded executor gives TimeoutException, not the IllegalStateException it asserts.
2. (moderate) reverting the whenCompleteAsync(..., reloadExecutor) fix back to inline whenComplete leaves JettyTlsFactoryTest at 17/17 green, rotationReloadRunsOnTheSuppliedExecutorNotTheDeliveryThread included.

Plus three minor items. Details, evidence and suggested fixes inline.

[lhotari]

Thanks — this one landed harder than the last. Both mutation results reproduce, and you're right about the cause in each case: the re-entrancy guard fired only under a direct executor, which is the one configuration where the deadlock cannot occur, and the reload-executor test asserted the executor's own thread naming rather than where publish ran.

All six are addressed in 5 commits. The two substantive ones:

• 323e0cc — the re-entrancy check moves to the caller thread, pre-dispatch, in both acquisition paths, with resolve(purpose) wrapped so its failure stays a failed future rather than a synchronous throw (the SPI forbids that, so the suggested snippet needed one amendment). The test now uses a real single-threaded executor with bounded waits and covers a rotation as well as the initial delivery.
• afa9268 — the dispatch test now gates the reload executor with a synchronously-completing companion. Gating the companion future, as suggested, would not have failed under your mutation: an inline whenComplete on an incomplete future also returns promptly.

Plus 82019a6 (executor required, validated at both entry points so the native-factory path can't bypass it), c7a8542 (@param) and 4487e52 (Javadoc move).

Every changed test in this round was mutation-tested against the unfixed code before I claimed it works — that was the gap in my previous round, and it's a fair correction to have had made twice.

1269 tests, 0 failures locally, plus the full lint/license gate. I've resolved the threads from the first review; these six are the open ones.

[david-streamlio]

All six items from the last round are addressed, and the two substantive ones are now backed by tests that can actually fail. I re-ran the mutations myself at 4487e52118 rather than take the commit messages for it:

Pre-dispatch guard (323e0cca8c) — removing the Thread.holdsLock(source) check from both acquisition entry points:

FileBasedTlsFactoryTest > reentrantAcquisitionFromAReloadCallbackFailsLoudlyInsteadOfDeadlocking FAILED
    java.util.concurrent.TimeoutException
        at FileBasedTlsFactoryTest.java:751

Worth noting the bounded wait does what you said it would: the regression surfaces as a failure in 46s, not as a hung suite.

Reload dispatch (afa92685af) — reverting whenCompleteAsync to inline whenComplete:

JettyTlsFactoryTest > rotationReloadRunsOnTheSuppliedExecutorNotTheDeliveryThread FAILED
    expected ... to refer to the same object
        at JettyTlsFactoryTest.java:646

That's the "must be dispatched, not run inline on the delivery thread" assertion, i.e. the one that matters. Clean run at HEAD: BUILD SUCCESSFUL, both classes green.

Corrections to my review, which you were right about

Four things in my comments were wrong, and three of them would have made the code worse if you'd applied them verbatim:

1. resolve(purpose) is not "just a registry lookup." I said moving it to the caller thread was free. It isn't — FileBasedTlsFactory:411-422 does Objects.requireNonNull and throws TlsMaterialUnavailableException for an unconfigured server purpose, and the SPI javadoc's "Never throws synchronously" clause covers argument validation explicitly. My snippet would have violated the SPI contract on the calling thread. Your try/catch is the right shape.

2. Gating the companion future doesn't discriminate. You're right and I should have caught this: registering an inline whenComplete on an incomplete future also returns promptly, so the delivery thread comes back under both variants and my proposed assertions would have passed under my own mutation. Gating the reloadExecutor with the companion resolving synchronously is the correct discriminator — and it's the real default-factory path, since SSLParameters is unsupported.

3. The line-657 javadoc is on RegisteredSource.deliverToSubscribers, not SubscriptionHandle. Confirmed.

4. "Every production caller would pass null" — wrong, and my own comment contradicted it a paragraph later. There are no production callers at all, which is exactly what makes tightening it free now.

Your sharpening on (1) is the part I'd want in the commit log, and it's there: under a direct executor the deadlock is unreachable anyway because monitors are reentrant, so the old guard fired only where nothing could hang and turned a working case into a failure. That's a better characterization than my three-way split.

Also agree on the requireNonNull placement — the native-factory path returns at line 175-178 before the coordinator is constructed, so my suggested placement would have let a null through there. And the point that non-null ≠ asynchronous is the right one: it's the dispatch test, not the null check, that holds the invariant.

Two non-blocking notes

• The Conscrypt follow-up issue doesn't appear to be filed yet. You said it would be opened and linked from PIP-478; I don't see a link in pip/pip-478.md at HEAD. Not a blocker for this PR — the substance is recorded in the review thread — but it's the one commitment here that lives outside the code, and it gates a different PR (the one turning SAN-only verification on by default). Worth filing before this merges so the link exists.

• @nodece's suggestion on DefaultBrokerTlsFactory:168 looks already handled by design rather than missed: that's the server-role policy, and brokerClientPolicy() does map conf.isTlsHostnameVerificationEnabled() at line 196. tlsHostnameVerificationEnabled is documented as the broker's outbound setting, so applying it to a server policy would mean verifying the client's hostname. The comment at 158-166 explains this, but since it reads as unaddressed, an explicit reply pointing at line 196 would probably close it out.

CI is red on infrastructure, not code — Preconditions died on Failed to resolve action download info. Error: Service Unavailable and everything downstream was skipped. Needs a re-run.

Approving. Thorough responses throughout this round — particularly catching that my own remediation for the tautological test was itself untestable.