Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 50 additions & 3 deletions client-v2/src/main/java/com/clickhouse/client/api/Client.java
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,8 @@
import java.util.function.Supplier;
import java.util.stream.Collectors;

import javax.net.ssl.SSLContext;

/**
* <p>Client is the starting point for all interactions with ClickHouse. </p>
*
Expand Down Expand Up @@ -145,10 +147,16 @@
private final Supplier<String> queryIdGenerator;
private final CredentialsManager credentialsManager;

private Client(Collection<Endpoint> endpoints, Map<String,String> configuration,

Check warning on line 150 in client-v2/src/main/java/com/clickhouse/client/api/Client.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Constructor has 8 parameters, which is greater than 7 authorized.

See more on https://sonarcloud.io/project/issues?id=ClickHouse_clickhouse-java&issues=AZ9HqWtfVWej1L9_isBX&open=AZ9HqWtfVWej1L9_isBX&pullRequest=2918
ExecutorService sharedOperationExecutor, ColumnToMethodMatchingStrategy columnToMethodMatchingStrategy,
Object metricsRegistry, Supplier<String> queryIdGenerator, CredentialsManager cManager) {
Object metricsRegistry, Supplier<String> queryIdGenerator, CredentialsManager cManager,
SSLContext sslContext) {
Map<String, Object> parsedConfiguration = new ConcurrentHashMap<>(ClientConfigProperties.parseConfigMap(configuration));
// A pre-built SSLContext is a live object and cannot travel through the string-based configuration
// map, so it is injected into the parsed (object) configuration directly.
if (sslContext != null) {
parsedConfiguration.put(ClientConfigProperties.SSL_CONTEXT.getKey(), sslContext);
}
this.credentialsManager = cManager;
this.session = Session.extractFrom(parsedConfiguration);
this.configuration = new ConcurrentHashMap<>(parsedConfiguration);
Expand Down Expand Up @@ -270,6 +278,7 @@
private ColumnToMethodMatchingStrategy columnToMethodMatchingStrategy;
private Object metricRegistry = null;
private Supplier<String> queryIdGenerator;
private SSLContext sslContext = null;

public Builder() {
this.endpoints = new HashSet<>();
Expand Down Expand Up @@ -785,6 +794,29 @@
return this;
}

/**
* Supplies a pre-built {@link SSLContext} to be used for secure connections instead of one built
* from the configured trust/key material (trust store, CA certificate, client certificate/key).
*
* <p>When a context is set, the client uses it as is - it is the application's responsibility to
* configure it correctly. Trust- and key-material options ({@link Builder#setSSLTrustStore(String)},
* {@link Builder#setRootCertificate(String)}, {@link Builder#setClientCertificate(String)}, ...)
* are then ignored because they only feed the context the client would otherwise build.
* {@link SSLMode} still applies, but only to server hostname verification: {@link SSLMode#TRUST}
* and {@link SSLMode#VERIFY_CA} skip the hostname check while {@link SSLMode#STRICT} (default)
* enforces it.</p>
*
* <p>This is primarily useful when certificates and keys are held in memory (for example, loaded
* from a secret store) and must never be written to disk.</p>
*
* @param sslContext a fully configured SSL context; {@code null} clears any previously set context
* @return same instance of the builder
*/
public Builder setSSLContext(SSLContext sslContext) {
this.sslContext = sslContext;
return this;
}

