Skip to content

Commit b966ab6

Browse files
committed
VPN: WireGuard - Some improvements in carp event handing for opnsense/plugins#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.
1 parent a108d60 commit b966ab6

File tree

3 files changed

+45
-18
lines changed

3 files changed

+45
-18
lines changed
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
#!/bin/sh
22

3-
configctl -dq wireguard configure
3+
configctl -dq wireguard configure $1

src/opnsense/scripts/Wireguard/wg-service-control.php

Lines changed: 42 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ function get_vhid_status()
4949
if (!empty($ifdata['carp'])) {
5050
foreach ($ifdata['carp'] as $data) {
5151
if (isset($uuids[$data['vhid']])) {
52-
$vhids[$uuids[$data['vhid']]] = $data['status'];
52+
$vhids[$uuids[$data['vhid']]] = ['status' => $data['status'], 'vhid' => $data['vhid']];
5353
}
5454
}
5555
}
@@ -182,21 +182,29 @@ function get_stat_hash($fhandle)
182182
openlog("wireguard", LOG_ODELAY, LOG_AUTH);
183183

184184
if (isset($opts['h']) || empty($args) || !in_array($args[0], ['start', 'stop', 'restart', 'configure'])) {
185-
echo "Usage: wg-service-control.php [-a] [-h] [stop|start|restart|configure] [uuid]\n\n";
185+
echo "Usage: wg-service-control.php [-a] [-h] [stop|start|restart|configure] [uuid|vhid]\n\n";
186186
echo "\t-a all instances\n";
187187
} elseif (isset($opts['a']) || !empty($args[1])) {
188-
$server_id = $args[1] ?? null;
188+
// either a server id (uuid) or a vhid could be offered
189+
$server_id = $vhid = null;
190+
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) {
191+
$server_id = $args[1];
192+
} elseif (!empty($args[1])) {
193+
$vhid = explode('@', $args[1])[0];
194+
}
195+
189196
$action = $args[0];
190197

191198
$server_devs = [];
192199
if (!empty((string)(new OPNsense\Wireguard\General())->enabled)) {
193-
$ifdetails = legacy_interfaces_details();
194200
$vhids = get_vhid_status();
195201
foreach ((new OPNsense\Wireguard\Server())->servers->server->iterateItems() as $key => $node) {
202+
$carp_depend_on = (string)$node->carp_depend_on;
196203
if (empty((string)$node->enabled)) {
197204
continue;
198-
}
199-
if ($server_id != null && $key != $server_id) {
205+
} elseif ($server_id != null && $key != $server_id) {
206+
continue;
207+
} elseif ($vhid != null && (!empty($vhids[$carp_depend_on]) && $vhids[$carp_depend_on]['vhid'] != $vhid)) {
200208
continue;
201209
}
202210
/**
@@ -206,15 +214,13 @@ function get_stat_hash($fhandle)
206214
* when in BACKUP or INIT mode.
207215
*/
208216
$carp_if_flag = 'up';
209-
if (
210-
!empty($vhids[(string)$node->carp_depend_on]) &&
211-
$vhids[(string)$node->carp_depend_on] != 'MASTER'
212-
) {
217+
if (!empty($vhids[$carp_depend_on]) && $vhids[$carp_depend_on]['status'] != 'MASTER') {
213218
$carp_if_flag = 'down';
214219
}
215220
$server_devs[] = (string)$node->interface;
216221
$statHandle = fopen($node->statFilename, "a+");
217222
if (flock($statHandle, LOCK_EX)) {
223+
$ifdetails = legacy_interfaces_details((string)$node->interface);
218224
switch ($action) {
219225
case 'stop':
220226
wg_stop($node);
@@ -227,7 +233,29 @@ function get_stat_hash($fhandle)
227233
wg_start($node, $statHandle, $carp_if_flag);
228234
break;
229235
case 'configure':
230-
if (@md5_file($node->cnfFilename) != get_stat_hash($statHandle)['file']) {
236+
$ifstatus = '-';
237+
if (!empty($ifdetails[(string)$node->interface])) {
238+
$ifstatus = in_array('up', $ifdetails[(string)$node->interface]['flags']) ? 'up' : 'down';
239+
}
240+
241+
if (!empty($carp_depend_on) && !empty($vhid)) {
242+
// CARP event traceability when a vhid is being passed
243+
syslog(
244+
LOG_NOTICE,
245+
sprintf(
246+
"Wireguard configure event instance %s (%s) vhid: %s carp: %s interface: %s",
247+
$node->name,
248+
$node->interface,
249+
$vhid,
250+
!empty($vhids[$carp_depend_on]) ? $vhids[$carp_depend_on]['status'] : '-',
251+
$ifstatus
252+
)
253+
);
254+
}
255+
if (
256+
@md5_file($node->cnfFilename) != get_stat_hash($statHandle)['file'] ||
257+
empty($ifdetails[(string)$node->interface])
258+
) {
231259
if (get_stat_hash($statHandle)['interface'] != wg_reconfigure_hash($node)) {
232260
// Fluent reloading not supported for this instance, make sure the user is informed
233261
syslog(
@@ -240,8 +268,7 @@ function get_stat_hash($fhandle)
240268
wg_start($node, $statHandle, $carp_if_flag);
241269
} else {
242270
// when triggered via a CARP event, check our interface status [UP|DOWN]
243-
$tmp = in_array('up', $ifdetails[(string)$node->interface]['flags']) ? 'up' : 'down';
244-
if ($tmp != $carp_if_flag) {
271+
if ($ifstatus != $carp_if_flag) {
245272
mwexecf('/sbin/ifconfig %s %s', [$node->interface, $carp_if_flag]);
246273
}
247274
}
@@ -254,9 +281,9 @@ function get_stat_hash($fhandle)
254281
}
255282

256283
/**
257-
* When -a is specified, cleaup up old or disabled instances (files and interfaces)
284+
* When -a is specified, cleanup up old or disabled instances (files and interfaces)
258285
*/
259-
if ($server_id == null) {
286+
if ($server_id == null && $vhid == null) {
260287
foreach (glob('/usr/local/etc/wireguard/wg*') as $filename) {
261288
$this_dev = explode('.', basename($filename))[0];
262289
if (!in_array($this_dev, $server_devs)) {

src/opnsense/service/conf/actions.d/actions_wireguard.conf

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,9 @@ message: restart wireguard instance %s
1818

1919
[configure]
2020
command:/usr/local/opnsense/scripts/Wireguard/wg-service-control.php
21-
parameters: -a configure
21+
parameters: -a configure %s
2222
type:script
23-
message: configure wireguard instances
23+
message: configure wireguard instances (%s)
2424

2525
[renew]
2626
command:/usr/local/opnsense/scripts/Wireguard/reresolve-dns.py

0 commit comments

Comments
 (0)