Vault 3999 Change permissions for directory/archive created by debug command

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  
+```

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,14 +954,18 @@ 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
 	}
 
 	return nil
 }
 
 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

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
+}