Merge pull request #26153 from vespa-engine/revert-26145-bjorncs/capabilities

Revert "Store original capability (set) names from JSON config in PeerPolicy" MERGEOK

6 files changed, 25 insertions, 72 deletions

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 b7cd03b49bb..010b8a5b228 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
@@ -1,16 +1,17 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.security.tls;
 
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.EnumSet;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
-import java.util.Optional;
 import java.util.Set;
+import java.util.SortedSet;
+import java.util.TreeSet;
 import java.util.logging.Logger;
 import java.util.stream.Collectors;
 
@@ -32,10 +33,10 @@ public class CapabilitySet implements ToCapabilitySet {
             Capability.CONTENT__STATUS_PAGES, Capability.CONTENT__METRICS_API, Capability.CONTAINER__STATE_API,
             Capability.METRICSPROXY__METRICS_API, Capability.SENTINEL__CONNECTIVITY_CHECK);
 
-    private static final CapabilitySet SHARED_CAPABILITIES_APP_NODE = CapabilitySet.unionOf(List.of(
+    private static final CapabilitySet SHARED_CAPABILITIES_APP_NODE = CapabilitySet.of(
             Capability.LOGSERVER_API, Capability.CONFIGSERVER__CONFIG_API,
             Capability.CONFIGSERVER__FILEDISTRIBUTION_API, Capability.CONFIGPROXY__CONFIG_API,
-            Capability.CONFIGPROXY__FILEDISTRIBUTION_API, Capability.SLOBROK__API, TELEMETRY));
+            Capability.CONFIGPROXY__FILEDISTRIBUTION_API, Capability.SLOBROK__API, TELEMETRY);
 
     public static final CapabilitySet CONTENT_NODE = predefined(
             "vespa.content_node",
@@ -58,7 +59,7 @@ public class CapabilitySet implements ToCapabilitySet {
             TELEMETRY);
 
     private static CapabilitySet predefined(String name, ToCapabilitySet... capabilities) {
-        var instance = CapabilitySet.unionOf(List.of(capabilities));
+        var instance = CapabilitySet.of(capabilities);
         PREDEFINED.put(name, instance);
         return instance;
     }
@@ -84,14 +85,14 @@ public class CapabilitySet implements ToCapabilitySet {
         return new CapabilitySet(caps);
     }
 
-    public static CapabilitySet ofSets(Collection<CapabilitySet> capSets) {
+    public static CapabilitySet unionOf(Collection<CapabilitySet> capSets) {
         EnumSet<Capability> union = EnumSet.noneOf(Capability.class);
         capSets.forEach(cs -> union.addAll(cs.caps));
         return new CapabilitySet(union);
     }
 
-    public static CapabilitySet unionOf(Collection<ToCapabilitySet> caps) {
-        return CapabilitySet.ofSets(caps.stream().map(ToCapabilitySet::toCapabilitySet).toList());
+    public static CapabilitySet of(ToCapabilitySet... capabilities) {
+        return CapabilitySet.unionOf(Arrays.stream(capabilities).map(ToCapabilitySet::toCapabilitySet).toList());
     }
 
     public static CapabilitySet of(EnumSet<Capability> caps) { return new CapabilitySet(EnumSet.copyOf(caps)); }
@@ -106,33 +107,8 @@ public class CapabilitySet implements ToCapabilitySet {
     public boolean has(Collection<Capability> caps) { return this.caps.containsAll(caps); }
     public boolean has(Capability... caps) {  return this.caps.containsAll(List.of(caps)); }
 
-    public Set<String> toCapabilityNames() {
-        return caps.stream().map(Capability::asString).collect(Collectors.toSet());
-    }
-
-    /** return name of the capability set if predefined, otherwise names of the individual capabilities */
-    public Set<String> resolveNames() {
-        var predefinedName = toPredefinedName().orElse(null);
-        if (predefinedName != null) return Set.of(predefinedName);
-        return toCapabilityNames();
-    }
-
-    /** @return the name if this is a predefined capability set, or empty if not */
-    public Optional<String> toPredefinedName() {
-        return PREDEFINED.entrySet().stream()
-            .filter(e -> e.getValue().equals(this))
-            .map(Map.Entry::getKey)
-            .findFirst();
-    }
-
-    public static Set<String> resolveNames(Collection<ToCapabilitySet> capabilities) {
-        var names = new HashSet<String>();
-        for (ToCapabilitySet tcs : capabilities) {
-            if (tcs instanceof Capability c) names.add(c.asString());
-            else if (tcs instanceof CapabilitySet cs) names.addAll(cs.resolveNames());
-            else throw new IllegalArgumentException(tcs.toString());
-        }
-        return Set.copyOf(names);
+    public SortedSet<String> toNames() {
+        return caps.stream().map(Capability::asString).collect(Collectors.toCollection(TreeSet::new));
     }
 
     public Set<Capability> asSet() { return Collections.unmodifiableSet(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 9252b5619f9..d7ea93955af 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
@@ -8,7 +8,6 @@ import java.util.List;
 import java.util.Optional;
 import java.util.Set;
 import java.util.logging.Logger;
-import java.util.stream.Collectors;
 
 import static com.yahoo.security.SubjectAlternativeName.Type.DNS;
 import static com.yahoo.security.SubjectAlternativeName.Type.URI;
@@ -79,14 +78,10 @@ public record ConnectionAuthContext(List<X509Certificate> peerCertificateChain,
         b.append(". Peer ");
         if (peer != null) b.append("'").append(peer).append("' ");
         return b.append("with ").append(peerCertificateString().orElse("<missing-certificate>")).append(". Requires capabilities ")
-                .append(toCapabilityNames(required)).append(" but peer has ").append(toCapabilityNames(capabilities))
+                .append(required.toNames()).append(" but peer has ").append(capabilities.toNames())
                 .append(".").toString();
     }
 
-    private static String toCapabilityNames(CapabilitySet capabilities) {
-        return capabilities.toCapabilityNames().stream().sorted().collect(Collectors.joining(", ", "[", "]"));
-    }
-
     public Optional<X509Certificate> peerCertificate() {
         return peerCertificateChain.isEmpty() ? Optional.empty() : Optional.of(peerCertificateChain.get(0));
     }
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 d0e1a33fcac..746fce0e290 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
@@ -49,7 +49,7 @@ public class PeerAuthorizer {
         // TODO Pass this through constructor
         CapabilityMode capabilityMode = TransportSecurityUtils.getCapabilityMode();
         return new ConnectionAuthContext(
-                certChain, CapabilitySet.ofSets(grantedCapabilities), matchedPolicies, capabilityMode);
+                certChain, CapabilitySet.unionOf(grantedCapabilities), matchedPolicies, capabilityMode);
     }
 
     private static boolean matchesPolicy(PeerPolicy peerPolicy, String cn, List<String> sans) {
diff --git a/security-utils/src/main/java/com/yahoo/security/tls/PeerPolicy.java b/security-utils/src/main/java/com/yahoo/security/tls/PeerPolicy.java
index f713bcb0b08..ea3d4cfe002 100644
--- a/security-utils/src/main/java/com/yahoo/security/tls/PeerPolicy.java
+++ b/security-utils/src/main/java/com/yahoo/security/tls/PeerPolicy.java
@@ -1,25 +1,17 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.security.tls;
 
-import java.util.Collection;
 import java.util.List;
 import java.util.Optional;
-import java.util.Set;
 
 /**
  * @author bjorncs
  */
-public record PeerPolicy(String policyName, Optional<String> description, Set<String> capabilityNames,
-                         CapabilitySet capabilities, List<RequiredPeerCredential> requiredCredentials) {
+public record PeerPolicy(String policyName, Optional<String> description, CapabilitySet capabilities,
+                         List<RequiredPeerCredential> requiredCredentials) {
 
     public PeerPolicy {
         requiredCredentials = List.copyOf(requiredCredentials);
-        capabilityNames = Set.copyOf(capabilityNames);
-    }
-
-    public PeerPolicy(String policyName, Optional<String> description,
-                      CapabilitySet capabilities, List<RequiredPeerCredential> requiredCredentials) {
-        this(policyName, description, capabilities.resolveNames(), capabilities, requiredCredentials);
     }
 
     public PeerPolicy(String policyName, List<RequiredPeerCredential> requiredCredentials) {
@@ -29,16 +21,4 @@ public record PeerPolicy(String policyName, Optional<String> description, Set<St
     public PeerPolicy(String policyName, String description, List<RequiredPeerCredential> requiredCredentials) {
         this(policyName, Optional.ofNullable(description), CapabilitySet.all(), requiredCredentials);
     }
-
-    public PeerPolicy(String policyName, Optional<String> description, Collection<ToCapabilitySet> capabilities,
-                      List<RequiredPeerCredential> requiredCredentials) {
-        this(policyName, description, CapabilitySet.resolveNames(capabilities),
-             CapabilitySet.unionOf(capabilities), requiredCredentials);
-    }
-
-    public PeerPolicy(String policyName, Optional<String> description, Set<String> capabilities,
-                      List<RequiredPeerCredential> requiredCredentials) {
-        this(policyName, description, capabilities, CapabilitySet.fromNames(capabilities),
-             requiredCredentials);
-    }
 }
diff --git a/security-utils/src/main/java/com/yahoo/security/tls/TransportSecurityOptionsJsonSerializer.java b/security-utils/src/main/java/com/yahoo/security/tls/TransportSecurityOptionsJsonSerializer.java
index 66b90b32f79..34626e23e7a 100644
--- a/security-utils/src/main/java/com/yahoo/security/tls/TransportSecurityOptionsJsonSerializer.java
+++ b/security-utils/src/main/java/com/yahoo/security/tls/TransportSecurityOptionsJsonSerializer.java
@@ -96,15 +96,15 @@ class TransportSecurityOptionsJsonSerializer {
             throw missingFieldException("required-credentials");
         }
         return new PeerPolicy(authorizedPeer.name, Optional.ofNullable(authorizedPeer.description),
-                              toCapabilities(authorizedPeer.capabilities), toRequestPeerCredentials(authorizedPeer.requiredCredentials));
+                toCapabilities(authorizedPeer.capabilities), toRequestPeerCredentials(authorizedPeer.requiredCredentials));
     }
 
-    private static Set<String> toCapabilities(List<String> capabilities) {
-        if (capabilities == null) return Set.of(CapabilitySet.ALL.toPredefinedName().get());
+    private static CapabilitySet toCapabilities(List<String> capabilities) {
+        if (capabilities == null) return CapabilitySet.all();
         if (capabilities.isEmpty())
             throw new IllegalArgumentException("\"capabilities\" array must either be not present " +
                     "(implies all capabilities) or contain at least one capability name");
-        return Set.copyOf(capabilities);
+        return CapabilitySet.fromNames(capabilities);
     }
 
     private static List<RequiredPeerCredential> toRequestPeerCredentials(List<RequiredCredential> requiredCredentials) {
@@ -148,7 +148,7 @@ class TransportSecurityOptionsJsonSerializer {
                     authorizedPeer.description = peerPolicy.description().orElse(null);
                     CapabilitySet caps = peerPolicy.capabilities();
                     if (!caps.hasAll()) {
-                        authorizedPeer.capabilities = peerPolicy.capabilityNames().stream().sorted().toList();
+                        authorizedPeer.capabilities = List.copyOf(caps.toNames());
                     }
                     for (RequiredPeerCredential requiredPeerCredential : peerPolicy.requiredCredentials()) {
                         RequiredCredential requiredCredential = new RequiredCredential();
diff --git a/security-utils/src/test/java/com/yahoo/security/tls/CapabilitySetTest.java b/security-utils/src/test/java/com/yahoo/security/tls/CapabilitySetTest.java
index 3fa75df27e1..87b16dbff1f 100644
--- a/security-utils/src/test/java/com/yahoo/security/tls/CapabilitySetTest.java
+++ b/security-utils/src/test/java/com/yahoo/security/tls/CapabilitySetTest.java
@@ -4,6 +4,8 @@ package com.yahoo.security.tls;
 import org.junit.jupiter.api.Test;
 
 import java.util.Arrays;
+import java.util.SortedSet;
+import java.util.TreeSet;
 import java.util.stream.Collectors;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -15,10 +17,10 @@ class CapabilitySetTest {
 
     @Test
     void contains_all_capabilities() {
-        var expectedNames = Arrays.stream(Capability.values())
+        SortedSet<String> expectedNames = Arrays.stream(Capability.values())
                 .map(Capability::asString)
-                .collect(Collectors.toSet());
-        var actualNames = CapabilitySet.all().toCapabilityNames();
+                .collect(Collectors.toCollection(TreeSet::new));
+        SortedSet<String> actualNames = CapabilitySet.all().toNames();
         assertEquals(expectedNames, actualNames);
     }