Merge pull request #4466 from vespa-engine/bjorncs/athenz-hostname-verifier

Bjorncs/athenz hostname verifier

7 files changed, 181 insertions, 42 deletions

diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/athenz/AthenzIdentityVerifier.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/athenz/AthenzIdentityVerifier.java
index 9eacbb48ddc..bfaa6c2acda 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/athenz/AthenzIdentityVerifier.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/athenz/AthenzIdentityVerifier.java
@@ -1,8 +1,6 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.controller.api.integration.athenz;
 
-import javax.naming.NamingException;
-import javax.naming.ldap.LdapName;
 import javax.net.ssl.HostnameVerifier;
 import javax.net.ssl.SSLPeerUnverifiedException;
 import javax.net.ssl.SSLSession;
@@ -31,7 +29,7 @@ public class AthenzIdentityVerifier implements HostnameVerifier {
     public boolean verify(String hostname, SSLSession session) {
         try {
             X509Certificate cert = (X509Certificate) session.getPeerCertificates()[0];
-            AthenzIdentity certificateIdentity = AthenzUtils.createAthenzIdentity(getCommonName(cert));
+            AthenzIdentity certificateIdentity = AthenzUtils.createAthenzIdentity(cert);
             return allowedIdentities.contains(certificateIdentity);
         } catch (SSLPeerUnverifiedException e) {
             log.log(Level.WARNING, "Unverified client: " + hostname);
@@ -39,17 +37,5 @@ public class AthenzIdentityVerifier implements HostnameVerifier {
         }
     }
 
-    private static String getCommonName(X509Certificate certificate) {
-        try {
-            String subjectPrincipal = certificate.getSubjectX500Principal().getName();
-            return new LdapName(subjectPrincipal).getRdns().stream()
-                    .filter(rdn -> rdn.getType().equalsIgnoreCase("cn"))
-                    .map(rdn -> rdn.getValue().toString())
-                    .findFirst()
-                    .orElseThrow(() -> new IllegalArgumentException("Could not find CN in certificate: " + subjectPrincipal));
-        } catch (NamingException e) {
-            throw new IllegalArgumentException("Invalid CN: " + e, e);
-        }
-    }
 }
 
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/athenz/AthenzPrincipal.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/athenz/AthenzPrincipal.java
index 8279edcd8e6..b31cb4a26bb 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/athenz/AthenzPrincipal.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/athenz/AthenzPrincipal.java
@@ -5,6 +5,7 @@ import com.yahoo.vespa.hosted.controller.api.identifiers.AthenzDomain;
 
 import java.security.Principal;
 import java.util.Objects;
+import java.util.Optional;
 
 /**
  * @author bjorncs
@@ -14,6 +15,10 @@ public class AthenzPrincipal implements Principal {
     private final AthenzIdentity athenzIdentity;
     private final NToken nToken;
 
+    public AthenzPrincipal(AthenzIdentity athenzIdentity) {
+        this(athenzIdentity, null);
+    }
+
     public AthenzPrincipal(AthenzIdentity athenzIdentity,
                            NToken nToken) {
         this.athenzIdentity = athenzIdentity;
@@ -33,8 +38,8 @@ public class AthenzPrincipal implements Principal {
         return athenzIdentity.getDomain();
     }
 
-    public NToken getNToken() {
-        return nToken;
+    public Optional<NToken> getNToken() {
+        return Optional.ofNullable(nToken);
     }
 
     @Override
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/athenz/AthenzUtils.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/athenz/AthenzUtils.java
index 62a7049a7c6..04ec0b61614 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/athenz/AthenzUtils.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/athenz/AthenzUtils.java
@@ -4,6 +4,10 @@ package com.yahoo.vespa.hosted.controller.api.integration.athenz;
 import com.yahoo.vespa.hosted.controller.api.identifiers.AthenzDomain;
 import com.yahoo.vespa.hosted.controller.api.identifiers.UserId;
 
+import javax.naming.NamingException;
+import javax.naming.ldap.LdapName;
+import java.security.cert.X509Certificate;
+
 /**
  * @author bjorncs
  */
@@ -35,4 +39,28 @@ public class AthenzUtils {
         return createAthenzIdentity(domain, identityName);
     }
 
+    public static AthenzIdentity createAthenzIdentity(X509Certificate certificate) {
+        String commonName = getCommonName(certificate);
+        if (isAthenzRoleIdentity(commonName)) {
+            throw new IllegalArgumentException("Athenz role certificate not supported");
+        }
+        return createAthenzIdentity(commonName);
+    }
+
+    private static boolean isAthenzRoleIdentity(String commonName) {
+        return commonName.contains(":role.");
+    }
+
+    private static String getCommonName(X509Certificate certificate) {
+        try {
+            String subjectPrincipal = certificate.getSubjectX500Principal().getName();
+            return new LdapName(subjectPrincipal).getRdns().stream()
+                    .filter(rdn -> rdn.getType().equalsIgnoreCase("cn"))
+                    .map(rdn -> rdn.getValue().toString())
+                    .findFirst()
+                    .orElseThrow(() -> new IllegalArgumentException("Could not find CN in certificate: " + subjectPrincipal));
+        } catch (NamingException e) {
+            throw new IllegalArgumentException("Invalid CN: " + e, e);
+        }
+    }
 }
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/athenz/InvalidTokenException.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/athenz/InvalidTokenException.java
index 1df1746b02e..967af1c553f 100644
--- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/athenz/InvalidTokenException.java
+++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/athenz/InvalidTokenException.java
@@ -4,7 +4,7 @@ package com.yahoo.vespa.hosted.controller.api.integration.athenz;
 /**
  * @author bjorncs
  */
