mirror of
https://github.com/netbirdio/netbird.git
synced 2026-08-24 16:41:30 +02:00
[client] Resolve profiles for the sudo invoking user instead of root
The SSH server flags force `netbird up` through sudo, but the CLI resolved every per-user path with the process user. As root that reads root's own (empty) local state, so a `sudo netbird up` silently switched the daemon from the user's profile to the default one — cancelling any login already waiting in the browser — and then ran an SSO login for the default profile's config. Whichever account that login returned, the default profile's peer belongs to someone else, so every attempt ended in "peer is already registered by a different User or a Setup Key", with nothing telling the user why. Resolve the acting user through SUDO_USER when running as root: the active profile, the profile config paths and the stored account email now come from the invoking user's directories. Privilege decisions are untouched — they stay on the kernel credentials of the daemon connection, which an environment variable can never influence; a forged SUDO_USER only selects a profile root could select anyway. The invoking user's directories are strictly read-only under sudo. Anything root wrote there would be root-owned and break the user's own runs, so instead of chowning files back, the local writes are skipped: the active-profile bookkeeping and the account-email state simply do not update from a sudo run (the daemon records the switch on its side; a skipped email write costs at most one extra account prompt later). Plain root — no sudo context — has no user to act for, so the ambiguity is refused instead of guessed at: when the daemon's active profile differs from what root resolves and no --profile was given, up fails with a message naming both profiles, instead of silently switching the daemon and failing later with the ownership error.
This commit is contained in:
@@ -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)
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
@@ -5,7 +5,6 @@ import (
|
||||
"fmt"
|
||||
"net"
|
||||
"net/netip"
|
||||
"os/user"
|
||||
"runtime"
|
||||
"strings"
|
||||
"time"
|
||||
@@ -122,7 +121,7 @@ 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)
|
||||
}
|
||||
@@ -295,6 +294,21 @@ func runInDaemonMode(ctx context.Context, cmd *cobra.Command, pm *profilemanager
|
||||
|
||||
client := proto.NewDaemonServiceClient(conn)
|
||||
|
||||
// Plain root has no invoking user to resolve profiles for, so the local
|
||||
// state falls back to root's own — the default profile. Acting on that
|
||||
// while the daemon runs another user's profile would silently switch the
|
||||
// daemon away from it (and a later browser login would register the
|
||||
// default profile's peer under whichever account the IdP returns). Refuse
|
||||
// the ambiguity instead of guessing.
|
||||
if profilemanager.IsPlainRoot() && profileName == "" {
|
||||
if active, err := client.GetActiveProfile(ctx, &proto.GetActiveProfileRequest{}); err == nil &&
|
||||
active.GetId() != "" && active.GetId() != activeProf.ID.String() {
|
||||
return fmt.Errorf(
|
||||
"running as root: the daemon's active profile is %q (user %q), but this invocation resolves to %q; pass --profile to choose one explicitly, or run via sudo from your own user",
|
||||
active.GetProfileName(), active.GetUsername(), activeProf.ID)
|
||||
}
|
||||
}
|
||||
|
||||
status, err := client.Status(ctx, &proto.StatusRequest{
|
||||
WaitForReady: func() *bool { b := true; return &b }(),
|
||||
})
|
||||
@@ -314,7 +328,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)
|
||||
}
|
||||
|
||||
@@ -217,6 +217,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 _, sudo := sudoInvokingUser(); sudo {
|
||||
return configDir, nil
|
||||
}
|
||||
if err := os.MkdirAll(configDir, 0o755); err != nil {
|
||||
return "", err
|
||||
}
|
||||
@@ -224,6 +230,9 @@ func getConfigDir() (string, error) {
|
||||
}
|
||||
|
||||
func baseConfigDir() (string, error) {
|
||||
if u, ok := sudoInvokingUser(); ok {
|
||||
return userBaseConfigDir(u)
|
||||
}
|
||||
if runtime.GOOS == "darwin" {
|
||||
if u, err := user.Current(); err == nil && u.HomeDir != "" {
|
||||
return filepath.Join(u.HomeDir, "Library", "Application Support"), nil
|
||||
|
||||
69
client/internal/profilemanager/invoking_user.go
Normal file
69
client/internal/profilemanager/invoking_user.go
Normal file
@@ -0,0 +1,69 @@
|
||||
package profilemanager
|
||||
|
||||
import (
|
||||
"fmt"
|
||||
"os"
|
||||
"os/user"
|
||||
"path/filepath"
|
||||
"runtime"
|
||||
|
||||
log "github.com/sirupsen/logrus"
|
||||
)
|
||||
|
||||
// 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
|
||||
}
|
||||
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 os.Geteuid() != 0 {
|
||||
return false
|
||||
}
|
||||
_, ok := sudoInvokingUser()
|
||||
return !ok
|
||||
}
|
||||
|
||||
// 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 os.Geteuid() != 0 {
|
||||
return nil, false
|
||||
}
|
||||
name := os.Getenv("SUDO_USER")
|
||||
if name == "" || name == "root" {
|
||||
return nil, false
|
||||
}
|
||||
u, err := user.Lookup(name)
|
||||
if err != nil {
|
||||
log.Warnf("failed to look up sudo invoking user %q, acting as root: %v", name, err)
|
||||
return nil, false
|
||||
}
|
||||
return u, true
|
||||
}
|
||||
|
||||
// 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
|
||||
}
|
||||
57
client/internal/profilemanager/invoking_user_test.go
Normal file
57
client/internal/profilemanager/invoking_user_test.go
Normal file
@@ -0,0 +1,57 @@
|
||||
package profilemanager
|
||||
|
||||
import (
|
||||
"os"
|
||||
"os/user"
|
||||
"path/filepath"
|
||||
"runtime"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
)
|
||||
|
||||
func TestInvokingUserFallsBackToProcessUser(t *testing.T) {
|
||||
t.Setenv("SUDO_USER", "")
|
||||
|
||||
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("SUDO_USER", "")
|
||||
_, ok := sudoInvokingUser()
|
||||
assert.False(t, ok)
|
||||
}
|
||||
|
||||
func TestSudoInvokingUserIgnoresRoot(t *testing.T) {
|
||||
if os.Geteuid() != 0 {
|
||||
t.Skip("needs root to enter the sudo branch")
|
||||
}
|
||||
t.Setenv("SUDO_USER", "root")
|
||||
_, ok := sudoInvokingUser()
|
||||
assert.False(t, ok, "sudo from a root shell must not redirect anything")
|
||||
}
|
||||
|
||||
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("SUDO_USER", "")
|
||||
assert.Equal(t, os.Geteuid() == 0, IsPlainRoot())
|
||||
}
|
||||
@@ -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 _, sudo := sudoInvokingUser(); !sudo {
|
||||
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 u, sudo := sudoInvokingUser(); sudo {
|
||||
log.Infof("running under sudo: not persisting active profile %q for user %s", id, u.Username)
|
||||
return nil
|
||||
}
|
||||
|
||||
configDir, err := getConfigDir()
|
||||
if err != nil {
|
||||
|
||||
@@ -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 u, sudo := sudoInvokingUser(); sudo {
|
||||
log.Debugf("running under sudo: not persisting profile state for user %s", u.Username)
|
||||
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)
|
||||
|
||||
Reference in New Issue
Block a user