change tlsVersion to minTlsVersion but keep the tls-version config name - #930
Draft
pjfanning wants to merge 1 commit into
Draft
change tlsVersion to minTlsVersion but keep the tls-version config name#930pjfanning wants to merge 1 commit into
pjfanning wants to merge 1 commit into
Conversation
pjfanning
marked this pull request as draft
August 10, 2026 00:03
pjfanning
force-pushed
the
min-tls-version
branch
from
August 10, 2026 00:09
b5b3789 to
f2677d2
Compare
pjfanning
force-pushed
the
min-tls-version
branch
from
August 29, 2026 20:34
f2677d2 to
3b5831f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
PemManagersProvider.createSslContexttook atlsVersionand passed it straight toSSLContext.getInstance(tlsVersion). That name promises more than it delivers: the argumentselects a context provider, it does not pin the protocol actually negotiated. On JDK 17
getInstance("TLSv1.3")yields a context whose engines enable[TLSv1.3, TLSv1.2], so theonly configuration that really restricted anything was the
TLSv1.2default, and it did so byalso excluding TLSv1.3.
Renaming the parameter to
minTlsVersionmakes the intent explicit, and requires actuallyenforcing a minimum rather than relying on a side effect of provider selection.
The enforcement point matters. An
SSLContexthas no mutable protocol list —getDefaultSSLParameters()returns a fresh copy on every call and there is nosetDefaultSSLParameters— so a minimum can only be applied to theSSLSocketorSSLEnginethat carries the connection.
Modification
createSslContextno longer takes a version at all; it builds an unrestricted"TLS"context.Its overloads take
StringorOption[String]for the CA path, whereNonemeans the defaultJVM trust store (previously open-coded in
HttpContactPointBootstrap).protocolsAtOrAbove(minTlsVersion, supportedProtocols)returns the satisfying subset andthrows
IllegalArgumentExceptionfor an unknown version or an unsatisfiable minimum. Protocolsoutside the known TLS ordering (
SSLv3, theSSLv2Hellopseudo-protocol) are never selected.configureClientEngine(engine, minTlsVersion)applies that set viaengine.setEnabledProtocols, sets client mode, and re-appliessetEndpointIdentificationAlgorithm("https")—ConnectionContext.httpsClient(sslContext)sets that itself, so the engine-factory overload must restore it or hostname verification is
silently lost.
management-cluster-bootstrap) now build their connection context from an engine factory.
tlsVersion->minTlsVersion. Thetls-versionconfig key isunchanged, so no user configuration has to change; the reference.conf comments now say
"minimum TLS version".
Effective behaviour change: with the default
tls-version = "TLSv1.2", TLSv1.3 is now enabledas well and TLSv1.2 remains the floor.
tls-version = "TLSv1.3"now genuinely excludes TLSv1.2,which it did not before.
Result
tls-versionmeans what its name says: a floor, enforced on the connection. ConfiguringTLSv1.3no longer silently continues to accept TLSv1.2.Tests
management-pki/.../PemManagersProviderSpec(9 tests) asserts ongetEnabledProtocolsof a real engine, plus client mode and thehttpsendpointidentification algorithm.
getSSLParameterscopy) fails"should actually restrict the protocols enabled on the engine"with
List("TLSv1.3", "TLSv1.2") was not equal to List("TLSv1.3"). The rest of the suite stillpasses, which is why the previous settings-only tests did not catch this.
HttpContactPointBootstrapSpecgains a case asserting the configured minimum reaches theengine and that endpoint identification survives.
sbt "management-pki/test" "discovery-kubernetes-api/test" "lease-kubernetes/test"— 115succeeded, 0 failed, 1 pending.
sbt "management-cluster-bootstrap/testOnly ...HttpContactPointBootstrapSpec"— 6 succeeded.mimaReportBinaryIssueson management-pki, discovery-kubernetes-api, lease-kubernetes,management-cluster-bootstrap, rolling-update-kubernetes — all success.
scalafmt --mode diff-ref=origin/mainandsbt +headerCheckAll— clean.References
None -
createSslContextgained itstlsVersionparameter earlier on this branch; this correctsit before it ships.