You can subscribe to this list here.
2002 |
Jan
|
Feb
|
Mar
|
Apr
(24) |
May
(14) |
Jun
(29) |
Jul
(33) |
Aug
(3) |
Sep
(8) |
Oct
(18) |
Nov
(1) |
Dec
(10) |
---|---|---|---|---|---|---|---|---|---|---|---|---|
2003 |
Jan
(3) |
Feb
(33) |
Mar
(7) |
Apr
(28) |
May
(30) |
Jun
(5) |
Jul
(10) |
Aug
(7) |
Sep
(32) |
Oct
(41) |
Nov
(20) |
Dec
(10) |
2004 |
Jan
(24) |
Feb
(18) |
Mar
(57) |
Apr
(40) |
May
(55) |
Jun
(48) |
Jul
(77) |
Aug
(15) |
Sep
(56) |
Oct
(80) |
Nov
(74) |
Dec
(52) |
2005 |
Jan
(38) |
Feb
(42) |
Mar
(39) |
Apr
(56) |
May
(79) |
Jun
(73) |
Jul
(16) |
Aug
(23) |
Sep
(68) |
Oct
(77) |
Nov
(52) |
Dec
(27) |
2006 |
Jan
(27) |
Feb
(18) |
Mar
(51) |
Apr
(62) |
May
(28) |
Jun
(50) |
Jul
(36) |
Aug
(33) |
Sep
(47) |
Oct
(50) |
Nov
(77) |
Dec
(13) |
2007 |
Jan
(15) |
Feb
(8) |
Mar
(14) |
Apr
(18) |
May
(25) |
Jun
(16) |
Jul
(16) |
Aug
(19) |
Sep
(32) |
Oct
(17) |
Nov
(5) |
Dec
(5) |
2008 |
Jan
(64) |
Feb
(25) |
Mar
(25) |
Apr
(6) |
May
(28) |
Jun
(20) |
Jul
(10) |
Aug
(27) |
Sep
(28) |
Oct
(59) |
Nov
(37) |
Dec
(43) |
2009 |
Jan
(40) |
Feb
(25) |
Mar
(12) |
Apr
(57) |
May
(46) |
Jun
(29) |
Jul
(39) |
Aug
(10) |
Sep
(20) |
Oct
(42) |
Nov
(50) |
Dec
(57) |
2010 |
Jan
(82) |
Feb
(165) |
Mar
(256) |
Apr
(260) |
May
(36) |
Jun
(87) |
Jul
(53) |
Aug
(89) |
Sep
(107) |
Oct
(51) |
Nov
(88) |
Dec
(117) |
2011 |
Jan
(69) |
Feb
(60) |
Mar
(113) |
Apr
(71) |
May
(67) |
Jun
(90) |
Jul
(88) |
Aug
(90) |
Sep
(48) |
Oct
(64) |
Nov
(69) |
Dec
(118) |
2012 |
Jan
(49) |
Feb
(528) |
Mar
(351) |
Apr
(190) |
May
(238) |
Jun
(193) |
Jul
(104) |
Aug
(100) |
Sep
(57) |
Oct
(41) |
Nov
(47) |
Dec
(51) |
2013 |
Jan
(94) |
Feb
(57) |
Mar
(96) |
Apr
(105) |
May
(77) |
Jun
(102) |
Jul
(27) |
Aug
(81) |
Sep
(32) |
Oct
(53) |
Nov
(127) |
Dec
(65) |
2014 |
Jan
(113) |
Feb
(59) |
Mar
(104) |
Apr
(259) |
May
(70) |
Jun
(70) |
Jul
(146) |
Aug
(45) |
Sep
(58) |
Oct
(149) |
Nov
(77) |
Dec
(83) |
2015 |
Jan
(53) |
Feb
(66) |
Mar
(86) |
Apr
(50) |
May
(135) |
Jun
(76) |
Jul
(151) |
Aug
(83) |
Sep
(97) |
Oct
(262) |
Nov
(245) |
Dec
(231) |
2016 |
Jan
(131) |
Feb
(233) |
Mar
(97) |
Apr
(138) |
May
(221) |
Jun
(254) |
Jul
(92) |
Aug
(248) |
Sep
(168) |
Oct
(275) |
Nov
(477) |
Dec
(445) |
2017 |
Jan
(218) |
Feb
(217) |
Mar
(146) |
Apr
(172) |
May
(216) |
Jun
(252) |
Jul
(164) |
Aug
(192) |
Sep
(190) |
Oct
(143) |
Nov
(255) |
Dec
(182) |
2018 |
Jan
(295) |
Feb
(164) |
Mar
(113) |
Apr
(147) |
May
(64) |
Jun
(262) |
Jul
(184) |
Aug
(90) |
Sep
(69) |
Oct
(364) |
Nov
(102) |
Dec
(101) |
2019 |
Jan
(119) |
Feb
(64) |
Mar
(64) |
Apr
(102) |
May
(57) |
Jun
(154) |
Jul
(84) |
Aug
(81) |
Sep
(76) |
Oct
(102) |
Nov
(233) |
Dec
(89) |
2020 |
Jan
(38) |
Feb
(170) |
Mar
(155) |
Apr
(172) |
May
(120) |
Jun
(223) |
Jul
(461) |
Aug
(227) |
Sep
(268) |
Oct
(113) |
Nov
(56) |
Dec
(124) |
2021 |
Jan
(121) |
Feb
(48) |
Mar
(334) |
Apr
(345) |
May
(207) |
Jun
(136) |
Jul
(71) |
Aug
(112) |
Sep
(122) |
Oct
(173) |
Nov
(184) |
Dec
(223) |
2022 |
Jan
(197) |
Feb
(206) |
Mar
(156) |
Apr
(212) |
May
(192) |
Jun
(170) |
Jul
(143) |
Aug
(380) |
Sep
(182) |
Oct
(148) |
Nov
(128) |
Dec
(269) |
2023 |
Jan
(248) |
Feb
(196) |
Mar
(264) |
Apr
(36) |
May
(123) |
Jun
(66) |
Jul
(120) |
Aug
(48) |
Sep
(157) |
Oct
(198) |
Nov
(300) |
Dec
(273) |
2024 |
Jan
(271) |
Feb
(147) |
Mar
(207) |
Apr
(78) |
May
(107) |
Jun
(168) |
Jul
(151) |
Aug
(51) |
Sep
(438) |
Oct
(221) |
Nov
(302) |
Dec
(357) |
2025 |
Jan
(451) |
Feb
(219) |
Mar
(326) |
Apr
(232) |
May
(306) |
Jun
(181) |
Jul
(452) |
Aug
(282) |
Sep
(620) |
Oct
(514) |
Nov
|
Dec
|
From: Jon C. <ro...@fo...> - 2025-10-22 22:11:01
|
Thanks Gert! :) I did find a nice built-in method in the OVPN source code to extract both the source and destination ipv4 addresses (mroute_extract_addr_ip) which I was able to use and build on top of to help solve this multi-threaded "issue" of mine. It has helped greatly in terms of performance now as I am able to simply associate an IPv4 connection state to an available thread which will result in ordered packet processing of data from the TUN device (read/write). This has helped greatly to prevent or reduce the amount of processing and reordering that TCP and QUIC-UDP have to perform and things are loading "snappier" now it seems. The other available threads can also work on their own connection states at the same time allowing them to work in parallel but also separately if you will. Thanks again for your work on this project and inputs on this proof-of-concept experiment I'm running. I wouldn't have been able to do any of this with WireGuard! Blog post: https://fossjon.com/2025/10/22/solving-a-final-remaining-performance-impact-with-mutli-threaded-operation-by-using-connection-state-mapping-in-the-highly-modified-openvpn-source-code-implementation/ On Tue, Oct 21, 2025 at 2:40 AM Gert Doering <ge...@gr...> wrote: > Hi, > > On Mon, Oct 20, 2025 at 04:15:22PM -0400, Jon Chiappetta via Openvpn-devel > wrote: > > I was wondering if any part of the OpenVPN source code parses the IPv4 > > packet header for source/destination address already in place? > > With tun/tap and ipv4/ipv6, this is a bit of nastiness... > > You might want to have a look at drop_if_recursive_routing() in master, > as that one has to find destination address & port and will log source > address & port. > > Not sure we have something more convenient. Grepping for openvpn_iphdr > might turn up something, though :-) (like somewhere in the NAT code). > > gert > -- > "If was one thing all people took for granted, was conviction that if you > feed honest figures into a computer, honest figures come out. Never > doubted > it myself till I met a computer with a sense of humor." > Robert A. Heinlein, The Moon is a Harsh > Mistress > > Gert Doering - Munich, Germany > ge...@gr... > |
From: Gert D. <ge...@gr...> - 2025-10-22 19:38:28
|
Hi, On Wed, Oct 22, 2025 at 06:06:21PM +0000, Joshua Rogers wrote: > By the way, as mentioned, this was found with the ZeroPath tool. I was wondering if it would be of interest to send the raw results of this scanner to somebody that could allow them to review the findings without me manually triaging? I have done this with curl (https://daniel.haxx.se/blog/2025/10/10/a-new-breed-of-analyzers/) and it was quite succesful (~20% false positive rate). > > If this is of interest, please let me know where to send them. The output is just markdown, and it includes potential security vulnerabilities. If not, I will (slowly) continue triaging myself. This is of interest. I'm not really sure where to send this - security bugs go to sec...@op..., but if it's not security, we should not spam this list. Non-security things could go to GH issues, but *if* there is security relevant things in between, we might want to keep the lid on it, for the moment... So you could send everything my way for a start and I discuss with my co-developers how to do this in the future. I'll then try to triage this in a timely fashion and forward to GH, security@, or just drop :-) gert -- "If was one thing all people took for granted, was conviction that if you feed honest figures into a computer, honest figures come out. Never doubted it myself till I met a computer with a sense of humor." Robert A. Heinlein, The Moon is a Harsh Mistress Gert Doering - Munich, Germany ge...@gr... |
From: Joshua R. <co...@jo...> - 2025-10-22 18:06:36
|
Hi all, By the way, as mentioned, this was found with the ZeroPath tool. I was wondering if it would be of interest to send the raw results of this scanner to somebody that could allow them to review the findings without me manually triaging? I have done this with curl (https://daniel.haxx.se/blog/2025/10/10/a-new-breed-of-analyzers/) and it was quite succesful (~20% false positive rate). If this is of interest, please let me know where to send them. The output is just markdown, and it includes potential security vulnerabilities. If not, I will (slowly) continue triaging myself. Thank you. On Wednesday, 22 October 2025 at 13:55, Gert Doering <ge...@gr...> wrote: > > > Hi, > > On Tue, Oct 21, 2025 at 10:34:21PM +0200, Arne Schwabe wrote: > > > Before commiting we have to check that port-share does not rely on this > > behaviour to pass the fd the forked instances. I didn't check right now. > > > Good point. I have a port-share test instance, will test. > > gert > > -- > "If was one thing all people took for granted, was conviction that if you > feed honest figures into a computer, honest figures come out. Never doubted > it myself till I met a computer with a sense of humor." > Robert A. Heinlein, The Moon is a Harsh Mistress > > Gert Doering - Munich, Germany ge...@gr... |
From: plaisthos (C. Review) <ge...@op...> - 2025-10-22 17:02:46
|
Attention is currently required from: flichtenheld. plaisthos has posted comments on this change by plaisthos. ( http://gerrit.openvpn.net/c/openvpn/+/1294?usp=email ) Change subject: Do not try to use the encrypt-then-mac ciphers from OpenSSL 3.6.0 ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: GHA run with OpenSSL 3.6.0 on macOS: https://github.com/schwabe/openvpn/actions/runs/18723419183 -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1294?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: Iafe3c94b952cd3fbecf6f3d05816e5859f425e7d Gerrit-Change-Number: 1294 Gerrit-PatchSet: 1 Gerrit-Owner: plaisthos <arn...@rf...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: flichtenheld <fr...@li...> Gerrit-Comment-Date: Wed, 22 Oct 2025 17:02:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No |
From: plaisthos (C. Review) <ge...@op...> - 2025-10-22 16:44:29
|
Attention is currently required from: flichtenheld. Hello flichtenheld, I'd like you to do a code review. Please visit http://gerrit.openvpn.net/c/openvpn/+/1294?usp=email to review the following change. Change subject: Do not try to use the encrypt-then-mac ciphers from OpenSSL 3.6.0 ...................................................................... Do not try to use the encrypt-then-mac ciphers from OpenSSL 3.6.0 These ciphers claim to be CBC but since they are also include an HMAC are more a mix of AEAD and CBC. Nevertheless, we do not support these and also have no (good) reason to support them. Change-Id: Iafe3c94b952cd3fbecf6f3d05816e5859f425e7d Signed-off-by: Arne Schwabe <ar...@rf...> --- M src/openvpn/crypto_openssl.c 1 file changed, 5 insertions(+), 1 deletion(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/94/1294/1 diff --git a/src/openvpn/crypto_openssl.c b/src/openvpn/crypto_openssl.c index 7688add..04aefa2 100644 --- a/src/openvpn/crypto_openssl.c +++ b/src/openvpn/crypto_openssl.c @@ -789,7 +789,11 @@ #ifdef EVP_CIPH_FLAG_CTS && !(EVP_CIPHER_flags(cipher) & EVP_CIPH_FLAG_CTS) #endif - && !(EVP_CIPHER_flags(cipher) & EVP_CIPH_FLAG_AEAD_CIPHER)); + && !(EVP_CIPHER_flags(cipher) & EVP_CIPH_FLAG_AEAD_CIPHER) +#ifdef EVP_CIPH_FLAG_ENC_THEN_MAC + && !(EVP_CIPHER_flags(cipher) & EVP_CIPH_FLAG_ENC_THEN_MAC) +#endif + ); EVP_CIPHER_free(cipher); return ret; } -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1294?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: Iafe3c94b952cd3fbecf6f3d05816e5859f425e7d Gerrit-Change-Number: 1294 Gerrit-PatchSet: 1 Gerrit-Owner: plaisthos <arn...@rf...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: flichtenheld <fr...@li...> |
From: Johan D. <jo...@op...> - 2025-10-22 16:15:52
|
Meeting summary for 22 October 2025: * *Updated: Release 2.7* OpenVPN 2.7 beta3 was released 13 October. It looks like next up will be beta4, after which a rc1 or stable release is expected, after the hackathon end of October. There are some bug reports for beta3 so we want to address those before doing a beta4. Current tentative release date for beta4 is 29 October. * *Updated: OpenVPN community meetup 2025* https://community.openvpn.net/openvpn/wiki/CommunityMeetup2025 T-shirts arrived at the hotel. * *Updated: forums situation* minx from OpenVPN Inc. originally set up flarum and started migration process, but he left the company before completion. Now we have eduardo at OpenVPN Inc. who is taking a look and he managed to get migration working. He suggests some further migration testing, and to then plan a hard cutover date. * *Updated: wolfSSL license changed from gplv2 to gplv3* The release notes of wolfSSL indicate the license changed to GPLv3. This basically makes wolfSSL incompatible in regards to licensing with OpenVPN. Currently trying to arrange a meeting between relevant parties. As always you're welcome to join at #openvpn-meeting on Libera IRC network every Wednesday at 14:00 Central European Time. Kind regards, Johan Draaisma |
From: Gert D. <ge...@gr...> - 2025-10-22 05:56:09
|
Hi, On Tue, Oct 21, 2025 at 10:34:21PM +0200, Arne Schwabe wrote: > Before commiting we have to check that port-share does not rely on this > behaviour to pass the fd the forked instances. I didn't check right now. Good point. I have a port-share test instance, will test. gert -- "If was one thing all people took for granted, was conviction that if you feed honest figures into a computer, honest figures come out. Never doubted it myself till I met a computer with a sense of humor." Robert A. Heinlein, The Moon is a Harsh Mistress Gert Doering - Munich, Germany ge...@gr... |
From: Arne S. <ar...@rf...> - 2025-10-21 20:34:40
|
Am 21.10.2025 um 21:40 schrieb Gert Doering: > Hi, > > On Tue, Oct 21, 2025 at 06:11:06PM +0000, Joshua Rogers via Openvpn-devel wrote: >> The accept path calls set_cloexec(sd) after accept(). That re-flags the >> listening socket, which is already CLOEXEC from create_socket_tcp(), and >> leaves new_sd inheritable. As a result, client-connect and auth scripts >> spawned after accept can inherit the connected socket and read or write >> the raw TCP stream. This defeats the stated intent to prevent scripts from >> accessing the client socket. > Impressive find. I had to actually look at the code to see what > you are talking about :-) > > So we do > > new_sd = accept(sd, &act->dest.addr.sa, &remote_len); > > and then > > /* set socket file descriptor to not pass across execs, so that > * scripts don't have access to it */ > set_cloexec(sd); > Before commiting we have to check that port-share does not rely on this behaviour to pass the fd the forked instances. I didn't check right now. Arne |
From: Gert D. <ge...@gr...> - 2025-10-21 19:46:10
|
Bad API hack, correct fix... verified by going to mroute.c and reading up on what mroute_extract_openvpn_sockaddr() does with "addr.proto" which should be an *output* structure, but this field is used as input as well... can someone fix this for good, please, after 2.7 release? (Quite an impressive find by GCC) Your patch has been applied to the master branch. commit 0abf6e716b5a50b2b7f4287b1d50f4889eed36aa Author: Frank Lichtenheld Date: Tue Oct 21 21:31:40 2025 +0200 multi: Fix wrong usage of mroute_extract_openvpn_sockaddr Signed-off-by: Frank Lichtenheld <fr...@li...> Acked-by: Gert Doering <ge...@gr...> Acked-by: Gianmarco De Gregori <gia...@ma...> Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1292 Message-Id: <202...@gr...> URL: https://www.mail-archive.com/ope...@li.../msg33830.html Signed-off-by: Gert Doering <ge...@gr...> -- kind regards, Gert Doering |
From: cron2 (C. Review) <ge...@op...> - 2025-10-21 19:46:07
|
cron2 has uploaded a new patch set (#3) to the change originally created by flichtenheld. ( http://gerrit.openvpn.net/c/openvpn/+/1292?usp=email ) The following approvals got outdated and were removed: Code-Review+2 by cron2, Code-Review+2 by its_Giaan Change subject: multi: Fix wrong usage of mroute_extract_openvpn_sockaddr ...................................................................... multi: Fix wrong usage of mroute_extract_openvpn_sockaddr maddr.proto needs to be set before the call since that will change the behavior. Found by GCC "'maddr.proto' is used uninitialized" Change-Id: I76babf08b041162ddedf7a9b7c2799847f15cbdc Signed-off-by: Frank Lichtenheld <fr...@li...> Acked-by: Gert Doering <ge...@gr...> Acked-by: Gianmarco De Gregori <gia...@ma...> Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1292 Message-Id: <202...@gr...> URL: https://www.mail-archive.com/ope...@li.../msg33830.html Signed-off-by: Gert Doering <ge...@gr...> --- M src/openvpn/multi.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/92/1292/3 diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c index e907524..fa9c654 100644 --- a/src/openvpn/multi.c +++ b/src/openvpn/multi.c @@ -3965,9 +3965,9 @@ saddr.addr.in4.sin_family = AF_INET; saddr.addr.in4.sin_addr.s_addr = htonl(addr); saddr.addr.in4.sin_port = htons(port); + maddr.proto = proto; if (mroute_extract_openvpn_sockaddr(&maddr, &saddr, true)) { - maddr.proto = proto; hash_iterator_init(m->iter, &hi); while ((he = hash_iterator_next(&hi))) { -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1292?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: I76babf08b041162ddedf7a9b7c2799847f15cbdc Gerrit-Change-Number: 1292 Gerrit-PatchSet: 3 Gerrit-Owner: flichtenheld <fr...@li...> Gerrit-Reviewer: cron2 <ge...@gr...> Gerrit-Reviewer: its_Giaan <gia...@ma...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> |
From: cron2 (C. Review) <ge...@op...> - 2025-10-21 19:46:07
|
cron2 has submitted this change. ( http://gerrit.openvpn.net/c/openvpn/+/1292?usp=email ) Change subject: multi: Fix wrong usage of mroute_extract_openvpn_sockaddr ...................................................................... multi: Fix wrong usage of mroute_extract_openvpn_sockaddr maddr.proto needs to be set before the call since that will change the behavior. Found by GCC "'maddr.proto' is used uninitialized" Change-Id: I76babf08b041162ddedf7a9b7c2799847f15cbdc Signed-off-by: Frank Lichtenheld <fr...@li...> Acked-by: Gert Doering <ge...@gr...> Acked-by: Gianmarco De Gregori <gia...@ma...> Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1292 Message-Id: <202...@gr...> URL: https://www.mail-archive.com/ope...@li.../msg33830.html Signed-off-by: Gert Doering <ge...@gr...> --- M src/openvpn/multi.c 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c index e907524..fa9c654 100644 --- a/src/openvpn/multi.c +++ b/src/openvpn/multi.c @@ -3965,9 +3965,9 @@ saddr.addr.in4.sin_family = AF_INET; saddr.addr.in4.sin_addr.s_addr = htonl(addr); saddr.addr.in4.sin_port = htons(port); + maddr.proto = proto; if (mroute_extract_openvpn_sockaddr(&maddr, &saddr, true)) { - maddr.proto = proto; hash_iterator_init(m->iter, &hi); while ((he = hash_iterator_next(&hi))) { -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1292?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: I76babf08b041162ddedf7a9b7c2799847f15cbdc Gerrit-Change-Number: 1292 Gerrit-PatchSet: 3 Gerrit-Owner: flichtenheld <fr...@li...> Gerrit-Reviewer: cron2 <ge...@gr...> Gerrit-Reviewer: its_Giaan <gia...@ma...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> |
From: Gert D. <ge...@gr...> - 2025-10-21 19:41:11
|
Hi, On Tue, Oct 21, 2025 at 06:11:06PM +0000, Joshua Rogers via Openvpn-devel wrote: > The accept path calls set_cloexec(sd) after accept(). That re-flags the > listening socket, which is already CLOEXEC from create_socket_tcp(), and > leaves new_sd inheritable. As a result, client-connect and auth scripts > spawned after accept can inherit the connected socket and read or write > the raw TCP stream. This defeats the stated intent to prevent scripts from > accessing the client socket. Impressive find. I had to actually look at the code to see what you are talking about :-) So we do new_sd = accept(sd, &act->dest.addr.sa, &remote_len); and then /* set socket file descriptor to not pass across execs, so that * scripts don't have access to it */ set_cloexec(sd); return new_sd; which very clearly is not intended behaviour. So, Acked-by: ge...@gr... will deal with it "as soon as possible" which will take a few days. gert -- "If was one thing all people took for granted, was conviction that if you feed honest figures into a computer, honest figures come out. Never doubted it myself till I met a computer with a sense of humor." Robert A. Heinlein, The Moon is a Harsh Mistress Gert Doering - Munich, Germany ge...@gr... |
From: Gert D. <ge...@gr...> - 2025-10-21 19:32:01
|
From: Frank Lichtenheld <fr...@li...> maddr.proto needs to be set before the call since that will change the behavior. Found by GCC "'maddr.proto' is used uninitialized" Change-Id: I76babf08b041162ddedf7a9b7c2799847f15cbdc Signed-off-by: Frank Lichtenheld <fr...@li...> Acked-by: Gert Doering <ge...@gr...> Acked-by: Gianmarco De Gregori <gia...@ma...> Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1292 --- 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/+/1292 This mail reflects revision 2 of this Change. Acked-by according to Gerrit (reflected above): Gert Doering <ge...@gr...> Gianmarco De Gregori <gia...@ma...> diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c index e907524..fa9c654 100644 --- a/src/openvpn/multi.c +++ b/src/openvpn/multi.c @@ -3965,9 +3965,9 @@ saddr.addr.in4.sin_family = AF_INET; saddr.addr.in4.sin_addr.s_addr = htonl(addr); saddr.addr.in4.sin_port = htons(port); + maddr.proto = proto; if (mroute_extract_openvpn_sockaddr(&maddr, &saddr, true)) { - maddr.proto = proto; hash_iterator_init(m->iter, &hi); while ((he = hash_iterator_next(&hi))) { |
From: cron2 (C. Review) <ge...@op...> - 2025-10-21 19:31:25
|
Attention is currently required from: flichtenheld, plaisthos. cron2 has posted comments on this change by flichtenheld. ( http://gerrit.openvpn.net/c/openvpn/+/1292?usp=email ) Change subject: multi: Fix wrong usage of mroute_extract_openvpn_sockaddr ...................................................................... Patch Set 2: Code-Review+2 (1 comment) Patchset: PS2: Oh, sorry. That was a gerrit comparison artefact. -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1292?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: I76babf08b041162ddedf7a9b7c2799847f15cbdc Gerrit-Change-Number: 1292 Gerrit-PatchSet: 2 Gerrit-Owner: flichtenheld <fr...@li...> Gerrit-Reviewer: cron2 <ge...@gr...> Gerrit-Reviewer: its_Giaan <gia...@ma...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: plaisthos <arn...@rf...> Gerrit-Attention: flichtenheld <fr...@li...> Gerrit-Comment-Date: Tue, 21 Oct 2025 19:31:15 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes |
From: cron2 (C. Review) <ge...@op...> - 2025-10-21 19:30:47
|
Attention is currently required from: flichtenheld, plaisthos. cron2 has posted comments on this change by flichtenheld. ( http://gerrit.openvpn.net/c/openvpn/+/1292?usp=email ) Change subject: multi: Fix wrong usage of mroute_extract_openvpn_sockaddr ...................................................................... Patch Set 2: Code-Review-2 (1 comment) Patchset: PS2: v2 has some stuff in there that doesn't belong here (push.c, ssl_verify.c integers) -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1292?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: I76babf08b041162ddedf7a9b7c2799847f15cbdc Gerrit-Change-Number: 1292 Gerrit-PatchSet: 2 Gerrit-Owner: flichtenheld <fr...@li...> Gerrit-Reviewer: cron2 <ge...@gr...> Gerrit-Reviewer: its_Giaan <gia...@ma...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: plaisthos <arn...@rf...> Gerrit-Attention: flichtenheld <fr...@li...> Gerrit-Comment-Date: Tue, 21 Oct 2025 19:30:38 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes |
From: Joshua R. <co...@jo...> - 2025-10-21 18:30:24
|
This avoids avoids string_alloc + strtok on every membership check, which for very long IV_CIPHERS with many colon-separated tokens, forced O(N*M) comparisons with I(M) allocations in a single negotiatin. |
From: flichtenheld (C. Review) <ge...@op...> - 2025-10-21 18:25:08
|
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/+/1293?usp=email to review the following change. Change subject: push: Fix conversion issues related to timeout in send_auth_pending_messages ...................................................................... push: Fix conversion issues related to timeout in send_auth_pending_messages Add additional checking to make sure that the required casts are safe. Change-Id: Icc31b7fa0da86220df45552aecc15dc6c769cd54 Signed-off-by: Frank Lichtenheld <fr...@li...> --- M src/openvpn/push.c M src/openvpn/ssl_verify.c 2 files changed, 15 insertions(+), 19 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/93/1293/1 diff --git a/src/openvpn/push.c b/src/openvpn/push.c index 6f146fc..a6f979d 100644 --- a/src/openvpn/push.c +++ b/src/openvpn/push.c @@ -429,11 +429,6 @@ gc_free(&gc); } -#if defined(__GNUC__) || defined(__clang__) -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wconversion" -#endif - bool send_auth_pending_messages(struct tls_multi *tls_multi, struct tls_session *session, const char *extra, unsigned int timeout) @@ -449,7 +444,12 @@ /* Calculate the maximum timeout and subtract the time we already waited */ unsigned int max_timeout = max_uint(tls_multi->opt.renegotiate_seconds / 2, tls_multi->opt.handshake_window); - max_timeout = max_timeout - (now - ks->initial); + time_t time_elapsed = now - ks->initial; + if (time_elapsed < 0 || time_elapsed >= (time_t)max_timeout) + { + return false; + } + max_timeout -= (unsigned int)time_elapsed; timeout = min_uint(max_timeout, timeout); struct gc_arena gc = gc_new(); @@ -734,6 +734,11 @@ return true; } +#if defined(__GNUC__) || defined(__clang__) +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wconversion" +#endif + static bool send_push_options(struct context *c, struct buffer *buf, struct push_list *push_list, int safe_cap, bool *push_sent, bool *multi_push) diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c index 04ef27e..fe95621 100644 --- a/src/openvpn/ssl_verify.c +++ b/src/openvpn/ssl_verify.c @@ -874,11 +874,6 @@ return supported; } -#if defined(__GNUC__) || defined(__clang__) -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wconversion" -#endif - /** * Checks if the deferred state should also send auth pending * request to the client. Also removes the auth_pending control file @@ -915,8 +910,9 @@ buf_chomp(iv_buf); buf_chomp(extra_buf); + errno = 0; long timeout = strtol(BSTR(timeout_buf), NULL, 10); - if (timeout == 0) + if (timeout <= 0 || timeout > UINT_MAX || errno) { msg(M_WARN, "could not parse auth pending file timeout"); buffer_list_free(lines); @@ -933,14 +929,13 @@ pending_method); auth_set_client_reason(multi, buf); msg(M_INFO, - "Client does not supported auth pending method " - "'%s'", + "Client does not supported auth pending method '%s'", pending_method); ret = false; } else { - send_auth_pending_messages(multi, session, BSTR(extra_buf), timeout); + send_auth_pending_messages(multi, session, BSTR(extra_buf), (unsigned int)timeout); } } @@ -950,10 +945,6 @@ return ret; } -#if defined(__GNUC__) || defined(__clang__) -#pragma GCC diagnostic pop -#endif - /** * Removes auth_pending and auth_control files from file system * and key_state structure -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1293?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: Icc31b7fa0da86220df45552aecc15dc6c769cd54 Gerrit-Change-Number: 1293 Gerrit-PatchSet: 1 Gerrit-Owner: flichtenheld <fr...@li...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: plaisthos <arn...@rf...> |
From: flichtenheld (C. Review) <ge...@op...> - 2025-10-21 18:20:18
|
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/+/1292?usp=email to review the following change. Change subject: multi: Fix wrong usage of mroute_extract_openvpn_sockaddr ...................................................................... multi: Fix wrong usage of mroute_extract_openvpn_sockaddr maddr.proto needs to be set before the call since that will change the behavior. Found by GCC "'maddr.proto' is used uninitialized" Change-Id: I76babf08b041162ddedf7a9b7c2799847f15cbdc Signed-off-by: Frank Lichtenheld <fr...@li...> --- M src/openvpn/multi.c M src/openvpn/push.c M src/openvpn/ssl_verify.c 3 files changed, 16 insertions(+), 20 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/92/1292/1 diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c index e907524..fa9c654 100644 --- a/src/openvpn/multi.c +++ b/src/openvpn/multi.c @@ -3965,9 +3965,9 @@ saddr.addr.in4.sin_family = AF_INET; saddr.addr.in4.sin_addr.s_addr = htonl(addr); saddr.addr.in4.sin_port = htons(port); + maddr.proto = proto; if (mroute_extract_openvpn_sockaddr(&maddr, &saddr, true)) { - maddr.proto = proto; hash_iterator_init(m->iter, &hi); while ((he = hash_iterator_next(&hi))) { diff --git a/src/openvpn/push.c b/src/openvpn/push.c index 2c717c7..1871816 100644 --- a/src/openvpn/push.c +++ b/src/openvpn/push.c @@ -429,11 +429,6 @@ gc_free(&gc); } -#if defined(__GNUC__) || defined(__clang__) -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wconversion" -#endif - bool send_auth_pending_messages(struct tls_multi *tls_multi, struct tls_session *session, const char *extra, unsigned int timeout) @@ -449,7 +444,12 @@ /* Calculate the maximum timeout and subtract the time we already waited */ unsigned int max_timeout = max_uint(tls_multi->opt.renegotiate_seconds / 2, tls_multi->opt.handshake_window); - max_timeout = max_timeout - (now - ks->initial); + time_t time_elapsed = now - ks->initial; + if (time_elapsed < 0 || time_elapsed >= (time_t)max_timeout) + { + return false; + } + max_timeout -= (unsigned int)time_elapsed; timeout = min_uint(max_timeout, timeout); struct gc_arena gc = gc_new(); @@ -734,6 +734,11 @@ return true; } +#if defined(__GNUC__) || defined(__clang__) +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wconversion" +#endif + static bool send_push_options(struct context *c, struct buffer *buf, struct push_list *push_list, int safe_cap, bool *push_sent, bool *multi_push) diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c index 04ef27e..fe95621 100644 --- a/src/openvpn/ssl_verify.c +++ b/src/openvpn/ssl_verify.c @@ -874,11 +874,6 @@ return supported; } -#if defined(__GNUC__) || defined(__clang__) -#pragma GCC diagnostic push -#pragma GCC diagnostic ignored "-Wconversion" -#endif - /** * Checks if the deferred state should also send auth pending * request to the client. Also removes the auth_pending control file @@ -915,8 +910,9 @@ buf_chomp(iv_buf); buf_chomp(extra_buf); + errno = 0; long timeout = strtol(BSTR(timeout_buf), NULL, 10); - if (timeout == 0) + if (timeout <= 0 || timeout > UINT_MAX || errno) { msg(M_WARN, "could not parse auth pending file timeout"); buffer_list_free(lines); @@ -933,14 +929,13 @@ pending_method); auth_set_client_reason(multi, buf); msg(M_INFO, - "Client does not supported auth pending method " - "'%s'", + "Client does not supported auth pending method '%s'", pending_method); ret = false; } else { - send_auth_pending_messages(multi, session, BSTR(extra_buf), timeout); + send_auth_pending_messages(multi, session, BSTR(extra_buf), (unsigned int)timeout); } } @@ -950,10 +945,6 @@ return ret; } -#if defined(__GNUC__) || defined(__clang__) -#pragma GCC diagnostic pop -#endif - /** * Removes auth_pending and auth_control files from file system * and key_state structure -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1292?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: I76babf08b041162ddedf7a9b7c2799847f15cbdc Gerrit-Change-Number: 1292 Gerrit-PatchSet: 1 Gerrit-Owner: flichtenheld <fr...@li...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: plaisthos <arn...@rf...> |
From: flichtenheld (C. Review) <ge...@op...> - 2025-10-21 18:16:33
|
Attention is currently required from: plaisthos. Hello plaisthos, I'd like you to reexamine a change. Please visit http://gerrit.openvpn.net/c/openvpn/+/1292?usp=email to look at the new patch set (#2). Change subject: multi: Fix wrong usage of mroute_extract_openvpn_sockaddr ...................................................................... multi: Fix wrong usage of mroute_extract_openvpn_sockaddr maddr.proto needs to be set before the call since that will change the behavior. Found by GCC "'maddr.proto' is used uninitialized" Change-Id: I76babf08b041162ddedf7a9b7c2799847f15cbdc Signed-off-by: Frank Lichtenheld <fr...@li...> --- M src/openvpn/multi.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/92/1292/2 diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c index e907524..fa9c654 100644 --- a/src/openvpn/multi.c +++ b/src/openvpn/multi.c @@ -3965,9 +3965,9 @@ saddr.addr.in4.sin_family = AF_INET; saddr.addr.in4.sin_addr.s_addr = htonl(addr); saddr.addr.in4.sin_port = htons(port); + maddr.proto = proto; if (mroute_extract_openvpn_sockaddr(&maddr, &saddr, true)) { - maddr.proto = proto; hash_iterator_init(m->iter, &hi); while ((he = hash_iterator_next(&hi))) { -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1292?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: I76babf08b041162ddedf7a9b7c2799847f15cbdc Gerrit-Change-Number: 1292 Gerrit-PatchSet: 2 Gerrit-Owner: flichtenheld <fr...@li...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: plaisthos <arn...@rf...> |
From: Joshua R. <co...@jo...> - 2025-10-21 18:11:57
|
The accept path calls set_cloexec(sd) after accept(). That re-flags the listening socket, which is already CLOEXEC from create_socket_tcp(), and leaves new_sd inheritable. As a result, client-connect and auth scripts spawned after accept can inherit the connected socket and read or write the raw TCP stream. This defeats the stated intent to prevent scripts from accessing the client socket. This bug was found using ZeroPath. |
From: its_Giaan (C. Review) <ge...@op...> - 2025-10-21 17:35:50
|
Attention is currently required from: flichtenheld, plaisthos. its_Giaan has posted comments on this change by flichtenheld. ( http://gerrit.openvpn.net/c/openvpn/+/1292?usp=email ) Change subject: multi: Fix wrong usage of mroute_extract_openvpn_sockaddr ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1292?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: I76babf08b041162ddedf7a9b7c2799847f15cbdc Gerrit-Change-Number: 1292 Gerrit-PatchSet: 2 Gerrit-Owner: flichtenheld <fr...@li...> Gerrit-Reviewer: its_Giaan <gia...@ma...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: plaisthos <arn...@rf...> Gerrit-Attention: flichtenheld <fr...@li...> Gerrit-Comment-Date: Tue, 21 Oct 2025 17:35:41 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes |
From: mrbff (C. Review) <ge...@op...> - 2025-10-21 14:10:54
|
Attention is currently required from: cron2, flichtenheld, plaisthos. mrbff has posted comments on this change by mrbff. ( http://gerrit.openvpn.net/c/openvpn/+/677?usp=email ) Change subject: route: copied 'gateway_needed' logic from add_route_ipv6 to add_route ...................................................................... Patch Set 12: (2 comments) Patchset: PS11: > This changes the logic of adding routes on all platforms in a very drastic way. […] Done File src/openvpn/route.c: http://gerrit.openvpn.net/c/openvpn/+/677/comment/2b9f3a14_b1ffc52f?usp=email : PS11, Line 1606: gateway_needed = true; > Sure but why is the gateway needed in this case? […] Done -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/677?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: Id1ec0c6e4c391604ec5dbb8b7122f2e47ad186d1 Gerrit-Change-Number: 677 Gerrit-PatchSet: 12 Gerrit-Owner: mrbff <ma...@ma...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: cron2 <ge...@gr...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: plaisthos <arn...@rf...> Gerrit-Attention: cron2 <ge...@gr...> Gerrit-Attention: flichtenheld <fr...@li...> Gerrit-Comment-Date: Tue, 21 Oct 2025 14:10:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: plaisthos <arn...@rf...> |
From: mrbff (C. Review) <ge...@op...> - 2025-10-21 14:10:33
|
Attention is currently required from: flichtenheld, ordex, plaisthos. mrbff has posted comments on this change by mrbff. ( http://gerrit.openvpn.net/c/openvpn/+/721?usp=email ) Change subject: route: extended logic to omit gateway when unnecessary ...................................................................... Patch Set 12: (6 comments) File src/openvpn/route.h: http://gerrit.openvpn.net/c/openvpn/+/721/comment/0a1b545d_f913bbc8?usp=email : PS11, Line 274: bool add_route_ipv6(struct route_ipv6 *r, const struct tuntap *tt, unsigned int flags, const struct env_set *es, openvpn_net_ctx_t *ctx, const bool is_multipoint); > The naming here and the usage seem to disagree. […] Done File src/openvpn/route.c: http://gerrit.openvpn.net/c/openvpn/+/721/comment/aaa6380b_66ab5947?usp=email : PS11, Line 1811: /* FIXME -- add on-link support for Dragonfly */ > Has this been really tested on DragonFly BSD? i tested on the first patch but i tested again the #12 on 6.4.2 and found also a bug with the ifconfig, i fixed just moving from #if defined(TARGET_FREEBSD) || defined (TARGET_DRAGONFLY) code to #if defined(TARGET_NETBSD) || defined (TARGET_DRAGONFLY) code in do_ifconfig_ipv4(). Here the logs: master: 2025-10-20 13:36:42 PUSH: Received control message: 'PUSH_REPLY,route-gateway 10.23.100.1,topology subnet,ping 5,ping-restart 30,ifconfig 10.23.100.2 255.255.255.0,peer-id 0,cipher AES-256-GCM,protocol-flags cc-exit tls-ekm dyn-tls-crypt,tun-mtu 1500' 2025-10-20 13:36:42 OPTIONS IMPORT: --ifconfig/up options modified 2025-10-20 13:36:42 OPTIONS IMPORT: route-related options modified 2025-10-20 13:36:42 OPTIONS IMPORT: tun-mtu set to 1500 2025-10-20 13:36:42 TUN/TAP device tun0 exists previously, keep at program end 2025-10-20 13:36:42 TUN/TAP device /dev/tun0 opened 2025-10-20 13:36:42 tun/tap device [tun0] opened 2025-10-20 13:36:42 /sbin/ifconfig tun0 10.23.100.2/24 mtu 1500 up ifconfig: ioctl (SIOCAIFADDR): Destination address required 2025-10-20 13:36:42 FreeBSD ifconfig failed: external program exited with error status: 1 2025-10-20 13:36:42 Exiting due to fatal error patchset #12: 2025-10-20 13:43:42 PUSH: Received control message: 'PUSH_REPLY,route-gateway 10.23.100.1,topology subnet,ping 5,ping-restart 30,ifconfig 10.23.100.2 255.255.255.0,peer-id 0,cipher AES-256-GCM,protocol-flags cc-exit tls-ekm dyn-tls-crypt,tun-mtu 1500' 2025-10-20 13:43:42 OPTIONS IMPORT: --ifconfig/up options modified 2025-10-20 13:43:42 OPTIONS IMPORT: route-related options modified 2025-10-20 13:43:42 OPTIONS IMPORT: tun-mtu set to 1500 2025-10-20 13:43:42 TUN/TAP device tun0 exists previously, keep at program end 2025-10-20 13:43:42 TUN/TAP device /dev/tun0 opened 2025-10-20 13:43:42 tun/tap device [tun0] opened 2025-10-20 13:43:42 /sbin/ifconfig tun0 10.23.100.2 10.23.100.1 mtu 1500 netmask 255.255.255.0 up 2025-10-20 13:43:42 /sbin/route add -net 10.23.100.0 -iface tun0 add net 10.23.100.0: gateway tun0 2025-10-20 13:43:42 Initialization Sequence Completed If you have any thought about it or something to suggest (or some particular test) please feel free to share http://gerrit.openvpn.net/c/openvpn/+/721/comment/b2bc803a_7e364e02?usp=email : PS11, Line 1600: && !( (r4->flags & RT_METRIC_DEFINED) && r4->metric == 0 ) ) > how does the metric play into needing a gateway or not? Done http://gerrit.openvpn.net/c/openvpn/+/721/comment/2a5d88e2_1d00ff0a?usp=email : PS11, Line 1607: return true; > There should at least be some idea/comment in the source code as well. […] Done http://gerrit.openvpn.net/c/openvpn/+/721/comment/85295ff5_ccdfe664?usp=email : PS11, Line 1898: } > Has this been tested on Open and NetBSD? it was already tested from the first patch but i retested the latest patch on dradonfly 6.4.2, openbsd 7.7, freebsd 14.3, netbsd 10.0 and MacOS sonoma http://gerrit.openvpn.net/c/openvpn/+/721/comment/0b6b4458_7d49c544?usp=email : PS11, Line 2010: { > I think this duplicates the code that was publicly accessible in https://gerrit.openvpn. […] Done -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/721?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: I87777e74b1fd34781e1d72c9f994eb84f39d800c Gerrit-Change-Number: 721 Gerrit-PatchSet: 12 Gerrit-Owner: mrbff <ma...@ma...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-CC: ordex <an...@ma...> Gerrit-Attention: plaisthos <arn...@rf...> Gerrit-Attention: flichtenheld <fr...@li...> Gerrit-Attention: ordex <an...@ma...> Gerrit-Comment-Date: Tue, 21 Oct 2025 14:10:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: plaisthos <arn...@rf...> Comment-In-Reply-To: ordex <an...@ma...> |
From: mrbff (C. Review) <ge...@op...> - 2025-10-21 14:05:34
|
Attention is currently required from: flichtenheld, mrbff. Hello flichtenheld, plaisthos, I'd like you to reexamine a change. Please visit http://gerrit.openvpn.net/c/openvpn/+/721?usp=email to look at the new patch set (#12). Change subject: route: extended logic to omit gateway when unnecessary ...................................................................... route: extended logic to omit gateway when unnecessary Extracted and extended the logic behind gateway_needed both in add_route() and add_route_ipv6(). Checking dev-type, special routes, if the gateway is on-link and if the gateway is in the vpn subnet. Additionally, extended support for these checks and conditions to DARWIN and BSD-based operating systems. These changes ensure that the gateway is only included when necessary, optimizing route configuration and potentially reducing redundant route entries. Additionally, Dragonfly's do_ifconfig_ipv4() code is now shared with NetBSD's, rather than FreeBSD's. Change-Id: I87777e74b1fd34781e1d72c9f994eb84f39d800c Signed-off-by: Marco Baffo <ma...@ma...> --- M src/openvpn/route.c M src/openvpn/tun.c 2 files changed, 210 insertions(+), 130 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/21/721/12 diff --git a/src/openvpn/route.c b/src/openvpn/route.c index c249b38..4dc4005 100644 --- a/src/openvpn/route.c +++ b/src/openvpn/route.c @@ -42,6 +42,7 @@ #include "options.h" #include "networking.h" #include "integer.h" +#include "openvpn.h" #include "memdbg.h" @@ -1396,6 +1397,7 @@ } gc_free(&gc); } + void setenv_routes_ipv6(struct env_set *es, const struct route_ipv6_list *rl6) { @@ -1472,6 +1474,37 @@ } #endif +static bool +is_gateway_needed_ipv4(const struct route_ipv4 *r4, + const struct route_gateway_info *rgi, + const struct tuntap *tt) +{ +#ifndef _WIN32 + /* server told us which interface to use; pass gateway if it gave one */ + if (rgi && (rgi->flags & RGI_IFACE_DEFINED) && rgi->iface[0] != 0) + { + if (rgi->flags & RGI_ADDR_DEFINED && r4->gateway != 0) + { + return true; + } + } +#endif + + /* TAP is an L2 device, so for destinations that are not explicitly + * on-link the kernel needs a gateway so it can ARP/ND for the + * MAC address. We make an exception only when the user explicitly sets + * metric 0, which is taken as a request for an on-link “route to + * interface” (no gateway). This keeps parity with the IPv6 code. + */ + if (tt->type == DEV_TYPE_TAP + && !((r4->flags & RT_METRIC_DEFINED) && r4->metric == 0)) + { + return true; + } + + return false; +} + bool add_route(struct route_ipv4 *r, const struct tuntap *tt, unsigned int flags, const struct route_gateway_info *rgi, /* may be NULL */ @@ -1503,28 +1536,7 @@ goto done; } -#ifndef _WIN32 - /* server told us which interface to use; pass gateway if it gave one */ - if (rgi && (rgi->flags & RGI_IFACE_DEFINED) && rgi->iface[0] != 0) - { - if (rgi->flags & RGI_ADDR_DEFINED && r->gateway != 0) - { - gateway_needed = true; - } - } -#endif - - /* TAP is an L2 device, so for destinations that are not explicitly - * on-link the kernel needs a gateway so it can ARP/ND for the - * MAC address. We make an exception only when the user explicitly sets - * metric 0, which is taken as a request for an on-link “route to - * interface” (no gateway). This keeps parity with the IPv6 code. - */ - if (tt->type == DEV_TYPE_TAP - && !((r->flags & RT_METRIC_DEFINED) && r->metric == 0)) - { - gateway_needed = true; - } + gateway_needed = is_gateway_needed_ipv4(r, rgi, tt); if (gateway_needed && r->gateway == 0) { @@ -1537,18 +1549,21 @@ goto done; } -#if defined(TARGET_LINUX) - const char *iface = tt->actual_name; +#if !defined(_WIN32) && !defined(TARGET_SOLARIS) \ + && !defined(TARGET_AIX) + const char *device = tt->actual_name; if (!gateway_needed && rgi && (rgi->flags & RGI_IFACE_DEFINED) && rgi->iface[0] != 0) /* vpn server special route */ { - iface = rgi->iface; + device = rgi->iface; } +#endif +#if defined(TARGET_LINUX) int metric = -1; if (is_on_link(is_local_route, flags, rgi)) { - iface = rgi->iface; + device = rgi->iface; } if (r->flags & RT_METRIC_DEFINED) @@ -1560,7 +1575,7 @@ status = RTA_SUCCESS; int ret = net_route_v4_add(ctx, &r->network, netmask_to_netbits2(r->netmask), gateway_needed ? &r->gateway : NULL, - iface, r->table_id, metric); + device, r->table_id, metric); if (ret == -EEXIST) { msg(D_ROUTE, "NOTE: Linux route add command failed because route exists"); @@ -1577,7 +1592,7 @@ if (rgi) { - snprintf(out, sizeof(out), "%s %s %s dev %s", network, netmask, gateway, rgi->iface); + snprintf(out, sizeof(out), "%s %s %s dev %s", network, netmask, gateway, device); } else { @@ -1672,7 +1687,7 @@ bool ret = openvpn_execve_check(&argv, es, 0, "ERROR: Solaris route add command failed"); status = ret ? RTA_SUCCESS : RTA_ERROR; -#elif defined(TARGET_FREEBSD) +#elif defined(TARGET_FREEBSD) || defined(TARGET_DRAGONFLY) argv_printf(&argv, "%s add", ROUTE_PATH); @@ -1683,31 +1698,18 @@ } #endif - argv_printf_cat(&argv, "-net %s %s %s", network, gateway, netmask); - - /* FIXME -- add on-link support for FreeBSD */ - - argv_msg(D_ROUTE, &argv); - bool ret = openvpn_execve_check(&argv, es, 0, "ERROR: FreeBSD route add command failed"); - status = ret ? RTA_SUCCESS : RTA_ERROR; - -#elif defined(TARGET_DRAGONFLY) - - argv_printf(&argv, "%s add", ROUTE_PATH); - -#if 0 - if (r->flags & RT_METRIC_DEFINED) + if (gateway_needed) { - argv_printf_cat(&argv, "-rtt %d", r->metric); + argv_printf_cat(&argv, "-net %s %s %s", network, gateway, netmask); } -#endif - - argv_printf_cat(&argv, "-net %s %s %s", network, gateway, netmask); - - /* FIXME -- add on-link support for Dragonfly */ + else + { + argv_printf_cat(&argv, "-net %s -iface %s", network, device); + } argv_msg(D_ROUTE, &argv); - bool ret = openvpn_execve_check(&argv, es, 0, "ERROR: DragonFly route add command failed"); + bool ret = openvpn_execve_check(&argv, es, 0, + "ERROR: BSD route add command failed"); status = ret ? RTA_SUCCESS : RTA_ERROR; #elif defined(TARGET_DARWIN) @@ -1730,7 +1732,14 @@ } else { - argv_printf_cat(&argv, "-net %s %s %s", network, gateway, netmask); + if (gateway_needed) + { + argv_printf_cat(&argv, "-net %s %s %s", network, gateway, netmask); + } + else + { + argv_printf_cat(&argv, "-net %s -interface %s", network, device); + } } argv_msg(D_ROUTE, &argv); @@ -1748,9 +1757,14 @@ } #endif - argv_printf_cat(&argv, "-net %s %s -netmask %s", network, gateway, netmask); - - /* FIXME -- add on-link support for OpenBSD/NetBSD */ + if (gateway_needed) + { + argv_printf_cat(&argv, "-net %s %s -netmask %s", network, gateway, netmask); + } + else + { + argv_printf_cat(&argv, "-net %s -netmask %s -link -iface %s", network, netmask, device); + } argv_msg(D_ROUTE, &argv); bool ret = openvpn_execve_check(&argv, es, 0, "ERROR: OpenBSD/NetBSD route add command failed"); @@ -1827,6 +1841,47 @@ } } +static bool +is_gateway_needed_ipv6(const struct route_ipv6 *r6, + const struct tuntap *tt, + const char *network) +{ +#ifndef _WIN32 + if (r6->iface != NULL) /* VPN server special route */ + { + if (!IN6_IS_ADDR_UNSPECIFIED(&r6->gateway)) + { + return true; + } + } +#endif + + /* + * Filter out routes which are essentially no-ops + * (not currently done for IPv6) + */ + + /* On "tun" interface, we never set a gateway if the operating system + * can do "route to interface" - it does not add value, as the target + * dev already fully qualifies the route destination on point-to-point + * interfaces. OTOH, on "tap" interface, we must always set the + * gateway unless the route is to be an on-link network + */ + if (tt->type == DEV_TYPE_TAP + && !((r6->flags & RT_METRIC_DEFINED) && r6->metric == 0)) + { + return true; + } + + if (ipv6_net_contains_host(&tt->local_ipv6, tt->netbits_ipv6, &r6->gateway)) + { + msg(D_ROUTE, "Ignoring gateway in VPN subnet for route %s/%d", network, r6->netbits); + return false; + } + + return false; +} + bool add_route_ipv6(struct route_ipv6 *r6, const struct tuntap *tt, unsigned int flags, const struct env_set *es, openvpn_net_ctx_t *ctx) @@ -1842,22 +1897,12 @@ struct argv argv = argv_new(); struct gc_arena gc = gc_new(); -#ifndef _WIN32 - const char *device = tt->actual_name; - if (r6->iface != NULL) /* vpn server special route */ - { - device = r6->iface; - if (!IN6_IS_ADDR_UNSPECIFIED(&r6->gateway)) - { - gateway_needed = true; - } - } -#endif - route_ipv6_clear_host_bits(r6); const char *network = print_in6_addr(r6->network, 0, &gc); const char *gateway = print_in6_addr(r6->gateway, 0, &gc); + gateway_needed = is_gateway_needed_ipv6(r6, tt, network); + #if defined(TARGET_DARWIN) || defined(TARGET_FREEBSD) || defined(TARGET_DRAGONFLY) \ || defined(TARGET_OPENBSD) || defined(TARGET_NETBSD) @@ -1875,6 +1920,15 @@ } #endif +#if !defined(_WIN32) + + const char *device = tt->actual_name; + if (r6->iface != NULL) + { + device = r6->iface; + } +#endif + #ifndef _WIN32 msg(D_ROUTE, "add_route_ipv6(%s/%d -> %s metric %d) dev %s", network, r6->netbits, gateway, r6->metric, device); @@ -1883,22 +1937,6 @@ r6->metric, r6->adapter_index ? r6->adapter_index : tt->adapter_index); #endif - /* - * Filter out routes which are essentially no-ops - * (not currently done for IPv6) - */ - - /* On "tun" interface, we never set a gateway if the operating system - * can do "route to interface" - it does not add value, as the target - * dev already fully qualifies the route destination on point-to-point - * interfaces. OTOH, on "tap" interface, we must always set the - * gateway unless the route is to be an on-link network - */ - if (tt->type == DEV_TYPE_TAP && !((r6->flags & RT_METRIC_DEFINED) && r6->metric == 0)) - { - gateway_needed = true; - } - if (gateway_needed && IN6_IS_ADDR_UNSPECIFIED(&r6->gateway)) { msg(M_WARN, @@ -1913,6 +1951,7 @@ #if defined(TARGET_LINUX) int metric = -1; + if ((r6->flags & RT_METRIC_DEFINED) && (r6->metric > 0)) { metric = r6->metric; @@ -2008,21 +2047,21 @@ bool ret = openvpn_execve_check(&argv, es, 0, "ERROR: MacOS X route add -inet6 command failed"); status = ret ? RTA_SUCCESS : RTA_ERROR; -#elif defined(TARGET_OPENBSD) +#elif defined(TARGET_OPENBSD) || defined(TARGET_NETBSD) - argv_printf(&argv, "%s add -inet6 %s -prefixlen %d %s", ROUTE_PATH, network, r6->netbits, - gateway); + argv_printf(&argv, "%s add -inet6 %s -prefixlen %d ", ROUTE_PATH, network, r6->netbits); + + if (gateway_needed) + { + argv_printf_cat(&argv, "%s", gateway); + } + else + { + argv_printf_cat(&argv, "-link -iface %s", device); + } argv_msg(D_ROUTE, &argv); - bool ret = openvpn_execve_check(&argv, es, 0, "ERROR: OpenBSD route add -inet6 command failed"); - status = ret ? RTA_SUCCESS : RTA_ERROR; - -#elif defined(TARGET_NETBSD) - - argv_printf(&argv, "%s add -inet6 %s/%d %s", ROUTE_PATH, network, r6->netbits, gateway); - - argv_msg(D_ROUTE, &argv); - bool ret = openvpn_execve_check(&argv, es, 0, "ERROR: NetBSD route add -inet6 command failed"); + bool ret = openvpn_execve_check(&argv, es, 0, "ERROR: OpenBSD/NetBSD route add -inet6 command failed"); status = ret ? RTA_SUCCESS : RTA_ERROR; #elif defined(TARGET_AIX) @@ -2080,6 +2119,21 @@ #endif int is_local_route; +#if defined(TARGET_DARWIN) || defined(TARGET_LINUX) \ + || defined(TARGET_FREEBSD) || defined(TARGET_DRAGONFLY) \ + || defined(TARGET_OPENBSD) || defined(TARGET_NETBSD) + + bool gateway_needed = is_gateway_needed_ipv4(r, rgi, tt); + +#if !defined(TARGET_OPENBSD) && !defined(TARGET_NETBSD) + const char *device = tt->actual_name; + if (!gateway_needed && rgi && (rgi->flags & RGI_IFACE_DEFINED) && rgi->iface[0] != 0) /* vpn server special route */ + { + device = rgi->iface; + } +#endif +#endif + if ((r->flags & (RT_DEFINED | RT_ADDED)) != (RT_DEFINED | RT_ADDED)) { return; @@ -2106,12 +2160,19 @@ #if defined(TARGET_LINUX) metric = -1; + + if (is_on_link(is_local_route, flags, rgi)) + { + device = rgi->iface; + } + if (r->flags & RT_METRIC_DEFINED) { metric = r->metric; } - if (net_route_v4_del(ctx, &r->network, netmask_to_netbits2(r->netmask), &r->gateway, NULL, + if (net_route_v4_del(ctx, &r->network, netmask_to_netbits2(r->netmask), + gateway_needed ? &r->gateway : NULL, device, r->table_id, metric) < 0) { @@ -2165,19 +2226,21 @@ argv_msg(D_ROUTE, &argv); openvpn_execve_check(&argv, es, 0, "ERROR: Solaris route delete command failed"); -#elif defined(TARGET_FREEBSD) +#elif defined(TARGET_FREEBSD) || defined(TARGET_DRAGONFLY) - argv_printf(&argv, "%s delete -net %s %s %s", ROUTE_PATH, network, gateway, netmask); + argv_printf(&argv, "%s delete", ROUTE_PATH); + + if (gateway_needed) + { + argv_printf_cat(&argv, "-net %s %s %s", network, gateway, netmask); + } + else + { + argv_printf_cat(&argv, "-net %s -iface %s", network, device); + } argv_msg(D_ROUTE, &argv); - openvpn_execve_check(&argv, es, 0, "ERROR: FreeBSD route delete command failed"); - -#elif defined(TARGET_DRAGONFLY) - - argv_printf(&argv, "%s delete -net %s %s %s", ROUTE_PATH, network, gateway, netmask); - - argv_msg(D_ROUTE, &argv); - openvpn_execve_check(&argv, es, 0, "ERROR: DragonFly route delete command failed"); + openvpn_execve_check(&argv, es, 0, "ERROR: BSD route delete command failed"); #elif defined(TARGET_DARWIN) @@ -2188,7 +2251,16 @@ } else { - argv_printf(&argv, "%s delete -net %s %s %s", ROUTE_PATH, network, gateway, netmask); + argv_printf(&argv, "%s delete ", ROUTE_PATH); + + if (gateway_needed) + { + argv_printf_cat(&argv, "-net %s %s %s", network, gateway, netmask); + } + else + { + argv_printf_cat(&argv, "-net %s -interface %s", network, device); + } } argv_msg(D_ROUTE, &argv); @@ -2196,7 +2268,16 @@ #elif defined(TARGET_OPENBSD) || defined(TARGET_NETBSD) - argv_printf(&argv, "%s delete -net %s %s -netmask %s", ROUTE_PATH, network, gateway, netmask); + argv_printf(&argv, "%s delete ", ROUTE_PATH); + + if (gateway_needed) + { + argv_printf_cat(&argv, "-net %s %s -netmask %s", network, gateway, netmask); + } + else + { + argv_printf_cat(&argv, "-net %s -netmask %s", network, netmask); + } argv_msg(D_ROUTE, &argv); openvpn_execve_check(&argv, es, 0, "ERROR: OpenBSD/NetBSD route delete command failed"); @@ -2250,23 +2331,16 @@ #if !defined(TARGET_LINUX) const char *gateway; #endif -#if !defined(TARGET_SOLARIS) - bool gateway_needed = false; +#if defined(TARGET_DARWIN) || defined(TARGET_LINUX) \ + || defined(TARGET_FREEBSD) || defined(TARGET_DRAGONFLY) + const char *device = tt->actual_name; if (r6->iface != NULL) /* vpn server special route */ { device = r6->iface; - gateway_needed = true; } (void)device; /* unused on some platforms */ - /* if we used a gateway on "add route", we also need to specify it on - * delete, otherwise some OSes will refuse to delete the route - */ - if (tt->type == DEV_TYPE_TAP && !((r6->flags & RT_METRIC_DEFINED) && r6->metric == 0)) - { - gateway_needed = true; - } #endif #endif @@ -2278,6 +2352,15 @@ gateway = print_in6_addr(r6->gateway, 0, &gc); #endif +#if defined(TARGET_DARWIN) || defined(TARGET_LINUX) || defined(TARGET_FREEBSD) \ + || defined(TARGET_DRAGONFLY) || defined(TARGET_OPENBSD) || defined(TARGET_NETBSD) + /* if we used a gateway on "add route", we also need to specify it on + * delete, otherwise some OSes will refuse to delete the route + */ + bool gateway_needed = false; + gateway_needed = is_gateway_needed_ipv6(r6, tt, network); +#endif + #if defined(TARGET_DARWIN) || defined(TARGET_FREEBSD) || defined(TARGET_DRAGONFLY) \ || defined(TARGET_OPENBSD) || defined(TARGET_NETBSD) @@ -2362,20 +2445,17 @@ argv_msg(D_ROUTE, &argv); openvpn_execve_check(&argv, es, 0, "ERROR: MacOS X route delete -inet6 command failed"); -#elif defined(TARGET_OPENBSD) +#elif defined(TARGET_OPENBSD) || defined(TARGET_NETBSD) - argv_printf(&argv, "%s delete -inet6 %s -prefixlen %d %s", ROUTE_PATH, network, r6->netbits, - gateway); + argv_printf(&argv, "%s delete -inet6 %s -prefixlen %d", ROUTE_PATH, network, r6->netbits); + + if (gateway_needed) + { + argv_printf_cat(&argv, "%s", gateway); + } argv_msg(D_ROUTE, &argv); - openvpn_execve_check(&argv, es, 0, "ERROR: OpenBSD route delete -inet6 command failed"); - -#elif defined(TARGET_NETBSD) - - argv_printf(&argv, "%s delete -inet6 %s/%d %s", ROUTE_PATH, network, r6->netbits, gateway); - - argv_msg(D_ROUTE, &argv); - openvpn_execve_check(&argv, es, 0, "ERROR: NetBSD route delete -inet6 command failed"); + openvpn_execve_check(&argv, es, 0, "ERROR: OpenBSD/NetBSD route delete -inet6 command failed"); #elif defined(TARGET_AIX) diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c index 06b7ae5..0df946b 100644 --- a/src/openvpn/tun.c +++ b/src/openvpn/tun.c @@ -1383,7 +1383,7 @@ add_route(&r, tt, 0, NULL, es, NULL); } -#elif defined(TARGET_NETBSD) +#elif defined(TARGET_NETBSD) || defined(TARGET_DRAGONFLY) in_addr_t remote_end = INADDR_ANY; /* for "virtual" subnet topology */ if (tun_p2p) @@ -1466,7 +1466,7 @@ add_route(&r, tt, 0, NULL, es, NULL); } -#elif defined(TARGET_FREEBSD) || defined(TARGET_DRAGONFLY) +#elif defined(TARGET_FREEBSD) /* example: ifconfig tun2 10.2.0.2 10.2.0.1 mtu 1450 netmask 255.255.255.255 up */ if (tun_p2p) /* point-to-point tun */ -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/721?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: I87777e74b1fd34781e1d72c9f994eb84f39d800c Gerrit-Change-Number: 721 Gerrit-PatchSet: 12 Gerrit-Owner: mrbff <ma...@ma...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-CC: ordex <an...@ma...> Gerrit-Attention: flichtenheld <fr...@li...> Gerrit-Attention: mrbff <ma...@ma...> |
From: mrbff (C. Review) <ge...@op...> - 2025-10-21 14:05:27
|
Attention is currently required from: cron2, flichtenheld, mrbff, plaisthos. Hello flichtenheld, plaisthos, I'd like you to reexamine a change. Please visit http://gerrit.openvpn.net/c/openvpn/+/677?usp=email to look at the new patch set (#12). The following approvals got outdated and were removed: Code-Review-1 by plaisthos Change subject: route: copied 'gateway_needed' logic from add_route_ipv6 to add_route ...................................................................... route: copied 'gateway_needed' logic from add_route_ipv6 to add_route Under certain circumstances it may not be necessary to pass the gateway when adding a new route via net_route_v4_add() API function. add_route_ipv6() already accounts for some of these cases and therefore this patch copies the same logic to add_route(). Change-Id: Id1ec0c6e4c391604ec5dbb8b7122f2e47ad186d1 Signed-off-by: Marco Baffo <ma...@ma...> --- M Changes.rst M src/openvpn/route.c 2 files changed, 51 insertions(+), 3 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/77/677/12 diff --git a/Changes.rst b/Changes.rst index 4feacad2..a3e00c2 100644 --- a/Changes.rst +++ b/Changes.rst @@ -170,6 +170,13 @@ COPYING: license details only relevant to our Windows installers have been updated and moved to the openvpn-build repo +The route gateway is now only added for IPv4 when strictly necessary. + The gateway-needed logic previously applied to IPv6 only has been copied + and extended to IPv4 route handling. Route additions now omit the + gateway unless required after checking device type, special routes, + on-link gateways, and whether the gateway lies inside the VPN subnet, + optimizing route configuration and potentially reducing redundant route + entries. Deprecated features ------------------- diff --git a/src/openvpn/route.c b/src/openvpn/route.c index 7d988da..c249b38 100644 --- a/src/openvpn/route.c +++ b/src/openvpn/route.c @@ -1479,6 +1479,7 @@ { int status = 0; int is_local_route; + bool gateway_needed = false; if (!(r->flags & RT_DEFINED)) { @@ -1488,8 +1489,8 @@ struct argv argv = argv_new(); struct gc_arena gc = gc_new(); -#if !defined(TARGET_LINUX) const char *network = print_in_addr_t(r->network, 0, &gc); +#if !defined(TARGET_LINUX) #if !defined(TARGET_AIX) const char *netmask = print_in_addr_t(r->netmask, 0, &gc); #endif @@ -1502,8 +1503,47 @@ goto done; } +#ifndef _WIN32 + /* server told us which interface to use; pass gateway if it gave one */ + if (rgi && (rgi->flags & RGI_IFACE_DEFINED) && rgi->iface[0] != 0) + { + if (rgi->flags & RGI_ADDR_DEFINED && r->gateway != 0) + { + gateway_needed = true; + } + } +#endif + + /* TAP is an L2 device, so for destinations that are not explicitly + * on-link the kernel needs a gateway so it can ARP/ND for the + * MAC address. We make an exception only when the user explicitly sets + * metric 0, which is taken as a request for an on-link “route to + * interface” (no gateway). This keeps parity with the IPv6 code. + */ + if (tt->type == DEV_TYPE_TAP + && !((r->flags & RT_METRIC_DEFINED) && r->metric == 0)) + { + gateway_needed = true; + } + + if (gateway_needed && r->gateway == 0) + { + msg(M_WARN, "ROUTE WARNING: " PACKAGE_NAME " needs a gateway " + "parameter for a --route option and no default was set via " + "--route-gateway or --ifconfig option. Not installing " + "IPv4 route to %s/%d.", + network, netmask_to_netbits2(r->netmask)); + status = 0; + goto done; + } + #if defined(TARGET_LINUX) - const char *iface = NULL; + const char *iface = tt->actual_name; + if (!gateway_needed && rgi && (rgi->flags & RGI_IFACE_DEFINED) && rgi->iface[0] != 0) /* vpn server special route */ + { + iface = rgi->iface; + } + int metric = -1; if (is_on_link(is_local_route, flags, rgi)) @@ -1518,7 +1558,8 @@ status = RTA_SUCCESS; - int ret = net_route_v4_add(ctx, &r->network, netmask_to_netbits2(r->netmask), &r->gateway, + int ret = net_route_v4_add(ctx, &r->network, netmask_to_netbits2(r->netmask), + gateway_needed ? &r->gateway : NULL, iface, r->table_id, metric); if (ret == -EEXIST) { -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/677?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: Id1ec0c6e4c391604ec5dbb8b7122f2e47ad186d1 Gerrit-Change-Number: 677 Gerrit-PatchSet: 12 Gerrit-Owner: mrbff <ma...@ma...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: cron2 <ge...@gr...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: plaisthos <arn...@rf...> Gerrit-Attention: cron2 <ge...@gr...> Gerrit-Attention: flichtenheld <fr...@li...> Gerrit-Attention: mrbff <ma...@ma...> |