Let's not merge it yet.

The reason why this patch was done is to prepare for Wintun support,
since Wintun device must be opened by privileged process. There is no need
to use privileged process to open tap-windows6 device.

Recently Wintun API has been changed and previous Wintun device path
(\\.\Global\WINTUNxxx) does not work anymore. In order to get a new path
one needs to use SetupAPI (such as SetupDiCreateDeviceInfo), which
must be called by privileged process.

So instead of having IPC call "open_tun_device(path)" it is probably better
to have something like "open_wintun_device()", which both figures out
device path
and opens device.

ke 24. heinäk. 2019 klo 17.44 Selva Nair (selva.n...@gmail.com) kirjoitti:

> Hi,
>
> Looks good now and works as expected.
>
> On Tue, Jul 23, 2019 at 5:22 AM Lev Stipakov <lstipa...@gmail.com> wrote:
> >
> > From: Lev Stipakov <l...@openvpn.net>
> >
> > This patch enables interactive service to open tun device.
> > This is mostly needed by Wintun, which could be opened
> > only by privileged process.
> >
> > When interactive service is used, instead of calling
> > CreateFile() directly by openvpn process we pass tun device path
> > into service process. There we open device, duplicate handle
> > and pass it back to openvpn process.
> >
> > Signed-off-by: Lev Stipakov <l...@openvpn.net>
> > ---
> >  v6:
> >   - simplify and strengthen guid check with wcsspn()
> >   - fix doxygen comment
> >
> >  v5:
> >   - further strengthen security by passing only device guid from client
> process
> >   to service, validating guid and constructing device path on service
> side
> >
> >  v4:
> >   - strengthen security by adding basic validation to device path
> >   - reorder fields in msg_open_tun_device_result for backward
> compatibility
> >
> >  v3:
> >   - ensure that device path passed by client is null-terminated
> >   - support for multiple openvpn processes
> >   - return proper error code when device handle is invalid
> >
> >  v2:
> >   - introduce send_msg_iservice_ex() instead of changing
> >   signature of existing send_msg_iservice()
> >   - use wchar_t strings in interactive service code
> >
> >  include/openvpn-msg.h         | 12 +++++++
> >  src/openvpn/tun.c             | 83
> ++++++++++++++++++++++++++++++++++---------
> >  src/openvpn/win32.c           |  9 ++++-
> >  src/openvpn/win32.h           | 30 +++++++++++++---
> >  src/openvpnserv/interactive.c | 78
> ++++++++++++++++++++++++++++++++++++++--
> >  5 files changed, 188 insertions(+), 24 deletions(-)
>
> ...
>
> > diff --git a/src/openvpnserv/interactive.c
> b/src/openvpnserv/interactive.c
> > index 623c3ff..1b4a5e2 100644
> > --- a/src/openvpnserv/interactive.c
> > +++ b/src/openvpnserv/interactive.c
> > @@ -58,7 +58,6 @@ static settings_t settings;
> >  static HANDLE rdns_semaphore = NULL;
> >  #define RDNS_TIMEOUT 600  /* seconds to wait for the semaphore */
> >
> > -
> >  openvpn_service_t interactive_service = {
> >      interactive,
> >      TEXT(PACKAGE_NAME "ServiceInteractive"),
> > @@ -1198,8 +1197,62 @@ HandleEnableDHCPMessage(const
> enable_dhcp_message_t *dhcp)
> >      return err;
> >  }
> >
> > +static DWORD
> > +HandleOpenTunDeviceMessage(const open_tun_device_message_t *open_tun,
> HANDLE ovpn_proc, HANDLE *remote_handle)
> > +{
> > +    DWORD err = ERROR_SUCCESS;
> > +    HANDLE local_handle;
> > +    LPWSTR wguid = NULL;
> > +    WCHAR device_path[256] = {0};
> > +    const WCHAR prefix[] = L"\\\\.\\Global\\";
> > +    const WCHAR tap_suffix[] = L".tap";
> > +
> > +    *remote_handle = INVALID_HANDLE_VALUE;
> > +
> > +    wguid = utf8to16(open_tun->device_guid);
> > +    if (!wguid)
> > +    {
> > +        err = ERROR_OUTOFMEMORY;
> > +        goto out;
> > +    }
> > +
> > +    /* validate device guid */
> > +    const size_t guid_len = wcslen(wguid);
> > +    if (guid_len != 38 || wcsspn(wguid, L"0123456789ABCDEFabcdef-{}")
> != guid_len)
> > +    {
> > +        err = ERROR_MESSAGE_DATA;
> > +        MsgToEventLog(MSG_FLAGS_ERROR, TEXT("Invalid device guild
> (%s)"), wguid);
>
> guild -> guid  (as pointed out by tincanteksup)
>
> > +        goto out;
> > +    }
> > +
>
> whitespace in line above.
>
> These could be fixed at commit time.
>
> Acked-by: Selva Nair <selva.n...@gmail.com>
>
> Selva
>
>
>
>
> Selva
>


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

Reply via email to