andreacavalli/netty5
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Remove call to SSL.setHostNameValidation(...) as it is done in the TrustManager (#8981)

Browse Source 
Motivation:

We do not need to call SSL.setHostNameValidation(...) as it should be done as part of the TrustManager implementation. This is consistent with the JDK implementation of SSLEngine.

Modifications:

Remove call to SSL.setHostNameValidation(...)

Result:

More consistent behaviour between our SSLEngine implementation and the one that comes with the JDK.
This commit is contained in:
Norman Maurer 2019-04-01 21:02:36 +02:00
parent 1eabb752c0
commit 6b8a0ed374
7 changed files with 7 additions and 92 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

21
handler/src/main/java/io/netty/handler/ssl/OpenSsl.java
View File

@ -58,7 +58,6 @@ public final class OpenSsl {
private static final Set<String> AVAILABLE_OPENSSL_CIPHER_SUITES;
private static final Set<String> AVAILABLE_JAVA_CIPHER_SUITES;
private static final boolean SUPPORTS_KEYMANAGER_FACTORY;
private static final boolean SUPPORTS_HOSTNAME_VALIDATION;
private static final boolean USE_KEYMANAGER_FACTORY;
private static final boolean SUPPORTS_OCSP;
private static final boolean TLSV13_SUPPORTED;
@ -200,7 +199,6 @@ public final class OpenSsl {
final Set<String> availableOpenSslCipherSuites = new LinkedHashSet<>(128);
boolean supportsKeyManagerFactory = false;
boolean useKeyManagerFactory = false;
boolean supportsHostNameValidation = false;
boolean tlsv13Supported = false;
IS_BORINGSSL = "BoringSSL".equals(versionString());
@ -257,16 +255,10 @@ public final class OpenSsl {
"AEAD-AES256-GCM-SHA384",
"AEAD-CHACHA20-POLY1305-SHA256");
}
try {
SSL.setHostNameValidation(ssl, 0, "netty.io");
supportsHostNameValidation = true;
} catch (Throwable ignore) {
logger.debug("Hostname Verification not supported.");
}
PemEncoded privateKey = PemPrivateKey.toPEM(UnpooledByteBufAllocator.DEFAULT, true, KEY_BYTES);
try {
X509Certificate certificate = selfSignedCertificate();
X509Certificate certificate = selfSignedCertificate();
certBio = ReferenceCountedOpenSslContext.toBIO(ByteBufAllocator.DEFAULT, certificate);
cert = SSL.parseX509Chain(certBio);
@ -338,7 +330,6 @@ public final class OpenSsl {
AVAILABLE_CIPHER_SUITES = availableCipherSuites;
SUPPORTS_KEYMANAGER_FACTORY = supportsKeyManagerFactory;
SUPPORTS_HOSTNAME_VALIDATION = supportsHostNameValidation;
USE_KEYMANAGER_FACTORY = useKeyManagerFactory;
Set<String> protocols = new LinkedHashSet<>(6);
@ -381,7 +372,6 @@ public final class OpenSsl {
AVAILABLE_JAVA_CIPHER_SUITES = Collections.emptySet();
AVAILABLE_CIPHER_SUITES = Collections.emptySet();
SUPPORTS_KEYMANAGER_FACTORY = false;
SUPPORTS_HOSTNAME_VALIDATION = false;
USE_KEYMANAGER_FACTORY = false;
SUPPORTED_PROTOCOLS_SET = Collections.emptySet();
SUPPORTS_OCSP = false;
@ -540,11 +530,14 @@ public final class OpenSsl {
}
/**
* Returns {@code true} if <a href="https://wiki.openssl.org/index.php/Hostname_validation">Hostname Validation</a>
* is supported when using OpenSSL.
* Always returns {@code true} if {@link #isAvailable()} returns {@code true}.
*
* @deprecated Will be removed because hostname validation is always done by a
* {@link javax.net.ssl.TrustManager} implementation.
*/
@Deprecated
public static boolean supportsHostnameValidation() {
return SUPPORTS_HOSTNAME_VALIDATION;
return isAvailable();
}
static boolean useKeyManagerFactory() {

16
handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslEngine.java
View File

@ -120,10 +120,6 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc
SSL.SSL_OP_NO_TLSv1_2,
SSL.SSL_OP_NO_TLSv1_3
};
/**
* <a href="https://www.openssl.org/docs/man1.0.2/crypto/X509_check_host.html">The flags argument is usually 0</a>.
*/
private static final int DEFAULT_HOSTNAME_VALIDATION_FLAGS = 0;
/**
* Depends upon tcnative ... only use if tcnative is available!
@ -1962,18 +1958,6 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc
final String endPointIdentificationAlgorithm = sslParameters.getEndpointIdentificationAlgorithm();
final boolean endPointVerificationEnabled = isEndPointVerificationEnabled(endPointIdentificationAlgorithm);
final boolean wasEndPointVerificationEnabled =
isEndPointVerificationEnabled(this.endPointIdentificationAlgorithm);
if (wasEndPointVerificationEnabled && !endPointVerificationEnabled) {
// Passing in null will disable hostname verification again so only do so if it was enabled before.
SSL.setHostNameValidation(ssl, DEFAULT_HOSTNAME_VALIDATION_FLAGS, null);
} else {
String host = endPointVerificationEnabled ? getPeerHost() : null;
if (host != null && !host.isEmpty()) {
SSL.setHostNameValidation(ssl, DEFAULT_HOSTNAME_VALIDATION_FLAGS, host);
}
}
// If the user asks for hostname verification we must ensure we verify the peer.
// If the user disables hostname verification we leave it up to the user to change the mode manually.
if (clientMode && endPointVerificationEnabled) {

14
handler/src/test/java/io/netty/handler/ssl/ConscryptOpenSslEngineInteropTest.java
View File

@ -109,20 +109,6 @@ public class ConscryptOpenSslEngineInteropTest extends ConscryptSslEngineTest {
super.testMutualAuthInvalidIntermediateCAFailWithRequiredClientAuth();
}
@Override
@Test
public void testClientHostnameValidationSuccess() throws InterruptedException, SSLException {
assumeTrue(OpenSsl.supportsHostnameValidation());
super.testClientHostnameValidationSuccess();
}
@Override
@Test
public void testClientHostnameValidationFail() throws InterruptedException, SSLException {
assumeTrue(OpenSsl.supportsHostnameValidation());
super.testClientHostnameValidationFail();
}
@Override
@Test
public void testSessionAfterHandshakeKeyManagerFactoryMutualAuth() throws Exception {

15
handler/src/test/java/io/netty/handler/ssl/OpenSslConscryptSslEngineInteropTest.java
View File

@ -22,7 +22,6 @@ import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
import javax.net.ssl.SSLEngine;
import javax.net.ssl.SSLException;
import java.security.Provider;
import java.util.ArrayList;
import java.util.Collection;
@ -109,20 +108,6 @@ public class OpenSslConscryptSslEngineInteropTest extends ConscryptSslEngineTest
super.testMutualAuthInvalidIntermediateCAFailWithRequiredClientAuth();
}
@Override
@Test
public void testClientHostnameValidationSuccess() throws InterruptedException, SSLException {
assumeTrue(OpenSsl.supportsHostnameValidation());
super.testClientHostnameValidationSuccess();
}
@Override
@Test
public void testClientHostnameValidationFail() throws InterruptedException, SSLException {
assumeTrue(OpenSsl.supportsHostnameValidation());
super.testClientHostnameValidationFail();
}
@Override
@Test
public void testSessionAfterHandshakeKeyManagerFactoryMutualAuth() throws Exception {

14
handler/src/test/java/io/netty/handler/ssl/OpenSslEngineTest.java
View File

@ -147,20 +147,6 @@ public class OpenSslEngineTest extends SSLEngineTest {
super.testMutualAuthValidClientCertChainTooLongFailRequireClientAuth();
}
@Override
@Test
public void testClientHostnameValidationSuccess() throws InterruptedException, SSLException {
assumeTrue(OpenSsl.supportsHostnameValidation());
super.testClientHostnameValidationSuccess();
}
@Override
@Test
public void testClientHostnameValidationFail() throws InterruptedException, SSLException {
assumeTrue(OpenSsl.supportsHostnameValidation());
super.testClientHostnameValidationFail();
}
@Override
public void testHandshakeSession() throws Exception {
checkShouldUseKeyManagerFactory();

14
handler/src/test/java/io/netty/handler/ssl/OpenSslJdkSslEngineInteroptTest.java
View File

@ -100,20 +100,6 @@ public class OpenSslJdkSslEngineInteroptTest extends SSLEngineTest {
super.testMutualAuthInvalidIntermediateCAFailWithRequiredClientAuth();
}
@Override
@Test
public void testClientHostnameValidationSuccess() throws InterruptedException, SSLException {
assumeTrue(OpenSsl.supportsHostnameValidation());
super.testClientHostnameValidationSuccess();
}
@Override
@Test
public void testClientHostnameValidationFail() throws InterruptedException, SSLException {
assumeTrue(OpenSsl.supportsHostnameValidation());
super.testClientHostnameValidationFail();
}
@Override
@Test
public void testSessionAfterHandshakeKeyManagerFactoryMutualAuth() throws Exception {

5
handler/src/test/java/io/netty/handler/ssl/SSLEngineTest.java
View File

@ -2690,11 +2690,6 @@ public abstract class SSLEngineTest {
@Test
public void testUsingX509TrustManagerVerifiesHostname() throws Exception {
SslProvider clientProvider = sslClientProvider();
if (clientProvider == SslProvider.OPENSSL || clientProvider == SslProvider.OPENSSL_REFCNT) {
// Need to check if we support hostname validation in the current used OpenSSL version before running
// the test.
Assume.assumeTrue(OpenSsl.supportsHostnameValidation());
}
SelfSignedCertificate cert = new SelfSignedCertificate();
clientSslCtx = SslContextBuilder
.forClient()