[client-v2, jdbc-v2] Implement SSL Modes #2874
Open
chernser wants to merge 9 commits into
Open
Conversation
|
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
|
…o match java naming convention. Misc fixes.
Contributor
Author
|
@cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
Author
|
@cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
1 issue from previous review remains unresolved.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit ea14e4f. Configure here.
TriageCategory: Summary What this impacts
Concerns
Required reviewer action
|
|
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.
Summary
Branch
06/10/26/ssl_modes— SummaryAdds an explicit
ssl_modesetting toclient-v2andjdbc-v2, replacing the old implicit SSL behavior. ~987 lines across 17 files (8 commits).Key changes
SSLModeenum (client-v2/.../enums/SSLMode.java) with 4 modes.SslContextProvider(client-v2-owned, builder-based) replacing the deprecated v1ClickHouseDefaultSslContextProvider. Key material (client cert/key for mTLS) and trust material (trust store / CA cert / trust-all) are now configured independently.Client.Builder.setSSLMode(...)+ssl_modeconfig property (defaultSTRICT).HttpAPIClientHelper.createSSLContextrewritten to build the context from the resolved mode; hostname verification is skipped forTRUST/VERIFY_CA.-----BEGINis treated as PEM content, otherwise as a file path.SSLExamplesfor both client-v2 and jdbc), anddocs/features.md(the compatibility contract) updated.Purpose of the modes (least → most strict)
DISABLED— no SSL; plain protocols only. Rejected if endpoint ishttps://.TRUST— encrypt but accept any server cert, skip hostname check. Trust store / CA cert are ignored (warning logged); client cert/key still used for mTLS. MITM-vulnerable, testing only.VERIFY_CA— validate cert chain, but skip hostname verification.STRICT(default) — full chain + hostname verification.Behavior worth noting (compatibility-sensitive)
ssl_modedoes not toggle encryption — the endpoint scheme (http/https) decides that.VERIFY_CA/STRICT: if both trust store and CA cert are set, trust store wins and CA cert is ignored (warning). Trust store +sslcerttogether still throwsClientMisconfigurationException.ClientMisconfigurationException(client) /SQLException(JDBC).V1 JDBC compatibility —
none=trustJdbcConfigurationaccepts the traditional JDBCssl_mode=nonevalue and aliases it toTRUST(the no-verification mode), so existing V1-style JDBC URLs/properties keep working. Other values are normalized case-insensitively before being forwarded toclient-v2; unrecognized values throwSQLException.This is the main backward-compatibility seam to trust/verify: V1 used
nonefor "don't verify," and the branch maps that onto the newTRUSTmode rather than introducing a breaking rename.Closes: #2389
Closes: #2309
Closes: #2819
Checklist
Delete items not relevant to your PR:
Review Guidance
SSL support consists of:
Client authendication and verification requires KeyStore and configured with user key and user certificate (key used to sign certificate).
In PR it is located here https://github.com/ClickHouse/clickhouse-java/pull/2874/changes#diff-40a5e6ba1f1b64d0bb9ee63a8f4f98a913545af2d4e8c3839f76a7cb97d3e9d2R170
This part has nothing to do with SSL Mode.
Server verification has two checks: server certificate and server identity matching the certificate.
Certificates are supplied via truststore.
Server certificate verification is configured by root certificate (CA certificate) or external trust store. When ssl mode is
TRUSTthen client skip this verification.In PR it is located here https://github.com/ClickHouse/clickhouse-java/pull/2874/changes#diff-40a5e6ba1f1b64d0bb9ee63a8f4f98a913545af2d4e8c3839f76a7cb97d3e9d2R174
So the logic is:
Server identity verification is done in another place thru SSL socket. When in TRUST or VERIFY_CA mode then no host verification needed.
In PR it is located here https://github.com/ClickHouse/clickhouse-java/pull/2874/changes#diff-40a5e6ba1f1b64d0bb9ee63a8f4f98a913545af2d4e8c3839f76a7cb97d3e9d2R289
Note
High Risk
Changes TLS trust and hostname verification defaults and wiring for all HTTPS clients; misconfiguration or mode semantics bugs could weaken security or break existing secure connections.
Overview
Adds an explicit
ssl_modesetting toclient-v2andjdbc-v2so HTTPS behavior is no longer implicit. NewSSLModeenum (DISABLED,TRUST,VERIFY_CA,STRICTdefault) is exposed viaClient.Builder.setSSLMode, thessl_modeconfig property, and JDBC URL/properties.HttpAPIClientHelpernow builds TLS through a newSslContextProvider(replacing the v1ClickHouseDefaultSslContextProvider): mTLS client cert/key is applied separately from trust strategy;TRUSTuses a trust-all manager and ignores configured trust store/CA (with warnings);VERIFY_CA/STRICTuse trust store, CA cert, or the JVM default, with trust store winning when both store and CA are set. Hostname checks are skipped forTRUST,VERIFY_CA, and custom SNI. Build-time validation normalizes case-insensitive mode names, rejects unknown values andDISABLEDwithhttps://endpoints, andSSLExceptionis wrapped asClickHouseException.JDBC maps legacy
ssl_mode=nonetoTRUSTand normalizes other values before forwarding to the client. Docs, feature contract, and SSL examples (includingTRUSTfor self-signed) are updated; integration/unit tests cover each mode.Reviewed by Cursor Bugbot for commit 683dd25. Bugbot is set up for automated code reviews on this repo. Configure here.