Browse code

wintun: remove SYSTEM elevation hack

As discussed a while ago on the mailing list and
community meetings, having SYSTEM elevation hack
inside openvpn code considered harmful.

Since interactive service is the recommended way
of using openvpn on Windows, limiting wintun usage to
interactive service should not be an issue.

Remove elevation hack code and provide an error message
telling user to use interactive service or do SYSTEM
elevation himself via psexec.

Move implementation of register_ring_buffers() to header
amd delete ring_buffer.c.

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

Lev Stipakov authored on 2020/07/24 19:48:41
Showing 10 changed files
... ...
@@ -142,6 +142,6 @@ openvpn_LDADD = \
142 142
 	$(OPTIONAL_DL_LIBS) \
143 143
 	$(OPTIONAL_INOTIFY_LIBS)
144 144
 if WIN32
145
-openvpn_SOURCES += openvpn_win32_resources.rc block_dns.c block_dns.h ring_buffer.c ring_buffer.h
145
+openvpn_SOURCES += openvpn_win32_resources.rc block_dns.c block_dns.h ring_buffer.h
146 146
 openvpn_LDADD += -lgdi32 -lws2_32 -lwininet -lcrypt32 -liphlpapi -lwinmm -lfwpuclnt -lrpcrt4 -lncrypt -lsetupapi
147 147
 endif
... ...
@@ -201,7 +201,6 @@
201 201
     <ClCompile Include="ps.c" />
202 202
     <ClCompile Include="push.c" />
203 203
     <ClCompile Include="reliable.c" />
204
-    <ClCompile Include="ring_buffer.c" />
205 204
     <ClCompile Include="route.c" />
206 205
     <ClCompile Include="run_command.c" />
207 206
     <ClCompile Include="schedule.c" />
... ...
@@ -240,9 +240,6 @@
240 240
     <ClCompile Include="vlan.c">
241 241
       <Filter>Source Files</Filter>
242 242
     </ClCompile>
243
-    <ClCompile Include="ring_buffer.c">
244
-      <Filter>Source Files</Filter>
245
-    </ClCompile>
246 243
     <ClCompile Include="ssl_ncp.c">
247 244
       <Filter>Source Files</Filter>
248 245
     </ClCompile>
249 246
deleted file mode 100644
... ...
@@ -1,56 +0,0 @@
1
-/*
2
- *  OpenVPN -- An application to securely tunnel IP networks
3
- *             over a single UDP port, with support for SSL/TLS-based
4
- *             session authentication and key exchange,
5
- *             packet encryption, packet authentication, and
6
- *             packet compression.
7
- *
8
- *  Copyright (C) 2002-2019 OpenVPN Inc <sales@openvpn.net>
9
- *                2019 Lev Stipakov <lev@openvpn.net>
10
- *
11
- *  This program is free software; you can redistribute it and/or modify
12
- *  it under the terms of the GNU General Public License version 2
13
- *  as published by the Free Software Foundation.
14
- *
15
- *  This program is distributed in the hope that it will be useful,
16
- *  but WITHOUT ANY WARRANTY; without even the implied warranty of
17
- *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
18
- *  GNU General Public License for more details.
19
- *
20
- *  You should have received a copy of the GNU General Public License along
21
- *  with this program; if not, write to the Free Software Foundation, Inc.,
22
- *  51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
23
- */
24
-
25
-#include "ring_buffer.h"
26
-
27
-#ifdef _WIN32
28
-
29
-bool
30
-register_ring_buffers(HANDLE device,
31
-                      struct tun_ring *send_ring,
32
-                      struct tun_ring *receive_ring,
33
-                      HANDLE send_tail_moved,
34
-                      HANDLE receive_tail_moved)
35
-{
36
-    struct tun_register_rings rr;
37
-    BOOL res;
38
-    DWORD bytes_returned;
39
-
40
-    ZeroMemory(&rr, sizeof(rr));
41
-
42
-    rr.send.ring = send_ring;
43
-    rr.send.ring_size = sizeof(struct tun_ring);
44
-    rr.send.tail_moved = send_tail_moved;
45
-
46
-    rr.receive.ring = receive_ring;
47
-    rr.receive.ring_size = sizeof(struct tun_ring);
48
-    rr.receive.tail_moved = receive_tail_moved;
49
-
50
-    res = DeviceIoControl(device, TUN_IOCTL_REGISTER_RINGS, &rr, sizeof(rr),
51
-                          NULL, 0, &bytes_returned, NULL);
52
-
53
-    return res != FALSE;
54
-}
55
-
56
-#endif /* ifdef _WIN32 */
57 1
\ No newline at end of file
... ...
@@ -94,11 +94,32 @@ struct TUN_PACKET
94 94
  *                            that data has been written to receive ring
