Skip to content

Commit

Permalink
Improve interoperability with Conscrypt (#2624)
Browse files Browse the repository at this point in the history
Improve interoperability with Conscrypt
  • Loading branch information
carterkozak authored May 18, 2023
1 parent b0632e8 commit e9ee89a
Show file tree
Hide file tree
Showing 7 changed files with 245 additions and 13 deletions.
5 changes: 5 additions & 0 deletions changelog/@unreleased/pr-2624.v2.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
type: improvement
improvement:
description: Improve TLSv1.3 interoperability with Conscrypt
links:
- https://github.com/palantir/conjure-java-runtime/pull/2624
8 changes: 8 additions & 0 deletions keystores/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,14 @@ dependencies {
compileOnly 'org.immutables:value::annotations'
}

moduleJvmArgs {
// This opens exists to unblock potential failures which occur when Conscrypt is used with a non-Conscrypt
// trust-manager. It's arguable whether this is the correct place for the setting to live, however this
// provides a wide net to prevent runtime failures.
opens 'java.base/java.net'
}


task generateCerts(type:Exec) {
workingDir './src/test/resources'
commandLine './certSetup.sh', '-f'
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,192 @@
/*
* (c) Copyright 2023 Palantir Technologies Inc. All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.palantir.conjure.java.config.ssl;

import java.net.Socket;
import java.security.cert.CertificateException;
import java.security.cert.X509Certificate;
import javax.net.ssl.SSLEngine;
import javax.net.ssl.TrustManager;
import javax.net.ssl.X509ExtendedTrustManager;
import javax.net.ssl.X509TrustManager;

/**
* Internal compatibility layer to work around
* <a href="https://github.com/google/conscrypt/issues/1033">conscrypt#1033</a>.
*/
final class ConscryptCompatTrustManagers {

static TrustManager[] wrap(TrustManager[] trustManager) {
if (trustManager == null || trustManager.length == 0) {
return trustManager;
}
TrustManager[] managers = new TrustManager[trustManager.length];
for (int i = 0; i < managers.length; i++) {
managers[i] = wrap(trustManager[i]);
}
return managers;
}

static TrustManager wrap(TrustManager trustManager) {
if (trustManager.getClass().getName().contains("org.conscrypt")) {
// We don't convert authType strings when a Conscrypt TrustManager is used.
// This check should be equivalent to Conscrypt.isConscrypt without a dependecy
// on Conscrypt itself.
return trustManager;
}
if (trustManager instanceof ConscryptCompatX509TrustManager
|| trustManager instanceof ConscryptCompatX509ExtendedTrustManager) {
// Already wrapped, nothing else is needed
return trustManager;
}
if (trustManager instanceof X509ExtendedTrustManager) {
return new ConscryptCompatX509ExtendedTrustManager((X509ExtendedTrustManager) trustManager);
}
if (trustManager instanceof X509TrustManager) {
return new ConscryptCompatX509TrustManager((X509TrustManager) trustManager);
}
return trustManager;
}

private static final class ConscryptCompatX509TrustManager implements X509TrustManager {

private final X509TrustManager delegate;

ConscryptCompatX509TrustManager(X509TrustManager delegate) {
this.delegate = delegate;
}

@Override
public void checkServerTrusted(X509Certificate[] chain, String authType) throws CertificateException {
delegate.checkServerTrusted(chain, conscryptToOpenjdkAuthType(authType));
}

@Override
public void checkClientTrusted(X509Certificate[] chain, String authType) throws CertificateException {
delegate.checkClientTrusted(chain, conscryptToOpenjdkAuthType(authType));
}

@Override
public X509Certificate[] getAcceptedIssuers() {
return delegate.getAcceptedIssuers();
}

@Override
public boolean equals(Object other) {
if (this == other) {
return true;
}
if (other == null || getClass() != other.getClass()) {
return false;
}
ConscryptCompatX509TrustManager that = (ConscryptCompatX509TrustManager) other;
return delegate.equals(that.delegate);
}

@Override
public int hashCode() {
return delegate.hashCode();
}

@Override
public String toString() {
return "ConscryptCompatX509TrustManager{" + delegate + '}';
}
}

private static final class ConscryptCompatX509ExtendedTrustManager extends X509ExtendedTrustManager {

private final X509ExtendedTrustManager delegate;

ConscryptCompatX509ExtendedTrustManager(X509ExtendedTrustManager delegate) {
this.delegate = delegate;
}

@Override
public void checkServerTrusted(X509Certificate[] chain, String authType, Socket socket)
throws CertificateException {
delegate.checkServerTrusted(chain, conscryptToOpenjdkAuthType(authType), socket);
}

@Override
public void checkServerTrusted(X509Certificate[] chain, String authType, SSLEngine engine)
throws CertificateException {
delegate.checkServerTrusted(chain, conscryptToOpenjdkAuthType(authType), engine);
}

@Override
public void checkServerTrusted(X509Certificate[] chain, String authType) throws CertificateException {
delegate.checkServerTrusted(chain, conscryptToOpenjdkAuthType(authType));
}

@Override
public void checkClientTrusted(X509Certificate[] chain, String authType, Socket socket)
throws CertificateException {
delegate.checkClientTrusted(chain, conscryptToOpenjdkAuthType(authType), socket);
}

@Override
public void checkClientTrusted(X509Certificate[] chain, String authType, SSLEngine engine)
throws CertificateException {
delegate.checkClientTrusted(chain, conscryptToOpenjdkAuthType(authType), engine);
}

@Override
public void checkClientTrusted(X509Certificate[] chain, String authType) throws CertificateException {
delegate.checkClientTrusted(chain, conscryptToOpenjdkAuthType(authType));
}

@Override
public X509Certificate[] getAcceptedIssuers() {
return delegate.getAcceptedIssuers();
}

@Override
public boolean equals(Object other) {
if (this == other) {
return true;
}
if (other == null || getClass() != other.getClass()) {
return false;
}
ConscryptCompatX509ExtendedTrustManager that = (ConscryptCompatX509ExtendedTrustManager) other;
return delegate.equals(that.delegate);
}

@Override
public int hashCode() {
return delegate.hashCode();
}

@Override
public String toString() {
return "ConscryptCompatX509ExtendedTrustManager{" + delegate + '}';
}
}

/**
* See <a href="https://github.com/google/conscrypt/issues/1033">conscrypt#1033</a>.
*/
private static String conscryptToOpenjdkAuthType(String authType) {
if ("GENERIC".equals(authType)) {
return "UNKNOWN";
}
return authType;
}

private ConscryptCompatTrustManagers() {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ public static SSLContext createSslContext(
TrustManager[] trustManagers, @Nullable KeyManager[] keyManagers, Provider provider) {
try {
SSLContext sslContext = SSLContext.getInstance("TLS", provider);
sslContext.init(keyManagers, trustManagers, null);
sslContext.init(keyManagers, ConscryptCompatTrustManagers.wrap(trustManagers), null);
return sslContext;
} catch (GeneralSecurityException e) {
throw Throwables.propagate(e);
Expand All @@ -178,8 +178,9 @@ public static SSLContext createSslContext(
* @return an {@link TrustManager} array according to the input configuration
*/
public static TrustManager[] createTrustManagers(SslConfiguration config) {
return createTrustManagerFactory(config.trustStorePath(), config.trustStoreType())
.getTrustManagers();
return ConscryptCompatTrustManagers.wrap(
createTrustManagerFactory(config.trustStorePath(), config.trustStoreType())
.getTrustManagers());
}

/**
Expand All @@ -193,7 +194,7 @@ public static TrustManager[] createTrustManagers(Map<String, PemX509Certificate>
TrustManagerFactory trustManagerFactory =
TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm());
trustManagerFactory.init(KeyStores.createTrustStoreFromCertificates(trustCertificatesByAlias));
return trustManagerFactory.getTrustManagers();
return ConscryptCompatTrustManagers.wrap(trustManagerFactory.getTrustManagers());
} catch (NoSuchAlgorithmException | KeyStoreException e) {
throw Throwables.propagate(e);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,16 @@
import java.io.PrintWriter;
import java.net.Socket;
import java.nio.charset.StandardCharsets;
import java.security.Provider;
import java.util.Optional;
import java.util.UUID;
import javax.net.ssl.SSLContext;
import javax.net.ssl.SSLException;
import javax.net.ssl.SSLHandshakeException;
import javax.net.ssl.SSLServerSocket;
import javax.net.ssl.SSLServerSocketFactory;
import javax.net.ssl.SSLSocketFactory;
import org.conscrypt.Conscrypt;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.Timeout;

Expand Down Expand Up @@ -104,7 +107,7 @@ public void testSslNoClientAuthenticationFailsWithoutProperClientTrustStore() {
fail("fail");
})
.isInstanceOf(RuntimeException.class)
.hasCauseInstanceOf(SSLHandshakeException.class)
.hasCauseInstanceOf(SSLException.class)
.hasMessageContaining("PKIX path building failed");
}

Expand Down Expand Up @@ -251,18 +254,42 @@ public void testSslWithClientAuthenticationFailsWithoutProperServerTrustStore()

private void runSslConnectionTest(
SslConfiguration serverConfig, SslConfiguration clientConfig, ClientAuth clientAuth) {
runSslConnectionTest(serverConfig, clientConfig, clientAuth, Optional.empty(), Optional.empty());
// Don't allow these tests to be silently ignored in CI, however modern macbooks aren't currently supported.
if (Conscrypt.isAvailable() || System.getProperty("CI") != null) {
Provider conscrypt = Conscrypt.newProviderBuilder().build();
runSslConnectionTest(serverConfig, clientConfig, clientAuth, Optional.empty(), Optional.of(conscrypt));
runSslConnectionTest(serverConfig, clientConfig, clientAuth, Optional.of(conscrypt), Optional.empty());
runSslConnectionTest(
serverConfig, clientConfig, clientAuth, Optional.of(conscrypt), Optional.of(conscrypt));
}
}

private void runSslConnectionTest(
SslConfiguration serverConfig,
SslConfiguration clientConfig,
ClientAuth clientAuth,
Optional<Provider> serverProvider,
Optional<Provider> clientProvider) {
String message = UUID.randomUUID().toString();

SSLContext sslContext = SslSocketFactories.createSslContext(serverConfig);
SSLContext sslContext = serverProvider
.map(prov -> SslSocketFactories.createSslContext(serverConfig, prov))
.orElseGet(() -> SslSocketFactories.createSslContext(serverConfig));
SSLServerSocketFactory factory = sslContext.getServerSocketFactory();

try (SSLServerSocket sslServerSocket = (SSLServerSocket) factory.createServerSocket(0)) {
Thread serverThread = createSslServerThread(sslServerSocket, clientAuth, message);
serverThread.start();

SSLSocketFactory sslSocketFactory = SslSocketFactories.createSslSocketFactory(clientConfig);
verifySslConnection(sslSocketFactory, sslServerSocket.getLocalPort(), message);
} catch (IOException ex) {
try {
SSLSocketFactory sslSocketFactory = clientProvider
.map(prov -> SslSocketFactories.createSslSocketFactory(clientConfig, prov))
.orElseGet(() -> SslSocketFactories.createSslSocketFactory(clientConfig));
verifySslConnection(sslSocketFactory, sslServerSocket.getLocalPort(), message);
} finally {
serverThread.join();
}
} catch (IOException | InterruptedException ex) {
Throwables.propagate(ex);
}
}
Expand Down
2 changes: 1 addition & 1 deletion versions.lock
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ org.apache-extras.beanshell:bsh:2.0b6 (1 constraints: ac07626b)
org.apache.commons:commons-lang3:3.12.0 (1 constraints: 38053b3b)
org.apiguardian:apiguardian-api:1.1.2 (5 constraints: 105480ac)
org.assertj:assertj-core:3.24.2 (2 constraints: 9c1928df)
org.conscrypt:conscrypt-openjdk-uber:1.4.1 (1 constraints: 0805fd35)
org.conscrypt:conscrypt-openjdk-uber:2.5.2 (1 constraints: 0b050636)
org.glassfish.jersey.containers:jersey-container-servlet-core:3.0.6 (1 constraints: 0b050036)
org.glassfish.jersey.ext:jersey-entity-filtering:3.0.6 (1 constraints: bf1525d3)
org.glassfish.jersey.media:jersey-media-json-jackson:3.0.6 (1 constraints: 0b050036)
Expand Down
3 changes: 1 addition & 2 deletions versions.props
Original file line number Diff line number Diff line change
Expand Up @@ -59,5 +59,4 @@ com.squareup.retrofit2:* = 2.4.0
# Pin to same version as spark
io.dropwizard.metrics:metrics-core = 3.2.6

# Allow tests to run on Centos6
org.conscrypt:conscrypt-openjdk-uber = 1.4.1
org.conscrypt:conscrypt-openjdk-uber = 2.5.2

0 comments on commit e9ee89a

Please sign in to comment.