Override hostname verification in PeerAuthorizerTrustManager

Override hostname verification on client-side.
Remove overriding of hostname verification for server-side.

5 files changed, 59 insertions, 29 deletions

diff --git a/security-utils/src/main/java/com/yahoo/security/tls/ConfigFileBasedTlsContext.java b/security-utils/src/main/java/com/yahoo/security/tls/ConfigFileBasedTlsContext.java
index 6a78e49fe1d..28854c59b2c 100644
--- a/security-utils/src/main/java/com/yahoo/security/tls/ConfigFileBasedTlsContext.java
+++ b/security-utils/src/main/java/com/yahoo/security/tls/ConfigFileBasedTlsContext.java
@@ -12,7 +12,6 @@ import com.yahoo.security.tls.policy.AuthorizedPeers;
 import javax.net.ssl.SSLContext;
 import javax.net.ssl.SSLEngine;
 import javax.net.ssl.SSLParameters;
-import javax.net.ssl.X509ExtendedTrustManager;
 import java.io.IOException;
 import java.io.UncheckedIOException;
 import java.lang.ref.WeakReference;
@@ -110,9 +109,10 @@ public class ConfigFileBasedTlsContext implements TlsContext {
                                                              MutableX509KeyManager mutableKeyManager,
                                                              PeerAuthentication peerAuthentication) {
 
+        HostnameVerification hostnameVerification = options.isHostnameValidationDisabled() ? HostnameVerification.DISABLED : HostnameVerification.ENABLED;
         PeerAuthorizerTrustManager authorizerTrustManager = options.getAuthorizedPeers()
-                .map(authorizedPeers -> new PeerAuthorizerTrustManager(authorizedPeers, mode, mutableTrustManager))
-                .orElseGet(() -> new PeerAuthorizerTrustManager(new AuthorizedPeers(com.yahoo.vespa.jdk8compat.Set.of()), AuthorizationMode.DISABLE, mutableTrustManager)));
+                .map(authorizedPeers -> new PeerAuthorizerTrustManager(authorizedPeers, mode, hostnameVerification, mutableTrustManager))
+                .orElseGet(() -> new PeerAuthorizerTrustManager(new AuthorizedPeers(com.yahoo.vespa.jdk8compat.Set.of()), AuthorizationMode.DISABLE, hostnameVerification, mutableTrustManager));
         SSLContext sslContext = new SslContextBuilder()
                 .withKeyManager(mutableKeyManager)
                 .withTrustManager(authorizerTrustManager)
