diff --git a/changelog/fragments/1764638389-secrets-fleet-ssl.yaml b/changelog/fragments/1764638389-secrets-fleet-ssl.yaml new file mode 100644 index 0000000000..41371f5c2a --- /dev/null +++ b/changelog/fragments/1764638389-secrets-fleet-ssl.yaml @@ -0,0 +1,32 @@ +# Kind can be one of: +# - breaking-change: a change to previously-documented behavior +# - deprecation: functionality that is being removed in a later release +# - bug-fix: fixes a problem in a previous version +# - enhancement: extends functionality but does not break or fix existing behavior +# - feature: new functionality +# - known-issue: problems that we are aware of in a given version +# - security: impacts on the security of a product or a user’s deployment. +# - upgrade: important information for someone upgrading from a prior version +# - other: does not fit into any of the other categories +kind: feature + +# Change summary; a 80ish characters long description of the change. +summary: Support secrets in fleet section of policy + +# Long description; in case the summary is not enough to describe the change +# this field accommodate a description without length limits. +# NOTE: This field will be rendered only for breaking-change and known-issue kinds at the moment. +#description: + +# Affected component; usually one of "elastic-agent", "fleet-server", "filebeat", "metricbeat", "auditbeat", "all", etc. +component: fleet-server + +# PR URL; optional; the PR number that added the changeset. +# If not present is automatically filled by the tooling finding the PR where this changelog fragment has been added. +# NOTE: the tooling supports backports, so it's able to fill the original PR number instead of the backport PR number. +# Please provide it if you are adding a fragment for a different PR. +#pr: https://github.com/owner/repo/1234 + +# Issue URL; optional; the GitHub issue related to this changeset (either closes or is part of). +# If not present is automatically filled by the tooling with the issue linked to the PR number. +#issue: https://github.com/owner/repo/1234 diff --git a/internal/pkg/api/handleCheckin.go b/internal/pkg/api/handleCheckin.go index 1aa720f8a7..f551cf2476 100644 --- a/internal/pkg/api/handleCheckin.go +++ b/internal/pkg/api/handleCheckin.go @@ -889,7 +889,10 @@ func processPolicy(ctx context.Context, zlog zerolog.Logger, bulker bulk.Bulk, a data := model.ClonePolicyData(pp.Policy.Data) for _, policyOutput := range data.Outputs { // NOTE: Not sure if output secret keys collected here include new entries, but they are collected for completeness - ks := secret.ProcessOutputSecret(policyOutput, secretValues) + ks, err := secret.ProcessOutputSecret(policyOutput, secretValues) + if err != nil { + return nil, fmt.Errorf("failed to process output secret for output %q: %w", policyOutput["name"], err) + } pp.SecretKeys = append(pp.SecretKeys, ks...) } // Iterate through the policy outputs and prepare them diff --git a/internal/pkg/model/schema.go b/internal/pkg/model/schema.go index 1ca8f94eaf..3771e6b10e 100644 --- a/internal/pkg/model/schema.go +++ b/internal/pkg/model/schema.go @@ -460,7 +460,7 @@ type PolicyData struct { Extensions map[string]interface{} `json:"extensions,omitempty"` // The policy's fleet configuration details - Fleet json.RawMessage `json:"fleet,omitempty"` + Fleet map[string]interface{} `json:"fleet,omitempty"` // The policy's ID ID string `json:"id"` diff --git a/internal/pkg/policy/parsed_policy.go b/internal/pkg/policy/parsed_policy.go index 4dc8b0952d..eb84c4f23c 100644 --- a/internal/pkg/policy/parsed_policy.go +++ b/internal/pkg/policy/parsed_policy.go @@ -51,6 +51,7 @@ type ParsedPolicy struct { Default ParsedPolicyDefaults Inputs []map[string]interface{} Agent map[string]interface{} + Fleet map[string]interface{} SecretKeys []string Links apm.SpanLink } @@ -76,7 +77,10 @@ func NewParsedPolicy(ctx context.Context, bulker bulk.Bulk, p model.Policy) (*Pa return nil, err } for name, policyOutput := range p.Data.Outputs { - ks := secret.ProcessOutputSecret(policyOutput, secretValues) + ks, err := secret.ProcessOutputSecret(policyOutput, secretValues) + if err != nil { + return nil, fmt.Errorf("failed to replace secrets in output section of policy '%s': %w", name, err) + } for _, key := range ks { secretKeys = append(secretKeys, "outputs."+name+"."+key) } @@ -91,7 +95,10 @@ func NewParsedPolicy(ctx context.Context, bulker bulk.Bulk, p model.Policy) (*Pa // Replace secrets in 'agent.download' section of policy if agentDownload, exists := p.Data.Agent["download"]; exists { if section, ok := agentDownload.(map[string]interface{}); ok { - agentDownloadSecretKeys := secret.ProcessMapSecrets(section, secretValues) + agentDownloadSecretKeys, err := secret.ProcessMapSecrets(section, secretValues) + if err != nil { + return nil, fmt.Errorf("failed to replace secrets in agent.download section of policy: %w", err) + } for _, key := range agentDownloadSecretKeys { secretKeys = append(secretKeys, "agent.download."+key) } @@ -99,6 +106,15 @@ func NewParsedPolicy(ctx context.Context, bulker bulk.Bulk, p model.Policy) (*Pa } } + // Replace secrets in `fleet` section of policy + fleetSecretKeys, err := secret.ProcessMapSecrets(p.Data.Fleet, secretValues) + if err != nil { + return nil, fmt.Errorf("failed to replace secrets in fleet section of policy: %w", err) + } + for _, key := range fleetSecretKeys { + secretKeys = append(secretKeys, "fleet."+key) + } + // Done replacing secrets. p.Data.SecretReferences = nil @@ -112,6 +128,7 @@ func NewParsedPolicy(ctx context.Context, bulker bulk.Bulk, p model.Policy) (*Pa }, Inputs: policyInputs, Agent: p.Data.Agent, + Fleet: p.Data.Fleet, SecretKeys: secretKeys, } diff --git a/internal/pkg/policy/parsed_policy_test.go b/internal/pkg/policy/parsed_policy_test.go index 08006194eb..fbf15f5c0b 100644 --- a/internal/pkg/policy/parsed_policy_test.go +++ b/internal/pkg/policy/parsed_policy_test.go @@ -125,13 +125,15 @@ func TestParsedPolicyMixedSecretsReplacement(t *testing.T) { require.NoError(t, err) // Validate that secrets were identified - require.Len(t, pp.SecretKeys, 6) + require.Len(t, pp.SecretKeys, 8) require.Contains(t, pp.SecretKeys, "outputs.fs-output.type") require.Contains(t, pp.SecretKeys, "outputs.fs-output.ssl.key") require.Contains(t, pp.SecretKeys, "inputs.0.streams.0.auth.basic.password") require.Contains(t, pp.SecretKeys, "inputs.0.streams.1.auth.basic.password") require.Contains(t, pp.SecretKeys, "agent.download.sourceURI") require.Contains(t, pp.SecretKeys, "agent.download.ssl.key") + require.Contains(t, pp.SecretKeys, "fleet.hosts.0") + require.Contains(t, pp.SecretKeys, "fleet.ssl.key") // Validate that secret references were replaced firstInputStreams := pp.Inputs[0]["streams"].([]any) @@ -143,4 +145,6 @@ func TestParsedPolicyMixedSecretsReplacement(t *testing.T) { require.Equal(t, "w8yELZoBTAyw4gQK9KZ7_value", pp.Policy.Data.Outputs["fs-output"]["ssl"].(map[string]interface{})["key"]) require.Equal(t, "bcdefg234_value", pp.Policy.Data.Agent["download"].(map[string]interface{})["sourceURI"]) require.Equal(t, "rwXzUJoBxE9I-QCxFt9m_value", pp.Policy.Data.Agent["download"].(map[string]interface{})["ssl"].(map[string]interface{})["key"]) + require.Equal(t, "abcdef123_value", pp.Policy.Data.Fleet["hosts"].([]interface{})[0]) + require.Equal(t, "w8yELZoBTAyw4gQK9KZ7_value", pp.Policy.Data.Fleet["ssl"].(map[string]interface{})["key"]) } diff --git a/internal/pkg/policy/testdata/policy_with_secrets_mixed.json b/internal/pkg/policy/testdata/policy_with_secrets_mixed.json index fe23b48ecf..aa52cb397f 100644 --- a/internal/pkg/policy/testdata/policy_with_secrets_mixed.json +++ b/internal/pkg/policy/testdata/policy_with_secrets_mixed.json @@ -463,7 +463,14 @@ }, "fleet": { "hosts": [ - "https://c3564859758d4e41a2b2109ade35c1a2.fleet.us-west2.gcp.elastic-cloud.com:443" - ] + "$co.elastic.secret{abcdef123}" + ], + "secrets": { + "ssl": { + "key": { + "id": "w8yELZoBTAyw4gQK9KZ7" + } + } + } } } \ No newline at end of file diff --git a/internal/pkg/secret/secret.go b/internal/pkg/secret/secret.go index 2939d2368b..90bdb7baef 100644 --- a/internal/pkg/secret/secret.go +++ b/internal/pkg/secret/secret.go @@ -7,6 +7,7 @@ package secret import ( "context" "encoding/json" + "fmt" "regexp" "strconv" "strings" @@ -323,17 +324,20 @@ func setSecretPath(section smap.Map, secretValue string, secretPaths []string) { } // Read secret from output and mutate output with secret value -func ProcessOutputSecret(output smap.Map, secretValues map[string]string) []string { +func ProcessOutputSecret(output smap.Map, secretValues map[string]string) ([]string, error) { // Unfortunately, there are two ways (formats) of specifying secret references in // policies: inline and path (see https://github.com/elastic/fleet-server/pull/5852). // So we try replacing secret references in both formats. - keys := processMapWithInlineSecrets(output, secretValues) + keys, err := processMapWithInlineSecrets(output, secretValues) + if err != nil { + return nil, fmt.Errorf("failed processing output secret with inline secrets: %w", err) + } k := processMapWithPathSecrets(output, secretValues) keys = append(keys, k...) - return keys + return keys, nil } // processMapWithPathSecrets reads secrets from the output and mutates the output with the secret values using @@ -370,25 +374,31 @@ func processMapWithPathSecrets(m smap.Map, secretValues map[string]string) []str return keys } -func processMapWithInlineSecrets(m smap.Map, secretValues map[string]string) []string { +func processMapWithInlineSecrets(m smap.Map, secretValues map[string]string) ([]string, error) { replacedM, keys := replaceInlineSecretRefsInMap(m, secretValues) for _, key := range keys { - m[key] = replacedM[key] + rm := smap.Map(replacedM) + if err := m.Set(key, rm.Get(key)); err != nil { + return nil, fmt.Errorf("failed processing map with inline secrets: failed to set secret value for key %s: %w", key, err) + } } - return keys + return keys, nil } // ProcessMapSecrets reads and replaces secrets in the agent.download section of the policy -func ProcessMapSecrets(m smap.Map, secretValues map[string]string) []string { +func ProcessMapSecrets(m smap.Map, secretValues map[string]string) ([]string, error) { // Unfortunately, there are two ways (formats) of specifying secret references in // policies: inline and path (see https://github.com/elastic/fleet-server/pull/5852). // So we try replacing secret references in both formats. - keys := processMapWithInlineSecrets(m, secretValues) + keys, err := processMapWithInlineSecrets(m, secretValues) + if err != nil { + return nil, fmt.Errorf("failed processing map secrets with inline secrets: %w", err) + } k := processMapWithPathSecrets(m, secretValues) keys = append(keys, k...) - return keys + return keys, nil } // replaceStringRef replaces values matching a secret ref regex, e.g. $co.elastic.secret{} -> diff --git a/internal/pkg/secret/secret_test.go b/internal/pkg/secret/secret_test.go index a0547af348..988ebcc273 100644 --- a/internal/pkg/secret/secret_test.go +++ b/internal/pkg/secret/secret_test.go @@ -309,7 +309,7 @@ func TestProcessOutputSecret(t *testing.T) { "sslother": "sslother_value", "sslkey": "sslkey_value", } - keys := ProcessOutputSecret(output, secretValues) + keys, err := ProcessOutputSecret(output, secretValues) assert.NoError(t, err) assert.Equal(t, expectOutput, output) diff --git a/internal/pkg/server/fleet_secrets_integration_test.go b/internal/pkg/server/fleet_secrets_integration_test.go index 38dea0c414..477d837ec3 100644 --- a/internal/pkg/server/fleet_secrets_integration_test.go +++ b/internal/pkg/server/fleet_secrets_integration_test.go @@ -110,6 +110,16 @@ func createAgentPolicyWithSecrets(t *testing.T, ctx context.Context, bulker bulk }, }, }, + Fleet: map[string]interface{}{ + "hosts": []string{inlineSecretRef}, + "secrets": map[string]interface{}{ + "ssl": map[string]interface{}{ + "key": map[string]interface{}{ + "id": pathSecretID, + }, + }, + }, + }, SecretReferences: []model.SecretReferencesItems{ {ID: inlineSecretID}, {ID: pathSecretID}, @@ -276,7 +286,26 @@ func Test_Agent_Policy_Secrets(t *testing.T) { }, }, actionData.Policy.Agent["download"]) + // expect fleet secrets to be replaced + assert.Equal(t, map[string]interface{}{ + "hosts": []interface{}{"inline_secret_value"}, + "ssl": map[string]interface{}{ + "key": "path_secret_value", + }, + }, actionData.Policy.Fleet) + assert.NotContains(t, output, "secrets") // expect that secret_paths lists the key - assert.ElementsMatch(t, []string{"inputs.0.package_var_secret", "outputs.default.secret-key", "agent.download.sourceURI", "agent.download.ssl.key"}, actionData.Policy.SecretPaths) + assert.ElementsMatch( + t, + []string{ + "inputs.0.package_var_secret", + "outputs.default.secret-key", + "agent.download.sourceURI", + "agent.download.ssl.key", + "fleet.hosts.0", + "fleet.ssl.key", + }, + actionData.Policy.SecretPaths, + ) } diff --git a/internal/pkg/smap/smap.go b/internal/pkg/smap/smap.go index 5e58310ad5..5cd4b6e662 100644 --- a/internal/pkg/smap/smap.go +++ b/internal/pkg/smap/smap.go @@ -11,6 +11,8 @@ import ( "crypto/sha256" "encoding/hex" "encoding/json" + "fmt" + "strings" ) type Map map[string]interface{} @@ -69,6 +71,118 @@ func (m Map) Marshal() ([]byte, error) { return json.Marshal(m) } +// Get returns the value at the specified key path +// For example: +// m.Get("a.b.c") => m["a"]["b"]["c"] +// m.Get("a.2") => m["a"][2] +// m.Get("a.2.c") => m["a"][2]["c"] +func (m Map) Get(keyPath string) any { + if m == nil { + return nil + } + + var curr any = m + var key string + var index uint + var isIndex bool + + parts := strings.Split(keyPath, ".") + for _, part := range parts { + key = part + + // Check if part is an index + index, isIndex = parseIndex(part) + + // Traverse to the next level + if isIndex { + sCurr, ok := curr.([]any) + if !ok { + return nil + } + if index >= uint(len(sCurr)) { + return nil + } + curr = sCurr[index] + } else { + mCurr, ok := isSMap(curr) + if !ok { + return nil + } + curr = mCurr[key] + } + } + + return curr +} + +// Set sets the value at the specified key path. +// For example: +// m.Set("a.b.c", value) => m["a"]["b"]["c"] = value +// m.Set("a.2", value) => m["a"][2] = value +// m.Set("a.2.c", value) => m["a"][2]["c"] = value +func (m Map) Set(keyPath string, value any) error { + if m == nil { + return nil + } + + var curr any = m + var parent any + var key string + var index uint + var isIndex bool + + parts := strings.Split(keyPath, ".") + for i, part := range parts { + key = part + parent = curr + parentPath := strings.Join(parts[:i], ".") + + // Check if part is an index + index, isIndex = parseIndex(part) + + // If last part, set the value + if i == len(parts)-1 { + if isIndex { + sParent, ok := parent.([]any) + if !ok { + return fmt.Errorf("expected slice at %s, got %T", parentPath, parent) + } + if index >= uint(len(sParent)) { + return fmt.Errorf("index out of bounds at %s: %d", parentPath, index) + } + sParent[index] = value + } else { + mParent, ok := isSMap(parent) + if !ok { + return fmt.Errorf("expected map at %s, got %T", parentPath, parent) + } + mParent[key] = value + } + return nil + } + + // Traverse to the next level + if isIndex { + sCurr, ok := curr.([]any) + if !ok { + return fmt.Errorf("expected slice at %s, got %T", parentPath, curr) + } + if index >= uint(len(sCurr)) { + return fmt.Errorf("index out of bounds at %s: %d", parentPath, index) + } + curr = sCurr[index] + } else { + mCurr, ok := isSMap(curr) + if !ok { + return fmt.Errorf("expected map at %s, got %T", parentPath, curr) + } + curr = mCurr[key] + } + } + + return nil +} + // Parse generates a Map from the passed data. // data is assumed to be a json object. // TODO Should we refactor this to UnmarshalJSON? @@ -83,3 +197,24 @@ func Parse(data []byte) (Map, error) { return m, err } + +func parseIndex(str string) (uint, bool) { + // Try to read str as an integer + var index uint + if _, err := fmt.Sscanf(str, "%d", &index); err != nil { + return 0, false + } + return index, true +} + +func isSMap(v any) (map[string]any, bool) { + if mapVal, ok := v.(map[string]any); ok { + return mapVal, true + } + + if mapVal, ok := v.(Map); ok { + return mapVal, true + } + + return nil, false +} diff --git a/internal/pkg/smap/smap_test.go b/internal/pkg/smap/smap_test.go new file mode 100644 index 0000000000..caeee08126 --- /dev/null +++ b/internal/pkg/smap/smap_test.go @@ -0,0 +1,106 @@ +// Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one +// or more contributor license agreements. Licensed under the Elastic License; +// you may not use this file except in compliance with the Elastic License. + +package smap + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestGet(t *testing.T) { + tests := map[string]struct { + input Map + keyPath string + expected any + }{ + "a": { + input: Map{"a": 1}, + keyPath: "a", + expected: 1, + }, + "a.b": { + input: Map{"a": Map{"b": 2}}, + keyPath: "a.b", + expected: 2, + }, + "a.1": { + input: Map{"a": []any{10, 20, 30}}, + keyPath: "a.1", + expected: 20, + }, + "a.b.2.c": { + input: Map{"a": Map{"b": []any{1, "b", map[string]any{"c": 3}}}}, + keyPath: "a.b.2.c", + expected: 3, + }, + "nonexistent": { + input: Map{"a": 1}, + keyPath: "b", + expected: nil, + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + result := tc.input.Get(tc.keyPath) + require.Equal(t, tc.expected, result) + }) + } +} + +func TestSet(t *testing.T) { + tests := map[string]struct { + input Map + keyPath string + value any + expected Map + expectedErr string + }{ + "a": { + input: Map{"a": 1}, + keyPath: "b", + value: 42, + expected: Map{"a": 1, "b": 42}, + }, + "a.c": { + input: Map{"a": Map{"b": map[string]any{"d": 1}}}, + keyPath: "a.c", + value: 42, + expected: Map{"a": Map{"b": map[string]any{"d": 1}, "c": 42}}, + }, + "a.1": { + input: Map{"a": []any{10, 20, 30}}, + keyPath: "a.1", + value: 42, + expected: Map{"a": []any{10, 42, 30}}, + }, + "a.3": { + input: Map{"a": []any{10, 20, 30}}, + keyPath: "a.3", + value: 42, + expected: Map{"a": []any{10, 20, 30}}, // No change, index out of bounds + expectedErr: "index out of bounds at a: 3", + }, + "a.b.2.c": { + input: Map{"a": Map{"b": []any{1, "b", map[string]any{"c": 3}}}}, + keyPath: "a.b.2.c", + value: 42, + expected: Map{"a": Map{"b": []any{1, "b", map[string]any{"c": 42}}}}, + }, + } + + for name, tc := range tests { + t.Run(name, func(t *testing.T) { + err := tc.input.Set(tc.keyPath, tc.value) + if tc.expectedErr == "" { + require.NoError(t, err) + } else { + require.Equal(t, tc.expectedErr, err.Error()) + } + require.Equal(t, tc.expected, tc.input) + }) + } +} diff --git a/model/schema.json b/model/schema.json index 74eb6cd794..64d6fd94b1 100644 --- a/model/schema.json +++ b/model/schema.json @@ -985,7 +985,8 @@ }, "fleet": { "description": "The policy's fleet configuration details", - "format": "raw" + "type": "object", + "additionalProperties": {} } }, "required": ["id", "revision", "outputs"]