diff --git a/audit/backend.go b/audit/backend.go index 4bce4b6edb..29be31065b 100644 --- a/audit/backend.go +++ b/audit/backend.go @@ -25,7 +25,7 @@ const ( optionFormat = "format" optionHMACAccessor = "hmac_accessor" optionLogRaw = "log_raw" - optionPrefix = "prefix" + OptionPrefix = "prefix" TypeFile = "file" TypeSocket = "socket" diff --git a/audit/backend_file.go b/audit/backend_file.go index b321c66176..8d35b72f7a 100644 --- a/audit/backend_file.go +++ b/audit/backend_file.go @@ -6,6 +6,7 @@ package audit import ( "fmt" "reflect" + "strconv" "strings" "github.com/hashicorp/eventlogger" @@ -78,6 +79,15 @@ func newFileBackend(conf *BackendConfig, headersConfig HeaderFormatter) (*fileBa sinkOpts := []event.Option{event.WithLogger(conf.Logger)} if mode, ok := conf.Config[optionMode]; ok { + if strings.TrimSpace(mode) != "" { + m, err := strconv.ParseUint(mode, 8, 32) + if err != nil { + return nil, fmt.Errorf("invalid mode: %s", mode) + } + if m&0o111 != 0 { + return nil, fmt.Errorf("file mode may not be executable: %s", mode) + } + } sinkOpts = append(sinkOpts, event.WithFileMode(mode)) } diff --git a/audit/backend_file_ce_test.go b/audit/backend_file_ce_test.go index 732fbc7eb4..c682695f36 100644 --- a/audit/backend_file_ce_test.go +++ b/audit/backend_file_ce_test.go @@ -84,7 +84,7 @@ func TestFileBackend_newFileBackend_FilterFormatterSink(t *testing.T) { cfg := map[string]string{ "file_path": "/tmp/foo", - "mode": "0777", + "mode": "0666", "format": "json", "filter": "mount_type == \"kv\"", } diff --git a/audit/backend_file_test.go b/audit/backend_file_test.go index a69edc469d..47a22a8fcc 100644 --- a/audit/backend_file_test.go +++ b/audit/backend_file_test.go @@ -22,7 +22,7 @@ import ( func TestAuditFile_fileModeNew(t *testing.T) { t.Parallel() - modeStr := "0777" + modeStr := "0666" mode, err := strconv.ParseUint(modeStr, 8, 32) require.NoError(t, err) @@ -55,7 +55,7 @@ func TestAuditFile_fileModeExisting(t *testing.T) { f, err := os.CreateTemp(dir, "auditTest.log") require.NoErrorf(t, err, "Failure to create test file.") - err = os.Chmod(f.Name(), 0o777) + err = os.Chmod(f.Name(), 0o666) require.NoErrorf(t, err, "Failure to chmod temp file for testing.") err = f.Close() @@ -117,7 +117,7 @@ func TestAuditFile_fileMode0000(t *testing.T) { // correctly sets the file mode when the useEventLogger argument is set to // true. func TestAuditFile_EventLogger_fileModeNew(t *testing.T) { - modeStr := "0777" + modeStr := "0666" mode, err := strconv.ParseUint(modeStr, 8, 32) require.NoError(t, err) @@ -140,6 +140,16 @@ func TestAuditFile_EventLogger_fileModeNew(t *testing.T) { info, err := os.Stat(file) require.NoError(t, err) require.Equalf(t, os.FileMode(mode), info.Mode(), "File mode does not match.") + + for _, modeStr := range []string{"0667", "0676", "0766", "0677", "0776", "0777"} { + mode, err = strconv.ParseUint(modeStr, 8, 32) + require.NoError(t, err) + + // Test that executable audit files are disallowed + backendConfig.Config["mode"] = modeStr + _, err = newFileBackend(backendConfig, &noopHeaderFormatter{}) + require.Error(t, err) + } } // TestFileBackend_newFileBackend ensures that we can correctly configure the sink diff --git a/audit/entry_formatter_config.go b/audit/entry_formatter_config.go index aa55a2ea2d..5b15d17395 100644 --- a/audit/entry_formatter_config.go +++ b/audit/entry_formatter_config.go @@ -94,7 +94,7 @@ func newFormatterConfig(headerFormatter HeaderFormatter, config map[string]strin opt = append(opt, withElision(v)) } - if prefix, ok := config[optionPrefix]; ok { + if prefix, ok := config[OptionPrefix]; ok { opt = append(opt, withPrefix(prefix)) } diff --git a/changelog/31211.txt b/changelog/31211.txt new file mode 100644 index 0000000000..03d19b01ea --- /dev/null +++ b/changelog/31211.txt @@ -0,0 +1,3 @@ +```release-note:improvement +audit: Add additional verifications to the target of file audit sinks. +``` diff --git a/command/server.go b/command/server.go index e564a977b3..440c68f2a5 100644 --- a/command/server.go +++ b/command/server.go @@ -2945,6 +2945,7 @@ func createCoreConfig(c *ServerCommand, config *server.Config, backend physical. DisableSealWrap: config.DisableSealWrap, DisablePerformanceStandby: config.DisablePerformanceStandby, DisableIndexing: config.DisableIndexing, + AllowAuditLogPrefixing: config.AllowAuditLogPrefixing, AllLoggers: c.allLoggers, BuiltinRegistry: builtinplugins.Registry, DisableKeyEncodingChecks: config.DisablePrintableCheck, diff --git a/command/server/config.go b/command/server/config.go index a0ca7f4a3a..eff210ab30 100644 --- a/command/server/config.go +++ b/command/server/config.go @@ -94,6 +94,9 @@ type Config struct { DisableClustering bool `hcl:"-"` DisableClusteringRaw interface{} `hcl:"disable_clustering,alias:DisableClustering"` + AllowAuditLogPrefixing bool `hcl:"-"` + AllowAuditLogPrefixingRaw interface{} `hcl:"allow_audit_log_prefixing,alias:AllowAuditLogPrefixing"` + DisablePerformanceStandby bool `hcl:"-"` DisablePerformanceStandbyRaw interface{} `hcl:"disable_performance_standby,alias:DisablePerformanceStandby"` @@ -402,6 +405,11 @@ func (c *Config) Merge(c2 *Config) *Config { result.DisablePerformanceStandby = c2.DisablePerformanceStandby } + result.AllowAuditLogPrefixing = c.AllowAuditLogPrefixing + if c2.AllowAuditLogPrefixing { + result.AllowAuditLogPrefixing = c2.AllowAuditLogPrefixing + } + result.DisableSealWrap = c.DisableSealWrap if c2.DisableSealWrap { result.DisableSealWrap = c2.DisableSealWrap @@ -752,6 +760,12 @@ func ParseConfigCheckDuplicate(d, source string) (cfg *Config, duplicate bool, e } } + if result.AllowAuditLogPrefixingRaw != nil { + if result.AllowAuditLogPrefixing, err = parseutil.ParseBool(result.AllowAuditLogPrefixingRaw); err != nil { + return nil, duplicate, err + } + } + if result.DisableSealWrapRaw != nil { if result.DisableSealWrap, err = parseutil.ParseBool(result.DisableSealWrapRaw); err != nil { return nil, duplicate, err @@ -1377,8 +1391,8 @@ func (c *Config) Sanitized() map[string]interface{} { "disable_sealwrap": c.DisableSealWrap, - "disable_indexing": c.DisableIndexing, - + "disable_indexing": c.DisableIndexing, + "allow_audit_log_prefixing": c.AllowAuditLogPrefixing, "enable_response_header_hostname": c.EnableResponseHeaderHostname, "enable_response_header_raft_node_id": c.EnableResponseHeaderRaftNodeID, diff --git a/command/server/config_test_helpers.go b/command/server/config_test_helpers.go index 0b9c7e2892..dc00be5e61 100644 --- a/command/server/config_test_helpers.go +++ b/command/server/config_test_helpers.go @@ -895,6 +895,7 @@ func testConfig_Sanitized(t *testing.T) { "enable_post_unseal_trace": true, "post_unseal_trace_directory": "/tmp", "remove_irrevocable_lease_after": (30 * 24 * time.Hour) / time.Second, + "allow_audit_log_prefixing": false, } addExpectedEntSanitizedConfig(expected, []string{"http"}) diff --git a/http/sys_config_state_test.go b/http/sys_config_state_test.go index 71cd832a67..c45cf1112c 100644 --- a/http/sys_config_state_test.go +++ b/http/sys_config_state_test.go @@ -182,6 +182,7 @@ func TestSysConfigState_Sanitized(t *testing.T) { "enable_post_unseal_trace": false, "post_unseal_trace_directory": "", "remove_irrevocable_lease_after": json.Number("0"), + "allow_audit_log_prefixing": false, } if tc.expectedHAStorageOutput != nil { diff --git a/vault/audit.go b/vault/audit.go index 478251324a..255a2f48c3 100644 --- a/vault/audit.go +++ b/vault/audit.go @@ -8,6 +8,7 @@ import ( "crypto/sha256" "errors" "fmt" + "path/filepath" "slices" "strconv" "strings" @@ -124,6 +125,36 @@ func (c *Core) enableAudit(ctx context.Context, entry *MountEntry, updateStorage entry.NamespaceID = ns.ID entry.namespace = ns + if entry.Type == "file" { + if prefix, ok := entry.Options[audit.OptionPrefix]; ok && prefix != "" && !c.allowAuditLogPrefixing { + return errors.New("audit prefixing is not enabled in configuration, audit prefixing may not be configured for file sinks") + } + + if c.pluginDirectory != "" { + // Validate that the audit log file is not in the plugin directory + auditDir := filepath.Dir(entry.Options["file_path"]) + auditDir, err = filepath.Abs(auditDir) + if err != nil { + return fmt.Errorf("error getting absolute path of audit dir for audit validation: %w", err) + } + pluginDir, err := filepath.Abs(c.pluginDirectory) + if err != nil { + return fmt.Errorf("error getting absolute path of plugin dir for audit validation: %w", err) + } + // Walk the audit path up checking that none of them are the plugin dir + for len(auditDir) > 1 || auditDir[0] != filepath.Separator { + rp, err := filepath.Rel(pluginDir, auditDir) + if err != nil { + return fmt.Errorf("error checking relative path for audit validation: %w", err) + } + if rp == "." { + return errors.New("audit file target may not be in the plugin directory") + } + auditDir = filepath.Dir(auditDir) + } + } + } + c.auditLock.Lock() defer c.auditLock.Unlock() diff --git a/vault/core.go b/vault/core.go index c954576dc2..05a0a87af3 100644 --- a/vault/core.go +++ b/vault/core.go @@ -692,6 +692,9 @@ type Core struct { // disableAutopilot is used to disable the autopilot subsystem in raft storage disableAutopilot bool + // allowAuditLogPrefixing must be enabled for audit devices to use a prefix (more secure without) + allowAuditLogPrefixing bool + // enable/disable identifying response headers enableResponseHeaderHostname bool enableResponseHeaderRaftNodeID bool @@ -906,6 +909,9 @@ type CoreConfig struct { // DisableAutopilot is used to disable autopilot subsystem in raft storage DisableAutopilot bool + // AllowAuditLogPrefixing must be enabled for audit devices to use a prefix (more secure without) + AllowAuditLogPrefixing bool + // Whether to send headers in the HTTP response showing hostname or raft node ID EnableResponseHeaderHostname bool EnableResponseHeaderRaftNodeID bool @@ -1098,6 +1104,7 @@ func CreateCore(conf *CoreConfig) (*Core, error) { numExpirationWorkers: conf.NumExpirationWorkers, raftFollowerStates: raft.NewFollowerStates(), disableAutopilot: conf.DisableAutopilot, + allowAuditLogPrefixing: conf.AllowAuditLogPrefixing, enableResponseHeaderHostname: conf.EnableResponseHeaderHostname, enableResponseHeaderRaftNodeID: conf.EnableResponseHeaderRaftNodeID, mountMigrationTracker: &sync.Map{},