Skip to content

Commit 3588e30

Browse files
client-v2: satisfy SonarCloud quality gate for cipher-suite selection
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
1 parent 1d53750 commit 3588e30

3 files changed

Lines changed: 105 additions & 5 deletions

File tree

CHANGELOG.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,13 @@
22

33
[Release Migration Guide](docs/releases/0_11_0.md)
44

5+
### New Features
6+
7+
- **[client-v2, jdbc-v2]** Added TLS cipher suite selection. `Client.Builder.setSSLCipherSuites(String...)` (client-v2)
8+
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
10+
trust configuration and `ssl_mode`. (https://github.com/ClickHouse/clickhouse-java/issues/2882)
11+
512
### Bug Fixes
613

714
- **[client-v2]** Fixed container query parameters being sent unquoted, so `Client.query(sql, params, settings)` binding

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,10 @@ public CloseableHttpClient createHttpClient(boolean initSslContext, Map<String,
296296
// Skip hostname verification only for trust-all modes or when a custom SNI is used (the
297297
// connection hostname would not match the certificate); otherwise a null verifier makes the
298298
// factory fall back to the JDK/HttpClient default verifier, keeping STRICT verification.
299+
// java:S5527 - the permissive verifier is applied only for SSLMode.TRUST/VERIFY_CA (where the
300+
// user has explicitly opted out of hostname verification) or a custom SNI; STRICT keeps the
301+
// default verifying behaviour, so secure-by-default hostname verification is preserved.
302+
@SuppressWarnings("java:S5527")
299303
HostnameVerifier hostnameVerifier = trustAllHostnames || hasSNI ? (hostname, session) -> true : null;
300304
sslConnectionSocketFactory = new CustomSSLConnectionFactory(socketSNI, sslContext, hostnameVerifier,
301305
enabledCipherSuites);
@@ -1074,6 +1078,7 @@ public static class CustomSSLConnectionFactory extends SSLConnectionSocketFactor
10741078

10751079
private final SNIHostName defaultSNI;
10761080

1081+
// Retained for backward compatibility; delegates with no cipher-suite restriction (JVM defaults).
10771082
public CustomSSLConnectionFactory(String defaultSNI, SSLContext sslContext, HostnameVerifier hostnameVerifier) {
10781083
this(defaultSNI, sslContext, hostnameVerifier, null);
10791084
}
Lines changed: 93 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,93 @@
1-
package com.clickhouse.client.api.internal;
2-
3-
public class HttpAPIClientHelperTest {
4-
5-
}
1+
package com.clickhouse.client.api.internal;
2+
3+
import com.clickhouse.client.api.internal.HttpAPIClientHelper.CustomSSLConnectionFactory;
4+
import org.apache.hc.client5.http.ssl.SSLConnectionSocketFactory;
5+
import org.mockito.ArgumentCaptor;
6+
import org.testng.annotations.Test;
7+
8+
import javax.net.ssl.SNIHostName;
9+
import javax.net.ssl.SNIServerName;
10+
import javax.net.ssl.SSLContext;
11+
import javax.net.ssl.SSLParameters;
12+
import javax.net.ssl.SSLSocket;
13+
import java.lang.reflect.Field;
14+
import java.util.List;
15+
16+
import static org.mockito.ArgumentMatchers.any;
17+
import static org.mockito.Mockito.mock;
18+
import static org.mockito.Mockito.never;
19+
import static org.mockito.Mockito.verify;
20+
import static org.mockito.Mockito.when;
21+
import static org.testng.Assert.assertEquals;
22+
import static org.testng.Assert.assertNull;
23+
24+
public class HttpAPIClientHelperTest {
25+
26+
/**
27+
* The configured cipher suites must be forwarded to the base {@link SSLConnectionSocketFactory}, which is
28+
* what enables them on each secure connection. This is the core of the cipher-suite feature.
29+
*/
30+
@Test
31+
public void testCipherSuiteConstructorForwardsSuitesToBaseFactory() throws Exception {
32+
String[] suites = {"TLS_AES_256_GCM_SHA384", "TLS_AES_128_GCM_SHA256"};
33+
CustomSSLConnectionFactory factory = new CustomSSLConnectionFactory(
34+
null, SSLContext.getDefault(), (hostname, session) -> true, suites);
35+
36+
assertEquals(baseSupportedCipherSuites(factory), suites,
37+
"configured cipher suites must reach the base socket factory that enables them per connection");
38+
}
39+
40+
/**
41+
* The three-argument constructor is retained for backward compatibility and must delegate with no cipher
42+
* restriction, so callers that do not configure cipher suites keep the JVM defaults.
43+
*/
44+
@Test
45+
public void testLegacyConstructorAppliesNoCipherRestriction() throws Exception {
46+
CustomSSLConnectionFactory factory = new CustomSSLConnectionFactory(
47+
"legacy.example.com", SSLContext.getDefault(), (hostname, session) -> true);
48+
49+
assertNull(baseSupportedCipherSuites(factory),
50+
"the legacy constructor must not restrict cipher suites");
51+
}
52+
53+
/**
54+
* A configured SNI host is applied to every prepared socket via the standard SSL parameters.
55+
*/
56+
@Test
57+
public void testConfiguredSniAppliedToPreparedSocket() throws Exception {
58+
CustomSSLConnectionFactory factory = new CustomSSLConnectionFactory(
59+
"sni.example.com", SSLContext.getDefault(), (hostname, session) -> true, null);
60+
61+
SSLSocket socket = mock(SSLSocket.class);
62+
when(socket.getSSLParameters()).thenReturn(new SSLParameters());
63+
64+
factory.prepareSocket(socket, null);
65+
66+
ArgumentCaptor<SSLParameters> params = ArgumentCaptor.forClass(SSLParameters.class);
67+
verify(socket).setSSLParameters(params.capture());
68+
List<SNIServerName> serverNames = params.getValue().getServerNames();
69+
assertEquals(serverNames.size(), 1, "the configured SNI host must be applied to the socket");
70+
assertEquals(((SNIHostName) serverNames.get(0)).getAsciiName(), "sni.example.com");
71+
}
72+
73+
/**
74+
* A blank SNI is treated as unset: the socket's SSL parameters must be left untouched so the defaults
75+
* (and any cipher suites the base factory already applied) are preserved.
76+
*/
77+
@Test
78+
public void testBlankSniLeavesSocketParametersUntouched() throws Exception {
79+
CustomSSLConnectionFactory factory = new CustomSSLConnectionFactory(
80+
" ", SSLContext.getDefault(), (hostname, session) -> true, null);
81+
82+
SSLSocket socket = mock(SSLSocket.class);
83+
factory.prepareSocket(socket, null);
84+
85+
verify(socket, never()).setSSLParameters(any());
86+
}
87+
88+
private static String[] baseSupportedCipherSuites(CustomSSLConnectionFactory factory) throws Exception {
89+
Field field = SSLConnectionSocketFactory.class.getDeclaredField("supportedCipherSuites");
90+
field.setAccessible(true);
91+
return (String[]) field.get(factory);
92+
}
93+
}

0 commit comments

Comments
 (0)