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 (793) | Nov (682) | Dec (2) |
| S | M | T | W | T | F | S |
|---|---|---|---|---|---|---|
| | | | | | | 1 |
| 2 | 3 (3) | 4 (1) | 5 (26) | 6 (34) | 7 (6) | 8 (17) |
| 9 (15) | 10 | 11 (8) | 12 (22) | 13 (3) | 14 (10) | 15 |
| 16 | 17 (4) | 18 (32) | 19 (18) | 20 (11) | 21 (5) | 22 |
| 23 (3) | 24 (35) | 25 (16) | 26 (14) | 27 (26) | 28 (7) | 29 (1) |
| 30 | 31 (9) | | | | | |
| From: cron2 (C. Review) <ge...@op...> - 2025-03-24 21:16:01 |
cron2 has submitted this change. ( http://gerrit.openvpn.net/c/openvpn/+/916?usp=email ) Change subject: Make 'lport 0' no longer sufficient to do '--bind'. ...................................................................... Make 'lport 0' no longer sufficient to do '--bind'. 'lport <anything>' used to trigger 'do socket bind', which is not useful in itself for the 'lport 0' case (port 0 -> OS assigns a random port, as it is done for unbound sockets) unless also binding to a particular local IP address ('--local 192.0.2.1'). The trigger for 'lport has been used, do socket bind' is ce.local_port_defined -> change the code to test for "0", and only set this for non-0 ports (NOTE: this is a string match, so if you really really want the old "lport 0" behaviour, using "lport 00" still does that...). The ce.local_port value is still set, so '--lport 0' together with '--local 192.0.2.1' will give you a random port number bound to that IP address - without 'lport 0' it would default to 1194 or the value of '--port' (if not using '--rport'). Summary: socket bind is now only done if one of these is set - --lport <port> with <port> not "0" - --bind (default on the client is "--nobind") - --local <address> Github: schwabe/ics-openvpn#1794 Change-Id: I1976307a7643c82f31d55ca32c79cbe64b6fffc6 Signed-off-by: Gert Doering <ge...@gr...> Acked-by: Arne Schwabe <arn...@rf...> Message-Id: <202...@gr...> URL: https://www.mail-archive.com/ope...@li.../msg31222.html Signed-off-by: Gert Doering <ge...@gr...> --- M doc/man-sections/link-options.rst M src/openvpn/options.c 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/doc/man-sections/link-options.rst b/doc/man-sections/link-options.rst index d48021e..287473e 100644 --- a/doc/man-sections/link-options.rst +++ b/doc/man-sections/link-options.rst @@ -122,7 +122,9 @@ --lport port Set default TCP/UDP port number. Cannot be used together with - ``--nobind`` option. + ``--nobind`` option. A port number of ``0`` is only honoured to + achieve "bind() to a random assigned port number" if a bind-to IP + address is specified with ``--local``. --mark value Mark encrypted packets being sent with ``value``. The mark value can be diff --git a/src/openvpn/options.c b/src/openvpn/options.c index ab56609..99dd60a 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -6710,7 +6710,12 @@ else if (streq(p[0], "lport") && p[1] && !p[2]) { VERIFY_PERMISSION(OPT_P_GENERAL|OPT_P_CONNECTION); - options->ce.local_port_defined = true; + + /* only trigger bind() if port is not 0 (or --local is used) */ + if (!streq(p[1], "0")) + { + options->ce.local_port_defined = true; + } options->ce.local_port = p[1]; } else if (streq(p[0], "rport") && p[1] && !p[2]) -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/916?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: I1976307a7643c82f31d55ca32c79cbe64b6fffc6 Gerrit-Change-Number: 916 Gerrit-PatchSet: 2 Gerrit-Owner: cron2 <ge...@gr...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-CC: ordex <an...@ma...> Gerrit-MessageType: merged |
| From: cron2 (C. Review) <ge...@op...> - 2025-03-24 21:15:58 |
Hello flichtenheld, plaisthos, I'd like you to reexamine a change. Please visit http://gerrit.openvpn.net/c/openvpn/+/916?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Code-Review+2 by plaisthos Change subject: Make 'lport 0' no longer sufficient to do '--bind'. ...................................................................... Make 'lport 0' no longer sufficient to do '--bind'. 'lport <anything>' used to trigger 'do socket bind', which is not useful in itself for the 'lport 0' case (port 0 -> OS assigns a random port, as it is done for unbound sockets) unless also binding to a particular local IP address ('--local 192.0.2.1'). The trigger for 'lport has been used, do socket bind' is ce.local_port_defined -> change the code to test for "0", and only set this for non-0 ports (NOTE: this is a string match, so if you really really want the old "lport 0" behaviour, using "lport 00" still does that...). The ce.local_port value is still set, so '--lport 0' together with '--local 192.0.2.1' will give you a random port number bound to that IP address - without 'lport 0' it would default to 1194 or the value of '--port' (if not using '--rport'). Summary: socket bind is now only done if one of these is set - --lport <port> with <port> not "0" - --bind (default on the client is "--nobind") - --local <address> Github: schwabe/ics-openvpn#1794 Change-Id: I1976307a7643c82f31d55ca32c79cbe64b6fffc6 Signed-off-by: Gert Doering <ge...@gr...> Acked-by: Arne Schwabe <arn...@rf...> Message-Id: <202...@gr...> URL: https://www.mail-archive.com/ope...@li.../msg31222.html Signed-off-by: Gert Doering <ge...@gr...> --- M doc/man-sections/link-options.rst M src/openvpn/options.c 2 files changed, 9 insertions(+), 2 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/16/916/2 diff --git a/doc/man-sections/link-options.rst b/doc/man-sections/link-options.rst index d48021e..287473e 100644 --- a/doc/man-sections/link-options.rst +++ b/doc/man-sections/link-options.rst @@ -122,7 +122,9 @@ --lport port Set default TCP/UDP port number. Cannot be used together with - ``--nobind`` option. + ``--nobind`` option. A port number of ``0`` is only honoured to + achieve "bind() to a random assigned port number" if a bind-to IP + address is specified with ``--local``. --mark value Mark encrypted packets being sent with ``value``. The mark value can be diff --git a/src/openvpn/options.c b/src/openvpn/options.c index ab56609..99dd60a 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -6710,7 +6710,12 @@ else if (streq(p[0], "lport") && p[1] && !p[2]) { VERIFY_PERMISSION(OPT_P_GENERAL|OPT_P_CONNECTION); - options->ce.local_port_defined = true; + + /* only trigger bind() if port is not 0 (or --local is used) */ + if (!streq(p[1], "0")) + { + options->ce.local_port_defined = true; + } options->ce.local_port = p[1]; } else if (streq(p[0], "rport") && p[1] && !p[2]) -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/916?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: I1976307a7643c82f31d55ca32c79cbe64b6fffc6 Gerrit-Change-Number: 916 Gerrit-PatchSet: 2 Gerrit-Owner: cron2 <ge...@gr...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-CC: ordex <an...@ma...> Gerrit-MessageType: newpatchset |
| From: Gert D. <ge...@gr...> - 2025-03-24 21:15:28 |
Tested this with various combinations of --port, --bind, --lport, --local <v4|v6>, etc. - a fascinating world of interesting effects. I've fixed a small oversight in the commit message - the line needs to read Summary: socket bind is now only done if one of these is set - --lport <port> with <port> not "0" (with "--lport", as "--port" never leads to an automatic bind) Also added a reference to GH schwabe/ics-openvpn#1794 where half the problems go away if "lport 0" is removed from the config - which, in these cases, translates to "is not enabling --bind" (the real issue is "any" bind AF vs. getaddrinfo(), but not binding at all helps). Patch has been applied to the master branch. commit c91948a0e03f0ad03e7fdde59ed9fce87ba00885 Author: Gert Doering Date: Mon Mar 24 19:27:26 2025 +0100 Make 'lport 0' no longer sufficient to do '--bind'. Signed-off-by: Gert Doering <ge...@gr...> Acked-by: Arne Schwabe <arn...@rf...> Message-Id: <202...@gr...> URL: https://www.mail-archive.com/ope...@li.../msg31222.html Signed-off-by: Gert Doering <ge...@gr...> -- kind regards, Gert Doering |
| From: cron2 (C. Review) <ge...@op...> - 2025-03-24 18:38:44 |
Attention is currently required from: flichtenheld, plaisthos. cron2 has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/909?usp=email ) Change subject: Mention address if not unspecific on DNS failure ...................................................................... Patch Set 1: Code-Review-1 (4 comments) Patchset: PS1: details File src/openvpn/socket.c: http://gerrit.openvpn.net/c/openvpn/+/909/comment/29ff33c4_efbe7f78 : PS1, Line 453: gettaddrinfo_addr_family_name(int af) is the "double t" intentional? ge`tt`addrinfo? http://gerrit.openvpn.net/c/openvpn/+/909/comment/9dc345c2_18bf229f : PS1, Line 581: intentional extra blank line? http://gerrit.openvpn.net/c/openvpn/+/909/comment/28600d47_bc36aa79 : PS1, Line 660: gettaddrinfo_addr_family_name(ai_family), double "t" -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/909?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: I1c8fcd5bb6e1fa0020d52879eefbafdb2630e7b5 Gerrit-Change-Number: 909 Gerrit-PatchSet: 1 Gerrit-Owner: plaisthos <arn...@rf...> Gerrit-Reviewer: cron2 <ge...@gr...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: plaisthos <arn...@rf...> Gerrit-Attention: flichtenheld <fr...@li...> Gerrit-Comment-Date: Mon, 24 Mar 2025 18:38:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment |
| From: Gert D. <ge...@gr...> - 2025-03-24 18:27:49 |
'lport <anything>' used to trigger 'do socket bind', which is not useful in itself for the 'lport 0' case (port 0 -> OS assigns a random port, as it is done for unbound sockets) unless also binding to a particular local IP address ('--local 192.0.2.1'). The trigger for 'lport has been used, do socket bind' is ce.local_port_defined -> change the code to test for "0", and only set this for non-0 ports (NOTE: this is a string match, so if you really really want the old "lport 0" behaviour, using "lport 00" still does that...). The ce.local_port value is still set, so '--lport 0' together with '--local 192.0.2.1' will give you a random port number bound to that IP address - without 'lport 0' it would default to 1194 or the value of '--port' (if not using '--rport'). Summary: socket bind is now only done if one of these is set - --port <port> with <port> not "0" - --bind (default on the client is "--nobind") - --local <address> Change-Id: I1976307a7643c82f31d55ca32c79cbe64b6fffc6 Signed-off-by: Gert Doering <ge...@gr...> Acked-by: Arne Schwabe <arn...@rf...> --- 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/+/916 This mail reflects revision 1 of this Change. Acked-by according to Gerrit (reflected above): Arne Schwabe <arn...@rf...> diff --git a/doc/man-sections/link-options.rst b/doc/man-sections/link-options.rst index d48021e..287473e 100644 --- a/doc/man-sections/link-options.rst +++ b/doc/man-sections/link-options.rst @@ -122,7 +122,9 @@ --lport port Set default TCP/UDP port number. Cannot be used together with - ``--nobind`` option. + ``--nobind`` option. A port number of ``0`` is only honoured to + achieve "bind() to a random assigned port number" if a bind-to IP + address is specified with ``--local``. --mark value Mark encrypted packets being sent with ``value``. The mark value can be diff --git a/src/openvpn/options.c b/src/openvpn/options.c index ab56609..99dd60a 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -6710,7 +6710,12 @@ else if (streq(p[0], "lport") && p[1] && !p[2]) { VERIFY_PERMISSION(OPT_P_GENERAL|OPT_P_CONNECTION); - options->ce.local_port_defined = true; + + /* only trigger bind() if port is not 0 (or --local is used) */ + if (!streq(p[1], "0")) + { + options->ce.local_port_defined = true; + } options->ce.local_port = p[1]; } else if (streq(p[0], "rport") && p[1] && !p[2]) |
| From: ordex (C. Review) <ge...@op...> - 2025-03-24 16:47:40 |
Attention is currently required from: cron2, flichtenheld. ordex has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/916?usp=email ) Change subject: Make 'lport 0' no longer sufficient to do '--bind'. ...................................................................... Patch Set 1: (1 comment) File src/openvpn/options.c: http://gerrit.openvpn.net/c/openvpn/+/916/comment/e5ea9251_3915c842 : PS1, Line 6719: options->ce.local_port = p[1]; what's the purpose of keeping this line outside the ifblock? -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/916?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: I1976307a7643c82f31d55ca32c79cbe64b6fffc6 Gerrit-Change-Number: 916 Gerrit-PatchSet: 1 Gerrit-Owner: cron2 <ge...@gr...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-CC: ordex <an...@ma...> Gerrit-Attention: cron2 <ge...@gr...> Gerrit-Attention: flichtenheld <fr...@li...> Gerrit-Comment-Date: Mon, 24 Mar 2025 16:47:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment |
| From: plaisthos (C. Review) <ge...@op...> - 2025-03-24 15:46:15 |
Attention is currently required from: cron2, flichtenheld. plaisthos has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/916?usp=email ) Change subject: Make 'lport 0' no longer sufficient to do '--bind'. ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/916?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: I1976307a7643c82f31d55ca32c79cbe64b6fffc6 Gerrit-Change-Number: 916 Gerrit-PatchSet: 1 Gerrit-Owner: cron2 <ge...@gr...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: cron2 <ge...@gr...> Gerrit-Attention: flichtenheld <fr...@li...> Gerrit-Comment-Date: Mon, 24 Mar 2025 15:45:59 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment |
| From: David S. <daz...@eu...> - 2025-03-24 15:38:45 |
On 18/03/2025 11:37, Marc Leeman wrote: > trivial spelling error raised by lintian. Thank you for the fix! And sorry for the slow response. It got handled last week, but forgot to reply and do the final push to the public repos. -------------------------------------------------------------------------- commit 27ec3ecdccebf946e8578d6a2b3123209cffe508 Author: Marc Leeman <mar...@gm...> Date: Tue Mar 18 11:37:33 2025 +0100 spelling: Fix typ0 in gdbuspp/authz-request.cpp Signed-off-by: Marc Leeman <mar...@gm...> -------------------------------------------------------------------------- -- kind regards, David Sommerseth OpenVPN Inc |
| From: cron2 (C. Review) <ge...@op...> - 2025-03-24 15:26:28 |
Attention is currently required from: flichtenheld, plaisthos. Hello plaisthos, flichtenheld, I'd like you to do a code review. Please visit http://gerrit.openvpn.net/c/openvpn/+/916?usp=email to review the following change. Change subject: Make 'lport 0' no longer sufficient to do '--bind'. ...................................................................... Make 'lport 0' no longer sufficient to do '--bind'. 'lport <anything>' used to trigger 'do socket bind', which is not useful in itself for the 'lport 0' case (port 0 -> OS assigns a random port, as it is done for unbound sockets) unless also binding to a particular local IP address ('--local 192.0.2.1'). The trigger for 'lport has been used, do socket bind' is ce.local_port_defined -> change the code to test for "0", and only set this for non-0 ports (NOTE: this is a string match, so if you really really want the old "lport 0" behaviour, using "lport 00" still does that...). The ce.local_port value is still set, so '--lport 0' together with '--local 192.0.2.1' will give you a random port number bound to that IP address - without 'lport 0' it would default to 1194 or the value of '--port' (if not using '--rport'). Summary: socket bind is now only done if one of these is set - --port <port> with <port> not "0" - --bind (default on the client is "--nobind") - --local <address> Change-Id: I1976307a7643c82f31d55ca32c79cbe64b6fffc6 Signed-off-by: Gert Doering <ge...@gr...> --- M doc/man-sections/link-options.rst M src/openvpn/options.c 2 files changed, 9 insertions(+), 2 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/16/916/1 diff --git a/doc/man-sections/link-options.rst b/doc/man-sections/link-options.rst index d48021e..287473e 100644 --- a/doc/man-sections/link-options.rst +++ b/doc/man-sections/link-options.rst @@ -122,7 +122,9 @@ --lport port Set default TCP/UDP port number. Cannot be used together with - ``--nobind`` option. + ``--nobind`` option. A port number of ``0`` is only honoured to + achieve "bind() to a random assigned port number" if a bind-to IP + address is specified with ``--local``. --mark value Mark encrypted packets being sent with ``value``. The mark value can be diff --git a/src/openvpn/options.c b/src/openvpn/options.c index ab56609..99dd60a 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -6710,7 +6710,12 @@ else if (streq(p[0], "lport") && p[1] && !p[2]) { VERIFY_PERMISSION(OPT_P_GENERAL|OPT_P_CONNECTION); - options->ce.local_port_defined = true; + + /* only trigger bind() if port is not 0 (or --local is used) */ + if (!streq(p[1], "0")) + { + options->ce.local_port_defined = true; + } options->ce.local_port = p[1]; } else if (streq(p[0], "rport") && p[1] && !p[2]) -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/916?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: I1976307a7643c82f31d55ca32c79cbe64b6fffc6 Gerrit-Change-Number: 916 Gerrit-PatchSet: 1 Gerrit-Owner: cron2 <ge...@gr...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: plaisthos <arn...@rf...> Gerrit-Attention: flichtenheld <fr...@li...> Gerrit-MessageType: newchange |
| From: flichtenheld (C. Review) <ge...@op...> - 2025-03-24 14:04:00 |
flichtenheld has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/913?usp=email ) Change subject: Use USER_PASS_LEN instead of TLS_USERNAME_LEN for override-username ...................................................................... Patch Set 3: -Code-Review -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/913?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: I1c2c050dd160746a0f8d9c234abe1e258bc8e48d Gerrit-Change-Number: 913 Gerrit-PatchSet: 3 Gerrit-Owner: plaisthos <arn...@rf...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Comment-Date: Mon, 24 Mar 2025 14:03:49 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment |
| From: cron2 (C. Review) <ge...@op...> - 2025-03-24 14:00:49 |
cron2 has submitted this change. ( http://gerrit.openvpn.net/c/openvpn/+/914?usp=email ) Change subject: Improve documentation for override-username ...................................................................... Improve documentation for override-username - Mention that pushing auth-token-user only happens when OpenVPN also generates the auth-token. - mention that OpenVPN will only accept the original and overridden username from a client - suggest to use auth-token-user when a user generates the auth-token Change-Id: Ifc7443974345042ab9945d6a10e1d1b4525e5e05 Signed-off-by: Arne Schwabe <ar...@rf...> Acked-by: Frank Lichtenheld <fr...@li...> Message-Id: <202...@gr...> URL: https://www.mail-archive.com/ope...@li.../msg31210.html Signed-off-by: Gert Doering <ge...@gr...> --- M doc/man-sections/server-options.rst 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/doc/man-sections/server-options.rst b/doc/man-sections/server-options.rst index e93b04d..ccc1374 100644 --- a/doc/man-sections/server-options.rst +++ b/doc/man-sections/server-options.rst @@ -432,7 +432,12 @@ The changed username will be picked up by the status output and also by the ``--auth-gen-token`` option. It will also be pushed to the client - using ``--auth-token-user``. + using ``--auth-token-user`` if ``--auth-gen-token`` is enabled. + + Internally on all subsequent renegotiations the client provided username + will be replaced by the username provided by ``--override-username``. + If the client changes to a username that is different from both the initial + and the overridden username, the client will be rejected. Special care should be taken that both the initial username of the client and the overridden username are handled correctly when using @@ -444,6 +449,10 @@ can be used for ``--auth-gen-token`` to allow providing a username in these scenarios. + If the ``--auth-token`` directive is pushed by another script/plugin or + management interface, consider also generating and pushing + ``--auth-token-user``. + --port-share args Share OpenVPN TCP with another service -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/914?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: Ifc7443974345042ab9945d6a10e1d1b4525e5e05 Gerrit-Change-Number: 914 Gerrit-PatchSet: 2 Gerrit-Owner: plaisthos <arn...@rf...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-MessageType: merged |
| From: cron2 (C. Review) <ge...@op...> - 2025-03-24 14:00:32 |
cron2 has uploaded a new patch set (#2) to the change originally created by plaisthos. ( http://gerrit.openvpn.net/c/openvpn/+/914?usp=email ) The following approvals got outdated and were removed: Code-Review+2 by flichtenheld Change subject: Improve documentation for override-username ...................................................................... Improve documentation for override-username - Mention that pushing auth-token-user only happens when OpenVPN also generates the auth-token. - mention that OpenVPN will only accept the original and overridden username from a client - suggest to use auth-token-user when a user generates the auth-token Change-Id: Ifc7443974345042ab9945d6a10e1d1b4525e5e05 Signed-off-by: Arne Schwabe <ar...@rf...> Acked-by: Frank Lichtenheld <fr...@li...> Message-Id: <202...@gr...> URL: https://www.mail-archive.com/ope...@li.../msg31210.html Signed-off-by: Gert Doering <ge...@gr...> --- M doc/man-sections/server-options.rst 1 file changed, 10 insertions(+), 1 deletion(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/14/914/2 diff --git a/doc/man-sections/server-options.rst b/doc/man-sections/server-options.rst index e93b04d..ccc1374 100644 --- a/doc/man-sections/server-options.rst +++ b/doc/man-sections/server-options.rst @@ -432,7 +432,12 @@ The changed username will be picked up by the status output and also by the ``--auth-gen-token`` option. It will also be pushed to the client - using ``--auth-token-user``. + using ``--auth-token-user`` if ``--auth-gen-token`` is enabled. + + Internally on all subsequent renegotiations the client provided username + will be replaced by the username provided by ``--override-username``. + If the client changes to a username that is different from both the initial + and the overridden username, the client will be rejected. Special care should be taken that both the initial username of the client and the overridden username are handled correctly when using @@ -444,6 +449,10 @@ can be used for ``--auth-gen-token`` to allow providing a username in these scenarios. + If the ``--auth-token`` directive is pushed by another script/plugin or + management interface, consider also generating and pushing + ``--auth-token-user``. + --port-share args Share OpenVPN TCP with another service -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/914?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: Ifc7443974345042ab9945d6a10e1d1b4525e5e05 Gerrit-Change-Number: 914 Gerrit-PatchSet: 2 Gerrit-Owner: plaisthos <arn...@rf...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-MessageType: newpatchset |
| From: Gert D. <ge...@gr...> - 2025-03-24 13:59:54 |
Documentation-only, nothing to test. (Stared at the change nonetheless, as we had discussions on some of the behavioural details - change makes sense) Your patch has been applied to the master branch. commit 5c1c57684b6a1e6bce24605d55fe8dc3d9d3480e Author: Arne Schwabe Date: Mon Mar 24 14:54:33 2025 +0100 Improve documentation for override-username Signed-off-by: Arne Schwabe <ar...@rf...> Acked-by: Frank Lichtenheld <fr...@li...> Message-Id: <202...@gr...> URL: https://www.mail-archive.com/ope...@li.../msg31210.html Signed-off-by: Gert Doering <ge...@gr...> -- kind regards, Gert Doering |
| From: Gert D. <ge...@gr...> - 2025-03-24 13:54:49 |
From: Arne Schwabe <ar...@rf...> - Mention that pushing auth-token-user only happens when OpenVPN also generates the auth-token. - mention that OpenVPN will only accept the original and overridden username from a client - suggest to use auth-token-user when a user generates the auth-token Change-Id: Ifc7443974345042ab9945d6a10e1d1b4525e5e05 Signed-off-by: Arne Schwabe <ar...@rf...> Acked-by: Frank Lichtenheld <fr...@li...> --- 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/+/914 This mail reflects revision 1 of this Change. Acked-by according to Gerrit (reflected above): Frank Lichtenheld <fr...@li...> diff --git a/doc/man-sections/server-options.rst b/doc/man-sections/server-options.rst index e93b04d..ccc1374 100644 --- a/doc/man-sections/server-options.rst +++ b/doc/man-sections/server-options.rst @@ -432,7 +432,12 @@ The changed username will be picked up by the status output and also by the ``--auth-gen-token`` option. It will also be pushed to the client - using ``--auth-token-user``. + using ``--auth-token-user`` if ``--auth-gen-token`` is enabled. + + Internally on all subsequent renegotiations the client provided username + will be replaced by the username provided by ``--override-username``. + If the client changes to a username that is different from both the initial + and the overridden username, the client will be rejected. Special care should be taken that both the initial username of the client and the overridden username are handled correctly when using @@ -444,6 +449,10 @@ can be used for ``--auth-gen-token`` to allow providing a username in these scenarios. + If the ``--auth-token`` directive is pushed by another script/plugin or + management interface, consider also generating and pushing + ``--auth-token-user``. + --port-share args Share OpenVPN TCP with another service |
| From: cron2 (C. Review) <ge...@op...> - 2025-03-24 13:54:25 |
cron2 has uploaded a new patch set (#3) to the change originally created by plaisthos. ( http://gerrit.openvpn.net/c/openvpn/+/915?usp=email ) The following approvals got outdated and were removed: Code-Review+2 by flichtenheld Change subject: Directly use _countof in array initialisation ...................................................................... Directly use _countof in array initialisation This fixes the build failures on MSVC cl compiler. MSVC cl does not thinks of the expression of a const variable times an integer to be compile time static. C23 introduce the constexpr (like in C++) statement for that but we are only on C11 for now. So directly use the _countof(msg->addr) expression in the array initialisation. Change-Id: Ib579c1538eb5440bb7008bc866a5cb7d74844374 Signed-off-by: Arne Schwabe <ar...@rf...> Acked-by: Frank Lichtenheld <fr...@li...> Message-Id: <202...@gr...> URL: https://www.mail-archive.com/ope...@li.../msg31205.html Signed-off-by: Gert Doering <ge...@gr...> --- M src/openvpnserv/interactive.c 1 file changed, 3 insertions(+), 1 deletion(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/15/915/3 diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c index e64ac30..c6963b3 100644 --- a/src/openvpnserv/interactive.c +++ b/src/openvpnserv/interactive.c @@ -1901,7 +1901,9 @@ if (msg->addr_len > 0) { /* prepare the comma separated address list */ - CHAR addrs[max_addrs * 64]; /* 64 is enough for one IPv4/6 address */ + /* cannot use max_addrs here as that is not considered compile + * time constant by all compilers and constexpr is C23 */ + CHAR addrs[_countof(msg->addr) * 64]; /* 64 is enough for one IPv4/6 address */ size_t offset = 0; for (int i = 0; i < addr_len; ++i) { -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/915?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: Ib579c1538eb5440bb7008bc866a5cb7d74844374 Gerrit-Change-Number: 915 Gerrit-PatchSet: 3 Gerrit-Owner: plaisthos <arn...@rf...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-MessageType: newpatchset |
| From: cron2 (C. Review) <ge...@op...> - 2025-03-24 13:54:21 |
cron2 has submitted this change. ( http://gerrit.openvpn.net/c/openvpn/+/915?usp=email ) Change subject: Directly use _countof in array initialisation ...................................................................... Directly use _countof in array initialisation This fixes the build failures on MSVC cl compiler. MSVC cl does not thinks of the expression of a const variable times an integer to be compile time static. C23 introduce the constexpr (like in C++) statement for that but we are only on C11 for now. So directly use the _countof(msg->addr) expression in the array initialisation. Change-Id: Ib579c1538eb5440bb7008bc866a5cb7d74844374 Signed-off-by: Arne Schwabe <ar...@rf...> Acked-by: Frank Lichtenheld <fr...@li...> Message-Id: <202...@gr...> URL: https://www.mail-archive.com/ope...@li.../msg31205.html Signed-off-by: Gert Doering <ge...@gr...> --- M src/openvpnserv/interactive.c 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c index e64ac30..c6963b3 100644 --- a/src/openvpnserv/interactive.c +++ b/src/openvpnserv/interactive.c @@ -1901,7 +1901,9 @@ if (msg->addr_len > 0) { /* prepare the comma separated address list */ - CHAR addrs[max_addrs * 64]; /* 64 is enough for one IPv4/6 address */ + /* cannot use max_addrs here as that is not considered compile + * time constant by all compilers and constexpr is C23 */ + CHAR addrs[_countof(msg->addr) * 64]; /* 64 is enough for one IPv4/6 address */ size_t offset = 0; for (int i = 0; i < addr_len; ++i) { -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/915?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: Ib579c1538eb5440bb7008bc866a5cb7d74844374 Gerrit-Change-Number: 915 Gerrit-PatchSet: 3 Gerrit-Owner: plaisthos <arn...@rf...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-MessageType: merged |
| From: Gert D. <ge...@gr...> - 2025-03-24 13:53:43 |
Thanks. Back to old-style const expressions, then... Your patch has been applied to the master branch. commit 1fc44d3b2ad3b96405650754a6e43f1576cda132 Author: Arne Schwabe Date: Mon Mar 24 14:37:53 2025 +0100 Directly use _countof in array initialisation Signed-off-by: Arne Schwabe <ar...@rf...> Acked-by: Frank Lichtenheld <fr...@li...> Message-Id: <202...@gr...> URL: https://www.mail-archive.com/ope...@li.../msg31205.html Signed-off-by: Gert Doering <ge...@gr...> -- kind regards, Gert Doering |
| From: plaisthos (C. Review) <ge...@op...> - 2025-03-24 13:44:01 |
Attention is currently required from: flichtenheld. Hello flichtenheld, I'd like you to reexamine a change. Please visit http://gerrit.openvpn.net/c/openvpn/+/913?usp=email to look at the new patch set (#3). Change subject: Use USER_PASS_LEN instead of TLS_USERNAME_LEN for override-username ...................................................................... Use USER_PASS_LEN instead of TLS_USERNAME_LEN for override-username Currently override-username is artificially restricted to the length of TLS common-name (64) for the corner case of using username-as-common-name, which we explicitly do not recommend to use. Do away with that limitation and only error out on longer usernames when username-as-common-name is actually in effect. Change-Id: I1c2c050dd160746a0f8d9c234abe1e258bc8e48d Signed-off-by: Arne Schwabe <ar...@rf...> --- M src/openvpn/multi.c M src/openvpn/options.c M src/openvpn/ssl_verify.c M src/openvpn/ssl_verify.h 4 files changed, 42 insertions(+), 7 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/13/913/3 diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c index a673ec1..a2d3fd1 100644 --- a/src/openvpn/multi.c +++ b/src/openvpn/multi.c @@ -2705,6 +2705,12 @@ if (!multi->locked_original_username && strcmp(multi->locked_username, options->override_username) != 0) { + /* Check if the username length is acceptable */ + if (!ssl_verify_username_length(session, options->override_username)) + { + return false; + } + multi->locked_original_username = multi->locked_username; multi->locked_username = strdup(options->override_username); diff --git a/src/openvpn/options.c b/src/openvpn/options.c index ab56609..f89fc7d 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -7875,10 +7875,10 @@ else if (streq(p[0], "override-username") && p[1] && !p[2]) { VERIFY_PERMISSION(OPT_P_INSTANCE); - if (strlen(p[1]) > TLS_USERNAME_LEN) + if (strlen(p[1]) > USER_PASS_LEN) { msg(msglevel, "override-username exceeds the maximum length of %d " - "characters", TLS_USERNAME_LEN); + "characters", USER_PASS_LEN); /* disable the connection since ignoring the request to * set another username might cause serious problems */ diff --git a/src/openvpn/ssl_verify.c b/src/openvpn/ssl_verify.c index 5f8f1d3..d2cc3d1 100644 --- a/src/openvpn/ssl_verify.c +++ b/src/openvpn/ssl_verify.c @@ -1568,6 +1568,24 @@ } } +bool +ssl_verify_username_length(struct tls_session *session, const char *username) +{ + if ((session->opt->ssl_flags & SSLF_USERNAME_AS_COMMON_NAME) + && strlen(username) > TLS_USERNAME_LEN) + { + msg(D_TLS_ERRORS, + "TLS Auth Error: --username-as-common name specified and " + "username is longer than the maximum permitted Common Name " + "length of %d characters", TLS_USERNAME_LEN); + return false; + } + else + { + return true; + } +} + /** * Main username/password verification entry point * @@ -1689,15 +1707,12 @@ } /* check sizing of username if it will become our common name */ - if ((session->opt->ssl_flags & SSLF_USERNAME_AS_COMMON_NAME) - && strlen(up->username)>TLS_USERNAME_LEN) + if (!ssl_verify_username_length(session, up->username)) { - msg(D_TLS_ERRORS, - "TLS Auth Error: --username-as-common name specified and username is longer than the maximum permitted Common Name length of %d characters", - TLS_USERNAME_LEN); plugin_status = OPENVPN_PLUGIN_FUNC_ERROR; script_status = OPENVPN_PLUGIN_FUNC_ERROR; } + /* auth succeeded? */ bool plugin_ok = plugin_status == OPENVPN_PLUGIN_FUNC_SUCCESS || plugin_status == OPENVPN_PLUGIN_FUNC_DEFERRED; diff --git a/src/openvpn/ssl_verify.h b/src/openvpn/ssl_verify.h index eba3832..7a4d44a 100644 --- a/src/openvpn/ssl_verify.h +++ b/src/openvpn/ssl_verify.h @@ -192,6 +192,20 @@ struct tls_session *session); +/** + * Checks if the username length is valid to use. This checks when + * username-as-common-name is active if the username is shorter than + * the maximum TLS common name length (64). + * + * It will also display an error message if the name is too long + * + * @param session current TLS session + * @param username username to check + * @return true if name is under limit or username-as-common-name + * is not active + */ +bool ssl_verify_username_length(struct tls_session *session, + const char *username); /** * Runs the --client-crresponse script if one is defined. -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/913?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: I1c2c050dd160746a0f8d9c234abe1e258bc8e48d Gerrit-Change-Number: 913 Gerrit-PatchSet: 3 Gerrit-Owner: plaisthos <arn...@rf...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: flichtenheld <fr...@li...> Gerrit-MessageType: newpatchset |
| From: plaisthos (C. Review) <ge...@op...> - 2025-03-24 13:42:56 |
Attention is currently required from: flichtenheld. plaisthos has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/913?usp=email ) Change subject: Use USER_PASS_LEN instead of TLS_USERNAME_LEN for override-username ...................................................................... Patch Set 2: (1 comment) File src/openvpn/ssl_verify.c: http://gerrit.openvpn.net/c/openvpn/+/913/comment/973c8450_1d433ea8 : PS2, Line 1710: if (ssl_verify_username_length(session, up->username)) > Missing "not"? Acknowledged -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/913?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: I1c2c050dd160746a0f8d9c234abe1e258bc8e48d Gerrit-Change-Number: 913 Gerrit-PatchSet: 2 Gerrit-Owner: plaisthos <arn...@rf...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: flichtenheld <fr...@li...> Gerrit-Comment-Date: Mon, 24 Mar 2025 13:42:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: flichtenheld <fr...@li...> Gerrit-MessageType: comment |
| From: flichtenheld (C. Review) <ge...@op...> - 2025-03-24 13:39:16 |
Attention is currently required from: plaisthos. flichtenheld has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/913?usp=email ) Change subject: Use USER_PASS_LEN instead of TLS_USERNAME_LEN for override-username ...................................................................... Patch Set 2: Code-Review-2 (1 comment) File src/openvpn/ssl_verify.c: http://gerrit.openvpn.net/c/openvpn/+/913/comment/471048b9_b3cd29b2 : PS2, Line 1710: if (ssl_verify_username_length(session, up->username)) Missing "not"? -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/913?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: I1c2c050dd160746a0f8d9c234abe1e258bc8e48d Gerrit-Change-Number: 913 Gerrit-PatchSet: 2 Gerrit-Owner: plaisthos <arn...@rf...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: plaisthos <arn...@rf...> Gerrit-Comment-Date: Mon, 24 Mar 2025 13:38:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment |
| From: Gert D. <ge...@gr...> - 2025-03-24 13:38:22 |
From: Arne Schwabe <ar...@rf...> This fixes the build failures on MSVC cl compiler. MSVC cl does not thinks of the expression of a const variable times an integer to be compile time static. C23 introduce the constexpr (like in C++) statement for that but we are only on C11 for now. So directly use the _countof(msg->addr) expression in the array initialisation. Change-Id: Ib579c1538eb5440bb7008bc866a5cb7d74844374 Signed-off-by: Arne Schwabe <ar...@rf...> Acked-by: Frank Lichtenheld <fr...@li...> --- 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/+/915 This mail reflects revision 2 of this Change. Acked-by according to Gerrit (reflected above): Frank Lichtenheld <fr...@li...> diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c index e64ac30..c6963b3 100644 --- a/src/openvpnserv/interactive.c +++ b/src/openvpnserv/interactive.c @@ -1901,7 +1901,9 @@ if (msg->addr_len > 0) { /* prepare the comma separated address list */ - CHAR addrs[max_addrs * 64]; /* 64 is enough for one IPv4/6 address */ + /* cannot use max_addrs here as that is not considered compile + * time constant by all compilers and constexpr is C23 */ + CHAR addrs[_countof(msg->addr) * 64]; /* 64 is enough for one IPv4/6 address */ size_t offset = 0; for (int i = 0; i < addr_len; ++i) { |
| From: flichtenheld (C. Review) <ge...@op...> - 2025-03-24 13:36:29 |
Attention is currently required from: plaisthos. flichtenheld has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/914?usp=email ) Change subject: Improve documentation for override-username ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/914?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: Ifc7443974345042ab9945d6a10e1d1b4525e5e05 Gerrit-Change-Number: 914 Gerrit-PatchSet: 1 Gerrit-Owner: plaisthos <arn...@rf...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: plaisthos <arn...@rf...> Gerrit-Comment-Date: Mon, 24 Mar 2025 13:36:14 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment |
| From: flichtenheld (C. Review) <ge...@op...> - 2025-03-24 13:32:26 |
Attention is currently required from: plaisthos. flichtenheld has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/915?usp=email ) Change subject: Directly use _countof in array initialisation ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/915?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: Ib579c1538eb5440bb7008bc866a5cb7d74844374 Gerrit-Change-Number: 915 Gerrit-PatchSet: 2 Gerrit-Owner: plaisthos <arn...@rf...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: plaisthos <arn...@rf...> Gerrit-Comment-Date: Mon, 24 Mar 2025 13:32:07 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment |
| From: plaisthos (C. Review) <ge...@op...> - 2025-03-24 13:19:49 |
Attention is currently required from: flichtenheld. Hello flichtenheld, I'd like you to reexamine a change. Please visit http://gerrit.openvpn.net/c/openvpn/+/915?usp=email to look at the new patch set (#2). Change subject: Directly use _countof in array initialisation ...................................................................... Directly use _countof in array initialisation This fixes the build failures on MSVC cl compiler. MSVC cl does not thinks of the expression of a const variable times an integer to be compile time static. C23 introduce the constexpr (like in C++) statement for that but we are only on C11 for now. So directly use the _countof(msg->addr) expression in the array initialisation. Change-Id: Ib579c1538eb5440bb7008bc866a5cb7d74844374 Signed-off-by: Arne Schwabe <ar...@rf...> --- M src/openvpnserv/interactive.c 1 file changed, 3 insertions(+), 1 deletion(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/15/915/2 diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c index e64ac30..c6963b3 100644 --- a/src/openvpnserv/interactive.c +++ b/src/openvpnserv/interactive.c @@ -1901,7 +1901,9 @@ if (msg->addr_len > 0) { /* prepare the comma separated address list */ - CHAR addrs[max_addrs * 64]; /* 64 is enough for one IPv4/6 address */ + /* cannot use max_addrs here as that is not considered compile + * time constant by all compilers and constexpr is C23 */ + CHAR addrs[_countof(msg->addr) * 64]; /* 64 is enough for one IPv4/6 address */ size_t offset = 0; for (int i = 0; i < addr_len; ++i) { -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/915?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: Ib579c1538eb5440bb7008bc866a5cb7d74844374 Gerrit-Change-Number: 915 Gerrit-PatchSet: 2 Gerrit-Owner: plaisthos <arn...@rf...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: flichtenheld <fr...@li...> Gerrit-MessageType: newpatchset |
| From: plaisthos (C. Review) <ge...@op...> - 2025-03-24 13:18:44 |
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/+/915?usp=email to review the following change. Change subject: Directly use _countof in array initialisation ...................................................................... Directly use _countof in array initialisation This fixes the build failures on MSVC cl compiler. MSVC cl does not thinks of the expression of a const variable times an integer to be compile time static. C23 introduce the constexpr (like in C++) statement for that but we are only on C11 for now. So directly use the _countof(msg->addr) expression in the array initialisation. Change-Id: Ib579c1538eb5440bb7008bc866a5cb7d74844374 Signed-off-by: Arne Schwabe <ar...@rf...> --- M src/openvpnserv/interactive.c 1 file changed, 4 insertions(+), 1 deletion(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/15/915/1 diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c index e64ac30..abc2cda 100644 --- a/src/openvpnserv/interactive.c +++ b/src/openvpnserv/interactive.c @@ -34,6 +34,7 @@ #include <shellapi.h> #include <mstcpip.h> #include <inttypes.h> +#include <assert.h> #include <versionhelpers.h> @@ -1901,7 +1902,9 @@ if (msg->addr_len > 0) { /* prepare the comma separated address list */ - CHAR addrs[max_addrs * 64]; /* 64 is enough for one IPv4/6 address */ + /* cannot use max_addrs here as that is not considered compile + * time constant by all compilers and constexpr is C23 */ + CHAR addrs[_countof(msg->addr) * 64]; /* 64 is enough for one IPv4/6 address */ size_t offset = 0; for (int i = 0; i < addr_len; ++i) { -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/915?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: Ib579c1538eb5440bb7008bc866a5cb7d74844374 Gerrit-Change-Number: 915 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-MessageType: newchange |