[Openvpn-devel] [PATCH v2] Route: remove incorrect routes on exit
From: Gianmarco De Gregori
Implemented a safeguard to verify the returned value
from add_route3() when the default gateway is not a local
remote host.
Prior to this implementation, RT_DID_LOCAL flag was
erroneously set even in case of add_route3() failure.
This problem typically occurs when there's no default
route and the --redirect-gateway def1 option is specified,
and in case of reconnection makes it impossible for the client
to reobtain the route to the server.
This fix ensures OpenVPN accurately deletes the appropriate
route on exit by properly handling add_route3() return value.
Fixes: Trac #1457
Change-Id: I8a67b82eb4afdc8d82c5a879c18457b41e77cbe7
Signed-off-by: Gianmarco De Gregori
Acked-by: Frank Lichtenheld
---
This change was reviewed on Gerrit and approved by at least one
developer. I request to merge it to master.
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/522
This mail reflects revision 2 of this Change.
Acked-by according to Gerrit (reflected above):
Frank Lichtenheld
diff --git a/src/openvpn/route.c b/src/openvpn/route.c
index 6c027d9..6ab4392 100644
--- a/src/openvpn/route.c
+++ b/src/openvpn/route.c
@@ -1055,7 +1055,10 @@
ret = add_route3(rl->spec.remote_host, IPV4_NETMASK_HOST,
rl->rgi.gateway.addr, tt, flags |
ROUTE_REF_GW,
>rgi, es, ctx);
-rl->iflags |= RL_DID_LOCAL;
+if (ret)
+{
+rl->iflags |= RL_DID_LOCAL;
+}
}
else
{
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v2] Minor fix to process_ip_header
From: Gianmarco De Gregori
Removed if-guard checking if any feature is
enabled before performing per-feature check.
It doesn't save us much but instead introduces
uneeded complexity.
While at it, fixed a typo IMCP -> ICMP for defined
PIPV6_ICMP_NOHOST_CLIENT and PIPV6_ICMP_NOHOST_SERVER
macros.
Fixes: Trac https://community.openvpn.net/openvpn/ticket/269
Change-Id: I4b5e8357d872c920efdb64632e9bce72cebee202
Signed-off-by: Gianmarco De Gregori
Acked-by: Frank Lichtenheld
---
This change was reviewed on Gerrit and approved by at least one
developer. I request to merge it to master.
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/525
This mail reflects revision 2 of this Change.
Acked-by according to Gerrit (reflected above):
Frank Lichtenheld
diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
index 0443ca0..556c465 100644
--- a/src/openvpn/forward.c
+++ b/src/openvpn/forward.c
@@ -1460,7 +1460,7 @@
* us to examine the IP header (IPv4 or IPv6).
*/
unsigned int flags = PIPV4_PASSTOS | PIP_MSSFIX | PIPV4_CLIENT_NAT
- | PIPV6_IMCP_NOHOST_CLIENT;
+ | PIPV6_ICMP_NOHOST_CLIENT;
process_ip_header(c, flags, >c2.buf);
#ifdef PACKET_TRUNCATION_CHECK
@@ -1644,73 +1644,60 @@
}
if (!c->options.block_ipv6)
{
-flags &= ~(PIPV6_IMCP_NOHOST_CLIENT | PIPV6_IMCP_NOHOST_SERVER);
+flags &= ~(PIPV6_ICMP_NOHOST_CLIENT | PIPV6_ICMP_NOHOST_SERVER);
}
if (buf->len > 0)
{
-/*
- * The --passtos and --mssfix options require
- * us to examine the IPv4 header.
- */
-
-if (flags & (PIP_MSSFIX
-#if PASSTOS_CAPABILITY
- | PIPV4_PASSTOS
-#endif
- | PIPV4_CLIENT_NAT
- ))
+struct buffer ipbuf = *buf;
+if (is_ipv4(TUNNEL_TYPE(c->c1.tuntap), ))
{
-struct buffer ipbuf = *buf;
-if (is_ipv4(TUNNEL_TYPE(c->c1.tuntap), ))
-{
#if PASSTOS_CAPABILITY
-/* extract TOS from IP header */
-if (flags & PIPV4_PASSTOS)
-{
-link_socket_extract_tos(c->c2.link_socket, );
-}
+/* extract TOS from IP header */
+if (flags & PIPV4_PASSTOS)
+{
+link_socket_extract_tos(c->c2.link_socket, );
+}
#endif
-/* possibly alter the TCP MSS */
-if (flags & PIP_MSSFIX)
-{
-mss_fixup_ipv4(, c->c2.frame.mss_fix);
-}
-
-/* possibly do NAT on packet */
-if ((flags & PIPV4_CLIENT_NAT) && c->options.client_nat)
-{
-const int direction = (flags & PIP_OUTGOING) ? CN_INCOMING
: CN_OUTGOING;
-client_nat_transform(c->options.client_nat, ,
direction);
-}
-/* possibly extract a DHCP router message */
-if (flags & PIPV4_EXTRACT_DHCP_ROUTER)
-{
-const in_addr_t dhcp_router =
dhcp_extract_router_msg();
-if (dhcp_router)
-{
-route_list_add_vpn_gateway(c->c1.route_list, c->c2.es,
dhcp_router);
-}
-}
-}
-else if (is_ipv6(TUNNEL_TYPE(c->c1.tuntap), ))
+/* possibly alter the TCP MSS */
+if (flags & PIP_MSSFIX)
{
-/* possibly alter the TCP MSS */
-if (flags & PIP_MSSFIX)
-{
-mss_fixup_ipv6(, c->c2.frame.mss_fix);
-}
-if (!(flags & PIP_OUTGOING) && (flags
-&(PIPV6_IMCP_NOHOST_CLIENT |
PIPV6_IMCP_NOHOST_SERVER)))
-{
-ipv6_send_icmp_unreachable(c, buf,
- (bool)(flags &
PIPV6_IMCP_NOHOST_CLIENT));
-/* Drop the IPv6 packet */
-buf->len = 0;
-}
-
+mss_fixup_ipv4(, c->c2.frame.mss_fix);
}
+
+/* possibly do NAT on packet */
+if ((flags & PIPV4_CLIENT_NAT) && c->options.client_nat)
+{
+const int direction = (flags & PIP_OUTGOING) ? CN_INCOMING :
CN_OUTGOING;
+client_nat_transform(c->options.client_nat, , direction);
+}
+/* possibly extract a DHCP router message */
+if (flags & PIPV4_EXTRACT_DHCP_ROUTER)
+{
+const in_addr_t dhcp_router = dhcp_extract_router_msg();
+if (dhcp_router)
+{
+route_list_add_vpn_gateway(c->c1.route_list, c->c2.es,
dhcp_router);
+}
+}
+}
+else if
[Openvpn-devel] [XS] Change in openvpn[master]: Test change
flichtenheld has abandoned this change. ( http://gerrit.openvpn.net/c/openvpn/+/401?usp=email ) Change subject: Test change .. Abandoned test -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/401?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: I9c1c255f412f3e8ea68ccb1b16ecdd7ad891b308 Gerrit-Change-Number: 401 Gerrit-PatchSet: 1 Gerrit-Owner: unauthorized Gerrit-Reviewer: flichtenheld Gerrit-Reviewer: plaisthos Gerrit-CC: openvpn-devel Gerrit-MessageType: abandon ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [M] Change in openvpn[master]: t_client.sh: Allow to skip tests
Attention is currently required from: ordex, plaisthos. flichtenheld has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/521?usp=email ) Change subject: t_client.sh: Allow to skip tests .. Patch Set 2: (2 comments) File tests/t_client.sh.in: http://gerrit.openvpn.net/c/openvpn/+/521/comment/c1be2335_8f5e8256 : PS2, Line 301: SKIP_ > I am probably clueless about this, but where is $SKIP filled? Like all the other test settings this is defined in the t_client.rc. I can add an example to t_client.rc-sample File tests/unit_tests/openvpn/mock_msg.c: http://gerrit.openvpn.net/c/openvpn/+/521/comment/0cb16f2d_9efcdae7 : PS2, Line 94: endif > I wonder if mock_msg.c is the right place for assert_failed(). […] assert_failed is defined in error.c like all the other functions mocked here, so it seems like a natural place. The alternatives I saw for my patch were: - duplicate mock_msg.c outside of unit_tests/openvpn. But except for assert_failed this works perfectly fine. - try to use original error.c. But that pulls in a lot of platform code. We could look into split out all the complex msg stuff from error.c into its own msg.c. Then we could use error.c in the tests and mock less stuff here. - move assert_failed to its own mock_assert.c. The current solution seems slightly simpler. But that would obviously be a possible alternative if we don't like the NO_CMOCKA define. -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/521?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: I13ea6752c8d102eabcc579e391828c05d5322899 Gerrit-Change-Number: 521 Gerrit-PatchSet: 2 Gerrit-Owner: flichtenheld Gerrit-Reviewer: plaisthos Gerrit-CC: openvpn-devel Gerrit-CC: ordex Gerrit-Attention: plaisthos Gerrit-Attention: ordex Gerrit-Comment-Date: Wed, 21 Feb 2024 10:47:39 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: ordex Gerrit-MessageType: comment ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [M] Change in openvpn[master]: t_client.sh: Allow to skip tests
Attention is currently required from: flichtenheld, plaisthos. ordex has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/521?usp=email ) Change subject: t_client.sh: Allow to skip tests .. Patch Set 2: (3 comments) Patchset: PS2: Feature ACK. I like having the possibility to run tests only when needed/possible. File tests/t_client.sh.in: http://gerrit.openvpn.net/c/openvpn/+/521/comment/d0f81f67_ff0f5ae9 : PS2, Line 301: SKIP_ I am probably clueless about this, but where is $SKIP filled? File tests/unit_tests/openvpn/mock_msg.c: http://gerrit.openvpn.net/c/openvpn/+/521/comment/9ff035ac_ffa92d48 : PS2, Line 94: endif I wonder if mock_msg.c is the right place for assert_failed(). Maybe it should just be moved somewhere else. opinions? -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/521?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: I13ea6752c8d102eabcc579e391828c05d5322899 Gerrit-Change-Number: 521 Gerrit-PatchSet: 2 Gerrit-Owner: flichtenheld Gerrit-Reviewer: plaisthos Gerrit-CC: openvpn-devel Gerrit-CC: ordex Gerrit-Attention: plaisthos Gerrit-Attention: flichtenheld Gerrit-Comment-Date: Wed, 21 Feb 2024 09:16:03 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
