diff --git a/changelog/20569.txt b/changelog/20569.txt new file mode 100644 index 000000000..e10a4643e --- /dev/null +++ b/changelog/20569.txt @@ -0,0 +1,3 @@ +```release-note:improvement +agent: Add logic to validate env_template entries in configuration +``` diff --git a/command/agent/config/config.go b/command/agent/config/config.go index 0277d6aae..424ddc909 100644 --- a/command/agent/config/config.go +++ b/command/agent/config/config.go @@ -22,6 +22,7 @@ import ( "github.com/hashicorp/hcl" "github.com/hashicorp/hcl/hcl/ast" "github.com/mitchellh/mapstructure" + "k8s.io/utils/strings/slices" "github.com/hashicorp/vault/command/agentproxyshared" "github.com/hashicorp/vault/helper/namespace" @@ -342,7 +343,8 @@ func (c *Config) ValidateConfig() error { if c.AutoAuth != nil { if len(c.AutoAuth.Sinks) == 0 && (c.APIProxy == nil || !c.APIProxy.UseAutoAuthToken) && - len(c.Templates) == 0 { + len(c.Templates) == 0 && + len(c.EnvTemplates) == 0 { return fmt.Errorf("auto_auth requires at least one sink or at least one template or api_proxy.use_auto_auth_token=true") } } @@ -351,6 +353,126 @@ func (c *Config) ValidateConfig() error { return fmt.Errorf("no auto_auth, cache, or listener block found in config") } + return c.validateEnvTemplateConfig() +} + +func (c *Config) validateEnvTemplateConfig() error { + // if we are not in env-template mode, exit early + if c.Exec == nil && len(c.EnvTemplates) == 0 { + return nil + } + + if c.Exec == nil { + return fmt.Errorf("a top-level 'exec' element must be specified with 'env_template' entries") + } + + if len(c.EnvTemplates) == 0 { + return fmt.Errorf("must specify at least one 'env_template' element with a top-level 'exec' element") + } + + if c.APIProxy != nil { + return fmt.Errorf("'api_proxy' cannot be specified with 'env_template' entries") + } + + if len(c.Templates) > 0 { + return fmt.Errorf("'template' cannot be specified with 'env_template' entries") + } + + if len(c.Exec.Command) == 0 { + return fmt.Errorf("'exec' requires a non-empty 'command' field") + } + + if !slices.Contains([]string{"always", "never"}, c.Exec.RestartOnSecretChanges) { + return fmt.Errorf("'exec.restart_on_secret_changes' unexpected value: %q", c.Exec.RestartOnSecretChanges) + } + + uniqueKeys := make(map[string]struct{}) + + for _, template := range c.EnvTemplates { + // Required: + // - the key (environment variable name) + // - either "contents" or "source" + // Optional / permitted: + // - error_on_missing_key + // - error_fatal + // - left_delimiter + // - right_delimiter + // - ExtFuncMap + // - function_denylist / function_blacklist + + if template.MapToEnvironmentVariable == nil { + return fmt.Errorf("env_template: an environment variable name is required") + } + + key := *template.MapToEnvironmentVariable + + if _, exists := uniqueKeys[key]; exists { + return fmt.Errorf("env_template: duplicate environment variable name: %q", key) + } + + uniqueKeys[key] = struct{}{} + + if template.Contents == nil && template.Source == nil { + return fmt.Errorf("env_template[%s]: either 'contents' or 'source' must be specified", key) + } + + if template.Contents != nil && template.Source != nil { + return fmt.Errorf("env_template[%s]: 'contents' and 'source' cannot be specified together", key) + } + + if template.Backup != nil { + return fmt.Errorf("env_template[%s]: 'backup' is not allowed", key) + } + + if template.Command != nil { + return fmt.Errorf("env_template[%s]: 'command' is not allowed", key) + } + + if template.CommandTimeout != nil { + return fmt.Errorf("env_template[%s]: 'command_timeout' is not allowed", key) + } + + if template.CreateDestDirs != nil { + return fmt.Errorf("env_template[%s]: 'create_dest_dirs' is not allowed", key) + } + + if template.Destination != nil { + return fmt.Errorf("env_template[%s]: 'destination' is not allowed", key) + } + + if template.Exec != nil { + return fmt.Errorf("env_template[%s]: 'exec' is not allowed", key) + } + + if template.Perms != nil { + return fmt.Errorf("env_template[%s]: 'perms' is not allowed", key) + } + + if template.User != nil { + return fmt.Errorf("env_template[%s]: 'user' is not allowed", key) + } + + if template.Uid != nil { + return fmt.Errorf("env_template[%s]: 'uid' is not allowed", key) + } + + if template.Group != nil { + return fmt.Errorf("env_template[%s]: 'group' is not allowed", key) + } + + if template.Gid != nil { + return fmt.Errorf("env_template[%s]: 'gid' is not allowed", key) + } + + if template.Wait != nil { + return fmt.Errorf("env_template[%s]: 'wait' is not allowed", key) + } + + if template.SandboxPath != nil { + return fmt.Errorf("env_template[%s]: 'sandbox_path' is not allowed", key) + } + } + return nil } diff --git a/command/agent/config/config_test.go b/command/agent/config/config_test.go index 3a2e567c0..a3e10f958 100644 --- a/command/agent/config/config_test.go +++ b/command/agent/config/config_test.go @@ -2112,13 +2112,17 @@ func TestLoadConfigFile_Bad_Value_Disable_Keep_Alives(t *testing.T) { } } -// TestLoadConfigFile_EnvTemplates loads and validates an env_template config -func TestLoadConfigFile_EnvTemplates(t *testing.T) { +// TestLoadConfigFile_EnvTemplates_Simple loads and validates an env_template config +func TestLoadConfigFile_EnvTemplates_Simple(t *testing.T) { cfg, err := LoadConfigFile("./test-fixtures/config-env-templates-simple.hcl") if err != nil { t.Fatalf("error loading config file: %s", err) } + if err := cfg.ValidateConfig(); err != nil { + t.Fatalf("validation error: %s", err) + } + expectedKey := "MY_DATABASE_USER" found := false for _, envTemplate := range cfg.EnvTemplates { @@ -2131,16 +2135,20 @@ func TestLoadConfigFile_EnvTemplates(t *testing.T) { } } -// TestLoadConfigFile_EnvTemplateComplex loads and validates an env_template config -func TestLoadConfigFile_EnvTemplateComplex(t *testing.T) { +// TestLoadConfigFile_EnvTemplates_Complex loads and validates an env_template config +func TestLoadConfigFile_EnvTemplates_Complex(t *testing.T) { cfg, err := LoadConfigFile("./test-fixtures/config-env-templates-complex.hcl") if err != nil { t.Fatalf("error loading config file: %s", err) } + + if err := cfg.ValidateConfig(); err != nil { + t.Fatalf("validation error: %s", err) + } + expectedKeys := []string{ - "FOO_DATA_LOCK", - "FOO_DATA_PASSWORD", - "FOO_DATA_USER", + "FOO_PASSWORD", + "FOO_USER", } envExists := func(key string) bool { @@ -2159,31 +2167,44 @@ func TestLoadConfigFile_EnvTemplateComplex(t *testing.T) { } } -// TestLoadConfigFile_EnvTemplateNoName ensures that env_template with no name triggers an error -func TestLoadConfigFile_EnvTemplateNoName(t *testing.T) { +// TestLoadConfigFile_EnvTemplates_WithSource loads and validates an +// env_template config with "source" instead of "contents" +func TestLoadConfigFile_EnvTemplates_WithSource(t *testing.T) { + cfg, err := LoadConfigFile("./test-fixtures/config-env-templates-with-source.hcl") + if err != nil { + t.Fatalf("error loading config file: %s", err) + } + + if err := cfg.ValidateConfig(); err != nil { + t.Fatalf("validation error: %s", err) + } +} + +// TestLoadConfigFile_EnvTemplates_NoName ensures that env_template with no name triggers an error +func TestLoadConfigFile_EnvTemplates_NoName(t *testing.T) { _, err := LoadConfigFile("./test-fixtures/bad-config-env-templates-no-name.hcl") if err == nil { t.Fatalf("expected error") } } -// TestLoadConfigFile_ExecInvalidSignal ensures that an invalid signal triggers an error -func TestLoadConfigFile_ExecInvalidSignal(t *testing.T) { +// TestLoadConfigFile_EnvTemplates_ExecInvalidSignal ensures that an invalid signal triggers an error +func TestLoadConfigFile_EnvTemplates_ExecInvalidSignal(t *testing.T) { _, err := LoadConfigFile("./test-fixtures/bad-config-env-templates-invalid-signal.hcl") if err == nil { t.Fatalf("expected error") } } -// TestLoadConfigFile_ExecSimple validates the exec section with default parameters -func TestLoadConfigFile_ExecSimple(t *testing.T) { +// TestLoadConfigFile_EnvTemplates_ExecSimple validates the exec section with default parameters +func TestLoadConfigFile_EnvTemplates_ExecSimple(t *testing.T) { cfg, err := LoadConfigFile("./test-fixtures/config-env-templates-simple.hcl") if err != nil { t.Fatalf("error loading config file: %s", err) } - if cfg.Exec == nil { - t.Fatal("expected exec config to be parsed") + if err := cfg.ValidateConfig(); err != nil { + t.Fatalf("validation error: %s", err) } expectedCmd := []string{"/path/to/my/app", "arg1", "arg2"} @@ -2201,13 +2222,17 @@ func TestLoadConfigFile_ExecSimple(t *testing.T) { } } -// TestLoadConfigFile_ExecComplex validates the exec section with non-default parameters -func TestLoadConfigFile_ExecComplex(t *testing.T) { +// TestLoadConfigFile_EnvTemplates_ExecComplex validates the exec section with non-default parameters +func TestLoadConfigFile_EnvTemplates_ExecComplex(t *testing.T) { cfg, err := LoadConfigFile("./test-fixtures/config-env-templates-complex.hcl") if err != nil { t.Fatalf("error loading config file: %s", err) } + if err := cfg.ValidateConfig(); err != nil { + t.Fatalf("validation error: %s", err) + } + if !slices.Equal(cfg.Exec.Command, []string{"env"}) { t.Fatal("exec.command does not have expected value") } @@ -2220,3 +2245,55 @@ func TestLoadConfigFile_ExecComplex(t *testing.T) { t.Fatalf("expected cfg.Exec.RestartStopSignal to be 'syscall.SIGINT', got %q", cfg.Exec.RestartStopSignal) } } + +// TestLoadConfigFile_Bad_EnvTemplates_MissingExec ensures that ValidateConfig +// errors when "env_template" stanza(s) are specified but "exec" is missing +func TestLoadConfigFile_Bad_EnvTemplates_MissingExec(t *testing.T) { + config, err := LoadConfigFile("./test-fixtures/bad-config-env-templates-missing-exec.hcl") + if err != nil { + t.Fatalf("error loading config file: %s", err) + } + + if err := config.ValidateConfig(); err == nil { + t.Fatal("expected an error from ValidateConfig: exec section is missing") + } +} + +// TestLoadConfigFile_Bad_EnvTemplates_WithProxy ensures that ValidateConfig +// errors when both env_template and api_proxy stanzas are present +func TestLoadConfigFile_Bad_EnvTemplates_WithProxy(t *testing.T) { + config, err := LoadConfigFile("./test-fixtures/bad-config-env-templates-with-proxy.hcl") + if err != nil { + t.Fatalf("error loading config file: %s", err) + } + + if err := config.ValidateConfig(); err == nil { + t.Fatal("expected an error from ValidateConfig: listener / api_proxy are not compatible with env_template") + } +} + +// TestLoadConfigFile_Bad_EnvTemplates_WithFileTemplates ensures that +// ValidateConfig errors when both env_template and template stanzas are present +func TestLoadConfigFile_Bad_EnvTemplates_WithFileTemplates(t *testing.T) { + config, err := LoadConfigFile("./test-fixtures/bad-config-env-templates-with-file-templates.hcl") + if err != nil { + t.Fatalf("error loading config file: %s", err) + } + + if err := config.ValidateConfig(); err == nil { + t.Fatal("expected an error from ValidateConfig: file template stanza is not compatible with env_template") + } +} + +// TestLoadConfigFile_Bad_EnvTemplates_DisalowedFields ensure that +// ValidateConfig errors for disalowed env_template fields +func TestLoadConfigFile_Bad_EnvTemplates_DisalowedFields(t *testing.T) { + config, err := LoadConfigFile("./test-fixtures/bad-config-env-templates-disalowed-fields.hcl") + if err != nil { + t.Fatalf("error loading config file: %s", err) + } + + if err := config.ValidateConfig(); err == nil { + t.Fatal("expected an error from ValidateConfig: disallowed fields specified in env_template") + } +} diff --git a/command/agent/config/test-fixtures/bad-config-env-templates-disalowed-fields.hcl b/command/agent/config/test-fixtures/bad-config-env-templates-disalowed-fields.hcl new file mode 100644 index 000000000..22ad96c5c --- /dev/null +++ b/command/agent/config/test-fixtures/bad-config-env-templates-disalowed-fields.hcl @@ -0,0 +1,33 @@ +auto_auth { + + method { + type = "token_file" + + config { + token_file_path = "/Users/avean/.vault-token" + } + } +} + +template_config { + static_secret_render_interval = "5m" + exit_on_retry_failure = true +} + +vault { + address = "http://localhost:8200" +} + +env_template "FOO_PASSWORD" { + contents = "{{ with secret \"secret/data/foo\" }}{{ .Data.data.password }}{{ end }}" + + # Error: destination and create_dest_dirs are not allowed in env_template + destination = "/path/on/disk/where/template/will/render.txt" + create_dest_dirs = true +} + +exec { + command = ["./my-app", "arg1", "arg2"] + restart_on_secret_changes = "always" + restart_stop_signal = "SIGTERM" +} diff --git a/command/agent/config/test-fixtures/bad-config-env-templates-missing-exec.hcl b/command/agent/config/test-fixtures/bad-config-env-templates-missing-exec.hcl new file mode 100644 index 000000000..6283e56a9 --- /dev/null +++ b/command/agent/config/test-fixtures/bad-config-env-templates-missing-exec.hcl @@ -0,0 +1,30 @@ +auto_auth { + + method { + type = "token_file" + + config { + token_file_path = "/Users/avean/.vault-token" + } + } +} + +template_config { + static_secret_render_interval = "5m" + exit_on_retry_failure = true +} + +vault { + address = "http://localhost:8200" +} + +env_template "FOO_PASSWORD" { + contents = "{{ with secret \"secret/data/foo\" }}{{ .Data.data.password }}{{ end }}" + error_on_missing_key = false +} +env_template "FOO_USER" { + contents = "{{ with secret \"secret/data/foo\" }}{{ .Data.data.user }}{{ end }}" + error_on_missing_key = false +} + +# Error: missing a required "exec" section! diff --git a/command/agent/config/test-fixtures/bad-config-env-templates-with-file-templates.hcl b/command/agent/config/test-fixtures/bad-config-env-templates-with-file-templates.hcl new file mode 100644 index 000000000..811b10d52 --- /dev/null +++ b/command/agent/config/test-fixtures/bad-config-env-templates-with-file-templates.hcl @@ -0,0 +1,40 @@ +auto_auth { + + method { + type = "token_file" + + config { + token_file_path = "/Users/avean/.vault-token" + } + } +} + +template_config { + static_secret_render_interval = "5m" + exit_on_retry_failure = true +} + +vault { + address = "http://localhost:8200" +} + +# Error: template is incompatible with env_template! +template { + source = "/path/on/disk/to/template.ctmpl" + destination = "/path/on/disk/where/template/will/render.txt" +} + +env_template "FOO_PASSWORD" { + contents = "{{ with secret \"secret/data/foo\" }}{{ .Data.data.password }}{{ end }}" + error_on_missing_key = false +} +env_template "FOO_USER" { + contents = "{{ with secret \"secret/data/foo\" }}{{ .Data.data.user }}{{ end }}" + error_on_missing_key = false +} + +exec { + command = ["./my-app", "arg1", "arg2"] + restart_on_secret_changes = "always" + restart_stop_signal = "SIGTERM" +} diff --git a/command/agent/config/test-fixtures/bad-config-env-templates-with-proxy.hcl b/command/agent/config/test-fixtures/bad-config-env-templates-with-proxy.hcl new file mode 100644 index 000000000..3c6095dde --- /dev/null +++ b/command/agent/config/test-fixtures/bad-config-env-templates-with-proxy.hcl @@ -0,0 +1,47 @@ +auto_auth { + + method { + type = "token_file" + + config { + token_file_path = "/Users/avean/.vault-token" + } + } +} + +template_config { + static_secret_render_interval = "5m" + exit_on_retry_failure = true +} + +vault { + address = "http://localhost:8200" +} + +env_template "FOO_PASSWORD" { + contents = "{{ with secret \"secret/data/foo\" }}{{ .Data.data.password }}{{ end }}" + error_on_missing_key = false +} +env_template "FOO_USER" { + contents = "{{ with secret \"secret/data/foo\" }}{{ .Data.data.user }}{{ end }}" + error_on_missing_key = false +} + +exec { + command = ["./my-app", "arg1", "arg2"] + restart_on_secret_changes = "always" + restart_stop_signal = "SIGTERM" +} + +# Error: api_proxy is incompatible with env_template +api_proxy { + use_auto_auth_token = "force" + enforce_consistency = "always" + when_inconsistent = "forward" +} + +# Error: listener is incompatible with env_template +listener "tcp" { + address = "127.0.0.1:8300" + tls_disable = true +} diff --git a/command/agent/config/test-fixtures/config-env-templates-complex.hcl b/command/agent/config/test-fixtures/config-env-templates-complex.hcl index 50328a69d..67f0152a1 100644 --- a/command/agent/config/test-fixtures/config-env-templates-complex.hcl +++ b/command/agent/config/test-fixtures/config-env-templates-complex.hcl @@ -9,19 +9,20 @@ auto_auth { } } +template_config { + static_secret_render_interval = "5m" + exit_on_retry_failure = true +} + vault { address = "http://localhost:8200" } -env_template "FOO_DATA_LOCK" { - contents = "{{ with secret \"secret/data/foo\" }}{{ .Data.data.lock }}{{ end }}" - error_on_missing_key = false -} -env_template "FOO_DATA_PASSWORD" { +env_template "FOO_PASSWORD" { contents = "{{ with secret \"secret/data/foo\" }}{{ .Data.data.password }}{{ end }}" error_on_missing_key = false } -env_template "FOO_DATA_USER" { +env_template "FOO_USER" { contents = "{{ with secret \"secret/data/foo\" }}{{ .Data.data.user }}{{ end }}" error_on_missing_key = false } diff --git a/command/agent/config/test-fixtures/config-env-templates-simple.hcl b/command/agent/config/test-fixtures/config-env-templates-simple.hcl index 4b5f385be..441563b1e 100644 --- a/command/agent/config/test-fixtures/config-env-templates-simple.hcl +++ b/command/agent/config/test-fixtures/config-env-templates-simple.hcl @@ -1,7 +1,18 @@ +auto_auth { + + method { + type = "token_file" + + config { + token_file_path = "/Users/avean/.vault-token" + } + } +} + env_template "MY_DATABASE_USER" { contents = "{{ with secret \"secret/db-secret\" }}{{ .Data.data.user }}{{ end }}" } exec { command = ["/path/to/my/app", "arg1", "arg2"] -} \ No newline at end of file +} diff --git a/command/agent/config/test-fixtures/config-env-templates-with-source.hcl b/command/agent/config/test-fixtures/config-env-templates-with-source.hcl new file mode 100644 index 000000000..d51cb5553 --- /dev/null +++ b/command/agent/config/test-fixtures/config-env-templates-with-source.hcl @@ -0,0 +1,16 @@ +auto_auth { + method { + type = "token_file" + config { + token_file_path = "/home/username/.vault-token" + } + } +} + +env_template "MY_PASSWORD" { + source = "/path/on/disk/to/template.ctmpl" +} + +exec { + command = ["/path/to/my/app", "arg1", "arg2"] +}