wireguard: T4702: actively revoke peer if it gets disabled

When any configured peer is set to `disable` while the Wireguard tunnel is up
and running it does not get actively revoked and removed. This poses a security
risk as connections keep beeing alive.

Whenever any parameter of a peer changes we actively remove the peer and fully
recreate it on the fly.

2 files changed, 58 insertions, 59 deletions

diff --git a/python/vyos/ifconfig/wireguard.py b/python/vyos/ifconfig/wireguard.py
index 28b5e2991..9a92c71b8 100644
--- a/python/vyos/ifconfig/wireguard.py
+++ b/python/vyos/ifconfig/wireguard.py
@@ -167,12 +167,8 @@ class WireGuardIf(Interface):
 
         # remove no longer associated peers first
         if 'peer_remove' in config:
-            for tmp in config['peer_remove']:
-                peer = config['peer_remove'][tmp]
-                peer['ifname'] = config['ifname']
-
-                cmd = 'wg set {ifname} peer {public_key} remove'
-                self._cmd(cmd.format(**peer))
+            for peer, public_key in config['peer_remove'].items():
+                self._cmd(f'wg set {self.ifname} peer {public_key} remove')
 
         config['private_key_file'] = '/tmp/tmp.wireguard.key'
         with open(config['private_key_file'], 'w') as f:
@@ -187,44 +183,49 @@ class WireGuardIf(Interface):
 
         base_cmd = base_cmd.format(**config)
 
-        for tmp in config['peer']:
-            peer = config['peer'][tmp]
-
-            # start of with a fresh 'wg' command
-            cmd = base_cmd + ' peer {public_key}'
-
-            # If no PSK is given remove it by using /dev/null - passing keys via
-            # the shell (usually bash) is considered insecure, thus we use a file
-            no_psk_file = '/dev/null'
-            psk_file = no_psk_file
-            if 'preshared_key' in peer:
-                psk_file = '/tmp/tmp.wireguard.psk'
-                with open(psk_file, 'w') as f:
-                    f.write(peer['preshared_key'])
-            cmd += f' preshared-key {psk_file}'
-
-            # Persistent keepalive is optional
-            if 'persistent_keepalive'in peer:
-                cmd += ' persistent-keepalive {persistent_keepalive}'
-
-            # Multiple allowed-ip ranges can be defined - ensure we are always
-            # dealing with a list
-            if isinstance(peer['allowed_ips'], str):
-                peer['allowed_ips'] = [peer['allowed_ips']]
-            cmd += ' allowed-ips ' + ','.join(peer['allowed_ips'])
-
-            # Endpoint configuration is optional
-            if {'address', 'port'} <= set(peer):
-                if is_ipv6(peer['address']):
-                    cmd += ' endpoint [{address}]:{port}'
-                else:
-                    cmd += ' endpoint {address}:{port}'
+        if 'peer' in config:
+            for peer, peer_config in config['peer'].items():
+                # T4702: No need to configure this peer when it was explicitly
+                # marked as disabled - also active sessions are terminated as
+                # the public key was already removed when entering this method!
+                if 'disable' in peer_config:
+                    continue
+
+                # start of with a fresh 'wg' command
+                cmd = base_cmd + ' peer {public_key}'
+
+                # If no PSK is given remove it by using /dev/null - passing keys via
+                # the shell (usually bash) is considered insecure, thus we use a file
+                no_psk_file = '/dev/null'
+                psk_file = no_psk_file
+                if 'preshared_key' in peer_config:
+                    psk_file = '/tmp/tmp.wireguard.psk'
+                    with open(psk_file, 'w') as f:
+                        f.write(peer_config['preshared_key'])
+                cmd += f' preshared-key {psk_file}'
+
+                # Persistent keepalive is optional
+                if 'persistent_keepalive'in peer_config:
+                    cmd += ' persistent-keepalive {persistent_keepalive}'
+
+                # Multiple allowed-ip ranges can be defined - ensure we are always
+                # dealing with a list
+                if isinstance(peer_config['allowed_ips'], str):
+                    peer_config['allowed_ips'] = [peer_config['allowed_ips']]
+                cmd += ' allowed-ips ' + ','.join(peer_config['allowed_ips'])
+
+                # Endpoint configuration is optional
+                if {'address', 'port'} <= set(peer_config):
+                    if is_ipv6(peer_config['address']):
+                        cmd += ' endpoint [{address}]:{port}'
+                    else:
+                        cmd += ' endpoint {address}:{port}'
 
-            self._cmd(cmd.format(**peer))
+                self._cmd(cmd.format(**peer_config))
 
-            # PSK key file is not required to be stored persistently as its backed by CLI
-            if psk_file != no_psk_file and os.path.exists(psk_file):
-                os.remove(psk_file)
+                # PSK key file is not required to be stored persistently as its backed by CLI
+                if psk_file != no_psk_file and os.path.exists(psk_file):
+                    os.remove(psk_file)
 
         # call base class
         super().update(config)
diff --git a/src/conf_mode/interfaces-wireguard.py b/src/conf_mode/interfaces-wireguard.py
index 61bab2feb..8d738f55e 100755
--- a/src/conf_mode/interfaces-wireguard.py
+++ b/src/conf_mode/interfaces-wireguard.py
@@ -14,16 +14,12 @@
 # You should have received a copy of the GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-import os
-
 from sys import exit
-from copy import deepcopy
 
 from vyos.config import Config
 from vyos.configdict import dict_merge
 from vyos.configdict import get_interface_dict
-from vyos.configdict import node_changed
-from vyos.configdict import leaf_node_changed
+from vyos.configdict import is_node_changed
 from vyos.configverify import verify_vrf
 from vyos.configverify import verify_address
 from vyos.configverify import verify_bridge_delete
@@ -50,17 +46,20 @@ def get_config(config=None):
     ifname, wireguard = get_interface_dict(conf, base)
 
     # Check if a port was changed
-    wireguard['port_changed'] = leaf_node_changed(conf, base + [ifname, 'port'])
+    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!
-    dict = {}
-    tmp = node_changed(conf, base + [ifname, 'peer'], key_mangling=('-', '_'))
-    for peer in (tmp or []):
-        public_key = leaf_node_changed(conf, base + [ifname, 'peer', peer, 'public_key'])
-        if public_key:
-            dict = dict_merge({'peer_remove' : {peer : {'public_key' : public_key[0]}}}, dict)
-            wireguard.update(dict)
+    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)
 
     return wireguard
 
@@ -81,12 +80,11 @@ def verify(wireguard):
     if 'peer' not in wireguard:
         raise ConfigError('At least one Wireguard peer is required!')
 
-    if 'port' in wireguard and wireguard['port_changed']:
+    if 'port' in wireguard and 'port_changed' in wireguard:
         listen_port = int(wireguard['port'])
         if check_port_availability('0.0.0.0', listen_port, 'udp') is not True:
-            raise ConfigError(
-                f'The UDP port {listen_port} is busy or unavailable and cannot be used for the interface'
-            )
+            raise ConfigError(f'UDP port {listen_port} is busy or unavailable and '
+                               'cannot be used for the interface!')
 
     # run checks on individual configured WireGuard peer
     for tmp in wireguard['peer']: