aboutsummaryrefslogtreecommitdiffstatshomepage
diff options
context:
space:
mode:
authorJason A. Donenfeld <Jason@zx2c4.com>2020-11-13 03:10:00 +0100
committerJason A. Donenfeld <Jason@zx2c4.com>2020-11-22 22:00:32 +0100
commit1c7606cea18e908cf76201ce1534b0afdc04cc89 (patch)
tree56c591b462989278a9bc89fafe927d7347122db5
parenttunnel: only enable DNS blocking for 0/0 configs (diff)
downloadwireguard-windows-1c7606cea18e908cf76201ce1534b0afdc04cc89.tar.xz
wireguard-windows-1c7606cea18e908cf76201ce1534b0afdc04cc89.zip
manager: allow S-1-5-32-556 users to launch a limited UI
I still have serious security reservations about this, both conceptually -- should users be allowed to do this stuff? -- and pratically -- there are issues with this implementation that need some examination. TODO: - Is that registry key a secure path? Should we double check it? - Are we leaking handles to the unpriv'd process from the manager? Audit this too. - IPC notifications are blocking. Should we move this to a go routine to mitigate DoS potential? - Is GOB deserialization secure? Can an NCO user crash or RCE the manager? Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
-rw-r--r--adminregistry.md18
-rw-r--r--attacksurface.md1
-rw-r--r--conf/admin_windows.go36
-rw-r--r--conf/config.go10
-rw-r--r--main.go18
-rw-r--r--manager/ipc_server.go45
-rw-r--r--manager/service.go57
-rw-r--r--ui/confview.go14
-rw-r--r--ui/tray.go4
-rw-r--r--ui/tunnelspage.go9
-rw-r--r--ui/ui.go1
-rw-r--r--updater/msirunner_windows.go1
12 files changed, 178 insertions, 36 deletions
diff --git a/adminregistry.md b/adminregistry.md
new file mode 100644
index 00000000..c7b8e559
--- /dev/null
+++ b/adminregistry.md
@@ -0,0 +1,18 @@
+## Registry Keys for Admins
+
+These are advanced configuration knobs that admins can set to do unusual things
+that are not recommended. There is no UI to enable these, and no such thing is
+planned. Use at your own risk, and please make sure you know what you're doing.
+
+#### `HKLM\Software\WireGuard\LimitedOperatorUI`
+
+When this key is set to `DWORD(1)`, the UI will be launched on desktops of
+users belonging to the Network Configuration Operators builtin group
+(S-1-5-32-556), with the following limitations for members of that group:
+
+ - Configurations are stripped of all public, private, and pre-shared keys;
+ - No version update notifications are delivered;
+ - Adding, removing, editing, importing, or exporting configurations is forbidden; and
+ - Quitting the manager is forbidden.
+
+However, basic functionality such as starting and stopping tunnels remains intact.
diff --git a/attacksurface.md b/attacksurface.md
index 7a4ff7d1..553806cf 100644
--- a/attacksurface.md
+++ b/attacksurface.md
@@ -31,6 +31,7 @@ The manager service is a userspace service running as Local System, responsible
- It listens for service changes in tunnel services according to the string prefix "WireGuardTunnel$".
- It manages DPAPI-encrypted configuration files in `C:\Program Files\WireGuard\Data`, which is created with `O:SYG:SYD:PAI(A;OICI;FA;;;SY)(A;OICI;FR;;;BA)`, and makes some effort to enforce good configuration filenames.
- It uses `WTSEnumerateSessions` and `WTSSESSION_NOTIFICATION` to walk through each available session. It then uses `WTSQueryUserToken`, and then calls `GetTokenInformation(TokenGroups)` on it. If one of the returned group's SIDs matches `IsWellKnownSid(WinBuiltinAdministratorsSid)`, and has attributes of either `SE_GROUP_ENABLED` or `SE_GROUP_USE_FOR_DENY_ONLY` and calling `GetTokenInformation(TokenElevation)` on it or its `TokenLinkedToken` indicates that either is elevated, then it spawns the UI process as that the elevated user token, passing it three unnamed pipe handles for IPC and the log mapping handle, as described above.
+ - In the event that the administrator has set `HKLM\Software\WireGuard\LimitedOperatorUI` to 1, sessions are started for users that are a member of group S-1-5-32-556, with a more limited IPC interface, in which these non-admin users are denied private keys and tunnel editing rights. (This means users can potentially DoS the IPC server by draining notifications too slowly, or exhausting memory of the manager by spawning too many watcher go routines, or by sending garbage data that Go's `gob` decoder isn't expecting.)
### UI
diff --git a/conf/admin_windows.go b/conf/admin_windows.go
new file mode 100644
index 00000000..2f97f4da
--- /dev/null
+++ b/conf/admin_windows.go
@@ -0,0 +1,36 @@
+/* SPDX-License-Identifier: MIT
+ *
+ * Copyright (C) 2020 WireGuard LLC. All Rights Reserved.
+ */
+
+package conf
+
+import "golang.org/x/sys/windows/registry"
+
+const adminRegKey = `Software\WireGuard`
+
+var adminKey registry.Key
+
+func openAdminKey() (registry.Key, error) {
+ if adminKey != 0 {
+ return adminKey, nil
+ }
+ var err error
+ adminKey, err = registry.OpenKey(registry.LOCAL_MACHINE, adminRegKey, registry.QUERY_VALUE)
+ if err != nil {
+ return 0, err
+ }
+ return adminKey, nil
+}
+
+func AdminBool(name string) bool {
+ key, err := openAdminKey()
+ if err != nil {
+ return false
+ }
+ val, _, err := key.GetIntegerValue(name)
+ if err != nil {
+ return false
+ }
+ return val != 0
+}
diff --git a/conf/config.go b/conf/config.go
index f5f25add..1ce1988d 100644
--- a/conf/config.go
+++ b/conf/config.go
@@ -84,7 +84,7 @@ func (r *IPCidr) IPNet() net.IPNet {
func (r *IPCidr) MaskSelf() {
bits := int(r.Bits())
mask := net.CIDRMask(int(r.Cidr), bits)
- for i := 0; i < bits / 8; i++ {
+ for i := 0; i < bits/8; i++ {
r.IP[i] &= mask[i]
}
}
@@ -238,3 +238,11 @@ func (conf *Config) DeduplicateNetworkEntries() {
peer.AllowedIPs = peer.AllowedIPs[:i]
}
}
+
+func (conf *Config) Redact() {
+ conf.Interface.PrivateKey = Key{}
+ for i := range conf.Peers {
+ conf.Peers[i].PublicKey = Key{}
+ conf.Peers[i].PresharedKey = Key{}
+ }
+}
diff --git a/main.go b/main.go
index 868be5d0..b8c385fe 100644
--- a/main.go
+++ b/main.go
@@ -137,10 +137,10 @@ func main() {
checkForWow64()
if len(os.Args) <= 1 {
- checkForAdminGroup()
if ui.RaiseUI() {
return
}
+ checkForAdminGroup()
err := execElevatedManagerServiceInstaller()
if err != nil {
fatal(err)
@@ -213,9 +213,18 @@ func main() {
if len(os.Args) != 6 {
usage()
}
- err := elevate.DropAllPrivileges(false)
- if err != nil {
- fatal(err)
+ var processToken windows.Token
+ isAdmin := false
+ err := windows.OpenProcessToken(windows.CurrentProcess(), windows.TOKEN_QUERY|windows.TOKEN_DUPLICATE, &processToken)
+ if err == nil {
+ isAdmin = elevate.TokenIsElevatedOrElevatable(processToken)
+ processToken.Close()
+ }
+ if isAdmin {
+ err := elevate.DropAllPrivileges(false)
+ if err != nil {
+ fatal(err)
+ }
}
readPipe, err := pipeFromHandleArgument(os.Args[2])
if err != nil {
@@ -234,6 +243,7 @@ func main() {
fatal(err)
}
manager.InitializeIPCClient(readPipe, writePipe, eventPipe)
+ ui.IsAdmin = isAdmin
ui.RunUI()
return
case "/dumplog":
diff --git a/manager/ipc_server.go b/manager/ipc_server.go
index 98d4ffd1..38b29336 100644
--- a/manager/ipc_server.go
+++ b/manager/ipc_server.go
@@ -38,7 +38,14 @@ type ManagerService struct {
}
func (s *ManagerService) StoredConfig(tunnelName string) (*conf.Config, error) {
- return conf.LoadFromName(tunnelName)
+ conf, err := conf.LoadFromName(tunnelName)
+ if err != nil {
+ return nil, err
+ }
+ if s.elevatedToken == 0 {
+ conf.Redact()
+ }
+ return conf, nil
}
func (s *ManagerService) RuntimeConfig(tunnelName string) (*conf.Config, error) {
@@ -69,7 +76,14 @@ func (s *ManagerService) RuntimeConfig(tunnelName string) (*conf.Config, error)
if err != nil {
return nil, err
}
- return conf.FromUAPI(string(resp), storedConfig)
+ conf, err := conf.FromUAPI(string(resp), storedConfig)
+ if err != nil {
+ return nil, err
+ }
+ if s.elevatedToken == 0 {
+ conf.Redact()
+ }
+ return conf, nil
}
func (s *ManagerService) Start(tunnelName string) error {
@@ -149,6 +163,9 @@ func (s *ManagerService) WaitForStop(tunnelName string) error {
}
func (s *ManagerService) Delete(tunnelName string) error {
+ if s.elevatedToken == 0 {
+ return windows.ERROR_ACCESS_DENIED
+ }
err := s.Stop(tunnelName)
if err != nil {
return err
@@ -193,6 +210,9 @@ func (s *ManagerService) GlobalState() TunnelState {
}
func (s *ManagerService) Create(tunnelConfig *conf.Config) (*Tunnel, error) {
+ if s.elevatedToken == 0 {
+ return nil, windows.ERROR_ACCESS_DENIED
+ }
err := tunnelConfig.Save()
if err != nil {
return nil, err
@@ -216,6 +236,9 @@ func (s *ManagerService) Tunnels() ([]Tunnel, error) {
}
func (s *ManagerService) Quit(stopTunnelsOnQuit bool) (alreadyQuit bool, err error) {
+ if s.elevatedToken == 0 {
+ return false, windows.ERROR_ACCESS_DENIED
+ }
if !atomic.CompareAndSwapUint32(&haveQuit, 0, 1) {
return true, nil
}
@@ -244,6 +267,9 @@ func (s *ManagerService) UpdateState() UpdateState {
}
func (s *ManagerService) Update() {
+ if s.elevatedToken == 0 {
+ return
+ }
progress := updater.DownloadVerifyAndExecute(uintptr(s.elevatedToken))
go func() {
for {
@@ -443,7 +469,7 @@ func IPCServerListen(reader *os.File, writer *os.File, events *os.File, elevated
}()
}
-func notifyAll(notificationType NotificationType, ifaces ...interface{}) {
+func notifyAll(notificationType NotificationType, adminOnly bool, ifaces ...interface{}) {
if len(managerServices) == 0 {
return
}
@@ -463,6 +489,9 @@ func notifyAll(notificationType NotificationType, ifaces ...interface{}) {
managerServicesLock.RLock()
for m := range managerServices {
+ if m.elevatedToken == 0 && adminOnly {
+ continue
+ }
m.events.SetWriteDeadline(time.Now().Add(time.Second))
m.events.Write(buf.Bytes())
}
@@ -477,22 +506,22 @@ func errToString(err error) string {
}
func IPCServerNotifyTunnelChange(name string, state TunnelState, err error) {
- notifyAll(TunnelChangeNotificationType, name, state, trackedTunnelsGlobalState(), errToString(err))
+ notifyAll(TunnelChangeNotificationType, false, name, state, trackedTunnelsGlobalState(), errToString(err))
}
func IPCServerNotifyTunnelsChange() {
- notifyAll(TunnelsChangeNotificationType)
+ notifyAll(TunnelsChangeNotificationType, false)
}
func IPCServerNotifyUpdateFound(state UpdateState) {
- notifyAll(UpdateFoundNotificationType, state)
+ notifyAll(UpdateFoundNotificationType, true, state)
}
func IPCServerNotifyUpdateProgress(dp updater.DownloadProgress) {
- notifyAll(UpdateProgressNotificationType, dp.Activity, dp.BytesDownloaded, dp.BytesTotal, errToString(dp.Error), dp.Complete)
+ notifyAll(UpdateProgressNotificationType, true, dp.Activity, dp.BytesDownloaded, dp.BytesTotal, errToString(dp.Error), dp.Complete)
}
func IPCServerNotifyManagerStopping() {
- notifyAll(ManagerStoppingNotificationType)
+ notifyAll(ManagerStoppingNotificationType, false)
time.Sleep(time.Millisecond * 200)
}
diff --git a/manager/service.go b/manager/service.go
index 6c3b039b..48ecd129 100644
--- a/manager/service.go
+++ b/manager/service.go
@@ -90,6 +90,7 @@ func (service *managerService) Execute(args []string, r <-chan svc.ChangeRequest
aliveSessions := make(map[uint32]bool)
procsLock := sync.Mutex{}
stoppingManager := false
+ operatorGroupSid, _ := windows.CreateWellKnownSid(windows.WinBuiltinNetworkConfigurationOperatorsSid)
startProcess := func(session uint32) {
defer func() {
@@ -104,7 +105,24 @@ func (service *managerService) Execute(args []string, r <-chan svc.ChangeRequest
if err != nil {
return
}
- if !elevate.TokenIsElevatedOrElevatable(userToken) {
+ isAdmin := elevate.TokenIsElevatedOrElevatable(userToken)
+ isOperator := false
+ if !isAdmin && conf.AdminBool("LimitedOperatorUI") && operatorGroupSid != nil {
+ linkedToken, err := userToken.GetLinkedToken()
+ var impersonationToken windows.Token
+ if err == nil {
+ err = windows.DuplicateTokenEx(linkedToken, windows.TOKEN_QUERY, nil, windows.SecurityImpersonation, windows.TokenImpersonation, &impersonationToken)
+ linkedToken.Close()
+ } else {
+ err = windows.DuplicateTokenEx(userToken, windows.TOKEN_QUERY, nil, windows.SecurityImpersonation, windows.TokenImpersonation, &impersonationToken)
+ }
+ if err == nil {
+ isOperator, err = impersonationToken.IsMember(operatorGroupSid)
+ isOperator = isOperator && err == nil
+ impersonationToken.Close()
+ }
+ }
+ if !isAdmin && !isOperator {
userToken.Close()
return
}
@@ -125,23 +143,28 @@ func (service *managerService) Execute(args []string, r <-chan svc.ChangeRequest
return
}
userProfileDirectory, _ := userToken.GetUserProfileDirectory()
- var elevatedToken windows.Token
- if userToken.IsElevated() {
- elevatedToken = userToken
- } else {
- elevatedToken, err = userToken.GetLinkedToken()
- userToken.Close()
- if err != nil {
- log.Printf("Unable to elevate token: %v", err)
- return
- }
- if !elevatedToken.IsElevated() {
- elevatedToken.Close()
- log.Println("Linked token is not elevated")
- return
+ var elevatedToken, runToken windows.Token
+ if isAdmin {
+ if userToken.IsElevated() {
+ elevatedToken = userToken
+ } else {
+ elevatedToken, err = userToken.GetLinkedToken()
+ userToken.Close()
+ if err != nil {
+ log.Printf("Unable to elevate token: %v", err)
+ return
+ }
+ if !elevatedToken.IsElevated() {
+ elevatedToken.Close()
+ log.Println("Linked token is not elevated")
+ return
+ }
}
+ runToken = elevatedToken
+ } else {
+ runToken = userToken
}
- defer elevatedToken.Close()
+ defer runToken.Close()
userToken = 0
first := true
for {
@@ -186,7 +209,7 @@ func (service *managerService) Execute(args []string, r <-chan svc.ChangeRequest
log.Printf("Starting UI process for user ‘%s@%s’ for session %d", username, domain, session)
attr := &os.ProcAttr{
Sys: &syscall.SysProcAttr{
- Token: syscall.Token(elevatedToken),
+ Token: syscall.Token(runToken),
},
Files: []*os.File{devNull, devNull, devNull},
Dir: userProfileDirectory,
diff --git a/ui/confview.go b/ui/confview.go
index e34d81b1..089f6572 100644
--- a/ui/confview.go
+++ b/ui/confview.go
@@ -364,7 +364,11 @@ func (iv *interfaceView) widgetsLines() []widgetsLine {
}
func (iv *interfaceView) apply(c *conf.Interface) {
- iv.publicKey.show(c.PrivateKey.Public().String())
+ if IsAdmin {
+ iv.publicKey.show(c.PrivateKey.Public().String())
+ } else {
+ iv.publicKey.hide()
+ }
if c.ListenPort > 0 {
iv.listenPort.show(strconv.Itoa(int(c.ListenPort)))
@@ -405,9 +409,13 @@ func (pv *peerView) widgetsLines() []widgetsLine {
}
func (pv *peerView) apply(c *conf.Peer) {
- pv.publicKey.show(c.PublicKey.String())
+ if IsAdmin {
+ pv.publicKey.show(c.PublicKey.String())
+ } else {
+ pv.publicKey.hide()
+ }
- if !c.PresharedKey.IsZero() {
+ if !c.PresharedKey.IsZero() && IsAdmin {
pv.presharedKey.show(l18n.Sprintf("enabled"))
} else {
pv.presharedKey.hide()
diff --git a/ui/tray.go b/ui/tray.go
index 768d72a3..006e20a5 100644
--- a/ui/tray.go
+++ b/ui/tray.go
@@ -82,10 +82,10 @@ func (tray *Tray) setup() error {
{separator: true},
{separator: true},
{label: l18n.Sprintf("&Manage tunnels…"), handler: tray.onManageTunnels, enabled: true, defawlt: true},
- {label: l18n.Sprintf("&Import tunnel(s) from file…"), handler: tray.onImport, enabled: true},
+ {label: l18n.Sprintf("&Import tunnel(s) from file…"), handler: tray.onImport, enabled: true, hidden: !IsAdmin},
{separator: true},
{label: l18n.Sprintf("&About WireGuard…"), handler: tray.onAbout, enabled: true},
- {label: l18n.Sprintf("E&xit"), handler: onQuit, enabled: true},
+ {label: l18n.Sprintf("E&xit"), handler: onQuit, enabled: true, hidden: !IsAdmin},
} {
var action *walk.Action
if item.separator {
diff --git a/ui/tunnelspage.go b/ui/tunnelspage.go
index 791007cf..e59e8166 100644
--- a/ui/tunnelspage.go
+++ b/ui/tunnelspage.go
@@ -76,6 +76,7 @@ func NewTunnelsPage() (*TunnelsPage, error) {
tp.fillerContainer.SetLayout(hlayout)
tp.fillerButton, _ = walk.NewPushButton(tp.fillerContainer)
tp.fillerButton.SetMinMaxSize(walk.Size{200, 0}, walk.Size{200, 0})
+ tp.fillerButton.SetVisible(IsAdmin)
tp.fillerButton.Clicked().Attach(func() {
if tp.fillerHandler != nil {
tp.fillerHandler()
@@ -105,6 +106,7 @@ func NewTunnelsPage() (*TunnelsPage, error) {
})
editTunnel.SetText(l18n.Sprintf("&Edit"))
editTunnel.Clicked().Attach(tp.onEditTunnel)
+ editTunnel.SetVisible(IsAdmin)
disposables.Spare()
@@ -133,6 +135,7 @@ func (tp *TunnelsPage) CreateToolbar() error {
hlayout := walk.NewHBoxLayout()
hlayout.SetMargins(walk.Margins{})
toolBarContainer.SetLayout(hlayout)
+ toolBarContainer.SetVisible(IsAdmin)
if tp.listToolbar, err = walk.NewToolBarWithOrientationAndButtonStyle(toolBarContainer, walk.Horizontal, walk.ToolBarButtonImageBeforeText); err != nil {
return err
@@ -206,34 +209,40 @@ func (tp *TunnelsPage) CreateToolbar() error {
importAction2.SetText(l18n.Sprintf("&Import tunnel(s) from file…"))
importAction2.SetShortcut(walk.Shortcut{walk.ModControl, walk.KeyO})
importAction2.Triggered().Attach(tp.onImport)
+ importAction2.SetVisible(IsAdmin)
contextMenu.Actions().Add(importAction2)
tp.ShortcutActions().Add(importAction2)
addAction2 := walk.NewAction()
addAction2.SetText(l18n.Sprintf("Add &empty tunnel…"))
addAction2.SetShortcut(walk.Shortcut{walk.ModControl, walk.KeyN})
addAction2.Triggered().Attach(tp.onAddTunnel)
+ addAction2.SetVisible(IsAdmin)
contextMenu.Actions().Add(addAction2)
tp.ShortcutActions().Add(addAction2)
exportAction2 := walk.NewAction()
exportAction2.SetText(l18n.Sprintf("Export all tunnels to &zip…"))
exportAction2.Triggered().Attach(tp.onExportTunnels)
+ exportAction2.SetVisible(IsAdmin)
contextMenu.Actions().Add(exportAction2)
contextMenu.Actions().Add(walk.NewSeparatorAction())
editAction := walk.NewAction()
editAction.SetText(l18n.Sprintf("Edit &selected tunnel…"))
editAction.SetShortcut(walk.Shortcut{walk.ModControl, walk.KeyE})
+ editAction.SetVisible(IsAdmin)
editAction.Triggered().Attach(tp.onEditTunnel)
contextMenu.Actions().Add(editAction)
tp.ShortcutActions().Add(editAction)
deleteAction2 := walk.NewAction()
deleteAction2.SetText(l18n.Sprintf("&Remove selected tunnel(s)"))
deleteAction2.SetShortcut(walk.Shortcut{0, walk.KeyDelete})
+ deleteAction2.SetVisible(IsAdmin)
deleteAction2.Triggered().Attach(tp.onDelete)
contextMenu.Actions().Add(deleteAction2)
tp.listView.ShortcutActions().Add(deleteAction2)
selectAllAction := walk.NewAction()
selectAllAction.SetText(l18n.Sprintf("Select &all"))
selectAllAction.SetShortcut(walk.Shortcut{walk.ModControl, walk.KeyA})
+ selectAllAction.SetVisible(IsAdmin)
selectAllAction.Triggered().Attach(tp.onSelectAll)
contextMenu.Actions().Add(selectAllAction)
tp.listView.ShortcutActions().Add(selectAllAction)
diff --git a/ui/ui.go b/ui/ui.go
index 0279a8dd..637a1848 100644
--- a/ui/ui.go
+++ b/ui/ui.go
@@ -23,6 +23,7 @@ import (
var noTrayAvailable = false
var shouldQuitManagerWhenExiting = false
var startTime = time.Now()
+var IsAdmin = false // A global, because this really is global for the process
func RunUI() {
runtime.LockOSThread()
diff --git a/updater/msirunner_windows.go b/updater/msirunner_windows.go
index 2f5ce5a4..d901274c 100644
--- a/updater/msirunner_windows.go
+++ b/updater/msirunner_windows.go
@@ -32,7 +32,6 @@ func (t *tempFile) ExclusivePath() string {
return t.Name()
}
-
func (t *tempFile) Delete() error {
if t.originalHandle == 0 {
name16, err := windows.UTF16PtrFromString(t.Name())