All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Christian Hewitt <christianshewitt@gmail.com>
To: Marcel Holtmann <marcel@holtmann.org>,
	Denis Kenzior <denkenz@gmail.com>,
	connman@lists.linux.dev
Subject: [PATCH] Revert "Don't add route for invalid dst and gateway address combinations"
Date: Fri, 14 Feb 2025 06:54:26 +0000	[thread overview]
Message-ID: <20250214065426.2697329-1-christianshewitt@gmail.com> (raw)

Commit 9eb1772d31b6 ("Don't add route for invalid dst and gateway address
combinations”) causes a problem regression in WireGuard support through
the connman-vpn agent. I asssume because the wg0 interface matches the
definition of "unspecified destination address coupled with unspecified
gateway” added.

Routing table with commit 9eb1772d31b6 and WireGuard (wg0) active:

RPi5:~ # route
Kernel IP routing table
Destination     Gateway         Genmask         Flags Metric Ref    Use Iface
default         172.16.50.1     0.0.0.0         UG    0      0        0 eth0
1.1.1.1         *               255.255.255.255 UH    0      0        0 wg0
8.8.8.8         *               255.255.255.255 UH    0      0        0 wg0
10.127.0.0      *               255.255.255.0   U     0      0        0 wg0
65.109.130.17   172.16.50.1     255.255.255.255 UGH   0      0        0 eth0
167.299.200.14  172.16.50.1     255.255.255.255 UGH   0      0        0 eth0
172.16.50.0     *               255.255.255.0   U     0      0        0 eth0
172.16.50.1     *               255.255.255.255 UH    0      0        0 eth0

As wg0 does not have the default route “all-traffic" is not routed down
the tunnel as defined in "WireGuard.AllowedIPs = 0.0.0.0/0" config.

Routing table with commit 9eb1772d31b6 reverted:

RPi5:~ # route
Kernel IP routing table
Destination     Gateway         Genmask         Flags Metric Ref    Use Iface
default         *               0.0.0.0         U     0      0        0 wg0
1.1.1.1         *               255.255.255.255 UH    0      0        0 wg0
8.8.8.8         *               255.255.255.255 UH    0      0        0 wg0
10.127.0.0      *               255.255.255.0   U     0      0        0 wg0
65.109.130.17   172.16.50.1     255.255.255.255 UGH   0      0        0 eth0
167.299.200.14  172.16.50.1     255.255.255.255 UGH   0      0        0 eth0
172.16.50.0     *               255.255.255.0   U     0      0        0 eth0
172.16.50.1     *               255.255.255.255 UH    0      0        0 eth0

WireGuard now correctly reoutes “all-traffic” through the tunnel again.

Signed-off-by: Christian Hewitt <christianshewitt@gmail.com>
---
This regression was pointed out in [0] shortly after it was merged
but it's now 18-months later and distros are still carrying revert
patches to make ConnMan + WireGuard usable. I'd prefer to see a
fix for the problem, but there's been no sign of anyone taking up
that challenge and I lack the skills to do it myself. Let's force
the issue by reverting the regression.

[0] https://lore.kernel.org/all/73D64378-2195-4669-8B60-39F808190977@nuovations.com/T/

 src/inet.c | 24 ------------------------
 1 file changed, 24 deletions(-)

diff --git a/src/inet.c b/src/inet.c
index 542e5a85..54c283ff 100644
--- a/src/inet.c
+++ b/src/inet.c
@@ -1697,16 +1697,6 @@ int connman_inet_add_network_route(int index, const char *host,
 	addr.sin_addr.s_addr = inet_addr(host);
 	memcpy(&rt.rt_dst, &addr, sizeof(rt.rt_dst));
 
-	/*
-	 * Don't add a routes for link-local or unspecified
-	 * destination address coupled with unspecified gateway.
-	 */
-	if ((!host || is_addr_ll(AF_INET, (struct sockaddr *)&addr) || __connman_inet_is_any_addr(host, AF_INET))
-			&& (!gateway || __connman_inet_is_any_addr(gateway, AF_INET))) {
-		close(sk);
-		return -EINVAL;
-	}
-
 	memset(&addr, 0, sizeof(addr));
 	addr.sin_family = AF_INET;
 	if (gateway)
@@ -2128,7 +2118,6 @@ int connman_inet_add_ipv6_network_route(int index, const char *host,
 					const char *gateway,
 					unsigned char prefix_len)
 {
-	struct sockaddr_in6 addr;
 	struct in6_rtmsg rt;
 	int sk, err = 0;
 
@@ -2137,19 +2126,6 @@ int connman_inet_add_ipv6_network_route(int index, const char *host,
 	if (!host)
 		return -EINVAL;
 
-	if (inet_pton(AF_INET6, host, &addr.sin6_addr) != 1) {
-		err = -errno;
-		goto out;
-	}
-
-	/*
-	 * Don't add a route for link-local or unspecified
-	 * destination address coupled with unspecified gateway.
-	 */
-	if ((!host || is_addr_ll(AF_INET6, (struct sockaddr *)&addr) || __connman_inet_is_any_addr(host, AF_INET6))
-			&& (!gateway || __connman_inet_is_any_addr(gateway, AF_INET6)))
-		return -EINVAL;
-
 	memset(&rt, 0, sizeof(rt));
 
 	rt.rtmsg_dst_len = prefix_len;
-- 
2.34.1


             reply	other threads:[~2025-02-14  6:54 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-02-14  6:54 Christian Hewitt [this message]
2025-02-21 23:20 ` [PATCH] Revert "Don't add route for invalid dst and gateway address combinations" patchwork-bot+connman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250214065426.2697329-1-christianshewitt@gmail.com \
    --to=christianshewitt@gmail.com \
    --cc=connman@lists.linux.dev \
    --cc=denkenz@gmail.com \
    --cc=marcel@holtmann.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.