Development discussion of WireGuard
 help / color / mirror / Atom feed
From: Patrik Holmqvist <patrik.holmqvist@su.se>
To: "Jason A. Donenfeld" <Jason@zx2c4.com>,
	"vh217@werehub.org" <vh217@werehub.org>
Cc: WireGuard mailing list <wireguard@lists.zx2c4.com>
Subject: RE: Using WireGuard on Windows as non-admin - proper solution?
Date: Sun, 15 Nov 2020 15:28:27 +0000	[thread overview]
Message-ID: <665e0a8610984d938ab013ec2aac8517@su.se> (raw)
In-Reply-To: <CAHmME9p6FoUsFou0LkeUZt4Ba1K4ycuA6f8X4c6Kt8-jzkAH7A@mail.gmail.com>

Hi!

We (Stockholm University) are also interested in a version that does not require local administrator permissions to start the application and handle tunnel configuration.
We are currently evaluating WireGuard as a VPN solution for our employees (and maybe students in the future) since we are not really happy with our current solution.

Unfortunately we ran into an issue that has been discussed on this list before, that WireGuard for Windows requires the user to be Local Admin on the machine in order to start the application.

We are not very keen on making all our users Local Admin on their managed machines, and we think we share that with most larger enterprise environments since it goes against best security practices [0] [1]. We thought that the builtin group "Network Configuration Operators" could be a good alternative to Local Admin as to who are allowed to manage WireGuard. So we decided to try it out and patched the code, see diff below. (This was just to test the concept. We do not claim that this is a good solution, but it was what we managed to produced with limited time and knowledge)

From our test it seems to work, the users in the NCO group are fully allowed to manage the tunnels. The installation is of course required to be done by a Local Admin or as in our setup by SCCM [2]. 

Do you think you could implement something like this to the upstream version of WireGuard in order to allow larger organizations to easier roll out WireGuard in a more selective manner? We are also fine with removing the requirement to be a member of any particular group if that could be achieved.

[0] https://docs.microsoft.com/en-us/windows-server/identity/ad-ds/plan/security-best-practices/implementing-least-privilege-administrative-models
[1] https://www.bleepingcomputer.com/news/microsoft/removing-user-admin-rights-mitigates-94-percent-of-all-critical-microsoft-vulnerabilities/
[2] https://en.wikipedia.org/wiki/Microsoft_System_Center_Configuration_Manager

Best regards and thank you for all the fine work with this great product!

--
Patrik

---
elevate/membership.go | 24 +++++++++++++++++++-----
main.go               |  2 ++
2 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/elevate/membership.go b/elevate/membership.go
index 07c2ef69..c1fdcb7b 100644
--- a/elevate/membership.go
+++ b/elevate/membership.go
@@ -14,14 +14,28 @@ func isAdmin(token windows.Token) bool {
                             if err != nil {
                                                          return false
                             }
-                           var checkableToken windows.Token
-                           err = windows.DuplicateTokenEx(token, windows.TOKEN_QUERY|windows.TOKEN_IMPERSONATE, nil, windows.SecurityIdentification, windows.TokenImpersonation, &checkableToken)
+                          builtinNetworkOperatorsGroup, err := windows.CreateWellKnownSid(windows.WinBuiltinNetworkConfigurationOperatorsSid)
                             if err != nil {
                                                          return false
                             }
-                           defer checkableToken.Close()
-                           isAdmin, err := checkableToken.IsMember(builtinAdminsGroup)
-                           return isAdmin && err == nil
+                          var checkableAdminToken windows.Token
+                          var checkableNetworkOperatorsToken windows.Token
+                          defer checkableAdminToken.Close()
+                          defer checkableNetworkOperatorsToken.Close()
+                          err = windows.DuplicateTokenEx(token, windows.TOKEN_QUERY|windows.TOKEN_IMPERSONATE, nil, windows.SecurityIdentification, windows.TokenImpersonation, &checkableAdminToken)
+                          if err != nil {
+                                                       return false
+                          }
+                          err = windows.DuplicateTokenEx(token, windows.TOKEN_QUERY|windows.TOKEN_IMPERSONATE, nil, windows.SecurityIdentification, windows.TokenImpersonation, &checkableNetworkOperatorsToken)
+                          if err != nil {
+                                                       return false
+                          }
+                          isAdmin, err := checkableAdminToken.IsMember(builtinAdminsGroup)
+                          isNetworkOperator, err := checkableNetworkOperatorsToken.IsMember(builtinNetworkOperatorsGroup)
+                          if isAdmin || isNetworkOperator {
+                                                       return true && err == nil
+                          }
+                          return false && err == nil
}

