fix: harden generated Windows wrapper ACLs

This commit is contained in:
2026-09-06 14:04:51 +00:00
parent 4e75524efa
commit 09e8a9df9e
2 changed files with 60 additions and 7 deletions
@@ -252,7 +252,7 @@ func (process *execProcess) startReaders(maxChunk uint64, remove func()) {
}() }()
} }
func materializeWrapper(directory string, wrapper Wrapper, now time.Time) (string, func(), error) { func materializeWrapper(directory string, wrapper Wrapper, now time.Time, secure func(string) error) (string, func(), error) {
if directory == "" { if directory == "" {
return "", nil, ErrInvalidWorkingDirectory return "", nil, ErrInvalidWorkingDirectory
} }
@@ -272,6 +272,12 @@ func materializeWrapper(directory string, wrapper Wrapper, now time.Time) (strin
cleanup() cleanup()
return "", nil, err return "", nil, err
} }
if secure != nil {
if err := secure(temporaryName); err != nil {
cleanup()
return "", nil, err
}
}
if _, err := temporary.Write(wrapper.Bytes); err != nil { if _, err := temporary.Write(wrapper.Bytes); err != nil {
cleanup() cleanup()
return "", nil, err return "", nil, err
@@ -379,7 +385,7 @@ func (manager *execSupervisor) startPortable(ctx context.Context, spec superviso
if err != nil { if err != nil {
return nil, err return nil, err
} }
wrapperPath, cleanup, err = materializeWrapper(spec.WorkingDirectory, wrapper, manager.options.Now()) wrapperPath, cleanup, err = materializeWrapper(spec.WorkingDirectory, wrapper, manager.options.Now(), nil)
if err != nil { if err != nil {
return nil, err return nil, err
} }
@@ -87,10 +87,7 @@ func (manager *execSupervisor) Start(ctx context.Context, spec supervisor.StartS
if err != nil { if err != nil {
return nil, err return nil, err
} }
wrapperPath, cleanup, err := materializeWrapper(spec.WorkingDirectory, wrapper, manager.options.Now()) var cleanup func()
if err != nil {
return nil, err
}
fail := func(cause error) (supervisor.Process, error) { fail := func(cause error) (supervisor.Process, error) {
if cleanup != nil { if cleanup != nil {
cleanup() cleanup()
@@ -103,6 +100,12 @@ func (manager *execSupervisor) Start(ctx context.Context, spec supervisor.StartS
return fail(err) return fail(err)
} }
defer token.Close() defer token.Close()
wrapperPath, cleanup, err := materializeWrapper(spec.WorkingDirectory, wrapper, manager.options.Now(), func(path string) error {
return secureWrapperFile(path, identity.UserSID)
})
if err != nil {
return fail(err)
}
baseEnvironment, err := token.Environ(false) baseEnvironment, err := token.Environ(false)
if err != nil { if err != nil {
return fail(fmt.Errorf("build token environment: %w", err)) return fail(fmt.Errorf("build token environment: %w", err))
@@ -264,6 +267,50 @@ func verifyExecutable(path string) error {
return nil return nil
} }
// secureWrapperFile replaces the inherited directory ACL with a protected
// DACL. The service writes the wrapper before this call; afterward only
// LocalSystem and the selected effective token SID can read it. This is done
// after token selection so active-user commands do not depend on inherited
// broad ProgramData permissions.
func secureWrapperFile(path, effectiveSID string) error {
systemSID, err := winapi.StringToSid(securitySystemRID)
if err != nil {
return err
}
effective := systemSID
if effectiveSID != "" && effectiveSID != securitySystemRID {
effective, err = winapi.StringToSid(effectiveSID)
if err != nil {
return err
}
}
entries := []winapi.EXPLICIT_ACCESS{{
AccessPermissions: winapi.GENERIC_ALL,
AccessMode: winapi.SET_ACCESS,
Trustee: winapi.TRUSTEE{
TrusteeForm: winapi.TRUSTEE_IS_SID,
TrusteeType: winapi.TRUSTEE_IS_WELL_KNOWN_GROUP,
TrusteeValue: winapi.TrusteeValueFromSID(systemSID),
},
}}
if effective != systemSID {
entries = append(entries, winapi.EXPLICIT_ACCESS{
AccessPermissions: winapi.GENERIC_READ,
AccessMode: winapi.SET_ACCESS,
Trustee: winapi.TRUSTEE{
TrusteeForm: winapi.TRUSTEE_IS_SID,
TrusteeType: winapi.TRUSTEE_IS_USER,
TrusteeValue: winapi.TrusteeValueFromSID(effective),
},
})
}
acl, err := winapi.ACLFromEntries(entries, nil)
if err != nil {
return err
}
return winapi.SetNamedSecurityInfo(path, winapi.SE_FILE_OBJECT, winapi.OWNER_SECURITY_INFORMATION|winapi.DACL_SECURITY_INFORMATION|winapi.PROTECTED_DACL_SECURITY_INFORMATION, systemSID, nil, acl, nil)
}
func createStandardPipes() (*os.File, *os.File, *os.File, *os.File, *os.File, *os.File, error) { func createStandardPipes() (*os.File, *os.File, *os.File, *os.File, *os.File, *os.File, error) {
security := &winapi.SecurityAttributes{Length: uint32(unsafe.Sizeof(winapi.SecurityAttributes{})), InheritHandle: 1} security := &winapi.SecurityAttributes{Length: uint32(unsafe.Sizeof(winapi.SecurityAttributes{})), InheritHandle: 1}
var stdinReadHandle, stdinWriteHandle winapi.Handle var stdinReadHandle, stdinWriteHandle winapi.Handle
@@ -559,7 +606,7 @@ func openTokenForAttempt(contextName ExecutionContext, candidate *SessionCandida
if err != nil { if err != nil {
return 0, supervisor.EffectiveIdentity{}, err return 0, supervisor.EffectiveIdentity{}, err
} }
return token, supervisor.EffectiveIdentity{Context: string(ContextLocalService), Elevated: false, Integrity: "medium"}, nil return token, supervisor.EffectiveIdentity{Context: string(ContextLocalService), UserSID: securityLocalServiceRID, Elevated: false, Integrity: "medium"}, nil
case ContextLocalSystem: case ContextLocalSystem:
return duplicateServiceToken() return duplicateServiceToken()
default: default: