diff --git a/CHANGELOG.yml b/CHANGELOG.yml index 46ba2852b5..54b629b154 100644 --- a/CHANGELOG.yml +++ b/CHANGELOG.yml @@ -53,6 +53,14 @@ items: All OSS telepresence images are now published at the public registry ghcr.io/telepresenceio and all references from the client and traffic-manager has been updated to use this registry instead of the one at docker.io/datawire. + - type: bugfix + title: Root daemon wouldn't start when sudo timeout was zero. + body: >- + The root daemon refused to start when sudo was configured with a timestamp_timeout=0. + This was due to logic that first requested root privileges using a sudo call, and then relied on that these + privileges were cached, so that a subsequent call using --non-interactive was guaranteed to succeed. + This logic will now instead do one single sudo call, and rely solely on sudo to print an informative prompt and + start the daemon in the background. - type: bugfix title: Detect minikube network when connecting with --docker body: >- diff --git a/integration_test/itest/rm_as_root_unix.go b/integration_test/itest/rm_as_root_unix.go index a97a0d1200..699fa0591c 100644 --- a/integration_test/itest/rm_as_root_unix.go +++ b/integration_test/itest/rm_as_root_unix.go @@ -7,14 +7,8 @@ package itest import ( "context" "os/exec" - - "github.com/telepresenceio/telepresence/v2/pkg/proc" ) func rmAsRoot(ctx context.Context, file string) error { - // We just wanna make sure that the credentials are cached in a uniform way. - if err := proc.CacheAdmin(ctx, ""); err != nil { - return err - } - return exec.Command("sudo", "rm", "-f", file).Run() + return exec.Command("sudo", "-n", "rm", "-f", file).Run() } diff --git a/pkg/proc/exec.go b/pkg/proc/exec.go index 72a1e156c8..b2281b12d7 100644 --- a/pkg/proc/exec.go +++ b/pkg/proc/exec.go @@ -115,13 +115,3 @@ func IsAdmin() bool { func Terminate(p *os.Process) error { return terminate(p) } - -// CacheAdmin will ensure that the current process is able to invoke subprocesses with admin rights -// without having to ask for the password again. This is needed among other things to make sure the -// integration tests can see that a password is being asked for. -func CacheAdmin(ctx context.Context, prompt string) error { - // These logs will get picked up by the test-reporter to make sure the user is asked for the password. - dlog.Info(ctx, "Asking for admin credentials") - defer dlog.Info(ctx, "Admin credentials acquired") - return cacheAdmin(ctx, prompt) -} diff --git a/pkg/proc/exec_unix.go b/pkg/proc/exec_unix.go index 90f26685fb..a389e88be6 100644 --- a/pkg/proc/exec_unix.go +++ b/pkg/proc/exec_unix.go @@ -43,48 +43,17 @@ func startInBackground(includeEnv bool, args ...string) error { return nil } -func cacheAdmin(ctx context.Context, prompt string) error { - // If we're going to be prompting for the `sudo` password, we want to first provide - // the user with some info about exactly what we're prompting for. We don't want to - // use `sudo`'s `--prompt` flag for this because (1) we don't want it to be - // re-displayed if they typo their password, and (2) it might be ignored anyway - // depending on `passprompt_override` in `/etc/sudoers`. So we'll do a pre-flight - // `sudo --non-interactive true` to decide whether to display it. - // - // Note: Using `sudo --non-interactive --validate` does not work well in situations - // where the user has configured `myuser ALL=(ALL:ALL) NOPASSWD: ALL` in the sudoers - // file. Hence the use of `sudo --non-interactive true`. A plausible cause can be - // found in the first comment here: - // https://unix.stackexchange.com/questions/50584/why-sudo-timestamp-is-not-updated-when-nopasswd-is-set - needPwCmd := dexec.CommandContext(ctx, "sudo", "--non-interactive", "true") - needPwCmd.DisableLogging = true - if err := needPwCmd.Run(); err != nil { - if prompt != "" { - fmt.Println(prompt) - } - pwCmd := dexec.CommandContext(ctx, "sudo", "true") - pwCmd.DisableLogging = true - if err := pwCmd.Run(); err != nil { - return err - } +func startInBackgroundAsRoot(_ context.Context, args ...string) error { + if isAdmin() { + return startInBackground(false, args...) } - return nil -} - -func startInBackgroundAsRoot(ctx context.Context, args ...string) error { - if !isAdmin() { - // `sudo` won't be able to read the password from the terminal when we run - // it with Setpgid=true, so do a pre-flight `sudo true` (i.e. cacheAdmin) to read the - // password, and then enforce that being re-used by passing - // `--non-interactive`. - prompt := fmt.Sprintf("Need root privileges to run: %s", shellquote.ShellString(args[0], args[1:])) - if err := CacheAdmin(ctx, prompt); err != nil { - return err - } - args = append([]string{"sudo", "--non-interactive"}, args...) - } - - return startInBackground(false, args...) + // Run sudo with a prompt explaining why root credentials are needed. + return exec.Command("sudo", append([]string{ + "-b", "-p", + fmt.Sprintf( + "Need root privileges to run: %s\nPassword:", + shellquote.ShellString(args[0], args[1:])), + }, args...)...).Run() } func terminate(p *os.Process) error { diff --git a/pkg/proc/exec_windows.go b/pkg/proc/exec_windows.go index a3c3cfa8dd..0ac3fcf577 100644 --- a/pkg/proc/exec_windows.go +++ b/pkg/proc/exec_windows.go @@ -30,11 +30,6 @@ func createNewProcessGroup(cmd *exec.Cmd) { cmd.SysProcAttr = &windows.SysProcAttr{CreationFlags: windows.CREATE_NEW_PROCESS_GROUP} } -func cacheAdmin(_ context.Context, _ string) error { - // No-op on windows, there's no sudo caching. Runas will just pop a window open. - return nil -} - func startInBackground(_ bool, args ...string) error { return shellExec("open", args[0], args[1:]...) } diff --git a/pkg/vif/router_test.go b/pkg/vif/router_test.go index db1b884796..44ca1dcb12 100644 --- a/pkg/vif/router_test.go +++ b/pkg/vif/router_test.go @@ -18,7 +18,6 @@ import ( "github.com/datawire/dlib/dgroup" "github.com/datawire/dlib/dlog" "github.com/telepresenceio/telepresence/v2/pkg/dos" - "github.com/telepresenceio/telepresence/v2/pkg/proc" "github.com/telepresenceio/telepresence/v2/pkg/routing" "github.com/telepresenceio/telepresence/v2/pkg/subnet" ) @@ -54,10 +53,6 @@ func (s *RoutingSuite) SetupSuite() { } else { err := dexec.CommandContext(context.Background(), "go", "build", "-o", "testdata/router/router", "testdata/router/main.go").Run() s.Require().NoError(err) - - // Run sudo to get a password prompt out of the way - err = proc.CacheAdmin(context.Background(), "") - s.Require().NoError(err) } // Make sure there's no existing route cidr := getCidr(2, 1, 32)