Re: [Openvpn-devel] [PATCH v6 4/7] wintun: ring buffers based I/O

2019-12-16 Thread Selva Nair
Hi

On Mon, Dec 16, 2019 at 4:31 PM Lev Stipakov  wrote:
>>
>> I have already said what I think of it. As an admin I wouldn't like to see
>> users running processes that elevate to SYSTEM like this.
>
>
> Would it be too much if
>
>  - openvpn.exe process detects that it is not started by interactive service
>  - interactive service process is running
>  - wintun is used as --windows-driver
>

I think this is possible and in fact I had something like this in mind
as well I wrote we could provide an openvpn launcher script.

And I would like to do this even if wintun is in use as ability to run
openvpn.exe as a regular would be a plus for everyone.

> then openvpn behaves as a command-line version of openvpn-gui - connects to 
> service, which
> runs openvpn.exe, and then prints output, received via management interface, 
> to console?

It may be better to redirect the output to the stdout or file as usual
so that no
user visible changes and easier to implement too. Just keep the process id
returned from the service and when the user kills the front-end, terminate the
background process started by the service too.

This should also allow to run the automatic service as LocalService or a
special service user as many services do.

Selva

>
> --
> -Lev


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH v6 4/7] wintun: ring buffers based I/O

2019-12-16 Thread Simon Rozman
Hi,

>>> TLDR:
>>> (i) stealing SYSTEM access from winlogon.exe is not a good thing to do
>> 
>> 
>> This doesn't happen for the majority of use cases - only when iservice is 
>> not used. We also
>> elevate only for the single DeviceIOControl call.
> 
> I understand. But stealing access token from winlogon.exe is a hack and not
> something I would expect to see in a trustworthy executable. Diagnostic
> and forensic tools may be justified in doing such things.

Wintun has a hardcoded check to allow ring registration from the SYSTEM user 
only.

To be honest: I am using a Windows 10 VPC in test mode with a modified Wintun 
driver installed that also allows ring registration for the members of 
Administrators group. This allows me to quickly test ideas while 
reviewing/upgrading Lev's work. I can run Visual Studio to compile openvpn.exe 
on my computer as an unprivileged user. Then have an elevated Remote Debugger 
running on the VPC and Visual Studio to remotely run openvpn.exe with debugger 
attached with a single F5 click.

Having to use OpenVPN GUI and iservice, or running openvpn.exe as service would 
require a lot more clicks to replace the binary, attach debugger etc.

As far as I am concerned, this elevation hack may be removed from the OpenVPN 
source code in the final release. However, mind that this would prevent people 
from running openvpn.exe+Wintun from the command line.

>>> +bool
>>> 
 +impersonate_as_system()
 +{
>>> 
>>> This is implemented by stealing the access token from
>>> winlogon.exe. I don't think such tricks belong to OpenVPN.
>>> It may also trip some anti-virus software.
>>> 
>>> That said, probably there are no "legitimate" ways of getting
>>> LOCAL SYSTEM rights on Windows without running a service.
>>> 
>>> Why does wintun require SYSTEM for using it? If there is a
>>> good reason for that, we should not let every admin
>>> user bypass it.
>> 
>> 
>> I'll defer it to Simon.

Unfortunately, I don't do security decisions in Wintun.

Wintun was originally designed for WireGuard. WireGuard is architectured to run 
all its tunnels as Windows services running as the SYSTEM user. Wintun's 
security is as tight as possible so the WireGuard can barely use it. I know a 
guy who is tempted to introduce a userspace binary code signature check to the 
Wintun. :)

Given the relative ease to get SYSTEM token just by being an elevated process — 
mind there's also a hack to get from non-elevated to elevated completely 
bypassing the UAC prompt as long as you are a member of Administrators — this 
SYSTEM restriction really doesn't provide considerable additional security 
compared to being a member of Administrators group.

>>> Those who really need to test OpenVPN with wintun from
>>> command prompt can use diagnostic tools available to get
>>> a cmd prompt as system (e.g., psexec). That also  makes
>>> it explicit that SYSTEM privilege is required.
>>> 
>>> In the longer run, we could provide a script to launch
>>> openvpn.exe using the interactive service. Modifying the
>>> automatic service to use interactive service for launching
>>> looks easy to do as well. Then, all privileged operations could
>>> be removed from openvpn core.
>> 
>> 
>> I think it is good not to break user experience and allow run openvpn as
>> an administrator without iservice using wintun at the expense on elevation
>> to system for single API call.
> 
> I have already said what I think of it. As an admin I wouldn't like to see
> users running processes that elevate to SYSTEM like this.

