Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove redundant SSL work from client config creation #1923

Merged
merged 9 commits into from
Apr 25, 2023
6 changes: 6 additions & 0 deletions changelog/@unreleased/pr-1923.v2.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
type: improvement
improvement:
description: Use TrustContextFactory when building ClientConfiguration instead of
overwriting already performed SSL work.
links:
- https://github.com/palantir/dialogue/pull/1923
1 change: 1 addition & 0 deletions dialogue-clients/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ dependencies {
implementation 'com.google.errorprone:error_prone_annotations'
implementation 'com.google.guava:guava'
implementation 'com.palantir.conjure.java.runtime:keystores'
implementation 'com.palantir.conjure.java.api:ssl-config'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had to add this line as CI was failing because of implicit dependencies

implementation 'com.palantir.safe-logging:logger'
implementation 'com.palantir.safe-logging:preconditions'
implementation 'com.palantir.safe-logging:safe-logging'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,18 @@
import com.palantir.conjure.java.api.config.service.UserAgent;
import com.palantir.conjure.java.client.config.ClientConfiguration;
import com.palantir.conjure.java.client.config.ClientConfigurations;
import com.palantir.conjure.java.client.config.ClientConfigurations.TrustContextFactory;
import com.palantir.conjure.java.client.config.HostEventsSink;
import com.palantir.conjure.java.client.config.NodeSelectionStrategy;
import com.palantir.conjure.java.config.ssl.SslSocketFactories;
import com.palantir.conjure.java.config.ssl.TrustContext;
import com.palantir.tritium.metrics.registry.SharedTaggedMetricRegistries;
import com.palantir.tritium.metrics.registry.TaggedMetricRegistry;
import java.security.Provider;
import java.util.Optional;
import javax.net.ssl.KeyManager;
import javax.net.ssl.SSLContext;
import javax.net.ssl.TrustManager;
import org.immutables.value.Value;

/**
Expand Down Expand Up @@ -62,18 +66,11 @@ default TaggedMetricRegistry taggedMetrics() {
Optional<HostEventsSink> hostEventsSink();

static ClientConfiguration getClientConf(ServiceConfiguration serviceConfig, AugmentClientConfig augment) {
TrustContextFactory trustContextFactory = buildTrustContextFactory(augment.securityProvider());
ClientConfiguration.Builder builder =
ClientConfiguration.builder().from(ClientConfigurations.of(serviceConfig));
ClientConfiguration.builder().from(ClientConfigurations.of(serviceConfig, trustContextFactory));

SSLContext context = augment.securityProvider()
.map(provider -> SslSocketFactories.createSslContext(serviceConfig.security(), provider))
.orElseGet(() -> SslSocketFactories.createSslContext(serviceConfig.security()));
// Reduce the session cache size for clients. We expect TLS connections to be reused, thus the cache isn't
// terribly important.
context.getClientSessionContext().setSessionCacheSize(100);
builder.sslSocketFactory(context.getSocketFactory());

if (!serviceConfig.maxNumRetries().isPresent()) {
if (serviceConfig.maxNumRetries().isEmpty()) {
augment.maxNumRetries().ifPresent(builder::maxNumRetries);
}

Expand All @@ -93,4 +90,25 @@ static ClientConfiguration getClientConf(ServiceConfiguration serviceConfig, Aug

return builder.build();
}

private static TrustContextFactory buildTrustContextFactory(Optional<Provider> securityProvider) {
return sslConfiguration -> {
TrustManager[] trustManagers = SslSocketFactories.createTrustManagers(sslConfiguration);
KeyManager[] keyManagers = SslSocketFactories.createKeyManagers(sslConfiguration);

SSLContext sslContext;
if (securityProvider.isPresent()) {
sslContext = SslSocketFactories.createSslContext(trustManagers, keyManagers, securityProvider.get());
} else {
sslContext = SslSocketFactories.createSslContext(trustManagers, keyManagers);
}

// Reduce the session cache size for clients. We expect TLS connections to be reused, thus the cache isn't
// terribly important.
sslContext.getClientSessionContext().setSessionCacheSize(100);

return TrustContext.of(
sslContext.getSocketFactory(), SslSocketFactories.getX509TrustManager(trustManagers));
};
}
}
10 changes: 5 additions & 5 deletions versions.lock
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@ com.palantir.common:streams:2.1.0 (1 constraints: 0505f835)
com.palantir.conjure.java:conjure-lib:7.6.0 (1 constraints: 0f052036)
com.palantir.conjure.java.api:errors:2.35.0 (2 constraints: 5c15c8da)
com.palantir.conjure.java.api:service-config:2.35.0 (2 constraints: 571956dc)
com.palantir.conjure.java.api:ssl-config:2.35.0 (2 constraints: a625f238)
com.palantir.conjure.java.runtime:client-config:7.54.0 (1 constraints: 4205693b)
com.palantir.conjure.java.runtime:conjure-java-jackson-optimizations:7.54.0 (1 constraints: 861ccaa4)
com.palantir.conjure.java.runtime:conjure-java-jackson-serialization:7.54.0 (2 constraints: 5f168f0f)
com.palantir.conjure.java.runtime:keystores:7.54.0 (2 constraints: 6319f2dd)
com.palantir.conjure.java.api:ssl-config:2.35.0 (3 constraints: e12affa3)
com.palantir.conjure.java.runtime:client-config:7.55.0 (1 constraints: 43056c3b)
com.palantir.conjure.java.runtime:conjure-java-jackson-optimizations:7.55.0 (1 constraints: 871ccda4)
com.palantir.conjure.java.runtime:conjure-java-jackson-serialization:7.55.0 (2 constraints: 6016c30f)
com.palantir.conjure.java.runtime:keystores:7.55.0 (2 constraints: 651931de)
com.palantir.goethe:goethe:0.10.0 (1 constraints: 3305233b)
com.palantir.refreshable:refreshable:2.2.0 (2 constraints: eb18d5b1)
com.palantir.ri:resource-identifier:2.5.0 (2 constraints: f714fcb6)
Expand Down
2 changes: 1 addition & 1 deletion versions.props
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ com.palantir.common:streams = 2.1.0
com.palantir.conjure:conjure = 4.36.0
com.palantir.conjure.java:* = 7.6.0
com.palantir.conjure.java.api:* = 2.35.0
com.palantir.conjure.java.runtime:* = 7.54.0
com.palantir.conjure.java.runtime:* = 7.55.0
com.palantir.refreshable:* = 2.2.0
com.palantir.ri:resource-identifier = 2.5.0
com.palantir.safe-logging:* = 3.4.0
Expand Down