Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
kind: security

summary: Store output API key secrets in .fleet-secrets instead of .fleet-agents

description: |
Fleet Server now writes output API key material to .fleet-secrets via the Fleet
secrets API (POST /_fleet/secret) and stores only a $co.elastic.secret{id} reference
in .fleet-agents-7. Previously, the raw id:secret string was stored in plaintext in
outputs.{name}.api_key, which the kibana_system role could read and use to escalate
privileges to SIEM data stream write access. Retired secrets are deleted from
.fleet-secrets on ACK. Existing plaintext values continue to work transparently
until the next key rotation.

component: fleet-server
24 changes: 24 additions & 0 deletions internal/pkg/api/handleAck.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import (
"github.com/elastic/fleet-server/v7/internal/pkg/logger/ecs"
"github.com/elastic/fleet-server/v7/internal/pkg/model"
"github.com/elastic/fleet-server/v7/internal/pkg/policy"
"github.com/elastic/fleet-server/v7/internal/pkg/secret"
"github.com/elastic/fleet-server/v7/internal/pkg/smap"
)

Expand Down Expand Up @@ -507,6 +508,7 @@ func updateAPIKey(ctx context.Context,
}
}
invalidateAPIKeys(ctx, zlog, bulk, toRetireAPIKeyIDs, apiKeyID)
deleteRetiredSecrets(ctx, zlog, bulk, toRetireAPIKeyIDs)
}

return nil
Expand Down Expand Up @@ -585,6 +587,17 @@ func (ack *AckT) handleUnenroll(ctx context.Context, zlog zerolog.Logger, agent
zlog.Info().Any("fleet.policy.apiKeyIDsToRetire", apiKeys).Msg("handleUnenroll invalidate API keys")
ack.invalidateAPIKeys(ctx, zlog, apiKeys, "")

// Delete secrets for all output API keys: retired ones (carried in apiKeys via SecretID)
// and the current active ones (secret ID embedded in output.APIKey as a reference string).
allSecretItems := make([]model.ToRetireAPIKeyIdsItems, 0, len(apiKeys)+len(agent.Outputs))
allSecretItems = append(allSecretItems, apiKeys...)
for _, output := range agent.Outputs {
if secretID, ok := secret.ParseSecretReference(output.APIKey); ok {
allSecretItems = append(allSecretItems, model.ToRetireAPIKeyIdsItems{SecretID: secretID})
}
}
deleteRetiredSecrets(ctx, zlog, ack.bulk, allSecretItems)

now := time.Now().UTC().Format(time.RFC3339)
doc := bulk.UpdateFields{
dl.FieldActive: false,
Expand Down Expand Up @@ -744,3 +757,14 @@ func invalidateAPIKeys(ctx context.Context, zlog zerolog.Logger, bulk bulk.Bulk,
}
}
}

func deleteRetiredSecrets(ctx context.Context, zlog zerolog.Logger, bulk bulk.Bulk, toRetireAPIKeyIDs []model.ToRetireAPIKeyIdsItems) {
for _, k := range toRetireAPIKeyIDs {
if k.SecretID == "" {
continue
}
if err := bulk.DeleteSecret(ctx, k.SecretID); err != nil {
zlog.Warn().Err(err).Str("secret_id", k.SecretID).Msg("Failed to delete retired output API key secret")
}
}
}
44 changes: 44 additions & 0 deletions internal/pkg/api/handleAck_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -834,3 +834,47 @@ func TestValidateAckRequest(t *testing.T) {
})
}
}