Selva, Windows is full of such hacks internally. :( This is no excuse for us 
doing the same of course. Just saying Windows is far from ideal world.

I am running OpenVPN on Windows using NSSM wrapper for years. I had a brief 
discussion on the Hackathon with Samuli about integrating SCM support directly 
into openvpn.exe (imagine --daemon for Windows):

sc create OpenVPN$MyTunnel binpath= "C:\Program Files\OpenVPN\bin\openvpn" 
--daemon --config "C:\Program Files\OpenVPN\config\MyTunnel.ovpn" --log 
"C:\Program Files\OpenVPN\log\MyTunnel.log" start= auto depend= dhcp
sc start OpenVPN$MyTunnel

This would install openvpn.exe as a Windows service and run it as the SYSTEM 
user — no need for iservice, no need for SYSTEM token hack. Other than me, 
perhaps it could cover at least some of the users now running openvpn.exe 
directly.

Regards,
Simon

smime.p7s
Description: S/MIME cryptographic signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH v6 4/7] wintun: ring buffers based I/O

2019-12-16 Thread Lev Stipakov
>
> I have already said what I think of it. As an admin I wouldn't like to see
> users running processes that elevate to SYSTEM like this.
>

Would it be too much if

 - openvpn.exe process detects that it is not started by interactive service
 - interactive service process is running
 - wintun is used as --windows-driver

then openvpn behaves as a command-line version of openvpn-gui - connects to
service, which
runs openvpn.exe, and then prints output, received via management
interface, to console?

-- 
-Lev
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH v6 4/7] wintun: ring buffers based I/O

2019-12-16 Thread Selva Nair
Hi

On Mon, Dec 16, 2019 at 3:01 PM Lev Stipakov  wrote:
>
> Hi,
>
> Thanks for looking into this. See my comments below.
>
>> TLDR:
>> (i) stealing SYSTEM access from winlogon.exe is not a good thing to do
>
>
> This doesn't happen for the majority of use cases - only when iservice is not 
> used. We also
> elevate only for the single DeviceIOControl call.

I understand. But stealing access token from winlogon.exe is a hack and not
something I would expect to see in a trustworthy executable. Diagnostic
and forensic tools may be justified in doing such things.

>
> Below you mentioned psexec as a one of workarounds, but I think
> those will make user experience worse.

I was not suggesting it as an approach for "users" -- only for testing
or for those
who insist running OpenVPN from command line.

> Also consider the case when OpenVPN GUI is running as Administrator.

That would be a wrong way of using the GUI in 2.4 + releases. We need not and
should not (IMO) support the GUI running as admin. That said, now that vista is
no longer supported we can enable the use of iservice with GUI running as admin.

