Skip to content

Commit

Permalink
Fix problem with root daemon not starting when sudo timeout is zero.
Browse files Browse the repository at this point in the history
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.

Signed-off-by: Thomas Hallgren <[email protected]>
  • Loading branch information
thallgren committed Aug 18, 2024
1 parent ea72611 commit b03c5a4
Show file tree
Hide file tree
Showing 6 changed files with 19 additions and 68 deletions.
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

0 comments on commit b03c5a4

Please sign in to comment.