95 95
  * @return                    true if registration is successful, false otherwise - use GetLastError()
96 96
  */
97
-bool register_ring_buffers(HANDLE device,
98
-                           struct tun_ring *send_ring,
99
-                           struct tun_ring *receive_ring,
100
-                           HANDLE send_tail_moved,
101
-                           HANDLE receive_tail_moved);
97
+static bool
98
+register_ring_buffers(HANDLE device,
99
+                      struct tun_ring *send_ring,
100
+                      struct tun_ring *receive_ring,
101
+                      HANDLE send_tail_moved,
102
+                      HANDLE receive_tail_moved)
103
+{
104
+    struct tun_register_rings rr;
105
+    BOOL res;
106
+    DWORD bytes_returned;
107
+
108
+    ZeroMemory(&rr, sizeof(rr));
109
+
110
+    rr.send.ring = send_ring;
111
+    rr.send.ring_size = sizeof(struct tun_ring);
112
+    rr.send.tail_moved = send_tail_moved;
113
+
114
+    rr.receive.ring = receive_ring;
115
+    rr.receive.ring_size = sizeof(struct tun_ring);
116
+    rr.receive.tail_moved = receive_tail_moved;
117
+
118
+    res = DeviceIoControl(device, TUN_IOCTL_REGISTER_RINGS, &rr, sizeof(rr),
119
+      NULL, 0, &bytes_returned, NULL);
120
+
121
+    return res != FALSE;
122
+}
102 123
 
103 124
 #endif /* ifndef OPENVPN_RING_BUFFER_H */
104 125
 #endif /* ifdef _WIN32 */
... ...
@@ -6148,35 +6148,10 @@ wintun_register_ring_buffer(struct tuntap *tt, const char *device_guid)
6148 6148
     }
6149 6149
     else
6150 6150
     {
6151
-        if (!impersonate_as_system())
6152
-        {
6153
-            msg(M_FATAL, "ERROR:  Failed to impersonate as SYSTEM, make sure process is running under privileged account");
6154
-        }
6155
-        if (!register_ring_buffers(tt->hand,
6156
-                                   tt->wintun_send_ring,
6157
-                                   tt->wintun_receive_ring,
6158
-                                   tt->rw_handle.read,
6159
-                                   tt->rw_handle.write))
6160
-        {
6161
-            switch (GetLastError())
6162
-            {
6163
-                case ERROR_ACCESS_DENIED:
6164
-                    msg(M_FATAL, "Access denied registering ring buffers. Is this process run as SYSTEM?");
6165
-                    break;
6166
-
6167
-                case ERROR_ALREADY_INITIALIZED:
6168
-                    msg(M_NONFATAL, "Adapter %s is already in use", device_guid);
6169
-                    break;
6170
-
6171
-                default:
6172
-                    msg(M_NONFATAL | M_ERRNO, "Failed to register ring buffers");
6173
-            }
6174
-            ret = false;
6175
-        }
6176
-        if (!RevertToSelf())
6177
-        {
6178
-            msg(M_FATAL, "ERROR:  RevertToSelf error: %lu", GetLastError());
6179
-        }
6151
+        msg(M_FATAL, "ERROR:  Wintun requires SYSTEM privileges and therefore "
6152
+                     "should be used with interactive service. If you want to "
6153
+                     "use openvpn from command line, you need to do SYSTEM "
6154
+                     "elevation yourself (for example with psexec).");
6180 6155
     }
6181 6156
 
6182 6157
     return ret;
... ...
@@ -1509,99 +1509,4 @@ send_msg_iservice(HANDLE pipe, const void *data, size_t size,
1509 1509
     return ret;
1510 1510
 }
1511 1511
 
