From 3bb8c840c105589f331ebdaa02a4b0d534b20c59 Mon Sep 17 00:00:00 2001 From: Orest Date: Mon, 2 Nov 2026 00:31:17 +0000 Subject: [PATCH] fix: preserve Linux service credentials after partial install --- cmd/cloudflared/common_service.go | 4 + cmd/cloudflared/linux_service.go | 102 ++++++-- cmd/cloudflared/linux_service_test.go | 356 ++++++++++++++++++++++++++ 3 files changed, 441 insertions(+), 21 deletions(-) diff --git a/cmd/cloudflared/common_service.go b/cmd/cloudflared/common_service.go index abf49e8cd07..0ae169790fb 100644 --- a/cmd/cloudflared/common_service.go +++ b/cmd/cloudflared/common_service.go @@ -27,6 +27,7 @@ func ensureConfigDirExists(configDir string) error { return nil } +//nolint:unused // Used by the macOS service installer, but not the Linux installer. func createTokenFileUnix(path string) error { const tokenPerms os.FileMode = 0o600 f, err := os.OpenFile(path, os.O_RDWR|os.O_CREATE, tokenPerms) //nolint:gosec // All callers of this function construct path from constant strings or well-known env vars (e.g., $HOME) @@ -48,6 +49,8 @@ func createTokenFileUnix(path string) error { // permissions. Since the method used to restrict the permissions is platform // dependent, make the function used to restrict the permissions an injectable // dependency +// +//nolint:unused // Used by the macOS and Windows service installers. func writeTokenToFile(path string, token string) error { if _, err := tunnel.ParseToken(token); err != nil { return cliutil.UsageError("Provided tunnel token is not valid (%s).", err) @@ -84,6 +87,7 @@ func tokenPath(configDir string) string { return filepath.Join(configDir, defaultTokenFile) } +//nolint:unused // Used by the macOS and Windows service installers. func writeTokenToConfigDir(c *cli.Context, configDir string) error { if err := ensureConfigDirExists(configDir); err != nil { return err diff --git a/cmd/cloudflared/linux_service.go b/cmd/cloudflared/linux_service.go index c862754e5ea..33cf5c80c8e 100644 --- a/cmd/cloudflared/linux_service.go +++ b/cmd/cloudflared/linux_service.go @@ -67,7 +67,9 @@ const ( cloudflaredOpenRCService = "cloudflared" ) -// OS-specific function for token file creation +// OS-specific function required by the shared non-exclusive token writer. +// +//nolint:unused // The macOS and Windows installers use that writer; Linux creates tokens exclusively. var createTokenFile = createTokenFileUnix var systemdAllTemplates = map[string]ServiceTemplate{ @@ -281,18 +283,6 @@ func installLinuxService(c *cli.Context) error { // write the token to the config directory and install the service // using --token-file pointing to that file. This is the quick setup // the tunnel UI suggests. - - // Ensure token file is removed if install fails - defer func() { - if err != nil { - removeTokenFile(serviceConfigDir, log) - } - }() - - if err = writeTokenToConfigDir(c, serviceConfigDir); err != nil { - return fmt.Errorf("could not write token to configuration directory: %w", err) - } - extraArgs = buildArgsForTokenFile(serviceConfigDir) } @@ -301,24 +291,90 @@ func installLinuxService(c *cli.Context) error { // Check if the "no update flag" is set autoUpdate := !c.IsSet(noUpdateServiceFlag.Name) + var serviceTemplate ServiceTemplate + var install func() error switch { case inits.IsSystemd(): log.Info().Msgf("Using Systemd") - err = installSystemd(&templateArgs, autoUpdate, log) + serviceTemplate = systemdAllTemplates[cloudflaredService] + install = func() error { return installSystemd(&templateArgs, autoUpdate, log) } case inits.IsOpenRC(): log.Info().Msgf("Using OpenRC") - err = installOpenRC(&templateArgs, autoUpdate) + serviceTemplate = openrcTemplate + install = func() error { return installOpenRC(&templateArgs, autoUpdate) } default: log.Info().Msgf("Using SysV") - err = installSysv(&templateArgs, autoUpdate, log) + serviceTemplate = sysvTemplate + install = func() error { return installSysv(&templateArgs, autoUpdate, log) } } + err = installLinuxServiceWithCredentials(c, serviceConfigDir, serviceTemplate, install, log) if err == nil { log.Info().Msg("Linux service for cloudflared installed successfully") } return err } +func installLinuxServiceWithCredentials(c *cli.Context, configDir string, serviceTemplate ServiceTemplate, install func() error, log *zerolog.Logger) (err error) { + if c.NArg() == 0 { + return install() + } + + servicePath, err := serviceTemplate.ResolvePath() + if err != nil { + return err + } + // Reject a repeat install before it can overwrite the existing service's + // credentials. Lstat also catches a dangling service symlink. + if _, err := os.Lstat(servicePath); err == nil { + return errors.New(serviceAlreadyExistsWarn(servicePath)) + } else if !errors.Is(err, os.ErrNotExist) { + return fmt.Errorf("check existing service at %s: %w", servicePath, err) + } + + token := c.Args().First() + if _, err := tunnel.ParseToken(token); err != nil { + return cliutil.UsageError("Provided tunnel token is not valid (%s).", err) + } + if err := ensureConfigDirExists(configDir); err != nil { + return err + } + + // Create exclusively so an existing file or symlink, including one created + // concurrently, cannot be followed or overwritten. + path := tokenPath(configDir) + tokenFile, err := os.OpenFile(path, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o600) //nolint:gosec // configDir is the fixed service directory or a test directory. + if err != nil { + if errors.Is(err, os.ErrExist) { + return fmt.Errorf("refusing to overwrite existing token file at %s; inspect it and move it aside before retrying installation: %w", path, err) + } + return fmt.Errorf("create service token file: %w", err) + } + + defer func() { + if err == nil { + return + } + // Installers leave service definitions in place on failure, including + // failures while writing a template or enabling/starting the service. + // Only remove a newly created token if no service definition remains. + // If the path cannot be checked, retain the token conservatively. + if _, statErr := os.Lstat(servicePath); errors.Is(statErr, os.ErrNotExist) { + removeTokenFile(configDir, log) + } else if statErr != nil { + err = errors.Join(err, fmt.Errorf("check service at %s before token cleanup: %w", servicePath, statErr)) + } + }() + defer func() { _ = tokenFile.Close() }() + if _, err = tokenFile.WriteString(token); err != nil { + return fmt.Errorf("write service token file: %w", err) + } + if err = tokenFile.Close(); err != nil { + return fmt.Errorf("close service token file: %w", err) + } + return install() +} + func buildArgsForConfig(c *cli.Context, log *zerolog.Logger) ([]string, error) { if err := ensureConfigDirExists(serviceConfigDir); err != nil { return nil, err @@ -353,16 +409,20 @@ func buildArgsForConfig(c *cli.Context, log *zerolog.Logger) ([]string, error) { } func installSystemd(templateArgs *ServiceTemplateArgs, autoUpdate bool, log *zerolog.Logger) error { + return installSystemdWithTemplates(templateArgs, autoUpdate, systemdAllTemplates, runCommand, log) +} + +func installSystemdWithTemplates(templateArgs *ServiceTemplateArgs, autoUpdate bool, templates map[string]ServiceTemplate, run commandRunner, log *zerolog.Logger) error { var systemdTemplates []ServiceTemplate if autoUpdate { systemdTemplates = []ServiceTemplate{ - systemdAllTemplates[cloudflaredService], - systemdAllTemplates[cloudflaredUpdateService], - systemdAllTemplates[cloudflaredUpdateTimer], + templates[cloudflaredService], + templates[cloudflaredUpdateService], + templates[cloudflaredUpdateTimer], } } else { systemdTemplates = []ServiceTemplate{ - systemdAllTemplates[cloudflaredService], + templates[cloudflaredService], } } @@ -373,7 +433,7 @@ func installSystemd(templateArgs *ServiceTemplateArgs, autoUpdate bool, log *zer return err } } - return enableSystemdUnits(runCommand, autoUpdate, log) + return enableSystemdUnits(run, autoUpdate, log) } // commandRunner runs an external command. It has the signature of runCommand, diff --git a/cmd/cloudflared/linux_service_test.go b/cmd/cloudflared/linux_service_test.go index 196718bc41a..25dff6f5432 100644 --- a/cmd/cloudflared/linux_service_test.go +++ b/cmd/cloudflared/linux_service_test.go @@ -3,13 +3,18 @@ package main import ( + "encoding/base64" "errors" + "flag" + "os" + "path/filepath" "strings" "testing" "github.com/rs/zerolog" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/urfave/cli/v2" ) // commandRecorder records the commands it is asked to run and fails the one @@ -19,6 +24,357 @@ type commandRecorder struct { failOn string } +func TestInstallLinuxServiceWithCredentials(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + withoutToken bool + autoUpdate bool + failTemplate string + failCommand string + }{ + {name: "success with auto-update", autoUpdate: true}, + {name: "success without auto-update"}, + {name: "main service template failure", failTemplate: cloudflaredService}, + {name: "update service template failure", autoUpdate: true, failTemplate: cloudflaredUpdateService}, + {name: "update timer template failure", autoUpdate: true, failTemplate: cloudflaredUpdateTimer}, + {name: "enable service failure", failCommand: "systemctl enable cloudflared.service"}, + {name: "enable timer failure", autoUpdate: true, failCommand: "systemctl enable cloudflared-update.timer"}, + {name: "start timer failure", autoUpdate: true, failCommand: "systemctl start cloudflared-update.timer"}, + {name: "daemon reload failure", failCommand: "systemctl daemon-reload"}, + {name: "service startup failure", failCommand: "systemctl start cloudflared.service"}, + {name: "service startup failure with auto-update", autoUpdate: true, failCommand: "systemctl start cloudflared.service"}, + {name: "config based installation", withoutToken: true}, + {name: "config based startup failure", withoutToken: true, failCommand: "systemctl start cloudflared.service"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + configDir := filepath.Join(t.TempDir(), "config") + templates := systemdTemplatesForTest(t) + if tt.failTemplate != "" { + template := templates[tt.failTemplate] + template.Content = "{{" + templates[tt.failTemplate] = template + } + token := base64.StdEncoding.EncodeToString([]byte(`{"a":"test-account","s":"dGVzdA==","t":"00000000-0000-0000-0000-000000000001"}`)) + args := []string{token} + templateArgs := ServiceTemplateArgs{Path: "/usr/bin/cloudflared", ExtraArgs: buildArgsForTokenFile(configDir)} + if tt.withoutToken { + args = nil + templateArgs.ExtraArgs = []string{"--config", filepath.Join(configDir, serviceConfigFile), "tunnel", "run"} + } + c := serviceInstallContext(t, args...) + recorder := &commandRecorder{failOn: tt.failCommand} + log := zerolog.Nop() + install := func() error { + if !tt.withoutToken { + contents, err := os.ReadFile(tokenPath(configDir)) + require.NoError(t, err) + require.Equal(t, token, string(contents), "the token must be written before installation starts") + } + return installSystemdWithTemplates(&templateArgs, tt.autoUpdate, templates, recorder.run, &log) + } + err := installLinuxServiceWithCredentials(c, configDir, templates[cloudflaredService], install, &log) + switch { + case tt.failTemplate != "": + require.ErrorContains(t, err, "template") + case tt.failCommand != "": + require.EqualError(t, err, "command failed") + default: + require.NoError(t, err) + } + + if tt.failTemplate == cloudflaredService { + assert.NoFileExists(t, templates[cloudflaredService].Path) + assert.NoFileExists(t, tokenPath(configDir)) + } else { + unit, err := os.ReadFile(templates[cloudflaredService].Path) + require.NoError(t, err) + assert.Contains(t, string(unit), strings.Join(templateArgs.ExtraArgs, " ")) + assert.NotContains(t, string(unit), token, "credentials must not be embedded in the unit") + if !tt.withoutToken { + contents, err := os.ReadFile(tokenPath(configDir)) + require.NoError(t, err, "an installed service must retain its token even when installation returns an error") + assert.Equal(t, token, string(contents)) + info, err := os.Stat(tokenPath(configDir)) + require.NoError(t, err) + assert.Equal(t, os.FileMode(0o600), info.Mode().Perm()) + } + } + if tt.withoutToken { + assert.NoDirExists(t, configDir, "config based installs must not create a token directory") + } + + var wantCommands []string + if tt.failTemplate == "" { + wantCommands = []string{"systemctl enable cloudflared.service"} + if tt.autoUpdate { + wantCommands = append(wantCommands, "systemctl enable cloudflared-update.timer", "systemctl start cloudflared-update.timer") + } + wantCommands = append(wantCommands, "systemctl daemon-reload", "systemctl start cloudflared.service") + for i, command := range wantCommands { + if command == tt.failCommand { + wantCommands = wantCommands[:i+1] + break + } + } + } + assert.Equal(t, wantCommands, recorder.commands, "failures must propagate without continuing or rolling back the installed service") + }) + } +} + +func TestInstallLinuxServicePreservesExistingServiceToken(t *testing.T) { + t.Parallel() + + for _, symlink := range []bool{false, true} { + name := "existing service" + if symlink { + name = "dangling service symlink" + } + t.Run(name, func(t *testing.T) { + t.Parallel() + + configDir := t.TempDir() + const originalToken = "existing-token" + require.NoError(t, os.WriteFile(tokenPath(configDir), []byte(originalToken), 0o600)) + serviceTemplate := ServiceTemplate{Path: filepath.Join(t.TempDir(), cloudflaredService)} + if symlink { + require.NoError(t, os.Symlink(serviceTemplate.Path+".missing", serviceTemplate.Path)) + } else { + require.NoError(t, os.WriteFile(serviceTemplate.Path, []byte("existing unit"), 0o600)) + } + for _, token := range []string{"invalid-token", base64.StdEncoding.EncodeToString([]byte(`{"a":"replacement"}`))} { + log := zerolog.Nop() + install := func() error { + t.Error("an existing service must be rejected before installation") + return errors.New("already installed") + } + err := installLinuxServiceWithCredentials(serviceInstallContext(t, token), configDir, serviceTemplate, install, &log) + require.ErrorContains(t, err, "already installed") + contents, err := os.ReadFile(tokenPath(configDir)) + require.NoError(t, err) + assert.Equal(t, originalToken, string(contents)) + } + }) + } +} + +func TestInstallLinuxServiceTokenWriteFailure(t *testing.T) { + t.Parallel() + + for _, existingToken := range []bool{false, true} { + name := "new token" + if existingToken { + name = "existing token" + } + t.Run(name, func(t *testing.T) { + t.Parallel() + + configDir := t.TempDir() + if existingToken { + require.NoError(t, os.WriteFile(tokenPath(configDir), []byte("existing-token"), 0o600)) + } + serviceTemplate := ServiceTemplate{Path: filepath.Join(t.TempDir(), cloudflaredService)} + log := zerolog.Nop() + install := func() error { + t.Error("invalid credentials must prevent installation") + return nil + } + err := installLinuxServiceWithCredentials(serviceInstallContext(t, "invalid-token"), configDir, serviceTemplate, install, &log) + require.ErrorContains(t, err, "Provided tunnel token is not valid") + assert.NoFileExists(t, serviceTemplate.Path) + if existingToken { + contents, err := os.ReadFile(tokenPath(configDir)) + require.NoError(t, err) + assert.Equal(t, "existing-token", string(contents)) + } else { + assert.NoFileExists(t, tokenPath(configDir)) + } + }) + } +} + +func TestInstallLinuxServiceRejectsExistingToken(t *testing.T) { + t.Parallel() + + for _, name := range []string{"different token", "identical token", "symlink", "dangling symlink"} { + t.Run(name, func(t *testing.T) { + t.Parallel() + + configDir := t.TempDir() + serviceTemplate := ServiceTemplate{Path: filepath.Join(t.TempDir(), cloudflaredService)} + tokenA := base64.StdEncoding.EncodeToString([]byte(`{"a":"original-account","s":"dGVzdA==","t":"00000000-0000-0000-0000-000000000001"}`)) + tokenB := base64.StdEncoding.EncodeToString([]byte(`{"a":"replacement-account","s":"dGVzdA==","t":"00000000-0000-0000-0000-000000000002"}`)) + if name == "identical token" { + tokenB = tokenA + } + path := tokenPath(configDir) + symlink := name == "symlink" || name == "dangling symlink" + if symlink { + path = filepath.Join(t.TempDir(), "original-token") + require.NoError(t, os.Symlink(path, tokenPath(configDir))) + } + if name != "dangling symlink" { + require.NoError(t, os.WriteFile(path, []byte(tokenA), 0o600)) + // A rejected installation must not even change existing permissions. + require.NoError(t, os.Chmod(path, 0o400)) + } + install := func() error { + t.Error("existing credentials must be rejected before installation") + return errors.New("installation failed") + } + log := zerolog.Nop() + err := installLinuxServiceWithCredentials(serviceInstallContext(t, tokenB), configDir, serviceTemplate, install, &log) + require.ErrorIs(t, err, os.ErrExist) + assert.NoFileExists(t, serviceTemplate.Path) + if name == "dangling symlink" { + assert.NoFileExists(t, path) + } else { + contents, err := os.ReadFile(path) //nolint:gosec // path refers to a test-owned file. + require.NoError(t, err) + assert.Equal(t, tokenA, string(contents)) + info, err := os.Stat(path) + require.NoError(t, err) + assert.Equal(t, os.FileMode(0o400), info.Mode().Perm()) + } + if symlink { + target, err := os.Readlink(tokenPath(configDir)) + require.NoError(t, err) + assert.Equal(t, path, target) + } + }) + } +} + +func TestInstallLinuxServiceFailedInstallCleanup(t *testing.T) { + t.Parallel() + + tests := []struct { + name string + partialUnit bool + unreadableDir bool + withoutToken bool + wantToken bool + template ServiceTemplate + }{ + {name: "new token removed before service creation"}, + {name: "partial service file retained", partialUnit: true, wantToken: true}, + {name: "unknown service state retains token", unreadableDir: true, wantToken: true}, + {name: "config based failure leaves token alone", withoutToken: true, wantToken: true}, + {name: "failure after SysV service creation", template: sysvTemplate, wantToken: true}, + {name: "failure after OpenRC service creation", template: openrcTemplate, wantToken: true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + configDir := t.TempDir() + unitDir := filepath.Join(t.TempDir(), "units") + require.NoError(t, os.Mkdir(unitDir, 0o700)) + serviceTemplate := tt.template + serviceTemplate.Path = filepath.Join(unitDir, "cloudflared") + token := base64.StdEncoding.EncodeToString([]byte(`{"a":"test-account"}`)) + if tt.withoutToken { + require.NoError(t, os.WriteFile(tokenPath(configDir), []byte(token), 0o600)) + } + args := []string{token} + if tt.withoutToken { + args = nil + } + installErr := errors.New("installation failed") + install := func() error { + if serviceTemplate.Content != "" { + require.NoError(t, serviceTemplate.Generate(&ServiceTemplateArgs{ + Path: "/usr/bin/cloudflared", ExtraArgs: buildArgsForTokenFile(configDir), + })) + } + if tt.partialUnit { + require.NoError(t, os.WriteFile(serviceTemplate.Path, []byte("partial unit"), 0o600)) + } + if tt.unreadableDir { + // A symlink loop makes Lstat fail even when tests run as root. + require.NoError(t, os.Remove(unitDir)) + require.NoError(t, os.Symlink(unitDir, unitDir)) + } + return installErr + } + log := zerolog.Nop() + err := installLinuxServiceWithCredentials(serviceInstallContext(t, args...), configDir, serviceTemplate, install, &log) + require.ErrorIs(t, err, installErr) + if tt.unreadableDir { + require.ErrorContains(t, err, "before token cleanup") + } + if tt.wantToken { + contents, err := os.ReadFile(tokenPath(configDir)) + require.NoError(t, err) + assert.Equal(t, token, string(contents)) + } else { + assert.NoFileExists(t, tokenPath(configDir)) + } + }) + } +} + +func TestInstallLinuxServicePathInspectionFailure(t *testing.T) { + t.Parallel() + + for _, inspectToken := range []bool{false, true} { + name := "service path" + if inspectToken { + name = "token path" + } + t.Run(name, func(t *testing.T) { + t.Parallel() + + root := t.TempDir() + loop := filepath.Join(root, "loop") + require.NoError(t, os.Symlink(loop, loop)) + configDir := filepath.Join(root, "config") + servicePath := filepath.Join(loop, cloudflaredService) + if inspectToken { + configDir = loop + servicePath = filepath.Join(root, cloudflaredService) + } + install := func() error { + t.Error("an uninspectable path must prevent installation") + return nil + } + log := zerolog.Nop() + token := base64.StdEncoding.EncodeToString([]byte(`{"a":"test-account"}`)) + err := installLinuxServiceWithCredentials(serviceInstallContext(t, token), configDir, ServiceTemplate{Path: servicePath}, install, &log) + if inspectToken { + require.ErrorContains(t, err, "create service token file") + } else { + require.ErrorContains(t, err, "check existing service") + } + assert.NoDirExists(t, filepath.Join(root, "config")) + }) + } +} + +func serviceInstallContext(t *testing.T, args ...string) *cli.Context { + t.Helper() + flags := flag.NewFlagSet("service install", flag.ContinueOnError) + require.NoError(t, flags.Parse(args)) + return cli.NewContext(cli.NewApp(), flags, nil) +} + +func systemdTemplatesForTest(t *testing.T) map[string]ServiceTemplate { + t.Helper() + templates := make(map[string]ServiceTemplate, len(systemdAllTemplates)) + unitDir := t.TempDir() + for name, template := range systemdAllTemplates { + template.Path = filepath.Join(unitDir, name) + templates[name] = template + } + return templates +} + func (r *commandRecorder) run(command string, args ...string) error { cmd := strings.Join(append([]string{command}, args...), " ") r.commands = append(r.commands, cmd)