diff --git a/cmd/fleet/serve.go b/cmd/fleet/serve.go index 08350ffdfeb..e85ca2852df 100644 --- a/cmd/fleet/serve.go +++ b/cmd/fleet/serve.go @@ -191,11 +191,8 @@ func runServeCmd(cmd *cobra.Command, configManager configpkg.Manager, debug, dev } // Validate OTEL server options - if config.Logging.OtelLogsEnabled && !config.Logging.TracingEnabled { - initFatal( - errors.New("logging.otel_logs_enabled requires logging.tracing_enabled to be true"), - "OTEL logs require tracing for trace correlation", - ) + if err := validateOTELLoggingConfig(config); err != nil { + initFatal(err, "OTEL logs require tracing for trace correlation") } // Init OTEL providers (traces, metrics, logs) @@ -317,36 +314,19 @@ func runServeCmd(cmd *cobra.Command, configManager configpkg.Manager, debug, dev createTestBuckets(cmd.Context(), &config, logger) } - allowedHostIdentifiers := map[string]bool{ - "provided": true, - "instance": true, - "uuid": true, - "hostname": true, - } - if !allowedHostIdentifiers[config.Osquery.HostIdentifier] { - initFatal(fmt.Errorf("%s is not a valid value for osquery_host_identifier", config.Osquery.HostIdentifier), "set host identifier") + if err := validateOsqueryHostIdentifier(config); err != nil { + initFatal(err, "set host identifier") } config.ConditionalAccess.Validate(initFatal) - if len(config.Server.URLPrefix) > 0 { - // Massage provided prefix to match expected format - config.Server.URLPrefix = strings.TrimSuffix(config.Server.URLPrefix, "/") - if len(config.Server.URLPrefix) > 0 && !strings.HasPrefix(config.Server.URLPrefix, "/") { - config.Server.URLPrefix = "/" + config.Server.URLPrefix - } - - if !allowedURLPrefixRegexp.MatchString(config.Server.URLPrefix) { - initFatal( - fmt.Errorf("prefix must match regexp \"%s\"", allowedURLPrefixRegexp.String()), - "setting server URL prefix", - ) - } + if err := normalizeAndValidateServerURLPrefix(&config); err != nil { + initFatal(err, "setting server URL prefix") } // Handle server private key configuration - either direct or via AWS Secrets Manager - if config.Server.PrivateKey != "" && config.Server.PrivateKeySecretArn != "" { - initFatal(errors.New("cannot specify both private_key and private_key_secret_arn"), "validate private key configuration") + if err := validateServerPrivateKeyExclusive(config); err != nil { + initFatal(err, "validate private key configuration") } // Retrieve private key from AWS Secrets Manager if specified @@ -364,11 +344,10 @@ func runServeCmd(cmd *cobra.Command, configManager configpkg.Manager, debug, dev config.Server.PrivateKey = privateKey } + if err := validateServerPrivateKeyLength(config); err != nil { + initFatal(err, "validate private key") + } if len(config.Server.PrivateKey) > 0 { - if len(config.Server.PrivateKey) < 32 { - initFatal(errors.New("private key must be at least 32 bytes long"), "validate private key") - } - // We truncate to 32 bytes because AES-256 requires a 32 byte (256 bit) PK, but some // infra setups generate keys that are longer than 32 bytes. config.Server.PrivateKey = config.Server.PrivateKey[:32] diff --git a/cmd/fleet/serve_validation.go b/cmd/fleet/serve_validation.go new file mode 100644 index 00000000000..460a60b54fa --- /dev/null +++ b/cmd/fleet/serve_validation.go @@ -0,0 +1,74 @@ +package main + +import ( + "errors" + "fmt" + "strings" + + "github.com/fleetdm/fleet/v4/server/config" +) + +// validateServerPrivateKeyExclusive enforces that the server private key may +// be supplied either directly via private_key or via private_key_arn (AWS +// Secrets Manager), but not both. +func validateServerPrivateKeyExclusive(cfg config.FleetConfig) error { + if cfg.Server.PrivateKey != "" && cfg.Server.PrivateKeySecretArn != "" { + return errors.New("cannot specify both private_key and private_key_secret_arn") + } + return nil +} + +// validateServerPrivateKeyLength enforces a 32-byte minimum length on the +// server private key when one is configured. The key is truncated to 32 +// bytes after this check because AES-256 requires a 32-byte key; rejecting +// shorter inputs prevents a silently invalid AES-256 setup. +func validateServerPrivateKeyLength(cfg config.FleetConfig) error { + if len(cfg.Server.PrivateKey) > 0 && len(cfg.Server.PrivateKey) < 32 { + return errors.New("private key must be at least 32 bytes long") + } + return nil +} + +// validateOsqueryHostIdentifier rejects osquery_host_identifier values +// outside the supported set. The osquery agent uses this to determine which +// identifier is reported as the host UUID. +func validateOsqueryHostIdentifier(cfg config.FleetConfig) error { + allowed := map[string]struct{}{ + "provided": {}, + "instance": {}, + "uuid": {}, + "hostname": {}, + } + if _, ok := allowed[cfg.Osquery.HostIdentifier]; !ok { + return fmt.Errorf("%s is not a valid value for osquery_host_identifier", cfg.Osquery.HostIdentifier) + } + return nil +} + +// validateOTELLoggingConfig enforces the dependency between OTEL logs and +// tracing: log records carry trace IDs only when tracing is enabled, so +// enabling logs without tracing produces correlation-broken telemetry. +func validateOTELLoggingConfig(cfg config.FleetConfig) error { + if cfg.Logging.OtelLogsEnabled && !cfg.Logging.TracingEnabled { + return errors.New("logging.otel_logs_enabled requires logging.tracing_enabled to be true") + } + return nil +} + +// normalizeAndValidateServerURLPrefix trims a trailing slash, ensures a +// leading slash, and validates the resulting prefix against +// allowedURLPrefixRegexp. The mutation is done in place because the +// normalized value is consumed downstream as part of route registration. +func normalizeAndValidateServerURLPrefix(cfg *config.FleetConfig) error { + if len(cfg.Server.URLPrefix) == 0 { + return nil + } + cfg.Server.URLPrefix = strings.TrimSuffix(cfg.Server.URLPrefix, "/") + if len(cfg.Server.URLPrefix) > 0 && !strings.HasPrefix(cfg.Server.URLPrefix, "/") { + cfg.Server.URLPrefix = "/" + cfg.Server.URLPrefix + } + if !allowedURLPrefixRegexp.MatchString(cfg.Server.URLPrefix) { + return fmt.Errorf("prefix must match regexp %q", allowedURLPrefixRegexp.String()) + } + return nil +} diff --git a/cmd/fleet/serve_validation_test.go b/cmd/fleet/serve_validation_test.go new file mode 100644 index 00000000000..de2d5b45811 --- /dev/null +++ b/cmd/fleet/serve_validation_test.go @@ -0,0 +1,165 @@ +package main + +import ( + "strings" + "testing" + + "github.com/fleetdm/fleet/v4/server/config" + "github.com/stretchr/testify/require" +) + +func TestValidateServerPrivateKeyExclusive(t *testing.T) { + for _, tc := range []struct { + name string + privateKey string + privateKeyArn string + wantErrSubstring string + }{ + {"both empty", "", "", ""}, + {"direct key only", "some-direct-key-value", "", ""}, + {"arn only", "", "arn:aws:secretsmanager:us-east-1:123:secret:foo", ""}, + { + "both set rejected", + "some-direct-key-value", + "arn:aws:secretsmanager:us-east-1:123:secret:foo", + "cannot specify both private_key and private_key_secret_arn", + }, + } { + t.Run(tc.name, func(t *testing.T) { + cfg := config.FleetConfig{ + Server: config.ServerConfig{ + PrivateKey: tc.privateKey, + PrivateKeySecretArn: tc.privateKeyArn, + }, + } + err := validateServerPrivateKeyExclusive(cfg) + if tc.wantErrSubstring == "" { + require.NoError(t, err) + return + } + require.Error(t, err) + require.Contains(t, err.Error(), tc.wantErrSubstring) + }) + } +} + +func TestValidateServerPrivateKeyLength(t *testing.T) { + for _, tc := range []struct { + name string + privateKey string + wantErrSubstring string + }{ + {"empty key", "", ""}, + {"key under minimum length", strings.Repeat("x", 16), "at least 32 bytes long"}, + {"key one byte under minimum", strings.Repeat("x", 31), "at least 32 bytes long"}, + {"key at exactly minimum length", strings.Repeat("x", 32), ""}, + {"key over minimum length", strings.Repeat("x", 64), ""}, + } { + t.Run(tc.name, func(t *testing.T) { + cfg := config.FleetConfig{ + Server: config.ServerConfig{ + PrivateKey: tc.privateKey, + }, + } + err := validateServerPrivateKeyLength(cfg) + if tc.wantErrSubstring == "" { + require.NoError(t, err) + return + } + require.Error(t, err) + require.Contains(t, err.Error(), tc.wantErrSubstring) + }) + } +} + +func TestValidateOsqueryHostIdentifier(t *testing.T) { + for _, tc := range []struct { + name string + identifier string + wantErrSubstring string + }{ + {"provided is allowed", "provided", ""}, + {"instance is allowed", "instance", ""}, + {"uuid is allowed", "uuid", ""}, + {"hostname is allowed", "hostname", ""}, + {"empty string rejected", "", "is not a valid value for osquery_host_identifier"}, + {"unknown value rejected", "serial", "is not a valid value for osquery_host_identifier"}, + {"case-sensitive: UUID rejected", "UUID", "is not a valid value for osquery_host_identifier"}, + } { + t.Run(tc.name, func(t *testing.T) { + cfg := config.FleetConfig{ + Osquery: config.OsqueryConfig{HostIdentifier: tc.identifier}, + } + err := validateOsqueryHostIdentifier(cfg) + if tc.wantErrSubstring == "" { + require.NoError(t, err) + return + } + require.Error(t, err) + require.Contains(t, err.Error(), tc.wantErrSubstring) + }) + } +} + +func TestValidateOTELLoggingConfig(t *testing.T) { + for _, tc := range []struct { + name string + otelLogs bool + tracing bool + wantErrSubstring string + }{ + {"both disabled", false, false, ""}, + {"tracing only", false, true, ""}, + {"both enabled", true, true, ""}, + {"otel logs without tracing rejected", true, false, "logging.otel_logs_enabled requires logging.tracing_enabled"}, + } { + t.Run(tc.name, func(t *testing.T) { + cfg := config.FleetConfig{ + Logging: config.LoggingConfig{ + OtelLogsEnabled: tc.otelLogs, + TracingEnabled: tc.tracing, + }, + } + err := validateOTELLoggingConfig(cfg) + if tc.wantErrSubstring == "" { + require.NoError(t, err) + return + } + require.Error(t, err) + require.Contains(t, err.Error(), tc.wantErrSubstring) + }) + } +} + +func TestNormalizeAndValidateServerURLPrefix(t *testing.T) { + for _, tc := range []struct { + name string + input string + wantNormalized string + wantErrSubstring string + }{ + {"empty prefix is left alone", "", "", ""}, + {"slash-prefixed value passes unchanged", "/fleet", "/fleet", ""}, + {"leading slash added", "fleet", "/fleet", ""}, + {"trailing slash trimmed", "/fleet/", "/fleet", ""}, + {"both fixes applied", "fleet/", "/fleet", ""}, + {"nested path is allowed", "/api/v1", "/api/v1", ""}, + {"single slash trims to empty and is rejected by regex", "/", "", "must match regexp"}, + {"invalid character rejected", "/fleet space", "", "must match regexp"}, + {"query-style suffix rejected", "/fleet?x=1", "", "must match regexp"}, + } { + t.Run(tc.name, func(t *testing.T) { + cfg := config.FleetConfig{ + Server: config.ServerConfig{URLPrefix: tc.input}, + } + err := normalizeAndValidateServerURLPrefix(&cfg) + if tc.wantErrSubstring == "" { + require.NoError(t, err) + require.Equal(t, tc.wantNormalized, cfg.Server.URLPrefix) + return + } + require.Error(t, err) + require.Contains(t, err.Error(), tc.wantErrSubstring) + }) + } +}