fix(auth): harden Windows auth store privacy
This commit is contained in:
@@ -6,7 +6,6 @@ import (
|
||||
"io"
|
||||
"os"
|
||||
"path/filepath"
|
||||
"runtime"
|
||||
|
||||
"github.com/aritmolab/thothii/tools/tht/internal/safeio"
|
||||
"github.com/gofrs/flock"
|
||||
@@ -112,26 +111,15 @@ func decodeStrictYAML(contents []byte, destination any) error {
|
||||
}
|
||||
|
||||
func requirePrivateDirectory(directory string) error {
|
||||
if err := safeio.ValidateCanonicalPath(directory); err != nil {
|
||||
return err
|
||||
}
|
||||
info, err := os.Lstat(directory)
|
||||
if err != nil || !info.IsDir() || info.Mode()&os.ModeSymlink != 0 {
|
||||
return safeio.ErrUnsafeFile
|
||||
}
|
||||
resolved, err := filepath.EvalSymlinks(directory)
|
||||
if err != nil || resolved != directory {
|
||||
return safeio.ErrUnsafeFile
|
||||
}
|
||||
if runtime.GOOS != "windows" && info.Mode().Perm() != 0o700 {
|
||||
return safeio.ErrUnsafeFile
|
||||
}
|
||||
return nil
|
||||
return safeio.ValidatePrivateDirectory(directory)
|
||||
}
|
||||
|
||||
func readPrivateFile(path string) ([]byte, error) {
|
||||
if err := safeio.ValidatePrivateRegular(path); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
before, err := os.Lstat(path)
|
||||
if err != nil || !before.Mode().IsRegular() || before.Mode()&os.ModeSymlink != 0 || (runtime.GOOS != "windows" && before.Mode().Perm() != 0o600) {
|
||||
if err != nil {
|
||||
return nil, safeio.ErrUnsafeFile
|
||||
}
|
||||
contents, err := safeio.ReadCanonicalRegular(path, maxYAMLBytes)
|
||||
@@ -139,7 +127,10 @@ func readPrivateFile(path string) ([]byte, error) {
|
||||
return nil, err
|
||||
}
|
||||
after, err := os.Lstat(path)
|
||||
if err != nil || !after.Mode().IsRegular() || after.Mode()&os.ModeSymlink != 0 || (runtime.GOOS != "windows" && after.Mode().Perm() != 0o600) || !os.SameFile(before, after) {
|
||||
if err != nil || !os.SameFile(before, after) {
|
||||
return nil, safeio.ErrUnsafeFile
|
||||
}
|
||||
if err := safeio.ValidatePrivateRegular(path); err != nil {
|
||||
return nil, safeio.ErrUnsafeFile
|
||||
}
|
||||
return contents, nil
|
||||
@@ -149,14 +140,18 @@ func acquireLock(directory string) (*flock.Flock, error) {
|
||||
path := filepath.Join(directory, lockFileName)
|
||||
if _, err := os.Lstat(path); errors.Is(err, os.ErrNotExist) {
|
||||
if err := safeio.WriteCanonicalNewFile(path, nil, 0o600); err != nil {
|
||||
info, statErr := os.Lstat(path)
|
||||
if statErr != nil || !info.Mode().IsRegular() || info.Mode()&os.ModeSymlink != 0 || (runtime.GOOS != "windows" && info.Mode().Perm() != 0o600) {
|
||||
// A competing mutation may have created the lock after our Lstat. Accept only
|
||||
// that exact safe/private lock; every other creation failure remains unsafe.
|
||||
if validateErr := safeio.ValidatePrivateRegular(path); validateErr != nil {
|
||||
return nil, safeio.ErrUnsafeFile
|
||||
}
|
||||
}
|
||||
} else if err != nil {
|
||||
return nil, safeio.ErrUnsafeFile
|
||||
}
|
||||
if err := safeio.ValidatePrivateRegular(path); err != nil {
|
||||
return nil, err
|
||||
}
|
||||
lock := flock.New(path, flock.SetPermissions(0o600))
|
||||
if err := lock.Lock(); err != nil {
|
||||
return nil, errInvalidAuthenticationConfig
|
||||
|
||||
@@ -0,0 +1,92 @@
|
||||
//go:build windows
|
||||
|
||||
package authconfig
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"path/filepath"
|
||||
"runtime"
|
||||
"testing"
|
||||
|
||||
"github.com/aritmolab/thothii/tools/tht/internal/safeio"
|
||||
"golang.org/x/sys/windows"
|
||||
)
|
||||
|
||||
func TestLoadRejectsPermissiveWindowsDirectoryAuthAndUsersDACLs(t *testing.T) {
|
||||
for name, makePermissive := range map[string]func(t *testing.T, directory string){
|
||||
"directory": func(t *testing.T, directory string) {
|
||||
setPermissiveAuthDACL(t, directory)
|
||||
},
|
||||
"auth file": func(t *testing.T, directory string) {
|
||||
setPermissiveAuthDACL(t, filepath.Join(directory, "auth.yaml"))
|
||||
},
|
||||
"users file": func(t *testing.T, directory string) {
|
||||
setPermissiveAuthDACL(t, filepath.Join(directory, "users.yaml"))
|
||||
},
|
||||
} {
|
||||
t.Run(name, func(t *testing.T) {
|
||||
directory := writePrivateWindowsAuthFiles(t)
|
||||
makePermissive(t, directory)
|
||||
if _, _, err := Load(directory); !errors.Is(err, safeio.ErrUnsafeFile) {
|
||||
t.Fatalf("Load() error = %v, want ErrUnsafeFile", err)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestMutateUsersCreatesAndRejectsPermissiveWindowsLockDACL(t *testing.T) {
|
||||
directory := writePrivateWindowsAuthFiles(t)
|
||||
if err := MutateUsers(directory, func(*Registry) error { return nil }); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
lockPath := filepath.Join(directory, lockFileName)
|
||||
if err := safeio.ValidatePrivateRegular(lockPath); err != nil {
|
||||
t.Fatalf("lock privacy error = %v", err)
|
||||
}
|
||||
setPermissiveAuthDACL(t, lockPath)
|
||||
if err := MutateUsers(directory, func(*Registry) error { return nil }); !errors.Is(err, safeio.ErrUnsafeFile) {
|
||||
t.Fatalf("MutateUsers() error = %v, want ErrUnsafeFile", err)
|
||||
}
|
||||
}
|
||||
|
||||
func writePrivateWindowsAuthFiles(t *testing.T) string {
|
||||
t.Helper()
|
||||
directory := writeAuthFiles(t, defaultAuthYAML, registryYAML(adminUserYAML("admin", "Admin", true, "admin")))
|
||||
if err := safeio.ProtectPrivateDirectory(directory); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
for _, name := range []string{"auth.yaml", "users.yaml"} {
|
||||
if err := safeio.ProtectPrivateRegular(filepath.Join(directory, name)); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
return directory
|
||||
}
|
||||
|
||||
func setPermissiveAuthDACL(t *testing.T, path string) {
|
||||
t.Helper()
|
||||
world, err := windows.StringToSid("S-1-1-0")
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
var pinner runtime.Pinner
|
||||
pinner.Pin(world)
|
||||
defer pinner.Unpin()
|
||||
acl, err := windows.ACLFromEntries([]windows.EXPLICIT_ACCESS{{
|
||||
AccessPermissions: windows.GENERIC_READ | windows.GENERIC_WRITE,
|
||||
AccessMode: windows.GRANT_ACCESS,
|
||||
Trustee: windows.TRUSTEE{
|
||||
TrusteeForm: windows.TRUSTEE_IS_SID,
|
||||
TrusteeType: windows.TRUSTEE_IS_GROUP,
|
||||
TrusteeValue: windows.TrusteeValueFromSID(world),
|
||||
},
|
||||
}}, nil)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := windows.SetNamedSecurityInfo(path, windows.SE_FILE_OBJECT,
|
||||
windows.DACL_SECURITY_INFORMATION|windows.PROTECTED_DACL_SECURITY_INFORMATION,
|
||||
nil, nil, acl, nil); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user