diff --git a/client/cmd/debug.go b/client/cmd/debug.go index 893b1e248..98fe53626 100644 --- a/client/cmd/debug.go +++ b/client/cmd/debug.go @@ -3,7 +3,6 @@ package cmd import ( "context" "fmt" - "os/user" "strings" "time" @@ -114,7 +113,7 @@ func debugConfigDump(cmd *cobra.Command, _ []string) error { if err != nil { return fmt.Errorf("get active profile: %v", err) } - currUser, err := user.Current() + currUser, err := profilemanager.InvokingUser() if err != nil { return fmt.Errorf("get current user: %v", err) } diff --git a/client/cmd/login.go b/client/cmd/login.go index 6aa019896..f703b32c4 100644 --- a/client/cmd/login.go +++ b/client/cmd/login.go @@ -4,7 +4,6 @@ import ( "context" "fmt" "os" - "os/user" "strings" log "github.com/sirupsen/logrus" @@ -53,7 +52,7 @@ var loginCmd = &cobra.Command{ // nolint ctx = context.WithValue(ctx, system.DeviceNameCtxKey, hostName) } - username, err := user.Current() + username, err := profilemanager.InvokingUser() if err != nil { return fmt.Errorf("get current user: %v", err) } @@ -74,7 +73,7 @@ var loginCmd = &cobra.Command{ if providedSetupKey != "" { return fmt.Errorf("--extend cannot be combined with a setup key; setup keys can only enrol new peers") } - if err := doExtendSession(ctx, cmd); err != nil { + if err := doExtendSession(ctx, cmd, activeProf); err != nil { return fmt.Errorf("extend session failed: %v", err) } return nil @@ -176,7 +175,7 @@ func doDaemonLogin(ctx context.Context, cmd *cobra.Command, providedSetupKey str // (browser + verification URL) and the resulting JWT is forwarded to the // management server's ExtendAuthSession RPC. The tunnel stays up // throughout — no Down/Up, no network-map resync. -func doExtendSession(ctx context.Context, cmd *cobra.Command) error { +func doExtendSession(ctx context.Context, cmd *cobra.Command, activeProf *profilemanager.Profile) error { conn, err := DialClientGRPCServer(ctx, daemonAddr) if err != nil { //nolint @@ -190,14 +189,12 @@ func doExtendSession(ctx context.Context, cmd *cobra.Command) error { // the CLI runs in the user's session, the daemon does not: tell it what we can see req := &proto.RequestExtendAuthSessionRequest{HasGraphicalSession: util.HasGraphicalSession()} - // Pre-fill the IdP login hint from the active profile so the user + // Pre-fill the IdP login hint from the resolved profile so the user // doesn't have to retype their email. Best-effort: we still proceed // without a hint if the lookup fails. pm := profilemanager.NewProfileManager() - if active, perr := pm.GetActiveProfile(); perr == nil { - if profState, sperr := pm.GetProfileState(active.ID); sperr == nil && profState.Email != "" { - req.Hint = &profState.Email - } + if profState, perr := pm.GetProfileState(activeProf.ID); perr == nil && profState.Email != "" { + req.Hint = &profState.Email } startResp, err := client.RequestExtendAuthSession(ctx, req) @@ -235,9 +232,11 @@ func getActiveProfile(ctx context.Context, pm *profilemanager.ProfileManager, pr // switch profile if provided if profileName != "" { - if err := switchProfileOnDaemon(ctx, pm, profileName, username); err != nil { + prof, err := switchProfileOnDaemon(ctx, pm, profileName, username) + if err != nil { return nil, fmt.Errorf("switch profile: %v", err) } + return prof, nil } activeProf, err := pm.GetActiveProfile() @@ -251,20 +250,19 @@ func getActiveProfile(ctx context.Context, pm *profilemanager.ProfileManager, pr return activeProf, nil } -func switchProfileOnDaemon(ctx context.Context, pm *profilemanager.ProfileManager, handle string, username string) error { +func switchProfileOnDaemon(ctx context.Context, pm *profilemanager.ProfileManager, handle string, username string) (*profilemanager.Profile, error) { resolvedID, err := switchProfile(ctx, handle, username) if err != nil { - return fmt.Errorf("switch profile on daemon: %v", err) + return nil, fmt.Errorf("switch profile on daemon: %v", err) } if err := pm.SwitchProfile(resolvedID); err != nil { - return fmt.Errorf("switch profile: %v", err) + return nil, fmt.Errorf("switch profile: %v", err) } conn, err := DialClientGRPCServer(ctx, daemonAddr) if err != nil { - log.Errorf("failed to connect to service CLI interface %v", err) - return err + return nil, fmt.Errorf("connect to service CLI interface: %w", err) } defer conn.Close() @@ -272,17 +270,17 @@ func switchProfileOnDaemon(ctx context.Context, pm *profilemanager.ProfileManage status, err := client.Status(ctx, &proto.StatusRequest{}) if err != nil { - return fmt.Errorf("unable to get daemon status: %v", err) + return nil, fmt.Errorf("unable to get daemon status: %v", err) } if status.Status == string(internal.StatusConnected) { if _, err := client.Down(ctx, &proto.DownRequest{}); err != nil { log.Errorf("call service down method: %v", err) - return err + return nil, err } } - return nil + return &profilemanager.Profile{ID: resolvedID}, nil } // switchProfile asks the daemon to switch to the profile identified by diff --git a/client/cmd/logout.go b/client/cmd/logout.go index dcd7b5075..cf2a4e446 100644 --- a/client/cmd/logout.go +++ b/client/cmd/logout.go @@ -3,11 +3,11 @@ package cmd import ( "context" "fmt" - "os/user" "time" "github.com/spf13/cobra" + "github.com/netbirdio/netbird/client/internal/profilemanager" "github.com/netbirdio/netbird/client/proto" ) @@ -37,7 +37,7 @@ var logoutCmd = &cobra.Command{ if profileName != "" { req.ProfileName = &profileName - currUser, err := user.Current() + currUser, err := profilemanager.InvokingUser() if err != nil { return fmt.Errorf("get current user: %v", err) } diff --git a/client/cmd/profile.go b/client/cmd/profile.go index 268034e70..2d6653537 100644 --- a/client/cmd/profile.go +++ b/client/cmd/profile.go @@ -4,7 +4,6 @@ import ( "context" "errors" "fmt" - "os/user" "strings" "text/tabwriter" "time" @@ -97,7 +96,7 @@ func listProfilesFunc(cmd *cobra.Command, _ []string) error { } defer conn.Close() - currUser, err := user.Current() + currUser, err := profilemanager.InvokingUser() if err != nil { return fmt.Errorf("get current user: %w", err) } @@ -138,7 +137,7 @@ func addProfileFunc(cmd *cobra.Command, args []string) error { return err } - currUser, err := user.Current() + currUser, err := profilemanager.InvokingUser() if err != nil { return fmt.Errorf("get current user: %w", err) } @@ -179,7 +178,7 @@ func renameProfileFunc(cmd *cobra.Command, args []string) error { } defer conn.Close() - currUser, err := user.Current() + currUser, err := profilemanager.InvokingUser() if err != nil { return fmt.Errorf("get current user: %w", err) } @@ -233,7 +232,7 @@ func removeProfileFunc(cmd *cobra.Command, args []string) error { } defer conn.Close() - currUser, err := user.Current() + currUser, err := profilemanager.InvokingUser() if err != nil { return fmt.Errorf("get current user: %w", err) } @@ -261,7 +260,7 @@ func selectProfileFunc(cmd *cobra.Command, args []string) error { profileManager := profilemanager.NewProfileManager() handle := args[0] - currUser, err := user.Current() + currUser, err := profilemanager.InvokingUser() if err != nil { return fmt.Errorf("get current user: %w", err) } diff --git a/client/cmd/up.go b/client/cmd/up.go index 5bc41a964..9cf5eea26 100644 --- a/client/cmd/up.go +++ b/client/cmd/up.go @@ -2,10 +2,10 @@ package cmd import ( "context" + "errors" "fmt" "net" "net/netip" - "os/user" "runtime" "strings" "time" @@ -48,6 +48,8 @@ const ( profileNameDesc = "profile name to use for the login. If not specified, the last used profile will be used." ) +var errDaemonActiveProfileUnsupported = errors.New("daemon does not support active profile lookup") + var ( foregroundMode bool dnsLabels []string @@ -122,23 +124,25 @@ func upFunc(cmd *cobra.Command, args []string) error { pm := profilemanager.NewProfileManager() - username, err := user.Current() + username, err := profilemanager.InvokingUser() if err != nil { return fmt.Errorf("get current user: %v", err) } + var activeProf *profilemanager.Profile var profileSwitched bool // switch profile if provided if profileName != "" { - if err := switchOrCreateProfile(cmd.Context(), pm, profileName, username.Username); err != nil { + activeProf, err = switchOrCreateProfile(cmd.Context(), pm, profileName, username.Username) + if err != nil { return fmt.Errorf("switch profile: %v", err) } profileSwitched = true - } - - activeProf, err := pm.GetActiveProfile() - if err != nil { - return fmt.Errorf("get active profile: %v", err) + } else { + activeProf, err = pm.GetActiveProfile() + if err != nil { + return fmt.Errorf("get active profile: %v", err) + } } if foregroundMode { @@ -150,13 +154,15 @@ func upFunc(cmd *cobra.Command, args []string) error { // switchOrCreateProfile switches the active profile to the one identified by // handle, creating it first when it does not exist yet. This restores the // pre-0.73 behaviour where `netbird up --profile ` auto-creates a -// missing profile instead of failing. -func switchOrCreateProfile(ctx context.Context, pm *profilemanager.ProfileManager, handle, username string) error { +// missing profile instead of failing. Returns the daemon-resolved profile so +// callers act on it directly instead of re-reading the local state, which is +// not updated under sudo. +func switchOrCreateProfile(ctx context.Context, pm *profilemanager.ProfileManager, handle, username string) (*profilemanager.Profile, error) { resolvedID, err := switchProfile(ctx, handle, username) if err != nil { st, ok := gstatus.FromError(err) if !ok || st.Code() != codes.NotFound { - return err + return nil, err } // Don't fail immediately on a create error: a concurrent run may // have created the profile between the NotFound above and this @@ -165,16 +171,16 @@ func switchOrCreateProfile(ctx context.Context, pm *profilemanager.ProfileManage _, createErr := createProfile(ctx, handle, username) if resolvedID, err = switchProfile(ctx, handle, username); err != nil { if createErr != nil { - return fmt.Errorf("create profile: %w", createErr) + return nil, fmt.Errorf("create profile: %w", createErr) } - return err + return nil, err } } if err := pm.SwitchProfile(resolvedID); err != nil { - return err + return nil, err } - return nil + return &profilemanager.Profile{ID: resolvedID}, nil } // createProfile dials the daemon and creates a new profile with the given @@ -302,6 +308,30 @@ func runInDaemonMode(ctx context.Context, cmd *cobra.Command, pm *profilemanager return fmt.Errorf("unable to get daemon status: %v", err) } + // Under sudo the invoking user's local active-profile mirror is never + // written (the SwitchProfile write is a no-op), and plain root has no + // invoking user at all — so the mirror read into activeProf above is stale + // or defaulted and must not drive the daemon. With no --profile to make the + // choice explicit, take the profile the daemon already holds for this user + // instead: it stays on the user's current profile rather than silently + // switching to the mirror's default, and refuses when the daemon is on + // another user's profile. + if profileName == "" && !profilemanager.MirrorIsAuthoritative() { + u, err := profilemanager.InvokingUser() + if err != nil { + return fmt.Errorf("get current user: %v", err) + } + resolved, err := daemonActiveProfileForUser(ctx, client, u.Username) + switch { + case errors.Is(err, errDaemonActiveProfileUnsupported): + log.Warnf("keeping the locally resolved profile: %v", err) + case err != nil: + return err + default: + activeProf = resolved + } + } + if status.Status == string(internal.StatusConnected) { if !profileSwitched { cmd.Println("Already connected") @@ -314,7 +344,7 @@ func runInDaemonMode(ctx context.Context, cmd *cobra.Command, pm *profilemanager } } - username, err := user.Current() + username, err := profilemanager.InvokingUser() if err != nil { return fmt.Errorf("get current user: %v", err) } @@ -881,3 +911,31 @@ func isValidAddrPort(input string) bool { _, err := netip.ParseAddrPort(input) return err == nil } + +// daemonActiveProfileForUser returns the profile the daemon currently holds for +// username, for the no --profile case where the local mirror is not +// authoritative (sudo or plain root). It returns that profile when the daemon +// owns it for this user or when the profile is unowned (empty username, as on a +// fresh install), so the caller acts on the daemon's real state instead of the +// stale mirror. It denies with a --profile hint when the daemon is on another +// user's profile, when the lookup fails, or when the daemon reports no active +// profile. Returns errDaemonActiveProfileUnsupported when the daemon predates +// the RPC; the caller keeps the mirror-derived profile in that case. +func daemonActiveProfileForUser(ctx context.Context, client proto.DaemonServiceClient, username string) (*profilemanager.Profile, error) { + active, err := client.GetActiveProfile(ctx, &proto.GetActiveProfileRequest{}) + if err != nil { + if st, ok := gstatus.FromError(err); ok && st.Code() == codes.Unimplemented { + return nil, fmt.Errorf("%w: %v", errDaemonActiveProfileUnsupported, err) + } + return nil, fmt.Errorf("pass --profile to choose the profile explicitly: the daemon's active profile could not be verified: %v", err) + } + if active.GetId() == "" { + return nil, fmt.Errorf("pass --profile to choose the profile explicitly: the daemon reported no active profile") + } + if active.GetUsername() != "" && active.GetUsername() != username { + return nil, fmt.Errorf( + "pass --profile to choose the profile explicitly: the daemon's active profile is %q (user %q) but this invocation runs for %q", + active.GetProfileName(), active.GetUsername(), username) + } + return &profilemanager.Profile{ID: profilemanager.ID(active.GetId())}, nil +} diff --git a/client/cmd/up_test.go b/client/cmd/up_test.go new file mode 100644 index 000000000..9b5f9fbea --- /dev/null +++ b/client/cmd/up_test.go @@ -0,0 +1,88 @@ +package cmd + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "google.golang.org/grpc" + "google.golang.org/grpc/codes" + gstatus "google.golang.org/grpc/status" + + "github.com/netbirdio/netbird/client/internal/profilemanager" + "github.com/netbirdio/netbird/client/proto" +) + +type fakeActiveProfileClient struct { + proto.DaemonServiceClient + resp *proto.GetActiveProfileResponse + err error +} + +func (f *fakeActiveProfileClient) GetActiveProfile(_ context.Context, _ *proto.GetActiveProfileRequest, _ ...grpc.CallOption) (*proto.GetActiveProfileResponse, error) { + return f.resp, f.err +} + +func TestDaemonActiveProfileForUserReturnsOwnProfile(t *testing.T) { + client := &fakeActiveProfileClient{resp: &proto.GetActiveProfileResponse{Id: "default", ProfileName: "default", Username: "root"}} + prof, err := daemonActiveProfileForUser(context.Background(), client, "root") + require.NoError(t, err) + require.NotNil(t, prof) + assert.Equal(t, profilemanager.ID("default"), prof.ID) +} + +func TestDaemonActiveProfileForUserReturnsUnownedProfile(t *testing.T) { + client := &fakeActiveProfileClient{resp: &proto.GetActiveProfileResponse{Id: "default", ProfileName: "default", Username: ""}} + prof, err := daemonActiveProfileForUser(context.Background(), client, "root") + require.NoError(t, err) + require.NotNil(t, prof) + assert.Equal(t, profilemanager.ID("default"), prof.ID) +} + +func TestDaemonActiveProfileForUserKeepsDaemonProfileOverStaleMirror(t *testing.T) { + client := &fakeActiveProfileClient{resp: &proto.GetActiveProfileResponse{Id: "ab12", ProfileName: "work", Username: "misha"}} + prof, err := daemonActiveProfileForUser(context.Background(), client, "misha") + require.NoError(t, err) + require.NotNil(t, prof) + assert.Equal(t, profilemanager.ID("ab12"), prof.ID) +} + +func TestDaemonActiveProfileForUserRejectsOtherUsersProfile(t *testing.T) { + client := &fakeActiveProfileClient{resp: &proto.GetActiveProfileResponse{Id: "ab12", ProfileName: "work", Username: "misha"}} + prof, err := daemonActiveProfileForUser(context.Background(), client, "root") + require.Error(t, err) + assert.Nil(t, prof) + assert.Contains(t, err.Error(), "--profile") +} + +func TestDaemonActiveProfileForUserRejectsOtherUsersDefaultProfile(t *testing.T) { + client := &fakeActiveProfileClient{resp: &proto.GetActiveProfileResponse{Id: "default", ProfileName: "default", Username: "misha"}} + prof, err := daemonActiveProfileForUser(context.Background(), client, "root") + require.Error(t, err) + assert.Nil(t, prof) + assert.Contains(t, err.Error(), "--profile") +} + +func TestDaemonActiveProfileForUserRejectsLookupError(t *testing.T) { + client := &fakeActiveProfileClient{err: gstatus.Error(codes.Internal, "boom")} + prof, err := daemonActiveProfileForUser(context.Background(), client, "root") + require.Error(t, err) + assert.Nil(t, prof) + assert.Contains(t, err.Error(), "--profile") +} + +func TestDaemonActiveProfileForUserRejectsEmptyResponse(t *testing.T) { + client := &fakeActiveProfileClient{resp: &proto.GetActiveProfileResponse{}} + prof, err := daemonActiveProfileForUser(context.Background(), client, "root") + require.Error(t, err) + assert.Nil(t, prof) + assert.Contains(t, err.Error(), "--profile") +} + +func TestDaemonActiveProfileForUserKeepsMirrorWhenDaemonWithoutRPC(t *testing.T) { + client := &fakeActiveProfileClient{err: gstatus.Error(codes.Unimplemented, "unknown method")} + prof, err := daemonActiveProfileForUser(context.Background(), client, "root") + require.ErrorIs(t, err, errDaemonActiveProfileUnsupported) + assert.Nil(t, prof) +} diff --git a/client/internal/profilemanager/config.go b/client/internal/profilemanager/config.go index eacc6fd5f..e83cb4015 100644 --- a/client/internal/profilemanager/config.go +++ b/client/internal/profilemanager/config.go @@ -225,6 +225,12 @@ func getConfigDir() (string, error) { } configDir := filepath.Join(base, "netbird") + // Under sudo this is the invoking user's directory and strictly read-only: + // anything root creates in it would be root-owned and break the user's own + // runs. Reads of a missing directory fall through to defaults. + if sudoActive() { + return configDir, nil + } if err := os.MkdirAll(configDir, 0o755); err != nil { return "", err } @@ -232,6 +238,16 @@ func getConfigDir() (string, error) { } func baseConfigDir() (string, error) { + if u, ok := sudoInvokingUser(); ok { + return userBaseConfigDir(u) + } + // Fail closed instead of falling through to root's own config directory: + // reading root's active-profile and email state for what is actually the + // invoking user's invocation is the very confusion this resolution exists + // to prevent. + if sudoActive() { + return "", fmt.Errorf("resolve sudo invoking user %q: refusing to fall back to root's config directory", os.Getenv(envSudoUser)) + } if runtime.GOOS == "darwin" { if u, err := user.Current(); err == nil && u.HomeDir != "" { return filepath.Join(u.HomeDir, "Library", "Application Support"), nil diff --git a/client/internal/profilemanager/invoking_user.go b/client/internal/profilemanager/invoking_user.go new file mode 100644 index 000000000..c86a6ce43 --- /dev/null +++ b/client/internal/profilemanager/invoking_user.go @@ -0,0 +1,100 @@ +package profilemanager + +import ( + "fmt" + "os" + "os/user" + "path/filepath" + "runtime" + + log "github.com/sirupsen/logrus" +) + +const envSudoUser = "SUDO_USER" + +var ( + geteuid = os.Geteuid + lookupUser = user.Lookup +) + +// InvokingUser returns the user a CLI invocation acts for. Under sudo that is +// the user who ran sudo, not root: privileged flags force commands through +// sudo, and resolving profiles as root would silently switch the daemon to +// root's (default) profile instead of the invoking user's. Privilege decisions +// are not made here — those stay on the kernel credentials of the daemon +// connection, which SUDO_USER (a plain environment variable) can never +// influence; a forged value only selects a profile root could select anyway. +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() +} + +// IsPlainRoot reports that the process runs as root with no usable sudo +// context: there is no invoking user to act for, so per-user resolution falls +// back to root's own (empty) state. Callers use it to refuse ambiguous +// operations instead of silently acting on the wrong profile. +func IsPlainRoot() bool { + if geteuid() != 0 { + return false + } + _, ok := sudoInvokingUser() + return !ok +} + +// MirrorIsAuthoritative reports whether the invoking user's local +// active-profile mirror can be trusted as the profile selector. It cannot under +// sudo (writes to it are skipped, so it goes stale) or as plain root (there is +// no invoking user, so it falls back to root's own default). Callers use it to +// decide whether to read the profile from the mirror or from the daemon. +func MirrorIsAuthoritative() bool { + return !sudoActive() && !IsPlainRoot() +} + +// sudoInvokingUser resolves SUDO_USER when the process runs as root under +// sudo. Returns false whenever the sudo context is absent or unusable, in +// which case callers fall back to the process user. +func sudoInvokingUser() (*user.User, bool) { + if !sudoActive() { + return nil, false + } + name := os.Getenv(envSudoUser) + u, err := lookupUser(name) + if err != nil { + log.Warnf("sudo invoking user %q lookup: %v", name, err) + return nil, false + } + return u, true +} + +// sudoActive reports a sudo context from the environment alone: write-skip +// decisions key off it so a transient user lookup failure can never flip a +// run from read-only to writing root-owned files into the user's directory. +func sudoActive() bool { + if geteuid() != 0 { + return false + } + name := os.Getenv(envSudoUser) + return name != "" && name != "root" +} + +// userBaseConfigDir mirrors os.UserConfigDir for a user other than the process +// owner. Environment overrides (XDG_CONFIG_HOME) cannot be honoured here: under +// sudo the environment is root's, not the invoking user's. +func userBaseConfigDir(u *user.User) (string, error) { + if u.HomeDir == "" { + return "", fmt.Errorf("user %s has no home directory", u.Username) + } + if runtime.GOOS == "darwin" { + return filepath.Join(u.HomeDir, "Library", "Application Support"), nil + } + return filepath.Join(u.HomeDir, ".config"), nil +} diff --git a/client/internal/profilemanager/invoking_user_test.go b/client/internal/profilemanager/invoking_user_test.go new file mode 100644 index 000000000..54c8ad8fd --- /dev/null +++ b/client/internal/profilemanager/invoking_user_test.go @@ -0,0 +1,230 @@ +package profilemanager + +import ( + "errors" + "io/fs" + "os" + "os/user" + "path/filepath" + "runtime" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestInvokingUserFallsBackToProcessUser(t *testing.T) { + t.Setenv(envSudoUser, "") + + got, err := InvokingUser() + require.NoError(t, err) + + current, err := user.Current() + require.NoError(t, err) + assert.Equal(t, current.Username, got.Username) +} + +func TestSudoInvokingUserInactiveWithoutSudoContext(t *testing.T) { + t.Setenv(envSudoUser, "") + _, ok := sudoInvokingUser() + assert.False(t, ok) +} + +func TestSudoInvokingUserIgnoresRoot(t *testing.T) { + t.Setenv(envSudoUser, "root") + origEuid := geteuid + geteuid = func() int { return 0 } + t.Cleanup(func() { geteuid = origEuid }) + + _, ok := sudoInvokingUser() + assert.False(t, ok, "sudo from a root shell must not redirect anything") + assert.False(t, sudoActive()) + assert.True(t, IsPlainRoot()) +} + +func TestSudoInvokingUserResolvesInvokingUser(t *testing.T) { + fakeSudo(t, filepath.Join("/home", "misha")) + + u, ok := sudoInvokingUser() + require.True(t, ok) + assert.Equal(t, "misha", u.Username) + + got, err := InvokingUser() + require.NoError(t, err) + assert.Equal(t, "misha", got.Username) + + 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") } + + _, ok := sudoInvokingUser() + assert.False(t, ok) + assert.True(t, sudoActive()) + assert.True(t, IsPlainRoot()) +} + +func TestGetConfigDirUnderSudoIsReadOnly(t *testing.T) { + home := t.TempDir() + fakeSudo(t, home) + + base, err := baseConfigDir() + require.NoError(t, err) + if runtime.GOOS == "darwin" { + assert.Equal(t, filepath.Join(home, "Library", "Application Support"), base) + } else { + assert.Equal(t, filepath.Join(home, ".config"), base) + } + + dir, err := getConfigDir() + require.NoError(t, err) + assert.Equal(t, filepath.Join(base, "netbird"), dir) + assert.NoDirExists(t, dir) +} + +func TestBaseConfigDirFailsClosedWhenSudoLookupFails(t *testing.T) { + fakeSudo(t, filepath.Join("/home", "misha")) + lookupUser = func(string) (*user.User, error) { return nil, errors.New("nss unavailable") } + + _, err := baseConfigDir() + require.Error(t, err) + + _, err = getConfigDir() + require.Error(t, err) +} + +func TestSwitchProfileSkipsStateWriteUnderSudo(t *testing.T) { + home := t.TempDir() + fakeSudo(t, home) + + pm := NewProfileManager() + require.NoError(t, pm.SwitchProfile(defaultProfileName)) + assertNoEntries(t, home) +} + +func TestSetProfileStateSkipsWriteUnderSudo(t *testing.T) { + home := t.TempDir() + fakeSudo(t, home) + + pm := NewProfileManager() + require.NoError(t, pm.SetProfileState(defaultProfileName, &ProfileState{Email: "misha@example.com"})) + assertNoEntries(t, home) +} + +func TestRemoveProfileStateSkipsRemoveUnderSudo(t *testing.T) { + home := t.TempDir() + stateDir := filepath.Join(home, ".config", "netbird") + if runtime.GOOS == "darwin" { + stateDir = filepath.Join(home, "Library", "Application Support", "netbird") + } + require.NoError(t, os.MkdirAll(stateDir, 0o700)) + stateFile := filepath.Join(stateDir, "default.state.json") + require.NoError(t, os.WriteFile(stateFile, []byte(`{"email":"misha@example.com"}`), 0o600)) + + fakeSudo(t, home) + pm := NewProfileManager() + require.NoError(t, pm.RemoveProfileState("default")) + assert.FileExists(t, stateFile) +} + +func TestUserBaseConfigDir(t *testing.T) { + u := &user.User{Username: "misha", HomeDir: filepath.Join("/home", "misha")} + dir, err := userBaseConfigDir(u) + require.NoError(t, err) + if runtime.GOOS == "darwin" { + assert.Equal(t, filepath.Join(u.HomeDir, "Library", "Application Support"), dir) + } else { + assert.Equal(t, filepath.Join(u.HomeDir, ".config"), dir) + } + + _, err = userBaseConfigDir(&user.User{Username: "nohome"}) + require.Error(t, err) +} + +func TestIsPlainRoot(t *testing.T) { + t.Setenv(envSudoUser, "") + origEuid := geteuid + t.Cleanup(func() { geteuid = origEuid }) + + geteuid = func() int { return 1000 } + assert.False(t, IsPlainRoot()) + + geteuid = func() int { return 0 } + assert.True(t, IsPlainRoot()) +} + +func TestMirrorIsAuthoritative(t *testing.T) { + t.Setenv(envSudoUser, "") + origEuid := geteuid + t.Cleanup(func() { geteuid = origEuid }) + + geteuid = func() int { return 1000 } + assert.True(t, MirrorIsAuthoritative(), "a normal user's own mirror is authoritative") + + geteuid = func() int { return 0 } + assert.False(t, MirrorIsAuthoritative(), "plain root has no authoritative mirror") +} + +func TestMirrorIsAuthoritativeFalseUnderSudo(t *testing.T) { + fakeSudo(t, filepath.Join("/home", "misha")) + assert.False(t, MirrorIsAuthoritative(), "the sudo mirror is frozen, so it is not authoritative") +} + +func fakeSudo(t *testing.T, home string) { + t.Helper() + t.Setenv(envSudoUser, "misha") + + origEuid := geteuid + origLookup := lookupUser + origOverride := ConfigDirOverride + geteuid = func() int { return 0 } + lookupUser = func(name string) (*user.User, error) { + return &user.User{Username: name, Uid: "1234", Gid: "1234", HomeDir: home}, nil + } + ConfigDirOverride = "" + t.Cleanup(func() { + geteuid = origEuid + lookupUser = origLookup + ConfigDirOverride = origOverride + }) +} + +func assertNoEntries(t *testing.T, root string) { + t.Helper() + err := filepath.WalkDir(root, func(path string, _ fs.DirEntry, err error) error { + if err != nil { + return err + } + if path != root { + t.Errorf("unexpected entry created under %s: %s", root, path) + } + return nil + }) + require.NoError(t, err) +} diff --git a/client/internal/profilemanager/profilemanager.go b/client/internal/profilemanager/profilemanager.go index e25d493d5..d2ed92bc5 100644 --- a/client/internal/profilemanager/profilemanager.go +++ b/client/internal/profilemanager/profilemanager.go @@ -3,7 +3,6 @@ package profilemanager import ( "fmt" "os" - "os/user" "path/filepath" "strings" "sync" @@ -54,7 +53,7 @@ func (p *Profile) FilePath() (string, error) { return "", fmt.Errorf("invalid profile ID: %q", id) } - username, err := user.Current() + username, err := InvokingUser() if err != nil { return "", fmt.Errorf("failed to get current user: %w", err) } @@ -130,7 +129,7 @@ func (pm *ProfileManager) getActiveProfileState() ID { if err != nil { if !os.IsNotExist(err) { log.Warnf("failed to read active profile state: %v", err) - } else { + } else if !sudoActive() { if err := pm.setActiveProfileState(defaultProfileName); err != nil { log.Warnf("failed to set default profile state: %v", err) } @@ -148,6 +147,13 @@ func (pm *ProfileManager) getActiveProfileState() ID { } func (pm *ProfileManager) setActiveProfileState(id ID) error { + // The invoking user's state is read-only under sudo — a root-owned file in + // the user's directory would break their own runs. The daemon still records + // the switch on its side; only the user-local bookkeeping is skipped. + if sudoActive() { + log.Infof("running under sudo: not persisting active profile %q for user %s", id, os.Getenv(envSudoUser)) + return nil + } configDir, err := getConfigDir() if err != nil { diff --git a/client/internal/profilemanager/state.go b/client/internal/profilemanager/state.go index ddb5dd056..81e6c085f 100644 --- a/client/internal/profilemanager/state.go +++ b/client/internal/profilemanager/state.go @@ -7,6 +7,8 @@ import ( "os" "path/filepath" + log "github.com/sirupsen/logrus" + "github.com/netbirdio/netbird/util" ) @@ -63,6 +65,15 @@ func (pm *ProfileManager) SetProfileState(id ID, state *ProfileState) error { return fmt.Errorf("invalid profile ID: %q", id) } + // The invoking user's state is read-only under sudo. The file only carries + // the account email for the login hint and display, so skipping the write + // costs at most one extra account prompt later — a root-owned file in the + // user's directory would cost every later update instead. + if sudoActive() { + log.Debugf("running under sudo: not persisting profile state for user %s", os.Getenv(envSudoUser)) + return nil + } + stateFile := filepath.Join(configDir, id.String()+".state.json") if err := util.WriteJsonWithRestrictedPermission(context.Background(), stateFile, state); err != nil { return fmt.Errorf("write profile state: %w", err) @@ -92,6 +103,11 @@ func (pm *ProfileManager) SetActiveProfileState(state *ProfileState) error { // equivalent to clearing it; the next SSO login recreates it. A missing file // is not an error. func (pm *ProfileManager) RemoveProfileState(profileName string) error { + if sudoActive() { + log.Debugf("running under sudo: not removing profile state for user %s", os.Getenv(envSudoUser)) + return nil + } + configDir, err := getConfigDir() if err != nil { return fmt.Errorf("get config directory: %w", err)