service: run UI at high integrity

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>

4 files changed, 35 insertions, 0 deletions

diff --git a/attacksurface.md b/attacksurface.md
index 4a4ab37a..7eefb677 100644
--- a/attacksurface.md
+++ b/attacksurface.md
@@ -37,6 +37,7 @@ The UI is an unprivileged process running as the ordinary user for each user who
 
   - The manager service (above) calls `GetSecurityInfo(ATTRIBUTE_SECURITY_INFORMATION|LABEL_SECURITY_INFORMATION|SCOPE_SECURITY_INFORMATION|OWNER_SECURITY_INFORMATION|GROUP_SECURITY_INFORMATION|DACL_SECURITY_INFORMATION)` on itself. Then it examines all of the groups associated with the users token, and finds the first one that is of type `SE_GROUP_LOGON_ID`. It adds to the prior DACL one for this SID for only `PROCESS_QUERY_LIMITED_INFORMATION` permission. Then it passes the returned security attributes as the UI's process and thread attributes in the call to `CreateProcessAsUser`. This means that debugging the UI process (in order for another process to steal its handles and exfiltrate private keys) is only possible by processes that can debug the manager service.
   - Right now, we're removing the SACL/integrity level from the above, which means the UI process only requires medium integrity to access it. Requiring greater integrity level access prevents the process from running properly, unfortunately (`ERROR_PRIVILEGE_NOT_HELD`). It would be nice to require high integrity to open the process, without having to increase the privileges that the process has in its own token.
+    - Actually, for the time being, we're giving the UI a high integrity token. This is a bummer but seems unavoidable.
   - Perhaps due to the above and other reasons, it appears that it is possible for other processes to write into the UI process's message loop, which is bad, and might defeat the purpose of the above. On the other hand, the permissions of the above are fairly strict (`O:BAG:SYD:(A;;0x1fffff;;;SY)(A;;0x121411;;;BA)(A;;0x1000;;;S-logonsid)`).
   - It renders highlighted config files to a msftedit.dll control, which typically is capable of all sorts of OLE and RTF nastiness that we make some attempt to avoid.
 
diff --git a/service/securityapi.go b/service/securityapi.go
index bf90625f..cf2e597a 100644
--- a/service/securityapi.go
+++ b/service/securityapi.go
@@ -111,6 +111,7 @@ type ACE_HEADER struct {
 //sys initializeAcl(acl *byte, len uint32, revision uint32) (err error) = advapi32.InitializeAcl
 //sys makeAbsoluteSd(selfRelativeSecurityDescriptor uintptr, absoluteSecurityDescriptor *byte, absoluteSecurityDescriptorSize *uint32, dacl *byte, daclSize *uint32, sacl *byte, saclSize *uint32, owner *byte, ownerSize *uint32, primaryGroup *byte, primaryGroupSize *uint32) (err error) = advapi32.MakeAbsoluteSD
 //sys makeSelfRelativeSd(absoluteSecurityDescriptor *byte, relativeSecurityDescriptor *byte, relativeSecurityDescriptorSize *uint32) (err error) = advapi32.MakeSelfRelativeSD
+//sys setTokenInformation(token windows.Token, infoClass uint32, info *byte, infoSize uint32) (err error) = advapi32.SetTokenInformation
 
 //sys createEnvironmentBlock(block *uintptr, token windows.Token, inheritExisting bool) (err error) = userenv.CreateEnvironmentBlock
 //sys destroyEnvironmentBlock(block uintptr) (err error) = userenv.DestroyEnvironmentBlock
@@ -296,3 +297,18 @@ func getSecurityAttributes(mainToken windows.Token, tokenThatHasLogonSession win
 
 	return relativeSecurityDescriptor, nil
 }
+
+func addElevatedIntegrityToUserToken(elevatedToken, userToken windows.Token) error {
+	//TODO: We really don't want to be doing this. See the note above. We'd rather the UI process have very few permissions in its token, and do everything with its SACL. But we can't.
+	var integrityLevel [0x2000]byte
+	len := uint32(len(integrityLevel))
+	err := windows.GetTokenInformation(elevatedToken, windows.TokenIntegrityLevel, &integrityLevel[0], len, &len)
+	if err != nil {
+		return err
+	}
+	err = setTokenInformation(userToken, windows.TokenIntegrityLevel, &integrityLevel[0], len)
+	if err != nil {
+		return err
+	}
+	return nil
+}
diff --git a/service/service_manager.go b/service/service_manager.go
index 109eeb8c..c1416c49 100644
--- a/service/service_manager.go
+++ b/service/service_manager.go
@@ -132,6 +132,11 @@ func (service *managerService) Execute(args []string, r <-chan svc.ChangeRequest
 			log.Printf("Unable to extract security attributes from manager token and combine them with SID from user token: %v", err)
 			return
 		}
+		err = addElevatedIntegrityToUserToken(userTokenInfo.elevatedToken, userToken)
+		if err != nil {
+			log.Printf("Unable to copy integrity level from elevated token to user token")
+			return
+		}
 		first := true
 		for {
 			if stoppingManager {
diff --git a/service/zsyscall_windows.go b/service/zsyscall_windows.go
index 38983370..9819b1f2 100644
--- a/service/zsyscall_windows.go
+++ b/service/zsyscall_windows.go
@@ -55,6 +55,7 @@ var (
 	procInitializeAcl              = modadvapi32.NewProc("InitializeAcl")
 	procMakeAbsoluteSD             = modadvapi32.NewProc("MakeAbsoluteSD")
 	procMakeSelfRelativeSD         = modadvapi32.NewProc("MakeSelfRelativeSD")
+	procSetTokenInformation        = modadvapi32.NewProc("SetTokenInformation")
 	procCreateEnvironmentBlock     = moduserenv.NewProc("CreateEnvironmentBlock")
 	procDestroyEnvironmentBlock    = moduserenv.NewProc("DestroyEnvironmentBlock")
 	procNotifyServiceStatusChangeW = modadvapi32.NewProc("NotifyServiceStatusChangeW")
@@ -234,6 +235,18 @@ func makeSelfRelativeSd(absoluteSecurityDescriptor *byte, relativeSecurityDescri
 	return
 }
 
+func setTokenInformation(token windows.Token, infoClass uint32, info *byte, infoSize uint32) (err error) {
+	r1, _, e1 := syscall.Syscall6(procSetTokenInformation.Addr(), 4, uintptr(token), uintptr(infoClass), uintptr(unsafe.Pointer(info)), uintptr(infoSize), 0, 0)
+	if r1 == 0 {
+		if e1 != 0 {
+			err = errnoErr(e1)
+		} else {
+			err = syscall.EINVAL
+		}
+	}
+	return
+}
+
 func createEnvironmentBlock(block *uintptr, token windows.Token, inheritExisting bool) (err error) {
 	var _p0 uint32
 	if inheritExisting {