diff --git a/lib/versioncontrol/upgradewindow/upgradewindow.go b/lib/versioncontrol/upgradewindow/upgradewindow.go index 346dd75d4e9e9..51a05a7cd5b93 100644 --- a/lib/versioncontrol/upgradewindow/upgradewindow.go +++ b/lib/versioncontrol/upgradewindow/upgradewindow.go @@ -31,6 +31,7 @@ import ( "github.com/gravitational/teleport/api/utils/retryutils" "github.com/gravitational/teleport/lib/backend" "github.com/gravitational/teleport/lib/backend/kubernetes" + "github.com/gravitational/teleport/lib/defaults" "github.com/gravitational/teleport/lib/utils" "github.com/gravitational/teleport/lib/utils/interval" ) @@ -412,13 +413,14 @@ func (e *systemdDriver) Sync(ctx context.Context, rsp proto.ExportUpgradeWindows return e.Reset(ctx) } - // ensure config dir exists - if err := os.MkdirAll(e.cfg.ConfigDir, teleport.PrivateDirMode); err != nil { + // ensure config dir exists. if created it is set to 755, which is reasonably safe and seems to + // be the standard choice for config dirs like this in /etc/. + if err := os.MkdirAll(e.cfg.ConfigDir, defaults.DirectoryPermissions); err != nil { return trace.Wrap(err) } - // export schedule file - if err := os.WriteFile(e.scheduleFile(), []byte(rsp.SystemdUnitSchedule), teleport.FileMaskOwnerOnly); err != nil { + // export schedule file. if created it is set to 644, which is reasonable for a sensitive but non-secret config value. + if err := os.WriteFile(e.scheduleFile(), []byte(rsp.SystemdUnitSchedule), defaults.FilePermissions); err != nil { return trace.Errorf("failed to write schedule file: %v", err) } @@ -426,11 +428,14 @@ func (e *systemdDriver) Sync(ctx context.Context, rsp proto.ExportUpgradeWindows } func (e *systemdDriver) Reset(_ context.Context) error { - if err := os.Remove(e.scheduleFile()); err != nil { - if os.IsNotExist(err) { - return nil - } + if _, err := os.Stat(e.scheduleFile()); os.IsNotExist(err) { + return nil + } + // note that we blank the file rather than deleting it, this is intended to allow us to + // preserve custom file permissions, such as those that might be used in a scenario where + // teleport is operating with limited privileges. + if err := os.WriteFile(e.scheduleFile(), []byte{}, teleport.FileMaskOwnerOnly); err != nil { return trace.Errorf("failed to reset schedule file: %v", err) } diff --git a/lib/versioncontrol/upgradewindow/upgradewindow_test.go b/lib/versioncontrol/upgradewindow/upgradewindow_test.go index 62eed3142b4a9..2551f01c52c19 100644 --- a/lib/versioncontrol/upgradewindow/upgradewindow_test.go +++ b/lib/versioncontrol/upgradewindow/upgradewindow_test.go @@ -152,11 +152,11 @@ func TestSystemdUnitDriver(t *testing.T) { err = driver.Reset(ctx) require.NoError(t, err) - _, err = os.ReadFile(schedPath) - require.Error(t, err) - require.True(t, os.IsNotExist(err)) + sb, err = os.ReadFile(schedPath) + require.NoError(t, err) + require.Equal(t, "", string(sb)) - // verify that NotExist error is suppressed + // verify that duplicate resets succeed err = driver.Reset(ctx) require.NoError(t, err) @@ -175,9 +175,9 @@ func TestSystemdUnitDriver(t *testing.T) { err = driver.Sync(ctx, proto.ExportUpgradeWindowsResponse{}) require.NoError(t, err) - _, err = os.ReadFile(schedPath) - require.Error(t, err) - require.True(t, os.IsNotExist(err)) + sb, err = os.ReadFile(schedPath) + require.NoError(t, err) + require.Equal(t, "", string(sb)) } // fakeDriver is used to inject custom behavior into a dummy Driver instance.