From c239352a7febab71565b8870d471ad263ab6f870 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zolt=C3=A1n=20Papp?= Date: Thu, 27 Aug 2026 16:01:01 +0200 Subject: [PATCH] [client] Fail closed in InvokingUser when the sudo user lookup fails A previous change made baseConfigDir fail closed when SUDO_USER cannot be resolved, but InvokingUser still fell through to user.Current(). Those two guards disagreed: the active-profile mirror and the email state refused to read root's directory, while every profile-path caller happily resolved as root. The consequence of a transient NSS failure under sudo was that Profile.FilePath resolved through getConfigDirForUser("root"), creating /var/lib/netbird/root and reading the profile JSON from there, and the CLI sent Username "root" to the daemon in SetConfig and ListProfiles, so the daemon resolved the same phantom namespace. The invoking user was silently moved onto a root-owned profile instead of being told the lookup failed. Fail closed at the single source of the fallback. getConfigDirForUser is left alone on purpose: it is a pure path helper that also serves daemon-supplied usernames, and under sudo with a successful lookup it must still create the invoking user's own profile directory. --- .../internal/profilemanager/invoking_user.go | 7 ++++++ .../profilemanager/invoking_user_test.go | 24 +++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/client/internal/profilemanager/invoking_user.go b/client/internal/profilemanager/invoking_user.go index a051a0da4..c86a6ce43 100644 --- a/client/internal/profilemanager/invoking_user.go +++ b/client/internal/profilemanager/invoking_user.go @@ -28,6 +28,13 @@ func InvokingUser() (*user.User, error) { if u, ok := sudoInvokingUser(); ok { return u, nil } + // Fail closed instead of falling through to root: every caller feeds this + // username into profile-path resolution, so a lookup failure would resolve + // (and create) a root-owned profile namespace and switch the daemon onto it + // behind the invoking user's back. + if sudoActive() { + return nil, fmt.Errorf("resolve sudo invoking user %q: refusing to fall back to root", os.Getenv(envSudoUser)) + } return user.Current() } diff --git a/client/internal/profilemanager/invoking_user_test.go b/client/internal/profilemanager/invoking_user_test.go index 95a636d80..54c8ad8fd 100644 --- a/client/internal/profilemanager/invoking_user_test.go +++ b/client/internal/profilemanager/invoking_user_test.go @@ -56,6 +56,30 @@ func TestSudoInvokingUserResolvesInvokingUser(t *testing.T) { assert.False(t, IsPlainRoot()) } +func TestInvokingUserFailsClosedWhenSudoLookupFails(t *testing.T) { + fakeSudo(t, filepath.Join("/home", "misha")) + lookupUser = func(string) (*user.User, error) { return nil, errors.New("nss unavailable") } + + got, err := InvokingUser() + require.Error(t, err) + assert.Nil(t, got, "must not resolve to the root process user") +} + +func TestProfileFilePathFailsClosedWhenSudoLookupFails(t *testing.T) { + profilesRoot := t.TempDir() + fakeSudo(t, filepath.Join("/home", "misha")) + lookupUser = func(string) (*user.User, error) { return nil, errors.New("nss unavailable") } + + origDir := DefaultConfigPathDir + DefaultConfigPathDir = profilesRoot + t.Cleanup(func() { DefaultConfigPathDir = origDir }) + + p := &Profile{ID: "0123456789abcdef0123456789abcdef"} + _, err := p.FilePath() + require.Error(t, err) + assertNoEntries(t, profilesRoot) +} + func TestSudoActiveSurvivesLookupFailure(t *testing.T) { fakeSudo(t, filepath.Join("/home", "misha")) lookupUser = func(string) (*user.User, error) { return nil, errors.New("nss unavailable") }