From 0d8e707533fada938c99eb06f8457150e7ef2b40 Mon Sep 17 00:00:00 2001 From: mptyl Date: Tue, 18 Aug 2026 11:04:21 +0200 Subject: [PATCH] fix(auth): remove Windows claims by retained handle --- tools/tht/internal/safeio/claim_windows.go | 51 +++---- .../tht/internal/safeio/claim_windows_test.go | 137 ++++++++++++++++++ 2 files changed, 154 insertions(+), 34 deletions(-) create mode 100644 tools/tht/internal/safeio/claim_windows_test.go diff --git a/tools/tht/internal/safeio/claim_windows.go b/tools/tht/internal/safeio/claim_windows.go index 6a524f48..86ec8986 100644 --- a/tools/tht/internal/safeio/claim_windows.go +++ b/tools/tht/internal/safeio/claim_windows.go @@ -123,41 +123,24 @@ func readCanonicalPrivateClaim(source, claim string, maximum int64) ([]byte, boo return contents, true, nil } -func removeCanonicalPrivateClaim(source, claim string) (bool, error) { - sourceFile, err := openWindowsPrivateRegular(source, 2) - if err != nil { - if isWindowsNotFound(err) && windowsPrivateClaimAbsentOrOrphan(claim) { - return false, nil +func removeCanonicalPrivateClaim(source, claim string) (removed bool, resultErr error) { + parentPath := filepath.Dir(source) + sourceName := filepath.Base(source) + claimName := filepath.Base(claim) + if parentPath != filepath.Dir(claim) || !validPrivateLeafName(sourceName) || !validPrivateLeafName(claimName) { + return false, ErrUnsafeFile + } + directory, found, err := OpenPrivateDirectory(parentPath, false) + if err != nil || !found { + return false, ErrUnsafeFile + } + defer func() { + if closeErr := directory.Close(); closeErr != nil && resultErr == nil { + resultErr = ErrUnsafeFile } - return false, ErrUnsafeFile - } - claimFile, err := openWindowsPrivateRegular(claim, 2) - if err != nil { - sourceFile.Close() - if isWindowsNotFound(err) { - return false, nil - } - return false, ErrUnsafeFile - } - if !sameWindowsPrivateFile(sourceFile.info, claimFile.info) { - sourceFile.Close() - claimFile.Close() - return false, ErrUnsafeFile - } - sourceFile.Close() - claimFile.Close() - if err := windows.DeleteFile(windows.StringToUTF16Ptr(source)); err != nil { - return false, ErrUnsafeFile - } - remaining, err := openWindowsPrivateRegular(claim, 1) - if err != nil { - return false, ErrUnsafeFile - } - remaining.Close() - if err := windows.DeleteFile(windows.StringToUTF16Ptr(claim)); err != nil { - return false, ErrUnsafeFile - } - return true, nil + }() + NotifyPrivateDirectoryTestHookForTest("after-canonical-private-claim-parent-open") + return directory.RemoveClaim(sourceName, claimName) } func openWindowsPrivateRegularWithAllowedLinks(path string, allowed ...uint32) (*windowsPrivateRegular, error) { diff --git a/tools/tht/internal/safeio/claim_windows_test.go b/tools/tht/internal/safeio/claim_windows_test.go new file mode 100644 index 00000000..4a30cc0f --- /dev/null +++ b/tools/tht/internal/safeio/claim_windows_test.go @@ -0,0 +1,137 @@ +//go:build windows + +package safeio + +import ( + "errors" + "os" + "path/filepath" + "testing" +) + +func createWindowsPrivateTestFile(t *testing.T, path string, contents []byte) { + t.Helper() + file, err := CreateCanonicalNewPrivateFile(path) + if err != nil { + t.Fatal(err) + } + if _, err := file.Write(contents); err != nil { + _ = file.Close() + t.Fatal(err) + } + if err := file.Close(); err != nil { + t.Fatal(err) + } +} + +func TestRemoveCanonicalPrivateClaimRetainsParentDuringDeletion(t *testing.T) { + parent := filepath.Join(t.TempDir(), "claims") + if err := os.Mkdir(parent, 0o700); err != nil { + t.Fatal(err) + } + if err := ProtectPrivateDirectory(parent); err != nil { + t.Fatal(err) + } + source := filepath.Join(parent, "state.json") + claim := filepath.Join(parent, "state.claim") + createWindowsPrivateTestFile(t, source, []byte("state")) + if claimed, err := ClaimCanonicalPrivateRegular(source, claim); err != nil || !claimed { + t.Fatalf("ClaimCanonicalPrivateRegular() = claimed %v, err %v", claimed, err) + } + + outside := filepath.Join(t.TempDir(), "outside") + if err := os.Mkdir(outside, 0o700); err != nil { + t.Fatal(err) + } + if err := ProtectPrivateDirectory(outside); err != nil { + t.Fatal(err) + } + sentinel := filepath.Join(outside, "sentinel") + createWindowsPrivateTestFile(t, sentinel, []byte("outside-safe")) + attemptedSwap := false + restoreHook := SetPrivateDirectoryTestHookForTest(func(stage string) { + if stage != "after-canonical-private-claim-parent-open" || attemptedSwap { + return + } + attemptedSwap = true + if err := os.Rename(parent, parent+"-moved"); err == nil { + t.Fatal("claim parent rename succeeded while removal retained its handle") + } + }) + defer restoreHook() + + removed, err := RemoveCanonicalPrivateClaim(source, claim) + if err != nil || !removed || !attemptedSwap { + t.Fatalf("RemoveCanonicalPrivateClaim() = removed %v, attempted %v, err %v", removed, attemptedSwap, err) + } + for _, path := range []string{source, claim} { + if _, err := os.Stat(path); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("removed path %q still exists: %v", filepath.Base(path), err) + } + } + if contents, err := os.ReadFile(sentinel); err != nil || string(contents) != "outside-safe" { + t.Fatalf("outside sentinel = %q, err %v", contents, err) + } +} + +func TestRemoveCanonicalPrivateClaimPreservesOrphan(t *testing.T) { + parent := filepath.Join(t.TempDir(), "claims") + if err := os.Mkdir(parent, 0o700); err != nil { + t.Fatal(err) + } + if err := ProtectPrivateDirectory(parent); err != nil { + t.Fatal(err) + } + source := filepath.Join(parent, "state.json") + claim := filepath.Join(parent, "state.claim") + createWindowsPrivateTestFile(t, claim, []byte("orphan")) + + removed, err := RemoveCanonicalPrivateClaim(source, claim) + if removed || err != nil { + t.Fatalf("RemoveCanonicalPrivateClaim() = removed %v, err %v, want false/nil", removed, err) + } + if _, err := os.Stat(source); !errors.Is(err, os.ErrNotExist) { + t.Fatalf("orphan source unexpectedly exists: %v", err) + } + if contents, err := os.ReadFile(claim); err != nil || string(contents) != "orphan" { + t.Fatalf("orphan claim = %q, err %v", contents, err) + } +} + +func TestRemoveCanonicalPrivateClaimRejectsMismatchedTwoLinkFiles(t *testing.T) { + parent := filepath.Join(t.TempDir(), "claims") + if err := os.Mkdir(parent, 0o700); err != nil { + t.Fatal(err) + } + if err := ProtectPrivateDirectory(parent); err != nil { + t.Fatal(err) + } + source := filepath.Join(parent, "state.json") + claim := filepath.Join(parent, "state.claim") + createWindowsPrivateTestFile(t, source, []byte("source")) + createWindowsPrivateTestFile(t, claim, []byte("claim")) + sourceAuxiliary := filepath.Join(parent, "state.json.aux") + if err := os.Link(source, sourceAuxiliary); err != nil { + t.Fatal(err) + } + claimAuxiliary := filepath.Join(parent, "state.claim.aux") + if err := os.Link(claim, claimAuxiliary); err != nil { + t.Fatal(err) + } + + removed, err := RemoveCanonicalPrivateClaim(source, claim) + if removed || !errors.Is(err, ErrUnsafeFile) { + t.Fatalf("RemoveCanonicalPrivateClaim() = removed %v, err %v, want false/ErrUnsafeFile", removed, err) + } + for path, want := range map[string]string{ + source: "source", + claim: "claim", + sourceAuxiliary: "source", + claimAuxiliary: "claim", + } { + contents, readErr := os.ReadFile(path) + if readErr != nil || string(contents) != want { + t.Fatalf("mismatched pair path %q = %q, err %v", filepath.Base(path), contents, readErr) + } + } +}