Update to latest Conscrypt release and add workarounds for bugs (#10211)

Motivation:

We are far behind with the version of Conscrypt we are using during testing. We should ensure we use the latest.

Modifications:

- Update conscrypt dependency
- Ensure we use conscrypt provider in tests
- Add workarounds for conscrypt bugs in testsuite

Result:

Use latest Conscrypt release
This commit is contained in:
Norman Maurer 2020-04-28 09:50:05 +02:00
parent 8fe9c013dd
commit ae95e9c3d6
4 changed files with 47 additions and 33 deletions

View File

@ -80,18 +80,4 @@ public class ConscryptJdkSslEngineInteropTest extends SSLEngineTest {
// TODO(scott): work around for a JDK issue. The exception should be SSLHandshakeException.
return super.mySetupMutualAuthServerIsValidServerException(cause) || causedBySSLException(cause);
}
@Ignore("Ignore due bug in Conscrypt")
@Override
public void testSessionBindingEvent() throws Exception {
// Ignore due bug in Conscrypt where the incorrect SSLSession object is used in the SSLSessionBindingEvent.
// See https://github.com/google/conscrypt/issues/593
}
@Ignore("Ignore due bug in Conscrypt")
@Override
public void testHandshakeSession() throws Exception {
// Ignore as Conscrypt does not correctly return the local certificates while the TrustManager is invoked.
// See https://github.com/google/conscrypt/issues/634
}
}

View File

@ -78,18 +78,4 @@ public class ConscryptSslEngineTest extends SSLEngineTest {
@Override
public void testMutualAuthValidClientCertChainTooLongFailRequireClientAuth() {
}
@Ignore("Ignore due bug in Conscrypt")
@Override
public void testSessionBindingEvent() throws Exception {
// Ignore due bug in Conscrypt where the incorrect SSLSession object is used in the SSLSessionBindingEvent.
// See https://github.com/google/conscrypt/issues/593
}
@Ignore("Ignore due bug in Conscrypt")
@Override
public void testHandshakeSession() throws Exception {
// Ignore as Conscrypt does not correctly return the local certificates while the TrustManager is invoked.
// See https://github.com/google/conscrypt/issues/634
}
}

View File

