[Openvpn-devel] tun.c: enable using wintun driver under SYSTEM

Message ID 20200819070746.197-1-lstipakov@gmail.com
State Accepted
Headers show
Series [Openvpn-devel] tun.c: enable using wintun driver under SYSTEM | expand

Commit Message

Lev Stipakov Aug. 18, 2020, 9:07 p.m. UTC
From: Lev Stipakov <lev@openvpn.net>

Commit 6d19775a468 has removed SYSTEM elevation hack,
but introduced regression - inability to use wintun without interactive service.

Proceed with ring buffers registration even if iservice is unavailable and display
relevant error message.

Trac #1318

Signed-off-by: Lev Stipakov <lev@openvpn.net>
---
 src/openvpn/tun.c | 30 +++++++++++++++++++++++++-----
 1 file changed, 25 insertions(+), 5 deletions(-)

Comments

Selva Nair Aug. 21, 2020, 11:13 a.m. UTC | #1
Hi,

On Wed, Aug 19, 2020 at 3:08 AM Lev Stipakov <lstipakov@gmail.com> wrote:

> From: Lev Stipakov <lev@openvpn.net>
>
> Commit 6d19775a468 has removed SYSTEM elevation hack,
> but introduced regression - inability to use wintun without interactive
> service.
>
> Proceed with ring buffers registration even if iservice is unavailable and
> display
> relevant error message.
>
> Trac #1318
>
> Signed-off-by: Lev Stipakov <lev@openvpn.net>
> ---
>  src/openvpn/tun.c | 30 +++++++++++++++++++++++++-----
>  1 file changed, 25 insertions(+), 5 deletions(-)
>
> diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
> index 30454454..62557364 100644
> --- a/src/openvpn/tun.c
> +++ b/src/openvpn/tun.c
> @@ -6158,12 +6158,32 @@ wintun_register_ring_buffer(struct tuntap *tt,
> const char *device_guid)
>      }
>      else
>      {
> -        msg(M_FATAL, "ERROR:  Wintun requires SYSTEM privileges and
> therefore "
> -                     "should be used with interactive service. If you
> want to "
> -                     "use openvpn from command line, you need to do
> SYSTEM "
> -                     "elevation yourself (for example with psexec).");
> -    }
> +        if (!register_ring_buffers(tt->hand,
> +                                   tt->wintun_send_ring,
> +                                   tt->wintun_receive_ring,
> +                                   tt->rw_handle.read,
> +                                   tt->rw_handle.write))
> +        {
> +            switch (GetLastError())
> +            {
> +                case ERROR_ACCESS_DENIED:
> +                    msg(M_FATAL, "ERROR:  Wintun requires SYSTEM
> privileges and therefore "
> +                                 "should be used with interactive
> service. If you want to "
> +                                 "use openvpn from command line, you need
> to do SYSTEM "
> +                                 "elevation yourself (for example with
> psexec).");
> +                    break;
> +
> +                case ERROR_ALREADY_INITIALIZED:
> +                    msg(M_NONFATAL, "Adapter %s is already in use",
> device_guid);
> +                    break;
>
> +                default:
> +                    msg(M_NONFATAL | M_ERRNO, "Failed to register ring
> buffers");
> +            }
> +            ret = false;
> +        }
> +
> +    }
>      return ret;
>  }
>

Looks good and running as SYSTEM works now as expected. Tested on 64 bit
Windows 10.

Acked-by: selva.nair@gmail.com
<div dir="ltr"><div>Hi,</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Aug 19, 2020 at 3:08 AM Lev Stipakov &lt;<a href="mailto:lstipakov@gmail.com">lstipakov@gmail.com</a>&gt; wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">From: Lev Stipakov &lt;<a href="mailto:lev@openvpn.net" target="_blank">lev@openvpn.net</a>&gt;<br>
<br>
Commit 6d19775a468 has removed SYSTEM elevation hack,<br>
but introduced regression - inability to use wintun without interactive service.<br>
<br>
Proceed with ring buffers registration even if iservice is unavailable and display<br>
relevant error message.<br>
<br>
Trac #1318<br>
<br>
Signed-off-by: Lev Stipakov &lt;<a href="mailto:lev@openvpn.net" target="_blank">lev@openvpn.net</a>&gt;<br>
---<br>
 src/openvpn/tun.c | 30 +++++++++++++++++++++++++-----<br>
 1 file changed, 25 insertions(+), 5 deletions(-)<br>
