Skip to content

Commit

Permalink
Vault 3999 Change permissions for directory/archive created by debug …
Browse files Browse the repository at this point in the history 
…command (#14846)

* adding debug changes from ent

* adding changelog
  • Loading branch information
@akshya96
akshya96 committed Apr 4, 2022
1 parent 1bde37c commit 59451c2
Show file tree
Hide file tree
Showing 3 changed files with 152 additions and 10 deletions.
3 changes: 3 additions & 0 deletions changelog/14846.txt
View file
@@ -0,0 +1,3 @@
```release-note:bug
core: fixing excessive unix file permissions on dir, files and archive created by vault debug command
```
24 changes: 15 additions & 9 deletions command/debug.go
View file
Expand Up @@ -8,9 +8,11 @@ import (
"net/url"
"os"
"path/filepath"
"runtime"
"strconv"
"strings"
"sync"
"syscall"
"time"

"github.com/hashicorp/go-hclog"
Expand Down Expand Up @@ -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)
}

Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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
}
Expand All @@ -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)
}
Expand All @@ -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)
}
Expand All @@ -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)
}
Expand All @@ -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)
}
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
135 changes: 134 additions & 1 deletion command/debug_test.go
View file
Expand Up @@ -8,7 +8,9 @@ import (
"io/ioutil"
"os"
"path/filepath"
"runtime"
"strings"
"syscall"
"testing"
"time"

Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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
}

0 comments on commit 59451c2

Please sign in to comment.