search

xkjyeah / openvpnserv2

OpenVPN service rewrite
10 stars 41 forks source link

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

Open mattock opened 8 years ago

mattock commented 8 years ago

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 commented 8 years ago

@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 commented 7 years ago

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

xkjyeah commented 7 years ago

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" notifications@github.com 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 https://github.com/xkjyeah/openvpnserv2/issues/10#issuecomment-257553487, or mute the thread https://github.com/notifications/unsubscribe-auth/ACiTR3rQin7zA3Kv6cZXdJ0uQssL8LWcks5q5y8vgaJpZM4KY8X1 .

mattock commented 7 years ago

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

selvanair commented 7 years ago

@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 commented 7 years ago

@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 commented 7 years ago

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 commented 7 years ago

Added a commit the branch mentioned above (https://github.com/selvanair/openvpnserv2/commit/ea691468ed655eb3cfe92f55b0513dd0045a2d4b) 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 commented 7 years ago

@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 commented 7 years ago

See this PR

mattock commented 7 years ago

@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.