diff --git a/go.mod b/go.mod index e823b76..91b4895 100644 --- a/go.mod +++ b/go.mod @@ -11,6 +11,7 @@ require ( github.com/aws/aws-sdk-go-v2/service/s3 v1.106.2 github.com/aws/smithy-go v1.27.5 github.com/charmbracelet/x/ansi v0.11.7 + github.com/ebitengine/purego v0.10.0 github.com/gofrs/flock v0.13.0 github.com/klauspost/compress v1.18.6 github.com/leanovate/gopter v0.2.11 @@ -63,7 +64,6 @@ require ( github.com/davecgh/go-spew v1.1.1 // indirect github.com/dnephin/pflag v1.0.7 // indirect github.com/dustin/go-humanize v1.0.1 // indirect - github.com/ebitengine/purego v0.10.0 // indirect github.com/fatih/color v1.18.0 // indirect github.com/fsnotify/fsnotify v1.9.0 // indirect github.com/go-logr/logr v1.4.3 // indirect diff --git a/safefileio/AGENTS.md b/safefileio/AGENTS.md index 579eb1b..671880b 100644 --- a/safefileio/AGENTS.md +++ b/safefileio/AGENTS.md @@ -20,6 +20,16 @@ callers responsible for their own file formats and higher-level policy. trusted user, token-owner, system, and administrator principals. Use SID semantics rather than username string comparisons. - If ownership or file type cannot be established, return an error. +- Private-file validation must inspect the same open handle used by the caller + and must never mutate permissions. Existing broad access may already have + produced handles that no in-place repair can revoke. +- On supported Unix platforms, require exact mode 0600 and no access ACL. + Reject Linux network and user-space filesystems whose effective access policy + cannot be verified through local mode and access-ACL operations. +- On Windows, require a protected DACL that grants access only to the current + user and trusted administrative principals. Callers recovering a broad or + inheritable file must create a private replacement rather than repair it in + place. ## Tests diff --git a/safefileio/open_file_other.go b/safefileio/open_file_other.go index c64b853..a160c74 100644 --- a/safefileio/open_file_other.go +++ b/safefileio/open_file_other.go @@ -25,3 +25,12 @@ func ValidateCurrentUserFile(*os.File) error { runtime.GOOS, ) } + +// ValidatePrivateCurrentUserFile fails closed when the platform cannot verify +// current-user-only file access. +func ValidatePrivateCurrentUserFile(*os.File) error { + return fmt.Errorf( + "safefileio: private current-user file validation is unsupported on %s", + runtime.GOOS, + ) +} diff --git a/safefileio/open_file_windows.go b/safefileio/open_file_windows.go index 690f894..0c4e602 100644 --- a/safefileio/open_file_windows.go +++ b/safefileio/open_file_windows.go @@ -9,6 +9,8 @@ import ( "golang.org/x/sys/windows" ) +var reOpenFile = windows.NewLazySystemDLL("kernel32.dll").NewProc("ReOpenFile") + // OpenCurrentUserFile opens path without following reparse points and verifies // the opened handle is a regular file owned by the current token user or token // owner. @@ -55,6 +57,50 @@ func ValidateCurrentUserFile(file *os.File) error { return validateWindowsFileHandle(file.Name(), windows.Handle(file.Fd())) } +// ValidatePrivateCurrentUserFile verifies that an open current-user-owned file +// has a protected DACL granting access only to the current user and Windows +// administrative principals. +func ValidatePrivateCurrentUserFile(file *os.File) error { + handle, err := reopenWindowsFileForDACL(file) + if err != nil { + return err + } + defer func() { _ = windows.CloseHandle(handle) }() + userSID, err := currentWindowsUserSID() + if err != nil { + return err + } + ownerSID, err := currentWindowsOwnerSID() + if err != nil { + return err + } + return verifyWindowsFileDACL(file.Name(), handle, userSID, ownerSID) +} + +func reopenWindowsFileForDACL(file *os.File) (windows.Handle, error) { + if err := ValidateCurrentUserFile(file); err != nil { + return 0, err + } + result, _, callErr := reOpenFile.Call( + file.Fd(), + uintptr(windows.READ_CONTROL), + uintptr(windows.FILE_SHARE_READ|windows.FILE_SHARE_WRITE|windows.FILE_SHARE_DELETE), + 0, + ) + handle := windows.Handle(result) + if handle == windows.InvalidHandle { + if callErr != windows.ERROR_SUCCESS { + return 0, callErr + } + return 0, windows.ERROR_INVALID_HANDLE + } + if err := validateWindowsFileHandle(file.Name(), handle); err != nil { + _ = windows.CloseHandle(handle) + return 0, err + } + return handle, nil +} + func validateWindowsFileHandle(path string, handle windows.Handle) error { var info windows.ByHandleFileInformation if err := windows.GetFileInformationByHandle(handle, &info); err != nil { diff --git a/safefileio/private_dir_windows.go b/safefileio/private_dir_windows.go index 50f0ccc..cb27da7 100644 --- a/safefileio/private_dir_windows.go +++ b/safefileio/private_dir_windows.go @@ -196,6 +196,18 @@ func verifyWindowsDirectoryOwner(path string, owner, userSID, ownerSID *windows. } func verifyWindowsDirDACL(path string, handle windows.Handle, userSID, ownerSID *windows.SID) error { + return verifyWindowsDACL(path, handle, userSID, ownerSID) +} + +func verifyWindowsFileDACL(path string, handle windows.Handle, userSID, ownerSID *windows.SID) error { + return verifyWindowsDACL(path, handle, userSID, ownerSID) +} + +func verifyWindowsDACL( + path string, + handle windows.Handle, + userSID, ownerSID *windows.SID, +) error { descriptor, err := windows.GetSecurityInfo( handle, windows.SE_FILE_OBJECT, diff --git a/safefileio/private_dir_windows_test.go b/safefileio/private_dir_windows_test.go index ecc968c..991713c 100644 --- a/safefileio/private_dir_windows_test.go +++ b/safefileio/private_dir_windows_test.go @@ -97,6 +97,119 @@ func TestOpenCurrentUserFileAcceptsCurrentTokenOwner(t *testing.T) { require.NoError(t, file.Close()) } +func TestValidatePrivateCurrentUserFileRejectsBroadDACL(t *testing.T) { + path := filepath.Join(t.TempDir(), "record.json") + require.NoError(t, os.WriteFile(path, []byte("{}"), 0o600)) + file, err := os.OpenFile(path, os.O_RDWR, 0) + require.NoError(t, err) + defer func() { _ = file.Close() }() + path16, err := windows.UTF16PtrFromString(path) + require.NoError(t, err) + handle, err := windows.CreateFile( + path16, + windows.READ_CONTROL|windows.WRITE_DAC, + windows.FILE_SHARE_READ|windows.FILE_SHARE_WRITE|windows.FILE_SHARE_DELETE, + nil, + windows.OPEN_EXISTING, + windows.FILE_FLAG_OPEN_REPARSE_POINT, + 0, + ) + require.NoError(t, err) + defer func() { _ = windows.CloseHandle(handle) }() + userSID, err := currentWindowsUserSID() + require.NoError(t, err) + ownerSID, err := currentWindowsOwnerSID() + require.NoError(t, err) + world, err := windows.CreateWellKnownSid(windows.WinWorldSid) + require.NoError(t, err) + acl, err := windows.ACLFromEntries([]windows.EXPLICIT_ACCESS{ + allowFullControl(userSID, windows.TRUSTEE_IS_USER), + allowFullControl(world, windows.TRUSTEE_IS_WELL_KNOWN_GROUP), + }, nil) + require.NoError(t, err) + require.NoError(t, windows.SetSecurityInfo( + handle, + windows.SE_FILE_OBJECT, + windows.DACL_SECURITY_INFORMATION|windows.PROTECTED_DACL_SECURITY_INFORMATION, + nil, + nil, + acl, + nil, + )) + require.Error(t, verifyWindowsDirDACL(path, handle, userSID, ownerSID)) + + require.Error(t, ValidatePrivateCurrentUserFile(file)) + require.Error(t, verifyWindowsDirDACL(path, handle, userSID, ownerSID)) +} + +func TestValidatePrivateCurrentUserFileRejectsUnprotectedPrivateDACL(t *testing.T) { + dir := filepath.Join(t.TempDir(), "private") + require.NoError(t, EnsurePrivateDir(dir)) + path := filepath.Join(dir, "record.json") + require.NoError(t, os.WriteFile(path, []byte("{}"), 0o600)) + file, err := os.OpenFile(path, os.O_RDWR, 0) + require.NoError(t, err) + defer func() { _ = file.Close() }() + path16, err := windows.UTF16PtrFromString(path) + require.NoError(t, err) + handle, err := windows.CreateFile( + path16, + windows.READ_CONTROL|windows.WRITE_DAC, + windows.FILE_SHARE_READ|windows.FILE_SHARE_WRITE|windows.FILE_SHARE_DELETE, + nil, + windows.OPEN_EXISTING, + windows.FILE_FLAG_OPEN_REPARSE_POINT, + 0, + ) + require.NoError(t, err) + defer func() { _ = windows.CloseHandle(handle) }() + userSID, err := currentWindowsUserSID() + require.NoError(t, err) + acl, err := windows.ACLFromEntries([]windows.EXPLICIT_ACCESS{ + allowFullControl(userSID, windows.TRUSTEE_IS_USER), + }, nil) + require.NoError(t, err) + require.NoError(t, windows.SetSecurityInfo( + handle, + windows.SE_FILE_OBJECT, + windows.DACL_SECURITY_INFORMATION|windows.UNPROTECTED_DACL_SECURITY_INFORMATION, + nil, + nil, + acl, + nil, + )) + + require.Error(t, ValidatePrivateCurrentUserFile(file)) +} + +func TestValidatePrivateCurrentUserFileAcceptsProtectedPrivateDACL(t *testing.T) { + dir := filepath.Join(t.TempDir(), "private") + require.NoError(t, EnsurePrivateDir(dir)) + path := filepath.Join(dir, "record.json") + require.NoError(t, os.WriteFile(path, []byte("{}"), 0o600)) + file, err := os.OpenFile(path, os.O_RDWR, 0) + require.NoError(t, err) + defer func() { _ = file.Close() }() + path16, err := windows.UTF16PtrFromString(path) + require.NoError(t, err) + handle, err := windows.CreateFile( + path16, + windows.READ_CONTROL|windows.WRITE_DAC, + windows.FILE_SHARE_READ|windows.FILE_SHARE_WRITE|windows.FILE_SHARE_DELETE, + nil, + windows.OPEN_EXISTING, + windows.FILE_FLAG_OPEN_REPARSE_POINT, + 0, + ) + require.NoError(t, err) + defer func() { _ = windows.CloseHandle(handle) }() + userSID, err := currentWindowsUserSID() + require.NoError(t, err) + require.NoError(t, restrictWindowsDir(handle, userSID)) + + require.NoError(t, ValidatePrivateCurrentUserFile(file)) +} + func TestWindowsOwnerMatchesCurrentUserAndTokenOwner(t *testing.T) { require := require.New(t) assert := assert.New(t) diff --git a/safefileio/private_file_darwin.go b/safefileio/private_file_darwin.go new file mode 100644 index 0000000..23b0860 --- /dev/null +++ b/safefileio/private_file_darwin.go @@ -0,0 +1,89 @@ +package safefileio + +import ( + "errors" + "fmt" + "os" + "runtime" + "sync" + "syscall" + + "github.com/ebitengine/purego" +) + +const ( + darwinACLExtended = 0x00000100 + darwinACLFirstEntry = 0 +) + +type darwinACLAPI struct { + getFD func(int32, int32) uintptr + getEntry func(uintptr, int32, *uintptr) int32 + free func(uintptr) int32 + errno func() *int32 +} + +var ( + darwinACLOnce sync.Once + darwinACL darwinACLAPI + darwinACLErr error +) + +// ValidatePrivateCurrentUserFile verifies that an open current-user-owned file +// has private mode bits and no macOS extended ACL. +func ValidatePrivateCurrentUserFile(file *os.File) error { + return validatePrivateCurrentUserFile(file, validateDarwinExtendedACL) +} + +func validateDarwinExtendedACL(file *os.File) error { + darwinACLOnce.Do(loadDarwinACL) + if darwinACLErr != nil { + return darwinACLErr + } + runtime.LockOSThread() + defer runtime.UnlockOSThread() + errno := darwinACL.errno() + *errno = 0 + acl := darwinACL.getFD(int32(file.Fd()), darwinACLExtended) + if acl == 0 { + callErr := syscall.Errno(*errno) + if callErr == syscall.ENOENT { + return nil + } + return fmt.Errorf("read extended ACL: %w", callErr) + } + defer func() { _ = darwinACL.free(acl) }() + var entry uintptr + result := darwinACL.getEntry(acl, darwinACLFirstEntry, &entry) + switch result { + case 0: + return errors.New("safefileio: file has a macOS extended ACL") + default: + return errors.New("read extended ACL entry failed") + } +} + +func loadDarwinACL() { + handle, err := purego.Dlopen( + "/usr/lib/libSystem.B.dylib", + purego.RTLD_NOW|purego.RTLD_LOCAL, + ) + if err != nil { + darwinACLErr = fmt.Errorf("load macOS ACL API: %w", err) + return + } + for name, target := range map[string]any{ + "acl_get_fd_np": &darwinACL.getFD, + "acl_get_entry": &darwinACL.getEntry, + "acl_free": &darwinACL.free, + "__error": &darwinACL.errno, + } { + symbol, symbolErr := purego.Dlsym(handle, name) + err = symbolErr + if err != nil { + darwinACLErr = fmt.Errorf("load macOS ACL function %s: %w", name, err) + return + } + purego.RegisterFunc(target, symbol) + } +} diff --git a/safefileio/private_file_darwin_test.go b/safefileio/private_file_darwin_test.go new file mode 100644 index 0000000..6afa90f --- /dev/null +++ b/safefileio/private_file_darwin_test.go @@ -0,0 +1,32 @@ +package safefileio_test + +import ( + "os" + "os/exec" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "go.kenn.io/kit/safefileio" +) + +func TestValidatePrivateCurrentUserFileRejectsExtendedACL(t *testing.T) { + path := filepath.Join(t.TempDir(), "record.json") + require.NoError(t, os.WriteFile(path, []byte("{}"), 0o600)) + output, err := exec.Command( + "chmod", + "+a", + "everyone allow read", + path, + ).CombinedOutput() + require.NoError(t, err, string(output)) + file, err := os.OpenFile(path, os.O_RDWR, 0) + require.NoError(t, err) + defer func() { _ = file.Close() }() + + require.Error(t, safefileio.ValidatePrivateCurrentUserFile(file)) + listing, err := exec.Command("ls", "-lde", path).CombinedOutput() + require.NoError(t, err, string(listing)) + assert.Contains(t, string(listing), "everyone allow read") +} diff --git a/safefileio/private_file_internal_darwin_test.go b/safefileio/private_file_internal_darwin_test.go new file mode 100644 index 0000000..66de9a2 --- /dev/null +++ b/safefileio/private_file_internal_darwin_test.go @@ -0,0 +1,16 @@ +package safefileio + +import ( + "os" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestValidateDarwinExtendedACLRejectsFailedInspection(t *testing.T) { + file, err := os.CreateTemp(t.TempDir(), "record-*.json") + require.NoError(t, err) + require.NoError(t, file.Close()) + + require.Error(t, validateDarwinExtendedACL(file)) +} diff --git a/safefileio/private_file_linux.go b/safefileio/private_file_linux.go new file mode 100644 index 0000000..2f612c0 --- /dev/null +++ b/safefileio/private_file_linux.go @@ -0,0 +1,67 @@ +package safefileio + +import ( + "errors" + "fmt" + "os" + + "golang.org/x/sys/unix" +) + +// ValidatePrivateCurrentUserFile verifies that an open current-user-owned file +// has private mode bits and no access ACL that could grant another principal. +func ValidatePrivateCurrentUserFile(file *os.File) error { + return validatePrivateCurrentUserFile(file, validateLinuxPrivateAccess) +} + +func validateLinuxPrivateAccess(file *os.File) error { + var status unix.Statfs_t + if err := unix.Fstatfs(int(file.Fd()), &status); err != nil { + return fmt.Errorf("inspect file filesystem: %w", err) + } + if linuxFilesystemHasExternalAccessPolicy(int64(status.Type)) { + return errors.New( + "safefileio: private current-user file validation is unsupported " + + "on filesystems with external access policy", + ) + } + return validateLinuxAccessACLs(file) +} + +func linuxFilesystemHasExternalAccessPolicy(filesystemType int64) bool { + switch uint32(filesystemType) { + case uint32(unix.AAFS_MAGIC), + uint32(unix.AFS_FS_MAGIC), + uint32(unix.AFS_SUPER_MAGIC), + uint32(unix.CEPH_SUPER_MAGIC), + uint32(unix.CIFS_SUPER_MAGIC), + uint32(unix.CODA_SUPER_MAGIC), + uint32(unix.FUSE_SUPER_MAGIC), + uint32(unix.NCP_SUPER_MAGIC), + uint32(unix.NFS_SUPER_MAGIC), + uint32(unix.SMB_SUPER_MAGIC), + uint32(unix.SMB2_SUPER_MAGIC), + uint32(unix.V9FS_MAGIC): + return true + default: + return false + } +} + +func validateLinuxAccessACLs(file *os.File) error { + for _, attribute := range []string{ + "system.posix_acl_access", + "system.nfs4_acl", + "system.cifs_acl", + } { + _, err := unix.Fgetxattr(int(file.Fd()), attribute, nil) + if err == nil { + return fmt.Errorf("safefileio: file has access ACL %s", attribute) + } + if errors.Is(err, unix.ENODATA) || errors.Is(err, unix.ENOTSUP) { + continue + } + return fmt.Errorf("inspect access ACL %s: %w", attribute, err) + } + return nil +} diff --git a/safefileio/private_file_linux_test.go b/safefileio/private_file_linux_test.go new file mode 100644 index 0000000..e40ad80 --- /dev/null +++ b/safefileio/private_file_linux_test.go @@ -0,0 +1,31 @@ +package safefileio + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "golang.org/x/sys/unix" +) + +func TestLinuxFilesystemHasExternalAccessPolicy(t *testing.T) { + for name, magic := range map[string]uint32{ + "Andrew FS": uint32(unix.AAFS_MAGIC), + "AFS fs": uint32(unix.AFS_FS_MAGIC), + "AFS": uint32(unix.AFS_SUPER_MAGIC), + "Ceph": uint32(unix.CEPH_SUPER_MAGIC), + "CIFS": uint32(unix.CIFS_SUPER_MAGIC), + "Coda": uint32(unix.CODA_SUPER_MAGIC), + "FUSE": uint32(unix.FUSE_SUPER_MAGIC), + "NCP": uint32(unix.NCP_SUPER_MAGIC), + "NFS": uint32(unix.NFS_SUPER_MAGIC), + "SMB": uint32(unix.SMB_SUPER_MAGIC), + "SMB2": uint32(unix.SMB2_SUPER_MAGIC), + "Plan 9 filesystem": uint32(unix.V9FS_MAGIC), + } { + t.Run(name, func(t *testing.T) { + assert.True(t, linuxFilesystemHasExternalAccessPolicy(int64(magic))) + assert.True(t, linuxFilesystemHasExternalAccessPolicy(int64(int32(magic)))) + }) + } + assert.False(t, linuxFilesystemHasExternalAccessPolicy(unix.EXT4_SUPER_MAGIC)) +} diff --git a/safefileio/private_file_supported_unix.go b/safefileio/private_file_supported_unix.go new file mode 100644 index 0000000..f3f2f88 --- /dev/null +++ b/safefileio/private_file_supported_unix.go @@ -0,0 +1,32 @@ +//go:build darwin || linux + +package safefileio + +import ( + "fmt" + "os" +) + +func validatePrivateCurrentUserFile( + file *os.File, + validatePlatformAccess func(*os.File) error, +) error { + if err := ValidateCurrentUserFile(file); err != nil { + return err + } + if err := verifyPrivateFileMode(file); err != nil { + return err + } + return validatePlatformAccess(file) +} + +func verifyPrivateFileMode(file *os.File) error { + info, err := file.Stat() + if err != nil { + return err + } + if mode := info.Mode().Perm(); mode != 0o600 { + return fmt.Errorf("safefileio: file mode is %04o, not 0600", mode) + } + return nil +} diff --git a/safefileio/private_file_supported_unix_external_test.go b/safefileio/private_file_supported_unix_external_test.go new file mode 100644 index 0000000..afb417a --- /dev/null +++ b/safefileio/private_file_supported_unix_external_test.go @@ -0,0 +1,36 @@ +//go:build darwin || linux + +package safefileio_test + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" + "go.kenn.io/kit/safefileio" +) + +func TestValidatePrivateCurrentUserFileRejectsPublicMode(t *testing.T) { + path := filepath.Join(t.TempDir(), "record.json") + require.NoError(t, os.WriteFile(path, []byte("{}"), 0o600)) + require.NoError(t, os.Chmod(path, 0o666)) + file, err := os.OpenFile(path, os.O_RDWR, 0) + require.NoError(t, err) + defer func() { _ = file.Close() }() + + require.Error(t, safefileio.ValidatePrivateCurrentUserFile(file)) + info, err := file.Stat() + require.NoError(t, err) + require.Equal(t, os.FileMode(0o666), info.Mode().Perm()) +} + +func TestValidatePrivateCurrentUserFileAcceptsPrivateMode(t *testing.T) { + path := filepath.Join(t.TempDir(), "record.json") + require.NoError(t, os.WriteFile(path, []byte("{}"), 0o600)) + file, err := os.OpenFile(path, os.O_RDWR, 0) + require.NoError(t, err) + defer func() { _ = file.Close() }() + + require.NoError(t, safefileio.ValidatePrivateCurrentUserFile(file)) +} diff --git a/safefileio/private_file_supported_unix_test.go b/safefileio/private_file_supported_unix_test.go new file mode 100644 index 0000000..8f542e0 --- /dev/null +++ b/safefileio/private_file_supported_unix_test.go @@ -0,0 +1,22 @@ +//go:build darwin || linux + +package safefileio + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestVerifyPrivateFileModeRejectsPublicMode(t *testing.T) { + path := filepath.Join(t.TempDir(), "record.json") + require.NoError(t, os.WriteFile(path, []byte("{}"), 0o600)) + require.NoError(t, os.Chmod(path, 0o666)) + file, err := os.OpenFile(path, os.O_RDWR, 0) + require.NoError(t, err) + defer func() { _ = file.Close() }() + + require.ErrorContains(t, verifyPrivateFileMode(file), "mode is 0666, not 0600") +} diff --git a/safefileio/private_file_unsupported_unix.go b/safefileio/private_file_unsupported_unix.go new file mode 100644 index 0000000..db0d985 --- /dev/null +++ b/safefileio/private_file_unsupported_unix.go @@ -0,0 +1,21 @@ +//go:build unix && !darwin && !linux + +package safefileio + +import ( + "fmt" + "os" + "runtime" +) + +// ValidatePrivateCurrentUserFile fails closed on Unix platforms where Kit +// cannot inspect access-control lists through the verified file handle. +func ValidatePrivateCurrentUserFile(file *os.File) error { + if err := ValidateCurrentUserFile(file); err != nil { + return err + } + return fmt.Errorf( + "safefileio: private current-user file validation is unsupported on %s", + runtime.GOOS, + ) +} diff --git a/safefileio/private_file_unsupported_unix_test.go b/safefileio/private_file_unsupported_unix_test.go new file mode 100644 index 0000000..064b022 --- /dev/null +++ b/safefileio/private_file_unsupported_unix_test.go @@ -0,0 +1,23 @@ +//go:build unix && !darwin && !linux + +package safefileio_test + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" + "go.kenn.io/kit/safefileio" +) + +func TestValidatePrivateCurrentUserFileFailsClosedWhenUnsupported(t *testing.T) { + path := filepath.Join(t.TempDir(), "record.json") + require.NoError(t, os.WriteFile(path, []byte("{}"), 0o600)) + file, err := os.OpenFile(path, os.O_RDWR, 0) + require.NoError(t, err) + defer func() { _ = file.Close() }() + + err = safefileio.ValidatePrivateCurrentUserFile(file) + require.ErrorContains(t, err, "private current-user file validation is unsupported") +}