mirror of
https://github.com/netbirdio/netbird.git
synced 2026-08-18 13:41:30 +02:00
Compare commits
1 Commits
dependabot
...
fix/sudo-u
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
bc2e1c699a |
@@ -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