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 <[email protected]>
---
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 <[email protected]>
Gerrit-Reviewer: flichtenheld <[email protected]>
Gerrit-Reviewer: plaisthos <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: plaisthos <[email protected]>
Gerrit-Attention: flichtenheld <[email protected]>
Gerrit-MessageType: newchange
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
