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 c2e8f7608e.

audit/backend.go

@@ -25,7 +25,7 @@ const (
 	optionFormat             = "format"
 	optionHMACAccessor       = "hmac_accessor"
 	optionLogRaw             = "log_raw"
-	optionPrefix             = "prefix"
+	OptionPrefix             = "prefix"
 
 	TypeFile   = "file"
 	TypeSocket = "socket"

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

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\"",
 	}

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

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

changelog/31211.txt

@@ -0,0 +1,3 @@
+```release-note:improvement
+audit: Add additional verifications to the target of file audit sinks.
+```

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,

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,

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"})

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 {

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()
 

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{},