Skip to content

Commit 0ac84ec

Browse files
nvanthaolaverya
andauthored
fix: [sc-118372] /var/lib/embedded-cluster is not exec by other non-root user (#1729)
* apply chmod after mkdir for directories in --data-dir * address code reviews * add more dir * update os.MkdirAll in all places * potentially fix airgap upgrade job * attempt setting umask 0077 in CI * use bash to run umask * remove e2e test in favor of future dryrun test * set restrictive umask before running a dryrun test * f * test that this does not pass without new code * properly set umask * return to using new code * octal prints, no new code * reenable new code * check all folders even after failure * update test to match behavior * set umask in root command * return to os.MkdirAll * printf debugging * test without setting umask to 077 first * check if the setting actually stuck * call umask in each prerun function * remove debug logs * check that the kubectl-preflight binary has appropriate permissions * set umask in main function, improve comments --------- Co-authored-by: Andrew Lavery <[email protected]>
1 parent 44d6dbd commit 0ac84ec

File tree

9 files changed

+69
-5
lines changed

9 files changed

+69
-5
lines changed

cmd/installer/cli/install.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"runtime"
1111
"sort"
1212
"strings"
13+
"syscall"
1314
"time"
1415

1516
"github.com/gosimple/slug"
@@ -171,6 +172,10 @@ func preRunInstall(cmd *cobra.Command, flags *InstallCmdFlags) error {
171172
return fmt.Errorf("install command must be run as root")
172173
}
173174

175+
// set the umask to 022 so that we can create files/directories with 755 permissions
176+
// this does not return an error - it returns the previous umask
177+
_ = syscall.Umask(0o022)
178+
174179
p, err := parseProxyFlags(cmd)
175180
if err != nil {
176181
return err

cmd/installer/cli/join.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"fmt"
77
"os"
88
"strings"
9+
"syscall"
910

1011
ecv1beta1 "github.com/replicatedhq/embedded-cluster/kinds/apis/v1beta1"
1112
"github.com/replicatedhq/embedded-cluster/pkg/addons"
@@ -96,6 +97,10 @@ func preRunJoin(flags *JoinCmdFlags) error {
9697

9798
flags.isAirgap = flags.airgapBundle != ""
9899

100+
// set the umask to 022 so that we can create files/directories with 755 permissions
101+
// this does not return an error - it returns the previous umask
102+
_ = syscall.Umask(0o022)
103+
99104
return nil
100105
}
101106

cmd/installer/cli/root.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"errors"
66
"fmt"
77
"os"
8+
"syscall"
89

910
"github.com/replicatedhq/embedded-cluster/pkg/dryrun"
1011
"github.com/replicatedhq/embedded-cluster/pkg/metrics"
@@ -68,6 +69,10 @@ func RootCmd(ctx context.Context, name string) *cobra.Command {
6869
metrics.DisableMetrics()
6970
}
7071

72+
// set the umask to 022 so that we can create files/directories with 755 permissions
73+
// this does not return an error - it returns the previous umask
74+
_ = syscall.Umask(0o022)
75+
7176
return nil
7277
},
7378
PersistentPostRunE: func(cmd *cobra.Command, args []string) error {

cmd/installer/main.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"context"
55
"os"
66
"path"
7+
"syscall"
78

89
"github.com/mattn/go-isatty"
910
"github.com/replicatedhq/embedded-cluster/cmd/installer/cli"
@@ -19,5 +20,10 @@ func main() {
1920

2021
name := path.Base(os.Args[0])
2122

23+
// set the umask to 022 so that we can create files/directories with 755 permissions
24+
// this does not return an error - it returns the previous umask
25+
// we do this before calling cli.InitAndExecute so that it is set before the process forks
26+
_ = syscall.Umask(0o022)
27+
2228
cli.InitAndExecute(ctx, name)
2329
}

cmd/local-artifact-mirror/main.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,11 @@ func main() {
1919

2020
name := path.Base(os.Args[0])
2121

22+
// set the umask to 022 so that we can create files/directories with 755 permissions
23+
// this does not return an error - it returns the previous umask
24+
// we do this before calling cli.InitAndExecute so that it is set before the process forks
25+
_ = syscall.Umask(0o022)
26+
2227
InitAndExecute(ctx, name)
2328
}
2429

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
module github.com/replicatedhq/embedded-cluster
22

3-
go 1.23.2
3+
go 1.23.4
44

55
require (
66
github.com/AlecAivazis/survey/v2 v2.3.7

operator/pkg/cli/upgrade_job.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ func UpgradeJobCmd() *cobra.Command {
6363

6464
airgapChartsPath := ""
6565
if in.Spec.AirGap {
66-
airgapChartsPath = runtimeconfig.EmbeddedClusterChartsSubDir()
66+
airgapChartsPath = runtimeconfig.EmbeddedClusterChartsSubDirNoCreate()
6767
}
6868

6969
hcli, err := helm.NewClient(helm.HelmOptions{

tests/dryrun/Dockerfile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
FROM golang:1.23-alpine AS build
1+
FROM golang:1.23.4-alpine AS build
22

33
RUN apk add --no-cache ca-certificates curl git make bash
44

tests/dryrun/install_test.go

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,19 +7,26 @@ import (
77
"os"
88
"path/filepath"
99
"regexp"
10+
"syscall"
1011
"testing"
1112
"time"
1213

1314
"github.com/replicatedhq/embedded-cluster/pkg/dryrun"
1415
"github.com/replicatedhq/embedded-cluster/pkg/helm"
1516
"github.com/replicatedhq/embedded-cluster/pkg/kubeutils"
17+
"github.com/replicatedhq/embedded-cluster/pkg/runtimeconfig"
1618
troubleshootv1beta2 "github.com/replicatedhq/troubleshoot/pkg/apis/troubleshoot/v1beta2"
1719
"github.com/stretchr/testify/assert"
1820
"github.com/stretchr/testify/mock"
1921
"github.com/stretchr/testify/require"
2022
)
2123

2224
func TestDefaultInstallation(t *testing.T) {
25+
testDefaultInstallationImpl(t)
26+
t.Logf("%s: test complete", time.Now().Format(time.RFC3339))
27+
}
28+
29+
func testDefaultInstallationImpl(t *testing.T) {
2330
hcli := &helm.MockClient{}
2431

2532
mock.InOrder(
@@ -152,8 +159,6 @@ func TestDefaultInstallation(t *testing.T) {
152159
assert.Equal(t, "10.244.0.0/17", k0sConfig.Spec.Network.PodCIDR)
153160
assert.Equal(t, "10.244.128.0/17", k0sConfig.Spec.Network.ServiceCIDR)
154161
assert.Contains(t, k0sConfig.Spec.API.SANs, "kubernetes.default.svc.cluster.local")
155-
156-
t.Logf("%s: test complete", time.Now().Format(time.RFC3339))
157162
}
158163

159164
func TestCustomDataDir(t *testing.T) {
@@ -391,3 +396,36 @@ func TestConfigValuesInstallation(t *testing.T) {
391396

392397
t.Logf("%s: test complete", time.Now().Format(time.RFC3339))
393398
}
399+
400+
func TestRestrictiveUmask(t *testing.T) {
401+
oldUmask := syscall.Umask(0o077)
402+
defer syscall.Umask(oldUmask)
403+
404+
testDefaultInstallationImpl(t)
405+
406+
// check that folders created in this test have the right permissions
407+
folderList := []string{
408+
runtimeconfig.EmbeddedClusterHomeDirectory(),
409+
runtimeconfig.EmbeddedClusterBinsSubDir(),
410+
runtimeconfig.EmbeddedClusterChartsSubDir(),
411+
runtimeconfig.PathToEmbeddedClusterBinary("kubectl-preflight"),
412+
}
413+
gotFailure := false
414+
for _, folder := range folderList {
415+
stat, err := os.Stat(folder)
416+
if err != nil {
417+
t.Logf("failed to stat %s: %v", folder, err)
418+
gotFailure = true
419+
continue
420+
}
421+
if stat.Mode().Perm() != 0755 {
422+
t.Logf("expected folder %s to have mode 0755, got %O", folder, stat.Mode().Perm())
423+
gotFailure = true
424+
}
425+
}
426+
if gotFailure {
427+
t.Fatalf("at least one folder had incorrect permissions")
428+
}
429+
430+
t.Logf("%s: test complete", time.Now().Format(time.RFC3339))
431+
}

0 commit comments

Comments
 (0)