Attention is currently required from: plaisthos.
Hello plaisthos,
I'd like you to do a code review.
Please visit
http://gerrit.openvpn.net/c/openvpn/+/1947?usp=email
to review the following change.
Change subject: route: implement net_gateway_ipv6 for --route-ipv6
......................................................................
route: implement net_gateway_ipv6 for --route-ipv6
--route has been able to name the system default gateway symbolically for
a long time: get_special_addr() resolves net_gateway, vpn_gateway and
remote_host. --route-ipv6 never gained the equivalent, so the usual IPv4
idiom for keeping specific traffic off the tunnel has no IPv6 counterpart.
The failure is silent and points the wrong way. init_route_ipv6() hands
the gateway parameter straight to inet_pton(), which rejects
"net_gateway_ipv6" with a warning and leaves the gateway unset.
add_route_ipv6() then falls back to device = tt->actual_name, so a route
meant to bypass the tunnel is installed through it instead -- the exact
opposite of what was asked for.
Resolve the name from rl6->ngi6, which init_route_ipv6_list() already
fills with the system default gateway and exports to scripts under this
very name, and pin the route to that gateway's interface so it leaves the
host the same way the corresponding IPv4 route does. Note this is ngi6
and not rgi6: the latter is the route towards the VPN server, which only
coincides with the default gateway on a singly-homed host. An on-link
gateway is not a next hop, so only the interface is set in that case.
Cover the three outcomes in route_testdriver: a resolved gateway, one
the system could not supply, and an on-link gateway where only the
interface is wanted.
This revives the patch originally submitted in 2021 for Trac #1161.
Github: OpenVPN/openvpn#1015
Change-Id: I9e45c0edbd2cb237f27e349f2f9d7d9b8d826690
Signed-off-by: François Kooman <[email protected]>
Signed-off-by: Charlie Vigue <[email protected]>
---
M src/openvpn/options.c
M src/openvpn/route.c
M src/openvpn/route.h
M tests/unit_tests/openvpn/test_route.c
4 files changed, 183 insertions(+), 3 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/47/1947/1
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index e899fa6..0badd88 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -4661,7 +4661,7 @@
msg(msglevel, "route-ipv6 parameter network/IP '%s' must be a
valid address", p[1]);
return false;
}
- if (p[2] && !ipv6_addr_safe(p[2]))
+ if (p[2] && !ipv6_addr_safe(p[2]) && !ipv6_get_special_addr(NULL,
p[2], NULL, NULL))
{
msg(msglevel, "route-ipv6 parameter gateway '%s' must be a valid
address", p[2]);
return false;
diff --git a/src/openvpn/route.c b/src/openvpn/route.c
index 8ea745d..23e7e7a 100644
--- a/src/openvpn/route.c
+++ b/src/openvpn/route.c
@@ -297,6 +297,65 @@
return false;
}
+/**
+ * Resolve the special IPv6 gateway name "net_gateway_ipv6".
+ *
+ * The IPv6 counterpart of get_special_addr(), which handles the IPv4 names.
+ * There is currently only one special IPv6 name.
+ *
+ * @param rl route list carrying the system default gateway in ngi6
+ * (not rgi6, which is the route towards the VPN server), or
+ * NULL to merely test whether @p string names a special
+ * address (used when validating options, where no route
+ * list exists yet)
+ * @param string the --route-ipv6 gateway parameter to examine
+ * @param out receives the resolved gateway; only written when @p rl is
+ * given and the system gateway is known
+ * @param status set to false if @p string names a special address that
+ * could not be resolved, so the caller can reject the route;
+ * set to true otherwise. May be NULL.
+ *
+ * @return true if @p string is a special address name, in which case the
+ * caller must not parse it as a literal address; false otherwise.
+ */
+bool
+ipv6_get_special_addr(const struct route_ipv6_list *rl, const char *string,
struct in6_addr *out,
+ bool *status)
+{
+ if (status)
+ {
+ *status = true;
+ }
+
+ if (strcmp(string, "net_gateway_ipv6") != 0)
+ {
+ return false;
+ }
+
+ if (rl)
+ {
+ if (rl->ngi6.flags & RGI_ADDR_DEFINED)
+ {
+ /* an on-link gateway is not a next hop, the interface is
+ * enough -- leave *out unspecified in that case */
+ if (!(rl->ngi6.flags & RGI_ON_LINK))
+ {
+ *out = rl->ngi6.gateway.addr_ipv6;
+ }
+ }
+ else
+ {
+ msg(M_INFO, PACKAGE_NAME " ROUTE: net_gateway_ipv6 undefined --
unable to get default "
+ "gateway from system");
+ if (status)
+ {
+ *status = false;
+ }
+ }
+ }
+ return true;
+}
+
bool
is_special_addr(const char *addr_str)
{
@@ -435,7 +494,7 @@
static bool
init_route_ipv6(struct route_ipv6 *r6, const struct route_ipv6_option *r6o,
- const struct route_ipv6_list *rl6)
+ struct route_ipv6_list *rl6)
{
CLEAR(*r6);
@@ -447,7 +506,26 @@
/* gateway */
if (is_route_parm_defined(r6o->gateway))
{
- if (inet_pton(AF_INET6, r6o->gateway, &r6->gateway) != 1)
+ bool status = true;
+
+ if (ipv6_get_special_addr(rl6, r6o->gateway, &r6->gateway, &status))
+ {
+ if (!status)
+ {
+ goto fail;
+ }
+ /* the route has to leave via the interface the system gateway
+ * is on, otherwise it would be sent down the tunnel */
+ if (rl6->ngi6.flags & RGI_IFACE_DEFINED)
+ {
+#ifdef _WIN32
+ r6->adapter_index = rl6->ngi6.adapter_index;
+#else
+ r6->iface = rl6->ngi6.iface;
+#endif
+ }
+ }
+ else if (inet_pton(AF_INET6, r6o->gateway, &r6->gateway) != 1)
{
msg(M_WARN, PACKAGE_NAME "ROUTE6: cannot parse gateway spec '%s'",
r6o->gateway);
}
diff --git a/src/openvpn/route.h b/src/openvpn/route.h
index 88107e9..90e3ad2 100644
--- a/src/openvpn/route.h
+++ b/src/openvpn/route.h
@@ -345,6 +345,8 @@
void setenv_routes_ipv6(struct env_set *es, const struct route_ipv6_list *rl6);
bool is_special_addr(const char *addr_str);
+bool ipv6_get_special_addr(const struct route_ipv6_list *rl, const char
*string,
+ struct in6_addr *out, bool *status);
/**
* @brief Retrieves the best gateway for a given destination based on the
routing table.
diff --git a/tests/unit_tests/openvpn/test_route.c
b/tests/unit_tests/openvpn/test_route.c
index db039c1..bfcccb8 100644
--- a/tests/unit_tests/openvpn/test_route.c
+++ b/tests/unit_tests/openvpn/test_route.c
@@ -31,6 +31,10 @@
#include "route.h"
#include "networking.h"
+/* the system default IPv6 gateway, and the route towards the peer */
+#define NET_GATEWAY6 "2001:db8:1::1"
+#define PEER_GATEWAY6 "2001:db8:2::1"
+
/* Stubs for functions route.c references but that no test here reaches.
* They assert rather than return quietly, so a test straying onto one
* fails loudly instead of silently doing nothing.
@@ -210,11 +214,107 @@
assert_clear_host_bits("2001:db8::1", 0, "::");
}
+/* Set up a route list where the system default gateway (ngi6) and the
+ * route towards the peer (rgi6) are different, so a test can tell which
+ * one an answer came from.
+ */
+static void
+init_test_route_ipv6_list(struct route_ipv6_list *rl6, unsigned int ngi6_flags)
+{
+ CLEAR(*rl6);
+
+ assert_int_equal(inet_pton(AF_INET6, NET_GATEWAY6,
&rl6->ngi6.gateway.addr_ipv6), 1);
+ rl6->ngi6.flags = ngi6_flags;
+
+ assert_int_equal(inet_pton(AF_INET6, PEER_GATEWAY6,
&rl6->rgi6.gateway.addr_ipv6), 1);
+ rl6->rgi6.flags = RGI_ADDR_DEFINED;
+}
+
+static void
+test_ipv6_get_special_addr_resolves(void **state)
+{
+ struct route_ipv6_list rl6;
+ struct in6_addr out, expect;
+ bool status = false;
+
+ init_test_route_ipv6_list(&rl6, RGI_ADDR_DEFINED);
+ CLEAR(out);
+
+ assert_true(ipv6_get_special_addr(&rl6, "net_gateway_ipv6", &out,
&status));
+ assert_true(status);
+
+ /* the system default gateway, not the route towards the peer */
+ assert_int_equal(inet_pton(AF_INET6, NET_GATEWAY6, &expect), 1);
+ assert_memory_equal(&out, &expect, sizeof(out));
+}
+
+static void
+test_ipv6_get_special_addr_not_special(void **state)
+{
+ struct route_ipv6_list rl6;
+ struct in6_addr out;
+ bool status = false;
+
+ init_test_route_ipv6_list(&rl6, RGI_ADDR_DEFINED);
+ CLEAR(out);
+
+ assert_false(ipv6_get_special_addr(&rl6, "2001:db8::1", &out, &status));
+ assert_true(IN6_IS_ADDR_UNSPECIFIED(&out));
+
+ /* the IPv4 names are not IPv6 names */
+ assert_false(ipv6_get_special_addr(&rl6, "net_gateway", &out, &status));
+
+ /* a caller with no route list only learns whether the name is special */
+ assert_true(ipv6_get_special_addr(NULL, "net_gateway_ipv6", NULL, NULL));
+ assert_false(ipv6_get_special_addr(NULL, "2001:db8::1", NULL, NULL));
+}
+
+static void
+test_ipv6_get_special_addr_undefined(void **state)
+{
+ struct route_ipv6_list rl6;
+ struct in6_addr out;
+ bool status = true;
+
+ /* no IPv6 default gateway on this system */
+ init_test_route_ipv6_list(&rl6, 0);
+ CLEAR(out);
+
+ /* still a special name, but one that could not be resolved, so the
+ * caller must reject the route rather than install it via the tunnel
+ */
+ assert_true(ipv6_get_special_addr(&rl6, "net_gateway_ipv6", &out,
&status));
+ assert_false(status);
+ assert_true(IN6_IS_ADDR_UNSPECIFIED(&out));
+}
+
+static void
+test_ipv6_get_special_addr_on_link(void **state)
+{
+ struct route_ipv6_list rl6;
+ struct in6_addr out;
+ bool status = false;
+
+ init_test_route_ipv6_list(&rl6, RGI_ADDR_DEFINED | RGI_ON_LINK);
+ CLEAR(out);
+
+ /* an on-link gateway is not a next hop: the name resolves, but no
+ * gateway address is handed back
+ */
+ assert_true(ipv6_get_special_addr(&rl6, "net_gateway_ipv6", &out,
&status));
+ assert_true(status);
+ assert_true(IN6_IS_ADDR_UNSPECIFIED(&out));
+}
+
const struct CMUnitTest route_tests[] = {
cmocka_unit_test(test_is_special_addr),
cmocka_unit_test(test_netmask_to_netbits),
cmocka_unit_test(test_netmask_to_netbits2),
cmocka_unit_test(test_route_ipv6_clear_host_bits),
+ cmocka_unit_test(test_ipv6_get_special_addr_resolves),
+ cmocka_unit_test(test_ipv6_get_special_addr_not_special),
+ cmocka_unit_test(test_ipv6_get_special_addr_undefined),
+ cmocka_unit_test(test_ipv6_get_special_addr_on_link),
};
int
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1947?usp=email
To unsubscribe, or for help writing mail filters, visit
http://gerrit.openvpn.net/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I9e45c0edbd2cb237f27e349f2f9d7d9b8d826690
Gerrit-Change-Number: 1947
Gerrit-PatchSet: 1
Gerrit-Owner: chugly <[email protected]>
Gerrit-Reviewer: plaisthos <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: plaisthos <[email protected]>
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
