[FIP-29] Add SSL config parsing and SslContext/SslHandler factory - #3813
[FIP-29] Add SSL config parsing and SslContext/SslHandler factory#3813MicheleGuerriero wants to merge 1 commit into
Conversation
First building block of FIP-29 (mTLS support), see apache#3796. Adds the server-side security.ssl.* and client-side client.security.ssl.* config options (protocols, cipher suites, keystore/truststore, endpoint identification, reload interval), an immutable SslConfig holder parsing them with fail-fast validation, and an SslContextFactory building Netty SslContext/SslHandler instances for server and client (JDK provider, per-listener client-auth requirement, SNI + hostname verification). These are currently-unused building blocks; later tickets wire them into the Netty pipeline. No runtime behavior change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
affo
left a comment
There was a problem hiding this comment.
The work looks very good and aligned with the FIP 1:1, thank you! 🤝
I only have a couple doubts marked as a comments 🤝
| // Server-side TLS (transport encryption + mTLS) options | ||
| // ------------------------------------------------------------------------ | ||
|
|
||
| public static final ConfigOption<List<String>> SERVER_SSL_ENABLED_LISTENERS = |
There was a problem hiding this comment.
I wonder if there is a way to mark these options as "experimental" for the time being 🤔
My concern is that these options are now exposed to the outer world without anything wired, and this may compromise the release (say that we don't finalize the feature and it is half baked).
I would need the feedback of a committer here, or a PMC even :) @fresh-borzoni would you feel like it?
| } | ||
|
|
||
| /** Build and validate the server-side TLS configuration. */ | ||
| public static SslConfig fromServerConfig(Configuration conf) { |
There was a problem hiding this comment.
I do understand that this contribution does not switch yet on SERVER_SSL_ENABLED_LISTENERS to enforce the validation that remains implicit, but this also makes sense as we don't have yet the wiring and those methods stay not invoked for now.
In the future one should extract the SSL config by checking that.
But that would turn to be cumbersome as part of the logic of checking whether SSL is enabled would fall out of the config class itself.
I wonder if it would be a neater approach to embed right now all this knowledge into this class, may return an Optional to embed the semantics of the fact that SSL may be there or not.
Purpose
Linked issue: close #3796
First building block of FIP-29 (mTLS support for Fluss RPC). This adds
the configuration surface and a factory for building Netty SSL
contexts/handlers, but does not wire them into any pipeline yet — no
runtime behavior changes as a result of this PR.
Brief change log
ConfigOptions: add server-sidesecurity.ssl.*options (enabledlisteners, protocols, cipher suites, keystore/truststore paths and
passwords, reload interval) and client-side
client.security.ssl.*options (mirrors the server options, plus endpoint identification
algorithm for hostname verification).
SslConfig: new immutable holder that parses and validates theabove options from a
Configuration, withfromServerConfig/fromClientConfigfactory methods. Fails fast with a clear messageif a server enables TLS without configuring a keystore.
SslContextFactory: new factory building NettySslContext/SslHandlerinstances from anSslConfig, using the JDK SSLprovider. Supports per-listener client-certificate requirement
(
setNeedClientAuth, for mTLS listeners) on the server side, andSNI + hostname verification on the client side.
Later tickets (#3792 server pipeline, #3797 client pipeline) will wire
SslContextFactoryinto the actual Netty channel pipelines.Tests
Added
SslContextFactoryTest, backed byTestSslUtils(generates aself-signed certificate and on-the-fly JKS keystore/truststore files,
no committed key material):
SslContextcreationverification on/off)
needClientAuthtoggling for mTLSclient
SslHandler, asserting the negotiated session uses a realTLS protocol/cipher (not plaintext)
Verified locally on
fluss-commonandfluss-rpc:mvn test(allgreen,
SslContextFactoryTest9/9),spotless:check,checkstyle:check, Apache RAT license check, and a JDK 8 build(
-Pjava8) confirming Java 8 source compatibility.API and Format
Adds new
ConfigOptions (security.ssl.*,client.security.ssl.*)and two new
@Internalclasses influss-rpc(
org.apache.fluss.rpc.netty.ssl). No changes to existing publicAPIs, RPC wire format, or storage format. The new classes are
currently unused by any production code path.
Documentation
No user-facing behavior yet, so no documentation changes in this PR.
Configuration docs for
security.ssl.*/client.security.ssl.*will be added once these options take effect (server/client pipeline
wiring in the following tickets).