Skip to content

Commit

Permalink
wireguard: unconditionally add NodeInternalIPs to allowed IPs
Browse files Browse the repository at this point in the history
[ upstream commit 1eb12e0 ]

Currently, we add the remote NodeInternalIPs to the list of allowed IPs
associated with a given WireGuard peer only in certain circumstances,
and more specifically when either tunneling or node to node encryption
are enabled. However, this logic doesn't practically buy us anything
in terms of additional security, but causes potential traffic disruption
in case users want to enable/disable node2node encryption in a running
cluster. Hence, let's just get rid of it, and unconditionally add
NodeInternalIPs to the list of allowed IPs.

Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
  • Loading branch information
giorio94 authored and ferozsalam committed Mar 10, 2024
1 parent 9610c09 commit 580a481
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 22 deletions.
27 changes: 9 additions & 18 deletions pkg/wireguard/agent/agent.go
Expand Up @@ -80,8 +80,7 @@ type Agent struct {
cleanup []func()

// initialized in InitLocalNodeFromWireGuard
optOut bool
requireNodesInPeerList bool
optOut bool
}

// NewAgent creates a new WireGuard Agent
Expand Down Expand Up @@ -156,10 +155,6 @@ func (a *Agent) InitLocalNodeFromWireGuard(localNode *node.LocalNode) {
}

a.optOut = localNode.OptOutNodeEncryption
a.requireNodesInPeerList = (option.Config.EncryptNode && !localNode.OptOutNodeEncryption) ||
// Enapsulated pkt is encrypted in tunneling mode. So, outer
// src/dst IP (= nodes IP) needs to be in the WG peer list.
option.Config.TunnelingEnabled()
}

func (a *Agent) initUserspaceDevice(linkMTU int) (netlink.Link, error) {
Expand Down Expand Up @@ -390,21 +385,17 @@ func (a *Agent) UpdatePeer(nodeName, pubKeyHex string, nodeIPv4, nodeIPv6 net.IP
var lookupIPv4, lookupIPv6 net.IP
if option.Config.EnableIPv4 && nodeIPv4 != nil {
lookupIPv4 = nodeIPv4
if a.requireNodesInPeerList {
allowedIPs = append(allowedIPs, net.IPNet{
IP: nodeIPv4,
Mask: net.CIDRMask(net.IPv4len*8, net.IPv4len*8),
})
}
allowedIPs = append(allowedIPs, net.IPNet{
IP: nodeIPv4,
Mask: net.CIDRMask(net.IPv4len*8, net.IPv4len*8),
})
}
if option.Config.EnableIPv6 && nodeIPv6 != nil {
lookupIPv6 = nodeIPv6
if a.requireNodesInPeerList {
allowedIPs = append(allowedIPs, net.IPNet{
IP: nodeIPv6,
Mask: net.CIDRMask(net.IPv6len*8, net.IPv6len*8),
})
}
allowedIPs = append(allowedIPs, net.IPNet{
IP: nodeIPv6,
Mask: net.CIDRMask(net.IPv6len*8, net.IPv6len*8),
})
}
allowedIPs = append(allowedIPs, a.ipCache.LookupByHostRLocked(lookupIPv4, lookupIPv6)...)
}
Expand Down
13 changes: 9 additions & 4 deletions pkg/wireguard/agent/agent_test.go
Expand Up @@ -115,7 +115,9 @@ func (a *AgentSuite) TestAgent_PeerConfig(c *C) {
c.Assert(k8s1.nodeIPv4, checker.DeepEquals, k8s1NodeIPv4)
c.Assert(k8s1.nodeIPv6, checker.DeepEquals, k8s1NodeIPv6)
c.Assert(k8s1.pubKey.String(), Equals, k8s1PubKey)
c.Assert(k8s1.allowedIPs, HasLen, 4)
c.Assert(k8s1.allowedIPs, HasLen, 6)
c.Assert(containsIP(k8s1.allowedIPs, iputil.IPToPrefix(k8s1NodeIPv4)), Equals, true)
c.Assert(containsIP(k8s1.allowedIPs, iputil.IPToPrefix(k8s1NodeIPv6)), Equals, true)
c.Assert(containsIP(k8s1.allowedIPs, pod1IPv4), Equals, true)
c.Assert(containsIP(k8s1.allowedIPs, pod1IPv6), Equals, true)
c.Assert(containsIP(k8s1.allowedIPs, pod2IPv4), Equals, true)
Expand Down Expand Up @@ -182,15 +184,19 @@ func (a *AgentSuite) TestAgent_PeerConfig(c *C) {
c.Assert(k8s1.nodeIPv4, checker.DeepEquals, k8s1NodeIPv4)
c.Assert(k8s1.nodeIPv6, checker.DeepEquals, k8s1NodeIPv6)
c.Assert(k8s1.pubKey.String(), Equals, k8s1PubKey)
c.Assert(k8s1.allowedIPs, HasLen, 2)
c.Assert(k8s1.allowedIPs, HasLen, 4)
c.Assert(containsIP(k8s1.allowedIPs, iputil.IPToPrefix(k8s1NodeIPv4)), Equals, true)
c.Assert(containsIP(k8s1.allowedIPs, iputil.IPToPrefix(k8s1NodeIPv6)), Equals, true)
c.Assert(containsIP(k8s1.allowedIPs, pod2IPv4), Equals, true)
c.Assert(containsIP(k8s1.allowedIPs, pod2IPv6), Equals, true)

k8s2 := wgAgent.peerByNodeName[k8s2NodeName]
c.Assert(k8s2.nodeIPv4, checker.DeepEquals, k8s2NodeIPv4)
c.Assert(k8s2.nodeIPv6, checker.DeepEquals, k8s2NodeIPv6)
c.Assert(k8s2.pubKey.String(), Equals, k8s2PubKey)
c.Assert(k8s2.allowedIPs, HasLen, 2)
c.Assert(k8s2.allowedIPs, HasLen, 4)
c.Assert(containsIP(k8s2.allowedIPs, iputil.IPToPrefix(k8s2NodeIPv4)), Equals, true)
c.Assert(containsIP(k8s2.allowedIPs, iputil.IPToPrefix(k8s2NodeIPv6)), Equals, true)
c.Assert(containsIP(k8s2.allowedIPs, pod3IPv4), Equals, true)
c.Assert(containsIP(k8s2.allowedIPs, pod3IPv6), Equals, true)

Expand All @@ -210,7 +216,6 @@ func (a *AgentSuite) TestAgent_PeerConfig_WithEncryptNode(c *C) {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
wgAgent, ipCache := newTestAgent(ctx)
wgAgent.requireNodesInPeerList = true
defer ipCache.Shutdown()

ipCache.Upsert(pod1IPv4Str, k8s1NodeIPv4, 0, nil, ipcache.Identity{ID: 1, Source: source.Kubernetes})
Expand Down

0 comments on commit 580a481

Please sign in to comment.