Hi,

On Wed, Jan 27, 2016 at 10:14:18AM +0200, Samuli Seppänen wrote:
An added bonus is that openvpnserv2 is written in C#, which means it can
be developed on Linux using Mono, and the language choice probably helps
getting new contributions from people not fluent with C.

I'm not totally convinced that "mixing in a new language" is a *bonus*
(as it means that the core team won't be able to help unless also fluent
with the other language).

Well, I just reviewed all of the code with no difficulty, so would I have no trouble co-maintaining it. Openvpnserv2 is built on top of generic Service C# classes provided by Microsoft and overrides a few methods such as OnStart, OnStop, and sets some some capabilities based on what OpenVPN processes can (be ordered to) do. For an overview of the ServiceBase class look here:

<https://msdn.microsoft.com/en-us/library/system.serviceprocess.servicebase%28v=vs.110%29.aspx>

The core of openvpnserv2 is in this file:

<https://github.com/xkjyeah/openvpnserv2/blob/master/Service.cs>

The remaining files are mostly stuff added by the IDE. The only thing that openvpnserv2 really adds on top of generic MS service classes is:

- loading of OpenVPN registry keys
- building the command-line(s) to launch the connection(s)
- monitoring the individual OpenVPN processes and restart them if they crash
- stopping the processes before suspend
- starting the processes on resume

That said, the code does not seem to handle 32/64-bit registry keys yet. This needs to be fixed, unless there is some underlying magic in there I missed. Adding support for selecting which connections to launch automatically would also be nice, e.g. via registry or a config file.

I would not be worried about current core developers not being able to help with openvpnserv2. First, they haven't really been able to help with openvpnserv.exe, either. If they had, we wouldn't be having this discussion in the first place. I would be inclined to think that instead of overloading the existing C maintainers we should try to get _new_ people to maintain the Windows service part.

I don't see a problem with moving to C# for a number of reasons:

- The codebase pretty small
- Most of the hard lifting is done in the classes maintained by MS
- There is already a maintainer for this code
- There is a volunteering co-maintainer (=me) in the core group
- C# is easier than C to understand/develop (imho), if you already know a high-level language (especially Java, but also Python etc.)

Given that services run with maximum privileges, strong review is as
important there as for core openvpn...

Yes, agreed. But note that most of the functionality is stock functionality provided by Microsoft in their libraries, so there's not much to review. As said, I reviewed the code already, but of course it does not hurt if an experienced C# developer has a second look.

One thing to bear in mind is that making horrible mistakes (e.g. create a gaping security hole) with C# is more difficult than with C, so while review is of course necessary, every single line does not have to picked apart for potential vulnerabilities.

If the only reason why everyone is disliking the old openvpnserv is
"it is not restarting openvpn.exe when it breaks" - *that* should be
fairly easy to add.  So, what is that people consider "broken"?

It does not work at all in Windows 10 afaik, and only barely works in Windows 8.1. It does not handle suspend/resume. It is not maintained, and there are no maintainers, and I don't see any maintainers appearing. There are a bunch of tickets in Trac related to openvpnserv.exe, directly or indirectly. Interestingly openvpnserv.exe has been dropped from the OpenVPN Chocolatey package[1], apparently because it's fairly useless nowadays.

--
Samuli Seppänen
Community Manager
OpenVPN Technologies, Inc

irc freenode net: mattock


[1] <https://chocolatey.org/packages/openvpn>

Reply via email to