gaborgsomogyi opened a new pull request, #29271: URL: https://github.com/apache/flink/pull/29271
## What is the purpose of the change This is backport of https://github.com/apache/flink/pull/29265. This pull request fixes two related SSL/TLS bugs found while auditing `security.ssl.protocol`. `security.ssl.protocol` is documented as not supporting a comma-separated list of protocols, but that is only true for one of its three consumers. The internal Netty data transport, the REST endpoints, and the Blob server/client already support a comma-separated list correctly (they build a generic SSL context and narrow it with the full protocol array). Only the Pekko-based RPC control-plane path breaks: the value is spliced unquoted into the generated HOCON config, so any comma in it causes a config parse failure, and even past that, Pekko's own `ConfigSSLEngineProvider` only accepts a single protocol name. While adding end-to-end negotiation tests for every consumer of this option, a second, unrelated and more serious issue surfaced: internal SSL is documented to skip hostname verification, because it is designed around one shared, mutually-trusted certificate distributed to every node in the cluster (this is what makes container-based setups such as Kubernetes practical - a certificate cannot realistically carry a Subject Alternative Name for every dynamically-scheduled pod). A prior cleanup of the SSL hostname-verification configuration removed the explicit override that enforced this for internal SSL, without replacing it. Since Netty enables hostname verification by default from 4.2 onward, internal SSL has since been silently verifying the peer hostname against the shared certificate on both the JDK and OpenSSL providers, and will fail every connection whose resolved hostname is not covered by that certificate - effectively any real multi-node deployment. Together, this pull request: - Fixes the Pekko RPC path so it honors a full comma-separated protocol list like the other consumers - Restores the documented behavior that internal SSL does not require the peer hostname to match the shared certificate - Corrects the `security.ssl.protocol` documentation to describe the actual (now consistent) behavior - Adds regression tests that lock in the negotiation behavior and the hostname-verification behavior for every consumer of internal SSL ## Brief change log - *Quote the `protocol` value when building the Pekko remoting HOCON config in `PekkoUtils`, fixing a config parse failure on any comma-separated value* - *Override `createServerSSLEngine()`/`createClientSSLEngine()` in `CustomSSLEngineProvider` to build the SSL engine from the full split protocol list instead of relying on Pekko's own single-protocol `ConfigSSLEngineProvider` implementation* - *Explicitly disable endpoint identification for internal SSL in `SSLUtils#createInternalNettySSLContext`, restoring hostname-verification-free negotiation against the shared certificate on both the JDK and OpenSSL providers* - *Update `SecurityOptions.SSL_PROTOCOL`'s description to state that a comma-separated list is supported and how the highest mutually supported protocol is negotiated* - *Regenerate the affected configuration documentation snippets* - *Add regression tests covering multi-protocol negotiation for internal SSL, REST SSL, Blob SSL, and Pekko RPC SSL* - *Add a regression test asserting that internal SSL negotiates successfully even when the peer hostname does not match the shared certificate* ## Verifying this change This change added tests and can be verified as follows: - *Added `CustomSSLEngineProviderTest`, which builds a real `ActorSystem` from the actual production config path and drives a real socket-based TLS handshake through the fixed `CustomSSLEngineProvider`, asserting negotiation to TLSv1.3 when both sides support it and a graceful fallback to TLSv1.2 when no TLSv1.3 cipher suite is configured* - *Extended `SSLUtilsTest` with equivalent real-handshake regression tests for the internal Netty data transport, REST, and Blob server/client SSL paths, asserting the same negotiate-or-fall-back behavior, run against both the JDK and OpenSSL providers* - *Added `SSLUtilsTest#testInternalSSLIgnoresPeerHostMismatch`, which connects to a real socket while telling the client engine to verify a fabricated hostname the shared certificate was never meant to cover, and asserts negotiation still succeeds on both the JDK and OpenSSL providers* - *Confirmed all added tests fail before the corresponding fix and pass after, and ran the full set of existing SSL-related test classes in `flink-runtime` with the OpenSSL provider enabled to confirm no regressions* ## Does this pull request potentially affect one of the following parts: - Dependencies (does it add or upgrade a dependency): no - The public API, i.e., is any changed class annotated with `@Public(Evolving)`: yes, `SecurityOptions` is `@PublicEvolving`; the change only clarifies and extends the accepted value format of an existing option and restores previously-documented behavior, it does not change any method signature - The serializers: no - The runtime per-record code paths (performance sensitive): no - Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: yes - this restores internal SSL connectivity between JobManager and TaskManager for any deployment where the shared certificate does not carry every node's hostname as a Subject Alternative Name, which is the common case for container-based deployments such as Kubernetes - The S3 file system connector: no ## Documentation - Does this pull request introduce a new feature? no - If yes, how is the feature documented? docs (updated `security.ssl.protocol` description, reflected in the generated configuration docs) --- ##### Was generative AI tooling used to co-author this PR? - [X] Yes (Claude Code) <!-- Generated-by: Claude Code (Sonnet 5) --> -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
