[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
This commit is contained in:
Theodor Midtlien
2026-09-17 11:20:13 +02:00
committed by GitHub
parent 52b16e7a5c
commit 90052cbefb
12 changed files with 712 additions and 13 deletions
+21 -3
View File
@@ -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.
@@ -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))
})
}
}
@@ -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)
}
+67
View File
@@ -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 {
+71 -6
View File
@@ -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")
})
}