From 90052cbefb030edd93b64041955cb50f7f3da2dd Mon Sep 17 00:00:00 2001 From: Theodor Midtlien Date: Thu, 17 Sep 2026 11:20:13 +0200 Subject: [PATCH] [client] Profile ownership console user tofu (#7529) * Add consoleuser and stamp default profile on known username in migration * Refactor consoleuser to verify Id, fix seats on linux and default stamp * Add default profile claim * Add disable auto-claim of default profile and always fail close * Add disable auto-claim flag to migration * Adding timeout to console user on Linux and close library load on darwin * Fixed failed close test * Close both Dlopen for darwin * Replace RegisterFunc with purego.Dlsym to avoid possible panic * Fix freebsd tty enumeration * Fix active profile migration logic and add test * Log defaultClaimDisabled error once * Guard against panicking console user lookup. * Fix merge conflict * Fix broken tests --- client/internal/ipcauth/consoleuser.go | 43 +++++ client/internal/ipcauth/consoleuser_darwin.go | 95 ++++++++++ .../internal/ipcauth/consoleuser_freebsd.go | 64 +++++++ client/internal/ipcauth/consoleuser_linux.go | 173 ++++++++++++++++++ client/internal/ipcauth/consoleuser_other.go | 13 ++ client/internal/ipcauth/consoleuser_test.go | 54 ++++++ .../internal/ipcauth/consoleuser_windows.go | 40 ++++ client/internal/profilemanager/migration.go | 24 ++- .../internal/profilemanager/migration_test.go | 70 +++++++ .../internal/profilemanager/profilemanager.go | 5 +- client/internal/profilemanager/service.go | 67 +++++++ .../internal/profilemanager/service_test.go | 77 +++++++- 12 files changed, 712 insertions(+), 13 deletions(-) create mode 100644 client/internal/ipcauth/consoleuser.go create mode 100644 client/internal/ipcauth/consoleuser_darwin.go create mode 100644 client/internal/ipcauth/consoleuser_freebsd.go create mode 100644 client/internal/ipcauth/consoleuser_linux.go create mode 100644 client/internal/ipcauth/consoleuser_other.go create mode 100644 client/internal/ipcauth/consoleuser_test.go create mode 100644 client/internal/ipcauth/consoleuser_windows.go diff --git a/client/internal/ipcauth/consoleuser.go b/client/internal/ipcauth/consoleuser.go new file mode 100644 index 000000000..982d2c104 --- /dev/null +++ b/client/internal/ipcauth/consoleuser.go @@ -0,0 +1,43 @@ +package ipcauth + +import ( + "sync" + + log "github.com/sirupsen/logrus" +) + +// logConsolePanic keeps the notice to once per process. +var logConsolePanic sync.Once + +// IsConsoleUser reports whether a caller is sitting at one of this machine's +// consoles right now. +// +// It is false on a headless machine, which has no seat to sit at, on a +// platform that exposes no console-user lookup at all, and whenever the lookup +// fails. Callers must read that as "cannot confirm" rather than as proof of +// absence. It gates handing out ownership, so a lookup that cannot answer +// withholds a claim, and never grants one. +func IsConsoleUser(id Identity) bool { + if !id.Known() { + return false + } + + return guardConsoleLookup(id, isConsoleUser) +} + +// guardConsoleLookup runs a platform lookup and turns a panic out of it into +// "cannot confirm". +func guardConsoleLookup(id Identity, lookup func(Identity) bool) (atConsole bool) { + defer func() { + r := recover() + if r == nil { + return + } + atConsole = false + logConsolePanic.Do(func() { + log.Errorf("console user lookup panicked, no caller will be treated as being at the console: %v", r) + }) + }() + + return lookup(id) +} diff --git a/client/internal/ipcauth/consoleuser_darwin.go b/client/internal/ipcauth/consoleuser_darwin.go new file mode 100644 index 000000000..53ed9c704 --- /dev/null +++ b/client/internal/ipcauth/consoleuser_darwin.go @@ -0,0 +1,95 @@ +package ipcauth + +import ( + "unsafe" + + "github.com/ebitengine/purego" +) + +// isConsoleUser reports whether id is the user currently logged into the macOS +// GUI console session. Uses SCDynamicStoreCopyConsoleUser from the +// SystemConfiguration framework via purego (no cgo). +func isConsoleUser(id Identity) bool { + // A SID belongs to a Windows principal and has no uid to compare. + if id.IsWindows() { + return false + } + + uid, ok := consoleUID() + return ok && uid == id.UID +} + +// consoleUID returns the uid of the GUI console session, and false when nobody +// is logged in at it. +func consoleUID() (uint32, bool) { + sc, err := purego.Dlopen( + "/System/Library/Frameworks/SystemConfiguration.framework/SystemConfiguration", + purego.RTLD_NOW|purego.RTLD_GLOBAL, + ) + if err != nil { + return 0, false + } + defer func() { + _ = purego.Dlclose(sc) + }() + + cf, err := purego.Dlopen( + "/System/Library/Frameworks/CoreFoundation.framework/CoreFoundation", + purego.RTLD_NOW|purego.RTLD_GLOBAL, + ) + if err != nil { + return 0, false + } + defer func() { + _ = purego.Dlclose(cf) + }() + + // CFStringRef SCDynamicStoreCopyConsoleUser(SCDynamicStoreRef store, + // uid_t *uid, gid_t *gid); + // + // We pass nil for the store (NULL is accepted; the framework creates a + // transient one), discard the returned CFStringRef username (we only + // need the UID), and read uid via the out-pointer. + copyConsoleUserSym, ok := resolveSymbol(sc, "SCDynamicStoreCopyConsoleUser") + if !ok { + return 0, false + } + cfReleaseSym, ok := resolveSymbol(cf, "CFRelease") + if !ok { + return 0, false + } + + var copyConsoleUser func(store uintptr, uidPtr, gidPtr unsafe.Pointer) uintptr + purego.RegisterFunc(©ConsoleUser, copyConsoleUserSym) + + var cfRelease func(uintptr) + purego.RegisterFunc(&cfRelease, cfReleaseSym) + + var uid uint32 + var gid uint32 + + cfStr := copyConsoleUser(0, unsafe.Pointer(&uid), unsafe.Pointer(&gid)) + if cfStr == 0 { + return 0, false + } + cfRelease(cfStr) + + // loginwindow / no GUI session reports uid 0. We don't want the + // console-user path to grant anything to root, so treat uid 0 as "no + // console user". + if uid == 0 { + return 0, false + } + + return uid, true +} + +// resolveSymbol looks up one symbol and reports false when it is not there, +// rather than passing it to RegisterFunc which would panic. +func resolveSymbol(handle uintptr, name string) (uintptr, bool) { + sym, err := purego.Dlsym(handle, name) + if err != nil || sym == 0 { + return 0, false + } + return sym, true +} diff --git a/client/internal/ipcauth/consoleuser_freebsd.go b/client/internal/ipcauth/consoleuser_freebsd.go new file mode 100644 index 000000000..a24eb5fb0 --- /dev/null +++ b/client/internal/ipcauth/consoleuser_freebsd.go @@ -0,0 +1,64 @@ +package ipcauth + +import ( + "os" + "path/filepath" + "strings" + "syscall" +) + +const ( + devDir = "/dev" + + // vtPrefix names the virtual terminals vt(4) publishes. How many there are + // is a kernel constant rather than a fixed number, and past the tenth they + // are not spelled in decimal: the unit is rendered in base 32, so the one + // after ttyv9 is ttyva. + vtPrefix = "ttyv" +) + +// isConsoleUser reports whether id is logged into the FreeBSD console. +// FreeBSD's vt(4) chowns the virtual terminal device to the user logged in on +// it, so a non-root owner of any /dev/ttyv* reliably identifies a console user. +// +// Network ptys (pts) are intentionally not considered: SSH'd users are not "at +// the console". +func isConsoleUser(id Identity) bool { + // A SID belongs to a Windows principal and has no uid to compare. + if id.IsWindows() { + return false + } + + // A root-owned ttyv is an unclaimed terminal rather than a root login, so + // uid 0 never matches. A root caller is privileged anyway and does not + // reach ownership checks through here. + if id.UID == 0 { + return false + } + + entries, err := os.ReadDir(devDir) + if err != nil { + return false + } + + for _, entry := range entries { + name := entry.Name() + suffix, ok := strings.CutPrefix(name, vtPrefix) + if !ok || suffix == "" { + continue + } + fi, err := os.Stat(filepath.Join(devDir, name)) + if err != nil { + continue + } + st, ok := fi.Sys().(*syscall.Stat_t) + if !ok { + continue + } + if st.Uid == id.UID { + return true + } + } + + return false +} diff --git a/client/internal/ipcauth/consoleuser_linux.go b/client/internal/ipcauth/consoleuser_linux.go new file mode 100644 index 000000000..adfd5272b --- /dev/null +++ b/client/internal/ipcauth/consoleuser_linux.go @@ -0,0 +1,173 @@ +package ipcauth + +import ( + "context" + "fmt" + "strings" + "time" + + "github.com/godbus/dbus/v5" + log "github.com/sirupsen/logrus" +) + +const ( + loginDest = "org.freedesktop.login1" + loginPath = dbus.ObjectPath("/org/freedesktop/login1") + loginInterface = "org.freedesktop.login1.Manager" + listSeats = loginInterface + ".ListSeats" + + seatInterface = "org.freedesktop.login1.Seat" + seatActiveSession = seatInterface + ".ActiveSession" + + sessionInterface = "org.freedesktop.login1.Session" + sessionActive = sessionInterface + ".Active" + sessionClass = sessionInterface + ".Class" + sessionRemote = sessionInterface + ".Remote" + sessionUser = sessionInterface + ".User" + + // propertiesGet is the standard property reader. godbus offers no + // context-aware GetProperty, and GetProperty is only this call underneath, + // so a bounded read has to make it directly. + propertiesGet = "org.freedesktop.DBus.Properties.Get" + + // nullObjectPath is what logind puts in an object path field that refers to + // nothing, a seat with no session in the foreground being the one that + // matters here. + nullObjectPath = dbus.ObjectPath("/") + + // consoleLookupTimeout bounds the whole seat walk, not each call in it, so + // a machine with several seats cannot multiply what an unanswering bus + // costs. A healthy lookup is well under a millisecond. + consoleLookupTimeout = 1 * time.Second +) + +// isConsoleUser reports whether id holds the foreground session of one of this +// machine's seats. +// +// No seats means no console, which is the honest answer for a headless machine +// and the one that keeps its profiles to the privileged caller. +func isConsoleUser(id Identity) bool { + // A SID belongs to a Windows principal and has no uid to compare. Nothing + // on Linux produces one, and comparing anyway would match uid 0. + if id.IsWindows() { + return false + } + + conn, err := dbus.SystemBus() + if err != nil { + return false + } + + ctx, cancel := context.WithTimeout(context.Background(), consoleLookupTimeout) + defer cancel() + + // ListSeats returns a(so): seat id and object path. + var seats []struct { + ID string + Path dbus.ObjectPath + } + if err := conn.Object(loginDest, loginPath).CallWithContext(ctx, listSeats, 0).Store(&seats); err != nil { + log.Debugf("cannot list seats, treating the caller as not at the console: %v", err) + return false + } + + for _, seat := range seats { + uid, ok := consoleSessionUID(ctx, conn, seat.Path) + if ok && uid == id.UID { + return true + } + } + + return false +} + +// consoleSessionUID returns who holds a seat's foreground session, and false +// unless that session is a person logged in locally. +func consoleSessionUID(ctx context.Context, conn *dbus.Conn, seatPath dbus.ObjectPath) (uint32, bool) { + // ActiveSession is (so): session id and object path. It names nothing on a + // seat whose VT has been switched away from, and on one whose display + // manager has not started a session yet. + prop, err := property(ctx, conn.Object(loginDest, seatPath), seatActiveSession) + if err != nil { + return 0, false + } + var active struct { + ID string + Path dbus.ObjectPath + } + if err := prop.Store(&active); err != nil { + return 0, false + } + if active.ID == "" || active.Path == "" || active.Path == nullObjectPath { + return 0, false + } + + session := conn.Object(loginDest, active.Path) + + // Only "user" sessions count: a greeter or a lock screen is the display + // manager sitting at the seat, not somebody to hand a profile to. + if class, ok := stringProperty(ctx, session, sessionClass); !ok || class != "user" { + return 0, false + } + + // A remote session can hold a seat, and unlike the session's class and + // type, which pam_systemd takes from the environment of whoever opened the + // session, remoteness is set by the thing that accepted the connection. So + // it is worth asking even once the seat is established. + if remote, ok := boolProperty(ctx, session, sessionRemote); !ok || remote { + return 0, false + } + + // Implied by the seat having named this session, and kept as a cross-check + // against a foreground that moved between the two calls. + if isActive, ok := boolProperty(ctx, session, sessionActive); !ok || !isActive { + return 0, false + } + + // User is (uo): uid and the user object's path. Read from the session + // object rather than carried over from a listing, so the uid returned and + // the checks above are known to describe the same session. + prop, err = property(ctx, session, sessionUser) + if err != nil { + return 0, false + } + var owner struct { + UID uint32 + Path dbus.ObjectPath + } + if err := prop.Store(&owner); err != nil { + return 0, false + } + + return owner.UID, true +} + +// property reads one property under ctx, name being the interface.member form +// godbus takes. It is what GetProperty does, with a deadline the caller owns. +func property(ctx context.Context, obj dbus.BusObject, name string) (dbus.Variant, error) { + idx := strings.LastIndex(name, ".") + if idx < 0 || idx+1 == len(name) { + return dbus.Variant{}, fmt.Errorf("invalid property name %q", name) + } + var v dbus.Variant + err := obj.CallWithContext(ctx, propertiesGet, 0, name[:idx], name[idx+1:]).Store(&v) + return v, err +} + +func stringProperty(ctx context.Context, obj dbus.BusObject, name string) (string, bool) { + prop, err := property(ctx, obj, name) + if err != nil { + return "", false + } + s, ok := prop.Value().(string) + return s, ok +} + +func boolProperty(ctx context.Context, obj dbus.BusObject, name string) (bool, bool) { + prop, err := property(ctx, obj, name) + if err != nil { + return false, false + } + b, ok := prop.Value().(bool) + return b, ok +} diff --git a/client/internal/ipcauth/consoleuser_other.go b/client/internal/ipcauth/consoleuser_other.go new file mode 100644 index 000000000..68e3f076a --- /dev/null +++ b/client/internal/ipcauth/consoleuser_other.go @@ -0,0 +1,13 @@ +//go:build !linux && !darwin && !freebsd && !windows + +package ipcauth + +// isConsoleUser has no meaning on a platform that exposes no console-user +// lookup, where nobody is ever at a console. +// +// Mobile is not built from here: ios satisfies darwin and android satisfies +// linux, so both take those lookups, which are present but never find a GUI +// session or a seat. +func isConsoleUser(Identity) bool { + return false +} diff --git a/client/internal/ipcauth/consoleuser_test.go b/client/internal/ipcauth/consoleuser_test.go new file mode 100644 index 000000000..c5d567f5e --- /dev/null +++ b/client/internal/ipcauth/consoleuser_test.go @@ -0,0 +1,54 @@ +package ipcauth + +import "testing" + +// An identity the kernel never vouched for must not reach a console lookup at +// all: the zero Identity carries uid 0, which a platform lookup would +// otherwise compare against a root session. +func TestIsConsoleUserRejectsUnattestedIdentity(t *testing.T) { + for _, tc := range []struct { + name string + id Identity + }{ + {"zero identity", Identity{}}, + {"unattested uid 0", Identity{UID: 0}}, + {"unattested uid", Identity{UID: 1000}}, + {"unattested sid", Identity{SID: "S-1-5-21-1-2-3-1001"}}, + } { + t.Run(tc.name, func(t *testing.T) { + if IsConsoleUser(tc.id) { + t.Fatal("an identity the kernel did not vouch for was reported as a console user") + } + }) + } +} + +// A lookup that panics must read as "cannot confirm". The daemon installs no +// gRPC recovery interceptor, so without this the panic ends the process from +// inside the per-RPC authorization path. +func TestGuardConsoleLookupContainsAPanic(t *testing.T) { + id := KnownForTest(Identity{UID: 1000}) + + if guardConsoleLookup(id, func(Identity) bool { + panic("pretending purego could not map a signature") + }) { + t.Fatal("a panicking lookup reported the caller as being at the console") + } +} + +// The guard must not swallow a real answer on its way out. +func TestGuardConsoleLookupPassesTheAnswerThrough(t *testing.T) { + id := KnownForTest(Identity{UID: 1000}) + + if !guardConsoleLookup(id, func(got Identity) bool { + if got.UID != id.UID { + t.Fatalf("lookup received uid %d, want %d", got.UID, id.UID) + } + return true + }) { + t.Fatal("a lookup that found the caller at the console reported false") + } + if guardConsoleLookup(id, func(Identity) bool { return false }) { + t.Fatal("a lookup that found nobody reported true") + } +} diff --git a/client/internal/ipcauth/consoleuser_windows.go b/client/internal/ipcauth/consoleuser_windows.go new file mode 100644 index 000000000..97943a1c7 --- /dev/null +++ b/client/internal/ipcauth/consoleuser_windows.go @@ -0,0 +1,40 @@ +package ipcauth + +import ( + "golang.org/x/sys/windows" +) + +// isConsoleUser reports whether id is the user logged into the active Windows +// console session. +// +// Returns false when there is no active console session, the session has no +// logged-in user, or any lookup fails. +func isConsoleUser(id Identity) bool { + // A caller with no SID is not a Windows principal and has nothing to + // compare against the console session's token. + if !id.IsWindows() { + return false + } + + sessionID := windows.WTSGetActiveConsoleSessionId() + if sessionID == 0xFFFFFFFF { + return false + } + + var token windows.Token + if err := windows.WTSQueryUserToken(sessionID, &token); err != nil { + return false + } + defer token.Close() + + console, err := identityFromToken(token) + if err != nil { + return false + } + + // The console session's token and the caller's token carry the same SID for + // the same account, so the SID is what the two identities share. Elevation + // is deliberately not compared: whether the caller's shell is elevated says + // nothing about who is sitting at the console. + return console.SID != "" && console.SID == id.SID +} diff --git a/client/internal/profilemanager/migration.go b/client/internal/profilemanager/migration.go index a53d5aff3..ec9f8a954 100644 --- a/client/internal/profilemanager/migration.go +++ b/client/internal/profilemanager/migration.go @@ -177,7 +177,8 @@ func undoMoves(moved []movedFile) { } // stampActiveUserDir records the owner of every unowned profile in the -// directory of the account the active profile state names. +// directory of the account the active profile state names and the default +// profile. // // That name is the one lossless input the old layout left behind. Resolving it // forward, from name to uid, avoids reversing a sanitized directory name, which @@ -198,21 +199,38 @@ func (s *ServiceManager) stampActiveUserDir(profiles []Profile, active *ActivePr } dir := sanitizeProfileName(active.Username) + if dir == "" { + log.Warnf("account %q leaves nothing after sanitizing, so its per-username profiles stay unowned", active.Username) + } + for i := range profiles { p := &profiles[i] - if len(p.Owners) > 0 || p.LegacyUserDir != dir { + if len(p.Owners) > 0 || !takesActiveAccountOwner(p, dir) { continue } if err := stampPrincipal(p.Path, principal); err != nil { log.Warnf("leaving %s unowned, its owner could not be recorded: %v", p.Path, err) continue } - log.Infof("recorded %s as the owner of %s, the directory it sits in is that account's", principal, p.Path) + log.Infof("recorded %s as the owner of %s, the account the active profile state names", principal, p.Path) } return nil } +// takesActiveAccountOwner reports whether an unowned profile should be stamped +// with the active account's principal, dir being the legacy directory name that +// account produced. +// +// An empty dir is the absence of a directory, not a directory whose name is +// empty, so nothing matches it. +func takesActiveAccountOwner(p *Profile, dir string) bool { + if dir != "" && p.LegacyUserDir == dir { + return true + } + return p.ID == defaultProfileName && !defaultProfileClaimDisabled() +} + // principalForUser turns a resolved account into an owner principal. os/user // reports a numeric id on Unix and a SID on Windows, which is what tells the // two kinds apart without a build tag. diff --git a/client/internal/profilemanager/migration_test.go b/client/internal/profilemanager/migration_test.go index 1a9c53385..597cb291e 100644 --- a/client/internal/profilemanager/migration_test.go +++ b/client/internal/profilemanager/migration_test.go @@ -289,3 +289,73 @@ func TestMigrate_SkipsAProfileItCannotStamp(t *testing.T) { assert.Equal(t, "null", string(data), "the profile it could not stamp is untouched") }) } + +// TestTakesActiveAccountOwner pins which profiles migration hands the active +// account's principal to. +func TestTakesActiveAccountOwner(t *testing.T) { + for _, tc := range []struct { + name string + dir string + profile Profile + disabled bool + want bool + }{ + { + name: "profile in the account's own directory", + dir: "alice", + profile: Profile{ID: "work", LegacyUserDir: "alice"}, + want: true, + }, + { + name: "profile in another account's directory", + dir: "alice", + profile: Profile{ID: "work", LegacyUserDir: "bob"}, + want: false, + }, + { + name: "default profile", + dir: "alice", + profile: Profile{ID: defaultProfileName}, + want: true, + }, + { + name: "default profile with the claim disabled", + dir: "alice", + profile: Profile{ID: defaultProfileName}, + disabled: true, + want: false, + }, + { + name: "default profile when the account has no directory name", + dir: "", + profile: Profile{ID: defaultProfileName}, + want: true, + }, + { + name: "default profile when the account has no directory name and the claim is disabled", + dir: "", + profile: Profile{ID: defaultProfileName}, + disabled: true, + want: false, + }, + { + name: "shared-directory profile when the account has no directory name", + dir: "", + profile: Profile{ID: "work"}, + want: false, + }, + { + name: "profile in a real directory when the account has no directory name", + dir: "", + profile: Profile{ID: "work", LegacyUserDir: "alice"}, + want: false, + }, + } { + t.Run(tc.name, func(t *testing.T) { + if tc.disabled { + t.Setenv(EnvDisableDefaultProfileClaim, "true") + } + assert.Equal(t, tc.want, takesActiveAccountOwner(&tc.profile, tc.dir)) + }) + } +} diff --git a/client/internal/profilemanager/profilemanager.go b/client/internal/profilemanager/profilemanager.go index 0ee037a3c..b3d287a48 100644 --- a/client/internal/profilemanager/profilemanager.go +++ b/client/internal/profilemanager/profilemanager.go @@ -48,10 +48,7 @@ func (p *Profile) AccessibleBy(id ipcauth.Identity) bool { return true } if len(p.Owners) == 0 { - // A profile in a per-username directory belonged to a use, unowned fails - // closed. The account named by the directory reclaims it on their next - // lookup or until claimed by a privileged caller. - return p.LegacyUserDir == "" && p.ID == DefaultProfileName + return false } return p.Owners[0].Matches(id) } diff --git a/client/internal/profilemanager/service.go b/client/internal/profilemanager/service.go index dc365c325..dccbf0693 100644 --- a/client/internal/profilemanager/service.go +++ b/client/internal/profilemanager/service.go @@ -22,6 +22,11 @@ import ( "github.com/netbirdio/netbird/util" ) +// EnvDisableDefaultProfileClaim turns off the console-user claim of an unowned +// default profile. The profile then stays unowned until a privileged caller +// records an owner. +const EnvDisableDefaultProfileClaim = "NB_DISABLE_DEFAULT_PROFILE_CLAIM" + var ( oldDefaultConfigPathDir = "" oldDefaultConfigPath = "" @@ -596,6 +601,7 @@ func (s *ServiceManager) loadAllProfilesForIdentity(userID ipcauth.Identity) ([] return nil, err } + s.claimDefaultProfileIfNeeded(allProfiles, userID) s.claimLegacyProfiles(allProfiles, userID) accessible := make([]Profile, 0, len(allProfiles)) @@ -659,6 +665,67 @@ func (s *ServiceManager) claimLegacyProfiles(profiles []Profile, id ipcauth.Iden } } +func (s *ServiceManager) claimDefaultProfileIfNeeded(profiles []Profile, id ipcauth.Identity) { + if !id.Known() || ipcauth.IsPrivilegedCaller(id) { + return + } + + var unowned bool + var p *Profile + for i := range profiles { + p = &profiles[i] + if p.ID == defaultProfileName && len(p.Owners) == 0 { + unowned = true + break + } + } + + if unowned && !defaultProfileClaimDisabled() && isConsoleUser(id) { + principal := ipcauth.OwnerPrincipalForIdentity(id) + parsed, ok := ipcauth.ParsePrincipal(principal) + if !ok { + log.Warnf("not claiming default profile, %q is not a usable owner", principal) + return + } + if err := StampOwner(p.Path, id); err != nil { + log.Warnf("could not claim default profile %s for %#v: %v", p.Path, id, err) + return + } + p.Owners = []ipcauth.Principal{parsed} + log.Infof("claimed default profile %s for %s", p.Path, principal) + } +} + +// isConsoleUser is a variable so a test can decide whether a caller is at the +// console without the machine running the test having a seat of its own. +var isConsoleUser = ipcauth.IsConsoleUser + +// logDefaultClaimDisabledOrError keeps the notice to once per process, since the claim +// path runs on every profile load. It also logs a parse failure once. +var logDefaultClaimDisabledOrError sync.Once + +// defaultProfileClaimDisabled reports whether the environment turns off the +// console-user claim of the default profile. +func defaultProfileClaimDisabled() bool { + val := os.Getenv(EnvDisableDefaultProfileClaim) + if val == "" { + return false + } + disabled, err := strconv.ParseBool(val) + if err != nil { + logDefaultClaimDisabledOrError.Do(func() { + log.Warnf("failed to parse %s: %v", EnvDisableDefaultProfileClaim, err) + }) + return false + } + if disabled { + logDefaultClaimDisabledOrError.Do(func() { + log.Infof("%s is set, the default profile stays unowned and reachable only by a privileged caller until an owner is recorded another way", EnvDisableDefaultProfileClaim) + }) + } + return disabled +} + func hasUnownedLegacyProfile(profiles []Profile) bool { for i := range profiles { if profiles[i].LegacyUserDir != "" && len(profiles[i].Owners) == 0 { diff --git a/client/internal/profilemanager/service_test.go b/client/internal/profilemanager/service_test.go index de71190c5..cc7e0acec 100644 --- a/client/internal/profilemanager/service_test.go +++ b/client/internal/profilemanager/service_test.go @@ -291,24 +291,30 @@ func TestListProfiles_PrivilegedResolvesUnfiltered(t *testing.T) { }) } -func TestListProfiles_OnlyTheDefaultFailsOpenWhenUnowned(t *testing.T) { +func TestListProfiles_UnownedProfilesArePrivilegedOnly(t *testing.T) { withTestSM(t, func(sm *ServiceManager, _ ipcauth.Identity) { + // Nobody at the console, so the claim cannot stamp an owner partway + // through and change what the assertions below are looking at, whatever + // the machine running the test happens to look like. + stubConsoleUser(t, false) + unowned, err := sm.AddProfile("unowned", nil) require.NoError(t, err) alice := ipcauth.KnownForTest(ipcauth.Identity{UID: 4242}) got, err := sm.ListProfiles(alice) require.NoError(t, err) - assert.Contains(t, profileIDs(got), defaultProfileName, - "a fresh install has to be usable before anything is claimed") + assert.NotContains(t, profileIDs(got), defaultProfileName, + "the default profile has no exemption, being claimed is what opens it") assert.NotContains(t, profileIDs(got), unowned.ID.String(), - "every other profile needs an owner before anyone can address it") + "every profile needs an owner before anyone can address it") root := ipcauth.KnownForTest(ipcauth.Identity{UID: 0}) got, err = sm.ListProfiles(root) require.NoError(t, err) - assert.Contains(t, profileIDs(got), unowned.ID.String(), - "root still reaches it, which is how it gets assigned") + assert.Contains(t, profileIDs(got), defaultProfileName, + "root still reaches both, which is how an unowned profile gets assigned") + assert.Contains(t, profileIDs(got), unowned.ID.String()) nobody, err := sm.ListProfiles(ipcauth.Identity{}) require.NoError(t, err) @@ -689,6 +695,27 @@ func TestListProfiles_ClaimKeepsFieldsThisVersionDoesNotModel(t *testing.T) { }) } +// stubConsoleUser replaces the console lookup, so the default-profile claim can +// be exercised without the machine running the test having a seat of its own. +func stubConsoleUser(t *testing.T, atConsole bool) { + t.Helper() + orig := isConsoleUser + isConsoleUser = func(ipcauth.Identity) bool { return atConsole } + t.Cleanup(func() { isConsoleUser = orig }) +} + +func TestClaimDefaultProfile_ConsoleUserClaimsIt(t *testing.T) { + withLegacyLayout(t, func(sm *ServiceManager, _ string) { + stubConsoleUser(t, true) + + alice := ipcauth.KnownForTest(ipcauth.Identity{UID: 4242}) + _, err := sm.ListProfiles(alice) + require.NoError(t, err) + assert.Equal(t, []string{"uid:4242"}, readOwners(t, DefaultConfigPath), + "the first caller at the console closes the window the default profile is open in") + }) +} + func TestSetProfileField_ReplacesAKeySpelledInAnotherCase(t *testing.T) { withLegacyLayout(t, func(sm *ServiceManager, configDir string) { path := writeLegacyProfile(t, configDir, "alice", "work", map[string]any{ @@ -760,3 +787,41 @@ func TestSetProfileField_KeepsKeysItWasNotAskedToWrite(t *testing.T) { assert.Len(t, doc, len(unknown)+3, "with nothing else added") }) } + +func TestClaimDefaultProfile_CallerAwayFromTheConsoleDoesNotClaimIt(t *testing.T) { + withLegacyLayout(t, func(sm *ServiceManager, _ string) { + stubConsoleUser(t, false) + + alice := ipcauth.KnownForTest(ipcauth.Identity{UID: 4242}) + _, err := sm.ListProfiles(alice) + require.NoError(t, err) + assert.Empty(t, readOwners(t, DefaultConfigPath), + "a local caller who is not at the console must not take the machine's profile") + }) +} + +func TestClaimDefaultProfile_DisableEnvWithholdsTheClaim(t *testing.T) { + withLegacyLayout(t, func(sm *ServiceManager, _ string) { + stubConsoleUser(t, true) + t.Setenv(EnvDisableDefaultProfileClaim, "true") + + alice := ipcauth.KnownForTest(ipcauth.Identity{UID: 4242}) + _, err := sm.ListProfiles(alice) + require.NoError(t, err) + assert.Empty(t, readOwners(t, DefaultConfigPath), + "the flag withholds the claim even from a caller who would otherwise get it") + }) +} + +func TestClaimDefaultProfile_UnparseableDisableEnvLeavesTheClaimOn(t *testing.T) { + withLegacyLayout(t, func(sm *ServiceManager, _ string) { + stubConsoleUser(t, true) + t.Setenv(EnvDisableDefaultProfileClaim, "yes please") + + alice := ipcauth.KnownForTest(ipcauth.Identity{UID: 4242}) + _, err := sm.ListProfiles(alice) + require.NoError(t, err) + assert.Equal(t, []string{"uid:4242"}, readOwners(t, DefaultConfigPath), + "a typo must not be what turns a safety mechanism off") + }) +}