From 635873cf4ab8e1609e68d2cad582193178d44237 Mon Sep 17 00:00:00 2001 From: Laura Bennett Date: Fri, 7 Oct 2016 15:09:32 -0400 Subject: [PATCH 1/8] initial commit for adding audit file permission changes --- audit/formatter.go | 2 ++ builtin/audit/file/backend.go | 22 ++++++++++++++++++++-- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/audit/formatter.go b/audit/formatter.go index 318bd1bc59..cb34957d5f 100644 --- a/audit/formatter.go +++ b/audit/formatter.go @@ -2,6 +2,7 @@ package audit import ( "io" + "os" "github.com/hashicorp/vault/helper/salt" "github.com/hashicorp/vault/logical" @@ -21,6 +22,7 @@ type FormatterConfig struct { Raw bool Salt *salt.Salt HMACAccessor bool + Permissions os.FileMode // This should only ever be used in a testing context OmitTime bool diff --git a/builtin/audit/file/backend.go b/builtin/audit/file/backend.go index afdc8e019d..3b6f93c3b8 100644 --- a/builtin/audit/file/backend.go +++ b/builtin/audit/file/backend.go @@ -53,6 +53,17 @@ func Factory(conf *audit.BackendConfig) (audit.Backend, error) { } logRaw = b } + + // Check if permissions is provided + var permissions os.FileMode + permissions = 0600 + if permRaw, ok := conf.Config["permissions"]; ok { + p, err := strconv.ParseUint(permRaw, 8, 32) + if err != nil { + return nil, err + } + permissions = os.FileMode(p) + } b := &Backend{ path: path, @@ -60,6 +71,7 @@ func Factory(conf *audit.BackendConfig) (audit.Backend, error) { Raw: logRaw, Salt: conf.Salt, HMACAccessor: hmacAccessor, + Permissions: permissions, }, } @@ -131,12 +143,18 @@ func (b *Backend) open() error { if b.f != nil { return nil } - if err := os.MkdirAll(filepath.Dir(b.path), 0600); err != nil { + if err := os.MkdirAll(filepath.Dir(b.path), b.formatConfig.Permissions); err != nil { return err } var err error - b.f, err = os.OpenFile(b.path, os.O_APPEND|os.O_WRONLY|os.O_CREATE, 0600) + b.f, err = os.OpenFile(b.path, os.O_APPEND|os.O_WRONLY|os.O_CREATE, b.formatConfig.Permissions) + if err != nil { + return err + } + + // Change the file mode in case the log file already existed + err = os.Chmod(b.path, b.formatConfig.Permissions) if err != nil { return err } From 487f0d74c110be5639f8732dc2e51d72739e056e Mon Sep 17 00:00:00 2001 From: Laura Bennett Date: Fri, 7 Oct 2016 15:48:29 -0400 Subject: [PATCH 2/8] website documentation update --- website/source/docs/audit/file.html.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/website/source/docs/audit/file.html.md b/website/source/docs/audit/file.html.md index 8cf9a0d704..cb1844ff24 100644 --- a/website/source/docs/audit/file.html.md +++ b/website/source/docs/audit/file.html.md @@ -69,6 +69,12 @@ Following are the configuration options available for the backend. optional A boolean, if set, enables the hashing of token accessor. Defaults to `true`. This option is useful only when `log_raw` is `false`. + +
  • + permissions + optional + An unsigned integer, if provided, represents the file mode and + permissions. This option defaults to `0600`.
  • format From a8813c4ff21ea051656be75e8dfe7b77c79ff5c1 Mon Sep 17 00:00:00 2001 From: Laura Bennett Date: Sat, 8 Oct 2016 19:52:49 -0400 Subject: [PATCH 3/8] changes for 'mode' --- audit/formatter.go | 2 +- builtin/audit/file/backend.go | 20 ++++++++++---------- website/source/docs/audit/file.html.md | 4 ++-- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/audit/formatter.go b/audit/formatter.go index cb34957d5f..61992e5d1a 100644 --- a/audit/formatter.go +++ b/audit/formatter.go @@ -22,7 +22,7 @@ type FormatterConfig struct { Raw bool Salt *salt.Salt HMACAccessor bool - Permissions os.FileMode + Mode os.FileMode // This should only ever be used in a testing context OmitTime bool diff --git a/builtin/audit/file/backend.go b/builtin/audit/file/backend.go index 3b6f93c3b8..a54219ce8c 100644 --- a/builtin/audit/file/backend.go +++ b/builtin/audit/file/backend.go @@ -54,15 +54,15 @@ func Factory(conf *audit.BackendConfig) (audit.Backend, error) { logRaw = b } - // Check if permissions is provided - var permissions os.FileMode - permissions = 0600 - if permRaw, ok := conf.Config["permissions"]; ok { - p, err := strconv.ParseUint(permRaw, 8, 32) + // Check if mode is provided + var mode os.FileMode + mode = 0600 + if modeRaw, ok := conf.Config["mode"]; ok { + m, err := strconv.ParseUint(modeRaw, 8, 32) if err != nil { return nil, err } - permissions = os.FileMode(p) + mode = os.FileMode(m) } b := &Backend{ @@ -71,7 +71,7 @@ func Factory(conf *audit.BackendConfig) (audit.Backend, error) { Raw: logRaw, Salt: conf.Salt, HMACAccessor: hmacAccessor, - Permissions: permissions, + Mode: mode, }, } @@ -143,18 +143,18 @@ func (b *Backend) open() error { if b.f != nil { return nil } - if err := os.MkdirAll(filepath.Dir(b.path), b.formatConfig.Permissions); err != nil { + if err := os.MkdirAll(filepath.Dir(b.path), b.formatConfig.Mode); err != nil { return err } var err error - b.f, err = os.OpenFile(b.path, os.O_APPEND|os.O_WRONLY|os.O_CREATE, b.formatConfig.Permissions) + b.f, err = os.OpenFile(b.path, os.O_APPEND|os.O_WRONLY|os.O_CREATE, b.formatConfig.Mode) if err != nil { return err } // Change the file mode in case the log file already existed - err = os.Chmod(b.path, b.formatConfig.Permissions) + err = os.Chmod(b.path, b.formatConfig.Mode) if err != nil { return err } diff --git a/website/source/docs/audit/file.html.md b/website/source/docs/audit/file.html.md index cb1844ff24..ea491d3284 100644 --- a/website/source/docs/audit/file.html.md +++ b/website/source/docs/audit/file.html.md @@ -71,10 +71,10 @@ Following are the configuration options available for the backend. to `true`. This option is useful only when `log_raw` is `false`.
  • - permissions + mode optional An unsigned integer, if provided, represents the file mode and - permissions. This option defaults to `0600`. + permissions. This option defaults to `0600`.
  • format From bef5a625d6616b367143a6185321b59ba354d749 Mon Sep 17 00:00:00 2001 From: Laura Bennett Date: Sun, 9 Oct 2016 00:33:24 -0400 Subject: [PATCH 4/8] adding unit tests for file mode --- builtin/audit/file/backend_test.go | 78 ++++++++++++++++++++++++++++++ 1 file changed, 78 insertions(+) create mode 100644 builtin/audit/file/backend_test.go diff --git a/builtin/audit/file/backend_test.go b/builtin/audit/file/backend_test.go new file mode 100644 index 0000000000..7faf2d4264 --- /dev/null +++ b/builtin/audit/file/backend_test.go @@ -0,0 +1,78 @@ +package file + +import ( + "os" + "strconv" + "testing" + + "github.com/hashicorp/vault/audit" + "github.com/hashicorp/vault/helper/salt" +) + +func TestAuditFile_fileModeNew(t *testing.T) { + salter, _ := salt.NewSalt(nil, nil) + + modeStr := "0777" + mode, err := strconv.ParseUint(modeStr, 8, 32) + + file := "auditTest.txt" + + config := map[string]string{ + "path": file, + "mode": modeStr, + } + + _, err = Factory(&audit.BackendConfig{ + Salt: salter, + Config: config, + }) + + if err != nil { + t.Fatalf("%v", err) + } + + info,_ := os.Stat(file) + createdMode := info.Mode() + if createdMode != os.FileMode(mode) { + t.Fatalf("File Mode does not match.") + } + + err = os.Remove(file) +} + +func TestAuditFile_fileModeExisting(t *testing.T) { + salter, _ := salt.NewSalt(nil, nil) + + modeStr := "0600" + m, err := strconv.ParseUint(modeStr, 8, 32) + mode := os.FileMode(m) + + file := "auditTest.txt" + + f, err := os.OpenFile(file, os.O_APPEND|os.O_WRONLY|os.O_CREATE, 0777) + err = f.Close() + if err != nil { + t.Fatalf("Failure to close the file.") + } + + config := map[string]string{ + "path": file, + } + + _, err = Factory(&audit.BackendConfig{ + Salt: salter, + Config: config, + }) + + if err != nil { + t.Fatalf("%v", err) + } + + info,_ := os.Stat(file) + createdMode := info.Mode() + if createdMode != mode { + t.Fatalf("File Mode does not match.") + } + + err = os.Remove(file) +} \ No newline at end of file From 3bf0520bbbe5529d18fd1e66691e439b4aa539c8 Mon Sep 17 00:00:00 2001 From: Laura Bennett Date: Sun, 9 Oct 2016 22:23:30 -0400 Subject: [PATCH 5/8] address feedback --- builtin/audit/file/backend_test.go | 153 ++++++++++++------------- website/source/docs/audit/file.html.md | 5 +- 2 files changed, 78 insertions(+), 80 deletions(-) diff --git a/builtin/audit/file/backend_test.go b/builtin/audit/file/backend_test.go index 7faf2d4264..f21e33b495 100644 --- a/builtin/audit/file/backend_test.go +++ b/builtin/audit/file/backend_test.go @@ -1,78 +1,75 @@ -package file - -import ( - "os" - "strconv" - "testing" - - "github.com/hashicorp/vault/audit" - "github.com/hashicorp/vault/helper/salt" -) - -func TestAuditFile_fileModeNew(t *testing.T) { - salter, _ := salt.NewSalt(nil, nil) - - modeStr := "0777" - mode, err := strconv.ParseUint(modeStr, 8, 32) - - file := "auditTest.txt" - - config := map[string]string{ - "path": file, - "mode": modeStr, - } - - _, err = Factory(&audit.BackendConfig{ - Salt: salter, - Config: config, - }) - - if err != nil { - t.Fatalf("%v", err) - } - - info,_ := os.Stat(file) - createdMode := info.Mode() - if createdMode != os.FileMode(mode) { - t.Fatalf("File Mode does not match.") - } - - err = os.Remove(file) -} - -func TestAuditFile_fileModeExisting(t *testing.T) { - salter, _ := salt.NewSalt(nil, nil) - - modeStr := "0600" - m, err := strconv.ParseUint(modeStr, 8, 32) - mode := os.FileMode(m) - - file := "auditTest.txt" - - f, err := os.OpenFile(file, os.O_APPEND|os.O_WRONLY|os.O_CREATE, 0777) - err = f.Close() - if err != nil { - t.Fatalf("Failure to close the file.") - } - - config := map[string]string{ - "path": file, - } - - _, err = Factory(&audit.BackendConfig{ - Salt: salter, - Config: config, - }) - - if err != nil { - t.Fatalf("%v", err) - } - - info,_ := os.Stat(file) - createdMode := info.Mode() - if createdMode != mode { - t.Fatalf("File Mode does not match.") - } - - err = os.Remove(file) -} \ No newline at end of file +package file + +import ( + "os" + "strconv" + "testing" + + "github.com/hashicorp/vault/audit" + "github.com/hashicorp/vault/helper/salt" +) + +func TestAuditFile_fileModeNew(t *testing.T) { + salter, _ := salt.NewSalt(nil, nil) + + modeStr := "0777" + mode, err := strconv.ParseUint(modeStr, 8, 32) + + file := "auditTest.txt" + + config := map[string]string{ + "path": file, + "mode": modeStr, + } + + _, err = Factory(&audit.BackendConfig{ + Salt: salter, + Config: config, + }) + if err != nil { + t.Fatalf(err) + } + defer os.Remove(file) + + info, _ := os.Stat(file) + createdMode := info.Mode() + if createdMode != os.FileMode(mode) { + t.Fatalf("File Mode does not match.") + } +} + +func TestAuditFile_fileModeExisting(t *testing.T) { + salter, _ := salt.NewSalt(nil, nil) + + mode := os.FileMode(0600) + + file := "auditTest.txt" + + f, err := os.OpenFile(file, os.O_APPEND|os.O_WRONLY|os.O_CREATE, 0777) + err = f.Close() + if err != nil { + t.Fatalf("Failure to close the file.") + } + defer os.Remove(file) + + config := map[string]string{ + "path": file, + } + + _, err = Factory(&audit.BackendConfig{ + Salt: salter, + Config: config, + }) + if err != nil { + t.Fatalf(err) + } + + info, err := os.Stat(file) + if err != nil { + t.Fatalf("cannot retrieve file mode from `Stat`") + } + createdMode := info.Mode() + if createdMode != mode { + t.Fatalf("File Mode does not match.") + } +} diff --git a/website/source/docs/audit/file.html.md b/website/source/docs/audit/file.html.md index ea491d3284..159197a246 100644 --- a/website/source/docs/audit/file.html.md +++ b/website/source/docs/audit/file.html.md @@ -73,8 +73,9 @@ Following are the configuration options available for the backend.
  • mode optional - An unsigned integer, if provided, represents the file mode and - permissions. This option defaults to `0600`. + A string containing an octal number representing the bit pattern + for the file mode, similar to `chmod`. This option defaults to + `0600`.
  • format From 18028ffcd6d1fbc5bf9b0dbfba15a088e8025864 Mon Sep 17 00:00:00 2001 From: Laura Bennett Date: Mon, 10 Oct 2016 10:05:36 -0400 Subject: [PATCH 6/8] minor fix --- builtin/audit/file/backend_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/builtin/audit/file/backend_test.go b/builtin/audit/file/backend_test.go index f21e33b495..095e83ee3b 100644 --- a/builtin/audit/file/backend_test.go +++ b/builtin/audit/file/backend_test.go @@ -27,7 +27,7 @@ func TestAuditFile_fileModeNew(t *testing.T) { Config: config, }) if err != nil { - t.Fatalf(err) + t.Fatal(err) } defer os.Remove(file) @@ -61,7 +61,7 @@ func TestAuditFile_fileModeExisting(t *testing.T) { Config: config, }) if err != nil { - t.Fatalf(err) + t.Fatal(err) } info, err := os.Stat(file) From 7def50799bfe9e79167b55d653d6b6cd91242227 Mon Sep 17 00:00:00 2001 From: Laura Bennett Date: Mon, 10 Oct 2016 11:58:26 -0400 Subject: [PATCH 7/8] address latest feedback --- audit/formatter.go | 2 -- builtin/audit/file/backend.go | 16 ++++++++-------- builtin/audit/file/backend_test.go | 28 ++++++++++++++++------------ 3 files changed, 24 insertions(+), 22 deletions(-) diff --git a/audit/formatter.go b/audit/formatter.go index 61992e5d1a..318bd1bc59 100644 --- a/audit/formatter.go +++ b/audit/formatter.go @@ -2,7 +2,6 @@ package audit import ( "io" - "os" "github.com/hashicorp/vault/helper/salt" "github.com/hashicorp/vault/logical" @@ -22,7 +21,6 @@ type FormatterConfig struct { Raw bool Salt *salt.Salt HMACAccessor bool - Mode os.FileMode // This should only ever be used in a testing context OmitTime bool diff --git a/builtin/audit/file/backend.go b/builtin/audit/file/backend.go index a54219ce8c..0318052a4b 100644 --- a/builtin/audit/file/backend.go +++ b/builtin/audit/file/backend.go @@ -53,10 +53,9 @@ func Factory(conf *audit.BackendConfig) (audit.Backend, error) { } logRaw = b } - + // Check if mode is provided - var mode os.FileMode - mode = 0600 + mode := os.FileMode(0600) if modeRaw, ok := conf.Config["mode"]; ok { m, err := strconv.ParseUint(modeRaw, 8, 32) if err != nil { @@ -67,11 +66,11 @@ func Factory(conf *audit.BackendConfig) (audit.Backend, error) { b := &Backend{ path: path, + mode: mode, formatConfig: audit.FormatterConfig{ Raw: logRaw, Salt: conf.Salt, HMACAccessor: hmacAccessor, - Mode: mode, }, } @@ -105,6 +104,7 @@ type Backend struct { fileLock sync.RWMutex f *os.File + mode os.FileMode } func (b *Backend) GetHash(data string) string { @@ -143,18 +143,18 @@ func (b *Backend) open() error { if b.f != nil { return nil } - if err := os.MkdirAll(filepath.Dir(b.path), b.formatConfig.Mode); err != nil { + if err := os.MkdirAll(filepath.Dir(b.path), b.mode); err != nil { return err } var err error - b.f, err = os.OpenFile(b.path, os.O_APPEND|os.O_WRONLY|os.O_CREATE, b.formatConfig.Mode) + b.f, err = os.OpenFile(b.path, os.O_APPEND|os.O_WRONLY|os.O_CREATE, b.mode) if err != nil { return err } - + // Change the file mode in case the log file already existed - err = os.Chmod(b.path, b.formatConfig.Mode) + err = os.Chmod(b.path, b.mode) if err != nil { return err } diff --git a/builtin/audit/file/backend_test.go b/builtin/audit/file/backend_test.go index 095e83ee3b..fbad8bef7b 100644 --- a/builtin/audit/file/backend_test.go +++ b/builtin/audit/file/backend_test.go @@ -1,7 +1,9 @@ package file import ( + "io/ioutil" "os" + "path/filepath" "strconv" "testing" @@ -15,7 +17,10 @@ func TestAuditFile_fileModeNew(t *testing.T) { modeStr := "0777" mode, err := strconv.ParseUint(modeStr, 8, 32) - file := "auditTest.txt" + path, err := ioutil.TempDir("", "test") + defer os.RemoveAll(path) + + file := filepath.Join(path, "auditTest.txt") config := map[string]string{ "path": file, @@ -34,26 +39,25 @@ func TestAuditFile_fileModeNew(t *testing.T) { info, _ := os.Stat(file) createdMode := info.Mode() if createdMode != os.FileMode(mode) { - t.Fatalf("File Mode does not match.") + t.Fatalf("File mode does not match.") } } func TestAuditFile_fileModeExisting(t *testing.T) { salter, _ := salt.NewSalt(nil, nil) - mode := os.FileMode(0600) - - file := "auditTest.txt" - - f, err := os.OpenFile(file, os.O_APPEND|os.O_WRONLY|os.O_CREATE, 0777) + f, err := ioutil.TempFile("", "test") + if err != nil { + t.Fatalf("Failure to create test file.") + } + defer os.Remove(f.Name()) err = f.Close() if err != nil { t.Fatalf("Failure to close the file.") } - defer os.Remove(file) config := map[string]string{ - "path": file, + "path": f.Name(), } _, err = Factory(&audit.BackendConfig{ @@ -64,12 +68,12 @@ func TestAuditFile_fileModeExisting(t *testing.T) { t.Fatal(err) } - info, err := os.Stat(file) + info, err := os.Stat(f.Name()) if err != nil { t.Fatalf("cannot retrieve file mode from `Stat`") } createdMode := info.Mode() - if createdMode != mode { - t.Fatalf("File Mode does not match.") + if createdMode != os.FileMode(0600) { + t.Fatalf("File mode does not match.") } } From 6770545cfd8971444b00631c7eff89a0b2a5bfe5 Mon Sep 17 00:00:00 2001 From: Laura Bennett Date: Mon, 10 Oct 2016 12:58:30 -0400 Subject: [PATCH 8/8] test updates to address feedback --- builtin/audit/file/backend_test.go | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/builtin/audit/file/backend_test.go b/builtin/audit/file/backend_test.go index fbad8bef7b..0a1a8c7751 100644 --- a/builtin/audit/file/backend_test.go +++ b/builtin/audit/file/backend_test.go @@ -17,7 +17,7 @@ func TestAuditFile_fileModeNew(t *testing.T) { modeStr := "0777" mode, err := strconv.ParseUint(modeStr, 8, 32) - path, err := ioutil.TempDir("", "test") + path, err := ioutil.TempDir("", "vault-test_audit_file-file_mode_new") defer os.RemoveAll(path) file := filepath.Join(path, "auditTest.txt") @@ -34,11 +34,12 @@ func TestAuditFile_fileModeNew(t *testing.T) { if err != nil { t.Fatal(err) } - defer os.Remove(file) - info, _ := os.Stat(file) - createdMode := info.Mode() - if createdMode != os.FileMode(mode) { + info, err := os.Stat(file) + if err != nil { + t.Fatalf("Cannot retrieve file mode from `Stat`") + } + if info.Mode() != os.FileMode(mode) { t.Fatalf("File mode does not match.") } } @@ -51,9 +52,15 @@ func TestAuditFile_fileModeExisting(t *testing.T) { t.Fatalf("Failure to create test file.") } defer os.Remove(f.Name()) + + err = os.Chmod(f.Name(), 0777) + if err != nil { + t.Fatalf("Failure to chmod temp file for testing.") + } + err = f.Close() if err != nil { - t.Fatalf("Failure to close the file.") + t.Fatalf("Failure to close temp file for test.") } config := map[string]string{ @@ -72,8 +79,7 @@ func TestAuditFile_fileModeExisting(t *testing.T) { if err != nil { t.Fatalf("cannot retrieve file mode from `Stat`") } - createdMode := info.Mode() - if createdMode != os.FileMode(0600) { + if info.Mode() != os.FileMode(0600) { t.Fatalf("File mode does not match.") } }