From c56e5e6e4fb433777a526062a9b5e68430f4ddf2 Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Wed, 19 May 2021 08:19:15 +0200 Subject: [PATCH] Fail the build if we can't load the OpenSSL library (#11269) Motivation: We should better fail the build if we can't load the OpenSSL library to ensure we not introduce a regression at some point related to native library loading Modifications: Remove usages of assumeTrue and let the tests fail if we cant load the native lib Result: Ensure we not regress --- docker/docker-compose.yaml | 6 ++--- .../ConscryptOpenSslEngineInteropTest.java | 2 +- .../ssl/JdkOpenSslEngineInteroptTest.java | 2 +- .../ssl/OpenSslCertificateExceptionTest.java | 10 ++++---- .../handler/ssl/OpenSslClientContextTest.java | 4 +--- .../OpenSslConscryptSslEngineInteropTest.java | 2 +- .../netty/handler/ssl/OpenSslEngineTest.java | 3 ++- .../ssl/OpenSslKeyMaterialManagerTest.java | 2 +- .../ssl/OpenSslKeyMaterialProviderTest.java | 2 +- .../handler/ssl/OpenSslRenegotiateTest.java | 2 +- .../handler/ssl/OpenSslServerContextTest.java | 6 +---- .../io/netty/handler/ssl/PemEncodedTest.java | 3 +-- .../ReferenceCountedOpenSslEngineTest.java | 4 ++-- .../handler/ssl/SslContextBuilderTest.java | 16 ++++++------- .../io/netty/handler/ssl/SslErrorTest.java | 2 +- .../io/netty/handler/ssl/SslHandlerTest.java | 12 +++++----- .../NettyBlockHoundIntegrationTest.java | 23 +++++++++++++++---- 17 files changed, 55 insertions(+), 46 deletions(-) diff --git a/docker/docker-compose.yaml b/docker/docker-compose.yaml index 0cc9a0733a..f2f95fc90f 100644 --- a/docker/docker-compose.yaml +++ b/docker/docker-compose.yaml @@ -25,11 +25,11 @@ services: build-leak: <<: *common - command: /bin/bash -cl "./mvnw -Pleak clean install -Dio.netty.testsuite.badHost=netty.io" + command: /bin/bash -cl "./mvnw -Pleak clean install -Dio.netty.testsuite.badHost=netty.io -Dtcnative.classifier=linux-x86_64-fedora" build: <<: *common - command: /bin/bash -cl "./mvnw clean install -Dio.netty.testsuite.badHost=netty.io" + command: /bin/bash -cl "./mvnw clean install -Dio.netty.testsuite.badHost=netty.io -Dtcnative.classifier=linux-x86_64-fedora" deploy: <<: *common @@ -52,7 +52,7 @@ services: - ~/.m2:/root/.m2 - ~/local-staging:/root/local-staging - ..:/code - command: /bin/bash -cl "cat <(echo -e \"${GPG_PRIVATE_KEY}\") | gpg --batch --import && ./mvnw clean javadoc:jar package gpg:sign org.sonatype.plugins:nexus-staging-maven-plugin:deploy -DnexusUrl=https://oss.sonatype.org -DserverId=sonatype-nexus-staging -DaltStagingDirectory=/root/local-staging -DskipRemoteStaging=true -DskipTests=true -Dgpg.passphrase=${GPG_PASSPHRASE} -Dgpg.keyname=${GPG_KEYNAME}" + command: /bin/bash -cl "cat <(echo -e \"${GPG_PRIVATE_KEY}\") | gpg --batch --import && ./mvnw clean javadoc:jar package gpg:sign org.sonatype.plugins:nexus-staging-maven-plugin:deploy -DnexusUrl=https://oss.sonatype.org -DserverId=sonatype-nexus-staging -DaltStagingDirectory=/root/local-staging -DskipRemoteStaging=true -DskipTests=true -Dgpg.passphrase=${GPG_PASSPHRASE} -Dgpg.keyname=${GPG_KEYNAME} -Dtcnative.classifier=linux-x86_64-fedora" build-boringssl-static: <<: *common diff --git a/handler/src/test/java/io/netty/handler/ssl/ConscryptOpenSslEngineInteropTest.java b/handler/src/test/java/io/netty/handler/ssl/ConscryptOpenSslEngineInteropTest.java index 7a13f9a478..bbf4c37c25 100644 --- a/handler/src/test/java/io/netty/handler/ssl/ConscryptOpenSslEngineInteropTest.java +++ b/handler/src/test/java/io/netty/handler/ssl/ConscryptOpenSslEngineInteropTest.java @@ -58,7 +58,7 @@ public class ConscryptOpenSslEngineInteropTest extends ConscryptSslEngineTest { @BeforeClass public static void checkOpenssl() { - assumeTrue(OpenSsl.isAvailable()); + OpenSsl.ensureAvailability(); } @Override diff --git a/handler/src/test/java/io/netty/handler/ssl/JdkOpenSslEngineInteroptTest.java b/handler/src/test/java/io/netty/handler/ssl/JdkOpenSslEngineInteroptTest.java index b3dd79ebcf..2e75594832 100644 --- a/handler/src/test/java/io/netty/handler/ssl/JdkOpenSslEngineInteroptTest.java +++ b/handler/src/test/java/io/netty/handler/ssl/JdkOpenSslEngineInteroptTest.java @@ -63,7 +63,7 @@ public class JdkOpenSslEngineInteroptTest extends SSLEngineTest { @BeforeClass public static void checkOpenSsl() { - assumeTrue(OpenSsl.isAvailable()); + OpenSsl.ensureAvailability(); } @Override diff --git a/handler/src/test/java/io/netty/handler/ssl/OpenSslCertificateExceptionTest.java b/handler/src/test/java/io/netty/handler/ssl/OpenSslCertificateExceptionTest.java index e17acd0db1..ea086b0e4d 100644 --- a/handler/src/test/java/io/netty/handler/ssl/OpenSslCertificateExceptionTest.java +++ b/handler/src/test/java/io/netty/handler/ssl/OpenSslCertificateExceptionTest.java @@ -17,16 +17,20 @@ package io.netty.handler.ssl; import io.netty.internal.tcnative.CertificateVerifier; import org.junit.Assert; -import org.junit.Assume; +import org.junit.BeforeClass; import org.junit.Test; import java.lang.reflect.Field; public class OpenSslCertificateExceptionTest { + @BeforeClass + public static void ensureOpenSsl() { + OpenSsl.ensureAvailability(); + } + @Test public void testValidErrorCode() throws Exception { - Assume.assumeTrue(OpenSsl.isAvailable()); Field[] fields = CertificateVerifier.class.getFields(); for (Field field : fields) { if (field.isAccessible()) { @@ -39,13 +43,11 @@ public class OpenSslCertificateExceptionTest { @Test(expected = IllegalArgumentException.class) public void testNonValidErrorCode() { - Assume.assumeTrue(OpenSsl.isAvailable()); new OpenSslCertificateException(Integer.MIN_VALUE); } @Test public void testCanBeInstancedWhenOpenSslIsNotAvailable() { - Assume.assumeFalse(OpenSsl.isAvailable()); new OpenSslCertificateException(0); } } diff --git a/handler/src/test/java/io/netty/handler/ssl/OpenSslClientContextTest.java b/handler/src/test/java/io/netty/handler/ssl/OpenSslClientContextTest.java index 5af89105c5..73890ce400 100644 --- a/handler/src/test/java/io/netty/handler/ssl/OpenSslClientContextTest.java +++ b/handler/src/test/java/io/netty/handler/ssl/OpenSslClientContextTest.java @@ -21,13 +21,11 @@ import org.junit.BeforeClass; import javax.net.ssl.SSLException; import java.io.File; -import static org.junit.Assume.assumeTrue; - public class OpenSslClientContextTest extends SslContextTest { @BeforeClass public static void checkOpenSsl() { - assumeTrue(OpenSsl.isAvailable()); + OpenSsl.ensureAvailability(); } @Override diff --git a/handler/src/test/java/io/netty/handler/ssl/OpenSslConscryptSslEngineInteropTest.java b/handler/src/test/java/io/netty/handler/ssl/OpenSslConscryptSslEngineInteropTest.java index 0db6dbc174..b1e67bded9 100644 --- a/handler/src/test/java/io/netty/handler/ssl/OpenSslConscryptSslEngineInteropTest.java +++ b/handler/src/test/java/io/netty/handler/ssl/OpenSslConscryptSslEngineInteropTest.java @@ -57,7 +57,7 @@ public class OpenSslConscryptSslEngineInteropTest extends ConscryptSslEngineTest @BeforeClass public static void checkOpenssl() { - assumeTrue(OpenSsl.isAvailable()); + OpenSsl.ensureAvailability(); } @Override diff --git a/handler/src/test/java/io/netty/handler/ssl/OpenSslEngineTest.java b/handler/src/test/java/io/netty/handler/ssl/OpenSslEngineTest.java index 71849042cc..dac92fb8b4 100644 --- a/handler/src/test/java/io/netty/handler/ssl/OpenSslEngineTest.java +++ b/handler/src/test/java/io/netty/handler/ssl/OpenSslEngineTest.java @@ -111,7 +111,7 @@ public class OpenSslEngineTest extends SSLEngineTest { @BeforeClass public static void checkOpenSsl() { - assumeTrue(OpenSsl.isAvailable()); + OpenSsl.ensureAvailability(); } @Override @@ -1320,6 +1320,7 @@ public class OpenSslEngineTest extends SSLEngineTest { @Test(expected = SSLException.class) public void testNoKeyFound() throws Exception { + checkShouldUseKeyManagerFactory(); clientSslCtx = wrapContext(SslContextBuilder .forClient() .trustManager(InsecureTrustManagerFactory.INSTANCE) diff --git a/handler/src/test/java/io/netty/handler/ssl/OpenSslKeyMaterialManagerTest.java b/handler/src/test/java/io/netty/handler/ssl/OpenSslKeyMaterialManagerTest.java index 2ec22607f8..6538849416 100644 --- a/handler/src/test/java/io/netty/handler/ssl/OpenSslKeyMaterialManagerTest.java +++ b/handler/src/test/java/io/netty/handler/ssl/OpenSslKeyMaterialManagerTest.java @@ -33,7 +33,7 @@ public class OpenSslKeyMaterialManagerTest { @Test public void testChooseClientAliasReturnsNull() throws SSLException { - Assume.assumeTrue(OpenSsl.isAvailable()); + OpenSsl.ensureAvailability(); X509ExtendedKeyManager keyManager = new X509ExtendedKeyManager() { @Override diff --git a/handler/src/test/java/io/netty/handler/ssl/OpenSslKeyMaterialProviderTest.java b/handler/src/test/java/io/netty/handler/ssl/OpenSslKeyMaterialProviderTest.java index 75c638e135..32ea2fcdc4 100644 --- a/handler/src/test/java/io/netty/handler/ssl/OpenSslKeyMaterialProviderTest.java +++ b/handler/src/test/java/io/netty/handler/ssl/OpenSslKeyMaterialProviderTest.java @@ -42,7 +42,7 @@ public class OpenSslKeyMaterialProviderTest { @BeforeClass public static void checkOpenSsl() { - assumeTrue(OpenSsl.isAvailable()); + OpenSsl.ensureAvailability(); } protected KeyManagerFactory newKeyManagerFactory() throws Exception { diff --git a/handler/src/test/java/io/netty/handler/ssl/OpenSslRenegotiateTest.java b/handler/src/test/java/io/netty/handler/ssl/OpenSslRenegotiateTest.java index a19cddf6e0..26fef9847e 100644 --- a/handler/src/test/java/io/netty/handler/ssl/OpenSslRenegotiateTest.java +++ b/handler/src/test/java/io/netty/handler/ssl/OpenSslRenegotiateTest.java @@ -30,7 +30,7 @@ public class OpenSslRenegotiateTest extends RenegotiateTest { @BeforeClass public static void checkOpenSsl() { - assumeTrue(OpenSsl.isAvailable()); + OpenSsl.ensureAvailability(); } @Override diff --git a/handler/src/test/java/io/netty/handler/ssl/OpenSslServerContextTest.java b/handler/src/test/java/io/netty/handler/ssl/OpenSslServerContextTest.java index f92a774c81..c771eb1118 100644 --- a/handler/src/test/java/io/netty/handler/ssl/OpenSslServerContextTest.java +++ b/handler/src/test/java/io/netty/handler/ssl/OpenSslServerContextTest.java @@ -16,24 +16,20 @@ package io.netty.handler.ssl; -import org.junit.Assume; import org.junit.BeforeClass; import javax.net.ssl.SSLException; import java.io.File; -import static org.junit.Assume.assumeTrue; - public class OpenSslServerContextTest extends SslContextTest { @BeforeClass public static void checkOpenSsl() { - assumeTrue(OpenSsl.isAvailable()); + OpenSsl.ensureAvailability(); } @Override protected SslContext newSslContext(File crtFile, File keyFile, String pass) throws SSLException { - Assume.assumeTrue(OpenSsl.isAvailable()); return SslContextBuilder.forServer(crtFile, keyFile, pass).sslProvider(SslProvider.OPENSSL).build(); } } diff --git a/handler/src/test/java/io/netty/handler/ssl/PemEncodedTest.java b/handler/src/test/java/io/netty/handler/ssl/PemEncodedTest.java index 7381512232..4cc9f8029e 100644 --- a/handler/src/test/java/io/netty/handler/ssl/PemEncodedTest.java +++ b/handler/src/test/java/io/netty/handler/ssl/PemEncodedTest.java @@ -19,7 +19,6 @@ package io.netty.handler.ssl; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; import static org.junit.Assume.assumeFalse; -import static org.junit.Assume.assumeTrue; import java.io.ByteArrayOutputStream; import java.io.File; @@ -45,7 +44,7 @@ public class PemEncodedTest { } private static void testPemEncoded(SslProvider provider) throws Exception { - assumeTrue(OpenSsl.isAvailable()); + OpenSsl.ensureAvailability(); assumeFalse(OpenSsl.supportsKeyManagerFactory()); PemPrivateKey pemKey; PemX509Certificate pemCert; diff --git a/handler/src/test/java/io/netty/handler/ssl/ReferenceCountedOpenSslEngineTest.java b/handler/src/test/java/io/netty/handler/ssl/ReferenceCountedOpenSslEngineTest.java index f795ed2395..666420ec60 100644 --- a/handler/src/test/java/io/netty/handler/ssl/ReferenceCountedOpenSslEngineTest.java +++ b/handler/src/test/java/io/netty/handler/ssl/ReferenceCountedOpenSslEngineTest.java @@ -48,7 +48,7 @@ public class ReferenceCountedOpenSslEngineTest extends OpenSslEngineTest { @Override protected void cleanupClientSslEngine(SSLEngine engine) { - ReferenceCountUtil.release(engine); + ReferenceCountUtil.release(unwrapEngine(engine)); } @Override @@ -58,7 +58,7 @@ public class ReferenceCountedOpenSslEngineTest extends OpenSslEngineTest { @Override protected void cleanupServerSslEngine(SSLEngine engine) { - ReferenceCountUtil.release(engine); + ReferenceCountUtil.release(unwrapEngine(engine)); } @Test(expected = NullPointerException.class) diff --git a/handler/src/test/java/io/netty/handler/ssl/SslContextBuilderTest.java b/handler/src/test/java/io/netty/handler/ssl/SslContextBuilderTest.java index d2c4cff450..16de1d7870 100644 --- a/handler/src/test/java/io/netty/handler/ssl/SslContextBuilderTest.java +++ b/handler/src/test/java/io/netty/handler/ssl/SslContextBuilderTest.java @@ -46,7 +46,7 @@ public class SslContextBuilderTest { @Test public void testClientContextFromFileOpenssl() throws Exception { - Assume.assumeTrue(OpenSsl.isAvailable()); + OpenSsl.ensureAvailability(); testClientContextFromFile(SslProvider.OPENSSL); } @@ -57,7 +57,7 @@ public class SslContextBuilderTest { @Test public void testClientContextOpenssl() throws Exception { - Assume.assumeTrue(OpenSsl.isAvailable()); + OpenSsl.ensureAvailability(); testClientContext(SslProvider.OPENSSL); } @@ -68,7 +68,7 @@ public class SslContextBuilderTest { @Test public void testKeyStoreTypeOpenssl() throws Exception { - Assume.assumeTrue(OpenSsl.isAvailable()); + OpenSsl.ensureAvailability(); testKeyStoreType(SslProvider.OPENSSL); } @@ -79,7 +79,7 @@ public class SslContextBuilderTest { @Test public void testServerContextFromFileOpenssl() throws Exception { - Assume.assumeTrue(OpenSsl.isAvailable()); + OpenSsl.ensureAvailability(); testServerContextFromFile(SslProvider.OPENSSL); } @@ -90,7 +90,7 @@ public class SslContextBuilderTest { @Test public void testServerContextOpenssl() throws Exception { - Assume.assumeTrue(OpenSsl.isAvailable()); + OpenSsl.ensureAvailability(); testServerContext(SslProvider.OPENSSL); } @@ -101,7 +101,7 @@ public class SslContextBuilderTest { @Test public void testContextFromManagersOpenssl() throws Exception { - Assume.assumeTrue(OpenSsl.isAvailable()); + OpenSsl.ensureAvailability(); Assume.assumeTrue(OpenSsl.supportsKeyManagerFactory()); testContextFromManagers(SslProvider.OPENSSL); } @@ -154,13 +154,13 @@ public class SslContextBuilderTest { @Test(expected = IllegalArgumentException.class) public void testInvalidCipherJdk() throws Exception { - Assume.assumeTrue(OpenSsl.isAvailable()); + OpenSsl.ensureAvailability(); testInvalidCipher(SslProvider.JDK); } @Test public void testInvalidCipherOpenSSL() throws Exception { - Assume.assumeTrue(OpenSsl.isAvailable()); + OpenSsl.ensureAvailability(); try { // This may fail or not depending on the OpenSSL version used // See https://github.com/openssl/openssl/issues/7196 diff --git a/handler/src/test/java/io/netty/handler/ssl/SslErrorTest.java b/handler/src/test/java/io/netty/handler/ssl/SslErrorTest.java index 74e29bc7a1..47c230c230 100644 --- a/handler/src/test/java/io/netty/handler/ssl/SslErrorTest.java +++ b/handler/src/test/java/io/netty/handler/ssl/SslErrorTest.java @@ -127,7 +127,7 @@ public class SslErrorTest { public void testCorrectAlert() throws Exception { // As this only works correctly at the moment when OpenSslEngine is used on the server-side there is // no need to run it if there is no openssl is available at all. - Assume.assumeTrue(OpenSsl.isAvailable()); + OpenSsl.ensureAvailability(); SelfSignedCertificate ssc = new SelfSignedCertificate(); diff --git a/handler/src/test/java/io/netty/handler/ssl/SslHandlerTest.java b/handler/src/test/java/io/netty/handler/ssl/SslHandlerTest.java index a88a6bdad3..bd2b1cdf06 100644 --- a/handler/src/test/java/io/netty/handler/ssl/SslHandlerTest.java +++ b/handler/src/test/java/io/netty/handler/ssl/SslHandlerTest.java @@ -365,7 +365,7 @@ public class SslHandlerTest { @Test public void testReleaseSslEngine() throws Exception { - assumeTrue(OpenSsl.isAvailable()); + OpenSsl.ensureAvailability(); SelfSignedCertificate cert = new SelfSignedCertificate(); try { @@ -1111,7 +1111,7 @@ public class SslHandlerTest { } private static void testSessionTickets(SslProvider provider, String protocol, boolean withKey) throws Throwable { - assumeTrue(OpenSsl.isAvailable()); + OpenSsl.ensureAvailability(); final SslContext sslClientCtx = SslContextBuilder.forClient() .trustManager(InsecureTrustManagerFactory.INSTANCE) .sslProvider(provider) @@ -1390,13 +1390,13 @@ public class SslHandlerTest { @Test public void testHandshakeFailureCipherMissmatchTLSv12OpenSsl() throws Exception { - Assume.assumeTrue(OpenSsl.isAvailable()); + OpenSsl.ensureAvailability(); testHandshakeFailureCipherMissmatch(SslProvider.OPENSSL, false); } @Test public void testHandshakeFailureCipherMissmatchTLSv13OpenSsl() throws Exception { - Assume.assumeTrue(OpenSsl.isAvailable()); + OpenSsl.ensureAvailability(); Assume.assumeTrue(SslProvider.isTlsv13Supported(SslProvider.OPENSSL)); Assume.assumeFalse("BoringSSL does not support setting ciphers for TLSv1.3 explicit", OpenSsl.isBoringSSL()); testHandshakeFailureCipherMissmatch(SslProvider.OPENSSL, true); @@ -1509,7 +1509,7 @@ public class SslHandlerTest { @Test public void testHandshakeEventsTls12Openssl() throws Exception { - assumeTrue(OpenSsl.isAvailable()); + OpenSsl.ensureAvailability(); testHandshakeEvents(SslProvider.OPENSSL, SslUtils.PROTOCOL_TLS_V1_2); } @@ -1521,7 +1521,7 @@ public class SslHandlerTest { @Test public void testHandshakeEventsTls13Openssl() throws Exception { - assumeTrue(OpenSsl.isAvailable()); + OpenSsl.ensureAvailability(); assumeTrue(SslProvider.isTlsv13Supported(SslProvider.OPENSSL)); testHandshakeEvents(SslProvider.OPENSSL, SslUtils.PROTOCOL_TLS_V1_3); } diff --git a/transport-blockhound-tests/src/test/java/io/netty/util/internal/NettyBlockHoundIntegrationTest.java b/transport-blockhound-tests/src/test/java/io/netty/util/internal/NettyBlockHoundIntegrationTest.java index 685c69f460..fe1de6dfd6 100644 --- a/transport-blockhound-tests/src/test/java/io/netty/util/internal/NettyBlockHoundIntegrationTest.java +++ b/transport-blockhound-tests/src/test/java/io/netty/util/internal/NettyBlockHoundIntegrationTest.java @@ -240,14 +240,25 @@ public class NettyBlockHoundIntegrationTest { } @Test - public void testTrustManagerVerify() throws Exception { - testTrustManagerVerify("TLSv1.2"); + public void testTrustManagerVerifyJDK() throws Exception { + testTrustManagerVerify(SslProvider.JDK, "TLSv1.2"); } @Test - public void testTrustManagerVerifyTLSv13() throws Exception { + public void testTrustManagerVerifyTLSv13JDK() throws Exception { assumeTrue(SslProvider.isTlsv13Supported(SslProvider.JDK)); - testTrustManagerVerify("TLSv1.3"); + testTrustManagerVerify(SslProvider.JDK, "TLSv1.3"); + } + + @Test + public void testTrustManagerVerifyOpenSSL() throws Exception { + testTrustManagerVerify(SslProvider.OPENSSL, "TLSv1.2"); + } + + @Test + public void testTrustManagerVerifyTLSv13OpenSSL() throws Exception { + assumeTrue(SslProvider.isTlsv13Supported(SslProvider.OPENSSL)); + testTrustManagerVerify(SslProvider.OPENSSL, "TLSv1.3"); } @Test @@ -378,9 +389,10 @@ public class NettyBlockHoundIntegrationTest { } } - private static void testTrustManagerVerify(String tlsVersion) throws Exception { + private static void testTrustManagerVerify(SslProvider provider, String tlsVersion) throws Exception { final SslContext sslClientCtx = SslContextBuilder.forClient() + .sslProvider(provider) .protocols(tlsVersion) .trustManager(ResourcesUtil.getFile( NettyBlockHoundIntegrationTest.class, "mutual_auth_ca.pem")) @@ -392,6 +404,7 @@ public class NettyBlockHoundIntegrationTest { ResourcesUtil.getFile( NettyBlockHoundIntegrationTest.class, "localhost_server.key"), null) + .sslProvider(provider) .protocols(tlsVersion) .build();