<br>
diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c<br>
index 30454454..62557364 100644<br>
--- a/src/openvpn/tun.c<br>
+++ b/src/openvpn/tun.c<br>
@@ -6158,12 +6158,32 @@ wintun_register_ring_buffer(struct tuntap *tt, const char *device_guid)<br>
     }<br>
     else<br>
     {<br>
-        msg(M_FATAL, &quot;ERROR:  Wintun requires SYSTEM privileges and therefore &quot;<br>
-                     &quot;should be used with interactive service. If you want to &quot;<br>
-                     &quot;use openvpn from command line, you need to do SYSTEM &quot;<br>
-                     &quot;elevation yourself (for example with psexec).&quot;);<br>
-    }<br>
+        if (!register_ring_buffers(tt-&gt;hand,<br>
+                                   tt-&gt;wintun_send_ring,<br>
+                                   tt-&gt;wintun_receive_ring,<br>
+                                   tt-&gt;rw_handle.read,<br>
+                                   tt-&gt;rw_handle.write))<br>
+        {<br>
+            switch (GetLastError())<br>
+            {<br>
+                case ERROR_ACCESS_DENIED:<br>
+                    msg(M_FATAL, &quot;ERROR:  Wintun requires SYSTEM privileges and therefore &quot;<br>
+                                 &quot;should be used with interactive service. If you want to &quot;<br>
+                                 &quot;use openvpn from command line, you need to do SYSTEM &quot;<br>
+                                 &quot;elevation yourself (for example with psexec).&quot;);<br>
+                    break;<br>
+<br>
+                case ERROR_ALREADY_INITIALIZED:<br>
+                    msg(M_NONFATAL, &quot;Adapter %s is already in use&quot;, device_guid);<br>
+                    break;<br>
<br>
+                default:<br>
+                    msg(M_NONFATAL | M_ERRNO, &quot;Failed to register ring buffers&quot;);<br>
+            }<br>
+            ret = false;<br>
+        }<br>
+<br>
+    }<br>
     return ret;<br>
 }<br></blockquote><div><br></div><div>Looks good and running as SYSTEM works now as expected. Tested on 64 bit Windows 10.</div><div><br></div><div>Acked-by: <a href="mailto:selva.nair@gmail.com">selva.nair@gmail.com</a> </div></div></div>
Marvin Aug. 21, 2020, 4:37 p.m. UTC | #2
Hi,

When will this be available as an installable (beta) msi?

Marvin 

