Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .github/workflows/v2.yml
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ jobs:
# missing GOCOVERDIR warning would contaminate psql probe output.
go build -o $env:EP_SUPERVISOR ./cmd/embedded-postgres
if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE }
go test -count=50 -timeout=2m -run '^TestWindowsNativeFailuresReleaseHandles$' ./internal/platform
if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE }
go test -v -count=1 -p=1 -timeout=4m -run '^(TestWindowsRestrictedChildCanOpenItsToken|TestWindowsNativeFailuresReleaseHandles|TestWindowsUserDACLRejectsInvalidToken|TestParallelLifecycle|TestLifetimeCancellation|TestPersistentDataNeverErased|TestCLIParentPipe|TestCLIBackground|TestCLIExec|TestWindowsExecCancellation)$' . ./internal/platform ./internal/cli
if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE }
}
Expand Down
13 changes: 7 additions & 6 deletions internal/platform/process_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,13 @@ type windowsCalls struct {
sid func(string) (*syscall.SID, error)
restrict func(syscall.Token, []syscall.SIDAndAttributes, *syscall.Token) (uintptr, error)
secure func(syscall.Token) error
close func(syscall.Handle) error
create func() (uintptr, error)
limit func(uintptr, *extendedLimits) (uintptr, error)
assign func(uintptr, syscall.Handle) (uintptr, error)
}

var nativeWindows = windowsCalls{syscall.OpenProcessToken, syscall.StringToSid, nativeRestrict, nativeUserDACL, nativeCreate, nativeLimit, nativeAssign}
var nativeWindows = windowsCalls{syscall.OpenProcessToken, syscall.StringToSid, nativeRestrict, nativeUserDACL, syscall.CloseHandle, nativeCreate, nativeLimit, nativeAssign}

// A token created by an elevated PowerShell process can inherit an
// administrators-only DACL. Once that SID is disabled, the child cannot reopen
Expand Down Expand Up @@ -121,7 +122,7 @@ func configureWindows(cmd *exec.Cmd, id *Identity, calls windowsCalls) (func(),
if err = calls.openToken(current, syscall.TOKEN_DUPLICATE|syscall.TOKEN_ASSIGN_PRIMARY|syscall.TOKEN_QUERY|syscall.TOKEN_ADJUST_DEFAULT|writeDACL, &original); err != nil {
return nil, err
}
defer original.Close()
defer calls.close(syscall.Handle(original))
admin, err := calls.sid("S-1-5-32-544")
if err != nil {
return nil, err
Expand All @@ -137,11 +138,11 @@ func configureWindows(cmd *exec.Cmd, id *Identity, calls windowsCalls) (func(),
return nil, fmt.Errorf("create restricted token: %w", callErr)
}
if err := calls.secure(token); err != nil {
token.Close()
calls.close(syscall.Handle(token))
return nil, fmt.Errorf("secure restricted token: %w", err)
}
cmd.SysProcAttr = &syscall.SysProcAttr{Token: token, CreationFlags: syscall.CREATE_NEW_PROCESS_GROUP, HideWindow: true}
return func() { _ = token.Close() }, nil
return func() { _ = calls.close(syscall.Handle(token)) }, nil
}

func Kill(p *os.Process) error {
Expand Down Expand Up @@ -180,13 +181,13 @@ func guardWindows(calls windowsCalls) error {
limits.Basic.LimitFlags = 0x2000
ok, err := calls.limit(job, &limits)
if ok == 0 {
syscall.CloseHandle(syscall.Handle(job))
calls.close(syscall.Handle(job))
return fmt.Errorf("configure job: %w", err)
}
current, _ := syscall.GetCurrentProcess()
ok, err = calls.assign(job, current)
if ok == 0 {
syscall.CloseHandle(syscall.Handle(job))
calls.close(syscall.Handle(job))
return fmt.Errorf("assign job: %w", err)
}
return nil
Expand Down
80 changes: 67 additions & 13 deletions internal/platform/process_windows_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,19 +56,64 @@ func TestWindowsUserDACLRejectsInvalidToken(t *testing.T) {
func TestWindowsNativeFailuresReleaseHandles(t *testing.T) {
failure := syscall.ERROR_ACCESS_DENIED

handleCount := func() uint32 {
t.Helper()
current, _ := syscall.GetCurrentProcess()
var count uint32
ok, _, e := kernel.NewProc("GetProcessHandleCount").Call(uintptr(current), uintptr(unsafe.Pointer(&count)))
if ok == 0 {
t.Fatal(e)
// Process-wide handle counts include runtime threads and unrelated activity.
// Audit the exact resources acquired by this call instead, and require each
// native CloseHandle to succeed. No unrelated-handle allowance is needed.
audit := func(calls windowsCalls) (windowsCalls, func()) {
live := make(map[syscall.Handle]bool)
track := func(handle syscall.Handle) {
t.Helper()
if live[handle] {
t.Fatal("handle acquired twice without closing", handle)
}
live[handle] = true
}
open := calls.openToken
calls.openToken = func(process syscall.Handle, access uint32, token *syscall.Token) error {
err := open(process, access, token)
if err == nil {
track(syscall.Handle(*token))
}
return err
}
restrict := calls.restrict
calls.restrict = func(original syscall.Token, denied []syscall.SIDAndAttributes, token *syscall.Token) (uintptr, error) {
ok, err := restrict(original, denied, token)
if ok != 0 {
track(syscall.Handle(*token))
}
return ok, err
}
create := calls.create
calls.create = func() (uintptr, error) {
handle, err := create()
if handle != 0 {
track(syscall.Handle(handle))
}
return handle, err
}
closeHandle := calls.close
calls.close = func(handle syscall.Handle) error {
t.Helper()
if !live[handle] {
t.Fatal("closing unowned or already closed handle", handle)
}
if err := closeHandle(handle); err != nil {
t.Fatal("native handle close failed", handle, err)
}
delete(live, handle)
return nil
}
return calls, func() {
t.Helper()
if len(live) != 0 {
t.Fatal("native handles left open", live)
}
}
return count
}
before := handleCount()

for range 3 {
for _, kind := range []string{"open", "admin-sid", "power-sid", "restrict", "secure"} {
for _, kind := range []string{"open", "admin-sid", "power-sid", "restrict", "secure", "success"} {
calls := nativeWindows
switch kind {
case "open":
Expand All @@ -85,11 +130,21 @@ func TestWindowsNativeFailuresReleaseHandles(t *testing.T) {
case "secure":
calls.secure = func(syscall.Token) error { return failure }
}
calls, assertClosed := audit(calls)
release, e := configureWindows(exec.Command("unused"), nil, calls)
if kind == "success" {
if e != nil {
t.Fatal("native token creation failed", e)
}
release()
assertClosed()
continue
}
if e == nil {
release()
t.Fatal("native token failure ignored", kind)
}
assertClosed()
if !errors.Is(e, failure) {
t.Fatal("native failure cause lost", kind, e)
}
Expand All @@ -104,13 +159,12 @@ func TestWindowsNativeFailuresReleaseHandles(t *testing.T) {
case "assign":
calls.assign = func(uintptr, syscall.Handle) (uintptr, error) { return 0, failure }
}
calls, assertClosed := audit(calls)
// Every case fails before any process can be assigned to the job.
if e := guardWindows(calls); !errors.Is(e, failure) {
t.Fatal("job failure cause lost", kind, e)
}
assertClosed()
}
}
if after := handleCount(); after > before+2 {
t.Fatalf("native failure paths leaked handles: before=%d after=%d", before, after)
}
}
Loading