mptyl/ThothII
1
0
Fork 0
Code Issues 25 Pull Requests Actions Packages Projects Releases 1 Wiki Activity

fix(auth): close diagnostic filesystem races

Browse Source
This commit is contained in:
marcopan
2026-08-17 13:04:15 +02:00
parent be1724890a
commit 6d438f4c7e
16 changed files with 545 additions and 90 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tools/tht/internal/authstorage/storage.go
+66 -2
View File
@@ -60,6 +60,7 @@ type response struct {
Entries *[]safeio.PrivateDirectoryEntry `json:"entries,omitempty"`
More *bool `json:"more,omitempty"`
Validated bool `json:"validated,omitempty"`
Prepared bool `json:"prepared,omitempty"`
}
// Run accepts exactly one strict JSON request on stdin and emits exactly one JSON response on
@@ -110,11 +111,17 @@ func execute(input request) (response, error) {
return response{}, errInvalid
}
if input.Operation == "validate-root" {
if _, err := preflightRoot(input.Root); err != nil {
if err := validateStorageLayout(input.Root); err != nil {
return response{}, errInvalid
}
return response{Version: protocolVersion, OK: true, Validated: true}, nil
}
if input.Operation == "ensure-layout" {
if err := ensureStorageLayout(input.Root); err != nil {
return response{}, errInvalid
}
return response{Version: protocolVersion, OK: true, Prepared: true}, nil
}
if input.Operation == "read-auth-config" {
root, err := existingPrivateRoot(input.Root)
if err != nil {
@@ -225,7 +232,7 @@ func validOperationShape(input request) bool {
noAfterName := input.AfterName == ""
noContinuation := !input.Continuation
switch input.Operation {
case "validate-root":
case "validate-root", "ensure-layout":
return input.Directory == "" && input.Filename == "" && noContents && noMaximumEntries && noAfterName && noContinuation
case "read-auth-config":
return input.Directory == "" && authConfigFilename.MatchString(input.Filename) && noContents && noMaximumEntries && noAfterName && noContinuation
@@ -260,6 +267,63 @@ func existingPrivateRoot(root string) (string, error) {
return root, nil
}
func validateStorageLayout(root string) error {
exists, err := preflightRoot(root)
if err != nil {
return errInvalid
}
if !exists {
return nil
}
existingChildren := make([]string, 0, 2)
for _, directory := range []string{"sessions", "oidc"} {
path := filepath.Join(root, directory)
if filepath.Dir(path) != root {
return errInvalid
}
childExists, err := safeio.PreflightPrivateDirectory(path)
if err != nil {
return errInvalid
}
if childExists {
existingChildren = append(existingChildren, path)
}
}
// Close permission/identity races between the individual side-effect-free preflights.
if safeio.ValidatePrivateDirectory(root) != nil {
return errInvalid
}
for _, directory := range []string{"sessions", "oidc"} {
if _, err := safeio.PreflightPrivateDirectory(filepath.Join(root, directory)); err != nil {
return errInvalid
}
}
for _, path := range existingChildren {
if safeio.ValidatePrivateDirectory(path) != nil {
return errInvalid
}
}
return nil
}
func ensureStorageLayout(root string) error {
if _, err := preflightRoot(root); err != nil || safeio.EnsurePrivateDirectory(root) != nil {
return errInvalid
}
for _, directory := range []string{"sessions", "oidc"} {
path := filepath.Join(root, directory)
if filepath.Dir(path) != root || safeio.EnsurePrivateDirectory(path) != nil {
return errInvalid
}
}
if safeio.ValidatePrivateDirectory(root) != nil ||
safeio.ValidatePrivateDirectory(filepath.Join(root, "sessions")) != nil ||
safeio.ValidatePrivateDirectory(filepath.Join(root, "oidc")) != nil {
return errInvalid
}
return nil
}
func contentResponse(found bool, contents []byte) response {
if !found {
return response{Version: protocolVersion, OK: true}
tools/tht/internal/authstorage/storage_test.go
+48
View File
@@ -9,6 +9,7 @@ import (
"fmt"
"os"
"path/filepath"
"runtime"
"strings"
"sync"
"testing"
@@ -119,6 +120,53 @@ func TestProtocolPreflightsRootWithoutCreatingOrFollowingLinks(t *testing.T) {
runRejected(t, request{Version: 1, Operation: "validate-root", Root: filepath.Join(parent, "auth\n")})
}
func TestProtocolValidatesTheCompleteSessionLayoutWithoutCreatingIt(t *testing.T) {
root := filepath.Join(privateTestRoot(t), "auth")
if err := safeio.EnsurePrivateDirectory(root); err != nil {
t.Fatal(err)
}
validated := runRequest(t, request{Version: 1, Operation: "validate-root", Root: root})
if !validated.Validated {
t.Fatal("layout with safely creatable children was not validated")
}
for _, child := range []string{"sessions", "oidc"} {
if _, err := os.Lstat(filepath.Join(root, child)); !errors.Is(err, os.ErrNotExist) {
t.Fatalf("validate-root created %s: %v", child, err)
}
}
outside := privateTestRoot(t)
testsupport.SymlinkOrSkip(t, outside, filepath.Join(root, "sessions"))
runRejected(t, request{Version: 1, Operation: "validate-root", Root: root})
if entries, err := os.ReadDir(outside); err != nil || len(entries) != 0 {
t.Fatalf("linked child target was mutated: entries=%v error=%v", entries, err)
}
if err := os.Remove(filepath.Join(root, "sessions")); err != nil {
t.Fatal(err)
}
if err := os.Mkdir(filepath.Join(root, "sessions"), 0o700); err != nil {
t.Fatal(err)
}
if runtime.GOOS == "windows" {
return // Native DACL coverage lives in storage_windows_test.go.
}
if err := os.Chmod(filepath.Join(root, "sessions"), 0o750); err != nil {
t.Fatal(err)
}
runRejected(t, request{Version: 1, Operation: "validate-root", Root: root})
}
func TestProtocolEnsuresTheCompletePrivateSessionLayout(t *testing.T) {
root := filepath.Join(privateTestRoot(t), "auth")
runRequest(t, request{Version: 1, Operation: "ensure-layout", Root: root})
for _, path := range []string{root, filepath.Join(root, "sessions"), filepath.Join(root, "oidc")} {
if err := safeio.ValidatePrivateDirectory(path); err != nil {
t.Fatalf("private layout path %q: %v", filepath.Base(path), err)
}
}
runRejected(t, request{Version: 1, Operation: "ensure-layout", Root: root, Directory: "sessions"})
}
func TestProtocolReadsOnlyBoundedPrivateAuthConfig(t *testing.T) {
root := privateTestRoot(t)
filename := "auth.yaml"
tools/tht/internal/authstorage/storage_windows_test.go
+16
View File
@@ -55,6 +55,22 @@ func TestProtocolRejectsRecordCreationAfterSessionsDirectoryDACLBecomesPermissiv
runRejected(t, request{Version: 1, Operation: "create", Root: root, Directory: "sessions", Filename: filename, ContentBase64: base64.StdEncoding.EncodeToString([]byte("record"))})
}
func TestProtocolValidatesAndCreatesTheCompleteWindowsSessionLayout(t *testing.T) {
root := filepath.Join(t.TempDir(), "auth")
runRequest(t, request{Version: 1, Operation: "ensure-layout", Root: root})
runRequest(t, request{Version: 1, Operation: "validate-root", Root: root})
for _, directory := range []string{"sessions", "oidc"} {
if err := safeio.ValidatePrivateDirectory(filepath.Join(root, directory)); err != nil {
t.Fatalf("%s DACL = %v", directory, err)
}
}
if err := setPermissiveDACL(filepath.Join(root, "oidc")); err != nil {
t.Fatal(err)
}
runRejected(t, request{Version: 1, Operation: "validate-root", Root: root})
}
func setPermissiveDACL(path string) error {
world, err := windows.StringToSid("S-1-1-0")
if err != nil {
tools/tht/internal/safeio/files_unix_test.go
+52
View File
@@ -107,3 +107,55 @@ func TestReplaceCanonicalRegularRejectsSymlinkedPathComponents(t *testing.T) {
t.Fatalf("final symlink replacement error = %v, want ErrUnsafeFile", err)
}
}
func TestPrivateDirectoryCreationUsesThePinnedParentAfterAncestorSwap(t *testing.T) {
temporaryRoot, err := filepath.EvalSymlinks(os.TempDir())
if err != nil {
t.Fatal(err)
}
root, err := os.MkdirTemp(temporaryRoot, "tht-safeio-mkdirat-")
if err != nil {
t.Fatal(err)
}
t.Cleanup(func() { _ = os.RemoveAll(root) })
for _, target := range []string{"root", "sessions", "oidc"} {
t.Run(target, func(t *testing.T) {
caseRoot := filepath.Join(root, target)
parent := filepath.Join(caseRoot, "parent")
if err := os.MkdirAll(parent, 0o700); err != nil {
t.Fatal(err)
}
path := filepath.Join(parent, "auth")
swappedAncestor := parent
if target != "root" {
if err := os.Mkdir(path, 0o700); err != nil {
t.Fatal(err)
}
swappedAncestor = path
path = filepath.Join(path, target)
}
outside := filepath.Join(caseRoot, "outside")
if err := os.Mkdir(outside, 0o700); err != nil {
t.Fatal(err)
}
movedAncestor := swappedAncestor + "-original"
if err := createPrivateDirectoryAfterParentOpen(path, func() {
if err := os.Rename(swappedAncestor, movedAncestor); err != nil {
t.Fatal(err)
}
if err := os.Symlink(outside, swappedAncestor); err != nil {
t.Fatal(err)
}
}); err != nil {
t.Fatal(err)
}
if err := ValidatePrivateDirectory(filepath.Join(movedAncestor, filepath.Base(path))); err != nil {
t.Fatalf("pinned-parent creation failed: %v", err)
}
if _, err := os.Lstat(filepath.Join(outside, filepath.Base(path))); !errors.Is(err, os.ErrNotExist) {
t.Fatalf("outside target was mutated: %v", err)
}
})
}
}
tools/tht/internal/safeio/private_unix.go
+17
View File
@@ -11,11 +11,28 @@ import (
)
func createPrivateDirectory(path string) error {
return createPrivateDirectoryAfterParentOpen(path, nil)
}
func createPrivateDirectoryAfterParentOpen(path string, afterOpen func()) error {
parents, err := openCanonicalUnixParent(path)
if err != nil {
return ErrUnsafeFile
}
defer parents.Close()
if afterOpen != nil {
afterOpen()
}
return createPrivateDirectoryAt(parents)
}
// createPrivateDirectoryAt performs every mutating operation relative to the already-opened
// parent. An attacker can rename or replace any lexical ancestor after the open without
// redirecting mkdir or chmod into a different directory tree.
func createPrivateDirectoryAt(parents *unixParentHandles) error {
if parents == nil || parents.parent < 0 || parents.target == "" {
return ErrUnsafeFile
}
if err := unix.Mkdirat(parents.parent, parents.target, 0o700); err != nil {
if errors.Is(err, unix.EEXIST) {
return os.ErrExist