aboutsummaryrefslogtreecommitdiffstatshomepage
diff options
context:
space:
mode:
authorJason A. Donenfeld <Jason@zx2c4.com>2020-11-21 14:59:03 +0100
committerJason A. Donenfeld <Jason@zx2c4.com>2020-11-22 22:00:32 +0100
commit259e2cb5eb2507d0321ceb9f46278df18ebe3066 (patch)
tree9f210d076e2768378b927440c67904c446d13f33
parentmanager: move IPC notification to go routine per client (diff)
downloadwireguard-windows-259e2cb5eb2507d0321ceb9f46278df18ebe3066.tar.xz
wireguard-windows-259e2cb5eb2507d0321ceb9f46278df18ebe3066.zip
conf: allow administrators to add and remove configs easily
We loosen the permissions a little bit while tightening the restrictions on encrypted files. This should allow administrators to easily drop unencrypted files into Data\Configurations\ and get them encrypted and made read-only, while also allowing them to delete unwanted configurations. Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
-rw-r--r--attacksurface.md3
-rw-r--r--conf/filewriter_windows.go72
-rw-r--r--conf/migration_windows.go10
-rw-r--r--conf/path_windows.go2
-rw-r--r--conf/store.go13
5 files changed, 78 insertions, 22 deletions
diff --git a/attacksurface.md b/attacksurface.md
index 553806cf..eb8baade 100644
--- a/attacksurface.md
+++ b/attacksurface.md
@@ -29,7 +29,8 @@ The manager service is a userspace service running as Local System, responsible
- Extensive IPC using unnamed pipes, inherited by the UI process.
- A readable `CreateFileMapping` handle to a binary ringlog shared by all services, inherited by the UI process.
- 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 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;FA;;;BA)`, and makes some effort to enforce good configuration filenames.
+ - The actual DPAPI-encrypted configuration files are created with `O:SYG:SYD:PAI(A;;FA;;;SY)(A;;SD;;;BA)`.
- 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.)
diff --git a/conf/filewriter_windows.go b/conf/filewriter_windows.go
new file mode 100644
index 00000000..9fb1f566
--- /dev/null
+++ b/conf/filewriter_windows.go
@@ -0,0 +1,72 @@
+/* SPDX-License-Identifier: MIT
+ *
+ * Copyright (C) 2020 WireGuard LLC. All Rights Reserved.
+ */
+
+package conf
+
+import (
+ "sync/atomic"
+ "unsafe"
+
+ "golang.org/x/sys/windows"
+)
+
+var encryptedFileSd unsafe.Pointer
+
+func writeEncryptedFile(destination string, contents []byte) error {
+ var err error
+ sa := &windows.SecurityAttributes{Length: uint32(unsafe.Sizeof(windows.SecurityAttributes{}))}
+ sa.SecurityDescriptor = (*windows.SECURITY_DESCRIPTOR)(atomic.LoadPointer(&encryptedFileSd))
+ if sa.SecurityDescriptor == nil {
+ sa.SecurityDescriptor, err = windows.SecurityDescriptorFromString("O:SYG:SYD:PAI(A;;FA;;;SY)(A;;SD;;;BA)")
+ if err != nil {
+ return err
+ }
+ atomic.StorePointer(&encryptedFileSd, unsafe.Pointer(sa.SecurityDescriptor))
+ }
+ destination16, err := windows.UTF16FromString(destination)
+ if err != nil {
+ return err
+ }
+ tmpDestination := destination + ".tmp"
+ tmpDestination16, err := windows.UTF16PtrFromString(tmpDestination)
+ if err != nil {
+ return err
+ }
+ handle, err := windows.CreateFile(tmpDestination16, windows.GENERIC_WRITE|windows.DELETE, 0, sa, windows.CREATE_ALWAYS, windows.FILE_ATTRIBUTE_NORMAL, 0)
+ if err != nil {
+ return err
+ }
+ defer windows.CloseHandle(handle)
+ deleteIt := func() {
+ yes := byte(1)
+ windows.SetFileInformationByHandle(handle, windows.FileDispositionInfo, &yes, 1)
+ }
+ n, err := windows.Write(handle, contents)
+ if err != nil {
+ deleteIt()
+ return err
+ }
+ if n != len(contents) {
+ deleteIt()
+ return windows.ERROR_IO_INCOMPLETE
+ }
+ fileRenameInfo := &struct {
+ replaceIfExists byte
+ rootDirectory windows.Handle
+ fileNameLength uint32
+ fileName [windows.MAX_PATH]uint16
+ }{replaceIfExists: 1, fileNameLength: uint32(len(destination16) - 1)}
+ if len(destination16) > len(fileRenameInfo.fileName) {
+ deleteIt()
+ return windows.ERROR_BUFFER_OVERFLOW
+ }
+ copy(fileRenameInfo.fileName[:], destination16[:])
+ err = windows.SetFileInformationByHandle(handle, windows.FileRenameInfo, (*byte)(unsafe.Pointer(fileRenameInfo)), uint32(unsafe.Sizeof(*fileRenameInfo)))
+ if err != nil {
+ deleteIt()
+ return err
+ }
+ return nil
+} \ No newline at end of file
diff --git a/conf/migration_windows.go b/conf/migration_windows.go
index 643d05c9..d2f0c765 100644
--- a/conf/migration_windows.go
+++ b/conf/migration_windows.go
@@ -9,7 +9,6 @@ import (
"fmt"
"io/ioutil"
"log"
- "os"
"path/filepath"
"regexp"
"strings"
@@ -57,17 +56,10 @@ func maybeMigrateConfiguration(c string) {
}
newPath := filepath.Join(c, fileName)
- newFile, err := os.OpenFile(newPath, os.O_EXCL|os.O_CREATE|os.O_WRONLY, 0600)
+ err = writeEncryptedFile(newPath, oldConfig)
if err != nil {
continue
}
- _, err = newFile.Write(oldConfig)
- if err != nil {
- newFile.Close()
- os.Remove(newPath)
- continue
- }
- newFile.Close()
oldPath16, err := windows.UTF16PtrFromString(oldPath)
if err == nil {
windows.MoveFileEx(oldPath16, nil, windows.MOVEFILE_DELAY_UNTIL_REBOOT)
diff --git a/conf/path_windows.go b/conf/path_windows.go
index 526aeba0..b328364b 100644
--- a/conf/path_windows.go
+++ b/conf/path_windows.go
@@ -102,7 +102,7 @@ func RootDirectory(create bool) (string, error) {
return "", err
}
- dataDirectorySd, err := windows.SecurityDescriptorFromString("O:SYG:SYD:PAI(A;OICI;FA;;;SY)(A;OICI;FR;;;BA)")
+ dataDirectorySd, err := windows.SecurityDescriptorFromString("O:SYG:SYD:PAI(A;OICI;FA;;;SY)(A;OICI;FA;;;BA)")
if err != nil {
return "", err
}
diff --git a/conf/store.go b/conf/store.go
index 21bd3a22..e79e24b8 100644
--- a/conf/store.go
+++ b/conf/store.go
@@ -103,7 +103,7 @@ func MigrateUnencryptedConfigs() (int, []error) {
e++
continue
}
- err = ioutil.WriteFile(dstFile, bytes, 0600)
+ err = writeEncryptedFile(dstFile, bytes)
if err != nil {
errs[e] = err
e++
@@ -185,16 +185,7 @@ func (config *Config) Save() error {
if err != nil {
return err
}
- err = ioutil.WriteFile(filename+".tmp", bytes, 0600)
- if err != nil {
- return err
- }
- err = os.Rename(filename+".tmp", filename)
- if err != nil {
- os.Remove(filename + ".tmp")
- return err
- }
- return nil
+ return writeEncryptedFile(filename, bytes)
}
func (config *Config) Path() (string, error) {