func TokenIsElevatedOrElevatable(token windows.Token) bool {
diff --git a/main.go b/main.go
index 79dfcdfc..e8a48e8e 100644
--- a/main.go
+++ b/main.go
@@ -78,6 +78,7 @@ func checkForAdminGroup() {
                             }
                             defer processToken.Close()
                             if !elevate.TokenIsElevatedOrElevatable(processToken) {
+                                                       // TODO Logic for multiple groups needs to be added for the correct error message to be displayed
                                                          fatalf("WireGuard may only be used by users who are a member of the Builtin %s group.", elevate.AdminGroupName())
                             }
}
@@ -85,6 +86,7 @@ func checkForAdminGroup() {
func checkForAdminDesktop() {
                             adminDesktop, err := elevate.IsAdminDesktop()
                             if !adminDesktop && err == nil {
+                                                       // TODO Logic for multiple groups needs to be added for the correct error message to be displayed
                                                          fatalf("WireGuard is running, but the UI is only accessible from desktops of the Builtin %s group.", elevate.AdminGroupName())
                             }
}
-- 
2.20.1

-----Original Message-----
From: WireGuard <wireguard-bounces@lists.zx2c4.com> On Behalf Of Jason A. Donenfeld
Sent: den 13 november 2020 03:16
To: vh217@werehub.org
Cc: WireGuard mailing list <wireguard@lists.zx2c4.com>
Subject: Re: Using WireGuard on Windows as non-admin - proper solution?

Hi Viktor,

I am actually interested in solving this. I took an initial stab at it here, but I'm not super comfortable with the implementation or the security implications:
https://git.zx2c4.com/wireguard-windows/commit/?h=jd/unprivd-knob

Aside from doing this from within our existing UI, the general solution using the service-based building blocks is to simply allow users to start and stop services that begin with "WireGuardTunnel$".
So the flow is something like:

1. wireguard /installtunnelservice  path\to\sometunnel.conf.
2. Change the ACLs on WireGuardTunnel$sometunnel to fit your user.
3. Have the user use `net start` and `net stop`, or similar, to control whether the service is up or down.

That's not super pretty, but it should work, and it is automatable.
Meanwhile, I'll keep thinking about various ways to do this in a more "first-party" way.

Jason

  parent reply	other threads:[~2020-11-15 22:46 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-12 15:18 vh217
2020-11-13  2:16 ` Jason A. Donenfeld
2020-11-13 12:03   ` Der PCFreak
2020-11-15 15:28   ` Patrik Holmqvist [this message]
2020-11-19 16:56     ` Jason A. Donenfeld
2020-11-20 11:49       ` Patrik Holmqvist
2020-11-20 12:52         ` Jason A. Donenfeld
2020-11-20 13:10           ` Patrick Fogarty
2020-11-20 13:14           ` Patrik Holmqvist
2020-11-17 10:18   ` Viktor H
2020-11-26  7:09   ` Chris Bennett
2020-11-21 10:05 ` Jason A. Donenfeld
2020-11-22 12:55   ` Jason A. Donenfeld
2020-11-23 14:57     ` Fatih USTA
2020-11-24 23:42   ` Riccardo Paolo Bestetti
2020-11-25  1:08     ` Jason A. Donenfeld
2020-11-25  7:49       ` Riccardo Paolo Bestetti
2020-11-25 10:30         ` Jason A. Donenfeld
2020-11-25 11:45           ` Jason A. Donenfeld
2020-11-25 14:08             ` Riccardo Paolo Bestetti
     [not found]               ` <8bf9e364f87bd0018dabca03dcc8c19b@mail.gmail.com>
2020-11-25 20:10                 ` Riccardo Paolo Bestetti
2020-11-25 21:42                 ` Jason A. Donenfeld
2020-11-26  8:53                   ` Adrian Larsen
2020-11-28 14:28                     ` Jason A. Donenfeld
2020-11-29  9:30                       ` Adrian Larsen
2020-11-29 10:52                         ` Jason A. Donenfeld
2020-11-29 12:09                           ` Phillip McMahon
2020-11-29 12:50                             ` Jason A. Donenfeld
2020-11-29 13:40                               ` Phillip McMahon
2020-11-29 17:52                                 ` Jason A. Donenfeld
2020-11-29 19:44                                   ` Phillip McMahon
2020-11-29 20:59                                     ` Jason A. Donenfeld
2020-11-30 18:34                                       ` Riccardo Paolo Bestetti
2022-04-22 20:21                                       ` zer0flash
2020-11-30 12:47                                   ` Probable Heresy ;-) Peter Whisker
2020-12-02 13:40                                     ` Jason A. Donenfeld
2021-01-03 11:08                                       ` Christopher Ng
2020-11-25 12:40     ` AW: Using WireGuard on Windows as non-admin - proper solution? Joachim Lindenberg
2020-11-25 13:08       ` Jason A. Donenfeld

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=665e0a8610984d938ab013ec2aac8517@su.se \
    --to=patrik.holmqvist@su.se \
    --cc=Jason@zx2c4.com \
    --cc=vh217@werehub.org \
    --cc=wireguard@lists.zx2c4.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).