> On Aug 21, 2020, at 2:13 PM, Selva Nair <selva.nair@gmail.com> wrote:
> 
> Hi,
> 
>> On Wed, Aug 19, 2020 at 3:08 AM Lev Stipakov <lstipakov@gmail.com> wrote:
>> From: Lev Stipakov <lev@openvpn.net>
>> 
>> Commit 6d19775a468 has removed SYSTEM elevation hack,
>> but introduced regression - inability to use wintun without interactive service.
>> 
>> Proceed with ring buffers registration even if iservice is unavailable and display
>> relevant error message.
>> 
>> Trac #1318
>> 
>> Signed-off-by: Lev Stipakov <lev@openvpn.net>
>> ---
>>  src/openvpn/tun.c | 30 +++++++++++++++++++++++++-----
>>  1 file changed, 25 insertions(+), 5 deletions(-)
>> 
>> diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
>> index 30454454..62557364 100644
>> --- a/src/openvpn/tun.c
>> +++ b/src/openvpn/tun.c
>> @@ -6158,12 +6158,32 @@ wintun_register_ring_buffer(struct tuntap *tt, const char *device_guid)
>>      }
>>      else
>>      {
>> -        msg(M_FATAL, "ERROR:  Wintun requires SYSTEM privileges and therefore "
>> -                     "should be used with interactive service. If you want to "
>> -                     "use openvpn from command line, you need to do SYSTEM "
>> -                     "elevation yourself (for example with psexec).");
>> -    }
>> +        if (!register_ring_buffers(tt->hand,
>> +                                   tt->wintun_send_ring,
>> +                                   tt->wintun_receive_ring,
>> +                                   tt->rw_handle.read,
>> +                                   tt->rw_handle.write))
>> +        {
>> +            switch (GetLastError())
>> +            {
>> +                case ERROR_ACCESS_DENIED:
>> +                    msg(M_FATAL, "ERROR:  Wintun requires SYSTEM privileges and therefore "
>> +                                 "should be used with interactive service. If you want to "
>> +                                 "use openvpn from command line, you need to do SYSTEM "
>> +                                 "elevation yourself (for example with psexec).");
>> +                    break;
>> +
>> +                case ERROR_ALREADY_INITIALIZED:
>> +                    msg(M_NONFATAL, "Adapter %s is already in use", device_guid);
>> +                    break;
>> 
>> +                default:
>> +                    msg(M_NONFATAL | M_ERRNO, "Failed to register ring buffers");
>> +            }
>> +            ret = false;
>> +        }
>> +
>> +    }
>>      return ret;
>>  }
> 
> Looks good and running as SYSTEM works now as expected. Tested on 64 bit Windows 10.
> 
> Acked-by: selva.nair@gmail.com 
> _______________________________________________
> Openvpn-devel mailing list
> Openvpn-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openvpn-devel
<html><head><meta http-equiv="content-type" content="text/html; charset=utf-8"></head><body dir="auto">Hi,<div><br></div><div>When will this be available as an installable (beta) msi?<br><br><div dir="ltr">Marvin&nbsp;</div><div dir="ltr"><br>On Aug 21, 2020, at 2:13 PM, Selva Nair &lt;<a href="mailto:selva.nair@gmail.com">selva.nair@gmail.com</a>&gt; wrote:<br><br></div><blockquote type="cite"><div dir="ltr"><div dir="ltr"><div>Hi,</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Aug 19, 2020 at 3:08 AM Lev Stipakov &lt;<a href="mailto:lstipakov@gmail.com">lstipakov@gmail.com</a>&gt; wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">From: Lev Stipakov &lt;<a href="mailto:lev@openvpn.net" target="_blank">lev@openvpn.net</a>&gt;<br>
<br>
Commit 6d19775a468 has removed SYSTEM elevation hack,<br>
but introduced regression - inability to use wintun without interactive service.<br>
<br>
Proceed with ring buffers registration even if iservice is unavailable and display<br>
relevant error message.<br>
<br>
Trac #1318<br>
<br>
Signed-off-by: Lev Stipakov &lt;<a href="mailto:lev@openvpn.net" target="_blank">lev@openvpn.net</a>&gt;<br>
---<br>
&nbsp;src/openvpn/tun.c | 30 +++++++++++++++++++++++++-----<br>
&nbsp;1 file changed, 25 insertions(+), 5 deletions(-)<br>
<br>
diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c<br>
index 30454454..62557364 100644<br>
--- a/src/openvpn/tun.c<br>
+++ b/src/openvpn/tun.c<br>
@@ -6158,12 +6158,32 @@ wintun_register_ring_buffer(struct tuntap *tt, const char *device_guid)<br>
&nbsp; &nbsp; &nbsp;}<br>
&nbsp; &nbsp; &nbsp;else<br>
&nbsp; &nbsp; &nbsp;{<br>
-&nbsp; &nbsp; &nbsp; &nbsp; msg(M_FATAL, "ERROR:&nbsp; Wintun requires SYSTEM privileges and therefore "<br>
-&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;"should be used with interactive service. If you want to "<br>
-&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;"use openvpn from command line, you need to do SYSTEM "<br>
-&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;"elevation yourself (for example with psexec).");<br>
-&nbsp; &nbsp; }<br>
+&nbsp; &nbsp; &nbsp; &nbsp; if (!register_ring_buffers(tt-&gt;hand,<br>
+&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;tt-&gt;wintun_send_ring,<br>
+&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;tt-&gt;wintun_receive_ring,<br>
+&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;tt-&gt;rw_handle.read,<br>
+&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;tt-&gt;rw_handle.write))<br>
+&nbsp; &nbsp; &nbsp; &nbsp; {<br>
+&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; switch (GetLastError())<br>
+&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; {<br>
+&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; case ERROR_ACCESS_DENIED:<br>
+&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; msg(M_FATAL, "ERROR:&nbsp; Wintun requires SYSTEM privileges and therefore "<br>
+&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;"should be used with interactive service. If you want to "<br>
+&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;"use openvpn from command line, you need to do SYSTEM "<br>
+&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp;"elevation yourself (for example with psexec).");<br>
+&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; break;<br>
+<br>
+&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; case ERROR_ALREADY_INITIALIZED:<br>
+&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; msg(M_NONFATAL, "Adapter %s is already in use", device_guid);<br>
+&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; break;<br>
<br>
+&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; default:<br>
+&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; msg(M_NONFATAL | M_ERRNO, "Failed to register ring buffers");<br>
+&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; }<br>
+&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; ret = false;<br>
+&nbsp; &nbsp; &nbsp; &nbsp; }<br>
+<br>
+&nbsp; &nbsp; }<br>
&nbsp; &nbsp; &nbsp;return ret;<br>
&nbsp;}<br></blockquote><div><br></div><div>Looks good and running as SYSTEM works now as expected. Tested on 64 bit Windows 10.</div><div><br></div><div>Acked-by: <a href="mailto:selva.nair@gmail.com">selva.nair@gmail.com</a>&nbsp;</div></div></div>
</div></blockquote><blockquote type="cite"><div dir="ltr"></div></blockquote><blockquote type="cite"><div dir="ltr"><span>_______________________________________________</span><br><span>Openvpn-devel mailing list</span><br><span><a href="mailto:Openvpn-devel@lists.sourceforge.net">Openvpn-devel@lists.sourceforge.net</a></span><br><span><a href="https://lists.sourceforge.net/lists/listinfo/openvpn-devel">https://lists.sourceforge.net/lists/listinfo/openvpn-devel</a></span><br></div></blockquote></div></body></html>
Gert Doering Aug. 21, 2020, 9:20 p.m. UTC | #3
Hi,