>
>> (ii) with a small change we can support multiple tunnels and  provide better
>> diagnostics when there are no free wintun adapters -- but this could also
>> be a follow up patch.
>
>
> This is already implemented by Simon on top of my patches - see 
> https://github.com/rozmansi/openvpn/commits/wintun
>
>> > -/* for wintun kernel doesn't send DHCP requests, so use ipapi to set 
>> > IP address and netmask */
>>
>> > +/* for wintun kernel doesn't send DHCP requests, so use netsh to set 
>> > IP address and netmask */
>> >  if (options->wintun)
>> >  {
>> > -options->tuntap_options.ip_win32_type = IPW32_SET_IPAPI;
>> > +options->tuntap_options.ip_win32_type = IPW32_SET_NETSH;
>>
>> This shouldn't be required. I would prefer to not use netsh
>> for tasks where API calls are available. We know IPAPI works
>> as we use it in the service.
>
>
> I remember having some issues with IPAPI which got resolved when I switched 
> to netsh.
>
> However, I did some testing and I cannot reproduce IPAPI specific issues 
> anymore. Too bad I didn't investigate
> it further back then. The problem I see now can also be reproduced also with 
> netsh:
>
>  - connect with tap adapter
>  - kill openvpn process
>  - connect with wintun adapter
>
> IPAPI / netsh fails to set IP address, since it is already assigned to 
> another adapter.
>
> If this is OK, I would prefer to submit a separate patch which switches back 
> to netsh. tun.c code
> has changed in follow-up patches (mostly here 
> https://patchwork.openvpn.net/patch/918/), those patches
> are based on an assumption that wintun uses netsh.
>
>>
>> > +msg(M_FATAL, "ERROR:  Failed to register ring buffers: 
>> > %lu", GetLastError());
>>
>> The correct error here is, very likely, the adapter is
>> already in use. To trigger that, why not do register_ring_buffers
>> soon after opening the device and on failure move on to the
>> next device (if any).
>>
>> That will also allow running multiple tunnels with no further changes
>> provided the user manually creates wintum adapters.
>
>
> As mentioned before, this is already implemented by Simon:  
> https://github.com/rozmansi/openvpn/commit/5bc09580e8771d8422feae4504fe2a74b7412fd1
>
>>
>> > +bool
>>
>> > +impersonate_as_system()
>> > +{
>>
>> This is implemented by stealing the access token from
>> winlogon.exe. I don't think such tricks belong to OpenVPN.
>> It may also trip some anti-virus software.
>>
>> That said, probably there are no "legitimate" ways of getting
>> LOCAL SYSTEM rights on Windows without running a service.
>>
>> Why does wintun require SYSTEM for using it? If there is a
>> good reason for that, we should not let every admin
>> user bypass it.
>
>
> I'll defer it to Simon.
>
>>
>>
>> I would suggest to not elevate to system as the code will still
>> work in two important cases
>> (i) started by automatic service
>> (ii) started using interactive service (e.g., through the GUI)
>>
>> Those who really need to test OpenVPN with wintun from
>> command prompt can use diagnostic tools available to get
>> a cmd prompt as system (e.g., psexec). That also  makes
>> it explicit that SYSTEM privilege is required.
>>
>> In the longer run, we could provide a script to launch
>> openvpn.exe using the interactive service. Modifying the
>> automatic service to use interactive service for launching
>> looks easy to do as well. Then, all privileged operations could
>> be removed from openvpn core.
>
>
> I think it is good not to break user experience and allow run openvpn as
> an administrator without iservice using wintun at the expense on elevation
> to system for single API call.

I have already said what I think of it. As an admin I wouldn't like to see
users running processes that elevate to SYSTEM like this.

Selva


___

Re: [Openvpn-devel] [PATCH v6 4/7] wintun: ring buffers based I/O

2019-12-16 Thread Lev Stipakov
Hi,

Thanks for looking into this. See my comments below.

TLDR:
> (i) stealing SYSTEM access from winlogon.exe is not a good thing to do
>

This doesn't happen for the majority of use cases - only when iservice is
not used. We also
elevate only for the single DeviceIOControl call.

Below you mentioned psexec as a one of workarounds, but I think those will
make user experience worse.
Also consider the case when OpenVPN GUI is running as Administrator.

(ii) with a small change we can support multiple tunnels and  provide better
> diagnostics when there are no free wintun adapters -- but this could also
> be a follow up patch.
>

This is already implemented by Simon on top of my patches - see
https://github.com/rozmansi/openvpn/commits/wintun

> -/* for wintun kernel doesn't send DHCP requests, so use ipapi to set
> IP address and netmask */

> +/* for wintun kernel doesn't send DHCP requests, so use netsh to set
> IP address and netmask */
> >  if (options->wintun)
> >  {
> > -options->tuntap_options.ip_win32_type = IPW32_SET_IPAPI;
> > +options->tuntap_options.ip_win32_type = IPW32_SET_NETSH;
>
> This shouldn't be required. I would prefer to not use netsh
> for tasks where API calls are available. We know IPAPI works
> as we use it in the service.
>

I remember having some issues with IPAPI which got resolved when I switched
to netsh.

However, I did some testing and I cannot reproduce IPAPI specific issues
anymore. Too bad I didn't investigate
it further back then. The problem I see now can also be reproduced also
with netsh:

 - connect with tap adapter
 - kill openvpn process
 - connect with wintun adapter

IPAPI / netsh fails to set IP address, since it is already assigned to
another adapter.

If this is OK, I would prefer to submit a separate patch which switches
back to netsh. tun.c code
has changed in follow-up patches (mostly here
https://patchwork.openvpn.net/patch/918/), those patches
are based on an assumption that wintun uses netsh.


> > +msg(M_FATAL, "ERROR:  Failed to register ring buffers:
> %lu", GetLastError());
>
> The correct error here is, very likely, the adapter is
> already in use. To trigger that, why not do register_ring_buffers
> soon after opening the device and on failure move on to the
> next device (if any).
>
> That will also allow running multiple tunnels with no further changes
> provided the user manually creates wintum adapters.
>

As mentioned before, this is already implemented by Simon:
https://github.com/rozmansi/openvpn/commit/5bc09580e8771d8422feae4504fe2a74b7412fd1


> > +bool

> +impersonate_as_system()
> > +{
>
> This is implemented by stealing the access token from
> winlogon.exe. I don't think such tricks belong to OpenVPN.
> It may also trip some anti-virus software.
>
> That said, probably there are no "legitimate" ways of getting
> LOCAL SYSTEM rights on Windows without running a service.
>
> Why does wintun require SYSTEM for using it? If there is a
> good reason for that, we should not let every admin
> user bypass it.
>

I'll defer it to Simon.


>
> I would suggest to not elevate to system as the code will still
> work in two important cases
> (i) started by automatic service
> (ii) started using interactive service (e.g., through the GUI)
>
> Those who really need to test OpenVPN with wintun from
> command prompt can use diagnostic tools available to get
> a cmd prompt as system (e.g., psexec). That also  makes
> it explicit that SYSTEM privilege is required.
>
> In the longer run, we could provide a script to launch
> openvpn.exe using the interactive service. Modifying the
> automatic service to use interactive service for launching
> looks easy to do as well. Then, all privileged operations could
> be removed from openvpn core.
>

I think it is good not to break user experience and allow run openvpn as
an administrator without iservice using wintun at the expense on elevation
to system for single API call.

In case we keep this function, please log such
> errors. Here and several such cases below.
>

Sure.


-- 
-Lev
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] OpenVPN 3 Linux client - v7 beta released

2019-12-16 Thread David Sommerseth
Hi,

The OpenVPN 3 Linux v7 beta release has finally arrived, overdue for
several months.  This is available in our git repositories [0] and URLs for
source tarballs are listed later in this e-mail.  We have pre-built
binaries for the following Linux distributions:

* Fedora 29, 30, 31 and Rawhide  (via Fedora Copr, x86_64 + ppc64le)
* RHEL/CentOS 7 and 8(via Fedora Copr, x86_64 + ppc64le)
* Debian 9 and 10 (amd64)
* Ubuntu 16.04, 18.04, 19.04 and 19.10 (amd64)

See the "Quick-start for OpenVPN 3 Linux" section later in this mail
for a pointer to documentation how to install this client.

The highlights of this release includes:

  # Support for --verify-x509-name
The OpenVPN 3 Core library which this client builds on has been
extended to support this option.  The openvpn2 command line front-end
shipped in this client has been updated to also pass this option
further.

  # New utility: openvpn3-as
This new utility can import a configuration profile directly from an
OpenVPN Access Server.  All which is needed is the URL to the Access
Server and the user credentials.

  # The output of openvpn3 sessions-list has been improved
The report this utility provides has been cleaned up a little and
now also reports which tun interfaces and session names the Core
library uses for each session.

  # Warning if compression is enabled on the tunnel
The OpenVPN 3 log entries will now contain a warning line if
compression has been enabled on the tunnel.  This goes further than to
only check the local configuration file, but also consider what the
server may push.  This means it will NOT give a compression warning
if the local client configuration contains compression arguments but
the server pushes compression settings disabling compression.

  # The openvpn3-admin version command now supports the --service argument
which will query all the OpenVPN 3 D-Bus services and report the
running version of each service.  This is useful for debugging and to
see if the system is running the proper versions of all OpenVPN 3 Linux
services.

  # The openvpn3-admin log-service command has been extended with
the --list-subscriptions argument.  This gives an admin (root user)
more information about which D-Bus services has requested the logger
service to attach/subscribe to log events.

  # The OpenVPN 3 Python module has been extended with NetCfgManager
This gives a limited set of methods useful for debugging and simpler
management of the OpenVPN 3 Network Configuration service. 

  # Error messages coming from the D-Bus infrastructure has been cleaned up
and not really helpful and quite technical references has been removed.

  # D-Bus policy has been split up
Earlier releases had all policies for all OpenVPN 3 Linux D-Bus
services in a single file.  This making the policy management harder
than needed and splitting it up into separate policy files per service
made it simpler to understand the policies in use.

  # Fixed a bug causing D-Bus services to exit while have been in use.
All the OpenVPN 3 Linux D-Bus services makes use of an idle-exit logic
which ensures the service is shut down if it has not been used for some
time.  Before this fix, the service could still exit if it had been
used for a shorter time interval than the idle-exit timer.  This was
incorrect and it will now also consider the time since the last
interaction with the service and not just if some D-Bus objects are
active and being managed by the service.

  # Several other bugfixes
The stability has been improved a lot in several areas and error
situations are handled more gracefully than before, either by trying
a bit harder to complete the task at hand or to provide a bit more
user friendly error messages.

OpenVPN 3 Linux is on track for prime-time production.  It will still come a
some more beta releases, to iron out last missing features and other
improvements.  But OpenVPN 3 Linux is essentially feature ready now.

If you are using OpenVPN 3 Linux, please report back if there are issues you
have or improvements you feel is important for the stable release.

* Quick-start for OpenVPN 3 Linux
See this community wiki page for information how to install and use
OpenVPN 3 Linux: 


[0] 



 Source tarballs 
* OpenVPN 3 Linux v7 beta

  
  


 SHA 256 Checksums --
eadde1b2f2f593dd5020086b53901c42fc5a4562ba105f2add3e4e2c71767c7f  
openvpn3-linux-7_beta.tar.xz

Re: [Openvpn-devel] [PATCH v6 4/7] wintun: ring buffers based I/O

2019-12-16 Thread Selva Nair
Hi,

I was reluctant to review this as I do not understand
the event processing in OpenVPN well enough. Now that Stefann
has reviewed those bits and given an Ack, here are some
comments on the rest of the code.

TLDR:
(i) stealing SYSTEM access from winlogon.exe is not a good thing to do
(ii) with a small change we can support multiple tunnels and  provide better
diagnostics when there are no free wintun adapters -- but this could also
be a follow up patch.

On Fri, Dec 13, 2019 at 2:20 PM Lev Stipakov  wrote:
>
> From: Lev Stipakov 
>
> Implemented according to Wintun documentation
> and reference client code.
>
> Wintun uses ring buffers to communicate between
> kernel driver and user process. Client allocates
> send and receive ring buffers, creates events
> and passes it to kernel driver under LocalSystem
> privileges.
>
> When data is available for read, wintun modifies
> "tail" pointer of send ring and signals via event.
> User process reads data from "head" to "tail" and
> updates "head" pointer.
>
> When user process is ready to write, it writes
> to receive ring, updates "tail" pointer and signals
> to kernel via event.
>
> In openvpn code we add send ring's event to event loop.
> Before performing io wait, we compare "head" and "tail"
> pointers of send ring and if they're different, we skip
> io wait and perform read.
>
> This also adds ring buffers support to tcp and udp
> server code.
>
> Signed-off-by: Lev Stipakov 
> ---
>  v6:
>   - added a sanity check to write_wintun() to avoid
> writing malformed IPv4/6 packet, which causes
> "ring buffer is out of capacity" error.
>
>  v5:
>   - fix crash at ring buffer registration on Win7
> (passing NULL to DeviceIOControl, reported by kitsune1)
>
>  v4:
>   - added helper function tuntap_ring_empty()
>   - refactored event handling, got rid of separate
> event_ctl() call for wintun and send/receive_tail_moved
> members
>   - added wintun_ prefix for ring buffer variables
>   - added a comment explaining the size of wintun-specific buffers
>
>  v3:
>   - simplified convoluted #ifdefs
>   - replaced "greater than" with "greater or equal than"
>
>  v2;
>   - rebased on top of master
>
>  src/openvpn/forward.c |  25 ++-
>  src/openvpn/forward.h |  38 +-
>  src/openvpn/mtcp.c|  19 -
>  src/openvpn/mudp.c|   7 +-
>  src/openvpn/options.c |   4 +-
>  src/openvpn/syshead.h |   1 +
>  src/openvpn/tun.c |  62 +++-
>  src/openvpn/tun.h | 169 +-
>  src/openvpn/win32.c   | 122 ++
>  src/openvpn/win32.h   |  51 +
>  10 files changed, 487 insertions(+), 11 deletions(-)
>
> diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
> index 8451706b..3dc04a40 100644
> --- a/src/openvpn/forward.c
> +++ b/src/openvpn/forward.c
> @@ -1256,8 +1256,24 @@ read_incoming_tun(struct context *c)
>  perf_push(PERF_READ_IN_TUN);
>
>  c->c2.buf = c->c2.buffers->read_tun_buf;
> +
>  #ifdef _WIN32
> -read_tun_buffered(c->c1.tuntap, >c2.buf);
> +if (c->c1.tuntap->wintun)
> +{
> +read_wintun(c->c1.tuntap, >c2.buf);
> +if (c->c2.buf.len == -1)
> +{
> +register_signal(c, SIGHUP, "tun-abort");
> +c->persist.restart_sleep_seconds = 1;
> +msg(M_INFO, "Wintun read error, restarting");
> +perf_pop();
> +return;
> +}
> +}
> +else
> +{
> +read_tun_buffered(c->c1.tuntap, >c2.buf);
> +}
>  #else
>  ASSERT(buf_init(>c2.buf, FRAME_HEADROOM(>c2.frame)));
>  ASSERT(buf_safe(>c2.buf, MAX_RW_SIZE_TUN(>c2.frame)));
> @@ -2099,6 +2115,13 @@ io_wait_dowork(struct context *c, const unsigned int 
> flags)
>  tuntap |= EVENT_READ;
>  }
>
> +#ifdef _WIN32
> +if (tuntap_is_wintun(c->c1.tuntap))
> +{
> +tuntap = EVENT_READ;
> +}
> +#endif
> +
>  /*
>   * Configure event wait based on socket, tuntap flags.
>   */
> diff --git a/src/openvpn/forward.h b/src/openvpn/forward.h
> index 48202c07..b711ff00 100644
> --- a/src/openvpn/forward.h
> +++ b/src/openvpn/forward.h
> @@ -375,6 +375,12 @@ p2p_iow_flags(const struct context *c)
>  {
>  flags |= IOW_TO_TUN;
>  }
> +#ifdef _WIN32
> +if (tuntap_ring_empty(c->c1.tuntap))
> +{
> +flags &= ~IOW_READ_TUN;
> +}
> +#endif
>  return flags;
>  }
>
> @@ -403,8 +409,36 @@ io_wait(struct context *c, const unsigned int flags)
>  }
>  else
>  {
> -/* slow path */
> -io_wait_dowork(c, flags);
> +#ifdef _WIN32
> +bool skip_iowait = flags & IOW_TO_TUN;
> +if (flags & IOW_READ_TUN)
> +{
> +/*
> + * don't read from tun if we have pending write to link,
> + * since every tun read overwrites to_link buffer filled
> + * by previous tun read
> + */
> +skip_iowait = !(flags & IOW_TO_LINK);
> +

[Openvpn-devel] [PATCH v7 4/7] wintun: ring buffers based I/O

2019-12-16 Thread Lev Stipakov
From: Lev Stipakov 

Implemented according to Wintun documentation
and reference client code.

Wintun uses ring buffers to communicate between
kernel driver and user process. Client allocates
send and receive ring buffers, creates events
and passes it to kernel driver under LocalSystem
privileges.

When data is available for read, wintun modifies
"tail" pointer of send ring and signals via event.
User process reads data from "head" to "tail" and
updates "head" pointer.

When user process is ready to write, it writes
to receive ring, updates "tail" pointer and signals
to kernel via event.

In openvpn code we add send ring's event to event loop.
Before performing io wait, we compare "head" and "tail"
pointers of send ring and if they're different, we skip
io wait and perform read.

This also adds ring buffers support to tcp and udp
server code.

Signed-off-by: Lev Stipakov 
---

 v7:
  - fix comments (no code changes)

 v6:
  - added a sanity check to write_wintun() to avoid 
writing malformed IPv4/6 packet, which causes
"ring buffer is out of capacity" error.

 v5:
  - fix crash at ring buffer registration on Win7
(passing NULL to DeviceIOControl, reported by kitsune1)

 v4:
  - added helper function tuntap_ring_empty()
  - refactored event handling, got rid of separate
event_ctl() call for wintun and send/receive_tail_moved
members
  - added wintun_ prefix for ring buffer variables
  - added a comment explaining the size of wintun-specific buffers

 v3:
  - simplified convoluted #ifdefs
  - replaced "greater than" with "greater or equal than"

 v2:
  - rebased on top of master
 

 src/openvpn/forward.c |  29 +++-
 src/openvpn/forward.h |  38 +-
 src/openvpn/mtcp.c|  19 -
 src/openvpn/mudp.c|   7 +-
 src/openvpn/options.c |   4 +-
 src/openvpn/syshead.h |   1 +
 src/openvpn/tun.c |  62 +++-
 src/openvpn/tun.h | 169 +-
 src/openvpn/win32.c   | 122 ++
 src/openvpn/win32.h   |  51 +
 10 files changed, 491 insertions(+), 11 deletions(-)

diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
index 8451706b..6b823613 100644
--- a/src/openvpn/forward.c
+++ b/src/openvpn/forward.c
@@ -1256,8 +1256,24 @@ read_incoming_tun(struct context *c)
 perf_push(PERF_READ_IN_TUN);
 
 c->c2.buf = c->c2.buffers->read_tun_buf;
+
 #ifdef _WIN32
-read_tun_buffered(c->c1.tuntap, >c2.buf);
+if (c->c1.tuntap->wintun)
+{
+read_wintun(c->c1.tuntap, >c2.buf);
+if (c->c2.buf.len == -1)
+{
+register_signal(c, SIGHUP, "tun-abort");
+c->persist.restart_sleep_seconds = 1;
+msg(M_INFO, "Wintun read error, restarting");
+perf_pop();
+return;
+}
+}
+else
+{
+read_tun_buffered(c->c1.tuntap, >c2.buf);
+}
 #else
 ASSERT(buf_init(>c2.buf, FRAME_HEADROOM(>c2.frame)));
 ASSERT(buf_safe(>c2.buf, MAX_RW_SIZE_TUN(>c2.frame)));
@@ -2099,6 +2115,17 @@ io_wait_dowork(struct context *c, const unsigned int 
flags)
 tuntap |= EVENT_READ;
 }
 
+#ifdef _WIN32
+if (tuntap_is_wintun(c->c1.tuntap))
+{
+/*
+ * With wintun we are only interested in read event. Ring buffer is
+ * always ready for write, so we don't do wait.
+ */
+tuntap = EVENT_READ;
+}
+#endif
+
 /*
  * Configure event wait based on socket, tuntap flags.
  */
diff --git a/src/openvpn/forward.h b/src/openvpn/forward.h
index 48202c07..b711ff00 100644
--- a/src/openvpn/forward.h
+++ b/src/openvpn/forward.h
@@ -375,6 +375,12 @@ p2p_iow_flags(const struct context *c)
 {
 flags |= IOW_TO_TUN;
 }
+#ifdef _WIN32
+if (tuntap_ring_empty(c->c1.tuntap))
+{
+flags &= ~IOW_READ_TUN;
+}
+#endif
 return flags;
 }
 
@@ -403,8 +409,36 @@ io_wait(struct context *c, const unsigned int flags)
 }
 else
 {
-/* slow path */
-io_wait_dowork(c, flags);
+#ifdef _WIN32
+bool skip_iowait = flags & IOW_TO_TUN;
+if (flags & IOW_READ_TUN)
+{
+/*
+ * don't read from tun if we have pending write to link,
+ * since every tun read overwrites to_link buffer filled
+ * by previous tun read
+ */
+skip_iowait = !(flags & IOW_TO_LINK);
+}
+if (tuntap_is_wintun(c->c1.tuntap) && skip_iowait)
+{
+unsigned int ret = 0;
+if (flags & IOW_TO_TUN)
+{
+ret |= TUN_WRITE;
+}
+if (flags & IOW_READ_TUN)
+{
+ret |= TUN_READ;
+}
+c->c2.event_set_status = ret;
+}
+else
+#endif
+{
+/* slow path */
+io_wait_dowork(c, flags);
+}
 }
 }
 
diff --git a/src/openvpn/mtcp.c b/src/openvpn/mtcp.c
index abe20593..ee28a710 100644
---