Should start openvpn with "--service exit-event" and stop openvpn instances with that event

[mattock]

This would allow openvpn to do cleanup before getting killed. While openvpn.exe usually recovers fine from a crash (or a forced kill), that is not always the case. For example, with latest Git "master" code, IPv6 routes can be left behind, and these can prevent subsequent VPN connections from setting their own. This is of course a problem in OpenVPN itself, but it would make sense for openvpnserv2 to try to avoid situations like this.

Searching the man-page for "exit-event" will show how it works. Another alternative is to use the management interface ("signal SIGTERM") like openvpn-gui does, but that is probably an overkill (and over-complicated) in case of openvpnserv2.

[mattock]

@xkjyeah : openvpnserv2 is now "live", bundled as the default in OpenVPN 2.4_alpha2 installers. It would be good to get the exit-event code implemented out before people start complaining too much. The automatic part of old openvpnserv also makes use of exit-events (see automatic.c).

[mattock]

@xkjyeah : can you fix this or shall I have a look?

[xkjyeah]

Sorry, I haven't had time to work on OpenVpnServ recently -- I set it up
for a friend and use it myself only rarely.

Since you seem pretty clear how it should be fixed and tested, I think I
will leave you to do it.

On Nov 1, 2016 20:23, "Samuli Seppänen" [email protected] wrote:

@xkjyeah https://github.com/xkjyeah : can you fix this or shall I have
a look?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#10 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ACiTR3rQin7zA3Kv6cZXdJ0uQssL8LWcks5q5y8vgaJpZM4KY8X1
.

[mattock]

Soon you will have millions of new friends using openvpnserv2 for 😄 . Anyways, I think I will be able to fix this, even though I'm not definitely a Windows / C# developer.

[selvanair]

@mattock : I took a stab at it (see https://github.com/selvanair/openvpnserv2/tree/exit-event). Not being a C# developer myself, a very critical review is needed. If excessive changes are required feel free to use any useful snippets from the patch.

It basically sets the event and then waits on each process to exit for a maximum of 2500msec (cumulative for all processes) and kills the process if still running. The wait is to allow time for the client to send exit-notify (if any) to the server etc. 2500 msec is based on our prior experience with setting timeout in nssm.

[mattock]

@selvanair : ok, I will have a look and do some testing early next week. I've written some C# but Windows internals are an uncharted territory for me.

We might actually want to make OpenVPN/openvpnserv2 the main project to simplify things. I do have write access here, but I probably can't grant permissions to others. Plus, if we take over maintenance from xkjyeah, then OpenVPN/openvpnserv2 is the more correct place for tracking development imho.

[selvanair]

OK, in that case I can issue a PR directed to OpenVPN/openvpnserv2 after your testing. Could you please enable "issues" in that repo so that the discussion can be moved there as well?

[selvanair]

Added a commit the branch mentioned above (selvanair@ea69146) to auto flush the log file. Without this its hard to test the exit-event patch as the "signal received message" may not appear in the logs otherwise.

Should also fix issue 4.

[mattock]

@selvanair: Based on OpenVPN logs the exit-event tree does seem to deliver on its promise. In OpenVPN logs I see that routes are being torned down, TUN/TAP is closed down, etc. A great improvement to what we had, even though OpenVPN was surprisingly tolerant to being killed without notice, based on my Powershell test suite. I will run the exit-event tree -based openvpnserv2 through the full testsuite later today, when I have access to my Windows laptop again.

I also enabled issues in OpenVPN/openvpnserv2, so we can start tracking things there.

[selvanair]

See this PR

[mattock]

@xkjyeah : would you mind reviewing the proposed fix for this ticket from a C# perspective? I already tested the fix and it does what it promises. We'd prefer to get the fixed version into OpenVPN 2.4_beta1 (due by Friday) with code review rather than without.