From 9f03f8607778aa6d3c4a8c4cbd70ce5ca05b0f64 Mon Sep 17 00:00:00 2001 From: akshya96 <87045294+akshya96@users.noreply.github.com> Date: Mon, 4 Apr 2022 09:44:03 -0700 Subject: [PATCH] Vault 3999 Change permissions for directory/archive created by debug command (#14846) * adding debug changes from ent * adding changelog --- changelog/14846.txt | 3 + command/debug.go | 24 +++++--- command/debug_test.go | 135 +++++++++++++++++++++++++++++++++++++++++- 3 files changed, 152 insertions(+), 10 deletions(-) create mode 100644 changelog/14846.txt diff --git a/changelog/14846.txt b/changelog/14846.txt new file mode 100644 index 000000000..10621ff4f --- /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 f981b1847..e0fd8cd31 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" @@ -356,7 +358,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) } @@ -453,7 +455,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) } @@ -741,7 +743,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 } @@ -758,7 +760,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) } @@ -776,7 +778,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) } @@ -800,7 +802,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) } @@ -816,7 +818,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) } @@ -952,7 +954,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 } @@ -960,6 +962,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) @@ -1044,7 +1050,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 3c6ca4567..1e3ee4760 100644 --- a/command/debug_test.go +++ b/command/debug_test.go @@ -8,7 +8,9 @@ import ( "io/ioutil" "os" "path/filepath" + "runtime" "strings" + "syscall" "testing" "time" @@ -599,7 +601,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) } @@ -705,3 +707,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 +}