On Fri, Aug 21, 2020 at 07:37:27PM -0700, Marvin Adeff wrote:
> When will this be available as an installable (beta) msi?

I'll merge the commit today or tomorrow (sorry for the lag on my side,
was travelling and had only limited access to "Internet and focus").

My plan is to do a formal "beta2" tag on Wednesday, but of course
Samuli/Lev could do a new installer ("beta1_1") earlier.

gert
Marvin Aug. 22, 2020, 7:15 a.m. UTC | #4
No need to rush.  I’m happy to wait until Wednesday for beta2. 

Cheers,
Marvin 

Sent from my iPhone

> On Aug 22, 2020, at 12:20 AM, Gert Doering <gert@greenie.muc.de> wrote:
> 
> Hi,
> 
>> On Fri, Aug 21, 2020 at 07:37:27PM -0700, Marvin Adeff wrote:
>> When will this be available as an installable (beta) msi?
> 
> I'll merge the commit today or tomorrow (sorry for the lag on my side,
> was travelling and had only limited access to "Internet and focus").
> 
> My plan is to do a formal "beta2" tag on Wednesday, but of course
> Samuli/Lev could do a new installer ("beta1_1") earlier.
> 
> gert
> -- 
> "If was one thing all people took for granted, was conviction that if you 
> feed honest figures into a computer, honest figures come out. Never doubted 
> it myself till I met a computer with a sense of humor."
>                             Robert A. Heinlein, The Moon is a Harsh Mistress
> 
> Gert Doering - Munich, Germany                             gert@greenie.muc.de
Gert Doering Aug. 23, 2020, 2:36 a.m. UTC | #5
Your patch has been applied to the master and release/2.5 branch.

(I have not tested anything, but diffing tun.c to 6d19775a468~1 shows
that's it is the same code that was removed, except for a more detailed
error message)

commit ed47c097db63f8334f32bf9482da488928ff909b (master)
commit f4761dd12ec5eb10a5897ecb7dc427f54a9b0769 (release/2.5)
Author: Lev Stipakov
Date:   Wed Aug 19 10:07:46 2020 +0300

     tun.c: enable using wintun driver under SYSTEM

     Signed-off-by: Lev Stipakov <lev@openvpn.net>
     Acked-by: Selva Nair <selva.nair@gmail.com>
     Message-Id: <20200819070746.197-1-lstipakov@gmail.com>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg20780.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering

Patch

diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
index 30454454..62557364 100644
--- a/src/openvpn/tun.c
+++ b/src/openvpn/tun.c
@@ -6158,12 +6158,32 @@  wintun_register_ring_buffer(struct tuntap *tt, const char *device_guid)
     }
     else
     {
-        msg(M_FATAL, "ERROR:  Wintun requires SYSTEM privileges and therefore "
-                     "should be used with interactive service. If you want to "
-                     "use openvpn from command line, you need to do SYSTEM "
-                     "elevation yourself (for example with psexec).");
-    }
+        if (!register_ring_buffers(tt->hand,
+                                   tt->wintun_send_ring,
+                                   tt->wintun_receive_ring,
+                                   tt->rw_handle.read,
+                                   tt->rw_handle.write))
+        {
+            switch (GetLastError())
+            {
+                case ERROR_ACCESS_DENIED:
+                    msg(M_FATAL, "ERROR:  Wintun requires SYSTEM privileges and therefore "
+                                 "should be used with interactive service. If you want to "
+                                 "use openvpn from command line, you need to do SYSTEM "
+                                 "elevation yourself (for example with psexec).");
+                    break;
+
+                case ERROR_ALREADY_INITIALIZED:
+                    msg(M_NONFATAL, "Adapter %s is already in use", device_guid);
+                    break;
 
+                default:
+                    msg(M_NONFATAL | M_ERRNO, "Failed to register ring buffers");
+            }
+            ret = false;
+        }
+
+    }
     return ret;
 }