diff --git a/security-utils/src/main/java/com/yahoo/security/tls/DefaultTlsContext.java b/security-utils/src/main/java/com/yahoo/security/tls/DefaultTlsContext.java
index c3f10a464a5..def3e49be4d 100644
--- a/security-utils/src/main/java/com/yahoo/security/tls/DefaultTlsContext.java
+++ b/security-utils/src/main/java/com/yahoo/security/tls/DefaultTlsContext.java
@@ -34,8 +34,9 @@ public class DefaultTlsContext implements TlsContext {
                              List<X509Certificate> caCertificates,
                              AuthorizedPeers authorizedPeers,
                              AuthorizationMode mode,
-                             PeerAuthentication peerAuthentication) {
-        this(createSslContext(certificates, privateKey, caCertificates, authorizedPeers, mode), peerAuthentication);
+                             PeerAuthentication peerAuthentication,
+                             HostnameVerification hostnameVerification) {
+        this(createSslContext(certificates, privateKey, caCertificates, authorizedPeers, mode, hostnameVerification), peerAuthentication);
     }
 
     public DefaultTlsContext(SSLContext sslContext, PeerAuthentication peerAuthentication) {
@@ -120,7 +121,8 @@ public class DefaultTlsContext implements TlsContext {
                                                PrivateKey privateKey,
                                                List<X509Certificate> caCertificates,
                                                AuthorizedPeers authorizedPeers,
-                                               AuthorizationMode mode) {
+                                               AuthorizationMode mode,
+                                               HostnameVerification hostnameVerification) {
         SslContextBuilder builder = new SslContextBuilder();
         if (!certificates.isEmpty()) {
             builder.withKeyStore(privateKey, certificates);
@@ -129,12 +131,12 @@ public class DefaultTlsContext implements TlsContext {
             builder.withTrustStore(caCertificates);
         }
         if (authorizedPeers != null) {
-            builder.withTrustManagerFactory(truststore -> new PeerAuthorizerTrustManager(authorizedPeers, mode, truststore));
+            builder.withTrustManagerFactory(truststore -> new PeerAuthorizerTrustManager(authorizedPeers, mode, hostnameVerification, truststore));
         } else {
-            builder.withTrustManagerFactory(truststore -> new PeerAuthorizerTrustManager(new AuthorizedPeers(com.yahoo.vespa.jdk8compat.Set.of()), AuthorizationMode.DISABLE, truststore));
+            builder.withTrustManagerFactory(truststore -> new PeerAuthorizerTrustManager(
+                    new AuthorizedPeers(com.yahoo.vespa.jdk8compat.Set.of()), AuthorizationMode.DISABLE, hostnameVerification, truststore));
         }
         return builder.build();
     }
 
-
 }
diff --git a/security-utils/src/main/java/com/yahoo/security/tls/HostnameVerification.java b/security-utils/src/main/java/com/yahoo/security/tls/HostnameVerification.java
new file mode 100644
index 00000000000..a41edc6dc44
--- /dev/null
+++ b/security-utils/src/main/java/com/yahoo/security/tls/HostnameVerification.java
@@ -0,0 +1,7 @@
+// Copyright 2020 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.security.tls;
+
+/**
+ * @author bjorncs
+ */
+public enum HostnameVerification { ENABLED, DISABLED }
diff --git a/security-utils/src/main/java/com/yahoo/security/tls/authz/PeerAuthorizerTrustManager.java b/security-utils/src/main/java/com/yahoo/security/tls/authz/PeerAuthorizerTrustManager.java
index 3ddd0861f39..03358190e8a 100644
--- a/security-utils/src/main/java/com/yahoo/security/tls/authz/PeerAuthorizerTrustManager.java
+++ b/security-utils/src/main/java/com/yahoo/security/tls/authz/PeerAuthorizerTrustManager.java
@@ -3,6 +3,7 @@ package com.yahoo.security.tls.authz;
 
 import com.yahoo.security.X509CertificateUtils;
 import com.yahoo.security.tls.AuthorizationMode;
+import com.yahoo.security.tls.HostnameVerification;
 import com.yahoo.security.tls.TrustManagerUtils;
 import com.yahoo.security.tls.policy.AuthorizedPeers;
 
@@ -14,7 +15,6 @@ import java.net.Socket;
 import java.security.KeyStore;
 import java.security.cert.CertificateException;
 import java.security.cert.X509Certificate;
-import java.util.Objects;
 import java.util.Optional;
 import java.util.logging.Logger;
 
@@ -33,15 +33,23 @@ public class PeerAuthorizerTrustManager extends X509ExtendedTrustManager {
     private final PeerAuthorizer authorizer;
     private final X509ExtendedTrustManager defaultTrustManager;
     private final AuthorizationMode mode;
+    private final HostnameVerification hostnameVerification;
 
-    public PeerAuthorizerTrustManager(AuthorizedPeers authorizedPeers, AuthorizationMode mode, X509ExtendedTrustManager defaultTrustManager) {
+    public PeerAuthorizerTrustManager(AuthorizedPeers authorizedPeers,
+                                      AuthorizationMode mode,
+                                      HostnameVerification hostnameVerification,
+                                      X509ExtendedTrustManager defaultTrustManager) {
         this.authorizer = new PeerAuthorizer(authorizedPeers);
         this.mode = mode;
+        this.hostnameVerification = hostnameVerification;
         this.defaultTrustManager = defaultTrustManager;
     }
 
-    public PeerAuthorizerTrustManager(AuthorizedPeers authorizedPeers, AuthorizationMode mode, KeyStore truststore) {
-        this(authorizedPeers, mode, TrustManagerUtils.createDefaultX509TrustManager(truststore));
+    public PeerAuthorizerTrustManager(AuthorizedPeers authorizedPeers,
+                                      AuthorizationMode mode,
+                                      HostnameVerification hostnameVerification,
+                                      KeyStore truststore) {
+        this(authorizedPeers, mode, hostnameVerification, TrustManagerUtils.createDefaultX509TrustManager(truststore));
     }
 
     @Override
@@ -58,28 +66,26 @@ public class PeerAuthorizerTrustManager extends X509ExtendedTrustManager {
 
     @Override
     public void checkClientTrusted(X509Certificate[] chain, String authType, Socket socket) throws CertificateException {
-        overrideHostnameVerification(socket);
         defaultTrustManager.checkClientTrusted(chain, authType, socket);
         authorizePeer(chain[0], authType, true, null);
     }
 
     @Override
     public void checkServerTrusted(X509Certificate[] chain, String authType, Socket socket) throws CertificateException {
-        overrideHostnameVerification(socket);
+        overrideHostnameVerificationForClient(socket);
         defaultTrustManager.checkServerTrusted(chain, authType, socket);
         authorizePeer(chain[0], authType, false, null);
     }
 
     @Override
     public void checkClientTrusted(X509Certificate[] chain, String authType, SSLEngine sslEngine) throws CertificateException {
-        overrideHostnameVerification(sslEngine);
         defaultTrustManager.checkClientTrusted(chain, authType, sslEngine);
         authorizePeer(chain[0], authType, true, sslEngine);
     }
 
     @Override
     public void checkServerTrusted(X509Certificate[] chain, String authType, SSLEngine sslEngine) throws CertificateException {
-        overrideHostnameVerification(sslEngine);
+        overrideHostnameVerificationForClient(sslEngine);
         defaultTrustManager.checkServerTrusted(chain, authType, sslEngine);
         authorizePeer(chain[0], authType, false, sslEngine);
     }
@@ -121,31 +127,44 @@ public class PeerAuthorizerTrustManager extends X509ExtendedTrustManager {
                              certificate.getSubjectX500Principal(), X509CertificateUtils.getSubjectAlternativeNames(certificate), authType, isVerifyingClient);
     }
 
-    private static void overrideHostnameVerification(SSLEngine engine) {
+    private void overrideHostnameVerificationForClient(SSLEngine engine) {
         SSLParameters params = engine.getSSLParameters();
-        if (overrideHostnameVerification(params)) {
+        if (overrideHostnameVerificationForClient(params)) {
             engine.setSSLParameters(params);
         }
     }
 
-    private static void overrideHostnameVerification(Socket socket) {
+    private void overrideHostnameVerificationForClient(Socket socket) {
         if (socket instanceof SSLSocket) {
             SSLSocket sslSocket = (SSLSocket) socket;
             SSLParameters params = sslSocket.getSSLParameters();
-            if (overrideHostnameVerification(params)) {
+            if (overrideHostnameVerificationForClient(params)) {
                 sslSocket.setSSLParameters(params);
             }
         }
     }
 
-    // Disable the default hostname verification that is performed by underlying trust manager when 'HTTPS' is used as endpoint identification algorithm.
-    // Some http clients, notably the new http client in Java 11, does not allow user configuration of the endpoint algorithm or custom HostnameVerifier.
-    private static boolean overrideHostnameVerification(SSLParameters params) {
-        if (Objects.equals("HTTPS", params.getEndpointIdentificationAlgorithm())) {
-            params.setEndpointIdentificationAlgorithm("");
-            return true;
+    // Overrides the endpoint identification algorithm specified in the ssl parameters of the ssl engine/socket.
+    // The underlying trust manager will perform hostname verification if endpoint identification algorithm is set to 'HTTPS'.
+    // Returns true if the parameter instance was modified
+    private boolean overrideHostnameVerificationForClient(SSLParameters params) {
+        String configuredAlgorithm = params.getEndpointIdentificationAlgorithm();
+        switch (hostnameVerification) {
+            case ENABLED:
+                if (!"HTTPS".equals(configuredAlgorithm)) {
+                    params.setEndpointIdentificationAlgorithm("HTTPS");
+                    return true;
+                }
+                return false;
+            case DISABLED:
+                if (configuredAlgorithm != null && !configuredAlgorithm.isEmpty()) {
+                    params.setEndpointIdentificationAlgorithm(""); // disable any configured endpoint identification algorithm
+                    return true;
+                }
+                return false;
+            default:
+                throw new IllegalStateException("Unknown host verification type: " + hostnameVerification);
         }
-        return false;
     }
 
 }
diff --git a/security-utils/src/test/java/com/yahoo/security/tls/DefaultTlsContextTest.java b/security-utils/src/test/java/com/yahoo/security/tls/DefaultTlsContextTest.java
index 727a64ae934..00928187f55 100644
--- a/security-utils/src/test/java/com/yahoo/security/tls/DefaultTlsContextTest.java
+++ b/security-utils/src/test/java/com/yahoo/security/tls/DefaultTlsContextTest.java
@@ -46,7 +46,9 @@ public class DefaultTlsContextTest {
                                 singletonList(new RequiredPeerCredential(RequiredPeerCredential.Field.CN, new HostGlobPattern("dummy"))))));
 
         DefaultTlsContext tlsContext =
-                new DefaultTlsContext(singletonList(certificate), keyPair.getPrivate(), singletonList(certificate), authorizedPeers, AuthorizationMode.ENFORCE, PeerAuthentication.NEED);
+                new DefaultTlsContext(
+                        singletonList(certificate), keyPair.getPrivate(), singletonList(certificate), authorizedPeers,
+                        AuthorizationMode.ENFORCE, PeerAuthentication.NEED, HostnameVerification.ENABLED);
 
         SSLEngine sslEngine = tlsContext.createSslEngine();
         assertThat(sslEngine).isNotNull();