[Openvpn-devel,v2] implement --session-timeout
Commit Message
From: Dmitry Zelenkovsky <dmitry@zelenkovsky.com>
Disconnect clients after session-timeout expires.
session-timeout can be defined in ccd files in order to limit
per-user connection time.
Signed-off-by: Dmitry Zelenkovsky <dmitry@zelenkovsky.com>
---
Changes from v1:
* added documentation to manpage
* added entry in Changes.rst
---
Changes.rst | 6 ++++++
doc/man-sections/link-options.rst | 15 +++++++++++++++
doc/man-sections/server-options.rst | 2 +-
src/openvpn/forward.c | 22 ++++++++++++++++++++++
src/openvpn/init.c | 7 +++++++
src/openvpn/openvpn.h | 2 ++
src/openvpn/options.c | 7 +++++++
src/openvpn/options.h | 2 ++
8 files changed, 62 insertions(+), 1 deletion(-)
Comments
Hi,
I have a few documentation / comment language issues, and a bit of
grumbling about the code (which can be ignored).
On Mon, Sep 19, 2022 at 03:41:08PM +0200, Antonio Quartulli wrote:
> +--session-timeout n
> + Raises :code:`SIGTERM` for the client instance after ``n`` seconds since
> + the beginning of the session, forcing OpenVPN to disconnect.
> + In client mode, OpenVPN will disconnect and exit, while in server mode
> + all client sessions are terminated.
> +
> + This option can also be specified in a client instance config file
> + using ``--client-config-dir`` or dynamically generated using a
> + ``--client-connect`` script. In these cases, only the related client
> + session is terminated.
> +
> + When using option on the server side, it may be useful to push
> + ``--explicit-exit-notify`` in order to terminate a client session
> + and be informed about it.
This paragraph is a bit unclear. Why would you want to *push*
--explicit-exit-notify? *Use* it, on the server, so the client gets
an EEN, this I would understand.
Maybe reword this as
"If this is used on the server (main config or per-client), this works
better if ``--explicit-exit-notify`` is also used in the server config,
so clients are not just silently disconnected but get told about it."
> +
> --socket-flags flags
> Apply the given flags to the OpenVPN transport socket. Currently, only
> :code:`TCP_NODELAY` is supported.
> diff --git a/doc/man-sections/server-options.rst b/doc/man-sections/server-options.rst
> index 54ea8b66..9d0c73b6 100644
> --- a/doc/man-sections/server-options.rst
> +++ b/doc/man-sections/server-options.rst
> @@ -426,7 +426,7 @@ fast hardware. SSL/TLS authentication must be used in this mode.
> ``--inactive``, ``--ping``, ``--ping-exit``, ``--ping-restart``,
> ``--setenv``, ``--auth-token``, ``--persist-key``, ``--persist-tun``,
> ``--echo``, ``--comp-lzo``, ``--socket-flags``, ``--sndbuf``,
> - ``--rcvbuf``
> + ``--rcvbuf``, ``--session-timeout``
>
> --push-remove opt
> Selectively remove all ``--push`` options matching "opt" from the option
> diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
> index e5cee665..810cb8a7 100644
> --- a/src/openvpn/forward.c
> +++ b/src/openvpn/forward.c
> @@ -630,6 +630,21 @@ encrypt_sign(struct context *c, bool comp_frag)
> buffer_turnover(orig_buf, &c->c2.to_link, &c->c2.buf, &b->read_tun_buf);
> }
>
> +/*
> + * Should we exit due to session timeout?
> + */
> +static void
> +check_session_timeout(struct context *c)
> +{
> + if (c->options.session_timeout
> + && event_timeout_trigger(&c->c2.session_interval, &c->c2.timeval,
> + ETT_DEFAULT))
> + {
> + msg(M_INFO, "Session timeout, exiting");
> + register_signal(c, SIGTERM, "session-timeout");
> + }
> +}
> +
> /*
> * Coarse timers work to 1 second resolution.
> */
> @@ -681,6 +696,13 @@ process_coarse_timers(struct context *c)
> return;
> }
>
> + /* kill session if time is over */
> + check_session_timeout(c);
> + if (c->sig->signal_received)
> + {
> + return;
> + }
This now-triple "if (c->sig->signal_received)" still rubs my sensitivies,
but I'm not sure if it's worth rewriting check_session_timeout() and
check_ping_restart() to return a bool instead, and make this
if (check_session_timeout(c)) /* game over? */
{
return;
}
if (check_ping_restart(c)) /* peer went to sleep? */
{
return;
}
instead...
But anyway, this is more "material for a general cleanup patch", this
patch is OK as it is, code-wise.
> + int session_timeout; /* Kill session after n seconds, regardless activity */
"regardless activity" is no good, "regardless of activity" is getting a bit
long.
What about "/* force-kill session after n seconds */"?
gert
@@ -93,6 +93,12 @@ Inline auth username and password
missing OpenVPN will prompt for input via stdin. This applies to inline'd
http-proxy-user-pass too.
+Session timeout
+ It is now possible to terminate a session (or all) after a specified amount
+ of seconds has passed session commencement. This behaviour can be configured
+ using ``--session-timeout``. This option can be configured on the server, on
+ the client or can also be pushed.
+
Deprecated features
-------------------
@@ -427,6 +427,21 @@ the local and the remote host.
default) and you are using either ``--secret`` (shared-secret key mode)
or TLS mode with ``--tls-auth``.
+--session-timeout n
+ Raises :code:`SIGTERM` for the client instance after ``n`` seconds since
+ the beginning of the session, forcing OpenVPN to disconnect.
+ In client mode, OpenVPN will disconnect and exit, while in server mode
+ all client sessions are terminated.
+
+ This option can also be specified in a client instance config file
+ using ``--client-config-dir`` or dynamically generated using a
+ ``--client-connect`` script. In these cases, only the related client
+ session is terminated.
+
+ When using option on the server side, it may be useful to push
+ ``--explicit-exit-notify`` in order to terminate a client session
+ and be informed about it.
+
--socket-flags flags
Apply the given flags to the OpenVPN transport socket. Currently, only
:code:`TCP_NODELAY` is supported.
@@ -426,7 +426,7 @@ fast hardware. SSL/TLS authentication must be used in this mode.
``--inactive``, ``--ping``, ``--ping-exit``, ``--ping-restart``,
``--setenv``, ``--auth-token``, ``--persist-key``, ``--persist-tun``,
``--echo``, ``--comp-lzo``, ``--socket-flags``, ``--sndbuf``,
- ``--rcvbuf``
+ ``--rcvbuf``, ``--session-timeout``
--push-remove opt
Selectively remove all ``--push`` options matching "opt" from the option
@@ -630,6 +630,21 @@ encrypt_sign(struct context *c, bool comp_frag)
buffer_turnover(orig_buf, &c->c2.to_link, &c->c2.buf, &b->read_tun_buf);
}
+/*
+ * Should we exit due to session timeout?
+ */
+static void
+check_session_timeout(struct context *c)
+{
+ if (c->options.session_timeout
+ && event_timeout_trigger(&c->c2.session_interval, &c->c2.timeval,
+ ETT_DEFAULT))
+ {
+ msg(M_INFO, "Session timeout, exiting");
+ register_signal(c, SIGTERM, "session-timeout");
+ }
+}
+
/*
* Coarse timers work to 1 second resolution.
*/
@@ -681,6 +696,13 @@ process_coarse_timers(struct context *c)
return;
}
+ /* kill session if time is over */
+ check_session_timeout(c);
+ if (c->sig->signal_received)
+ {
+ return;
+ }
+
/* restart if ping not received */
check_ping_restart(c);
if (c->sig->signal_received)
@@ -1322,6 +1322,13 @@ do_init_timers(struct context *c, bool deferred)
event_timeout_init(&c->c2.inactivity_interval, c->options.inactivity_timeout, now);
}
+ /* initialize inactivity timeout */
+ if (c->options.session_timeout)
+ {
+ event_timeout_init(&c->c2.session_interval, c->options.session_timeout,
+ now);
+ }
+
/* initialize pings */
if (dco_enabled(&c->options))
{
@@ -288,6 +288,8 @@ struct context_2
struct event_timeout inactivity_interval;
int64_t inactivity_bytes;
+ struct event_timeout session_interval;
+
/* the option strings must match across peers */
char *options_string_local;
char *options_string_remote;
@@ -261,6 +261,7 @@ static const char usage_message[] =
" for m seconds.\n"
"--inactive n [bytes] : Exit after n seconds of activity on tun/tap device\n"
" produces a combined in/out byte count < bytes.\n"
+ "--session-timeout n: Limit connection time to n seconds.\n"
"--ping-exit n : Exit if n seconds pass without reception of remote ping.\n"
"--ping-restart n: Restart if n seconds pass without reception of remote ping.\n"
"--ping-timer-rem: Run the --ping-exit/--ping-restart timer only if we have a\n"
@@ -1823,6 +1824,7 @@ show_settings(const struct options *o)
SHOW_INT(keepalive_ping);
SHOW_INT(keepalive_timeout);
SHOW_INT(inactivity_timeout);
+ SHOW_INT(session_timeout);
SHOW_INT64(inactivity_minimum_bytes);
SHOW_INT(ping_send_timeout);
SHOW_INT(ping_rec_timeout);
@@ -6598,6 +6600,11 @@ add_option(struct options *options,
}
}
}
+ else if (streq(p[0], "session-timeout") && p[1] && !p[2])
+ {
+ VERIFY_PERMISSION(OPT_P_TIMER);
+ options->session_timeout = positive_atoi(p[1]);
+ }
else if (streq(p[0], "proto") && p[1] && !p[2])
{
int proto;
@@ -317,6 +317,8 @@ struct options
int inactivity_timeout; /* --inactive */
int64_t inactivity_minimum_bytes;
+ int session_timeout; /* Kill session after n seconds, regardless activity */
+
int ping_send_timeout; /* Send a TCP/UDP ping to remote every n seconds */
int ping_rec_timeout; /* Expect a TCP/UDP ping from remote at least once every n seconds */
bool ping_timer_remote; /* Run ping timer only if we have a remote address */