-public class InvalidTokenException extends Exception {
+public class InvalidTokenException extends RuntimeException {
     public InvalidTokenException(String message) {
         super(message);
     }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/filter/AthenzPrincipalFilter.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/filter/AthenzPrincipalFilter.java
index 328461355db..7aaaad534db 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/filter/AthenzPrincipalFilter.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/athenz/filter/AthenzPrincipalFilter.java
@@ -7,17 +7,24 @@ import com.yahoo.jdisc.handler.ResponseHandler;
 import com.yahoo.jdisc.http.filter.DiscFilterRequest;
 import com.yahoo.jdisc.http.filter.SecurityRequestFilter;
 import com.yahoo.vespa.hosted.controller.api.integration.athenz.AthenzPrincipal;
-import com.yahoo.vespa.hosted.controller.api.integration.athenz.InvalidTokenException;
+import com.yahoo.vespa.hosted.controller.api.integration.athenz.AthenzUtils;
 import com.yahoo.vespa.hosted.controller.api.integration.athenz.NToken;
 import com.yahoo.vespa.hosted.controller.api.integration.athenz.ZmsKeystore;
 import com.yahoo.vespa.hosted.controller.athenz.config.AthenzConfig;
 
+import java.security.cert.X509Certificate;
+import java.util.Optional;
 import java.util.concurrent.Executor;
 
 import static com.yahoo.vespa.hosted.controller.athenz.filter.SecurityFilterUtils.sendErrorResponse;
 
 /**
- * Performs authentication by validating the principal token (NToken) header.
+ * Authenticates Athenz principal, either through:
+ *  1. TLS client authentication (based on Athenz x509 identity certficiate).
+ *  2. The principal token (NToken) header.
+ * The TLS authentication is based on the following assumptions:
+ *  - The underlying connector is configured with 'clientAuth' set to either WANT_AUTH or NEED_AUTH.
+ *  - The trust store is configured with the Athenz CA certificates only.
  *
  * @author bjorncs
  */
@@ -43,18 +50,45 @@ public class AthenzPrincipalFilter implements SecurityRequestFilter {
 
     @Override
     public void filter(DiscFilterRequest request, ResponseHandler responseHandler) {
-        String rawToken = request.getHeader(principalTokenHeader);
-        if (rawToken == null || rawToken.isEmpty()) {
-            sendErrorResponse(responseHandler, Response.Status.UNAUTHORIZED, "NToken is missing");
-            return;
-        }
         try {
-            AthenzPrincipal principal = validator.validate(new NToken(rawToken));
+            Optional<AthenzPrincipal> certificatePrincipal = getClientCertificate(request)
+                    .map(AthenzUtils::createAthenzIdentity)
+                    .map(AthenzPrincipal::new);
+            Optional<AthenzPrincipal> nTokenPrincipal = getPrincipalToken(request, principalTokenHeader)
+                    .map(validator::validate);
+
+            if (!certificatePrincipal.isPresent() && !nTokenPrincipal.isPresent()) {
+                String errorMessage = "Unable to authenticate Athenz identity. " +
+                        "Both client certificate missing and principal token header are missing.";
+                sendErrorResponse(responseHandler, Response.Status.UNAUTHORIZED, errorMessage);
+                return;
+            }
+            if (certificatePrincipal.isPresent() && nTokenPrincipal.isPresent()
+                    && !certificatePrincipal.get().getIdentity().equals(nTokenPrincipal.get().getIdentity())) {
+                String errorMessage = String.format(
+                        "Identity in principal token does not match x509 CN: token-identity=%s, cert-identity=%s",
+                        nTokenPrincipal.get().getIdentity().getFullName(),
+                        certificatePrincipal.get().getIdentity().getFullName());
+                sendErrorResponse(responseHandler, Response.Status.UNAUTHORIZED, errorMessage);
+                return;
+            }
+            AthenzPrincipal principal = nTokenPrincipal.orElseGet(certificatePrincipal::get);
             request.setUserPrincipal(principal);
             request.setRemoteUser(principal.getName());
-        } catch (InvalidTokenException e) {
+        } catch (Exception e) {
             sendErrorResponse(responseHandler,Response.Status.UNAUTHORIZED, e.getMessage());
         }
     }
 
+    private static Optional<X509Certificate> getClientCertificate(DiscFilterRequest request) {
+        return Optional.ofNullable((X509Certificate[]) request.getAttribute("jdisc.request.X509Certificate"))
+                .map(chain -> chain[0]);
+    }
+
+    private static Optional<NToken> getPrincipalToken(DiscFilterRequest request, String principalTokenHeaderName) {
+        return Optional.ofNullable(request.getHeader(principalTokenHeaderName))
+                .filter(token -> !token.isEmpty())
+                .map(NToken::new);
+    }
+
 }
diff --git a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/Authorizer.java b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/Authorizer.java
index b7080a763f0..77ce49eaf47 100644
--- a/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/Authorizer.java
+++ b/controller-server/src/main/java/com/yahoo/vespa/hosted/controller/restapi/application/Authorizer.java
@@ -9,14 +9,13 @@ import com.yahoo.vespa.hosted.controller.api.identifiers.AthenzDomain;
 import com.yahoo.vespa.hosted.controller.api.identifiers.TenantId;
 import com.yahoo.vespa.hosted.controller.api.identifiers.UserGroup;
 import com.yahoo.vespa.hosted.controller.api.identifiers.UserId;
-import com.yahoo.vespa.hosted.controller.api.integration.entity.EntityService;
 import com.yahoo.vespa.hosted.controller.api.integration.athenz.AthenzClientFactory;
 import com.yahoo.vespa.hosted.controller.api.integration.athenz.AthenzIdentity;
 import com.yahoo.vespa.hosted.controller.api.integration.athenz.AthenzPrincipal;
 import com.yahoo.vespa.hosted.controller.api.integration.athenz.AthenzUser;
 import com.yahoo.vespa.hosted.controller.api.integration.athenz.NToken;
+import com.yahoo.vespa.hosted.controller.api.integration.entity.EntityService;
 import com.yahoo.vespa.hosted.controller.common.ContextAttributes;
-import com.yahoo.vespa.hosted.controller.restapi.filter.NTokenRequestFilter;
 
 import javax.ws.rs.ForbiddenException;
 import javax.ws.rs.HttpMethod;
@@ -78,8 +77,7 @@ public class Authorizer {
     }
 
     public Optional<NToken> getNToken(HttpRequest request) {
-        String nTokenHeader = (String)request.getJDiscRequest().context().get(NTokenRequestFilter.NTOKEN_HEADER);
-        return Optional.ofNullable(nTokenHeader).map(NToken::new);
+        return getPrincipalIfAny(request).flatMap(AthenzPrincipal::getNToken);
     }
 
     public boolean isSuperUser(HttpRequest request) {
diff --git a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/athenz/filter/AthenzPrincipalFilterTest.java b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/athenz/filter/AthenzPrincipalFilterTest.java
index ffb78b7342a..c887fbfc1a8 100644
--- a/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/athenz/filter/AthenzPrincipalFilterTest.java
+++ b/controller-server/src/test/java/com/yahoo/vespa/hosted/controller/athenz/filter/AthenzPrincipalFilterTest.java
@@ -7,10 +7,19 @@ import com.yahoo.jdisc.handler.ReadableContentChannel;
 import com.yahoo.jdisc.handler.ResponseHandler;
 import com.yahoo.jdisc.http.filter.DiscFilterRequest;
 import com.yahoo.vespa.hosted.controller.api.identifiers.UserId;
+import com.yahoo.vespa.hosted.controller.api.integration.athenz.AthenzIdentity;
 import com.yahoo.vespa.hosted.controller.api.integration.athenz.AthenzPrincipal;
 import com.yahoo.vespa.hosted.controller.api.integration.athenz.AthenzUser;
 import com.yahoo.vespa.hosted.controller.api.integration.athenz.InvalidTokenException;
 import com.yahoo.vespa.hosted.controller.api.integration.athenz.NToken;
+import org.bouncycastle.asn1.x500.X500Name;
+import org.bouncycastle.cert.X509v3CertificateBuilder;
+import org.bouncycastle.cert.jcajce.JcaX509CertificateConverter;
+import org.bouncycastle.cert.jcajce.JcaX509v3CertificateBuilder;
+import org.bouncycastle.jce.provider.BouncyCastleProvider;
+import org.bouncycastle.operator.ContentSigner;
+import org.bouncycastle.operator.OperatorCreationException;
+import org.bouncycastle.operator.jcajce.JcaContentSignerBuilder;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -18,6 +27,15 @@ import java.io.BufferedReader;
 import java.io.IOException;
 import java.io.InputStreamReader;
 import java.io.UncheckedIOException;
+import java.math.BigInteger;
+import java.security.KeyPair;
+import java.security.KeyPairGenerator;
+import java.security.NoSuchAlgorithmException;
+import java.security.cert.CertificateException;
+import java.security.cert.X509Certificate;
+import java.time.Duration;
+import java.time.Instant;
+import java.util.Date;
 import java.util.Objects;
 
 import static com.yahoo.jdisc.Response.Status.UNAUTHORIZED;
@@ -37,21 +55,21 @@ public class AthenzPrincipalFilterTest {
 
     private static final NToken NTOKEN = new NToken("dummy");
     private static final String ATHENZ_PRINCIPAL_HEADER = "Athenz-Principal-Auth";
+    private static final AthenzIdentity IDENTITY = AthenzUser.fromUserId(new UserId("bob"));
+    private static final X509Certificate CERTIFICATE = createSelfSignedCertificate(IDENTITY);
 
     private NTokenValidator validator;
-    private AthenzPrincipal principal;
 
     @Before
     public void before() {
         validator = mock(NTokenValidator.class);
-        principal = new AthenzPrincipal(AthenzUser.fromUserId(new UserId("bob")), NTOKEN);
     }
 
     @Test
-    public void valid_ntoken_is_accepted() throws Exception {
+    public void valid_ntoken_is_accepted() {
         DiscFilterRequest request = mock(DiscFilterRequest.class);
+        AthenzPrincipal principal = new AthenzPrincipal(IDENTITY, NTOKEN);
         when(request.getHeader(ATHENZ_PRINCIPAL_HEADER)).thenReturn(NTOKEN.getRawToken());
-
         when(validator.validate(NTOKEN)).thenReturn(principal);
 
         AthenzPrincipalFilter filter = new AthenzPrincipalFilter(validator, Runnable::run, ATHENZ_PRINCIPAL_HEADER);
@@ -61,7 +79,7 @@ public class AthenzPrincipalFilterTest {
     }
 
     @Test
-    public void missing_token_is_unauthorized() throws Exception {
+    public void missing_token_and_certificate_is_unauthorized() {
         DiscFilterRequest request = mock(DiscFilterRequest.class);
         when(request.getHeader(ATHENZ_PRINCIPAL_HEADER)).thenReturn(null);
 
@@ -70,26 +88,76 @@ public class AthenzPrincipalFilterTest {
         AthenzPrincipalFilter filter = new AthenzPrincipalFilter(validator, Runnable::run, ATHENZ_PRINCIPAL_HEADER);
         filter.filter(request, responseHandler);
 
-        assertThat(responseHandler.response, notNullValue());
-        assertThat(responseHandler.response.getStatus(), equalTo(UNAUTHORIZED));
-        assertThat(responseHandler.getResponseContent(), containsString("NToken is missing"));
+        assertUnauthorized(responseHandler, "Unable to authenticate Athenz identity");
+    }
+
+    @Test
+    public void invalid_token_is_unauthorized() {
+        DiscFilterRequest request = mock(DiscFilterRequest.class);
+        String errorMessage = "Invalid token";
+        when(request.getHeader(ATHENZ_PRINCIPAL_HEADER)).thenReturn(NTOKEN.getRawToken());
+        when(validator.validate(NTOKEN)).thenThrow(new InvalidTokenException(errorMessage));
+
+        ResponseHandlerMock responseHandler = new ResponseHandlerMock();
+
+        AthenzPrincipalFilter filter = new AthenzPrincipalFilter(validator, Runnable::run, ATHENZ_PRINCIPAL_HEADER);
+        filter.filter(request, responseHandler);
+
+        assertUnauthorized(responseHandler, errorMessage);
+    }
+
+    @Test
+    public void certificate_is_accepted() {
+        DiscFilterRequest request = mock(DiscFilterRequest.class);
+        when(request.getHeader(ATHENZ_PRINCIPAL_HEADER)).thenReturn(null);
+        when(request.getAttribute("jdisc.request.X509Certificate")).thenReturn(new X509Certificate[]{CERTIFICATE});
+
+        ResponseHandlerMock responseHandler = new ResponseHandlerMock();
+
+        AthenzPrincipalFilter filter = new AthenzPrincipalFilter(validator, Runnable::run, ATHENZ_PRINCIPAL_HEADER);
+        filter.filter(request, responseHandler);
+
+        AthenzPrincipal expectedPrincipal = new AthenzPrincipal(IDENTITY);
+        verify(request).setUserPrincipal(expectedPrincipal);
     }
 
     @Test
-    public void invalid_token_is_unauthorized() throws Exception {
+    public void both_ntoken_and_certificate_is_accepted() {
         DiscFilterRequest request = mock(DiscFilterRequest.class);
+        AthenzPrincipal principalWithToken = new AthenzPrincipal(IDENTITY, NTOKEN);
         when(request.getHeader(ATHENZ_PRINCIPAL_HEADER)).thenReturn(NTOKEN.getRawToken());
+        when(request.getAttribute("jdisc.request.X509Certificate")).thenReturn(new X509Certificate[]{CERTIFICATE});
+        when(validator.validate(NTOKEN)).thenReturn(principalWithToken);
+
+        ResponseHandlerMock responseHandler = new ResponseHandlerMock();
+
+        AthenzPrincipalFilter filter = new AthenzPrincipalFilter(validator, Runnable::run, ATHENZ_PRINCIPAL_HEADER);
+        filter.filter(request, responseHandler);
 
-        when(validator.validate(NTOKEN)).thenThrow(new InvalidTokenException("Invalid token"));
+        verify(request).setUserPrincipal(principalWithToken);
+    }
+
+    @Test
+    public void conflicting_ntoken_and_certificate_is_unauthorized() {
+        DiscFilterRequest request = mock(DiscFilterRequest.class);
+        AthenzUser conflictingIdentity = AthenzUser.fromUserId(new UserId("mallory"));
+        when(request.getHeader(ATHENZ_PRINCIPAL_HEADER)).thenReturn(NTOKEN.getRawToken());
+        when(request.getAttribute("jdisc.request.X509Certificate"))
+                .thenReturn(new X509Certificate[]{createSelfSignedCertificate(conflictingIdentity)});
+        when(validator.validate(NTOKEN)).thenReturn(new AthenzPrincipal(IDENTITY));
 
         ResponseHandlerMock responseHandler = new ResponseHandlerMock();
 
         AthenzPrincipalFilter filter = new AthenzPrincipalFilter(validator, Runnable::run, ATHENZ_PRINCIPAL_HEADER);
         filter.filter(request, responseHandler);
 
+        assertUnauthorized(responseHandler, "Identity in principal token does not match x509 CN");
+    }
+
+    private static void assertUnauthorized(ResponseHandlerMock responseHandler, String expectedMessageSubstring) {
         assertThat(responseHandler.response, notNullValue());
         assertThat(responseHandler.response.getStatus(), equalTo(UNAUTHORIZED));
-        assertThat(responseHandler.getResponseContent(), containsString("Invalid token"));
+        assertThat(responseHandler.getResponseContent(), containsString(expectedMessageSubstring));
     }
 
     private static class ResponseHandlerMock implements ResponseHandler {
@@ -114,4 +182,24 @@ public class AthenzPrincipalFilterTest {
 
     }
 
+    // TODO Move this to separate athenz module/bundle
+    private static X509Certificate createSelfSignedCertificate(AthenzIdentity identity) {
+        try {
+            KeyPairGenerator keyGen = KeyPairGenerator.getInstance("RSA");
+            keyGen.initialize(512);
+            KeyPair keyPair = keyGen.genKeyPair();
+            ContentSigner contentSigner = new JcaContentSignerBuilder("SHA256WithRSA").build(keyPair.getPrivate());
+            X500Name x500Name = new X500Name("CN="+ identity.getFullName());
+            X509v3CertificateBuilder certificateBuilder =
+                    new JcaX509v3CertificateBuilder(
+                            x500Name, BigInteger.ONE, new Date(), Date.from(Instant.now().plus(Duration.ofDays(30))),
+                            x500Name, keyPair.getPublic());
+            return new JcaX509CertificateConverter()
+                    .setProvider(new BouncyCastleProvider())
+                    .getCertificate(certificateBuilder.build(contentSigner));
+        } catch (CertificateException | NoSuchAlgorithmException | OperatorCreationException e) {
+            throw new RuntimeException(e);
+        }
+    }
+
 }