func TestDeleteRetiredSecrets(t *testing.T) {
logger := testlog.SetLogger(t)
_ = logger

t.Run("deletes secrets for items with a SecretID", func(t *testing.T) {
bulker := ftesting.NewMockBulk()
bulker.On("DeleteSecret", mock.Anything, "secret-1").Return(nil).Once()
bulker.On("DeleteSecret", mock.Anything, "secret-2").Return(nil).Once()

items := []model.ToRetireAPIKeyIdsItems{
{ID: "key-1", SecretID: "secret-1"},
{ID: "key-2", SecretID: "secret-2"},
{ID: "key-3"}, // no SecretID — plaintext key, no cleanup needed
}

deleteRetiredSecrets(context.Background(), testlog.SetLogger(t), bulker, items)
bulker.AssertExpectations(t)
bulker.AssertNotCalled(t, "DeleteSecret", mock.Anything, "")
})

t.Run("logs warning on delete failure but does not return error", func(t *testing.T) {
bulker := ftesting.NewMockBulk()
bulker.On("DeleteSecret", mock.Anything, "secret-bad").Return(errors.New("delete failed")).Once()

items := []model.ToRetireAPIKeyIdsItems{
{ID: "key-1", SecretID: "secret-bad"},
}

// Should not panic or propagate the error.
deleteRetiredSecrets(context.Background(), testlog.SetLogger(t), bulker, items)
bulker.AssertExpectations(t)
})

t.Run("no-op when all items lack a SecretID", func(t *testing.T) {
bulker := ftesting.NewMockBulk()
items := []model.ToRetireAPIKeyIdsItems{
{ID: "key-1"},
{ID: "key-2"},
}
deleteRetiredSecrets(context.Background(), testlog.SetLogger(t), bulker, items)
bulker.AssertNotCalled(t, "DeleteSecret", mock.Anything, mock.Anything)
})
}
10 changes: 10 additions & 0 deletions internal/pkg/bulk/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,8 @@ type Bulk interface {
RemoteOutputConfigChanged(zlog zerolog.Logger, name string, newCfg map[string]any) bool

ReadSecrets(ctx context.Context, secretIds []string) (map[string]string, error)
WriteSecret(ctx context.Context, value string) (string, error)
DeleteSecret(ctx context.Context, secretID string) error
}

