fix: secure profile persistence ownership
This commit is contained in:
proxyctl coordinator
@@ -2,14 +2,37 @@ package profile
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"os"
|
||||
"os/exec"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"syscall"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
type ownerCall struct {
|
||||
path string
|
||||
uid, gid int
|
||||
}
|
||||
|
||||
type fakeFileOperations struct {
|
||||
calls []ownerCall
|
||||
failContains string
|
||||
syncErr error
|
||||
}
|
||||
|
||||
func (f *fakeFileOperations) SetOwner(file *os.File, uid, gid int) error {
|
||||
f.calls = append(f.calls, ownerCall{path: file.Name(), uid: uid, gid: gid})
|
||||
if f.failContains != "" && strings.Contains(filepath.Base(file.Name()), f.failContains) {
|
||||
return errors.New("injected ownership failure")
|
||||
}
|
||||
return nil
|
||||
}
|
||||
|
||||
func (f *fakeFileOperations) SyncDirectory(*os.File) error { return f.syncErr }
|
||||
|
||||
type fakeIdentityEnvironment struct {
|
||||
current UserIdentity
|
||||
users map[string]UserIdentity
|
||||
@@ -86,29 +109,182 @@ func TestFileStoreLockSerializesAcrossProcesses(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestOwnedStoreAssignsInvokingUserToDirectoryLockAndReplacement(t *testing.T) {
|
||||
path := filepath.Join(t.TempDir(), "proxyctl", "profiles.json")
|
||||
owner := UserIdentity{Username: "alice", UID: 1001, GID: 1002}
|
||||
operations := &fakeFileOperations{}
|
||||
store := newFileStore(path, &owner, operations)
|
||||
if _, err := store.Mutate(func(state State) (State, error) {
|
||||
state.Profiles = append(state.Profiles, Profile{Name: "office"})
|
||||
return state, nil
|
||||
}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
wantKinds := map[string]bool{"directory": false, "lock": false, "temporary replacement": false}
|
||||
for _, call := range operations.calls {
|
||||
if call.uid != owner.UID || call.gid != owner.GID {
|
||||
t.Fatalf("ownership call for %q used %d:%d, want %d:%d", call.path, call.uid, call.gid, owner.UID, owner.GID)
|
||||
}
|
||||
switch {
|
||||
case call.path == filepath.Dir(path):
|
||||
wantKinds["directory"] = true
|
||||
case call.path == path+".lock":
|
||||
wantKinds["lock"] = true
|
||||
case strings.HasPrefix(filepath.Base(call.path), ".profiles-"):
|
||||
wantKinds["temporary replacement"] = true
|
||||
}
|
||||
}
|
||||
for kind, seen := range wantKinds {
|
||||
if !seen {
|
||||
t.Errorf("no ownership assignment for %s; calls=%#v", kind, operations.calls)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
func TestOwnedStoreReportsOwnershipFailuresBeforeCommit(t *testing.T) {
|
||||
for _, failure := range []string{"proxyctl", "profiles.json.lock", ".profiles-"} {
|
||||
t.Run(failure, func(t *testing.T) {
|
||||
path := filepath.Join(t.TempDir(), "proxyctl", "profiles.json")
|
||||
owner := UserIdentity{Username: "alice", UID: 1001, GID: 1002}
|
||||
operations := &fakeFileOperations{failContains: failure}
|
||||
store := newFileStore(path, &owner, operations)
|
||||
_, err := store.Mutate(func(state State) (State, error) {
|
||||
state.Profiles = append(state.Profiles, Profile{Name: "must-not-commit"})
|
||||
return state, nil
|
||||
})
|
||||
if err == nil || !strings.Contains(err.Error(), "owner") {
|
||||
t.Fatalf("ownership failure = %v", err)
|
||||
}
|
||||
if _, statErr := os.Stat(path); !errors.Is(statErr, os.ErrNotExist) {
|
||||
t.Fatalf("configuration was committed after ownership failure: %v", statErr)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestOwnedLoadRejectsConfigurationOwnershipFailure(t *testing.T) {
|
||||
path := filepath.Join(t.TempDir(), "proxyctl", "profiles.json")
|
||||
if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := os.WriteFile(path, []byte(`{"profiles":[]}`), 0o600); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
owner := UserIdentity{Username: "alice", UID: 1001, GID: 1002}
|
||||
operations := &fakeFileOperations{failContains: "profiles.json"}
|
||||
_, err := newFileStore(path, &owner, operations).Load()
|
||||
if err == nil || !strings.Contains(err.Error(), "set Profile configuration owner") {
|
||||
t.Fatalf("configuration ownership failure = %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestLoadRejectsProfileConfigurationSymlinkWithoutChangingTarget(t *testing.T) {
|
||||
directory := t.TempDir()
|
||||
target := filepath.Join(directory, "target.json")
|
||||
if err := os.WriteFile(target, []byte(`{"profiles":[]}`), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
path := filepath.Join(directory, "profiles.json")
|
||||
if err := os.Symlink(target, path); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
_, err := NewFileStore(path).Load()
|
||||
if err == nil || !strings.Contains(err.Error(), "without following links") {
|
||||
t.Fatalf("symlink load error = %v", err)
|
||||
}
|
||||
info, statErr := os.Stat(target)
|
||||
if statErr != nil {
|
||||
t.Fatal(statErr)
|
||||
}
|
||||
if info.Mode().Perm() != 0o644 {
|
||||
t.Fatalf("symlink target mode changed to %#o", info.Mode().Perm())
|
||||
}
|
||||
}
|
||||
|
||||
func TestMutationRejectsLockSymlinkWithoutChangingTarget(t *testing.T) {
|
||||
directory := filepath.Join(t.TempDir(), "proxyctl")
|
||||
if err := os.MkdirAll(directory, 0o700); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
target := filepath.Join(t.TempDir(), "lock-target")
|
||||
if err := os.WriteFile(target, []byte("unchanged"), 0o644); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
path := filepath.Join(directory, "profiles.json")
|
||||
if err := os.Symlink(target, path+".lock"); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
_, err := NewFileStore(path).Mutate(func(state State) (State, error) { return state, nil })
|
||||
if err == nil || !strings.Contains(err.Error(), "without following links") {
|
||||
t.Fatalf("lock symlink error = %v", err)
|
||||
}
|
||||
contents, readErr := os.ReadFile(target)
|
||||
if readErr != nil {
|
||||
t.Fatal(readErr)
|
||||
}
|
||||
if string(contents) != "unchanged" {
|
||||
t.Fatalf("lock symlink target was modified: %q", contents)
|
||||
}
|
||||
info, statErr := os.Stat(target)
|
||||
if statErr != nil {
|
||||
t.Fatal(statErr)
|
||||
}
|
||||
if info.Mode().Perm() != 0o644 {
|
||||
t.Fatalf("lock symlink target mode changed to %#o", info.Mode().Perm())
|
||||
}
|
||||
}
|
||||
|
||||
func TestDirectorySyncFailureReturnsCommittedStateAndRecognizableError(t *testing.T) {
|
||||
path := filepath.Join(t.TempDir(), "proxyctl", "profiles.json")
|
||||
operations := &fakeFileOperations{syncErr: errors.New("injected sync failure")}
|
||||
store := newFileStore(path, nil, operations)
|
||||
next, err := store.Mutate(func(state State) (State, error) {
|
||||
state.Profiles = append(state.Profiles, Profile{Name: "committed"})
|
||||
return state, nil
|
||||
})
|
||||
var committed *CommitError
|
||||
if !errors.As(err, &committed) {
|
||||
t.Fatalf("sync failure = %v, want CommitError", err)
|
||||
}
|
||||
if len(next.Profiles) != 1 || next.Profiles[0].Name != "committed" {
|
||||
t.Fatalf("returned committed state = %#v", next)
|
||||
}
|
||||
loaded, loadErr := NewFileStore(path).Load()
|
||||
if loadErr != nil {
|
||||
t.Fatal(loadErr)
|
||||
}
|
||||
if fmt.Sprint(loaded) != fmt.Sprint(next) {
|
||||
t.Fatalf("disk state = %#v, committed state = %#v", loaded, next)
|
||||
}
|
||||
}
|
||||
|
||||
func TestConfigPathUsesInvokingUserIdentityUnderSudo(t *testing.T) {
|
||||
environment := fakeIdentityEnvironment{
|
||||
current: UserIdentity{Username: "root", HomeDir: "/root"},
|
||||
users: map[string]UserIdentity{"alice": {Username: "alice", HomeDir: "/home/alice"}},
|
||||
users: map[string]UserIdentity{"alice": {Username: "alice", HomeDir: "/home/alice", UID: 1001, GID: 1002}},
|
||||
env: map[string]string{
|
||||
"SUDO_USER": "alice",
|
||||
"XDG_CONFIG_HOME": "/home/alice/.xdg/config",
|
||||
},
|
||||
}
|
||||
path, err := ConfigPathWith(environment)
|
||||
configuration, err := ResolveConfiguration(environment)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
want := filepath.Join("/home/alice/.xdg/config", "proxyctl", "profiles.json")
|
||||
if path != want {
|
||||
t.Fatalf("configuration path = %q, want invoking user's XDG path %q", path, want)
|
||||
if configuration.Path != want {
|
||||
t.Fatalf("configuration path = %q, want invoking user's XDG path %q", configuration.Path, want)
|
||||
}
|
||||
if configuration.Owner.Username != "alice" || configuration.Owner.UID != 1001 || configuration.Owner.GID != 1002 {
|
||||
t.Fatalf("configuration owner = %#v, want verified invoking-user identity", configuration.Owner)
|
||||
}
|
||||
}
|
||||
|
||||
func TestConfigPathDoesNotReuseRootXDGForInvokingUser(t *testing.T) {
|
||||
environment := fakeIdentityEnvironment{
|
||||
current: UserIdentity{Username: "root", HomeDir: "/root"},
|
||||
users: map[string]UserIdentity{"alice": {Username: "alice", HomeDir: "/home/alice"}},
|
||||
users: map[string]UserIdentity{"alice": {Username: "alice", HomeDir: "/home/alice", UID: 1001, GID: 1002}},
|
||||
env: map[string]string{
|
||||
"SUDO_USER": "alice",
|
||||
"XDG_CONFIG_HOME": "/root/.config",
|
||||
|
||||
Reference in New Issue
Block a user