From 806fb05c1cf5781005c001e6f6444edfcc0bdb8f Mon Sep 17 00:00:00 2001 From: Ad Schellevis Date: Mon, 30 Oct 2023 18:47:12 +0100 Subject: [PATCH] net/wireguard: Some improvements in carp event handing for https://github.com/opnsense/plugins/issues/3579 This commit addresses a couple of possible issues. 1. When a sequence of carp events is being processed and these processes lock eachother, its possible that collected interface state via legacy_interfaces_details() doesn't match the active one anymore. To prevent this from happening, only fetch the wireguard interface we're interested in inside the lock. 2. To limit the number of events being handled in wg-service-control.php it's likely cleaner to push the vhid as well when we're handling carp events. This means that we should switch between server id (current parameter) and vhid by looking at its format. 3. In case the target (wg) interface doesn't exist, make sure to create it. Although in practice this shouldn't happen (as the stat file is being removed on boot), dropping an interface manually should preferably lead to a funcitonal setup anyway (otherwise it will crash trying to pull it up) 4. When a vhid is passed and affects the interface in question, log relevant information to syslog. --- net/wireguard/Makefile | 2 +- .../src/etc/rc.syshook.d/carp/20-wireguard | 2 +- .../scripts/Wireguard/wg-service-control.php | 63 +++++++++++++------ .../conf/actions.d/actions_wireguard.conf | 4 +- 4 files changed, 49 insertions(+), 22 deletions(-) diff --git a/net/wireguard/Makefile b/net/wireguard/Makefile index 5454ec51c..a7ffd83b3 100644 --- a/net/wireguard/Makefile +++ b/net/wireguard/Makefile @@ -1,6 +1,6 @@ PLUGIN_NAME= wireguard PLUGIN_VERSION= 2.4 -PLUGIN_REVISION= 1 +PLUGIN_REVISION= 2 PLUGIN_COMMENT= WireGuard VPN service kernel implementation PLUGIN_DEPENDS= wireguard-kmod PLUGIN_CONFLICTS= wireguard-go diff --git a/net/wireguard/src/etc/rc.syshook.d/carp/20-wireguard b/net/wireguard/src/etc/rc.syshook.d/carp/20-wireguard index 8c69f6f95..5e5e42d67 100755 --- a/net/wireguard/src/etc/rc.syshook.d/carp/20-wireguard +++ b/net/wireguard/src/etc/rc.syshook.d/carp/20-wireguard @@ -1,3 +1,3 @@ #!/bin/sh -configctl -dq wireguard configure +configctl -dq wireguard configure $1 diff --git a/net/wireguard/src/opnsense/scripts/Wireguard/wg-service-control.php b/net/wireguard/src/opnsense/scripts/Wireguard/wg-service-control.php index f66d0a08e..2ac781a96 100755 --- a/net/wireguard/src/opnsense/scripts/Wireguard/wg-service-control.php +++ b/net/wireguard/src/opnsense/scripts/Wireguard/wg-service-control.php @@ -49,7 +49,7 @@ function get_vhid_status() if (!empty($ifdata['carp'])) { foreach ($ifdata['carp'] as $data) { if (isset($uuids[$data['vhid']])) { - $vhids[$uuids[$data['vhid']]] = $data['status']; + $vhids[$uuids[$data['vhid']]] = ['status' => $data['status'], 'vhid' => $data['vhid']]; } } } @@ -123,7 +123,7 @@ function wg_start($server, $fhandle, $ifcfgflag = 'up') fseek($fhandle, 0); ftruncate($fhandle, 0); fwrite($fhandle, @md5_file($server->cnfFilename) . "|" . wg_reconfigure_hash($server)); - syslog(LOG_NOTICE, "Wireguard interface {$server->name} ({$server->interface}) started"); + syslog(LOG_NOTICE, "wireguard instance {$server->name} ({$server->interface}) started"); interfaces_restart_by_device(false, [(string)$server->interface], false); } @@ -135,7 +135,7 @@ function wg_stop($server) if (does_interface_exist($server->interface)) { legacy_interface_destroy($server->interface); } - syslog(LOG_NOTICE, "Wireguard interface {$server->name} ({$server->interface}) stopped"); + syslog(LOG_NOTICE, "wireguard instance {$server->name} ({$server->interface}) stopped"); } @@ -182,21 +182,29 @@ $args = array_slice($argv, $optind); openlog("wireguard", LOG_ODELAY, LOG_AUTH); if (isset($opts['h']) || empty($args) || !in_array($args[0], ['start', 'stop', 'restart', 'configure'])) { - echo "Usage: wg-service-control.php [-a] [-h] [stop|start|restart|configure] [uuid]\n\n"; + echo "Usage: wg-service-control.php [-a] [-h] [stop|start|restart|configure] [uuid|vhid]\n\n"; echo "\t-a all instances\n"; } elseif (isset($opts['a']) || !empty($args[1])) { - $server_id = $args[1] ?? null; + // either a server id (uuid) or a vhid could be offered + $server_id = $vhid = null; + if (preg_match('/^[0-9a-f]{8}-[0-9a-f]{4}-4[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/', $args[1] ?? '') == 1) { + $server_id = $args[1]; + } elseif (!empty($args[1])) { + $vhid = explode('@', $args[1])[0]; + } + $action = $args[0]; $server_devs = []; if (!empty((string)(new OPNsense\Wireguard\General())->enabled)) { - $ifdetails = legacy_interfaces_details(); $vhids = get_vhid_status(); foreach ((new OPNsense\Wireguard\Server())->servers->server->iterateItems() as $key => $node) { + $carp_depend_on = (string)$node->carp_depend_on; if (empty((string)$node->enabled)) { continue; - } - if ($server_id != null && $key != $server_id) { + } elseif ($server_id != null && $key != $server_id) { + continue; + } elseif ($vhid != null && (!empty($vhids[$carp_depend_on]) && $vhids[$carp_depend_on]['vhid'] != $vhid)) { continue; } /** @@ -206,15 +214,13 @@ if (isset($opts['h']) || empty($args) || !in_array($args[0], ['start', 'stop', ' * when in BACKUP or INIT mode. */ $carp_if_flag = 'up'; - if ( - !empty($vhids[(string)$node->carp_depend_on]) && - $vhids[(string)$node->carp_depend_on] != 'MASTER' - ) { + if (!empty($vhids[$carp_depend_on]) && $vhids[$carp_depend_on]['status'] != 'MASTER') { $carp_if_flag = 'down'; } $server_devs[] = (string)$node->interface; $statHandle = fopen($node->statFilename, "a+"); if (flock($statHandle, LOCK_EX)) { + $ifdetails = legacy_interfaces_details((string)$node->interface); switch ($action) { case 'stop': wg_stop($node); @@ -227,12 +233,34 @@ if (isset($opts['h']) || empty($args) || !in_array($args[0], ['start', 'stop', ' wg_start($node, $statHandle, $carp_if_flag); break; case 'configure': - if (@md5_file($node->cnfFilename) != get_stat_hash($statHandle)['file']) { + $ifstatus = '-'; + if (!empty($ifdetails[(string)$node->interface])) { + $ifstatus = in_array('up', $ifdetails[(string)$node->interface]['flags']) ? 'up' : 'down'; + } + + if (!empty($carp_depend_on) && !empty($vhid)) { + // CARP event traceability when a vhid is being passed + syslog( + LOG_NOTICE, + sprintf( + "Wireguard configure event instance %s (%s) vhid: %s carp: %s interface: %s", + $node->name, + $node->interface, + $vhid, + !empty($vhids[$carp_depend_on]) ? $vhids[$carp_depend_on]['status'] : '-', + $ifstatus + ) + ); + } + if ( + @md5_file($node->cnfFilename) != get_stat_hash($statHandle)['file'] || + empty($ifdetails[(string)$node->interface]) + ) { if (get_stat_hash($statHandle)['interface'] != wg_reconfigure_hash($node)) { // Fluent reloading not supported for this instance, make sure the user is informed syslog( LOG_NOTICE, - "Wireguard interface {$node->name} ({$node->interface}) " . + "wireguard instance {$node->name} ({$node->interface}) " . "can not reconfigure without stopping it first." ); wg_stop($node); @@ -240,8 +268,7 @@ if (isset($opts['h']) || empty($args) || !in_array($args[0], ['start', 'stop', ' wg_start($node, $statHandle, $carp_if_flag); } else { // when triggered via a CARP event, check our interface status [UP|DOWN] - $tmp = in_array('up', $ifdetails[(string)$node->interface]['flags']) ? 'up' : 'down'; - if ($tmp != $carp_if_flag) { + if ($ifstatus != $carp_if_flag) { mwexecf('/sbin/ifconfig %s %s', [$node->interface, $carp_if_flag]); } } @@ -254,9 +281,9 @@ if (isset($opts['h']) || empty($args) || !in_array($args[0], ['start', 'stop', ' } /** - * When -a is specified, cleaup up old or disabled instances (files and interfaces) + * When -a is specified, cleanup up old or disabled instances (files and interfaces) */ - if ($server_id == null) { + if ($server_id == null && $vhid == null) { foreach (glob('/usr/local/etc/wireguard/wg*') as $filename) { $this_dev = explode('.', basename($filename))[0]; if (!in_array($this_dev, $server_devs)) { diff --git a/net/wireguard/src/opnsense/service/conf/actions.d/actions_wireguard.conf b/net/wireguard/src/opnsense/service/conf/actions.d/actions_wireguard.conf index 44c925e86..555afb2cd 100644 --- a/net/wireguard/src/opnsense/service/conf/actions.d/actions_wireguard.conf +++ b/net/wireguard/src/opnsense/service/conf/actions.d/actions_wireguard.conf @@ -18,9 +18,9 @@ message: restart wireguard instance %s [configure] command:/usr/local/opnsense/scripts/Wireguard/wg-service-control.php -parameters: -a configure +parameters: -a configure %s type:script -message: configure wireguard instances +message: configure wireguard instances (%s) [renew] command:/usr/local/opnsense/scripts/Wireguard/reresolve-dns.py