diff options
author | Bjørn Christian Seime <bjorncs@yahooinc.com> | 2022-07-21 14:56:51 +0200 |
---|---|---|
committer | Bjørn Christian Seime <bjorncs@yahooinc.com> | 2022-07-21 15:30:19 +0200 |
commit | f4965306b79f0015ca9e8e32072877e57f7f532c (patch) | |
tree | c3bc93a0916de30dcb70435531c1aa850b27c51c | |
parent | d2864cf3be9a93d784ac98b6beee0813dc60b290 (diff) |
Move logic for capability checking/logging to ConnectionAuthContext
6 files changed, 126 insertions, 38 deletions
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/security/MultiTenantRpcAuthorizerTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/security/MultiTenantRpcAuthorizerTest.java index bffed6eb0b1..2ab959fcaa0 100644 --- a/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/security/MultiTenantRpcAuthorizerTest.java +++ b/configserver/src/test/java/com/yahoo/vespa/config/server/rpc/security/MultiTenantRpcAuthorizerTest.java @@ -18,6 +18,7 @@ import com.yahoo.security.KeyAlgorithm; import com.yahoo.security.KeyUtils; import com.yahoo.security.SignatureAlgorithm; import com.yahoo.security.X509CertificateBuilder; +import com.yahoo.security.tls.CapabilityMode; import com.yahoo.security.tls.CapabilitySet; import com.yahoo.security.tls.ConnectionAuthContext; import com.yahoo.slime.Cursor; @@ -250,7 +251,7 @@ public class MultiTenantRpcAuthorizerTest { private static Request mockJrtRpcRequest(String payload) { ConnectionAuthContext authContext = - new ConnectionAuthContext(PEER_CERTIFICATE_CHAIN, CapabilitySet.all(), Set.of()); + new ConnectionAuthContext(PEER_CERTIFICATE_CHAIN, CapabilitySet.all(), Set.of(), CapabilityMode.ENFORCE); Target target = mock(Target.class); when(target.connectionAuthContext()).thenReturn(authContext); Request request = mock(Request.class); diff --git a/jrt/src/com/yahoo/jrt/RequireCapabilitiesFilter.java b/jrt/src/com/yahoo/jrt/RequireCapabilitiesFilter.java index bb2eafcf711..8b7fc3c1a46 100644 --- a/jrt/src/com/yahoo/jrt/RequireCapabilitiesFilter.java +++ b/jrt/src/com/yahoo/jrt/RequireCapabilitiesFilter.java @@ -2,24 +2,13 @@ package com.yahoo.jrt; import com.yahoo.security.tls.Capability; -import com.yahoo.security.tls.CapabilityMode; import com.yahoo.security.tls.CapabilitySet; -import com.yahoo.security.tls.ConnectionAuthContext; -import com.yahoo.security.tls.TransportSecurityUtils; - -import java.util.logging.Logger; - -import static com.yahoo.security.tls.CapabilityMode.DISABLE; -import static com.yahoo.security.tls.CapabilityMode.LOG_ONLY; /** * @author bjorncs */ public class RequireCapabilitiesFilter implements RequestAccessFilter { - private static final Logger log = Logger.getLogger(RequireCapabilitiesFilter.class.getName()); - private static final CapabilityMode MODE = TransportSecurityUtils.getCapabilityMode(); - private final CapabilitySet requiredCapabilities; public RequireCapabilitiesFilter(CapabilitySet requiredCapabilities) { @@ -32,23 +21,8 @@ public class RequireCapabilitiesFilter implements RequestAccessFilter { @Override public boolean allow(Request r) { - if (MODE == DISABLE) return true; - ConnectionAuthContext ctx = r.target().connectionAuthContext(); - CapabilitySet peerCapabilities = ctx.capabilities(); - boolean authorized = peerCapabilities.has(requiredCapabilities); - if (!authorized) { - String msg = "%sPermission denied for RPC method '%s'. Peer at %s with %s. Call requires %s, but peer has %s" - .formatted(MODE == LOG_ONLY ? "Dry-run: " : "", r.methodName(), r.target().peerSpec(), ctx.peerCertificateString().orElseThrow(), - requiredCapabilities.toNames(), peerCapabilities.toNames()); - if (MODE == LOG_ONLY) { - log.info(msg); - return true; - } else { - log.warning(msg); - return false; - } - } - return true; + return r.target().connectionAuthContext() + .hasCapabilities(requiredCapabilities, "RPC", r.methodName(), r.target().peerSpec().toString()); } } diff --git a/security-utils/src/main/java/com/yahoo/security/tls/CapabilitySet.java b/security-utils/src/main/java/com/yahoo/security/tls/CapabilitySet.java index ec402719efa..7e6c7f394cd 100644 --- a/security-utils/src/main/java/com/yahoo/security/tls/CapabilitySet.java +++ b/security-utils/src/main/java/com/yahoo/security/tls/CapabilitySet.java @@ -30,15 +30,17 @@ public class CapabilitySet { ; private final String name; - private final EnumSet<Capability> caps; + private final CapabilitySet set; Predefined(String name, Capability... caps) { this.name = name; - this.caps = caps.length == 0 ? EnumSet.noneOf(Capability.class) : EnumSet.copyOf(List.of(caps)); } + this.set = caps.length == 0 ? CapabilitySet.none() : CapabilitySet.from(caps); } public static Optional<Predefined> fromName(String name) { return Arrays.stream(values()).filter(p -> p.name.equals(name)).findAny(); } + + public CapabilitySet capabilities() { return set; } } private static final CapabilitySet ALL_CAPABILITIES = new CapabilitySet(EnumSet.allOf(Capability.class)); @@ -52,7 +54,7 @@ public class CapabilitySet { EnumSet<Capability> caps = EnumSet.noneOf(Capability.class); for (String name : names) { Predefined predefined = Predefined.fromName(name).orElse(null); - if (predefined != null) caps.addAll(predefined.caps); + if (predefined != null) caps.addAll(predefined.set.caps); else caps.add(Capability.fromName(name)); } return new CapabilitySet(caps); diff --git a/security-utils/src/main/java/com/yahoo/security/tls/ConnectionAuthContext.java b/security-utils/src/main/java/com/yahoo/security/tls/ConnectionAuthContext.java index b4e8878fb01..a28dab16d08 100644 --- a/security-utils/src/main/java/com/yahoo/security/tls/ConnectionAuthContext.java +++ b/security-utils/src/main/java/com/yahoo/security/tls/ConnectionAuthContext.java @@ -7,28 +7,77 @@ import java.security.cert.X509Certificate; import java.util.List; import java.util.Optional; import java.util.Set; +import java.util.function.Supplier; +import java.util.logging.Logger; import static com.yahoo.security.SubjectAlternativeName.Type.DNS; import static com.yahoo.security.SubjectAlternativeName.Type.URI; +import static com.yahoo.security.tls.CapabilityMode.DISABLE; +import static com.yahoo.security.tls.CapabilityMode.LOG_ONLY; /** * @author bjorncs */ public record ConnectionAuthContext(List<X509Certificate> peerCertificateChain, CapabilitySet capabilities, - Set<String> matchedPolicies) { + Set<String> matchedPolicies, + CapabilityMode capabilityMode) { - private static final ConnectionAuthContext DEFAULT_ALL_CAPABILITIES = new ConnectionAuthContext(List.of()); + private static final Logger log = Logger.getLogger(ConnectionAuthContext.class.getName()); public ConnectionAuthContext { peerCertificateChain = List.copyOf(peerCertificateChain); matchedPolicies = Set.copyOf(matchedPolicies); } - private ConnectionAuthContext(List<X509Certificate> certs) { this(certs, CapabilitySet.all(), Set.of()); } + private ConnectionAuthContext(List<X509Certificate> certs, CapabilityMode capabilityMode) { + this(certs, CapabilitySet.all(), Set.of(), capabilityMode); + } public boolean authorized() { return !capabilities.hasNone(); } + public boolean hasCapabilities(CapabilitySet requiredCapabilities) { + return hasCapabilities(requiredCapabilities, null, null, null); + } + + /** Provided strings are used for improved logging only */ + public boolean hasCapabilities(CapabilitySet requiredCapabilities, String action, String resource, String peer) { + if (capabilityMode == DISABLE) return authorized(); + boolean hasCapabilities = capabilities.has(requiredCapabilities); + if (!hasCapabilities) { + Supplier<String> errorMessageProvider = () -> + createPermissionDeniedErrorMessage(requiredCapabilities, action, resource, peer); + if (capabilityMode == LOG_ONLY) { + log.info(errorMessageProvider); + return true; + } else { + // Ideally log as warning but we have no mechanism for de-duplicating repeated log spamming. + log.fine(errorMessageProvider); + return false; + } + } + return true; + } + + String createPermissionDeniedErrorMessage( + CapabilitySet required, String action, String resource, String peer) { + StringBuilder b = new StringBuilder(); + if (capabilityMode == LOG_ONLY) b.append("Dry-run: "); + b.append("Permission denied"); + if (resource != null) { + b.append(" for '"); + if (action != null) { + b.append(action).append("' on '"); + } + b.append(resource).append("'"); + } + b.append(". Peer "); + if (peer != null) b.append("'").append(peer).append("' "); + return b.append("with ").append(peerCertificateString()).append(". Requires capabilities ") + .append(required.toNames()).append(" but peer has ").append(capabilities.toNames()) + .append(".").toString(); + } + public Optional<X509Certificate> peerCertificate() { return peerCertificateChain.isEmpty() ? Optional.empty() : Optional.of(peerCertificateChain.get(0)); } @@ -62,11 +111,11 @@ public record ConnectionAuthContext(List<X509Certificate> peerCertificateChain, } /** Construct instance with all capabilities */ - public static ConnectionAuthContext defaultAllCapabilities() { return DEFAULT_ALL_CAPABILITIES; } + public static ConnectionAuthContext defaultAllCapabilities() { return new ConnectionAuthContext(List.of(), DISABLE); } /** Construct instance with all capabilities */ public static ConnectionAuthContext defaultAllCapabilities(List<X509Certificate> certs) { - return new ConnectionAuthContext(certs); + return new ConnectionAuthContext(certs, DISABLE); } } diff --git a/security-utils/src/main/java/com/yahoo/security/tls/PeerAuthorizer.java b/security-utils/src/main/java/com/yahoo/security/tls/PeerAuthorizer.java index 44293de6eb7..951b5c57c9e 100644 --- a/security-utils/src/main/java/com/yahoo/security/tls/PeerAuthorizer.java +++ b/security-utils/src/main/java/com/yahoo/security/tls/PeerAuthorizer.java @@ -47,7 +47,10 @@ public class PeerAuthorizer { grantedCapabilities.add(peerPolicy.capabilities()); } } - return new ConnectionAuthContext(certChain, CapabilitySet.unionOf(grantedCapabilities), matchedPolicies); + // TODO Pass this through constructor + CapabilityMode capabilityMode = TransportSecurityUtils.getCapabilityMode(); + return new ConnectionAuthContext( + certChain, CapabilitySet.unionOf(grantedCapabilities), matchedPolicies, capabilityMode); } private static boolean matchesPolicy(PeerPolicy peerPolicy, String cn, List<String> sans) { diff --git a/security-utils/src/test/java/com/yahoo/security/tls/ConnectionAuthContextTest.java b/security-utils/src/test/java/com/yahoo/security/tls/ConnectionAuthContextTest.java new file mode 100644 index 00000000000..8036bc38d90 --- /dev/null +++ b/security-utils/src/test/java/com/yahoo/security/tls/ConnectionAuthContextTest.java @@ -0,0 +1,59 @@ +package com.yahoo.security.tls;// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. + +import com.yahoo.security.KeyAlgorithm; +import com.yahoo.security.KeyUtils; +import com.yahoo.security.X509CertificateBuilder; +import org.junit.jupiter.api.Test; + +import javax.security.auth.x500.X500Principal; +import java.math.BigInteger; +import java.security.KeyPair; +import java.security.cert.X509Certificate; +import java.time.Instant; +import java.time.temporal.ChronoUnit; +import java.util.List; +import java.util.Set; + +import static com.yahoo.security.SignatureAlgorithm.SHA256_WITH_ECDSA; +import static org.assertj.core.api.AssertionsForInterfaceTypes.assertThat; +import static org.junit.jupiter.api.Assertions.assertFalse; + +/** + * @author bjorncs + */ +class ConnectionAuthContextTest { + @Test + void fails_on_missing_capabilities() { + ConnectionAuthContext ctx = createConnectionAuthContext(); + assertFalse(ctx.hasCapabilities(CapabilitySet.from(Capability.CONTENT__STATUS_PAGES))); + } + + @Test + void creates_correct_error_message() { + ConnectionAuthContext ctx = createConnectionAuthContext(); + CapabilitySet requiredCaps = CapabilitySet.from(Capability.CONTENT__STATUS_PAGES); + String expectedMessage = """ + Permission denied for 'myaction' on 'myresource'. Peer 'mypeer' with Optional[[CN='myidentity']]. + Requires capabilities [vespa.content.status_pages] but peer has + [vespa.content.document_api, vespa.content.search_api, vespa.slobrok.api]. + """; + String actualMessage = ctx.createPermissionDeniedErrorMessage(requiredCaps, "myaction", "myresource", "mypeer"); + assertThat(actualMessage).isEqualToIgnoringWhitespace(expectedMessage); + } + + private static ConnectionAuthContext createConnectionAuthContext() { + return new ConnectionAuthContext( + List.of(createCertificate()), CapabilitySet.Predefined.CONTAINER_NODE.capabilities(), Set.of(), + CapabilityMode.ENFORCE); + } + + private static X509Certificate createCertificate() { + KeyPair keyPair = KeyUtils.generateKeypair(KeyAlgorithm.EC, 256); + return X509CertificateBuilder.fromKeypair( + keyPair, new X500Principal("CN=myidentity"), Instant.EPOCH, + Instant.EPOCH.plus(100000, ChronoUnit.DAYS), SHA256_WITH_ECDSA, BigInteger.ONE) + .build(); + } + + +}
\ No newline at end of file |