From 2a056e05db247b39ec395631364b8c0d286d6085 Mon Sep 17 00:00:00 2001 From: Bjørn Christian Seime Date: Wed, 29 Jan 2020 17:10:45 +0100 Subject: Access tokens should not be an empty string --- .../main/java/com/yahoo/vespa/athenz/api/AthenzAccessToken.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/api/AthenzAccessToken.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/api/AthenzAccessToken.java index 86deb0b59b3..ec8c1f3f9f3 100644 --- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/api/AthenzAccessToken.java +++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/api/AthenzAccessToken.java @@ -22,9 +22,13 @@ public class AthenzAccessToken { private static String stripBearerTokenPrefix(String rawValue) { String stripped = rawValue.strip(); - return stripped.startsWith(BEARER_TOKEN_PREFIX) - ? stripped.substring(BEARER_TOKEN_PREFIX.length()) + String prefixRemoved = stripped.startsWith(BEARER_TOKEN_PREFIX) + ? stripped.substring(BEARER_TOKEN_PREFIX.length()).strip() : stripped; + if (prefixRemoved.isBlank()) { + throw new IllegalArgumentException(String.format("Access token is blank: '%s'", prefixRemoved)); + } + return prefixRemoved; } public String value() { return value; } -- cgit v1.2.3 From 79bd9b2122efb99805d89e89a0354f70161420ed Mon Sep 17 00:00:00 2001 From: Bjørn Christian Seime Date: Wed, 29 Jan 2020 17:11:17 +0100 Subject: Add toString() to ResourceNameAndAction --- .../jdisc/http/filter/security/athenz/RequestResourceMapper.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/jdisc-security-filters/src/main/java/com/yahoo/jdisc/http/filter/security/athenz/RequestResourceMapper.java b/jdisc-security-filters/src/main/java/com/yahoo/jdisc/http/filter/security/athenz/RequestResourceMapper.java index 77709975cba..0bf000efc00 100644 --- a/jdisc-security-filters/src/main/java/com/yahoo/jdisc/http/filter/security/athenz/RequestResourceMapper.java +++ b/jdisc-security-filters/src/main/java/com/yahoo/jdisc/http/filter/security/athenz/RequestResourceMapper.java @@ -33,5 +33,13 @@ public interface RequestResourceMapper { public String action() { return action; } + + @Override + public String toString() { + return "ResourceNameAndAction{" + + "resourceName=" + resourceName + + ", action='" + action + '\'' + + '}'; + } } } -- cgit v1.2.3 From 6df641688fed097505d3e675960b49d7162c6828 Mon Sep 17 00:00:00 2001 From: Bjørn Christian Seime Date: Wed, 29 Jan 2020 17:12:24 +0100 Subject: Add debug logging of error responses --- .../http/filter/security/base/JsonSecurityRequestFilterBase.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/jdisc-security-filters/src/main/java/com/yahoo/jdisc/http/filter/security/base/JsonSecurityRequestFilterBase.java b/jdisc-security-filters/src/main/java/com/yahoo/jdisc/http/filter/security/base/JsonSecurityRequestFilterBase.java index ec8a93019b0..e654182d665 100644 --- a/jdisc-security-filters/src/main/java/com/yahoo/jdisc/http/filter/security/base/JsonSecurityRequestFilterBase.java +++ b/jdisc-security-filters/src/main/java/com/yahoo/jdisc/http/filter/security/base/JsonSecurityRequestFilterBase.java @@ -10,9 +10,11 @@ import com.yahoo.jdisc.handler.ResponseDispatch; import com.yahoo.jdisc.handler.ResponseHandler; import com.yahoo.jdisc.http.filter.DiscFilterRequest; import com.yahoo.jdisc.http.filter.SecurityRequestFilter; +import com.yahoo.log.LogLevel; import java.io.UncheckedIOException; import java.util.Optional; +import java.util.logging.Logger; /** * A base class for {@link SecurityRequestFilter} implementations that renders an error response as JSON. @@ -21,22 +23,25 @@ import java.util.Optional; */ public abstract class JsonSecurityRequestFilterBase implements SecurityRequestFilter { + private static final Logger log = Logger.getLogger(JsonSecurityRequestFilterBase.class.getName()); private static final ObjectMapper mapper = new ObjectMapper(); @Override public final void filter(DiscFilterRequest request, ResponseHandler handler) { filter(request) - .ifPresent(errorResponse -> writeResponse(errorResponse, handler)); + .ifPresent(errorResponse -> writeResponse(request, errorResponse, handler)); } protected abstract Optional filter(DiscFilterRequest request); - private void writeResponse(ErrorResponse error, ResponseHandler responseHandler) { + private void writeResponse(DiscFilterRequest request, ErrorResponse error, ResponseHandler responseHandler) { ObjectNode errorMessage = mapper.createObjectNode(); errorMessage.put("code", error.errorCode); errorMessage.put("message", error.message); error.response.headers().put("Content-Type", "application/json"); // Note: Overwrites header if already exists error.response.headers().put("Cache-Control", "must-revalidate,no-cache,no-store"); + log.log(LogLevel.DEBUG, () -> String.format("Error response for '%s': statusCode=%d, errorCode=%d, message='%s'", + request, error.response.getStatus(), error.errorCode, error.message)); try (FastContentWriter writer = ResponseDispatch.newInstance(error.response).connectFastWriter(responseHandler)) { writer.write(mapper.writerWithDefaultPrettyPrinter().writeValueAsString(errorMessage)); } catch (JsonProcessingException e) { -- cgit v1.2.3 From eb4b7b8fcdc7aa5de13c05872a1fdca4076179b9 Mon Sep 17 00:00:00 2001 From: Bjørn Christian Seime Date: Thu, 30 Jan 2020 12:43:28 +0100 Subject: Add methods to convert AthenzRole to and from single string --- .../main/java/com/yahoo/vespa/athenz/api/AthenzRole.java | 14 ++++++++++++++ .../yahoo/vespa/athenz/tls/AthenzX509CertificateUtils.java | 5 +---- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/api/AthenzRole.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/api/AthenzRole.java index 3a81e4a5e17..a7c9dbff3f8 100644 --- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/api/AthenzRole.java +++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/api/AthenzRole.java @@ -7,6 +7,8 @@ import java.util.Objects; * @author tokle */ public class AthenzRole { + private static final String DOMAIN_ROLE_NAME_DELIMITER = ":role."; + private final AthenzDomain domain; private final String roleName; @@ -20,6 +22,16 @@ public class AthenzRole { this.roleName = roleName; } + public static AthenzRole fromString(String string) { + if (!string.contains(DOMAIN_ROLE_NAME_DELIMITER)) { + throw new IllegalArgumentException("Not a valid role: " + string); + } + int delimiterIndex = string.indexOf(DOMAIN_ROLE_NAME_DELIMITER); + String domain = string.substring(0, delimiterIndex); + String roleName = string.substring(delimiterIndex + DOMAIN_ROLE_NAME_DELIMITER.length()); + return new AthenzRole(domain, roleName); + } + public AthenzDomain domain() { return domain; } @@ -28,6 +40,8 @@ public class AthenzRole { return roleName; } + public String asString() { return domain.getName() + DOMAIN_ROLE_NAME_DELIMITER + roleName; } + @Override public boolean equals(Object o) { if (this == o) return true; diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/tls/AthenzX509CertificateUtils.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/tls/AthenzX509CertificateUtils.java index 33e5552eaf6..6793d5804c7 100644 --- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/tls/AthenzX509CertificateUtils.java +++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/tls/AthenzX509CertificateUtils.java @@ -43,10 +43,7 @@ public class AthenzX509CertificateUtils { public static AthenzRole getRolesFromRoleCertificate(X509Certificate certificate) { String commonName = com.yahoo.security.X509CertificateUtils.getSubjectCommonNames(certificate).get(0); - int delimiterIndex = commonName.indexOf(COMMON_NAME_ROLE_DELIMITER); - String domain = commonName.substring(0, delimiterIndex); - String roleName = commonName.substring(delimiterIndex + COMMON_NAME_ROLE_DELIMITER.length()); - return new AthenzRole(domain, roleName); + return AthenzRole.fromString(commonName); } private static AthenzIdentity getIdentityFromSanEmail(String email) { -- cgit v1.2.3 From 23bdf059f1b8345495e0b72f61d30bc15761d4da Mon Sep 17 00:00:00 2001 From: Bjørn Christian Seime Date: Thu, 30 Jan 2020 12:43:37 +0100 Subject: Rewrite AthenzAuthorizationFilter to accept access tokens Change athenz-authorization-filter.def to have an enum set of enabled credentials. Delegate to ZPE to determine if a certificate is an Athenz role or identity certificate. Introduce various request attributes to propagate result from ZPE. --- .../security/athenz/AthenzAuthorizationFilter.java | 199 ++++++++++++--------- .../athenz-authorization-filter.def | 6 +- .../athenz/AthenzAuthorizationFilterTest.java | 185 ++++++++++++++----- 3 files changed, 266 insertions(+), 124 deletions(-) diff --git a/jdisc-security-filters/src/main/java/com/yahoo/jdisc/http/filter/security/athenz/AthenzAuthorizationFilter.java b/jdisc-security-filters/src/main/java/com/yahoo/jdisc/http/filter/security/athenz/AthenzAuthorizationFilter.java index 9151aa1b693..bf39ddb29b1 100644 --- a/jdisc-security-filters/src/main/java/com/yahoo/jdisc/http/filter/security/athenz/AthenzAuthorizationFilter.java +++ b/jdisc-security-filters/src/main/java/com/yahoo/jdisc/http/filter/security/athenz/AthenzAuthorizationFilter.java @@ -2,26 +2,32 @@ package com.yahoo.jdisc.http.filter.security.athenz; import com.google.inject.Inject; -import com.yahoo.jdisc.Response; import com.yahoo.jdisc.http.filter.DiscFilterRequest; -import com.yahoo.jdisc.http.filter.security.athenz.AthenzAuthorizationFilterConfig.CredentialsToVerify; import com.yahoo.jdisc.http.filter.security.athenz.RequestResourceMapper.ResourceNameAndAction; import com.yahoo.jdisc.http.filter.security.base.JsonSecurityRequestFilterBase; +import com.yahoo.log.LogLevel; +import com.yahoo.vespa.athenz.api.AthenzAccessToken; import com.yahoo.vespa.athenz.api.AthenzIdentity; import com.yahoo.vespa.athenz.api.AthenzPrincipal; -import com.yahoo.vespa.athenz.api.AthenzResourceName; -import com.yahoo.vespa.athenz.api.AthenzRole; import com.yahoo.vespa.athenz.api.ZToken; import com.yahoo.vespa.athenz.tls.AthenzX509CertificateUtils; +import com.yahoo.vespa.athenz.utils.AthenzIdentities; import com.yahoo.vespa.athenz.zpe.AuthorizationResult; import com.yahoo.vespa.athenz.zpe.DefaultZpe; import com.yahoo.vespa.athenz.zpe.Zpe; import java.security.cert.X509Certificate; +import java.util.EnumSet; +import java.util.List; import java.util.Optional; -import java.util.function.Function; +import java.util.logging.Logger; -import static java.util.Collections.singletonList; +import static com.yahoo.jdisc.Response.Status.FORBIDDEN; +import static com.yahoo.jdisc.Response.Status.UNAUTHORIZED; +import static com.yahoo.jdisc.http.filter.security.athenz.AthenzAuthorizationFilterConfig.EnabledCredentials; +import static com.yahoo.jdisc.http.filter.security.athenz.AthenzAuthorizationFilterConfig.EnabledCredentials.ACCESS_TOKEN; +import static com.yahoo.jdisc.http.filter.security.athenz.AthenzAuthorizationFilterConfig.EnabledCredentials.ROLE_CERTIFICATE; +import static com.yahoo.jdisc.http.filter.security.athenz.AthenzAuthorizationFilterConfig.EnabledCredentials.ROLE_TOKEN; /** * An Athenz security filter that uses a configured action and resource name to control access. @@ -30,10 +36,18 @@ import static java.util.Collections.singletonList; */ public class AthenzAuthorizationFilter extends JsonSecurityRequestFilterBase { - private final String headerName; + private static final String ATTRIBUTE_PREFIX = "jdisc-security-filters.athenz-authorization-filter"; + public static final String RESULT_ATTRIBUTE = ATTRIBUTE_PREFIX + ".result"; + public static final String MATCHED_ROLE_ATTRIBUTE = ATTRIBUTE_PREFIX + ".matched-role"; + public static final String IDENTITY_NAME_ATTRIBUTE = ATTRIBUTE_PREFIX + ".identity-name"; + public static final String MATCHED_CREDENTIAL_TYPE_ATTRIBUTE = ATTRIBUTE_PREFIX + ".credentials-type"; + + private static final Logger log = Logger.getLogger(AthenzAuthorizationFilter.class.getName()); + + private final String roleTokenHeaderName; + private final EnumSet enabledCredentials; private final Zpe zpe; private final RequestResourceMapper requestResourceMapper; - private final CredentialsToVerify.Enum credentialsToVerify; @Inject public AthenzAuthorizationFilter(AthenzAuthorizationFilterConfig config, RequestResourceMapper resourceMapper) { @@ -43,104 +57,127 @@ public class AthenzAuthorizationFilter extends JsonSecurityRequestFilterBase { AthenzAuthorizationFilter(AthenzAuthorizationFilterConfig config, RequestResourceMapper resourceMapper, Zpe zpe) { - this.headerName = config.roleTokenHeaderName(); - this.credentialsToVerify = config.credentialsToVerify(); + this.roleTokenHeaderName = config.roleTokenHeaderName(); + List enabledCredentials = config.enabledCredentials(); + this.enabledCredentials = enabledCredentials.isEmpty() + ? EnumSet.allOf(EnabledCredentials.Enum.class) + : EnumSet.copyOf(enabledCredentials); this.requestResourceMapper = resourceMapper; this.zpe = zpe; } @Override protected Optional filter(DiscFilterRequest request) { - Optional resourceMapping = - requestResourceMapper.getResourceNameAndAction(request.getMethod(), request.getRequestURI(), request.getQueryString()); - if (!resourceMapping.isPresent()) { - return Optional.empty(); - } - Optional roleCertificate = getRoleCertificate(request); - Optional roleToken = getRoleToken(request, headerName); - switch (credentialsToVerify) { - case CERTIFICATE_ONLY: { - if (!roleCertificate.isPresent()) { - return Optional.of(new ErrorResponse(Response.Status.UNAUTHORIZED, "Missing client certificate")); - } - return checkAccessAllowed(roleCertificate.get(), resourceMapping.get(), request); - } - case TOKEN_ONLY: { - if (!roleToken.isPresent()) { - return Optional.of(new ErrorResponse(Response.Status.UNAUTHORIZED, - String.format("Role token header '%s' is missing or does not have a value.", headerName))); - } - return checkAccessAllowed(roleToken.get(), resourceMapping.get(), request); - } - case ANY: { - if (!roleCertificate.isPresent() && !roleToken.isPresent()) { - return Optional.of(new ErrorResponse(Response.Status.UNAUTHORIZED, "Both role token and role certificate is missing")); - } - if (roleCertificate.isPresent()) { - return checkAccessAllowed(roleCertificate.get(), resourceMapping.get(), request); - } else { - return checkAccessAllowed(roleToken.get(), resourceMapping.get(), request); - } + try { + Optional resourceMapping = + requestResourceMapper.getResourceNameAndAction(request.getMethod(), request.getRequestURI(), request.getQueryString()); + log.log(LogLevel.DEBUG, () -> String.format("Resource mapping for '%s': %s", request, resourceMapping)); + if (resourceMapping.isEmpty()) { + return Optional.empty(); } - default: { - throw new IllegalStateException("Unexpected mode: " + credentialsToVerify); + Result result = checkAccessAllowed(request, resourceMapping.get()); + AuthorizationResult.Type resultType = result.zpeResult.type(); + setAttribute(request, RESULT_ATTRIBUTE, resultType.name()); + if (resultType == AuthorizationResult.Type.ALLOW) { + populateRequestWithResult(request, result); + return Optional.empty(); } + log.log(LogLevel.DEBUG, () -> String.format("Forbidden (403) for '%s': %s", request, resultType.name())); + return Optional.of(new ErrorResponse(FORBIDDEN, "Access forbidden: " + resultType.getDescription())); + } catch (IllegalArgumentException e) { + log.log(LogLevel.DEBUG, () -> String.format("Unauthorized (401) for '%s': %s", request, e.getMessage())); + return Optional.of(new ErrorResponse(UNAUTHORIZED, e.getMessage())); } } - private static Optional getRoleCertificate(DiscFilterRequest request) { - return Optional.of(request.getClientCertificateChain()) - .filter(chain -> !chain.isEmpty()) - .map(chain -> chain.get(0)) - .filter(AthenzX509CertificateUtils::isAthenzRoleCertificate); + private Result checkAccessAllowed(DiscFilterRequest request, ResourceNameAndAction resourceAndAction) { + // Note: the ordering of the if-constructs determines the precedence of the credential types + if (enabledCredentials.contains(ACCESS_TOKEN) + && isAccessTokenPresent(request) + && isClientCertificatePresent(request)) { + return checkAccessWithAccessToken(request, resourceAndAction); + } else if (enabledCredentials.contains(ROLE_CERTIFICATE) + && isClientCertificatePresent(request)) { + return checkAccessWithRoleCertificate(request, resourceAndAction); + } else if (enabledCredentials.contains(ROLE_TOKEN) + && isRoleTokenPresent(request)) { + return checkAccessWithRoleToken(request, resourceAndAction); + } else { + throw new IllegalArgumentException( + "Not authorized - request did not contain any of the allowed credentials: " + enabledCredentials); + } } - private static Optional getRoleToken(DiscFilterRequest request, String headerName) { - return Optional.ofNullable(request.getHeader(headerName)) - .filter(token -> !token.isEmpty()) - .map(ZToken::new); + private Result checkAccessWithAccessToken(DiscFilterRequest request, ResourceNameAndAction resourceAndAction) { + AthenzAccessToken accessToken = getAccessToken(request); + X509Certificate identityCertificate = getClientCertificate(request); + var zpeResult = zpe.checkAccessAllowed( + accessToken, identityCertificate, resourceAndAction.resourceName(), resourceAndAction.action()); + return new Result(ACCESS_TOKEN, AthenzIdentities.from(identityCertificate), zpeResult); } - private Optional checkAccessAllowed(X509Certificate certificate, - ResourceNameAndAction resourceNameAndAction, - DiscFilterRequest request) { - return checkAccessAllowed( - certificate, resourceNameAndAction, request, zpe::checkAccessAllowed, AthenzAuthorizationFilter::createPrincipal); + private Result checkAccessWithRoleCertificate(DiscFilterRequest request, ResourceNameAndAction resourceAndAction) { + X509Certificate roleCertificate = getClientCertificate(request); + var zpeResult = zpe.checkAccessAllowed(roleCertificate, resourceAndAction.resourceName(), resourceAndAction.action()); + AthenzIdentity identity = AthenzX509CertificateUtils.getIdentityFromRoleCertificate(roleCertificate); + return new Result(ROLE_CERTIFICATE, identity, zpeResult); } - private Optional checkAccessAllowed(ZToken roleToken, - ResourceNameAndAction resourceNameAndAction, - DiscFilterRequest request) { - return checkAccessAllowed( - roleToken, resourceNameAndAction, request, zpe::checkAccessAllowed, AthenzAuthorizationFilter::createPrincipal); + private Result checkAccessWithRoleToken(DiscFilterRequest request, ResourceNameAndAction resourceAndAction) { + ZToken roleToken = getRoleToken(request); + var zpeResult = zpe.checkAccessAllowed(roleToken, resourceAndAction.resourceName(), resourceAndAction.action()); + return new Result(ROLE_TOKEN, roleToken.getIdentity(), zpeResult); } - private static Optional checkAccessAllowed(C credentials, - ResourceNameAndAction resAndAction, - DiscFilterRequest request, - ZpeCheck accessCheck, - Function principalFactory) { - AuthorizationResult authorizationResult = accessCheck.checkAccess(credentials, resAndAction.resourceName(), resAndAction.action()); - if (authorizationResult.type() == AuthorizationResult.Type.ALLOW) { - request.setUserPrincipal(principalFactory.apply(credentials)); - authorizationResult.matchedRole().ifPresent(role -> request.setUserRoles(new String[] {role.roleName()})); - return Optional.empty(); - } - return Optional.of(new ErrorResponse(Response.Status.FORBIDDEN, "Access forbidden: " + authorizationResult.type().getDescription())); + private static boolean isAccessTokenPresent(DiscFilterRequest request) { + return request.getHeader(AthenzAccessToken.HTTP_HEADER_NAME) != null; + } + + private static boolean isClientCertificatePresent(DiscFilterRequest request) { + return !request.getClientCertificateChain().isEmpty(); } - private static AthenzPrincipal createPrincipal(X509Certificate certificate) { - AthenzIdentity identity = AthenzX509CertificateUtils.getIdentityFromRoleCertificate(certificate); - AthenzRole role = AthenzX509CertificateUtils.getRolesFromRoleCertificate(certificate); - return new AthenzPrincipal(identity, singletonList(role)); + private boolean isRoleTokenPresent(DiscFilterRequest request) { + return request.getHeader(roleTokenHeaderName) != null; } - private static AthenzPrincipal createPrincipal(ZToken roleToken) { - return new AthenzPrincipal(roleToken.getIdentity(), roleToken.getRoles()); + private static AthenzAccessToken getAccessToken(DiscFilterRequest request) { + return new AthenzAccessToken(request.getHeader(AthenzAccessToken.HTTP_HEADER_NAME)); } - @FunctionalInterface private interface ZpeCheck { - AuthorizationResult checkAccess(C credentials, AthenzResourceName resourceName, String action); + private static X509Certificate getClientCertificate(DiscFilterRequest request) { + return request.getClientCertificateChain().get(0); } + private ZToken getRoleToken(DiscFilterRequest request) { + return new ZToken(request.getHeader(roleTokenHeaderName)); + } + + private static void populateRequestWithResult(DiscFilterRequest request, Result result) { + request.setUserPrincipal( + new AthenzPrincipal(result.identity, result.zpeResult.matchedRole().map(List::of).orElse(List.of()))); + result.zpeResult.matchedRole().ifPresent(role -> { + request.setUserRoles(new String[]{role.roleName()}); + setAttribute(request, MATCHED_ROLE_ATTRIBUTE, role.roleName()); + }); + setAttribute(request, IDENTITY_NAME_ATTRIBUTE, result.identity.getFullName()); + setAttribute(request, MATCHED_CREDENTIAL_TYPE_ATTRIBUTE, result.credentialType.name()); + } + + private static void setAttribute(DiscFilterRequest request, String name, String value) { + log.log(LogLevel.DEBUG, () -> String.format("Setting attribute on '%s': '%s' = '%s'", request, name, value)); + request.setAttribute(name, value); + } + + private static class Result { + final EnabledCredentials.Enum credentialType; + final AthenzIdentity identity; + final AuthorizationResult zpeResult; + + Result(EnabledCredentials.Enum credentialType, AthenzIdentity identity, AuthorizationResult zpeResult) { + this.credentialType = credentialType; + this.identity = identity; + this.zpeResult = zpeResult; + } + } } diff --git a/jdisc-security-filters/src/main/resources/configdefinitions/athenz-authorization-filter.def b/jdisc-security-filters/src/main/resources/configdefinitions/athenz-authorization-filter.def index c60b7a125f8..ab8c4a204df 100644 --- a/jdisc-security-filters/src/main/resources/configdefinitions/athenz-authorization-filter.def +++ b/jdisc-security-filters/src/main/resources/configdefinitions/athenz-authorization-filter.def @@ -2,7 +2,7 @@ namespace=jdisc.http.filter.security.athenz # Which credentials to verify. Note: ANY will prioritize token over certificate if both are present. -credentialsToVerify enum { CERTIFICATE_ONLY, TOKEN_ONLY, ANY } default=ANY +enabledCredentials[] enum { ROLE_CERTIFICATE, ROLE_TOKEN, ACCESS_TOKEN } -# Name of header which includes role token. Must be set if 'credentialsTypeRequired' is set to TOKEN_ONLY or ANY. -roleTokenHeaderName string default="" +# Name of role token http header +roleTokenHeaderName string default="Athenz-Role-Token" diff --git a/jdisc-security-filters/src/test/java/com/yahoo/jdisc/http/filter/security/athenz/AthenzAuthorizationFilterTest.java b/jdisc-security-filters/src/test/java/com/yahoo/jdisc/http/filter/security/athenz/AthenzAuthorizationFilterTest.java index ecf746179a3..1fe8d73eb44 100644 --- a/jdisc-security-filters/src/test/java/com/yahoo/jdisc/http/filter/security/athenz/AthenzAuthorizationFilterTest.java +++ b/jdisc-security-filters/src/test/java/com/yahoo/jdisc/http/filter/security/athenz/AthenzAuthorizationFilterTest.java @@ -1,28 +1,44 @@ // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.jdisc.http.filter.security.athenz; -import com.yahoo.container.jdisc.RequestHandlerTestDriver; +import com.yahoo.container.jdisc.RequestHandlerTestDriver.MockResponseHandler; import com.yahoo.jdisc.Response; import com.yahoo.jdisc.http.filter.DiscFilterRequest; +import com.yahoo.jdisc.http.filter.security.athenz.AthenzAuthorizationFilterConfig.EnabledCredentials; +import com.yahoo.security.KeyAlgorithm; +import com.yahoo.security.KeyUtils; +import com.yahoo.security.SubjectAlternativeName; +import com.yahoo.security.X509CertificateBuilder; import com.yahoo.vespa.athenz.api.AthenzAccessToken; +import com.yahoo.vespa.athenz.api.AthenzIdentity; import com.yahoo.vespa.athenz.api.AthenzResourceName; import com.yahoo.vespa.athenz.api.AthenzRole; import com.yahoo.vespa.athenz.api.ZToken; +import com.yahoo.vespa.athenz.utils.AthenzIdentities; import com.yahoo.vespa.athenz.zpe.AuthorizationResult; import com.yahoo.vespa.athenz.zpe.Zpe; import org.junit.Test; import org.mockito.Mockito; +import javax.security.auth.x500.X500Principal; +import java.math.BigInteger; +import java.security.KeyPair; import java.security.cert.X509Certificate; - -import static com.yahoo.jdisc.http.filter.security.athenz.AthenzAuthorizationFilterConfig.CredentialsToVerify.Enum.ANY; -import static com.yahoo.vespa.athenz.zpe.AuthorizationResult.*; -import static java.util.Collections.emptyList; -import static org.hamcrest.CoreMatchers.containsString; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertNull; +import java.time.Duration; +import java.time.Instant; +import java.util.List; + +import static com.yahoo.jdisc.http.filter.security.athenz.AthenzAuthorizationFilter.MATCHED_CREDENTIAL_TYPE_ATTRIBUTE; +import static com.yahoo.jdisc.http.filter.security.athenz.AthenzAuthorizationFilter.MATCHED_ROLE_ATTRIBUTE; +import static com.yahoo.jdisc.http.filter.security.athenz.AthenzAuthorizationFilter.RESULT_ATTRIBUTE; +import static com.yahoo.security.SignatureAlgorithm.SHA256_WITH_ECDSA; +import static com.yahoo.security.SubjectAlternativeName.Type.RFC822_NAME; +import static com.yahoo.vespa.athenz.zpe.AuthorizationResult.Type; +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.CoreMatchers.notNullValue; +import static org.hamcrest.CoreMatchers.nullValue; import static org.junit.Assert.assertThat; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; /** @@ -31,73 +47,162 @@ import static org.mockito.Mockito.when; public class AthenzAuthorizationFilterTest { private static final AthenzResourceName RESOURCE_NAME = new AthenzResourceName("domain", "my-resource-name"); + private static final ZToken ROLE_TOKEN = new ZToken("v=Z1;d=domain;r=my-role;p=my-domain.my-service"); + private static final AthenzAccessToken ACCESS_TOKEN = new AthenzAccessToken("access-token"); + private static final AthenzIdentity IDENTITY = AthenzIdentities.from("user.john"); + private static final AthenzRole ROLE = new AthenzRole("my.domain", "my-role"); + private static final X509Certificate IDENTITY_CERTIFICATE = createDummyIdentityCertificate(IDENTITY); + private static final X509Certificate ROLE_CERTIFICATE = createDummyRoleCertificate(ROLE, IDENTITY); private static final String ACTION = "update"; private static final String HEADER_NAME = "Athenz-Role-Token"; - private static final AthenzAuthorizationFilterConfig CONFIG = createConfig(); - private static AthenzAuthorizationFilterConfig createConfig() { - return new AthenzAuthorizationFilterConfig( - new AthenzAuthorizationFilterConfig.Builder() - .roleTokenHeaderName(HEADER_NAME) - .credentialsToVerify(ANY)); + @Test + public void accepts_request_with_access_token() { + AthenzAuthorizationFilter filter = createFilter(new AllowingZpe(), List.of()); + + MockResponseHandler responseHandler = new MockResponseHandler(); + DiscFilterRequest request = createRequest(null, ACCESS_TOKEN, IDENTITY_CERTIFICATE); + filter.filter(request, responseHandler); + + assertAuthorizationResult(request, Type.ALLOW); + assertRequestNotFiltered(responseHandler); + assertMatchedCredentialType(request, EnabledCredentials.ACCESS_TOKEN); + assertMatchedRole(request, ROLE); } @Test - public void accepts_valid_requests() { - AthenzAuthorizationFilter filter = - new AthenzAuthorizationFilter( - CONFIG, new StaticRequestResourceMapper(RESOURCE_NAME, ACTION), new AllowingZpe()); + public void accepts_request_with_role_certificate() { + AthenzAuthorizationFilter filter = createFilter(new AllowingZpe(), List.of()); - RequestHandlerTestDriver.MockResponseHandler responseHandler = new RequestHandlerTestDriver.MockResponseHandler(); - filter.filter(createRequest(), responseHandler); + MockResponseHandler responseHandler = new MockResponseHandler(); + DiscFilterRequest request = createRequest(null, null, ROLE_CERTIFICATE); + filter.filter(request, responseHandler); - assertNull(responseHandler.getResponse()); + assertAuthorizationResult(request, Type.ALLOW); + assertRequestNotFiltered(responseHandler); + assertMatchedCredentialType(request, EnabledCredentials.ROLE_CERTIFICATE); + assertMatchedRole(request, ROLE); } @Test - public void returns_error_on_forbidden_requests() { + public void accepts_request_with_role_token() { + AthenzAuthorizationFilter filter = createFilter(new AllowingZpe(), List.of()); + + MockResponseHandler responseHandler = new MockResponseHandler(); + DiscFilterRequest request = createRequest(ROLE_TOKEN, null, null); + filter.filter(request, responseHandler); + + assertAuthorizationResult(request, Type.ALLOW); + assertRequestNotFiltered(responseHandler); + assertMatchedCredentialType(request, EnabledCredentials.ROLE_TOKEN); + assertMatchedRole(request, ROLE); + } + + @Test + public void returns_unauthorized_for_request_with_disabled_credential_type() { AthenzAuthorizationFilter filter = - new AthenzAuthorizationFilter( - CONFIG, new StaticRequestResourceMapper(RESOURCE_NAME, ACTION), new DenyingZpe()); + createFilter(new AllowingZpe(), List.of(EnabledCredentials.ROLE_CERTIFICATE, EnabledCredentials.ACCESS_TOKEN)); - RequestHandlerTestDriver.MockResponseHandler responseHandler = new RequestHandlerTestDriver.MockResponseHandler(); - filter.filter(createRequest(), responseHandler); + MockResponseHandler responseHandler = new MockResponseHandler(); + DiscFilterRequest request = createRequest(ROLE_TOKEN, null, null); + filter.filter(request, responseHandler); - Response response = responseHandler.getResponse(); - assertNotNull(response); - assertEquals(403, response.getStatus()); - String content = responseHandler.readAll(); - assertThat(content, containsString(Type.DENY.getDescription())); + assertStatusCode(responseHandler, 401); + } + + @Test + public void returns_forbidden_for_credentials_rejected_by_zpe() { + AthenzAuthorizationFilter filter = createFilter(new DenyingZpe(), List.of()); + + MockResponseHandler responseHandler = new MockResponseHandler(); + DiscFilterRequest request = createRequest(ROLE_TOKEN, null, null); + filter.filter(request, responseHandler); + + assertStatusCode(responseHandler, 403); + assertAuthorizationResult(request, Type.DENY); + } + + private static X509Certificate createDummyIdentityCertificate(AthenzIdentity identity) { + KeyPair keyPair = KeyUtils.generateKeypair(KeyAlgorithm.EC, 256); + X500Principal x500Name = new X500Principal("CN="+ identity.getFullName()); + Instant now = Instant.now(); + return X509CertificateBuilder + .fromKeypair(keyPair, x500Name, now, now.plus(Duration.ofDays(30)), SHA256_WITH_ECDSA, BigInteger.ONE) + .build(); + } + + private static X509Certificate createDummyRoleCertificate(AthenzRole role, AthenzIdentity identity) { + KeyPair keyPair = KeyUtils.generateKeypair(KeyAlgorithm.EC, 256); + X500Principal x500Name = new X500Principal("CN="+ role.domain().getName() + ":role." + role.roleName()); + Instant now = Instant.now(); + return X509CertificateBuilder + .fromKeypair(keyPair, x500Name, now, now.plus(Duration.ofDays(30)), SHA256_WITH_ECDSA, BigInteger.ONE) + .addSubjectAlternativeName(new SubjectAlternativeName(RFC822_NAME, identity.getFullName() + "@my.domain.my-identity-provider")) + .build(); } - private static DiscFilterRequest createRequest() { + private static DiscFilterRequest createRequest(ZToken roleToken, AthenzAccessToken accessToken, X509Certificate clientCert) { DiscFilterRequest request = Mockito.mock(DiscFilterRequest.class); - when(request.getHeader(HEADER_NAME)).thenReturn("v=Z1;d=domain;r=my-role;p=my-domain.my-service"); + when(request.getHeader(HEADER_NAME)).thenReturn(roleToken != null ? roleToken.getRawToken() : null); + when(request.getHeader(AthenzAccessToken.HTTP_HEADER_NAME)).thenReturn(accessToken != null ? "Bearer " + accessToken.value() : null); when(request.getMethod()).thenReturn("GET"); when(request.getRequestURI()).thenReturn("/my/path"); when(request.getQueryString()).thenReturn(null); - when(request.getClientCertificateChain()).thenReturn(emptyList()); + when(request.getClientCertificateChain()).thenReturn(clientCert != null ? List.of(clientCert) : List.of()); return request; } - static class AllowingZpe implements Zpe { + private static AthenzAuthorizationFilter createFilter(Zpe zpe, List enabledCredentials) { + return new AthenzAuthorizationFilter( + new AthenzAuthorizationFilterConfig( + new AthenzAuthorizationFilterConfig.Builder() + .roleTokenHeaderName(HEADER_NAME) + .enabledCredentials(enabledCredentials)), + new StaticRequestResourceMapper(RESOURCE_NAME, ACTION), + zpe); + } + + private static void assertAuthorizationResult(DiscFilterRequest request, Type expectedResult) { + verify(request).setAttribute(RESULT_ATTRIBUTE, expectedResult.name()); + } + + private static void assertStatusCode(MockResponseHandler responseHandler, int statusCode) { + Response response = responseHandler.getResponse(); + assertThat(response, notNullValue()); + assertThat(response.getStatus(), equalTo(statusCode)); + } + + private static void assertMatchedCredentialType(DiscFilterRequest request, EnabledCredentials.Enum expectedType) { + verify(request).setAttribute(MATCHED_CREDENTIAL_TYPE_ATTRIBUTE, expectedType.name()); + } + + private static void assertRequestNotFiltered(MockResponseHandler responseHandler) { + assertThat(responseHandler.getResponse(), nullValue()); + } + + private static void assertMatchedRole(DiscFilterRequest request, AthenzRole role) { + verify(request).setAttribute(MATCHED_ROLE_ATTRIBUTE, role.roleName()); + } + + private static class AllowingZpe implements Zpe { + @Override public AuthorizationResult checkAccessAllowed(ZToken roleToken, AthenzResourceName resourceName, String action) { - return new AuthorizationResult(Type.ALLOW, new AthenzRole(resourceName.getDomain(), "rolename")); + return new AuthorizationResult(Type.ALLOW, ROLE); } @Override public AuthorizationResult checkAccessAllowed(X509Certificate roleCertificate, AthenzResourceName resourceName, String action) { - return new AuthorizationResult(Type.ALLOW, new AthenzRole(resourceName.getDomain(), "rolename")); + return new AuthorizationResult(Type.ALLOW, ROLE); } @Override public AuthorizationResult checkAccessAllowed(AthenzAccessToken accessToken, X509Certificate identityCertificate, AthenzResourceName resourceName, String action) { - return new AuthorizationResult(Type.ALLOW, new AthenzRole(resourceName.getDomain(), "rolename")); + return new AuthorizationResult(Type.ALLOW, ROLE); } } - static class DenyingZpe implements Zpe { + private static class DenyingZpe implements Zpe { @Override public AuthorizationResult checkAccessAllowed(ZToken roleToken, AthenzResourceName resourceName, String action) { return new AuthorizationResult(Type.DENY); -- cgit v1.2.3 From 959960a0e24d33a22d360468834cb4e41fa145c5 Mon Sep 17 00:00:00 2001 From: Bjørn Christian Seime Date: Thu, 30 Jan 2020 12:54:05 +0100 Subject: Remove unused methods Methods were unused and relied on hardcoded issuer names (ouch!). --- .../yahoo/vespa/athenz/tls/AthenzX509CertificateUtils.java | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/tls/AthenzX509CertificateUtils.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/tls/AthenzX509CertificateUtils.java index 6793d5804c7..a555f955962 100644 --- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/tls/AthenzX509CertificateUtils.java +++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/tls/AthenzX509CertificateUtils.java @@ -17,20 +17,8 @@ import static com.yahoo.security.SubjectAlternativeName.Type.RFC822_NAME; */ public class AthenzX509CertificateUtils { - private static final String COMMON_NAME_ROLE_DELIMITER = ":role."; - private AthenzX509CertificateUtils() {} - public static boolean isAthenzRoleCertificate(X509Certificate certificate) { - return isAthenzIssuedCertificate(certificate) && - com.yahoo.security.X509CertificateUtils.getSubjectCommonNames(certificate).get(0).contains(COMMON_NAME_ROLE_DELIMITER); - } - - public static boolean isAthenzIssuedCertificate(X509Certificate certificate) { - return com.yahoo.security.X509CertificateUtils.getIssuerCommonNames(certificate).stream() - .anyMatch(cn -> cn.equalsIgnoreCase("Yahoo Athenz CA") || cn.equalsIgnoreCase("Athenz AWS CA")); - } - public static AthenzIdentity getIdentityFromRoleCertificate(X509Certificate certificate) { List sans = com.yahoo.security.X509CertificateUtils.getSubjectAlternativeNames(certificate); return sans.stream() -- cgit v1.2.3 From 045cb0fa8fb519f7470f2f63c5c0e6884d63b3b0 Mon Sep 17 00:00:00 2001 From: Bjørn Christian Seime Date: Fri, 31 Jan 2020 11:18:04 +0100 Subject: Improve naming of string conversion methods for AthenzRole --- .../com/yahoo/vespa/athenz/api/AthenzRole.java | 23 +++++++++++++--------- .../athenz/tls/AthenzX509CertificateUtils.java | 2 +- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/api/AthenzRole.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/api/AthenzRole.java index a7c9dbff3f8..4e432768298 100644 --- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/api/AthenzRole.java +++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/api/AthenzRole.java @@ -7,7 +7,7 @@ import java.util.Objects; * @author tokle */ public class AthenzRole { - private static final String DOMAIN_ROLE_NAME_DELIMITER = ":role."; + private static final String ROLE_RESOURCE_PREFIX = "role."; private final AthenzDomain domain; private final String roleName; @@ -22,14 +22,17 @@ public class AthenzRole { this.roleName = roleName; } - public static AthenzRole fromString(String string) { - if (!string.contains(DOMAIN_ROLE_NAME_DELIMITER)) { - throw new IllegalArgumentException("Not a valid role: " + string); + public static AthenzRole fromResourceNameString(String string) { + return fromResourceName(AthenzResourceName.fromString(string)); + } + + public static AthenzRole fromResourceName(AthenzResourceName resourceName) { + String entityName = resourceName.getEntityName(); + if (!entityName.startsWith(ROLE_RESOURCE_PREFIX)) { + throw new IllegalArgumentException("Not a valid role: " + resourceName.toResourceNameString()); } - int delimiterIndex = string.indexOf(DOMAIN_ROLE_NAME_DELIMITER); - String domain = string.substring(0, delimiterIndex); - String roleName = string.substring(delimiterIndex + DOMAIN_ROLE_NAME_DELIMITER.length()); - return new AthenzRole(domain, roleName); + String roleName = entityName.substring(ROLE_RESOURCE_PREFIX.length()); + return new AthenzRole(resourceName.getDomain(), roleName); } public AthenzDomain domain() { @@ -40,7 +43,9 @@ public class AthenzRole { return roleName; } - public String asString() { return domain.getName() + DOMAIN_ROLE_NAME_DELIMITER + roleName; } + public String toResourceNameString() { return toResourceName().toResourceNameString(); } + + public AthenzResourceName toResourceName() { return new AthenzResourceName(domain, ROLE_RESOURCE_PREFIX + roleName); } @Override public boolean equals(Object o) { diff --git a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/tls/AthenzX509CertificateUtils.java b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/tls/AthenzX509CertificateUtils.java index a555f955962..bec21a5b25f 100644 --- a/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/tls/AthenzX509CertificateUtils.java +++ b/vespa-athenz/src/main/java/com/yahoo/vespa/athenz/tls/AthenzX509CertificateUtils.java @@ -31,7 +31,7 @@ public class AthenzX509CertificateUtils { public static AthenzRole getRolesFromRoleCertificate(X509Certificate certificate) { String commonName = com.yahoo.security.X509CertificateUtils.getSubjectCommonNames(certificate).get(0); - return AthenzRole.fromString(commonName); + return AthenzRole.fromResourceNameString(commonName); } private static AthenzIdentity getIdentityFromSanEmail(String email) { -- cgit v1.2.3 From 293994259901b6c5e80f2df20313f88238ce7cdb Mon Sep 17 00:00:00 2001 From: Bjørn Christian Seime Date: Fri, 31 Jan 2020 16:58:25 +0100 Subject: Add public modifier to constructor and filter() --- .../http/filter/security/athenz/AthenzAuthorizationFilter.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/jdisc-security-filters/src/main/java/com/yahoo/jdisc/http/filter/security/athenz/AthenzAuthorizationFilter.java b/jdisc-security-filters/src/main/java/com/yahoo/jdisc/http/filter/security/athenz/AthenzAuthorizationFilter.java index bf39ddb29b1..f3d9fa9583c 100644 --- a/jdisc-security-filters/src/main/java/com/yahoo/jdisc/http/filter/security/athenz/AthenzAuthorizationFilter.java +++ b/jdisc-security-filters/src/main/java/com/yahoo/jdisc/http/filter/security/athenz/AthenzAuthorizationFilter.java @@ -54,9 +54,9 @@ public class AthenzAuthorizationFilter extends JsonSecurityRequestFilterBase { this(config, resourceMapper, new DefaultZpe()); } - AthenzAuthorizationFilter(AthenzAuthorizationFilterConfig config, - RequestResourceMapper resourceMapper, - Zpe zpe) { + public AthenzAuthorizationFilter(AthenzAuthorizationFilterConfig config, + RequestResourceMapper resourceMapper, + Zpe zpe) { this.roleTokenHeaderName = config.roleTokenHeaderName(); List enabledCredentials = config.enabledCredentials(); this.enabledCredentials = enabledCredentials.isEmpty() @@ -67,7 +67,7 @@ public class AthenzAuthorizationFilter extends JsonSecurityRequestFilterBase { } @Override - protected Optional filter(DiscFilterRequest request) { + public Optional filter(DiscFilterRequest request) { try { Optional resourceMapping = requestResourceMapper.getResourceNameAndAction(request.getMethod(), request.getRequestURI(), request.getQueryString()); -- cgit v1.2.3