/**
* Configure client to use server timezone for date/datetime columns. Default is true.
* If this options is selected then server timezone should be set as well.
Expand Down Expand Up @@ -1165,7 +1197,21 @@

CredentialsManager cManager = new CredentialsManager(this.configuration);

if (configuration.containsKey(ClientConfigProperties.SSL_TRUST_STORE.getKey()) &&
// A pre-built SSLContext is a live object and can only be supplied programmatically via
// setSSLContext(SSLContext). A textual 'ssl_context' value (e.g. from setOption(...), a JDBC
// property, or a URL query parameter) can never represent a real context, so it is rejected
// here instead of being silently ignored when the SSL context is created.
if (configuration.containsKey(ClientConfigProperties.SSL_CONTEXT.getKey())) {
throw new ClientMisconfigurationException("'" + ClientConfigProperties.SSL_CONTEXT.getKey()
+ "' cannot be set as a string; supply a javax.net.ssl.SSLContext object via "
+ "Client.Builder.setSSLContext(...)");
}

// Trust- and key-material options only feed a context the client would otherwise build. When
// the application supplies its own SSLContext they are ignored (see createSSLContext), so this
// conflict cannot arise and must not be reported.
if (this.sslContext == null &&
configuration.containsKey(ClientConfigProperties.SSL_TRUST_STORE.getKey()) &&
configuration.containsKey(ClientConfigProperties.SSL_CERTIFICATE.getKey())) {
throw new ClientMisconfigurationException("Trust store and certificates cannot be used together");
}
Expand Down Expand Up @@ -1229,7 +1275,8 @@
}

return new Client(this.endpoints, this.configuration, this.sharedOperationExecutor,
this.columnToMethodMatchingStrategy, this.metricRegistry, this.queryIdGenerator, cManager);
Comment thread
polyglotAI-bot marked this conversation as resolved.
this.columnToMethodMatchingStrategy, this.metricRegistry, this.queryIdGenerator, cManager,
this.sslContext);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.net.ssl.SSLContext;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
Expand Down Expand Up @@ -118,6 +120,15 @@ public enum ClientConfigProperties {

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

/**
* A pre-built {@link javax.net.ssl.SSLContext} supplied by the application. When set, the client uses
* it as is instead of building one from the configured trust/key material, and {@link #SSL_MODE} then
* only controls server hostname verification. The value is a live object, so it can only be provided
* programmatically (for example via {@code Client.Builder#setSSLContext}); it is never parsed from a
* string and has no textual representation in a configuration map.
*/
SSL_CONTEXT("ssl_context", SSLContext.class),

Comment thread
polyglotAI-bot marked this conversation as resolved.
RETRY_ON_FAILURE("retry", Integer.class, "3"),

