Support TLS cipher suite selection in client-v2 and jdbc-v2#2919
Open
polyglotAI-bot wants to merge 4 commits into
Open
Support TLS cipher suite selection in client-v2 and jdbc-v2#2919polyglotAI-bot wants to merge 4 commits into
polyglotAI-bot wants to merge 4 commits into
Conversation
Neither client could restrict the negotiated TLS cipher suites (#2882): the SSL configuration exposed trust store, CA cert, client cert/key, SNI and mode, but nothing for cipher suites, so applications could not enforce a stronger or compliance-mandated set. client-v2: - New ClientConfigProperties.SSL_CIPHER_SUITES ("ssl_cipher_suites"), a comma-separated list parsed into a List<String>. - Client.Builder.setSSLCipherSuites(String...) to set it programmatically. - HttpAPIClientHelper applies the configured suites to the SSL socket via the connection socket factory. CustomSSLConnectionFactory gains a constructor that passes supportedCipherSuites to the base SSLConnectionSocketFactory. Existing hostname-verification behavior is unchanged: verification is skipped only for TRUST/VERIFY_CA or when a custom SNI is set, and the default verifier is kept otherwise (so STRICT + cipher suites still verifies the hostname). jdbc-v2 needs no code change: ssl_cipher_suites is forwarded to client-v2 as a regular string property (via URL or Properties). Tests: client-v2 ClientBuilderTest (builder + comma-separated string parse to a list, and an HTTPS client builds with cipher suites configured) and jdbc-v2 JdbcConfigurationTest (ssl_cipher_suites forwarded via Properties and URL). Examples (client-v2 and jdbc SSLExamples) and docs/features.md updated. Fixes: #2882 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
Repository collaborators can run the JMH benchmark suite against this PR by commenting: Optional regression threshold override (Δ% on Time or Alloc/op; defaults to 10%): Only one benchmark run per PR is active at a time — issuing a new |
Client V2 CoverageCoverage Report
Class Coverage
|
JDBC V2 CoverageCoverage Report
Class Coverage
|
JDBC V1 CoverageCoverage Report
Class Coverage
|
Client V1 CoverageCoverage Report
Class Coverage
|
Clears the two new-code quality-gate conditions introduced by the cipher-suite change on this branch: - Security rating (java:S5527): scope a @SuppressWarnings on the intentional trust-all HostnameVerifier. It is used only for SSLMode.TRUST/VERIFY_CA (the user explicitly opted out of hostname verification) or a custom SNI; STRICT keeps the default verifying behaviour via a null verifier, so secure-by-default verification is preserved. SonarCloud only flagged it because the restructure moved the pre-existing line into new code. - New-code coverage: add CustomSSLConnectionFactory unit tests that assert the configured cipher suites are forwarded to the base socket factory, that the legacy 3-arg constructor applies no restriction, and the existing SNI handling in prepareSocket. This covers the previously-uncovered legacy constructor. Also adds the missing CHANGELOG entry for the feature. Fixes: #2882
Collaborator
Author
|
Pushed Security rating ( New-code coverage (78.1% → ≥ 80%): added Also added the missing |
…ge gate Adds focused unit tests for HttpAPIClientHelper.createHttpClient's TLS socket-factory selection, which was the uncovered new code behind the SonarCloud "coverage on new code" quality-gate failure (73% < 80%). The tests drive the real createHttpClient SSL branch (via Mockito mockConstruction) and pin the factory wiring for each mode: STRICT + cipher suites forwards the suites and keeps hostname verification (null verifier); TRUST and VERIFY_CA install the permissive verifier; a custom SNI installs the permissive verifier and passes the SNI through; SNI + cipher suites combine; and the default STRICT path plus an empty cipher list keep the plain verifying SSLConnectionSocketFactory (no restriction). Test-only change; no existing tests modified. New-code coverage for the cipher-suite change is now 100%.
TriageCategory: Summary What this impacts
Concerns
Required reviewer action
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit d17cf3d. Configure here.
A leading, trailing or doubled comma in ssl_cipher_suites (or a raw List
option) leaves an empty or whitespace-only token in the parsed list. The
value was only checked for emptiness (zero elements), so a blank token
reached SSLSocket#setEnabledCipherSuites, which throws IllegalArgumentException
("invalid null or empty string elements") and breaks the TLS handshake even
when valid suites are also present. Blank tokens are now dropped and each
surviving name is trimmed; a list that reduces to nothing is treated as
"no restriction" (JVM defaults), matching the empty/unset behaviour.
Addresses the Cursor Bugbot review comment on PR #2919.
|
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
1 participant
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.
Description
Fixes #2882.
Neither client could restrict the TLS cipher suites negotiated on secure connections. The SSL configuration exposed trust store, CA certificate, client certificate/key, SNI and
ssl_mode, but nothing for cipher suites — so applications that must enforce a stronger or compliance-mandated set (rather than the JVM defaults) had no supported way to do so.-Dhttps.socketCipherSuites/-Djdk.tls.client.cipherSuitesare JVM-global and were not honored per-connection.This adds cipher-suite selection to client-v2 (with the value flowing to jdbc-v2 as a normal connection property).
Changes
client-v2
ClientConfigProperties.SSL_CIPHER_SUITES("ssl_cipher_suites") — a comma-separated list parsed into aList<String>.Client.Builder.setSSLCipherSuites(String...)sets it programmatically.HttpAPIClientHelperapplies the configured suites to the SSL socket through the connection socket factory:CustomSSLConnectionFactorygains a constructor that forwardssupportedCipherSuitesto the baseSSLConnectionSocketFactory(which enables them on each socket). Hostname-verification behavior is unchanged — verification is skipped only forTRUST/VERIFY_CAor when a custom SNI is set; otherwise the default verifier is used, soSTRICT+ cipher suites still verifies the hostname. When no cipher suites are configured, the socket-factory selection is byte-for-byte the previous behavior.jdbc-v2
ssl_cipher_suitesis forwarded toclient-v2as a regular string property (via the JDBC URL orProperties).Examples & docs
SSLExamples(client-v2 and jdbc) gain aconnectWithCipherSuitesexample.docs/features.mdupdated (client-v2 feature + jdbc note).Test
ClientBuilderTest: cipher suites set viasetSSLCipherSuites(...)are stored as a parsed list; the comma-separated string form (the URL/JDBC path) parses to the same list; and an HTTPS client builds successfully with cipher suites configured (exercises the new socket-factory branch inSTRICTmode).JdbcConfigurationTest:ssl_cipher_suitessupplied viaPropertiesand via the URL is forwarded to the client properties.Focused runs:
ClientBuilderTest14/14,JdbcConfigurationTest92/92. Both example projects compile.Pre-PR validation gate
ClientBuilderTest/JdbcConfigurationTeststill passAGENTS.md/docs/changes_checklist.md/docs/features.mddocs/changes_checklist.mdwalkthroughClientConfigProperties.SSL_CIPHER_SUITES): keyssl_cipher_suitesis unique; value typeList.classmatches the existing list-valued properties (SESSION_DB_ROLES); default is unset (null), andparseValue(null)returnsnull, handled defensively where read. It round-trips as a comma-separated string via the existingcommaSeparated/valuesFromCommaSeparatedhelpers.Client.Builder.setSSLCipherSuites):String...varargs, returns the builder, matching the sibling SSL setters; public because it is a user-facing builder option; behavior tested; reflected indocs/features.md.CustomSSLConnectionFactory): added as an overload; the existing 3-arg constructor is preserved and delegates to it, so no existing caller changes behavior.CustomSSLConnectionFactorylives in the internal package.STRICT(verification kept).docs/features.mdupdated.Notes