From de17384e7a88c4e48010600eccdec46174732730 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 | 3 +-- .../io/netty/handler/ssl/PemEncodedTest.java | 2 +- .../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(+), 42 deletions(-) diff --git a/docker/docker-compose.yaml b/docker/docker-compose.yaml index 0cc9a0733a3..f2f95fc90fd 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 7a13f9a4781..bbf4c37c255 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 ConscryptOpenSslEngineInteropTest(BufferType type, ProtocolCipherCombo co @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 b77d82fa305..7a36a6ffffc 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 JdkOpenSslEngineInteroptTest(BufferType type, ProtocolCipherCombo protoco @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 e17acd0db1d..ea086b0e4d1 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 @@ 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 void testValidErrorCode() throws Exception { @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 fb88de2775d..7586d24f18b 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 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 0db6dbc1749..b1e67bded90 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 OpenSslConscryptSslEngineInteropTest(BufferType type, ProtocolCipherCombo @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 106c90edd64..cf367fd8cf3 100644 --- a/handler/src/test/java/io/netty/handler/ssl/OpenSslEngineTest.java +++ b/handler/src/test/java/io/netty/handler/ssl/OpenSslEngineTest.java @@ -110,7 +110,7 @@ public OpenSslEngineTest(BufferType type, ProtocolCipherCombo cipherCombo, boole @BeforeClass public static void checkOpenSsl() { - assumeTrue(OpenSsl.isAvailable()); + OpenSsl.ensureAvailability(); } @Override @@ -1321,6 +1321,7 @@ public void testExtractMasterkeyWorksCorrectly() throws Exception { @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 2ec22607f8a..6538849416f 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 75c638e135f..32ea2fcdc47 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 a19cddf6e04..26fef9847e2 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 a4b9079e784..a1b23436f38 100644 --- a/handler/src/test/java/io/netty/handler/ssl/OpenSslServerContextTest.java +++ b/handler/src/test/java/io/netty/handler/ssl/OpenSslServerContextTest.java @@ -28,12 +28,11 @@ 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 new OpenSslServerContext(crtFile, keyFile, pass); } } 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 525f6555659..9b75f5c7290 100644 --- a/handler/src/test/java/io/netty/handler/ssl/PemEncodedTest.java +++ b/handler/src/test/java/io/netty/handler/ssl/PemEncodedTest.java @@ -45,7 +45,7 @@ public void testPemEncodedOpenSslRef() throws Exception { } private static void testPemEncoded(SslProvider provider) throws Exception { - assumeTrue(OpenSsl.isAvailable()); + OpenSsl.ensureAvailability(); assumeFalse(OpenSsl.useKeyManagerFactory()); 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 f795ed2395f..666420ec60a 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 @@ protected void cleanupClientSslContext(SslContext ctx) { @Override protected void cleanupClientSslEngine(SSLEngine engine) { - ReferenceCountUtil.release(engine); + ReferenceCountUtil.release(unwrapEngine(engine)); } @Override @@ -58,7 +58,7 @@ protected void cleanupServerSslContext(SslContext ctx) { @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 0c30cac2fe2..390121b45bb 100644 --- a/handler/src/test/java/io/netty/handler/ssl/SslContextBuilderTest.java +++ b/handler/src/test/java/io/netty/handler/ssl/SslContextBuilderTest.java @@ -47,7 +47,7 @@ public void testClientContextFromFileJdk() throws Exception { @Test public void testClientContextFromFileOpenssl() throws Exception { - Assume.assumeTrue(OpenSsl.isAvailable()); + OpenSsl.ensureAvailability(); testClientContextFromFile(SslProvider.OPENSSL); } @@ -58,7 +58,7 @@ public void testClientContextJdk() throws Exception { @Test public void testClientContextOpenssl() throws Exception { - Assume.assumeTrue(OpenSsl.isAvailable()); + OpenSsl.ensureAvailability(); testClientContext(SslProvider.OPENSSL); } @@ -69,7 +69,7 @@ public void testKeyStoreTypeJdk() throws Exception { @Test public void testKeyStoreTypeOpenssl() throws Exception { - Assume.assumeTrue(OpenSsl.isAvailable()); + OpenSsl.ensureAvailability(); testKeyStoreType(SslProvider.OPENSSL); } @@ -80,7 +80,7 @@ public void testServerContextFromFileJdk() throws Exception { @Test public void testServerContextFromFileOpenssl() throws Exception { - Assume.assumeTrue(OpenSsl.isAvailable()); + OpenSsl.ensureAvailability(); testServerContextFromFile(SslProvider.OPENSSL); } @@ -91,7 +91,7 @@ public void testServerContextJdk() throws Exception { @Test public void testServerContextOpenssl() throws Exception { - Assume.assumeTrue(OpenSsl.isAvailable()); + OpenSsl.ensureAvailability(); testServerContext(SslProvider.OPENSSL); } @@ -102,7 +102,7 @@ public void testContextFromManagersJdk() throws Exception { @Test public void testContextFromManagersOpenssl() throws Exception { - Assume.assumeTrue(OpenSsl.isAvailable()); + OpenSsl.ensureAvailability(); Assume.assumeTrue(OpenSsl.useKeyManagerFactory()); testContextFromManagers(SslProvider.OPENSSL); } @@ -155,13 +155,13 @@ private static void testUnsupportedPrivateKeyFailsFast(boolean server) throws Ex @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 2b264712ca6..89b71e47165 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 SslErrorTest(SslProvider serverProvider, SslProvider clientProvider, 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 1945d1e260a..861a3469fe9 100644 --- a/handler/src/test/java/io/netty/handler/ssl/SslHandlerTest.java +++ b/handler/src/test/java/io/netty/handler/ssl/SslHandlerTest.java @@ -367,7 +367,7 @@ public void testIncompleteWriteDoesNotCompletePromisePrematurely() throws NoSuch @Test public void testReleaseSslEngine() throws Exception { - assumeTrue(OpenSsl.isAvailable()); + OpenSsl.ensureAvailability(); SelfSignedCertificate cert = new SelfSignedCertificate(); try { @@ -1136,7 +1136,7 @@ public void testSessionTicketsWithTLSv13AndNoKey() throws Throwable { } 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) @@ -1418,13 +1418,13 @@ public void testHandshakeFailureCipherMissmatchTLSv13Jdk() throws Exception { @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); @@ -1537,7 +1537,7 @@ public void testHandshakeEventsTls12JDK() throws Exception { @Test public void testHandshakeEventsTls12Openssl() throws Exception { - assumeTrue(OpenSsl.isAvailable()); + OpenSsl.ensureAvailability(); testHandshakeEvents(SslProvider.OPENSSL, SslUtils.PROTOCOL_TLS_V1_2); } @@ -1549,7 +1549,7 @@ public void testHandshakeEventsTls13JDK() throws Exception { @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 1b454a1802e..7849e15de0c 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 void testHandshakeWithExecutorTLSv13() throws Exception { } @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 @@ protected void run() { } } - 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 @@ private static void testTrustManagerVerify(String tlsVersion) throws Exception { ResourcesUtil.getFile( NettyBlockHoundIntegrationTest.class, "localhost_server.key"), null) + .sslProvider(provider) .protocols(tlsVersion) .build();