const kModBulk = "bulk"
Expand Down Expand Up @@ -335,6 +337,14 @@ func (b *Bulker) ReadSecrets(ctx context.Context, secretIds []string) (map[strin
return result, nil
}

func (b *Bulker) WriteSecret(ctx context.Context, value string) (string, error) {
return WriteSecret(ctx, b.Client(), value)
}

func (b *Bulker) DeleteSecret(ctx context.Context, secretID string) error {
return DeleteSecret(ctx, b.Client(), secretID)
}

// Stop timer, but don't stall on channel.
// API doesn't not seem to work as specified.
func stopTimer(t *time.Timer) {
Expand Down
78 changes: 78 additions & 0 deletions internal/pkg/bulk/secret.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,11 @@
package bulk

import (
"bytes"
"context"
"encoding/json"
"fmt"
"io"
"net/http"

"github.com/elastic/go-elasticsearch/v8"
Expand Down Expand Up @@ -37,6 +40,10 @@ func (c *ExtendedAPI) Read(ctx context.Context, secretID string) (*SecretRespons
return nil, err
}
defer res.Body.Close()
if res.StatusCode >= 400 {
body, _ := io.ReadAll(res.Body)
return nil, fmt.Errorf("unexpected status %d from fleet secret read: %s", res.StatusCode, body)
}
var secretResp SecretResponse

err = json.NewDecoder(res.Body).Decode(&secretResp)
Expand All @@ -46,6 +53,42 @@ func (c *ExtendedAPI) Read(ctx context.Context, secretID string) (*SecretRespons
return &secretResp, nil
}

// Write stores a secret value via the Fleet ES plugin secrets API and returns the assigned secret ID.
// POST /_fleet/secret
func (c *ExtendedAPI) Write(ctx context.Context, value string) (string, error) {
body, err := json.Marshal(struct {
Value string `json:"value"`
}{Value: value})
if err != nil {
return "", err
}

req, err := http.NewRequestWithContext(ctx, "POST", "/_fleet/secret", bytes.NewReader(body))
if err != nil {
return "", err
}
req.Header.Set("Content-Type", "application/json")
req.Header.Set("Accept", "application/json")

res, err := c.Perform(req)
Comment thread
samuelvl marked this conversation as resolved.
if err != nil {
return "", err
}
defer res.Body.Close()
if res.StatusCode >= 400 {
body, _ := io.ReadAll(res.Body)
return "", fmt.Errorf("unexpected status %d from fleet secret write: %s", res.StatusCode, body)
}

var resp struct {
ID string `json:"id"`
}
if err := json.NewDecoder(res.Body).Decode(&resp); err != nil {
return "", err
}
return resp.ID, nil
}

type SecretResponse struct {
Value string
}
Expand All @@ -63,3 +106,38 @@ func ReadSecret(ctx context.Context, client *elasticsearch.Client, secretID stri
}
return (*res).Value, err
}

func WriteSecret(ctx context.Context, client *elasticsearch.Client, value string) (string, error) {
span, ctx := apm.StartSpan(ctx, "writeSecret", "elasticsearch")
defer span.End()
es := ExtendedClient{Client: client, Custom: &ExtendedAPI{client}}
return es.Custom.Write(ctx, value)
}

// Delete removes a secret from the Fleet secrets store.
// DELETE /_fleet/secret/secretId
func (c *ExtendedAPI) Delete(ctx context.Context, secretID string) error {
req, err := http.NewRequestWithContext(ctx, "DELETE", "/_fleet/secret/"+secretID, nil)
if err != nil {
return err
}
req.Header.Set("Accept", "application/json")

res, err := c.Perform(req)
if err != nil {
return err
}
defer res.Body.Close()
if res.StatusCode >= 400 {
body, _ := io.ReadAll(res.Body)
return fmt.Errorf("unexpected status %d from fleet secret delete: %s", res.StatusCode, body)
}
return nil
}

func DeleteSecret(ctx context.Context, client *elasticsearch.Client, secretID string) error {
span, ctx := apm.StartSpan(ctx, "deleteSecret", "elasticsearch")
defer span.End()
es := ExtendedClient{Client: client, Custom: &ExtendedAPI{client}}
return es.Custom.Delete(ctx, secretID)
}
3 changes: 3 additions & 0 deletions internal/pkg/model/schema.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

52 changes: 41 additions & 11 deletions internal/pkg/policy/policy_output.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/elastic/fleet-server/v7/internal/pkg/dl"
"github.com/elastic/fleet-server/v7/internal/pkg/logger/ecs"
"github.com/elastic/fleet-server/v7/internal/pkg/model"
"github.com/elastic/fleet-server/v7/internal/pkg/secret"
"github.com/elastic/fleet-server/v7/internal/pkg/smap"
)

Expand Down Expand Up @@ -129,11 +130,15 @@ func (p *Output) prepareElasticsearch(
}
if !found {
zlog.Info().Str(ecs.APIKeyID, agentOutput.APIKeyID).Str(ecs.PolicyOutputName, agentOutputName).Msg("Output removed, will retire API key")
toRetireAPIKeys = &model.ToRetireAPIKeyIdsItems{
retiring := model.ToRetireAPIKeyIdsItems{
ID: agentOutput.APIKeyID,
RetiredAt: time.Now().UTC().Format(time.RFC3339),
Output: agentOutputName,
}
if secretID, ok := secret.ParseSecretReference(agentOutput.APIKey); ok {
retiring.SecretID = secretID
}
toRetireAPIKeys = &retiring
removedOutputName = agentOutputName
break
}
Expand Down Expand Up @@ -307,8 +312,14 @@ func (p *Output) prepareElasticsearch(
Str(ecs.DefaultOutputAPIKeyID, outputAPIKey.ID).
Msg("Updating agent record to pick up default output key.")

secretID, err := bulker.WriteSecret(ctx, outputAPIKey.Agent())
Comment thread
samuelvl marked this conversation as resolved.
if err != nil {
return fmt.Errorf("failed writing output API key secret: %w", err)
}
apiKeyRef := secret.MakeSecretReference(secretID)

fields := map[string]any{
dl.FieldPolicyOutputAPIKey: outputAPIKey.Agent(),
dl.FieldPolicyOutputAPIKey: apiKeyRef,
dl.FieldPolicyOutputAPIKeyID: outputAPIKey.ID,
dl.FieldPolicyOutputPermissionsHash: p.Role.Sha2,
}
Expand All @@ -317,11 +328,15 @@ func (p *Output) prepareElasticsearch(
fields[dl.FiledType] = OutputTypeElasticsearch
}
if output.APIKeyID != "" {
fields[dl.FieldPolicyOutputToRetireAPIKeyIDs] = model.ToRetireAPIKeyIdsItems{
retiring := model.ToRetireAPIKeyIdsItems{
ID: output.APIKeyID,
RetiredAt: time.Now().UTC().Format(time.RFC3339),
Output: p.Name,
}
if secretID, ok := secret.ParseSecretReference(output.APIKey); ok {
retiring.SecretID = secretID
}
fields[dl.FieldPolicyOutputToRetireAPIKeyIDs] = retiring
}

// Using painless script to append the old keys to the history
Expand All @@ -332,6 +347,9 @@ func (p *Output) prepareElasticsearch(

if err = bulker.Update(ctx, dl.FleetAgents, agent.Id, body, bulk.WithRefresh(), bulk.WithRetryOnConflict(3)); err != nil {
zlog.Error().Err(err).Msg("fail update agent record")
if delErr := bulker.DeleteSecret(ctx, secretID); delErr != nil {
zlog.Warn().Err(delErr).Str("secret_id", secretID).Msg("failed to delete orphaned output API key secret after agent update failure")
}
return fmt.Errorf("fail update agent record: %w", err)
}

Expand All @@ -340,7 +358,7 @@ func (p *Output) prepareElasticsearch(
// data is correct and in sync with ES, so it can be safely used after
// this method returns.
output.Type = OutputTypeElasticsearch
output.APIKey = outputAPIKey.Agent()
output.APIKey = apiKeyRef
output.APIKeyID = outputAPIKey.ID
output.PermissionsHash = p.Role.Sha2 // for the sake of consistency
}
Expand All @@ -362,7 +380,19 @@ func (p *Output) prepareElasticsearch(
// in place to reduce number of agent policy allocation when sending the updated
// agent policy to multiple agents.
// See: https://github.com/elastic/fleet-server/issues/1301
outputMap[p.Name]["api_key"] = output.APIKey
apiKey := output.APIKey
if secretID, ok := secret.ParseSecretReference(apiKey); ok {
resolved, err := bulker.ReadSecrets(ctx, []string{secretID})
if err != nil {
return fmt.Errorf("failed resolving output API key secret: %w", err)
}
val, ok := resolved[secretID]
if !ok || val == "" {
return fmt.Errorf("output API key secret %q not found", secretID)
}
apiKey = val
}
outputMap[p.Name]["api_key"] = apiKey
return nil
}

Expand Down Expand Up @@ -476,29 +506,29 @@ func renderUpdatePainlessScript(outputName string, fields map[string]any) ([]byt
var source strings.Builder

// prepare agent.elasticsearch_outputs[OUTPUT_NAME]
source.WriteString(fmt.Sprintf(`
fmt.Fprintf(&source, `
if (ctx._source['outputs']==null)
{ctx._source['outputs']=new HashMap();}
if (ctx._source['outputs']['%s']==null)
{ctx._source['outputs']['%s']=new HashMap();}
`, outputName, outputName))
`, outputName, outputName)

for field := range fields {
if field == dl.FieldPolicyOutputToRetireAPIKeyIDs {
// dl.FieldPolicyOutputToRetireAPIKeyIDs is a special case.
// It's an array that gets deleted when the keys are invalidated.
// Thus, append the old API key ID, create the field if necessary.
source.WriteString(fmt.Sprintf(`
fmt.Fprintf(&source, `
if (ctx._source['outputs']['%s'].%s==null)
{ctx._source['outputs']['%s'].%s=new ArrayList();}
if (!ctx._source['outputs']['%s'].%s.contains(params.%s))
{ctx._source['outputs']['%s'].%s.add(params.%s);}
`, outputName, field, outputName, field, outputName, field, field, outputName, field, field))
`, outputName, field, outputName, field, outputName, field, field, outputName, field, field)
} else {
// Update the other fields
source.WriteString(fmt.Sprintf(`
fmt.Fprintf(&source, `
ctx._source['outputs']['%s'].%s=params.%s;`,
outputName, field, field))
outputName, field, field)
}
}

Expand Down
Loading
Loading