From patchwork Fri Jul 24 00:48:41 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Lev Stipakov X-Patchwork-Id: 1328 Return-Path: Delivered-To: patchwork@openvpn.net Delivered-To: patchwork@openvpn.net Received: from director11.mail.ord1d.rsapps.net ([172.31.255.6]) by backend30.mail.ord1d.rsapps.net with LMTP id 2LKBKtq8Gl9pfQAAIUCqbw for ; Fri, 24 Jul 2020 06:50:02 -0400 Received: from proxy19.mail.iad3b.rsapps.net ([172.31.255.6]) by director11.mail.ord1d.rsapps.net with LMTP id UF4AKdq8Gl9CTAAAvGGmqA (envelope-from ) for ; Fri, 24 Jul 2020 06:50:02 -0400 Received: from smtp3.gate.iad3b ([172.31.255.6]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by proxy19.mail.iad3b.rsapps.net with LMTP id QCwXItq8Gl/TNAAAIG4riQ ; Fri, 24 Jul 2020 06:50:02 -0400 X-Spam-Threshold: 95 X-Spam-Score: 0 X-Spam-Flag: NO X-Virus-Scanned: OK X-Orig-To: openvpnslackdevel@openvpn.net X-Originating-Ip: [216.105.38.7] Authentication-Results: smtp3.gate.iad3b.rsapps.net; iprev=pass policy.iprev="216.105.38.7"; spf=pass smtp.mailfrom="openvpn-devel-bounces@lists.sourceforge.net" smtp.helo="lists.sourceforge.net"; dkim=fail (signature verification failed) header.d=sourceforge.net; dkim=fail (signature verification failed) header.d=sf.net; dkim=fail (signature verification failed) header.d=gmail.com; dmarc=fail (p=none; dis=none) header.from=gmail.com X-Suspicious-Flag: YES X-Classification-ID: 6cb39ec8-cd9b-11ea-9da5-525400bb3479-1-1 Received: from [216.105.38.7] ([216.105.38.7:55162] helo=lists.sourceforge.net) by smtp3.gate.iad3b.rsapps.net (envelope-from ) (ecelerity 4.2.38.62370 r(:)) with ESMTPS (cipher=DHE-RSA-AES256-GCM-SHA384) id 1A/02-07694-ADCBA1F5; Fri, 24 Jul 2020 06:50:02 -0400 Received: from [127.0.0.1] (helo=sfs-ml-2.v29.lw.sourceforge.com) by sfs-ml-2.v29.lw.sourceforge.com with esmtp (Exim 4.90_1) (envelope-from ) id 1jyvGP-0007ZB-6b; Fri, 24 Jul 2020 10:49:17 +0000 Received: from [172.30.20.202] (helo=mx.sourceforge.net) by sfs-ml-2.v29.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.90_1) (envelope-from ) id 1jyvGO-0007Z3-3h for openvpn-devel@lists.sourceforge.net; Fri, 24 Jul 2020 10:49:16 +0000 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sourceforge.net; s=x; h=Message-Id:Date:Subject:Cc:To:From:Sender:Reply-To: MIME-Version:Content-Type:Content-Transfer-Encoding:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:In-Reply-To:References:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=HO+2dDl9pAURS91gwb5Z+JKJBcA4kdJh+V10kDdgJ9k=; b=Aou9iXNjiZ/uxZke2wkXq+tzKw nD8JCoiIQ6nqTOuACP/M4QyEJG1Sj86kFplzAuK7MChlDOgcO/wU727nPkM9kTM/GMthqS87EEQJI 9kliB0kvvevztd31kyc5m/1pCtLnJjyI8uaZOWHH28+OgBPj1UQmgOrQ5yqBkc6ADHvA=; DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=sf.net; s=x ; h=Message-Id:Date:Subject:Cc:To:From:Sender:Reply-To:MIME-Version: Content-Type:Content-Transfer-Encoding:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: In-Reply-To:References:List-Id:List-Help:List-Unsubscribe:List-Subscribe: List-Post:List-Owner:List-Archive; bh=HO+2dDl9pAURS91gwb5Z+JKJBcA4kdJh+V10kDdgJ9k=; b=fJGZs0d3bS19rj1rjWKQV09YR9 Nnw2yOMityQPZj0BzrbcyfoXatYV/0hTI2JBi8Nue52ZtjQ3GltOXqy9VjpbccwOihZJOuubog42g VHsrDvNAgHJjubl/PqzkHsWJFYKwIWP5JE6QqYyMpX+mNgRB28armBl+0miuASnQglag=; Received: from mail-wr1-f54.google.com ([209.85.221.54]) by sfi-mx-1.v28.lw.sourceforge.com with esmtps (TLSv1.2:ECDHE-RSA-AES128-GCM-SHA256:128) (Exim 4.92.2) id 1jyvGM-007RBw-FB for openvpn-devel@lists.sourceforge.net; Fri, 24 Jul 2020 10:49:16 +0000 Received: by mail-wr1-f54.google.com with SMTP id r2so2795766wrs.8 for ; Fri, 24 Jul 2020 03:49:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id; bh=HO+2dDl9pAURS91gwb5Z+JKJBcA4kdJh+V10kDdgJ9k=; b=NZvuc/99K8z18eiaOux7Vpr1v3uUQcsb5u5lY2VEPPrS938FBoeLTWD5BDwwTC5EBQ jS7a8zej+O4hwHv/ETM/WOcSjnVnk5M2v3nJiUQ4Zf0cqGdjqEdEIREVQz7WzzO3e4R7 RKq9E7YqHLFcyI6u0QlNeSmBEUXk0gGwfgWdATPhT5ejZGbNif6HcQRHYX4IRLNuAlnF XU7AByxDra94vCEpIn8UDcDFtkg4DcM8zGKplfVV8zAUU4HaDpsLyNxM3XX2LP6kAExq roWB5sJ3Z6GGBwNzGNyYZhV5pIOBpc+oUAJxFpN9poK8IGiQUDu2QWRVbxP6Ne1QV8tF 9PGA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=HO+2dDl9pAURS91gwb5Z+JKJBcA4kdJh+V10kDdgJ9k=; b=WkdQTcf6zges+Mlpc0MBqss/BfH2SMZr0TUDaw7h9bOKcKI0Ws90grcQvgtxdsWXq7 6a9Em4jq4eWLRExGp/gjcuGhDSl6qTEMkp2c2YvmvylRU4K79E7ofOQl50LF8Dv7Db1q gy5WvQoJ7bOX/w2/v/ZdT5QYlCI0btlxBpO8nJZTH5fXHVGKkgGdJrkZtL3eOLwf/2hP efFE7oa78Rt2GvQhKx6bGvzt4+3BhKMjNHwfvWPj7HWCquXwdGMztHQeo22KkOnUH7za j06FlbdTbjVXCQeOFq1Ig0/YeHB73NC1Umcjgu2D3dbptZY6a6PHjviJ3Pr9WGPuz4+n 3xxw== X-Gm-Message-State: AOAM530JLhkRw+byeDDj9BmoTe85cnYBVMhRV0ZxDQKVjPiJsETPTtMn NragwVNFExZEb8LYceqraklQrTQk X-Google-Smtp-Source: ABdhPJzgyWec1qS37ihVaUABdXMUOpSAjXD8EzxQqEr6P2QLVQZlfTGAlvtfrwYg9PwvPY8C9uFdtA== X-Received: by 2002:a5d:56c6:: with SMTP id m6mr8702450wrw.363.1595587739575; Fri, 24 Jul 2020 03:48:59 -0700 (PDT) Received: from LAPTOP-4L3N7KFS.localdomain (nat1.panoulu.net. [185.38.2.1]) by smtp.gmail.com with ESMTPSA id s19sm981696wrb.54.2020.07.24.03.48.58 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 24 Jul 2020 03:48:59 -0700 (PDT) From: Lev Stipakov To: openvpn-devel@lists.sourceforge.net Date: Fri, 24 Jul 2020 13:48:41 +0300 Message-Id: <20200724104841.89-1-lstipakov@gmail.com> X-Mailer: git-send-email 2.17.1 X-Spam-Report: Spam Filtering performed by mx.sourceforge.net. See http://spamassassin.org/tag/ for more details. 0.0 FREEMAIL_FROM Sender email is commonly abused enduser mail provider (lstipakov[at]gmail.com) 0.0 URIBL_BLOCKED ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [URIs: makefile.am] -0.0 SPF_PASS SPF: sender matches SPF record 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record -0.0 RCVD_IN_MSPIKE_H2 RBL: Average reputation (+2) [209.85.221.54 listed in wl.mailspike.net] -0.0 RCVD_IN_DNSWL_NONE RBL: Sender listed at https://www.dnswl.org/, no trust [209.85.221.54 listed in list.dnswl.org] -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid X-Headers-End: 1jyvGM-007RBw-FB Subject: [Openvpn-devel] [PATCH] wintun: remove SYSTEM elevation hack X-BeenThere: openvpn-devel@lists.sourceforge.net X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Lev Stipakov MIME-Version: 1.0 Errors-To: openvpn-devel-bounces@lists.sourceforge.net X-getmail-retrieved-from-mailbox: Inbox From: Lev Stipakov 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 Acked-by: Gert Doering --- src/openvpn/Makefile.am | 2 +- src/openvpn/openvpn.vcxproj | 1 - src/openvpn/openvpn.vcxproj.filters | 3 - src/openvpn/ring_buffer.c | 56 ------------ src/openvpn/ring_buffer.h | 31 +++++-- src/openvpn/tun.c | 33 +------ src/openvpn/win32.c | 95 --------------------- src/openvpnserv/Makefile.am | 2 +- src/openvpnserv/openvpnserv.vcxproj | 1 - src/openvpnserv/openvpnserv.vcxproj.filters | 3 - 10 files changed, 32 insertions(+), 195 deletions(-) delete mode 100644 src/openvpn/ring_buffer.c diff --git a/src/openvpn/Makefile.am b/src/openvpn/Makefile.am index f0e0ad23..47ad762d 100644 --- a/src/openvpn/Makefile.am +++ b/src/openvpn/Makefile.am @@ -141,6 +141,6 @@ openvpn_LDADD = \ $(OPTIONAL_DL_LIBS) \ $(OPTIONAL_INOTIFY_LIBS) if WIN32 -openvpn_SOURCES += openvpn_win32_resources.rc block_dns.c block_dns.h ring_buffer.c ring_buffer.h +openvpn_SOURCES += openvpn_win32_resources.rc block_dns.c block_dns.h ring_buffer.h openvpn_LDADD += -lgdi32 -lws2_32 -lwininet -lcrypt32 -liphlpapi -lwinmm -lfwpuclnt -lrpcrt4 -lncrypt -lsetupapi endif diff --git a/src/openvpn/openvpn.vcxproj b/src/openvpn/openvpn.vcxproj index c34733ea..bd1a5844 100644 --- a/src/openvpn/openvpn.vcxproj +++ b/src/openvpn/openvpn.vcxproj @@ -189,7 +189,6 @@ - diff --git a/src/openvpn/openvpn.vcxproj.filters b/src/openvpn/openvpn.vcxproj.filters index 80eb52b3..e0bc7d5e 100644 --- a/src/openvpn/openvpn.vcxproj.filters +++ b/src/openvpn/openvpn.vcxproj.filters @@ -240,9 +240,6 @@ Source Files - - Source Files - Source Files diff --git a/src/openvpn/ring_buffer.c b/src/openvpn/ring_buffer.c deleted file mode 100644 index 8c81dc46..00000000 --- a/src/openvpn/ring_buffer.c +++ /dev/null @@ -1,56 +0,0 @@ -/* - * OpenVPN -- An application to securely tunnel IP networks - * over a single UDP port, with support for SSL/TLS-based - * session authentication and key exchange, - * packet encryption, packet authentication, and - * packet compression. - * - * Copyright (C) 2002-2019 OpenVPN Inc - * 2019 Lev Stipakov - * - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License version 2 - * as published by the Free Software Foundation. - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License along - * with this program; if not, write to the Free Software Foundation, Inc., - * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. - */ - -#include "ring_buffer.h" - -#ifdef _WIN32 - -bool -register_ring_buffers(HANDLE device, - struct tun_ring *send_ring, - struct tun_ring *receive_ring, - HANDLE send_tail_moved, - HANDLE receive_tail_moved) -{ - struct tun_register_rings rr; - BOOL res; - DWORD bytes_returned; - - ZeroMemory(&rr, sizeof(rr)); - - rr.send.ring = send_ring; - rr.send.ring_size = sizeof(struct tun_ring); - rr.send.tail_moved = send_tail_moved; - - rr.receive.ring = receive_ring; - rr.receive.ring_size = sizeof(struct tun_ring); - rr.receive.tail_moved = receive_tail_moved; - - res = DeviceIoControl(device, TUN_IOCTL_REGISTER_RINGS, &rr, sizeof(rr), - NULL, 0, &bytes_returned, NULL); - - return res != FALSE; -} - -#endif /* ifdef _WIN32 */ \ No newline at end of file diff --git a/src/openvpn/ring_buffer.h b/src/openvpn/ring_buffer.h index af46f106..4293f633 100644 --- a/src/openvpn/ring_buffer.h +++ b/src/openvpn/ring_buffer.h @@ -94,11 +94,32 @@ struct TUN_PACKET * that data has been written to receive ring * @return true if registration is successful, false otherwise - use GetLastError() */ -bool register_ring_buffers(HANDLE device, - struct tun_ring *send_ring, - struct tun_ring *receive_ring, - HANDLE send_tail_moved, - HANDLE receive_tail_moved); +static bool +register_ring_buffers(HANDLE device, + struct tun_ring *send_ring, + struct tun_ring *receive_ring, + HANDLE send_tail_moved, + HANDLE receive_tail_moved) +{ + struct tun_register_rings rr; + BOOL res; + DWORD bytes_returned; + + ZeroMemory(&rr, sizeof(rr)); + + rr.send.ring = send_ring; + rr.send.ring_size = sizeof(struct tun_ring); + rr.send.tail_moved = send_tail_moved; + + rr.receive.ring = receive_ring; + rr.receive.ring_size = sizeof(struct tun_ring); + rr.receive.tail_moved = receive_tail_moved; + + res = DeviceIoControl(device, TUN_IOCTL_REGISTER_RINGS, &rr, sizeof(rr), + NULL, 0, &bytes_returned, NULL); + + return res != FALSE; +} #endif /* ifndef OPENVPN_RING_BUFFER_H */ #endif /* ifdef _WIN32 */ diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c index e96368ca..82d96927 100644 --- a/src/openvpn/tun.c +++ b/src/openvpn/tun.c @@ -6148,35 +6148,10 @@ wintun_register_ring_buffer(struct tuntap *tt, const char *device_guid) } else { - if (!impersonate_as_system()) - { - msg(M_FATAL, "ERROR: Failed to impersonate as SYSTEM, make sure process is running under privileged account"); - } - 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, "Access denied registering ring buffers. Is this process run as SYSTEM?"); - 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; - } - if (!RevertToSelf()) - { - msg(M_FATAL, "ERROR: RevertToSelf error: %lu", GetLastError()); - } + 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)."); } return ret; diff --git a/src/openvpn/win32.c b/src/openvpn/win32.c index b2f2a19f..eb4c0307 100644 --- a/src/openvpn/win32.c +++ b/src/openvpn/win32.c @@ -1493,99 +1493,4 @@ send_msg_iservice(HANDLE pipe, const void *data, size_t size, return ret; } -bool -impersonate_as_system() -{ - HANDLE thread_token, process_snapshot, winlogon_process, winlogon_token, duplicated_token; - PROCESSENTRY32 entry; - BOOL ret; - DWORD pid = 0; - TOKEN_PRIVILEGES privileges; - - CLEAR(entry); - CLEAR(privileges); - - entry.dwSize = sizeof(PROCESSENTRY32); - - privileges.PrivilegeCount = 1; - privileges.Privileges->Attributes = SE_PRIVILEGE_ENABLED; - - if (!LookupPrivilegeValue(NULL, SE_DEBUG_NAME, &privileges.Privileges[0].Luid)) - { - return false; - } - - if (!ImpersonateSelf(SecurityImpersonation)) - { - return false; - } - - if (!OpenThreadToken(GetCurrentThread(), TOKEN_ADJUST_PRIVILEGES, FALSE, &thread_token)) - { - RevertToSelf(); - return false; - } - if (!AdjustTokenPrivileges(thread_token, FALSE, &privileges, sizeof(privileges), NULL, NULL)) - { - CloseHandle(thread_token); - RevertToSelf(); - return false; - } - CloseHandle(thread_token); - - process_snapshot = CreateToolhelp32Snapshot(TH32CS_SNAPPROCESS, 0); - if (process_snapshot == INVALID_HANDLE_VALUE) - { - RevertToSelf(); - return false; - } - for (ret = Process32First(process_snapshot, &entry); ret; ret = Process32Next(process_snapshot, &entry)) - { - if (!_stricmp(entry.szExeFile, "winlogon.exe")) - { - pid = entry.th32ProcessID; - break; - } - } - CloseHandle(process_snapshot); - if (!pid) - { - RevertToSelf(); - return false; - } - - winlogon_process = OpenProcess(PROCESS_QUERY_INFORMATION, FALSE, pid); - if (!winlogon_process) - { - RevertToSelf(); - return false; - } - - if (!OpenProcessToken(winlogon_process, TOKEN_IMPERSONATE | TOKEN_DUPLICATE, &winlogon_token)) - { - CloseHandle(winlogon_process); - RevertToSelf(); - return false; - } - CloseHandle(winlogon_process); - - if (!DuplicateToken(winlogon_token, SecurityImpersonation, &duplicated_token)) - { - CloseHandle(winlogon_token); - RevertToSelf(); - return false; - } - CloseHandle(winlogon_token); - - if (!SetThreadToken(NULL, duplicated_token)) - { - CloseHandle(duplicated_token); - RevertToSelf(); - return false; - } - CloseHandle(duplicated_token); - - return true; -} - #endif /* ifdef _WIN32 */ diff --git a/src/openvpnserv/Makefile.am b/src/openvpnserv/Makefile.am index f8d3319c..5dc38c9b 100644 --- a/src/openvpnserv/Makefile.am +++ b/src/openvpnserv/Makefile.am @@ -37,4 +37,4 @@ openvpnserv_SOURCES = \ validate.c validate.h \ $(top_srcdir)/src/openvpn/block_dns.c $(top_srcdir)/src/openvpn/block_dns.h \ openvpnserv_resources.rc \ - $(top_srcdir)/src/openvpn/ring_buffer.c $(top_srcdir)/src/openvpn/ring_buffer.h + $(top_srcdir)/src/openvpn/ring_buffer.h diff --git a/src/openvpnserv/openvpnserv.vcxproj b/src/openvpnserv/openvpnserv.vcxproj index c5a34b87..5e973df4 100644 --- a/src/openvpnserv/openvpnserv.vcxproj +++ b/src/openvpnserv/openvpnserv.vcxproj @@ -115,7 +115,6 @@ - diff --git a/src/openvpnserv/openvpnserv.vcxproj.filters b/src/openvpnserv/openvpnserv.vcxproj.filters index 3cb14ef6..41ad3e80 100644 --- a/src/openvpnserv/openvpnserv.vcxproj.filters +++ b/src/openvpnserv/openvpnserv.vcxproj.filters @@ -33,9 +33,6 @@ Source Files - - Source Files -