Re: [PATCH 00/12] Improve WireGuard disconnect, error and hostname lookup

> On 24 Jan 2025, at 10:59 pm, Jussi Laakkonen <[email protected]> wrote:
> 
> This set of patches improves the WireGuard plugin and adds better error case
> support for the vpn/plugins/vpn.c. This allows also to propagate the errors
> upward and with other changes, allows the shutdown to follow the same process
> as the other VPNs. Alsom fix the PrefixLength use in connmand and in the
> WireGuard plugin by tokenizing the host before getaddrinfo() check.
> 
> First, the basic saving of the WireGuard configuration is done similarly to
> other plugins, as well as to what wg-quick is utilizing.
> 
> Second, the handling of errors is improved within the plugin and vpn.c as well.
> This will make it possible to pass the errors upwards from the plugin  In
> addition to this there is a limit for reresolve errors (5 by default) after
> which WireGuard plugin dies in case the configuration is wrong, or network is
> broken.
> 
> Third, the use of getaddrinfo() will block with invalid configuration when
> doing the reresolve for the endpoint. This is now replaced with GResolv by
> adding a wrapper for it in vpn-util.c so it can be used within VPN plugins as
> well. This avoids the blocking of the non-existent address resolve that made
> vpnd unresponsive for the time being, for example, disconnects did not work.
> 
> Fourth, the shutdown is now simulated in a same way other daemon utilizing VPNs
> do, by calling the vpn_died() with a slight delay. This makes daemonless VPNs
> work in the same way as the rest of the plugins to do the same cleanup steps.
> 
> Fifth, the host given in the configuration as an IP-address should contain 
> CIDR notation but as getaddrinfo() uses inet_pton(), which is relying on the
> address to not to have the notation, the host is tokenized first for this use.
> 
> 
> Jussi Laakkonen (12):
>  wireguard: Add saving of provider properties
>  wireguard: Use positive errors for VPN provider connect_cb
>  vpn: Fix VPN_FLAG_NO_DAEMON use in error cases
>  wireguard: Handle disconnect, error and network errors better
>  gresolv: Add generic error for GResolv struct with getter
>  vpn-util: Add wrappers for GResolv hostname lookup use
>  wireguard: Use GResolv for DNS reresolve to avoid blocking
>  vpn: Drop state changes from update_provider_state()
>  wireguard: Fix shutdown, ensure one exit and set no agent is used
>  vpn: Check if disconnect is implemented before calling in stop_vpn()
>  vpn: Fix extracting of PrefixLength D-Bus value
>  wireguard: Tokenize host for getaddrinfo()
> 
> gweb/gresolv.c          |  15 +-
> gweb/gresolv.h          |   2 +
> plugins/vpn.c           |   5 +-
> vpn/plugins/vpn.c       |  63 +++---
> vpn/plugins/wireguard.c | 430 +++++++++++++++++++++++++++++++++++-----
> vpn/vpn-util.c          |  30 +++
> vpn/vpn.h               |  10 +
> 7 files changed, 476 insertions(+), 79 deletions(-)

I’ve been using this series combined with the following series and patch for several weeks:

https://patchwork.kernel.org/project/connman/cover/[email protected]/
https://patchwork.kernel.org/project/connman/patch/DB6PR10MB1845EBB07DB785B580B3F6ECE1E02@DB6PR10MB1845.EURPRD10.PROD.OUTLOOK.COM/

Plus a revert of this commit which I’ve previously flagged as breaking the default “route all traffic down tunnel” with WireGuard (the main use-case for LibreELEC users):

https://git.kernel.org/pub/scm/network/connman/connman.git/commit/?id=9eb1772d31b6fcf78e8711976696491aec9ff5df

For easier testing all patches are in this branch: https://github.com/chewitt/connman/commits/wireguard

No issues observed, so:

Tested-by: Christian Hewitt <[email protected]>