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

[teleport-update] Set umask 0022 for teleport-update to avoid errors on enable #52725

Merged
merged 14 commits into from
Mar 4, 2025
1 change: 1 addition & 0 deletions lib/autoupdate/agent/installer.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ const (

// LocalInstaller manages the creation and removal of installations
// of Teleport.
// SetRequiredUmask must be called before any methods are executed.
type LocalInstaller struct {
// InstallDir contains each installation, named by version.
InstallDir string
Expand Down
20 changes: 20 additions & 0 deletions lib/autoupdate/agent/updater.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import (
"path/filepath"
"runtime"
"slices"
"syscall"
"time"

"github.com/gravitational/trace"
Expand Down Expand Up @@ -57,6 +58,9 @@ const (
reservedFreeDisk = 10_000_000
// debugSocketFileName is the name of Teleport's debug socket in the data dir.
debugSocketFileName = "debug.sock" // 10 MB
// requiredUmask must be set before this package can be used.
// Use syscall.Umask to set when no other goroutines are running.
requiredUmask = 0o022
)

// Log keys
Expand All @@ -67,6 +71,20 @@ const (
errorKey = "error"
)

// SetRequiredUmask sets the umask to match the systemd umask that the teleport-update service will execute with.
// This ensures consistent file permissions.
// NOTE: This must be run in main.go before any goroutines that create files are started.
func SetRequiredUmask(ctx context.Context, log *slog.Logger) {
warnUmask(ctx, log, syscall.Umask(requiredUmask))
}

func warnUmask(ctx context.Context, log *slog.Logger, old int) {
if old&^requiredUmask != 0 {
log.WarnContext(ctx, "Restrictive umask detected. Umask has been changed to 0022 for teleport-update and all child processes.")
log.WarnContext(ctx, "All files created by teleport-update will have permissions set according to this umask.")
}
}

// NewLocalUpdater returns a new Updater that auto-updates local
// installations of the Teleport agent.
// The AutoUpdater uses an HTTP client with sane defaults for downloads, and
Expand Down Expand Up @@ -174,6 +192,7 @@ type LocalUpdaterConfig struct {
}

// Updater implements the agent-local logic for Teleport agent auto-updates.
// SetRequiredUmask must be called before any methods are executed, except for Status.
type Updater struct {
// Log contains a logger.
Log *slog.Logger
Expand Down Expand Up @@ -545,6 +564,7 @@ func isActiveOrEnabled(ctx context.Context, s Process) (bool, error) {

// Status returns all available local and remote fields related to agent auto-updates.
// Status is safe to run concurrently with other Updater commands.
// Status does not write files, and therefore does not require SetRequiredUmask.
func (u *Updater) Status(ctx context.Context) (Status, error) {
var out Status
// Read configuration from update.yaml.
Expand Down
37 changes: 37 additions & 0 deletions lib/autoupdate/agent/updater_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,12 @@
package agent

import (
"bytes"
"context"
"encoding/json"
"errors"
"fmt"
"log/slog"
"net/http"
"net/http/httptest"
"os"
Expand All @@ -40,6 +43,40 @@ import (
"github.com/gravitational/teleport/lib/utils/testutils/golden"
)

func TestWarnUmask(t *testing.T) {
t.Parallel()

for _, tt := range []struct {
old int
warn bool
}{
{old: 0o000, warn: false},
{old: 0o001, warn: true},
{old: 0o011, warn: true},
{old: 0o111, warn: true},
{old: 0o002, warn: false},
{old: 0o020, warn: false},
{old: 0o022, warn: false},
{old: 0o220, warn: true},
{old: 0o200, warn: true},
{old: 0o222, warn: true},
{old: 0o333, warn: true},
{old: 0o444, warn: true},
{old: 0o555, warn: true},
{old: 0o666, warn: true},
{old: 0o777, warn: true},
} {
t.Run(fmt.Sprintf("old umask %o", tt.old), func(t *testing.T) {
ctx := context.Background()
out := &bytes.Buffer{}
warnUmask(ctx, slog.New(slog.NewTextHandler(out,
&slog.HandlerOptions{ReplaceAttr: msgOnly},
)), tt.old)
assert.Equal(t, tt.warn, strings.Contains(out.String(), "detected"))
})
}
}

func TestUpdater_Disable(t *testing.T) {
t.Parallel()

Expand Down
7 changes: 7 additions & 0 deletions tool/teleport-update/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,13 @@ func Run(args []string) int {
return 1
}

// Set required umask for commands that write files to system directories as root, and warn loudly if it changes.
switch command {
case statusCmd.FullCommand(), versionCmd.FullCommand():
default:
autoupdate.SetRequiredUmask(ctx, plog)
}

switch command {
case enableCmd.FullCommand():
ccfg.Enabled = true
Expand Down
Loading