Skip to content

Commit 395a862

Browse files
fix(client-v2): address review feedback on TLS cipher-suite selection
- Append SSL_CIPHER_SUITES at the end of ClientConfigProperties instead of inserting it after SSL_MODE, so the ordinal and values() ordering of every existing constant is preserved (docs/changes_checklist.md: "New constant is appended, not inserted in the middle"). - Correct the "JVM defaults" wording in the Javadoc, docs/features.md, the examples and the changelog: when cipher suites are unset the effective suites are the transport defaults, i.e. Apache HttpClient enables the JVM's default suites minus the ones it considers weak (TlsCiphers.excludeWeak). When suites are set they are enabled verbatim, so that wording is unchanged. Addresses the cursor review on #2919. Implements: #2882
1 parent 675b8f5 commit 395a862

8 files changed

Lines changed: 28 additions & 23 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66

77
- **[client-v2, jdbc-v2]** Added TLS cipher suite selection. `Client.Builder.setSSLCipherSuites(String...)` (client-v2)
88
and the comma-separated `ssl_cipher_suites` connection property (client-v2 and jdbc-v2) restrict the cipher suites
9-
enabled on secure connections; when unset, the JVM defaults are used. Cipher-suite selection is independent of the
9+
enabled on secure connections; when unset, the transport defaults are used. Cipher-suite selection is independent of the
1010
trust configuration and `ssl_mode`. (https://github.com/ClickHouse/clickhouse-java/issues/2882)
1111

1212
### Bug Fixes

client-v2/src/main/java/com/clickhouse/client/api/Client.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -789,7 +789,8 @@ public Builder setSSLMode(SSLMode sslMode) {
789789
/**
790790
* Restricts the TLS cipher suites the client may negotiate on secure connections. When set, only
791791
* the listed cipher suites are enabled on the SSL socket (subject to what the JVM and the server
792-
* support); when not set, the JVM defaults are used. Suite names use the standard JSSE names, for
792+
* support); when not set, the transport defaults are used (Apache HttpClient enables the JVM's
793+
* default suites minus those it considers weak). Suite names use the standard JSSE names, for
793794
* example {@code TLS_AES_256_GCM_SHA384}.
794795
*
795796
* @param cipherSuites cipher suite names to enable

client-v2/src/main/java/com/clickhouse/client/api/ClientConfigProperties.java

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -118,13 +118,6 @@ public enum ClientConfigProperties {
118118

119119
SSL_MODE("ssl_mode", SSLMode.class, SSLMode.STRICT.name()),
120120

121-
/**
122-
* Comma-separated list of TLS cipher suites the client is allowed to negotiate on secure connections.
123-
* When set, only these cipher suites are enabled on the SSL socket (subject to what the JVM and server
124-
* support); when unset, the JVM defaults are used.
125-
*/
126-
SSL_CIPHER_SUITES("ssl_cipher_suites", List.class),
127-
128121
RETRY_ON_FAILURE("retry", Integer.class, "3"),
129122

130123
INPUT_OUTPUT_FORMAT("format", ClickHouseFormat.class),
@@ -211,6 +204,17 @@ public Object parseValue(String value) {
211204
* See <a href="https://clickhouse.com/docs/operations/settings/query-level#custom_settings">ClickHouse Docs</a>
212205
*/
213206
CUSTOM_SETTINGS_PREFIX("custom_settings_prefix", String.class, "custom_"),
207+
208+
/**
209+
* Comma-separated list of TLS cipher suites the client is allowed to negotiate on secure connections.
210+
* When set, only these cipher suites are enabled on the SSL socket (subject to what the JVM and server
211+
* support); when unset, the transport defaults are used (Apache HttpClient enables the JVM's default
212+
* suites minus those it considers weak).
213+
* <p>
214+
* Appended at the end of the enum on purpose: adding a constant in the middle would shift the ordinal
215+
* of every following constant (see {@code docs/changes_checklist.md}).
216+
*/
217+
SSL_CIPHER_SUITES("ssl_cipher_suites", List.class),
214218
;
215219

216220
private static final Logger LOG = LoggerFactory.getLogger(ClientConfigProperties.class);

client-v2/src/main/java/com/clickhouse/client/api/internal/HttpAPIClientHelper.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1087,14 +1087,14 @@ public static class CustomSSLConnectionFactory extends SSLConnectionSocketFactor
10871087

10881088
private final SNIHostName defaultSNI;
10891089

1090-
// Retained for backward compatibility; delegates with no cipher-suite restriction (JVM defaults).
1090+
// Retained for backward compatibility; delegates with no cipher-suite restriction (transport defaults).
10911091
public CustomSSLConnectionFactory(String defaultSNI, SSLContext sslContext, HostnameVerifier hostnameVerifier) {
10921092
this(defaultSNI, sslContext, hostnameVerifier, null);
10931093
}
10941094

10951095
public CustomSSLConnectionFactory(String defaultSNI, SSLContext sslContext, HostnameVerifier hostnameVerifier,
10961096
String[] supportedCipherSuites) {
1097-
// supportedProtocols is left as null (JDK defaults are used); supportedCipherSuites, when
1097+
// supportedProtocols is left as null (transport defaults apply); supportedCipherSuites, when
10981098
// provided, restricts the cipher suites the base factory enables on each socket. A null
10991099
// hostnameVerifier makes the base factory fall back to its default verifier.
11001100
super(sslContext, null, supportedCipherSuites, hostnameVerifier);

client-v2/src/test/java/com/clickhouse/client/api/internal/HttpAPIClientHelperTest.java

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ public void testCipherSuiteConstructorForwardsSuitesToBaseFactory() throws Excep
5050

5151
/**
5252
* The three-argument constructor is retained for backward compatibility and must delegate with no cipher
53-
* restriction, so callers that do not configure cipher suites keep the JVM defaults.
53+
* restriction, so callers that do not configure cipher suites keep the transport defaults.
5454
*/
5555
@Test
5656
public void testLegacyConstructorAppliesNoCipherRestriction() throws Exception {
@@ -120,7 +120,7 @@ public void testCreateHttpClientStrictWithCipherSuitesForwardsSuitesAndKeepsHost
120120

121121
/**
122122
* TRUST mode opts out of hostname verification, so the factory must receive a permissive (non-null)
123-
* verifier; with no cipher suites configured the factory must keep the JVM defaults (no restriction).
123+
* verifier; with no cipher suites configured the factory must keep the transport defaults (no restriction).
124124
*/
125125
@Test
126126
public void testCreateHttpClientTrustModeInstallsPermissiveVerifierWithoutCipherRestriction() {
@@ -206,7 +206,7 @@ public void testCreateHttpClientSniAndCipherSuitesCombine() {
206206
}
207207

208208
/**
209-
* Boundary case: an empty cipher-suite list must be treated as "no restriction" (JVM defaults), exactly
209+
* Boundary case: an empty cipher-suite list must be treated as "no restriction" (transport defaults), exactly
210210
* like an unset value - it must NOT be turned into an empty cipher array, which would enable zero suites
211211
* and make every handshake fail. STRICT with no SNI therefore keeps using the plain factory.
212212
*/
@@ -217,8 +217,8 @@ public void testCreateHttpClientEmptyCipherSuitesTreatedAsNoRestriction() {
217217

218218
List<List<?>> calls = captureCustomFactoryConstruction(config);
219219

220-
assertEquals(calls.size(), 0, "an empty cipher-suite list must be treated as no restriction (JVM "
221-
+ "defaults), so the plain SSLConnectionSocketFactory is used");
220+
assertEquals(calls.size(), 0, "an empty cipher-suite list must be treated as no restriction "
221+
+ "(transport defaults), so the plain SSLConnectionSocketFactory is used");
222222
}
223223

224224
/**
@@ -247,7 +247,7 @@ public void testCreateHttpClientDropsBlankAndWhitespaceCipherTokens() {
247247
/**
248248
* Boundary case: a cipher-suite list that contains only blank tokens (e.g. from a property that is just
249249
* commas/whitespace) has no usable suite, so - like an empty or unset list - it must be treated as "no
250-
* restriction" (JVM defaults) rather than forwarding an all-blank array that would fail every handshake.
250+
* restriction" (transport defaults) rather than forwarding an all-blank array that would fail every handshake.
251251
*/
252252
@Test
253253
public void testCreateHttpClientBlankOnlyCipherSuitesTreatedAsNoRestriction() {
@@ -257,7 +257,7 @@ public void testCreateHttpClientBlankOnlyCipherSuitesTreatedAsNoRestriction() {
257257
List<List<?>> calls = captureCustomFactoryConstruction(config);
258258

259259
assertEquals(calls.size(), 0, "a cipher-suite list of only blank tokens must be treated as no "
260-
+ "restriction (JVM defaults), so the plain SSLConnectionSocketFactory is used");
260+
+ "restriction (transport defaults), so the plain SSLConnectionSocketFactory is used");
261261
}
262262

263263
/**

docs/features.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ This document lists stable, user-visible behavior in `client-v2` and `jdbc-v2` t
77
- HTTP and HTTPS connectivity: Connects to ClickHouse over HTTP(S), supports endpoint paths, and exposes a basic `ping` health check.
88
- TLS configuration: Supports trust stores, client certificates/keys, SSL certificate authentication, and SNI for HTTPS connections. Trust material (root CA and client certificate/key) can be supplied either as a file path or directly as PEM content.
99
- SSL verification modes: `Client.Builder.setSSLMode(SSLMode)` (or the `ssl_mode` property) controls how strictly the server identity is verified on secure connections: `DISABLED` (SSL not used; plain protocols only), `TRUST` (accept any server certificate and skip hostname verification; a configured trust store or CA certificate is ignored with a warning, while a client certificate/key is still applied for mTLS if configured), `VERIFY_CA` (validate the certificate chain but skip hostname verification), and `STRICT` (full chain and hostname verification, default).
10-
- TLS cipher suite selection: `Client.Builder.setSSLCipherSuites(String...)` (or the comma-separated `ssl_cipher_suites` property) restricts the cipher suites enabled on secure connections. When set, only the listed suites are enabled on the SSL socket (subject to JVM and server support); when unset, the JVM defaults apply. Cipher-suite selection is independent of the trust configuration and `ssl_mode`.
10+
- TLS cipher suite selection: `Client.Builder.setSSLCipherSuites(String...)` (or the comma-separated `ssl_cipher_suites` property) restricts the cipher suites enabled on secure connections. When set, only the listed suites are enabled on the SSL socket (subject to JVM and server support); when unset, the transport defaults apply (Apache HttpClient enables the JVM's default suites minus those it considers weak). Cipher-suite selection is independent of the trust configuration and `ssl_mode`.
1111
- Authentication modes: Supports username/password credentials, ClickHouse auth headers, bearer tokens, and optional HTTP Basic authentication.
1212
- Runtime credential updates: Existing `Client` instances can update username/password or bearer-token credentials for subsequent requests without rebuilding the client.
1313
- Proxy support: Can send requests through configured HTTP proxies, including proxy credentials.

examples/client-v2/src/main/java/com/clickhouse/examples/client_v2/SSLExamples.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828
* Use it only for testing or in fully trusted environments.</li>
2929
* <li>Restricting the negotiated TLS cipher suites with
3030
* {@link Client.Builder#setSSLCipherSuites(String...)} - useful to enforce a stronger or
31-
* compliance-mandated set of cipher suites instead of the JVM defaults.</li>
31+
* compliance-mandated set of cipher suites instead of the transport defaults.</li>
3232
* </ul>
3333
*
3434
* <p>More SSL examples (mTLS, trust stores, SNI) will be added to this class later.</p>
@@ -202,7 +202,7 @@ static void connectWithRootCertificateAsString(String endpoint, String database,
202202
* Connects while restricting the TLS cipher suites the client is allowed to negotiate, using
203203
* {@link Client.Builder#setSSLCipherSuites(String...)}. Only the listed suites are enabled on the
204204
* socket (subject to what the JVM and the server support); this is useful to enforce a stronger or
205-
* compliance-mandated set of cipher suites rather than relying on the JVM defaults.
205+
* compliance-mandated set of cipher suites rather than relying on the transport defaults.
206206
*
207207
* <p>The CA certificate is still used to verify the server, and hostname verification stays enabled -
208208
* cipher-suite selection is independent of the trust configuration and the SSL mode. The suites below

examples/jdbc/src/main/java/com/clickhouse/examples/jdbc/SSLExamples.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333
* testing or in fully trusted environments.</li>
3434
* <li>Restricting the negotiated TLS cipher suites with the {@code ssl_cipher_suites} connection
3535
* property (a comma-separated list) - useful to enforce a stronger or compliance-mandated set of
36-
* cipher suites instead of the JVM defaults.</li>
36+
* cipher suites instead of the transport defaults.</li>
3737
* </ul>
3838
*
3939
* <p>More SSL examples (mTLS, trust stores, SNI) will be added to this class later.</p>
@@ -206,7 +206,7 @@ static void connectWithRootCertificateAsString(String url, String user, String p
206206
* Connects while restricting the TLS cipher suites the driver is allowed to negotiate, using the
207207
* {@code ssl_cipher_suites} connection property (a comma-separated list). Only the listed suites are
208208
* enabled on the socket (subject to what the JVM and the server support); this is useful to enforce a
209-
* stronger or compliance-mandated set of cipher suites rather than relying on the JVM defaults.
209+
* stronger or compliance-mandated set of cipher suites rather than relying on the transport defaults.
210210
*
211211
* <p>The CA certificate is still used to verify the server and hostname verification stays enabled -
212212
* cipher-suite selection is independent of the trust configuration and {@code ssl_mode}. Keep at least

0 commit comments

Comments
 (0)