@ -1959,12 +1959,14 @@ public abstract class SSLEngineTest {
.trustManager(InsecureTrustManagerFactory.INSTANCE)
.ciphers(Collections.singletonList(sharedCipher))
.protocols(PROTOCOL_TLS_V1_2, PROTOCOL_TLS_V1)
.sslContextProvider(clientSslContextProvider())
.sslProvider(sslClientProvider())
.build());
serverSslCtx = wrapContext(SslContextBuilder.forServer(ssc.certificate(), ssc.privateKey())
.ciphers(Collections.singletonList(sharedCipher))
.protocols(PROTOCOL_TLS_V1_2, PROTOCOL_TLS_V1)
.sslContextProvider(serverSslContextProvider())
.sslProvider(sslServerProvider())
.build());
SSLEngine clientEngine = null;
@ -1990,12 +1992,14 @@ public abstract class SSLEngineTest {
.trustManager(InsecureTrustManagerFactory.INSTANCE)
.ciphers(Collections.singletonList(sharedCipher), SupportedCipherSuiteFilter.INSTANCE)
.protocols(PROTOCOL_TLS_V1_2, PROTOCOL_TLS_V1)
.sslContextProvider(clientSslContextProvider())
.sslProvider(sslClientProvider())
.build());
serverSslCtx = wrapContext(SslContextBuilder.forServer(ssc.certificate(), ssc.privateKey())
.ciphers(Collections.singletonList(sharedCipher), SupportedCipherSuiteFilter.INSTANCE)
.protocols(PROTOCOL_TLS_V1_2, PROTOCOL_TLS_V1)
.sslContextProvider(serverSslContextProvider())
.sslProvider(sslServerProvider())
.build());
SSLEngine clientEngine = null;
@ -2018,6 +2022,7 @@ public abstract class SSLEngineTest {
clientSslCtx = wrapContext(SslContextBuilder
.forClient()
.trustManager(cert.cert())
.sslContextProvider(clientSslContextProvider())
.sslProvider(sslClientProvider())
.protocols(protocols())
.ciphers(ciphers())
@ -2026,6 +2031,7 @@ public abstract class SSLEngineTest {
serverSslCtx = wrapContext(SslContextBuilder
.forServer(cert.certificate(), cert.privateKey())
.sslContextProvider(serverSslContextProvider())
.sslProvider(sslServerProvider())
.protocols(protocols())
.ciphers(ciphers())
@ -2061,6 +2067,7 @@ public abstract class SSLEngineTest {
public void testSSLEngineUnwrapNoSslRecord() throws Exception {
clientSslCtx = wrapContext(SslContextBuilder
.forClient()
.sslContextProvider(clientSslContextProvider())
.sslProvider(sslClientProvider())
.protocols(protocols())
.ciphers(ciphers())
@ -2091,6 +2098,7 @@ public abstract class SSLEngineTest {
public void testBeginHandshakeAfterEngineClosed() throws SSLException {
clientSslCtx = wrapContext(SslContextBuilder
.forClient()
.sslContextProvider(clientSslContextProvider())
.sslProvider(sslClientProvider())
.protocols(protocols())
.ciphers(ciphers())
@ -2105,6 +2113,12 @@ public abstract class SSLEngineTest {
fail();
} catch (SSLException expected) {
// expected
} catch (IllegalStateException e) {
if (!Conscrypt.isEngineSupported(client)) {
throw e;
}
// Workaround for conscrypt bug
// See https://github.com/google/conscrypt/issues/840
}
} finally {
cleanupClientSslEngine(client);
@ -2117,6 +2131,7 @@ public abstract class SSLEngineTest {
clientSslCtx = wrapContext(SslContextBuilder
.forClient()
.sslContextProvider(clientSslContextProvider())
.sslProvider(sslClientProvider())
.protocols(protocols())
.ciphers(ciphers())
@ -2125,6 +2140,7 @@ public abstract class SSLEngineTest {
serverSslCtx = wrapContext(SslContextBuilder
.forServer(cert.certificate(), cert.privateKey())
.sslContextProvider(serverSslContextProvider())
.sslProvider(sslServerProvider())
.protocols(protocols())
.ciphers(ciphers())
@ -2168,6 +2184,7 @@ public abstract class SSLEngineTest {
clientSslCtx = wrapContext(SslContextBuilder
.forClient()
.sslContextProvider(clientSslContextProvider())
.sslProvider(sslClientProvider())
.protocols(protocols())
.ciphers(ciphers())
@ -2176,6 +2193,7 @@ public abstract class SSLEngineTest {
serverSslCtx = wrapContext(SslContextBuilder
.forServer(cert.certificate(), cert.privateKey())
.sslContextProvider(serverSslContextProvider())
.sslProvider(sslServerProvider())
.protocols(protocols())
.ciphers(ciphers())
@ -2196,7 +2214,11 @@ public abstract class SSLEngineTest {
engine.beginHandshake();
try {
engine.closeInbound();
fail();
// Workaround for conscrypt bug
// See https://github.com/google/conscrypt/issues/839
if (!Conscrypt.isEngineSupported(engine)) {
fail();
}
} catch (SSLException expected) {
// expected
}
@ -2209,6 +2231,7 @@ public abstract class SSLEngineTest {
clientSslCtx = wrapContext(SslContextBuilder
.forClient()
.trustManager(cert.cert())
.sslContextProvider(clientSslContextProvider())
.sslProvider(sslClientProvider())
// This test only works for non TLSv1.3 for now
.protocols(PROTOCOL_TLS_V1_2)
@ -2217,6 +2240,7 @@ public abstract class SSLEngineTest {
serverSslCtx = wrapContext(SslContextBuilder
.forServer(cert.certificate(), cert.privateKey())
.sslContextProvider(serverSslContextProvider())
.sslProvider(sslServerProvider())
// This test only works for non TLSv1.3 for now
.protocols(PROTOCOL_TLS_V1_2)
@ -2246,7 +2270,8 @@ public abstract class SSLEngineTest {
assertEquals(SSLEngineResult.Status.CLOSED, result.getStatus());
// Need an UNWRAP to read the response of the close_notify
if (PlatformDependent.javaVersion() >= 12 && sslClientProvider() == SslProvider.JDK) {
if ((PlatformDependent.javaVersion() >= 12 && sslClientProvider() == SslProvider.JDK)
|| Conscrypt.isEngineSupported(client)) {
// This is a workaround for a possible JDK12+ bug.
//
// See http://mail.openjdk.java.net/pipermail/security-dev/2019-February/019406.html.
@ -2364,6 +2389,7 @@ public abstract class SSLEngineTest {
.forClient()
.trustManager(cert.cert())
.sslProvider(sslClientProvider())
.sslContextProvider(clientSslContextProvider())
.protocols(protocols())
.ciphers(ciphers())
.build());
@ -2372,6 +2398,7 @@ public abstract class SSLEngineTest {
serverSslCtx = wrapContext(SslContextBuilder
.forServer(cert.certificate(), cert.privateKey())
.sslProvider(sslServerProvider())
.sslContextProvider(serverSslContextProvider())
.protocols(protocols())
.ciphers(ciphers())
.build());
@ -2406,6 +2433,7 @@ public abstract class SSLEngineTest {
clientSslCtx = wrapContext(SslContextBuilder
.forClient()
.trustManager(cert.cert())
.sslContextProvider(clientSslContextProvider())
.sslProvider(sslClientProvider())
.protocols(protocols())
.ciphers(ciphers())
@ -2414,6 +2442,7 @@ public abstract class SSLEngineTest {
serverSslCtx = wrapContext(SslContextBuilder
.forServer(cert.certificate(), cert.privateKey())
.sslContextProvider(serverSslContextProvider())
.sslProvider(sslServerProvider())
.protocols(protocols())
.ciphers(ciphers())
@ -2485,6 +2514,7 @@ public abstract class SSLEngineTest {
clientSslCtx = wrapContext(SslContextBuilder
.forClient()
.trustManager(cert.cert())
.sslContextProvider(clientSslContextProvider())
.sslProvider(sslClientProvider())
.protocols(protocols())
.ciphers(ciphers())
@ -2493,6 +2523,7 @@ public abstract class SSLEngineTest {
serverSslCtx = wrapContext(SslContextBuilder
.forServer(cert.certificate(), cert.privateKey())
.sslContextProvider(serverSslContextProvider())
.sslProvider(sslServerProvider())
.protocols(protocols())
.ciphers(ciphers())
@ -2553,6 +2584,7 @@ public abstract class SSLEngineTest {
clientSslCtx = wrapContext(SslContextBuilder
.forClient()
.trustManager(cert.cert())
.sslContextProvider(clientSslContextProvider())
.sslProvider(sslClientProvider())
.protocols(protocols())
.ciphers(ciphers())
@ -2561,6 +2593,7 @@ public abstract class SSLEngineTest {
serverSslCtx = wrapContext(SslContextBuilder
.forServer(cert.certificate(), cert.privateKey())
.sslContextProvider(serverSslContextProvider())
.sslProvider(sslServerProvider())
.protocols(protocols())
.ciphers(ciphers())
@ -2628,6 +2661,7 @@ public abstract class SSLEngineTest {
clientSslCtx = wrapContext(SslContextBuilder
.forClient()
.trustManager(cert.cert())
.sslContextProvider(clientSslContextProvider())
.sslProvider(sslClientProvider())
.protocols(protocols())
.ciphers(ciphers())
@ -2636,6 +2670,7 @@ public abstract class SSLEngineTest {
serverSslCtx = wrapContext(SslContextBuilder
.forServer(cert.certificate(), cert.privateKey())
.sslContextProvider(serverSslContextProvider())
.sslProvider(sslServerProvider())
.protocols(protocols())
.ciphers(ciphers())
@ -2684,6 +2719,7 @@ public abstract class SSLEngineTest {
SslContext ctx = wrapContext(SslContextBuilder
.forServer(cert.certificate(), cert.privateKey())
.sslContextProvider(serverSslContextProvider())
.sslProvider(sslServerProvider())
.protocols(protocols())
.ciphers(ciphers())
@ -2716,7 +2752,10 @@ public abstract class SSLEngineTest {
@Test
public void testUsingX509TrustManagerVerifiesHostname() throws Exception {
SslProvider clientProvider = sslClientProvider();
if (clientSslContextProvider() != null) {
// Not supported when using conscrypt
return;
}
SelfSignedCertificate cert = new SelfSignedCertificate();
clientSslCtx = wrapContext(SslContextBuilder
.forClient()
@ -2755,6 +2794,7 @@ public abstract class SSLEngineTest {
}
}, null, TrustManagerFactory.getDefaultAlgorithm()) {
})
.sslContextProvider(clientSslContextProvider())
.sslProvider(sslClientProvider())
.build());
@ -2765,6 +2805,7 @@ public abstract class SSLEngineTest {
serverSslCtx = wrapContext(SslContextBuilder
.forServer(cert.certificate(), cert.privateKey())
.sslContextProvider(serverSslContextProvider())
.sslProvider(sslServerProvider())
.build());
@ -2790,7 +2831,8 @@ public abstract class SSLEngineTest {
SSLEngine server = null;
try {
serverSslCtx = wrapContext(SslContextBuilder.forServer(cert.key(), cert.cert())
.sslProvider(sslClientProvider())
.sslContextProvider(serverSslContextProvider())
.sslProvider(sslServerProvider())
.ciphers(cipherList).build());
server = wrapEngine(serverSslCtx.newEngine(UnpooledByteBufAllocator.DEFAULT));
fail();

View File

@ -350,7 +350,7 @@
<tcnative.classifier>${os.detected.classifier}</tcnative.classifier>
<conscrypt.groupId>org.conscrypt</conscrypt.groupId>
<conscrypt.artifactId>conscrypt-openjdk-uber</conscrypt.artifactId>
<conscrypt.version>1.3.0</conscrypt.version>
<conscrypt.version>2.4.0</conscrypt.version>
<conscrypt.classifier />
<jni.classifier>${os.detected.name}-${os.detected.arch}</jni.classifier>
<logging.config>${project.basedir}/../common/src/test/resources/logback-test.xml</logging.config>