diff options
author | Jon Bratseth <bratseth@gmail.com> | 2023-01-16 16:26:16 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-01-16 16:26:16 +0100 |
commit | d47768eb2c125135f0add87756ef0cb773cf58c3 (patch) | |
tree | 7664a780d0e66281513d8fb21edec7278d5c2f23 | |
parent | 2ee6905f0c6535fe95cc0516e4634f3ac37414b2 (diff) | |
parent | 529acb49e1369a4bf1842cd7e84de91caf66b769 (diff) |
Merge pull request #25588 from vespa-engine/revert-25586-andreer/wg-wip-3
Revert "open wireguard port for config servers"
6 files changed, 68 insertions, 109 deletions
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/Acl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/Acl.java index 311a95e1a12..87dd42d8008 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/Acl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/Acl.java @@ -23,28 +23,25 @@ import java.util.stream.Collectors; */ public class Acl { - public static final Acl EMPTY = new Acl(Set.of(), Set.of(), Set.of(), Set.of()); + public static final Acl EMPTY = new Acl(Set.of(), Set.of(), Set.of()); private final Set<Node> trustedNodes; private final Set<Integer> trustedPorts; - private final Set<Integer> trustedUdpPorts; private final Set<String> trustedNetworks; /** - * @param trustedPorts TCP Ports to trust - * @param trustedUdpPorts UDP ports to trust + * @param trustedPorts Ports to trust * @param trustedNodes Nodes to trust * @param trustedNetworks Networks (in CIDR notation) to trust */ - public Acl(Set<Integer> trustedPorts, Set<Integer> trustedUdpPorts, Set<Node> trustedNodes, Set<String> trustedNetworks) { + public Acl(Set<Integer> trustedPorts, Set<Node> trustedNodes, Set<String> trustedNetworks) { this.trustedNodes = copyOfNullable(trustedNodes); this.trustedPorts = copyOfNullable(trustedPorts); - this.trustedUdpPorts = copyOfNullable(trustedUdpPorts); this.trustedNetworks = copyOfNullable(trustedNetworks); } public Acl(Set<Integer> trustedPorts, Set<Node> trustedNodes) { - this(trustedPorts, Set.of(), trustedNodes, Set.of()); + this(trustedPorts, trustedNodes, Set.of()); } public List<String> toRules(IPVersion ipVersion) { @@ -69,11 +66,6 @@ public class Acl { rules.add("-A INPUT -p tcp -m multiport --dports " + joinPorts(trustedPorts) + " -j ACCEPT"); } - // Allow trusted UDP ports if any - if (!trustedUdpPorts.isEmpty()) { - rules.add("-A INPUT -p udp -m multiport --dports " + joinPorts(trustedUdpPorts) + " -j ACCEPT"); - } - // Allow traffic from trusted nodes, limited to specific ports, if any getTrustedNodes(ipVersion).stream() .map(node -> { @@ -121,8 +113,8 @@ public class Acl { return trustedPorts; } - public Set<Integer> getTrustedUdpPorts() { - return trustedUdpPorts; + public Set<Integer> getTrustedPorts(IPVersion ipVersion) { + return trustedPorts; } @Override @@ -132,13 +124,12 @@ public class Acl { Acl acl = (Acl) o; return trustedNodes.equals(acl.trustedNodes) && trustedPorts.equals(acl.trustedPorts) && - trustedUdpPorts.equals(acl.trustedUdpPorts) && trustedNetworks.equals(acl.trustedNetworks); } @Override public int hashCode() { - return Objects.hash(trustedNodes, trustedPorts, trustedUdpPorts, trustedNetworks); + return Objects.hash(trustedNodes, trustedPorts, trustedNetworks); } @Override @@ -146,7 +137,6 @@ public class Acl { return "Acl{" + "trustedNodes=" + trustedNodes + ", trustedPorts=" + trustedPorts + - ", trustedUdpPorts=" + trustedUdpPorts + ", trustedNetworks=" + trustedNetworks + '}'; } @@ -185,7 +175,6 @@ public class Acl { private final Set<Node> trustedNodes = new HashSet<>(); private final Set<Integer> trustedPorts = new HashSet<>(); - private final Set<Integer> trustedUdpPorts = new HashSet<>(); private final Set<String> trustedNetworks = new HashSet<>(); public Builder() { } @@ -218,18 +207,13 @@ public class Acl { return this; } - public Builder withTrustedUdpPorts(Integer... ports) { - trustedUdpPorts.addAll(List.of(ports)); - return this; - } - public Builder withTrustedNetworks(Set<String> networks) { trustedNetworks.addAll(networks); return this; } public Acl build() { - return new Acl(trustedPorts, trustedUdpPorts, trustedNodes, trustedNetworks); + return new Acl(trustedPorts, trustedNodes, trustedNetworks); } } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java index c15998a48df..36a4703a415 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/RealNodeRepository.java @@ -91,12 +91,6 @@ public class RealNodeRepository implements NodeRepository { GetAclResponse.Port::getTrustedBy, Collectors.mapping(port -> port.port, Collectors.toSet()))); - // Group UDP ports by container hostname that trusts them - Map<String, Set<Integer>> trustedUdpPorts = response.trustedUdpPorts.stream() - .collect(Collectors.groupingBy( - GetAclResponse.Port::getTrustedBy, - Collectors.mapping(port -> port.port, Collectors.toSet()))); - // Group node ip-addresses by container hostname that trusts them Map<String, Set<Acl.Node>> trustedNodes = response.trustedNodes.stream() .collect(Collectors.groupingBy( @@ -112,14 +106,12 @@ public class RealNodeRepository implements NodeRepository { // For each hostname create an ACL - return Stream.of(trustedNodes.keySet(), trustedPorts.keySet(), trustedUdpPorts.keySet(), trustedNetworks.keySet()) + return Stream.of(trustedNodes.keySet(), trustedPorts.keySet(), trustedNetworks.keySet()) .flatMap(Set::stream) .distinct() .collect(Collectors.toMap( Function.identity(), - hostname -> new Acl(trustedPorts.get(hostname), - trustedUdpPorts.get(hostname), - trustedNodes.get(hostname), + hostname -> new Acl(trustedPorts.get(hostname), trustedNodes.get(hostname), trustedNetworks.get(hostname)))); } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/bindings/GetAclResponse.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/bindings/GetAclResponse.java index 6e12d55888f..08d145b3ac8 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/bindings/GetAclResponse.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/bindings/GetAclResponse.java @@ -24,18 +24,13 @@ public class GetAclResponse { @JsonProperty("trustedPorts") public final List<Port> trustedPorts; - @JsonProperty("trustedUdpPorts") - public final List<Port> trustedUdpPorts; - @JsonCreator public GetAclResponse(@JsonProperty("trustedNodes") List<Node> trustedNodes, @JsonProperty("trustedNetworks") List<Network> trustedNetworks, - @JsonProperty("trustedPorts") List<Port> trustedPorts, - @JsonProperty("trustedUdpPorts") List<Port> trustedUdpPorts) { + @JsonProperty("trustedPorts") List<Port> trustedPorts) { this.trustedNodes = trustedNodes == null ? List.of() : List.copyOf(trustedNodes); this.trustedNetworks = trustedNetworks == null ? List.of() : List.copyOf(trustedNetworks); this.trustedPorts = trustedPorts == null ? List.of() : List.copyOf(trustedPorts); - this.trustedUdpPorts = trustedUdpPorts == null ? List.of() : List.copyOf(trustedUdpPorts); } @JsonIgnoreProperties(ignoreUnknown = true) diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/FilterTableLineEditor.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/FilterTableLineEditor.java index d687f959d3b..462790b8d0f 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/FilterTableLineEditor.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/acl/FilterTableLineEditor.java @@ -9,7 +9,7 @@ import com.yahoo.vespa.hosted.node.admin.task.util.network.IPVersion; import java.util.List; /** - * An editor that assumes all rules in the filter table are exactly as the wanted rules + * An editor that assumes all rules in the filter table are exactly as the the wanted rules * * @author smorgrav */ diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/AclTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/AclTest.java index 0b0184975a0..9fbe22482ea 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/AclTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/configserver/noderepository/AclTest.java @@ -19,12 +19,12 @@ import static org.junit.jupiter.api.Assertions.assertEquals; public class AclTest { private static final Acl aclCommon = new Acl( - Set.of(1234, 453), Set.of(4321), + Set.of(1234, 453), testNodes(Set.of(), "192.1.2.2", "fb00::1", "fe80::2", "fe80::3"), Set.of()); private static final Acl aclWithoutPorts = new Acl( - Set.of(), Set.of(), + Set.of(), testNodes(Set.of(), "192.1.2.2", "fb00::1", "fe80::2"), Set.of()); @@ -32,15 +32,14 @@ public class AclTest { void no_trusted_ports() { String listRulesIpv4 = String.join("\n", aclWithoutPorts.toRules(IPVersion.IPv4)); assertEquals( - """ - -P INPUT ACCEPT - -P FORWARD ACCEPT - -P OUTPUT ACCEPT - -A INPUT -m state --state RELATED,ESTABLISHED -j ACCEPT - -A INPUT -i lo -j ACCEPT - -A INPUT -p icmp -j ACCEPT - -A INPUT -s 192.1.2.2/32 -j ACCEPT - -A INPUT -j REJECT --reject-with icmp-port-unreachable""", + "-P INPUT ACCEPT\n" + + "-P FORWARD ACCEPT\n" + + "-P OUTPUT ACCEPT\n" + + "-A INPUT -m state --state RELATED,ESTABLISHED -j ACCEPT\n" + + "-A INPUT -i lo -j ACCEPT\n" + + "-A INPUT -p icmp -j ACCEPT\n" + + "-A INPUT -s 192.1.2.2/32 -j ACCEPT\n" + + "-A INPUT -j REJECT --reject-with icmp-port-unreachable", listRulesIpv4); } @@ -48,17 +47,15 @@ public class AclTest { void ipv4_rules() { String listRulesIpv4 = String.join("\n", aclCommon.toRules(IPVersion.IPv4)); assertEquals( - """ - -P INPUT ACCEPT - -P FORWARD ACCEPT - -P OUTPUT ACCEPT - -A INPUT -m state --state RELATED,ESTABLISHED -j ACCEPT - -A INPUT -i lo -j ACCEPT - -A INPUT -p icmp -j ACCEPT - -A INPUT -p tcp -m multiport --dports 453,1234 -j ACCEPT - -A INPUT -p udp -m multiport --dports 4321 -j ACCEPT - -A INPUT -s 192.1.2.2/32 -j ACCEPT - -A INPUT -j REJECT --reject-with icmp-port-unreachable""", + "-P INPUT ACCEPT\n" + + "-P FORWARD ACCEPT\n" + + "-P OUTPUT ACCEPT\n" + + "-A INPUT -m state --state RELATED,ESTABLISHED -j ACCEPT\n" + + "-A INPUT -i lo -j ACCEPT\n" + + "-A INPUT -p icmp -j ACCEPT\n" + + "-A INPUT -p tcp -m multiport --dports 453,1234 -j ACCEPT\n" + + "-A INPUT -s 192.1.2.2/32 -j ACCEPT\n" + + "-A INPUT -j REJECT --reject-with icmp-port-unreachable", listRulesIpv4); } @@ -66,25 +63,23 @@ public class AclTest { void ipv6_rules() { String listRulesIpv6 = String.join("\n", aclCommon.toRules(IPVersion.IPv6)); assertEquals( - """ - -P INPUT ACCEPT - -P FORWARD ACCEPT - -P OUTPUT ACCEPT - -A INPUT -m state --state RELATED,ESTABLISHED -j ACCEPT - -A INPUT -i lo -j ACCEPT - -A INPUT -p ipv6-icmp -j ACCEPT - -A INPUT -p tcp -m multiport --dports 453,1234 -j ACCEPT - -A INPUT -p udp -m multiport --dports 4321 -j ACCEPT - -A INPUT -s fb00::1/128 -j ACCEPT - -A INPUT -s fe80::2/128 -j ACCEPT - -A INPUT -s fe80::3/128 -j ACCEPT - -A INPUT -j REJECT --reject-with icmp6-port-unreachable""", listRulesIpv6); + "-P INPUT ACCEPT\n" + + "-P FORWARD ACCEPT\n" + + "-P OUTPUT ACCEPT\n" + + "-A INPUT -m state --state RELATED,ESTABLISHED -j ACCEPT\n" + + "-A INPUT -i lo -j ACCEPT\n" + + "-A INPUT -p ipv6-icmp -j ACCEPT\n" + + "-A INPUT -p tcp -m multiport --dports 453,1234 -j ACCEPT\n" + + "-A INPUT -s fb00::1/128 -j ACCEPT\n" + + "-A INPUT -s fe80::2/128 -j ACCEPT\n" + + "-A INPUT -s fe80::3/128 -j ACCEPT\n" + + "-A INPUT -j REJECT --reject-with icmp6-port-unreachable", listRulesIpv6); } @Test void ipv6_rules_stable_order() { Acl aclCommonDifferentOrder = new Acl( - Set.of(453, 1234), Set.of(4321), + Set.of(453, 1234), testNodes(Set.of(), "fe80::2", "192.1.2.2", "fb00::1", "fe80::3"), Set.of()); @@ -95,31 +90,29 @@ public class AclTest { @Test void trusted_networks() { - Acl acl = new Acl(Set.of(4080), Set.of(), testNodes(Set.of(), "127.0.0.1"), Set.of("10.0.0.0/24", "2001:db8::/32")); - - assertEquals(""" - -P INPUT ACCEPT - -P FORWARD ACCEPT - -P OUTPUT ACCEPT - -A INPUT -m state --state RELATED,ESTABLISHED -j ACCEPT - -A INPUT -i lo -j ACCEPT - -A INPUT -p icmp -j ACCEPT - -A INPUT -p tcp -m multiport --dports 4080 -j ACCEPT - -A INPUT -s 127.0.0.1/32 -j ACCEPT - -A INPUT -s 10.0.0.0/24 -j ACCEPT - -A INPUT -j REJECT --reject-with icmp-port-unreachable""", + Acl acl = new Acl(Set.of(4080), testNodes(Set.of(), "127.0.0.1"), Set.of("10.0.0.0/24", "2001:db8::/32")); + + assertEquals("-P INPUT ACCEPT\n" + + "-P FORWARD ACCEPT\n" + + "-P OUTPUT ACCEPT\n" + + "-A INPUT -m state --state RELATED,ESTABLISHED -j ACCEPT\n" + + "-A INPUT -i lo -j ACCEPT\n" + + "-A INPUT -p icmp -j ACCEPT\n" + + "-A INPUT -p tcp -m multiport --dports 4080 -j ACCEPT\n" + + "-A INPUT -s 127.0.0.1/32 -j ACCEPT\n" + + "-A INPUT -s 10.0.0.0/24 -j ACCEPT\n" + + "-A INPUT -j REJECT --reject-with icmp-port-unreachable", String.join("\n", acl.toRules(IPVersion.IPv4))); - assertEquals(""" - -P INPUT ACCEPT - -P FORWARD ACCEPT - -P OUTPUT ACCEPT - -A INPUT -m state --state RELATED,ESTABLISHED -j ACCEPT - -A INPUT -i lo -j ACCEPT - -A INPUT -p ipv6-icmp -j ACCEPT - -A INPUT -p tcp -m multiport --dports 4080 -j ACCEPT - -A INPUT -s 2001:db8::/32 -j ACCEPT - -A INPUT -j REJECT --reject-with icmp6-port-unreachable""", + assertEquals("-P INPUT ACCEPT\n" + + "-P FORWARD ACCEPT\n" + + "-P OUTPUT ACCEPT\n" + + "-A INPUT -m state --state RELATED,ESTABLISHED -j ACCEPT\n" + + "-A INPUT -i lo -j ACCEPT\n" + + "-A INPUT -p ipv6-icmp -j ACCEPT\n" + + "-A INPUT -p tcp -m multiport --dports 4080 -j ACCEPT\n" + + "-A INPUT -s 2001:db8::/32 -j ACCEPT\n" + + "-A INPUT -j REJECT --reject-with icmp6-port-unreachable", String.join("\n", acl.toRules(IPVersion.IPv6))); } @@ -128,7 +121,7 @@ public class AclTest { Set<Acl.Node> testNodes = Stream.concat(testNodes(NodeType.config, Set.of(), "172.17.0.41", "172.17.0.42", "172.17.0.43").stream(), testNodes(NodeType.tenant, Set.of(19070), "172.17.0.81", "172.17.0.82", "172.17.0.83").stream()) .collect(Collectors.toSet()); - Acl acl = new Acl(Set.of(22, 4443), Set.of(), testNodes, Set.of()); + Acl acl = new Acl(Set.of(22, 4443), testNodes, Set.of()); assertEquals(""" -P INPUT ACCEPT -P FORWARD ACCEPT @@ -149,7 +142,7 @@ public class AclTest { Set<Acl.Node> testNodes2 = Stream.concat(testNodes(NodeType.config, Set.of(), "2001:db8::41", "2001:db8::42", "2001:db8::43").stream(), testNodes(NodeType.tenant, Set.of(19070), "2001:db8::81", "2001:db8::82", "2001:db8::83").stream()) .collect(Collectors.toSet()); - Acl acl2 = new Acl(Set.of(22, 4443), Set.of(), testNodes2, Set.of()); + Acl acl2 = new Acl(Set.of(22, 4443), testNodes2, Set.of()); assertEquals(""" -P INPUT ACCEPT diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/NodeAcl.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/NodeAcl.java index 1baa8086772..e61f9b79d75 100644 --- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/NodeAcl.java +++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/node/NodeAcl.java @@ -26,11 +26,9 @@ import java.util.stream.StreamSupport; public record NodeAcl(Node node, Set<TrustedNode> trustedNodes, Set<String> trustedNetworks, - Set<Integer> trustedPorts, - Set<Integer> trustedUdpPorts) { + Set<Integer> trustedPorts) { private static final Set<Integer> RPC_PORTS = Set.of(19070); - private static final int WIREGUARD_PORT = 51820; public NodeAcl { Objects.requireNonNull(node, "node must be non-null"); @@ -42,7 +40,6 @@ public record NodeAcl(Node node, public static NodeAcl from(Node node, NodeList allNodes, LoadBalancers loadBalancers) { Set<TrustedNode> trustedNodes = new TreeSet<>(Comparator.comparing(TrustedNode::hostname)); Set<Integer> trustedPorts = new LinkedHashSet<>(); - Set<Integer> trustedUdpPorts = new LinkedHashSet<>(); Set<String> trustedNetworks = new LinkedHashSet<>(); // For all cases below, trust: @@ -89,12 +86,10 @@ public record NodeAcl(Node node, // - port 19070 (RPC) from all tenant nodes (and their hosts, in case traffic is NAT-ed via parent) // - port 19070 (RPC) from all proxy nodes (and their hosts, in case traffic is NAT-ed via parent) // - port 4443 from the world - // - udp port 51820 from the world trustedNodes.addAll(TrustedNode.of(allNodes.nodeType(NodeType.host, NodeType.tenant, NodeType.proxyhost, NodeType.proxy), RPC_PORTS)); trustedPorts.add(4443); - trustedUdpPorts.add(WIREGUARD_PORT); } case proxy -> { // Proxy nodes trust: @@ -114,7 +109,7 @@ public record NodeAcl(Node node, default -> throw new IllegalArgumentException("Don't know how to create ACL for " + node + " of type " + node.type()); } - return new NodeAcl(node, trustedNodes, trustedNetworks, trustedPorts, trustedUdpPorts); + return new NodeAcl(node, trustedNodes, trustedNetworks, trustedPorts); } public record TrustedNode(String hostname, NodeType type, Set<String> ipAddresses, Set<Integer> ports) { |