1512
-bool
1513
-impersonate_as_system()
1514
-{
1515
-    HANDLE thread_token, process_snapshot, winlogon_process, winlogon_token, duplicated_token;
1516
-    PROCESSENTRY32 entry;
1517
-    BOOL ret;
1518
-    DWORD pid = 0;
1519
-    TOKEN_PRIVILEGES privileges;
1520
-
1521
-    CLEAR(entry);
1522
-    CLEAR(privileges);
1523
-
1524
-    entry.dwSize = sizeof(PROCESSENTRY32);
1525
-
1526
-    privileges.PrivilegeCount = 1;
1527
-    privileges.Privileges->Attributes = SE_PRIVILEGE_ENABLED;
1528
-
1529
-    if (!LookupPrivilegeValue(NULL, SE_DEBUG_NAME, &privileges.Privileges[0].Luid))
1530
-    {
1531
-        return false;
1532
-    }
1533
-
1534
-    if (!ImpersonateSelf(SecurityImpersonation))
1535
-    {
1536
-        return false;
1537
-    }
1538
-
1539
-    if (!OpenThreadToken(GetCurrentThread(), TOKEN_ADJUST_PRIVILEGES, FALSE, &thread_token))
1540
-    {
1541
-        RevertToSelf();
1542
-        return false;
1543
-    }
1544
-    if (!AdjustTokenPrivileges(thread_token, FALSE, &privileges, sizeof(privileges), NULL, NULL))
1545
-    {
1546
-        CloseHandle(thread_token);
1547
-        RevertToSelf();
1548
-        return false;
1549
-    }
1550
-    CloseHandle(thread_token);
1551
-
1552
-    process_snapshot = CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, 0);
1553
-    if (process_snapshot == INVALID_HANDLE_VALUE)
1554
-    {
1555
-        RevertToSelf();
1556
-        return false;
1557
-    }
1558
-    for (ret = Process32First(process_snapshot, &entry); ret; ret = Process32Next(process_snapshot, &entry))
1559
-    {
1560
-        if (!_stricmp(entry.szExeFile, "winlogon.exe"))
1561
-        {
1562
-            pid = entry.th32ProcessID;
1563
-            break;
1564
-        }
1565
-    }
1566
-    CloseHandle(process_snapshot);
1567
-    if (!pid)
1568
-    {
1569
-        RevertToSelf();
1570
-        return false;
1571
-    }
1572
-
1573
-    winlogon_process = OpenProcess(PROCESS_QUERY_INFORMATION, FALSE, pid);
1574
-    if (!winlogon_process)
1575
-    {
1576
-        RevertToSelf();
1577
-        return false;
1578
-    }
1579
-
1580
-    if (!OpenProcessToken(winlogon_process, TOKEN_IMPERSONATE | TOKEN_DUPLICATE, &winlogon_token))
1581
-    {
1582
-        CloseHandle(winlogon_process);
1583
-        RevertToSelf();
1584
-        return false;
1585
-    }
1586
-    CloseHandle(winlogon_process);
1587
-
1588
-    if (!DuplicateToken(winlogon_token, SecurityImpersonation, &duplicated_token))
1589
-    {
1590
-        CloseHandle(winlogon_token);
1591
-        RevertToSelf();
1592
-        return false;
1593
-    }
1594
-    CloseHandle(winlogon_token);
1595
-
1596
-    if (!SetThreadToken(NULL, duplicated_token))
1597
-    {
1598
-        CloseHandle(duplicated_token);
1599
-        RevertToSelf();
1600
-        return false;
1601
-    }
1602
-    CloseHandle(duplicated_token);
1603
-
1604
-    return true;
1605
-}
1606
-
1607 1512
 #endif /* ifdef _WIN32 */
... ...
@@ -37,4 +37,4 @@ openvpnserv_SOURCES = \
37 37
 	validate.c validate.h \
38 38
 	$(top_srcdir)/src/openvpn/block_dns.c $(top_srcdir)/src/openvpn/block_dns.h \
39 39
 	openvpnserv_resources.rc \
40
-	$(top_srcdir)/src/openvpn/ring_buffer.c $(top_srcdir)/src/openvpn/ring_buffer.h
40
+	$(top_srcdir)/src/openvpn/ring_buffer.h
... ...
@@ -115,7 +115,6 @@
115 115
     </Link>
116 116
   </ItemDefinitionGroup>
117 117
   <ItemGroup>
118
-    <ClCompile Include="..\openvpn\ring_buffer.c" />
119 118
     <ClCompile Include="automatic.c" />
120 119
     <ClCompile Include="common.c" />
121 120
     <ClCompile Include="interactive.c" />
... ...
@@ -33,9 +33,6 @@
33 33
     <ClCompile Include="..\openvpn\block_dns.c">
34 34
       <Filter>Source Files</Filter>
35 35
     </ClCompile>
36
-    <ClCompile Include="..\openvpn\ring_buffer.c">
37
-      <Filter>Source Files</Filter>
38
-    </ClCompile>
39 36
   </ItemGroup>
40 37
   <ItemGroup>
41 38
     <ClInclude Include="service.h">