diff --git a/changelog/14846.txt b/changelog/14846.txt new file mode 100644 index 0000000000000..10621ff4fba16 --- /dev/null +++ b/changelog/14846.txt @@ -0,0 +1,3 @@ +```release-note:bug +core: fixing excessive unix file permissions on dir, files and archive created by vault debug command +``` \ No newline at end of file diff --git a/command/debug.go b/command/debug.go index afd4471d0f74a..c5667e14e7235 100644 --- a/command/debug.go +++ b/command/debug.go @@ -8,9 +8,11 @@ import ( "net/url" "os" "path/filepath" + "runtime" "strconv" "strings" "sync" + "syscall" "time" "github.com/hashicorp/go-hclog" @@ -354,7 +356,7 @@ func (c *DebugCommand) generateIndex() error { } // Write out file - if err := ioutil.WriteFile(filepath.Join(c.flagOutput, "index.json"), bytes, 0o644); err != nil { + if err := ioutil.WriteFile(filepath.Join(c.flagOutput, "index.json"), bytes, 0o600); err != nil { return fmt.Errorf("error generating index file; %s", err) } @@ -451,7 +453,7 @@ func (c *DebugCommand) preflight(rawArgs []string) (string, error) { _, err = os.Stat(c.flagOutput) switch { case os.IsNotExist(err): - err := os.MkdirAll(c.flagOutput, 0o755) + err := os.MkdirAll(c.flagOutput, 0o700) if err != nil { return "", fmt.Errorf("unable to create output directory: %s", err) } @@ -724,7 +726,7 @@ func (c *DebugCommand) collectPprof(ctx context.Context) { // Create a sub-directory for pprof data currentDir := currentTimestamp.Format(fileFriendlyTimeFormat) dirName := filepath.Join(c.flagOutput, currentDir) - if err := os.MkdirAll(dirName, 0o755); err != nil { + if err := os.MkdirAll(dirName, 0o700); err != nil { c.UI.Error(fmt.Sprintf("Error creating sub-directory for time interval: %s", err)) continue } @@ -741,7 +743,7 @@ func (c *DebugCommand) collectPprof(ctx context.Context) { return } - err = ioutil.WriteFile(filepath.Join(dirName, target+".prof"), data, 0o644) + err = ioutil.WriteFile(filepath.Join(dirName, target+".prof"), data, 0o600) if err != nil { c.captureError("pprof."+target, err) } @@ -759,7 +761,7 @@ func (c *DebugCommand) collectPprof(ctx context.Context) { return } - err = ioutil.WriteFile(filepath.Join(dirName, "goroutines.txt"), data, 0o644) + err = ioutil.WriteFile(filepath.Join(dirName, "goroutines.txt"), data, 0o600) if err != nil { c.captureError("pprof.goroutines-text", err) } @@ -783,7 +785,7 @@ func (c *DebugCommand) collectPprof(ctx context.Context) { return } - err = ioutil.WriteFile(filepath.Join(dirName, "profile.prof"), data, 0o644) + err = ioutil.WriteFile(filepath.Join(dirName, "profile.prof"), data, 0o600) if err != nil { c.captureError("pprof.profile", err) } @@ -799,7 +801,7 @@ func (c *DebugCommand) collectPprof(ctx context.Context) { return } - err = ioutil.WriteFile(filepath.Join(dirName, "trace.out"), data, 0o644) + err = ioutil.WriteFile(filepath.Join(dirName, "trace.out"), data, 0o600) if err != nil { c.captureError("pprof.trace", err) } @@ -892,7 +894,7 @@ func (c *DebugCommand) persistCollection(collection []map[string]interface{}, ou if err != nil { return err } - if err := ioutil.WriteFile(filepath.Join(c.flagOutput, outFile), bytes, 0o644); err != nil { + if err := ioutil.WriteFile(filepath.Join(c.flagOutput, outFile), bytes, 0o600); err != nil { return err } @@ -900,6 +902,10 @@ func (c *DebugCommand) persistCollection(collection []map[string]interface{}, ou } func (c *DebugCommand) compress(dst string) error { + if runtime.GOOS != "windows" { + defer syscall.Umask(syscall.Umask(0o077)) + } + tgz := archiver.NewTarGz() if err := tgz.Archive([]string{c.flagOutput}, dst); err != nil { return fmt.Errorf("failed to compress data: %s", err) @@ -984,7 +990,7 @@ func (c *DebugCommand) captureError(target string, err error) { } func (c *DebugCommand) writeLogs(ctx context.Context) { - out, err := os.Create(filepath.Join(c.flagOutput, "vault.log")) + out, err := os.OpenFile(filepath.Join(c.flagOutput, "vault.log"), os.O_CREATE, 0o600) if err != nil { c.captureError("log", err) return diff --git a/command/debug_test.go b/command/debug_test.go index 885b0de63ef50..7e184dc54f87a 100644 --- a/command/debug_test.go +++ b/command/debug_test.go @@ -7,7 +7,9 @@ import ( "io/ioutil" "os" "path/filepath" + "runtime" "strings" + "syscall" "testing" "time" @@ -593,7 +595,7 @@ func TestDebugCommand_OutputExists(t *testing.T) { t.Fatal(err) } } else { - err = os.Mkdir(outputPath, 0o755) + err = os.Mkdir(outputPath, 0o700) if err != nil { t.Fatal(err) } @@ -699,3 +701,134 @@ func TestDebugCommand_PartialPermissions(t *testing.T) { t.Fatal(err) } } + +// set insecure umask to see if the files and directories get created with right permissions +func TestDebugCommand_InsecureUmask(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("test does not work in windows environment") + } + t.Parallel() + + cases := []struct { + name string + compress bool + outputFile string + expectError bool + }{ + { + "with-compress", + true, + "with-compress.tar.gz", + false, + }, + { + "no-compress", + false, + "no-compress", + false, + }, + } + + for _, tc := range cases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + // set insecure umask + defer syscall.Umask(syscall.Umask(0)) + + testDir, err := ioutil.TempDir("", "vault-debug") + if err != nil { + t.Fatal(err) + } + defer os.RemoveAll(testDir) + + client, closer := testVaultServer(t) + defer closer() + + ui, cmd := testDebugCommand(t) + cmd.client = client + cmd.skipTimingChecks = true + + outputPath := filepath.Join(testDir, tc.outputFile) + + args := []string{ + fmt.Sprintf("-compress=%t", tc.compress), + "-duration=1s", + "-interval=1s", + "-metrics-interval=1s", + fmt.Sprintf("-output=%s", outputPath), + } + + code := cmd.Run(args) + if exp := 0; code != exp { + t.Log(ui.ErrorWriter.String()) + t.Fatalf("expected %d to be %d", code, exp) + } + // If we expect an error we're done here + if tc.expectError { + return + } + + bundlePath := filepath.Join(testDir, tc.outputFile) + fs, err := os.Stat(bundlePath) + if os.IsNotExist(err) { + t.Log(ui.OutputWriter.String()) + t.Fatal(err) + } + // check permissions of the parent debug directory + err = isValidFilePermissions(fs) + if err != nil { + t.Fatalf(err.Error()) + } + + // check permissions of the files within the parent directory + switch tc.compress { + case true: + tgz := archiver.NewTarGz() + + err = tgz.Walk(bundlePath, func(f archiver.File) error { + fh, ok := f.Header.(*tar.Header) + if !ok { + return fmt.Errorf("invalid file header: %#v", f.Header) + } + err = isValidFilePermissions(fh.FileInfo()) + if err != nil { + t.Fatalf(err.Error()) + } + return nil + }) + + case false: + err = filepath.Walk(bundlePath, func(path string, info os.FileInfo, err error) error { + err = isValidFilePermissions(info) + if err != nil { + t.Fatalf(err.Error()) + } + return nil + }) + } + + if err != nil { + t.Fatal(err) + } + }) + } +} + +func isValidFilePermissions(info os.FileInfo) (err error) { + mode := info.Mode() + // check group permissions + for i := 4; i < 7; i++ { + if string(mode.String()[i]) != "-" { + return fmt.Errorf("expected no permissions for group but got %s permissions for file %s", string(mode.String()[i]), info.Name()) + } + } + + // check others permissions + for i := 7; i < 10; i++ { + if string(mode.String()[i]) != "-" { + return fmt.Errorf("expected no permissions for others but got %s permissions for file %s", string(mode.String()[i]), info.Name()) + } + } + return err +}