summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDaniil Baturin <daniil@vyos.io>2023-11-03 17:02:51 +0000
committerGitHub <noreply@github.com>2023-11-03 17:02:51 +0000
commit8391fb5f2c3db32a925f23c3df4fa08d4980c252 (patch)
tree0a219712b8a5f179433584109de6280e5e1c1d8e
parenta327526240c9d9f42930e9a1ce871e9fa8036258 (diff)
parentecdb142f3c60bd30601289a0397104eb6afd3a55 (diff)
downloadvyos-1x-8391fb5f2c3db32a925f23c3df4fa08d4980c252.tar.gz
vyos-1x-8391fb5f2c3db32a925f23c3df4fa08d4980c252.zip
Merge pull request #2433 from vyos/mergify/bp/sagitta/pr-2431
wireguard: T5707: remove previously deconfigured peer (backport #2431)
-rw-r--r--python/vyos/ifconfig/wireguard.py5
-rwxr-xr-xsmoketest/scripts/cli/test_interfaces_wireguard.py48
-rwxr-xr-xsrc/conf_mode/interfaces-wireguard.py33
3 files changed, 65 insertions, 21 deletions
diff --git a/python/vyos/ifconfig/wireguard.py b/python/vyos/ifconfig/wireguard.py
index 4aac103ec..5704f8b64 100644
--- a/python/vyos/ifconfig/wireguard.py
+++ b/python/vyos/ifconfig/wireguard.py
@@ -167,11 +167,6 @@ class WireGuardIf(Interface):
interface setup code and provide a single point of entry when workin
on any interface. """
- # remove no longer associated peers first
- if 'peer_remove' in config:
- for peer, public_key in config['peer_remove'].items():
- self._cmd(f'wg set {self.ifname} peer {public_key} remove')
-
tmp_file = NamedTemporaryFile('w')
tmp_file.write(config['private_key'])
tmp_file.flush()
diff --git a/smoketest/scripts/cli/test_interfaces_wireguard.py b/smoketest/scripts/cli/test_interfaces_wireguard.py
index 48c7cb6a1..4b994a659 100755
--- a/smoketest/scripts/cli/test_interfaces_wireguard.py
+++ b/smoketest/scripts/cli/test_interfaces_wireguard.py
@@ -20,6 +20,7 @@ import unittest
from base_vyostest_shim import VyOSUnitTestSHIM
from vyos.configsession import ConfigSessionError
from vyos.utils.file import read_file
+from vyos.utils.process import cmd
base_path = ['interfaces', 'wireguard']
@@ -152,5 +153,52 @@ class WireGuardInterfaceTest(VyOSUnitTestSHIM.TestCase):
tmp = read_file(f'/sys/class/net/{interface}/threaded')
self.assertTrue(tmp, "1")
+ def test_05_wireguard_peer_pubkey_change(self):
+ # T5707 changing WireGuard CLI public key of a peer - it's not removed
+
+ def get_peers(interface) -> list:
+ tmp = cmd(f'sudo wg show {interface} dump')
+ first_line = True
+ peers = []
+ for line in tmp.split('\n'):
+ if not line:
+ continue # Skip empty lines and last line
+ items = line.split('\t')
+ if first_line:
+ self.assertEqual(privkey, items[0])
+ first_line = False
+ else:
+ peers.append(items[0])
+ return peers
+
+
+ interface = 'wg1337'
+ port = '1337'
+ privkey = 'iJi4lb2HhkLx2KSAGOjji2alKkYsJjSPkHkrcpxgEVU='
+ pubkey_1 = 'srQ8VF6z/LDjKCzpxBzFpmaNUOeuHYzIfc2dcmoc/h4='
+ pubkey_2 = '8pbMHiQ7NECVP7F65Mb2W8+4ldGG2oaGvDSpSEsOBn8='
+
+ self.cli_set(base_path + [interface, 'address', '172.16.0.1/24'])
+ self.cli_set(base_path + [interface, 'port', port])
+ self.cli_set(base_path + [interface, 'private-key', privkey])
+
+ self.cli_set(base_path + [interface, 'peer', 'VyOS', 'public-key', pubkey_1])
+ self.cli_set(base_path + [interface, 'peer', 'VyOS', 'allowed-ips', '10.205.212.10/32'])
+
+ self.cli_commit()
+
+ peers = get_peers(interface)
+ self.assertIn(pubkey_1, peers)
+ self.assertNotIn(pubkey_2, peers)
+
+ # Now change the public key of our peer
+ self.cli_set(base_path + [interface, 'peer', 'VyOS', 'public-key', pubkey_2])
+ self.cli_commit()
+
+ # Verify config
+ peers = get_peers(interface)
+ self.assertNotIn(pubkey_1, peers)
+ self.assertIn(pubkey_2, peers)
+
if __name__ == '__main__':
unittest.main(verbosity=2)
diff --git a/src/conf_mode/interfaces-wireguard.py b/src/conf_mode/interfaces-wireguard.py
index 122d9589a..79e5d3f44 100755
--- a/src/conf_mode/interfaces-wireguard.py
+++ b/src/conf_mode/interfaces-wireguard.py
@@ -51,17 +51,9 @@ def get_config(config=None):
tmp = is_node_changed(conf, base + [ifname, 'port'])
if tmp: wireguard['port_changed'] = {}
- # Determine which Wireguard peer has been removed.
- # Peers can only be removed with their public key!
- if 'peer' in wireguard:
- peer_remove = {}
- for peer, peer_config in wireguard['peer'].items():
- # T4702: If anything on a peer changes we remove the peer first and re-add it
- if is_node_changed(conf, base + [ifname, 'peer', peer]):
- if 'public_key' in peer_config:
- peer_remove = dict_merge({'peer_remove' : {peer : peer_config['public_key']}}, peer_remove)
- if peer_remove:
- wireguard.update(peer_remove)
+ # T4702: If anything on a peer changes we remove the peer first and re-add it
+ if is_node_changed(conf, base + [ifname, 'peer']):
+ wireguard.update({'rebuild_required': {}})
return wireguard
@@ -113,12 +105,21 @@ def verify(wireguard):
public_keys.append(peer['public_key'])
def apply(wireguard):
- tmp = WireGuardIf(wireguard['ifname'])
- if 'deleted' in wireguard:
- tmp.remove()
- return None
+ if 'rebuild_required' in wireguard or 'deleted' in wireguard:
+ wg = WireGuardIf(**wireguard)
+ # WireGuard only supports peer removal based on the configured public-key,
+ # by deleting the entire interface this is the shortcut instead of parsing
+ # out all peers and removing them one by one.
+ #
+ # Peer reconfiguration will always come with a short downtime while the
+ # WireGuard interface is recreated (see below)
+ wg.remove()
+
+ # Create the new interface if required
+ if 'deleted' not in wireguard:
+ wg = WireGuardIf(**wireguard)
+ wg.update(wireguard)
- tmp.update(wireguard)
return None
if __name__ == '__main__':