[Openvpn-devel] [PATCH applied] Re: Preparing for better signal handling: some code refactoring
Acked-by: Gert Doering (Lightly) tested on Linux, FreeBSD - namely, does it compile, pass t_client tests (which send SIGTERM), pass t_server tests (which uses per-instance SIGUSR1/SIGTERM). Plus "push to github, does it break windows?". Stared at the code for a long time - most of it is really straightforward replacement, but the "easy" ones tend to create oversights. I'm not sure I fully understand the nuances why we went from "c" to "c->sig", but I assume this is due to "definition of 'c' not available in all the places that need the new code". But even if the latter could be made possible (opaque struct pointer etc), having "just the signal stuff" passed to register_signal() sounds more clean anyway. The continued existance of "volatile int *signal_received" in dco_connect_wait() confused me at first, but is needed for get_signal() (and that is needed for "do not set signal if there already is one", which should be covered much more nicely by 4/5... not there yet). Generally speaking, using signal_reset() and register_signal() everywhere is much nicer, especially (old) code like this -c->sig->signal_received = SIGUSR1; -c->sig->signal_text = "remote-exit"; +register_signal(c->sig, SIGUSR1, "remote-exit"); .. should have never been accepted in the first place... The /* */ comment here is arguably wrong, and was wrong before... @@ -555,8 +554,8 @@ send_push_request(struct context *c) .. msg(D_STREAM_ERRORS, "No reply from server to push requests in %ds", .. +/* SOFT-SIGUSR1 -- server-pushed connection reset */ +register_signal(c->sig, SIGUSR1, "no-push-reply"); I've left this on the list for a few days so people had the chance to stand up and complain "this is all the wrong way to do it" - nobody came up, so I consider this as "no objection". Your patch has been applied to the master and release/2.6 branch. commit 05715485b45816e18b52ffb9b47ca22a55abb334 (master) commit 264ce74c409018f42b178ba2cab544bdcecb1767 (release/2.6) Author: Selva Nair Date: Sun Jan 1 16:51:05 2023 -0500 Preparing for better signal handling: some code refactoring Signed-off-by: Selva Nair Acked-by: Gert Doering Message-Id: <20230101215109.1521549-2-selva.n...@gmail.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25874.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Refactor signal handling in openvpn_getaddrinfo
Acked-by: Gert Doering
This is basically the same thing as 1/5, just for the getaddrinfo()
related functions. And no, I have no idea why they are treated "special",
and getaddrinfo() is not a syscall. OTOH, getaddrinfo() spends most of
its time in "waiting for DNS packets", which might result in visible
behaviour similar to a restarted/restartable syscall on signals coming
in... the "let's rewrite that thing" part would be 2.7 material, though.
The code changes look reasonable ("straightforward as 1/5"), the test
rigs still works I do not have specific "test signal handling / timeout
in DNS" test cases today, though.
Your patch has been applied to the master and release/2.6 branch.
commit eff95d500481c7927c5a9edd6b5c0dfa056a0cbb (master)
commit 1ff87267f819b512c34a202b9a023c76e7031af4 (release/2.6)
Author: Selva Nair
Date: Sun Jan 1 16:51:06 2023 -0500
Refactor signal handling in openvpn_getaddrinfo
Signed-off-by: Selva Nair
Acked-by: Gert Doering
Message-Id: <20230101215109.1521549-3-selva.n...@gmail.com>
URL:
https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg25872.html
Signed-off-by: Gert Doering
--
kind regards,
Gert Doering
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v2 4/5] Fix signal handling on Windows
Hi,
> -else if (n > 0)
> +else
> {
> -sleep(n);
> +#ifdef _WIN32
> +win32_sleep(n);
> +#else
> +if (n > 0)
> +{
> +sleep(n);
My understanding is that we want to have interruptible sleep. In this case,
what is the point of calling win32_sleep(0) ? I understand that
management_sleep(0)
is required to service possible management commands even if we don't
need a delay,
but shouldn't it be a no-op without management?
> +void
> +win32_sleep(const int n)
> +{
> +if (n < 0)
> +{
> +return;
> +}
Why not <= 0 ?
> +/* Sleep() is not interruptible. Use a WAIT_OBJECT to catch signal */
> +if (HANDLE_DEFINED(win32_signal.in.read))
> +{
I would reverse this condition, and do just
Sleep(n*1000);
return;
This way we get rid of the indentation level.
> +time_t expire = 0;
> +update_time();
> +expire = now + n;
I would declare and initialize time_t expire after update_time().
> +DWORD status = WaitForSingleObject(win32_signal.in.read, (expire
> - now)*1000);
> +if (win32_signal_get(&win32_signal) || status == WAIT_TIMEOUT)
> +{
> +return;
> +}
Shouldn't we call win32_signal_get() only if status == WAIT_OBJECT_0 ?
Under what circumstances are we supposed to do the waiting again? If
we get a signal, we bail out.
If the wait times out, we bail out. If wait fails, we do Sleep
(although at this point we probably have a bigger issue).
-Lev
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v2 4/5] Fix signal handling on Windows
Hi,
Thanks for the careful review
On Thu, Jan 5, 2023 at 11:20 AM Lev Stipakov wrote:
> Hi,
>
> > -else if (n > 0)
> > +else
> > {
> > -sleep(n);
> > +#ifdef _WIN32
> > +win32_sleep(n);
> > +#else
> > +if (n > 0)
> > +{
> > +sleep(n);
>
>
> My understanding is that we want to have interruptible sleep. In this case,
> what is the point of calling win32_sleep(0) ?
I understand that
> management_sleep(0)
> is required to service possible management commands even if we don't
> need a delay,
> but shouldn't it be a no-op without management?
>
Right, I'm going beyond the original which does nothing for
management_sleep(0) if management is not active or enabled.
I want to err on the side of checking windows signal rather than missing
it. In case of unix/linux OS signals do get delivered out of band into the
global signal_received. In the case of Windows, it has to be picked up
(except for the obscure ctrl-Break).
Note that management_sleep(0) when management is enabled does call
get_signal()==win32_signal_get and picks up Windows signals, not just
management commands. win32_sleep(0) also calls win32_signal_get() even if
there is no time to pause.
>
> > +void
> > +win32_sleep(const int n)
> > +{
> > +if (n < 0)
> > +{
> > +return;
> > +}
>
> Why not <= 0 ?
>
See above.
>
> > +/* Sleep() is not interruptible. Use a WAIT_OBJECT to catch signal
> */
> > +if (HANDLE_DEFINED(win32_signal.in.read))
> > +{
>
> I would reverse this condition, and do just
>
> Sleep(n*1000);
> return;
> This way we get rid of the indentation level.
>
> > +time_t expire = 0;
> > +update_time();
> > +expire = now + n;
>
> I would declare and initialize time_t expire after update_time().
> > +DWORD status = WaitForSingleObject(win32_signal.in.read,
> (expire - now)*1000);
> > +if (win32_signal_get(&win32_signal) || status ==
> WAIT_TIMEOUT)
> > +{
> > +return;
> > +}
>
> Shouldn't we call win32_signal_get() only if status == WAIT_OBJECT_0 ?
>
To ensure the signal is checked at least once in all cases including n = 0.
Not sure whether status will be WAIT_TIMEOUT or WAIT_OBJECT_0 in case n = 0
and there is a signal to pickup.
>
> Under what circumstances are we supposed to do the waiting again? If
> we get a signal, we bail out.
> If the wait times out, we bail out. If wait fails, we do Sleep
> (although at this point we probably have a bigger issue).
>
Probably never -- there is a WAIT_ABANDONED which may not apply here. But,
sometimes windows functions do exit with error codes not listed in the
docs. As the loop is slowed down by a Sleep(1000), this can't hurt, can it?
The common thread in all of the above is that I'm trying to err on the side
of checking for signals an extra round as opposed to missing it. Can do a
v3 if you think it's warranted.
Thanks,
Selva
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH] Add connect-freq-initial option to limit initial connection responses
This limits the nubmer of packets OpenVPN will respond to. This avoid
OpenVPN server being abused for refelection attacks in a large scale
as we gotten a lot more efficient with the cookie approach in our
initial connection approach.
The defaults of 100 attempts per 10s should work for most people,
esepcially since completed three way handshakes are not counted. So
the default will throttle connection attempts on server with high packet
loss or that are actually under a DOS.
The 100 per 10s are similar in size to the old 2.5 and earlier behaviour
where every initial connection attempt would take up a slot of the
max-clients sessions and those would only expire after the TLS timeout.
This roughly translates to 1024 connection attempts in 60s on an
empty server.
OpenVPN will announce once per period how many packets it dropped:
Dropped 11 initial handshake packets due to --connect-freq-initial 100 10
to inform an amdin about the consequence of this feature.
Signed-off-by: Arne Schwabe
---
Changes.rst | 4 ++
doc/man-sections/server-options.rst | 19 ++
src/openvpn/Makefile.am | 1 +
src/openvpn/mudp.c | 14 +
src/openvpn/multi.c | 4 ++
src/openvpn/multi.h | 2 +
src/openvpn/options.c | 24
src/openvpn/options.h | 5 ++
src/openvpn/reflect_filter.c| 91 +
src/openvpn/reflect_filter.h| 70 ++
10 files changed, 234 insertions(+)
create mode 100644 src/openvpn/reflect_filter.c
create mode 100644 src/openvpn/reflect_filter.h
diff --git a/Changes.rst b/Changes.rst
index 47933ae09..187d03fcf 100644
--- a/Changes.rst
+++ b/Changes.rst
@@ -92,6 +92,10 @@ Cookie based handshake for UDP server
shake. The tls-crypt-v2 option allows controlling if older clients are
accepted.
+By default the rate of initial packet responses is limited to 100 per 10s
+interval to avoid OpenVPN servers being abused in reflection attacks
+(see ``--connect-freq-initial``).
+
Data channel offloading with ovpn-dco
2.6.0+ implements support for data-channel offloading where the data
packets
are directly processed and forwarded in kernel space thanks to the ovpn-dco
diff --git a/doc/man-sections/server-options.rst
b/doc/man-sections/server-options.rst
index 99263fff3..cbb2c3c92 100644
--- a/doc/man-sections/server-options.rst
+++ b/doc/man-sections/server-options.rst
@@ -184,6 +184,25 @@ fast hardware. SSL/TLS authentication must be used in this
mode.
For the best protection against DoS attacks in server mode, use
``--proto udp`` and either ``--tls-auth`` or ``--tls-crypt``.
+--connect-freq-initial args
+ (UDP only) Allow a maximum of ``n`` initial connection packet responses
+ per ``sec`` seconds from to clients.
+
+ Valid syntax:
+ ::
+
+ connect-freq-initial n sec
+
+ OpenVPN starting at 2.6 is very efficient in responding to initial
+ connection packets. When not limiting the initial responses
+ an OpenVPN daemon can be abused in reflection attacks.
+ This option is designed to limit the rate OpenVPN will respond to initial
+ attacks.
+
+ Connection attempts that complete the initial three-way handshake
+ will not be counted against the limit. The default is to allow
+ 100 initial connection per 10s.
+
--duplicate-cn
Allow multiple clients with the same common name to concurrently
connect. In the absence of this option, OpenVPN will disconnect a client
diff --git a/src/openvpn/Makefile.am b/src/openvpn/Makefile.am
index e80a35abd..35d60a65b 100644
--- a/src/openvpn/Makefile.am
+++ b/src/openvpn/Makefile.am
@@ -113,6 +113,7 @@ openvpn_SOURCES = \
ps.c ps.h \
push.c push.h \
pushlist.h \
+ reflect_filter.c reflect_filter.h \
reliable.c reliable.h \
route.c route.h \
run_command.c run_command.h \
diff --git a/src/openvpn/mudp.c b/src/openvpn/mudp.c
index c27c6da5b..4ef2f355b 100644
--- a/src/openvpn/mudp.c
+++ b/src/openvpn/mudp.c
@@ -82,6 +82,16 @@ do_pre_decrypt_check(struct multi_context *m,
struct openvpn_sockaddr *from = &m->top.c2.from.dest;
int handwindow = m->top.options.handshake_window;
+if (verdict == VERDICT_VALID_RESET_V3 || verdict == VERDICT_VALID_RESET_V2)
+{
+/* Check if we are still below our limit for sending out
+ * responses */
+if (!reflect_filter_rate_limit_check(m->initial_rate_limiter))
+{
+return false;
+}
+}
+
if (verdict == VERDICT_VALID_RESET_V3)
{
/* Extract the packet id to check if it has the special format that
@@ -244,6 +254,10 @@ multi_get_create_instance_udp(struct multi_context *m,
bool *floated)
if (frequency_limit_event_allowed(m->new_connection_limiter))
{
+/* a successful three-way handshake only counts against
+
Re: [Openvpn-devel] [PATCH v2 4/5] Fix signal handling on Windows
Hi > >> >> Under what circumstances are we supposed to do the waiting again? If >> we get a signal, we bail out. >> If the wait times out, we bail out. If wait fails, we do Sleep >> (although at this point we probably have a bigger issue). >> > > Probably never -- there is a WAIT_ABANDONED which may not apply here. But, > sometimes windows functions do exit with error codes not listed in the > docs. As the loop is slowed down by a Sleep(1000), this can't hurt, can it? > I see your point now :) I had a different logic earlier where it always slept for a second before trying to wait again, but not so any longer. v3 is coming! Selva > ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] Add connect-freq-initial option to limit initial connection responses
Hi,
first of all, feature-ACK, and "structure-ACK" - this is a lightweight
implementation that can go into 2.6 with not much testing, which should
"get the thing done" for most cases. A more sophisticated implementation
with per-IP hashing (to avoid (ab-)using the rate-limiter to drown-out
legitimate clients) can follwo for 2.7.
This said, most of the code looks good, except for this part:
On Thu, Jan 05, 2023 at 06:56:14PM +0100, Arne Schwabe wrote:
> index ee3783046..cf8622f46 100644
> --- a/src/openvpn/options.c
> +++ b/src/openvpn/options.c
> @@ -864,6 +865,8 @@ init_options(struct options *o, const bool init_gc)
> o->n_bcast_buf = 256;
> o->tcp_queue_limit = 64;
> o->max_clients = 1024;
> +o->cf_initial_per = 10;
> +o->cf_initial_max = 100;
... so this is on-by-default...
> @@ -2613,6 +2618,10 @@ options_postprocess_verify_ce(const struct options
> *options,
> {
> msg(M_USAGE, "--connect-freq only works with --mode server
> --proto udp. Try --max-clients instead.");
> }
> +if (!proto_is_udp(ce->proto) && (options->cf_initial_max ||
> options->cf_initial_per))
> +{
> +msg(M_USAGE, "--connect-freq only works with --mode server
> --proto udp. Try --max-clients instead.");
> +}
... and if it is enabled for !UDP modes, we'll refuse cooperation.
But wouldn't that trigger for *all* TCP servers now, since it defaults
to "active"?
Besides that, the "--connect-freq" there is wrong and needs to be
"--connect-freq-initial" ;-)
[..]
> +else if (streq(p[0], "connect-freq-initial") && p[1] && p[2] && !p[3])
> +{
> +int cf_max, cf_per;
> +
> +VERIFY_PERMISSION(OPT_P_GENERAL);
> +cf_max = atoi(p[1]);
> +cf_per = atoi(p[2]);
> +if (cf_max < 0 || cf_per < 0)
" < 0 " or " <= 0"?
> +{
> +msg(msglevel, "--connect-freq-initial parms must be > 0");
> +goto err;
> +}
... because the inverse of ">0" is not "<0"...
> +options->cf_initial_max = cf_max;
> +options->cf_initial_per = cf_per;
> +}
... now we could of course document "0 turns it off", but then the
message needs to be ">= 0".
> +bool
> +reflect_filter_rate_limit_check(struct initial_packet_rate_limit *irl)
> +{
> +if (now > irl->last_period_reset + irl->period_length)
> +{
> +int dropped = max_int(irl->curr_period_counter -
> irl->max_per_period, 0);
> +if (dropped > 0)
> +{
> +msg(D_TLS_DEBUG_LOW, "Dropped %d initial handshake packets due
> to "
> +"--connect-freq-initial %d %d",
> +dropped, irl->max_per_period, irl->period_length);
> +}
> +irl->last_period_reset = now;
> +irl->curr_period_counter = 0;
> +}
> +/* avoid overflow by capping the counter to INT_MAX -1. Still counting
> + * the number of packets during a period is still useful */
> +irl->curr_period_counter = min_int(irl->curr_period_counter + 1, INT_MAX
> - 1);
This looks uglysh.
With a 10-seconds bucket, an (32bit) "int" would overflow at about
430 million packets per seconds - which is, I think, way beyond what
any reasonable system can handle (10G ethernet saturates at 15 Mpps).
But if you are really worried about overflows - why not use a (64bit)
long here?
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 g...@greenie.muc.de
signature.asc
Description: PGP signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v3 4/5] Fix signal handling on Windows
From: Selva Nair
- In win32_signal_get() re-order the check so that Windows
signals are picked up even if signal_received is non-zero
- When management is not active, management_sleep() becomes sleep()
but it is not interruptible by signals on Windows. Fix this by
periodically checking for signal.
Fixes Trac #311 #639 (windows specific part)
Github: Fixes OpenVPN/openvpn#205 (windows specific part)
Note: if stuck in address resolution, press ctrl-C and wait for getaddrinfo()
to timeout.
v2: WIN32 --> _WIN32
add a chunk in management_sleep that was missed by sloppy
conflict-resolution
v3: following review by Lev Stipakov
win32_sleep()
- Early fallback to Sleep() if no wait handles -- less indentation
- Check signal only if wait-object triggered
- Exit the while loop if not safe to continue
Behaviour of win32_sleep(0) checking signal is retained though may be
redundant
Signed-off-by: Selva Nair
---
More on why the while loop is required for the wait:
The wait handle in.read can trigger at any key-board input. Even with ctrl-C,
it first triggers for the ctrl keypress and only on a second round when keypress
of "C" triggers it again we get it parsed as SIGINT/SIGTERM by
win32_signal_get().
To make this safer, and not overload the process with repeated waits,
we now filter out any status other than WAIT_OBJECT_0, not just WAIT_FAILED.
That should be safe enough.
src/openvpn/manage.c | 16 +++-
src/openvpn/win32.c | 98 +---
src/openvpn/win32.h | 3 ++
3 files changed, 81 insertions(+), 36 deletions(-)
diff --git a/src/openvpn/manage.c b/src/openvpn/manage.c
index 5465b7e9..9bd5ef4e 100644
--- a/src/openvpn/manage.c
+++ b/src/openvpn/manage.c
@@ -4045,9 +4045,16 @@ management_sleep(const int n)
{
management_event_loop_n_seconds(management, n);
}
-else if (n > 0)
+else
{
-sleep(n);
+#ifdef _WIN32
+win32_sleep(n);
+#else
+if (n > 0)
+{
+sleep(n);
+}
+#endif
}
}
@@ -4088,13 +4095,18 @@ man_persist_client_stats(struct management *man, struct
context *c)
#else /* ifdef ENABLE_MANAGEMENT */
+#include "win32.h"
void
management_sleep(const int n)
{
+#ifdef _WIN32
+win32_sleep(n);
+#else
if (n > 0)
{
sleep(n);
}
+#endif /* ifdef _WIN32 */
}
#endif /* ENABLE_MANAGEMENT */
diff --git a/src/openvpn/win32.c b/src/openvpn/win32.c
index c3520bca..b82f28b9 100644
--- a/src/openvpn/win32.c
+++ b/src/openvpn/win32.c
@@ -642,50 +642,44 @@ int
win32_signal_get(struct win32_signal *ws)
{
int ret = 0;
-if (siginfo_static.signal_received)
-{
-ret = siginfo_static.signal_received;
-}
-else
+
+if (ws->mode == WSO_MODE_SERVICE)
{
-if (ws->mode == WSO_MODE_SERVICE)
+if (win32_service_interrupt(ws))
{
-if (win32_service_interrupt(ws))
-{
-ret = SIGTERM;
-}
+ret = SIGTERM;
}
-else if (ws->mode == WSO_MODE_CONSOLE)
+}
+else if (ws->mode == WSO_MODE_CONSOLE)
+{
+switch (win32_keyboard_get(ws))
{
-switch (win32_keyboard_get(ws))
-{
-case 0x3B: /* F1 -> USR1 */
-ret = SIGUSR1;
-break;
+case 0x3B: /* F1 -> USR1 */
+ret = SIGUSR1;
+break;
-case 0x3C: /* F2 -> USR2 */
-ret = SIGUSR2;
-break;
+case 0x3C: /* F2 -> USR2 */
+ret = SIGUSR2;
+break;
-case 0x3D: /* F3 -> HUP */
-ret = SIGHUP;
-break;
+case 0x3D: /* F3 -> HUP */
+ret = SIGHUP;
+break;
-case 0x3E: /* F4 -> TERM */
-ret = SIGTERM;
-break;
+case 0x3E: /* F4 -> TERM */
+ret = SIGTERM;
+break;
-case 0x03: /* CTRL-C -> TERM */
-ret = SIGTERM;
-break;
-}
-}
-if (ret)
-{
-throw_signal(ret); /* this will update signinfo_static.signal
received */
+case 0x03: /* CTRL-C -> TERM */
+ret = SIGTERM;
+break;
}
}
-return ret;
+if (ret)
+{
+throw_signal(ret); /* this will update signinfo_static.signal received
*/
+}
+return (siginfo_static.signal_received);
}
void
@@ -1603,4 +1597,40 @@ set_openssl_env_vars()
}
}
+void
+win32_sleep(const int n)
+{
+if (n < 0)
+{
+return;
+}
+
+/* Sleep() is not interruptible. Use a WAIT_OBJECT to catch signal */
+if (!HANDLE_DEFINED(win32_signal.in.read))
+{
+Sleep(n*1000); /* n=0 will just yield if there are threads
[Openvpn-devel] [PATCH v4 4/5] Fix signal handling on Windows
From: Selva Nair
- In win32_signal_get() re-order the check so that Windows
signals are picked up even if signal_received is non-zero
- When management is not active, management_sleep() becomes sleep()
but it is not interruptible by signals on Windows. Fix this by
periodically checking for signal.
Fixes Trac #311 #639 (windows specific part)
Github: Fixes OpenVPN/openvpn#205 (windows specific part)
Note: if stuck in address resolution, press ctrl-C and wait for getaddrinfo()
to timeout.
v2: WIN32 --> _WIN32
add a chunk in management_sleep that was missed by sloppy
conflict-resolution
v3: following review by Lev Stipakov
win32_sleep()
- Early fallback to Sleep() if no wait handles -- less indentation
- Check signal only if wait-object triggered
- Exit the while loop if not safe to continue
Behaviour of win32_sleep(0) checking signal is retained though may be
redundant
v4: Avoid Sleep(0) and never loop back to wait again if wait-failed
Signed-off-by: Selva Nair
---
More on why the while loop is required for the wait:
The wait handle in.read can trigger at any key-board input. Even with ctrl-C,
it first triggers for the ctrl key-press and only on a second round when
key-press of "C" triggers it again we get it parsed as SIGINT/SIGTERM by
win32_signal_get().
To make this safer, and not overload the process with repeated waits,
we now filter out any status other than WAIT_OBJECT_0, not just WAIT_FAILED.
That should be safe enough.
src/openvpn/manage.c | 16 ++-
src/openvpn/win32.c | 105 +--
src/openvpn/win32.h | 3 ++
3 files changed, 88 insertions(+), 36 deletions(-)
diff --git a/src/openvpn/manage.c b/src/openvpn/manage.c
index 5465b7e9..9bd5ef4e 100644
--- a/src/openvpn/manage.c
+++ b/src/openvpn/manage.c
@@ -4045,9 +4045,16 @@ management_sleep(const int n)
{
management_event_loop_n_seconds(management, n);
}
-else if (n > 0)
+else
{
-sleep(n);
+#ifdef _WIN32
+win32_sleep(n);
+#else
+if (n > 0)
+{
+sleep(n);
+}
+#endif
}
}
@@ -4088,13 +4095,18 @@ man_persist_client_stats(struct management *man, struct
context *c)
#else /* ifdef ENABLE_MANAGEMENT */
+#include "win32.h"
void
management_sleep(const int n)
{
+#ifdef _WIN32
+win32_sleep(n);
+#else
if (n > 0)
{
sleep(n);
}
+#endif /* ifdef _WIN32 */
}
#endif /* ENABLE_MANAGEMENT */
diff --git a/src/openvpn/win32.c b/src/openvpn/win32.c
index c3520bca..569e086e 100644
--- a/src/openvpn/win32.c
+++ b/src/openvpn/win32.c
@@ -642,50 +642,44 @@ int
win32_signal_get(struct win32_signal *ws)
{
int ret = 0;
-if (siginfo_static.signal_received)
-{
-ret = siginfo_static.signal_received;
-}
-else
+
+if (ws->mode == WSO_MODE_SERVICE)
{
-if (ws->mode == WSO_MODE_SERVICE)
+if (win32_service_interrupt(ws))
{
-if (win32_service_interrupt(ws))
-{
-ret = SIGTERM;
-}
+ret = SIGTERM;
}
-else if (ws->mode == WSO_MODE_CONSOLE)
+}
+else if (ws->mode == WSO_MODE_CONSOLE)
+{
+switch (win32_keyboard_get(ws))
{
-switch (win32_keyboard_get(ws))
-{
-case 0x3B: /* F1 -> USR1 */
-ret = SIGUSR1;
-break;
+case 0x3B: /* F1 -> USR1 */
+ret = SIGUSR1;
+break;
-case 0x3C: /* F2 -> USR2 */
-ret = SIGUSR2;
-break;
+case 0x3C: /* F2 -> USR2 */
+ret = SIGUSR2;
+break;
-case 0x3D: /* F3 -> HUP */
-ret = SIGHUP;
-break;
+case 0x3D: /* F3 -> HUP */
+ret = SIGHUP;
+break;
-case 0x3E: /* F4 -> TERM */
-ret = SIGTERM;
-break;
+case 0x3E: /* F4 -> TERM */
+ret = SIGTERM;
+break;
-case 0x03: /* CTRL-C -> TERM */
-ret = SIGTERM;
-break;
-}
-}
-if (ret)
-{
-throw_signal(ret); /* this will update signinfo_static.signal
received */
+case 0x03: /* CTRL-C -> TERM */
+ret = SIGTERM;
+break;
}
}
-return ret;
+if (ret)
+{
+throw_signal(ret); /* this will update signinfo_static.signal received
*/
+}
+return (siginfo_static.signal_received);
}
void
@@ -1603,4 +1597,47 @@ set_openssl_env_vars()
}
}
+void
+win32_sleep(const int n)
+{
+if (n < 0)
+{
+return;
+}
+
+/* Sleep() is not interruptible. Use a WAIT_OBJECT to catch signal */
+
+if (!HANDLE_DEFINED(win32_signal.in.read))
