Skip to content

Commit

Permalink
deny default write permissions from windows root directory (#1962)
Browse files Browse the repository at this point in the history
  • Loading branch information
zackattack01 authored Nov 19, 2024
1 parent caa9ede commit 7d72304
Show file tree
Hide file tree
Showing 2 changed files with 169 additions and 0 deletions.
132 changes: 132 additions & 0 deletions cmd/launcher/svc_config_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,14 @@ import (
"context"
"fmt"
"log/slog"
"strings"
"time"
"unsafe"

"github.com/kolide/kit/version"
"github.com/kolide/launcher/pkg/launcher"

"golang.org/x/sys/windows"
"golang.org/x/sys/windows/registry"
"golang.org/x/sys/windows/svc/mgr"
)
Expand All @@ -33,6 +36,10 @@ const (
// we add or update the currentVersionKeyName alongside the existing keys from installation
currentVersionRegistryKeyFmt = `Software\Kolide\Launcher\%s\%s`
currentVersionKeyName = `CurrentVersionNum`

// these are the flag values for the actual "write" ACLs that we see through Get-Acl in powershell. they are not exposed as external constants
// github.com/Microsoft/go-winio/internal/fs.FILE_WRITE_DATA|github.com/Microsoft/go-winio/internal/fs.FILE_CREATE_PIPE_INSTANCE|github.com/Microsoft/go-winio/internal/fs.FILE_WRITE_PROPERTIES|github.com/Microsoft/go-winio/internal/fs.FILE_WRITE_ATTRIBUTES (278) = 0x116
accessPermissionsAllWrites = 0x116
)

func checkServiceConfiguration(logger *slog.Logger, opts *launcher.Options) {
Expand Down Expand Up @@ -103,6 +110,8 @@ func checkServiceConfiguration(logger *slog.Logger, opts *launcher.Options) {
checkRecoveryActions(context.TODO(), logger, launcherService)

checkCurrentVersionMetadata(logger, opts.Identifier)

checkRootDirACLs(logger, opts.RootDirectory)
}

// checkDelayedAutostart checks the current value of `DelayedAutostart` (whether to wait ~2 minutes
Expand Down Expand Up @@ -318,3 +327,126 @@ func checkCurrentVersionMetadata(logger *slog.Logger, identifier string) {
"previous_registry_version", currentVersionVal,
)
}

// checkRootDirACLs verifies that there is an explicit denial for builtin/users write permissions
// set on the root directory. If none exists, a new one is created and added to the existing
// security configuration for the directory. errors are logged but not retried, as we will attempt this
// on every launcher startup
func checkRootDirACLs(logger *slog.Logger, rootDirectory string) {
if strings.TrimSpace(rootDirectory) == "" {
logger.Log(context.TODO(), slog.LevelError,
"unable to check directory permissions without root dir set, skipping",
"root_dir", rootDirectory,
)

return
}

// Get the current security descriptor for the directory
sd, err := windows.GetNamedSecurityInfo(
rootDirectory,
windows.SE_FILE_OBJECT,
windows.DACL_SECURITY_INFORMATION,
)

if err != nil {
logger.Log(context.TODO(), slog.LevelError,
"gathering existing ACL from named sec info",
"err", err,
)

return
}

existingDACL, _, err := sd.DACL()
if err != nil {
logger.Log(context.TODO(), slog.LevelError,
"getting DACL from security descriptor",
"err", err,
)

return
}

usersSID, err := windows.CreateWellKnownSid(windows.WinBuiltinUsersSid)
if err != nil {
logger.Log(context.TODO(), slog.LevelError,
"failed getting builtin users SID",
"err", err,
)

return
}

// first iterate the existing ACEs for the directory, we're checking to see
// if there is already a DENY entry set for user's group to avoid recreating every time
for i := 0; i < int(existingDACL.AceCount); i++ {
var ace *windows.ACCESS_ALLOWED_ACE
if aceErr := windows.GetAce(existingDACL, uint32(i), &ace); aceErr != nil {
logger.Log(context.TODO(), slog.LevelWarn,
"encountered error parsing ACE from existing DACL",
"err", aceErr,
)

return
}

// do the easy checks first and continue if this isn't the ACE we're looking for
if ace.Mask != accessPermissionsAllWrites || ace.Header.AceType != windows.ACCESS_DENIED_ACE_TYPE {
continue
}

sid := (*windows.SID)(unsafe.Pointer(uintptr(unsafe.Pointer(ace)) + unsafe.Offsetof(ace.SidStart)))
if sid.Equals(usersSID) {
logger.Log(context.TODO(), slog.LevelDebug,
"root directory already had proper DACL permissions set, skipping",
)

return
}
}

explicitAccessPolicies := []windows.EXPLICIT_ACCESS{
{
AccessPermissions: accessPermissionsAllWrites, // deny writes
AccessMode: windows.DENY_ACCESS,
Inheritance: windows.SUB_CONTAINERS_AND_OBJECTS_INHERIT, // ensure denial is inherited by sub folders
Trustee: windows.TRUSTEE{
TrusteeForm: windows.TRUSTEE_IS_SID,
TrusteeType: windows.TRUSTEE_IS_GROUP,
TrusteeValue: windows.TrusteeValueFromSID(usersSID),
},
},
}

// merge our existing DACL with our new explicit denial entry
newDACL, err := windows.ACLFromEntries(explicitAccessPolicies, existingDACL)
if err != nil {
logger.Log(context.TODO(), slog.LevelError,
"generating new DACL from access entries",
"err", err,
)

return
}

// apply the merged DACL to the root directory
err = windows.SetNamedSecurityInfo(
rootDirectory,
windows.SE_FILE_OBJECT,
// PROTECTED_DACL_SECURITY_INFORMATION here ensures we don't re-inherit the parent permissions
windows.DACL_SECURITY_INFORMATION|windows.PROTECTED_DACL_SECURITY_INFORMATION,
nil, nil, newDACL, nil,
)

if err != nil {
logger.Log(context.TODO(), slog.LevelError,
"setting named security info from new DACL",
"err", err,
)

return
}

logger.Log(context.TODO(), slog.LevelInfo, "updated ACLs for root directory")
}
37 changes: 37 additions & 0 deletions cmd/launcher/svc_config_windows_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
//go:build windows
// +build windows

package main

import (
"io"
"log/slog"
"testing"

"github.com/kolide/launcher/pkg/threadsafebuffer"
"github.com/stretchr/testify/require"
)

func Test_checkRootDirACLs(t *testing.T) {
t.Parallel()

rootDir := t.TempDir()
var logBytes threadsafebuffer.ThreadSafeBuffer

slogger := slog.New(slog.NewTextHandler(&logBytes, &slog.HandlerOptions{
Level: slog.LevelDebug,
}))

// run the check once, expecting that we will correctly work all the way through
// and log that we've updated the ACLs for our new directory
checkRootDirACLs(slogger, rootDir)
require.Contains(t, logBytes.String(), "updated ACLs for root directory")

// now clear the log, and rerun. if the previous run did what it was supposed to,
// and our check-before-write logic works correctly, we should detect the ACL we
// just added and exit early
io.Copy(io.Discard, &logBytes)
checkRootDirACLs(slogger, rootDir)
require.NotContains(t, logBytes.String(), "updated ACLs for root directory")
require.Contains(t, logBytes.String(), "root directory already had proper DACL permissions set, skipping")
}

0 comments on commit 7d72304

Please sign in to comment.