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/+/1798?usp=email
to review the following change.
Change subject: options: limit ping and keepalive values to one day
......................................................................
options: limit ping and keepalive values to one day
A correct configuration should not require such large ping intervals or
timeouts, and an upper limit of one day is already generous.
Limit --ping, --ping-exit, --ping-restart and --keepalive values to
86400 seconds. Since --keepalive doubles the timeout in server mode,
limit its timeout argument to 43200 seconds there.
Related:
https://lore.kernel.org/all/[email protected]/
Change-Id: Ib18e1ed0851bc0fe6d8448e601d11d6abaa710c1
Signed-off-by: Marco Baffo <[email protected]>
---
M doc/man-sections/link-options.rst
M src/openvpn/helper.c
M src/openvpn/options.c
M src/openvpn/options.h
4 files changed, 27 insertions(+), 6 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/98/1798/1
diff --git a/doc/man-sections/link-options.rst
b/doc/man-sections/link-options.rst
index df8c917..77c3c41 100644
--- a/doc/man-sections/link-options.rst
+++ b/doc/man-sections/link-options.rst
@@ -74,7 +74,9 @@
keepalive interval timeout
Send ping once every ``interval`` seconds, restart if ping is not received
- for ``timeout`` seconds.
+ for ``timeout`` seconds. Both values are limited to 86400 seconds. Since the
+ server-side timeout is doubled, ``timeout`` is limited to 43200 seconds in
+ server mode.
This option can be used on both client and server side, but it is enough
to add this on the server side as it will push appropriate ``--ping``
@@ -247,6 +249,8 @@
cause ping packets to be sent in both directions since OpenVPN ping
packets are not echoed like IP ping packets).
+ The maximum value for ``n`` is 86400 seconds.
+
This option has two intended uses:
(1) Compatibility with stateful firewalls. The periodic ping will ensure
@@ -264,6 +268,8 @@
``--inactive``, ``--ping`` and ``--ping-exit`` to create a two-tiered
inactivity disconnect.
+ The maximum value for ``n`` is 86400 seconds.
+
For example,
::
@@ -278,6 +284,8 @@
``n`` seconds pass without reception of a ping or other packet from
remote.
+ The maximum value for ``n`` is 86400 seconds.
+
This option is useful in cases where the remote peer has a dynamic IP
address and a low-TTL DNS name is used to track the IP address using a
service such as https://www.nsupdate.info/ + a dynamic DNS client such as
diff --git a/src/openvpn/helper.c b/src/openvpn/helper.c
index 4c540a6..6fd2689 100644
--- a/src/openvpn/helper.c
+++ b/src/openvpn/helper.c
@@ -563,6 +563,12 @@
msg(M_USAGE,
"--keepalive conflicts with --ping, --ping-exit, or
--ping-restart. If you use --keepalive, you don't need any of the other --ping
directives.");
}
+ if (o->mode == MODE_SERVER && o->keepalive_timeout > PING_TIMEOUT_MAX
/ 2)
+ {
+ msg(M_USAGE,
+ "The second parameter to --keepalive must not exceed %d in
server mode.",
+ PING_TIMEOUT_MAX / 2);
+ }
/*
* Expand.
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 87218d4..003b460 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -6789,24 +6789,29 @@
else if (streq(p[0], "keepalive") && p[1] && p[2] && !p[3])
{
VERIFY_PERMISSION(OPT_P_GENERAL);
- atoi_constrained(p[1], &options->keepalive_ping, "keepalive ping", 1,
INT_MAX, msglevel);
- atoi_constrained(p[2], &options->keepalive_timeout, "keepalive
timeout", 1, INT_MAX, msglevel);
+ atoi_constrained(p[1], &options->keepalive_ping, "keepalive ping",
+ 1, PING_TIMEOUT_MAX, msglevel);
+ atoi_constrained(p[2], &options->keepalive_timeout, "keepalive
timeout",
+ 1, PING_TIMEOUT_MAX, msglevel);
}
else if (streq(p[0], "ping") && p[1] && !p[2])
{
VERIFY_PERMISSION(OPT_P_TIMER);
- options->ping_send_timeout = positive_atoi(p[1], msglevel);
+ atoi_constrained(p[1], &options->ping_send_timeout, p[0],
+ 0, PING_TIMEOUT_MAX, msglevel);
}
else if (streq(p[0], "ping-exit") && p[1] && !p[2])
{
VERIFY_PERMISSION(OPT_P_TIMER);
- options->ping_rec_timeout = positive_atoi(p[1], msglevel);
+ atoi_constrained(p[1], &options->ping_rec_timeout, p[0],
+ 0, PING_TIMEOUT_MAX, msglevel);
options->ping_rec_timeout_action = PING_EXIT;
}
else if (streq(p[0], "ping-restart") && p[1] && !p[2])
{
VERIFY_PERMISSION(OPT_P_TIMER);
- options->ping_rec_timeout = positive_atoi(p[1], msglevel);
+ atoi_constrained(p[1], &options->ping_rec_timeout, p[0],
+ 0, PING_TIMEOUT_MAX, msglevel);
options->ping_rec_timeout_action = PING_RESTART;
}
else if (streq(p[0], "ping-timer-rem") && !p[1])
diff --git a/src/openvpn/options.h b/src/openvpn/options.h
index a111cf8..2f7fe30 100644
--- a/src/openvpn/options.h
+++ b/src/openvpn/options.h
@@ -56,6 +56,8 @@
#define OPTION_PARM_SIZE 256
#define OPTION_LINE_SIZE 256
+#define PING_TIMEOUT_MAX 86400 /* one day (in seconds) */
+
extern const char title_string[];
/* certain options are saved before --pull modifications are applied */
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1798?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: Ib18e1ed0851bc0fe6d8448e601d11d6abaa710c1
Gerrit-Change-Number: 1798
Gerrit-PatchSet: 1
Gerrit-Owner: mrbff <[email protected]>
Gerrit-Reviewer: plaisthos <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: plaisthos <[email protected]>
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
