Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix problem with root daemon not starting when sudo timeout is zero. #3669

Merged
merged 2 commits into from
Aug 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions CHANGELOG.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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 <code>sudo</code> was configured with a <code>timestamp_timeout=0</code>.
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 <code>--non-interactive</code> 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: >-
Expand Down
8 changes: 1 addition & 7 deletions integration_test/itest/rm_as_root_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
10 changes: 0 additions & 10 deletions pkg/proc/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
51 changes: 10 additions & 41 deletions pkg/proc/exec_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
5 changes: 0 additions & 5 deletions pkg/proc/exec_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:]...)
}
Expand Down
5 changes: 0 additions & 5 deletions pkg/vif/router_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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)
Expand Down
Loading