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

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 committed by GitHub
parent a2b85a306d
commit f8c89e2e05
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 7 additions and 92 deletions

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_OPENSSL_CIPHER_SUITES;
private static final Set<String> AVAILABLE_JAVA_CIPHER_SUITES; private static final Set<String> AVAILABLE_JAVA_CIPHER_SUITES;
private static final boolean SUPPORTS_KEYMANAGER_FACTORY; 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 USE_KEYMANAGER_FACTORY;
private static final boolean SUPPORTS_OCSP; private static final boolean SUPPORTS_OCSP;
private static final boolean TLSV13_SUPPORTED; private static final boolean TLSV13_SUPPORTED;
@ -200,7 +199,6 @@ public final class OpenSsl {
final Set<String> availableOpenSslCipherSuites = new LinkedHashSet<String>(128); final Set<String> availableOpenSslCipherSuites = new LinkedHashSet<String>(128);
boolean supportsKeyManagerFactory = false; boolean supportsKeyManagerFactory = false;
boolean useKeyManagerFactory = false; boolean useKeyManagerFactory = false;
boolean supportsHostNameValidation = false;
boolean tlsv13Supported = false; boolean tlsv13Supported = false;
IS_BORINGSSL = "BoringSSL".equals(versionString()); IS_BORINGSSL = "BoringSSL".equals(versionString());
@ -257,12 +255,6 @@ public final class OpenSsl {
"AEAD-AES256-GCM-SHA384", "AEAD-AES256-GCM-SHA384",
"AEAD-CHACHA20-POLY1305-SHA256"); "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); PemEncoded privateKey = PemPrivateKey.toPEM(UnpooledByteBufAllocator.DEFAULT, true, KEY_BYTES);
try { try {
@ -342,7 +334,6 @@ public final class OpenSsl {
AVAILABLE_CIPHER_SUITES = availableCipherSuites; AVAILABLE_CIPHER_SUITES = availableCipherSuites;
SUPPORTS_KEYMANAGER_FACTORY = supportsKeyManagerFactory; SUPPORTS_KEYMANAGER_FACTORY = supportsKeyManagerFactory;
SUPPORTS_HOSTNAME_VALIDATION = supportsHostNameValidation;
USE_KEYMANAGER_FACTORY = useKeyManagerFactory; USE_KEYMANAGER_FACTORY = useKeyManagerFactory;
Set<String> protocols = new LinkedHashSet<String>(6); Set<String> protocols = new LinkedHashSet<String>(6);
@ -385,7 +376,6 @@ public final class OpenSsl {
AVAILABLE_JAVA_CIPHER_SUITES = Collections.emptySet(); AVAILABLE_JAVA_CIPHER_SUITES = Collections.emptySet();
AVAILABLE_CIPHER_SUITES = Collections.emptySet(); AVAILABLE_CIPHER_SUITES = Collections.emptySet();
SUPPORTS_KEYMANAGER_FACTORY = false; SUPPORTS_KEYMANAGER_FACTORY = false;
SUPPORTS_HOSTNAME_VALIDATION = false;
USE_KEYMANAGER_FACTORY = false; USE_KEYMANAGER_FACTORY = false;
SUPPORTED_PROTOCOLS_SET = Collections.emptySet(); SUPPORTED_PROTOCOLS_SET = Collections.emptySet();
SUPPORTS_OCSP = false; SUPPORTS_OCSP = false;
@ -544,11 +534,14 @@ public final class OpenSsl {
} }
/** /**
* Returns {@code true} if <a href="https://wiki.openssl.org/index.php/Hostname_validation">Hostname Validation</a> * Always returns {@code true} if {@link #isAvailable()} returns {@code true}.
* is supported when using OpenSSL. *
* @deprecated Will be removed because hostname validation is always done by a
* {@link javax.net.ssl.TrustManager} implementation.
*/ */
@Deprecated
public static boolean supportsHostnameValidation() { public static boolean supportsHostnameValidation() {
return SUPPORTS_HOSTNAME_VALIDATION; return isAvailable();
} }
static boolean useKeyManagerFactory() { static boolean useKeyManagerFactory() {

View File

@ -120,10 +120,6 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc
SSL.SSL_OP_NO_TLSv1_2, SSL.SSL_OP_NO_TLSv1_2,
SSL.SSL_OP_NO_TLSv1_3 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! * Depends upon tcnative ... only use if tcnative is available!
@ -1961,18 +1957,6 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc
final String endPointIdentificationAlgorithm = sslParameters.getEndpointIdentificationAlgorithm(); final String endPointIdentificationAlgorithm = sslParameters.getEndpointIdentificationAlgorithm();
final boolean endPointVerificationEnabled = isEndPointVerificationEnabled(endPointIdentificationAlgorithm); 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 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 the user disables hostname verification we leave it up to the user to change the mode manually.
if (clientMode && endPointVerificationEnabled) { if (clientMode && endPointVerificationEnabled) {

View File

@ -109,20 +109,6 @@ public class ConscryptOpenSslEngineInteropTest extends ConscryptSslEngineTest {
super.testMutualAuthInvalidIntermediateCAFailWithRequiredClientAuth(); 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 @Override
@Test @Test
public void testSessionAfterHandshakeKeyManagerFactoryMutualAuth() throws Exception { public void testSessionAfterHandshakeKeyManagerFactoryMutualAuth() throws Exception {

View File

@ -22,7 +22,6 @@ import org.junit.runner.RunWith;
import org.junit.runners.Parameterized; import org.junit.runners.Parameterized;
import javax.net.ssl.SSLEngine; import javax.net.ssl.SSLEngine;
import javax.net.ssl.SSLException;
import java.security.Provider; import java.security.Provider;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Collection; import java.util.Collection;
@ -109,20 +108,6 @@ public class OpenSslConscryptSslEngineInteropTest extends ConscryptSslEngineTest
super.testMutualAuthInvalidIntermediateCAFailWithRequiredClientAuth(); 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 @Override
@Test @Test
public void testSessionAfterHandshakeKeyManagerFactoryMutualAuth() throws Exception { public void testSessionAfterHandshakeKeyManagerFactoryMutualAuth() throws Exception {

View File

@ -146,20 +146,6 @@ public class OpenSslEngineTest extends SSLEngineTest {
super.testMutualAuthValidClientCertChainTooLongFailRequireClientAuth(); 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 @Override
public void testHandshakeSession() throws Exception { public void testHandshakeSession() throws Exception {
checkShouldUseKeyManagerFactory(); checkShouldUseKeyManagerFactory();

View File

@ -100,20 +100,6 @@ public class OpenSslJdkSslEngineInteroptTest extends SSLEngineTest {
super.testMutualAuthInvalidIntermediateCAFailWithRequiredClientAuth(); 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 @Override
@Test @Test
public void testSessionAfterHandshakeKeyManagerFactoryMutualAuth() throws Exception { public void testSessionAfterHandshakeKeyManagerFactoryMutualAuth() throws Exception {

View File

@ -2678,11 +2678,6 @@ public abstract class SSLEngineTest {
@Test @Test
public void testUsingX509TrustManagerVerifiesHostname() throws Exception { public void testUsingX509TrustManagerVerifiesHostname() throws Exception {
SslProvider clientProvider = sslClientProvider(); 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(); SelfSignedCertificate cert = new SelfSignedCertificate();
clientSslCtx = SslContextBuilder clientSslCtx = SslContextBuilder
.forClient() .forClient()