Re: [PATCH] gateway: Fix typo when setting 'is_vpn6'

> On 13 Jan 2025, at 5:59 pm, Gabriel Sanches <[email protected]> wrote:
> 
> Hi,  
> 
> I was testing connman's wireguard plugin and got a segmentation fault.
> 
> Looking at the backtrace revealed that there are some typos in the `src/gateway.c` file where `is_vpn6` was incorrectly set using ipv4 variables instead of ipv6 regarding vpn.
> 
> I simply changed them to ipv6 and got it working.

Hello Gabriel,

I’d like to suggest a change to the commit message for a v2 patch
(removing the discussion and questions so it can be merged):

>>>
Subject: fix calculation of is_vpn6

Fix is_vpn6 to use ipv6 variables not ipv4 variables. This stops
the WireGuard plugin from faulting on startup.

Fixes: commit 823d5a2a8cc4 ("connection: Refactor ‘__connman_connection_gateway_add’”)”

Signed-of-by: Gabriel Sanches <[email protected]>
Tested-by: Christian Hewitt <[email protected]>
<<<

^ You can add my Tested-by. The distro I maintain (LibreELEC) has
been using the same patch for a few months [0] - I’ve asked the
patch contributor to send it upstream but that hasn’t happened, 
so it’s great you have.

[0] https://github.com/LibreELEC/LibreELEC.tv/blob/master/packages/network/connman/patches/connman-07-connman_gateway_add-fix-calculation-of-is_vpn6.patch

> not related to the patch:
> I had to manually add ip route so the other devices could be pinged
> The plugin adds routes for the WireGuard.DNS configuration setting
> # ip route
> 1.1.1.1 dev wg0 scope link 
> 8.8.8.8 dev wg0 scope link 
> 10.42.0.0/24 dev end1 proto kernel scope link src 10.42.0.83 
> 10.42.0.1 dev end1 scope link 
> <vpn host ip> via 10.42.0.1 dev end1
> 
> the missing route added manually:
> ip route add 10.33.0.0/16 dev wg0 scope link
> 
> an equivalent wireguard file adds it properly (using wg-quick)
> 
> Is the plugin lacking some implementation?

The patch in [1] reverts an earlier change from Zoltán that broke
WireGuard routing. I’ve flagged this on the list at the time, but
then the very large refactoring changeset from Grant (requiring
this patch) total broke WireGuard so discussion on the earlier
issue fizzled out. If you’d like to submit the revert patch at
the same time as v2 of this patch - please do (no need for patch
attribution on the revert, though feel free to add my tested-by
again).

[1] https://github.com/LibreELEC/LibreELEC.tv/blob/master/packages/network/connman/patches/connman-06-Revert-Don-t-add-route-for-invalid-dst-and-gateway-a.patch

Christian

> ---
> src/gateway.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/src/gateway.c b/src/gateway.c
> index 16c87f83..b2c211e2 100644
> --- a/src/gateway.c
> +++ b/src/gateway.c
> @@ -3704,10 +3704,10 @@ int __connman_gateway_add(struct connman_service *service,
> is_gateway_config_vpn(
> new_gateway->ipv4_config);
> 
> - is_vpn6 = do_ipv4 &&
> - new_gateway->ipv4_config &&
> + is_vpn6 = do_ipv6 &&
> + new_gateway->ipv6_config &&
> is_gateway_config_vpn(
> - new_gateway->ipv4_config);
> + new_gateway->ipv6_config);
> 
> } else {
> if (do_ipv4 && new_gateway->ipv4_config)
> -- 
> 2.43.0