From 2dd2e2b0be165492d1609f3a84eab29b3f1d2324 Mon Sep 17 00:00:00 2001 From: Andreas Eriksen Date: Mon, 16 Jan 2023 16:53:32 +0100 Subject: Revert "Revert "open wireguard port for config servers"" --- .../admin/configserver/noderepository/Acl.java | 32 ++++-- .../noderepository/RealNodeRepository.java | 12 ++- .../noderepository/bindings/GetAclResponse.java | 7 +- .../maintenance/acl/FilterTableLineEditor.java | 2 +- .../admin/configserver/noderepository/AclTest.java | 115 +++++++++++---------- .../yahoo/vespa/hosted/provision/node/NodeAcl.java | 9 +- 6 files changed, 109 insertions(+), 68 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 87dd42d8008..311a95e1a12 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,25 +23,28 @@ import java.util.stream.Collectors; */ public class Acl { - public static final Acl EMPTY = new Acl(Set.of(), Set.of(), Set.of()); + public static final Acl EMPTY = new Acl(Set.of(), Set.of(), Set.of(), Set.of()); private final Set trustedNodes; private final Set trustedPorts; + private final Set trustedUdpPorts; private final Set trustedNetworks; /** - * @param trustedPorts Ports to trust + * @param trustedPorts TCP Ports to trust + * @param trustedUdpPorts UDP ports to trust * @param trustedNodes Nodes to trust * @param trustedNetworks Networks (in CIDR notation) to trust */ - public Acl(Set trustedPorts, Set trustedNodes, Set trustedNetworks) { + public Acl(Set trustedPorts, Set trustedUdpPorts, Set trustedNodes, Set trustedNetworks) { this.trustedNodes = copyOfNullable(trustedNodes); this.trustedPorts = copyOfNullable(trustedPorts); + this.trustedUdpPorts = copyOfNullable(trustedUdpPorts); this.trustedNetworks = copyOfNullable(trustedNetworks); } public Acl(Set trustedPorts, Set trustedNodes) { - this(trustedPorts, trustedNodes, Set.of()); + this(trustedPorts, Set.of(), trustedNodes, Set.of()); } public List toRules(IPVersion ipVersion) { @@ -66,6 +69,11 @@ 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 -> { @@ -113,8 +121,8 @@ public class Acl { return trustedPorts; } - public Set getTrustedPorts(IPVersion ipVersion) { - return trustedPorts; + public Set getTrustedUdpPorts() { + return trustedUdpPorts; } @Override @@ -124,12 +132,13 @@ 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, trustedNetworks); + return Objects.hash(trustedNodes, trustedPorts, trustedUdpPorts, trustedNetworks); } @Override @@ -137,6 +146,7 @@ public class Acl { return "Acl{" + "trustedNodes=" + trustedNodes + ", trustedPorts=" + trustedPorts + + ", trustedUdpPorts=" + trustedUdpPorts + ", trustedNetworks=" + trustedNetworks + '}'; } @@ -175,6 +185,7 @@ public class Acl { private final Set trustedNodes = new HashSet<>(); private final Set trustedPorts = new HashSet<>(); + private final Set trustedUdpPorts = new HashSet<>(); private final Set trustedNetworks = new HashSet<>(); public Builder() { } @@ -207,13 +218,18 @@ public class Acl { return this; } + public Builder withTrustedUdpPorts(Integer... ports) { + trustedUdpPorts.addAll(List.of(ports)); + return this; + } + public Builder withTrustedNetworks(Set networks) { trustedNetworks.addAll(networks); return this; } public Acl build() { - return new Acl(trustedPorts, trustedNodes, trustedNetworks); + return new Acl(trustedPorts, trustedUdpPorts, 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 36a4703a415..c15998a48df 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,6 +91,12 @@ 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> 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> trustedNodes = response.trustedNodes.stream() .collect(Collectors.groupingBy( @@ -106,12 +112,14 @@ public class RealNodeRepository implements NodeRepository { // For each hostname create an ACL - return Stream.of(trustedNodes.keySet(), trustedPorts.keySet(), trustedNetworks.keySet()) + return Stream.of(trustedNodes.keySet(), trustedPorts.keySet(), trustedUdpPorts.keySet(), trustedNetworks.keySet()) .flatMap(Set::stream) .distinct() .collect(Collectors.toMap( Function.identity(), - hostname -> new Acl(trustedPorts.get(hostname), trustedNodes.get(hostname), + hostname -> new Acl(trustedPorts.get(hostname), + trustedUdpPorts.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 08d145b3ac8..6e12d55888f 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,13 +24,18 @@ public class GetAclResponse { @JsonProperty("trustedPorts") public final List trustedPorts; + @JsonProperty("trustedUdpPorts") + public final List trustedUdpPorts; + @JsonCreator public GetAclResponse(@JsonProperty("trustedNodes") List trustedNodes, @JsonProperty("trustedNetworks") List trustedNetworks, - @JsonProperty("trustedPorts") List trustedPorts) { + @JsonProperty("trustedPorts") List trustedPorts, + @JsonProperty("trustedUdpPorts") List trustedUdpPorts) { 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 462790b8d0f..d687f959d3b 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 the wanted rules + * An editor that assumes all rules in the filter table are exactly as 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 9fbe22482ea..0b0184975a0 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(1234, 453), Set.of(4321), 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,14 +32,15 @@ public class AclTest { void no_trusted_ports() { String listRulesIpv4 = String.join("\n", aclWithoutPorts.toRules(IPVersion.IPv4)); 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 -s 192.1.2.2/32 -j ACCEPT\n" + - "-A INPUT -j REJECT --reject-with icmp-port-unreachable", + """ + -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""", listRulesIpv4); } @@ -47,15 +48,17 @@ public class AclTest { void ipv4_rules() { String listRulesIpv4 = String.join("\n", aclCommon.toRules(IPVersion.IPv4)); 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 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", + """ + -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""", listRulesIpv4); } @@ -63,23 +66,25 @@ public class AclTest { void ipv6_rules() { String listRulesIpv6 = String.join("\n", aclCommon.toRules(IPVersion.IPv6)); 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 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); + """ + -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); } @Test void ipv6_rules_stable_order() { Acl aclCommonDifferentOrder = new Acl( - Set.of(453, 1234), + Set.of(453, 1234), Set.of(4321), testNodes(Set.of(), "fe80::2", "192.1.2.2", "fb00::1", "fe80::3"), Set.of()); @@ -90,29 +95,31 @@ public class AclTest { @Test void trusted_networks() { - 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", + 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""", String.join("\n", acl.toRules(IPVersion.IPv4))); - 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", + 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""", String.join("\n", acl.toRules(IPVersion.IPv6))); } @@ -121,7 +128,7 @@ public class AclTest { Set 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), testNodes, Set.of()); + Acl acl = new Acl(Set.of(22, 4443), Set.of(), testNodes, Set.of()); assertEquals(""" -P INPUT ACCEPT -P FORWARD ACCEPT @@ -142,7 +149,7 @@ public class AclTest { Set 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), testNodes2, Set.of()); + Acl acl2 = new Acl(Set.of(22, 4443), Set.of(), 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 e61f9b79d75..1baa8086772 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,9 +26,11 @@ import java.util.stream.StreamSupport; public record NodeAcl(Node node, Set trustedNodes, Set trustedNetworks, - Set trustedPorts) { + Set trustedPorts, + Set trustedUdpPorts) { private static final Set RPC_PORTS = Set.of(19070); + private static final int WIREGUARD_PORT = 51820; public NodeAcl { Objects.requireNonNull(node, "node must be non-null"); @@ -40,6 +42,7 @@ public record NodeAcl(Node node, public static NodeAcl from(Node node, NodeList allNodes, LoadBalancers loadBalancers) { Set trustedNodes = new TreeSet<>(Comparator.comparing(TrustedNode::hostname)); Set trustedPorts = new LinkedHashSet<>(); + Set trustedUdpPorts = new LinkedHashSet<>(); Set trustedNetworks = new LinkedHashSet<>(); // For all cases below, trust: @@ -86,10 +89,12 @@ 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: @@ -109,7 +114,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); + return new NodeAcl(node, trustedNodes, trustedNetworks, trustedPorts, trustedUdpPorts); } public record TrustedNode(String hostname, NodeType type, Set ipAddresses, Set ports) { -- cgit v1.2.3