Skip to content

Commit 63ba96c

Browse files
authored
Add additional validations to the destination and properties of file audit sinks (#31211)
* Add additional validations to the destination and properties of file audit sinks * changelog * docs * Revert "docs" This reverts commit c2e8f76.
1 parent 75e1108 commit 63ba96c

12 files changed

Lines changed: 86 additions & 8 deletions

audit/backend.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ const (
2525
optionFormat = "format"
2626
optionHMACAccessor = "hmac_accessor"
2727
optionLogRaw = "log_raw"
28-
optionPrefix = "prefix"
28+
OptionPrefix = "prefix"
2929

3030
TypeFile = "file"
3131
TypeSocket = "socket"

audit/backend_file.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ package audit
66
import (
77
"fmt"
88
"reflect"
9+
"strconv"
910
"strings"
1011

1112
"github.com/hashicorp/eventlogger"
@@ -78,6 +79,15 @@ func newFileBackend(conf *BackendConfig, headersConfig HeaderFormatter) (*fileBa
7879

7980
sinkOpts := []event.Option{event.WithLogger(conf.Logger)}
8081
if mode, ok := conf.Config[optionMode]; ok {
82+
if strings.TrimSpace(mode) != "" {
83+
m, err := strconv.ParseUint(mode, 8, 32)
84+
if err != nil {
85+
return nil, fmt.Errorf("invalid mode: %s", mode)
86+
}
87+
if m&0o111 != 0 {
88+
return nil, fmt.Errorf("file mode may not be executable: %s", mode)
89+
}
90+
}
8191
sinkOpts = append(sinkOpts, event.WithFileMode(mode))
8292
}
8393

audit/backend_file_ce_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ func TestFileBackend_newFileBackend_FilterFormatterSink(t *testing.T) {
8484

8585
cfg := map[string]string{
8686
"file_path": "/tmp/foo",
87-
"mode": "0777",
87+
"mode": "0666",
8888
"format": "json",
8989
"filter": "mount_type == \"kv\"",
9090
}

audit/backend_file_test.go

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import (
2222
func TestAuditFile_fileModeNew(t *testing.T) {
2323
t.Parallel()
2424

25-
modeStr := "0777"
25+
modeStr := "0666"
2626
mode, err := strconv.ParseUint(modeStr, 8, 32)
2727
require.NoError(t, err)
2828

@@ -55,7 +55,7 @@ func TestAuditFile_fileModeExisting(t *testing.T) {
5555
f, err := os.CreateTemp(dir, "auditTest.log")
5656
require.NoErrorf(t, err, "Failure to create test file.")
5757

58-
err = os.Chmod(f.Name(), 0o777)
58+
err = os.Chmod(f.Name(), 0o666)
5959
require.NoErrorf(t, err, "Failure to chmod temp file for testing.")
6060

6161
err = f.Close()
@@ -117,7 +117,7 @@ func TestAuditFile_fileMode0000(t *testing.T) {
117117
// correctly sets the file mode when the useEventLogger argument is set to
118118
// true.
119119
func TestAuditFile_EventLogger_fileModeNew(t *testing.T) {
120-
modeStr := "0777"
120+
modeStr := "0666"
121121
mode, err := strconv.ParseUint(modeStr, 8, 32)
122122
require.NoError(t, err)
123123

@@ -140,6 +140,16 @@ func TestAuditFile_EventLogger_fileModeNew(t *testing.T) {
140140
info, err := os.Stat(file)
141141
require.NoError(t, err)
142142
require.Equalf(t, os.FileMode(mode), info.Mode(), "File mode does not match.")
143+
144+
for _, modeStr := range []string{"0667", "0676", "0766", "0677", "0776", "0777"} {
145+
mode, err = strconv.ParseUint(modeStr, 8, 32)
146+
require.NoError(t, err)
147+
148+
// Test that executable audit files are disallowed
149+
backendConfig.Config["mode"] = modeStr
150+
_, err = newFileBackend(backendConfig, &noopHeaderFormatter{})
151+
require.Error(t, err)
152+
}
143153
}
144154

145155
// TestFileBackend_newFileBackend ensures that we can correctly configure the sink

audit/entry_formatter_config.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -94,7 +94,7 @@ func newFormatterConfig(headerFormatter HeaderFormatter, config map[string]strin
9494
opt = append(opt, withElision(v))
9595
}
9696

97-
if prefix, ok := config[optionPrefix]; ok {
97+
if prefix, ok := config[OptionPrefix]; ok {
9898
opt = append(opt, withPrefix(prefix))
9999
}
100100

changelog/31211.txt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
```release-note:improvement
2+
audit: Add additional verifications to the target of file audit sinks.
3+
```

command/server.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2945,6 +2945,7 @@ func createCoreConfig(c *ServerCommand, config *server.Config, backend physical.
29452945
DisableSealWrap: config.DisableSealWrap,
29462946
DisablePerformanceStandby: config.DisablePerformanceStandby,
29472947
DisableIndexing: config.DisableIndexing,
2948+
AllowAuditLogPrefixing: config.AllowAuditLogPrefixing,
29482949
AllLoggers: c.allLoggers,
29492950
BuiltinRegistry: builtinplugins.Registry,
29502951
DisableKeyEncodingChecks: config.DisablePrintableCheck,

command/server/config.go

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,9 @@ type Config struct {
9494
DisableClustering bool `hcl:"-"`
9595
DisableClusteringRaw interface{} `hcl:"disable_clustering,alias:DisableClustering"`
9696

97+
AllowAuditLogPrefixing bool `hcl:"-"`
98+
AllowAuditLogPrefixingRaw interface{} `hcl:"allow_audit_log_prefixing,alias:AllowAuditLogPrefixing"`
99+
97100
DisablePerformanceStandby bool `hcl:"-"`
98101
DisablePerformanceStandbyRaw interface{} `hcl:"disable_performance_standby,alias:DisablePerformanceStandby"`
99102

@@ -402,6 +405,11 @@ func (c *Config) Merge(c2 *Config) *Config {
402405
result.DisablePerformanceStandby = c2.DisablePerformanceStandby
403406
}
404407

408+
result.AllowAuditLogPrefixing = c.AllowAuditLogPrefixing
409+
if c2.AllowAuditLogPrefixing {
410+
result.AllowAuditLogPrefixing = c2.AllowAuditLogPrefixing
411+
}
412+
405413
result.DisableSealWrap = c.DisableSealWrap
406414
if c2.DisableSealWrap {
407415
result.DisableSealWrap = c2.DisableSealWrap
@@ -752,6 +760,12 @@ func ParseConfigCheckDuplicate(d, source string) (cfg *Config, duplicate bool, e
752760
}
753761
}
754762

763+
if result.AllowAuditLogPrefixingRaw != nil {
764+
if result.AllowAuditLogPrefixing, err = parseutil.ParseBool(result.AllowAuditLogPrefixingRaw); err != nil {
765+
return nil, duplicate, err
766+
}
767+
}
768+
755769
if result.DisableSealWrapRaw != nil {
756770
if result.DisableSealWrap, err = parseutil.ParseBool(result.DisableSealWrapRaw); err != nil {
757771
return nil, duplicate, err
@@ -1377,8 +1391,8 @@ func (c *Config) Sanitized() map[string]interface{} {
13771391

13781392
"disable_sealwrap": c.DisableSealWrap,
13791393

1380-
"disable_indexing": c.DisableIndexing,
1381-
1394+
"disable_indexing": c.DisableIndexing,
1395+
"allow_audit_log_prefixing": c.AllowAuditLogPrefixing,
13821396
"enable_response_header_hostname": c.EnableResponseHeaderHostname,
13831397

13841398
"enable_response_header_raft_node_id": c.EnableResponseHeaderRaftNodeID,

command/server/config_test_helpers.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -895,6 +895,7 @@ func testConfig_Sanitized(t *testing.T) {
895895
"enable_post_unseal_trace": true,
896896
"post_unseal_trace_directory": "/tmp",
897897
"remove_irrevocable_lease_after": (30 * 24 * time.Hour) / time.Second,
898+
"allow_audit_log_prefixing": false,
898899
}
899900

900901
addExpectedEntSanitizedConfig(expected, []string{"http"})

http/sys_config_state_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,7 @@ func TestSysConfigState_Sanitized(t *testing.T) {
182182
"enable_post_unseal_trace": false,
183183
"post_unseal_trace_directory": "",
184184
"remove_irrevocable_lease_after": json.Number("0"),
185+
"allow_audit_log_prefixing": false,
185186
}
186187

187188
if tc.expectedHAStorageOutput != nil {

0 commit comments

Comments
 (0)