INPUT_OUTPUT_FORMAT("format", ClickHouseFormat.class),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,15 @@ public HttpAPIClientHelper(Map<String, Object> configuration, Object metricsRegi
* @return SSLContext
*/
public SSLContext createSSLContext(Map<String, Object> configuration) {
// A pre-built SSLContext supplied by the application is used as is; the client does not build one
// from the configured trust/key material. Server hostname verification is still governed by the
// SSL mode where the connection socket factory is created (see createHttpClient).
final Object customSSLContext = configuration.get(ClientConfigProperties.SSL_CONTEXT.getKey());
if (customSSLContext instanceof SSLContext) {
LOG.debug("Using application-supplied SSLContext; trust/key material options are ignored.");
return (SSLContext) customSSLContext;
}

final SSLMode sslMode = ClientConfigProperties.SSL_MODE.getOrDefault(configuration);
final String trustStorePath = (String) configuration.get(ClientConfigProperties.SSL_TRUST_STORE.getKey());
final String caCertificate = (String) configuration.get(ClientConfigProperties.CA_CERTIFICATE.getKey());
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
package com.clickhouse.client.api;

import com.clickhouse.client.api.enums.SSLMode;
import com.clickhouse.client.api.internal.HttpAPIClientHelper;
import org.testng.Assert;
import org.testng.annotations.DataProvider;
import org.testng.annotations.Test;

import javax.net.ssl.SSLContext;
import java.lang.reflect.Field;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

public class ClientBuilderTest {

Expand Down Expand Up @@ -86,6 +90,123 @@ public void testSslModeInvalidValueRejected() {
.build());
}

@Test
public void testStringSSLContextRejected() {
// ssl_context is an object-only property; a textual value can never be a real context, so the
// builder must reject it instead of silently ignoring it (previously parsed to null and dropped).
Assert.expectThrows(ClientMisconfigurationException.class, () -> new Client.Builder()
.addEndpoint("https://localhost:8443")
.setUsername("default")
.setPassword("")
.setOption(ClientConfigProperties.SSL_CONTEXT.getKey(), "not-a-context")
.build());
}

@Test
public void testCustomSSLContextIgnoresTrustAndKeyMaterialConflict() throws Exception {
SSLContext customContext = SSLContext.getInstance("TLS");
customContext.init(null, null, null);

// A trust store and a client certificate normally conflict, but when a custom SSLContext is
// supplied that material is ignored, so the conflict must not be reported and the client builds.
try (Client client = new Client.Builder()
.addEndpoint("https://localhost:8443")
.setUsername("default")
.setPassword("")
.setSSLTrustStore("/path/to/truststore.jks")
.setClientCertificate("client.crt")
.setSSLContext(customContext)
.build()) {
Assert.assertSame(extractConfiguration(client).get(ClientConfigProperties.SSL_CONTEXT.getKey()),
customContext, "The application-supplied SSLContext should be used as is");
}
}

@Test
public void testTrustStoreAndClientCertificateConflictRejectedWithoutCustomContext() {
// Contrast case: without a custom SSLContext the trust-store/certificate conflict must still be
// rejected exactly as before - the fix only suppresses the check when a context is supplied.
Assert.expectThrows(ClientMisconfigurationException.class, () -> new Client.Builder()
.addEndpoint("https://localhost:8443")
.setUsername("default")
.setPassword("")
.setSSLTrustStore("/path/to/truststore.jks")
.setClientCertificate("client.crt")
.build());
}

@Test
public void testSetSSLContextStoredInConfiguration() throws Exception {
SSLContext customContext = SSLContext.getInstance("TLS");
customContext.init(null, null, null);

try (Client client = new Client.Builder()
.addEndpoint("https://localhost:8443")
.setUsername("default")
.setPassword("")
.setSSLContext(customContext)
.build()) {
Assert.assertSame(extractConfiguration(client).get(ClientConfigProperties.SSL_CONTEXT.getKey()),
customContext, "The application-supplied SSLContext should be stored in the configuration");
}

// Without setSSLContext the key must be absent so the client builds its own context.
try (Client client = new Client.Builder()
.addEndpoint("https://localhost:8443")
.setUsername("default")
.setPassword("")
.build()) {
Assert.assertNull(extractConfiguration(client).get(ClientConfigProperties.SSL_CONTEXT.getKey()),
"No SSLContext should be stored when none is supplied");
}
}

@Test
public void testCreateSSLContextReturnsCustomContext() throws Exception {
SSLContext customContext = SSLContext.getInstance("TLS");
customContext.init(null, null, null);

try (Client client = new Client.Builder()
.addEndpoint("https://localhost:8443")
.setUsername("default")
.setPassword("")
.setSSLContext(customContext)
.build()) {
HttpAPIClientHelper helper = extractHttpClientHelper(client);
SSLContext resolved = helper.createSSLContext(extractConfiguration(client));
Assert.assertSame(resolved, customContext,
"createSSLContext must return the application-supplied context as is");
}

// When no custom context is configured, createSSLContext builds a context (not the custom one).
try (Client client = new Client.Builder()
.addEndpoint("https://localhost:8443")
.setUsername("default")
.setPassword("")
.build()) {
HttpAPIClientHelper helper = extractHttpClientHelper(client);
Map<String, Object> configWithCustom = new HashMap<>(extractConfiguration(client));
configWithCustom.put(ClientConfigProperties.SSL_CONTEXT.getKey(), customContext);
Assert.assertSame(helper.createSSLContext(configWithCustom), customContext,
"createSSLContext must honor a custom context supplied via the configuration map");
Assert.assertNotSame(helper.createSSLContext(extractConfiguration(client)), customContext,
"createSSLContext must build its own context when none is supplied");
}
}

private static HttpAPIClientHelper extractHttpClientHelper(Client client) throws Exception {
Field helperField = Client.class.getDeclaredField("httpClientHelper");
helperField.setAccessible(true);
return (HttpAPIClientHelper) helperField.get(client);
}

@SuppressWarnings("unchecked")
private static Map<String, Object> extractConfiguration(Client client) throws Exception {
Field configField = Client.class.getDeclaredField("configuration");
configField.setAccessible(true);
return (Map<String, Object>) configField.get(client);
}

private static String extractFirstEndpointUri(Client client) throws Exception {
Field endpointsField = Client.class.getDeclaredField("endpoints");
endpointsField.setAccessible(true);
Expand Down
Loading
Loading