diff --git a/changelog/fragments/1762447389-fix-issue-where-malformed-components-field-prevents-agent-authentication.yaml b/changelog/fragments/1762447389-fix-issue-where-malformed-components-field-prevents-agent-authentication.yaml new file mode 100644 index 0000000000..85f2c21709 --- /dev/null +++ b/changelog/fragments/1762447389-fix-issue-where-malformed-components-field-prevents-agent-authentication.yaml @@ -0,0 +1,45 @@ +# REQUIRED +# 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: bug-fix + +# REQUIRED for all kinds +# Change summary; a 80ish characters long description of the change. +summary: fix issue where malformed components field prevents agent authentication + +# REQUIRED for breaking-change, deprecation, known-issue +# Long description; in case the summary is not enough to describe the change +# this field accommodate a description without length limits. +# description: + +# REQUIRED for breaking-change, deprecation, known-issue +# impact: + +# REQUIRED for breaking-change, deprecation, known-issue +# action: + +# REQUIRED for all kinds +# Affected component; usually one of "elastic-agent", "fleet-server", "filebeat", "metricbeat", "auditbeat", "all", etc. +component: fleet-server + +# AUTOMATED +# OPTIONAL to manually add other PR URLs +# PR URL: A link the PR 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 + +# AUTOMATED +# OPTIONAL to manually add other issue URLs +# 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 6baf1d0b4e..f28bc19ecf 100644 --- a/internal/pkg/api/handleCheckin.go +++ b/internal/pkg/api/handleCheckin.go @@ -1037,10 +1037,11 @@ func parseMeta(zlog zerolog.Logger, agent *model.Agent, req *CheckinRequest) ([] return nil, nil } - // Deserialize the agent's metadata copy + // Deserialize the agent's metadata copy. If it fails, it's ignored as it will just + // be replaced with the correct contents from the clients checkin. var agentLocalMeta interface{} if err := json.Unmarshal(agent.LocalMetadata, &agentLocalMeta); err != nil { - return nil, fmt.Errorf("parseMeta local: %w", err) + zlog.Warn().Err(err).Msg("local_metadata in document invalid; ignoring it") } var outMeta []byte @@ -1077,14 +1078,9 @@ func parseComponents(zlog zerolog.Logger, agent *model.Agent, req *CheckinReques return nil, &unhealthyReason, nil } - agentComponentsJSON, err := json.Marshal(agent.Components) - if err != nil { - return nil, &unhealthyReason, fmt.Errorf("agent.Components marshal: %w", err) - } - // Quick comparison first; compare the JSON payloads. // If the data is not consistently normalized, this short-circuit will not work. - if bytes.Equal(req.Components, agentComponentsJSON) { + if bytes.Equal(req.Components, agent.Components) { zlog.Trace().Msg("quick comparing agent components data is equal") return nil, &unhealthyReason, nil } @@ -1102,13 +1098,20 @@ func parseComponents(zlog zerolog.Logger, agent *model.Agent, req *CheckinReques return nil, &unhealthyReason, nil } + // Deserialize the agent's components. If it fails, it's ignored as it will just + // be replaced with the correct contents from the clients checkin. + var agentComponents []model.ComponentsItems + if err := json.Unmarshal(agent.Components, &agentComponents); err != nil { + zlog.Warn().Err(err).Msg("components in document invalid; ignoring it") + } + var outComponents []byte // Compare the deserialized meta structures and return the bytes to update if different - if !reflect.DeepEqual(reqComponents, agent.Components) { + if !reflect.DeepEqual(reqComponents, agentComponents) { reqComponentsJSON, _ := json.Marshal(req.Components) zlog.Trace(). - Str("oldComponents", string(agentComponentsJSON)). + Str("oldComponents", string(agent.Components)). Str("req.Components", string(reqComponentsJSON)). Msg("local components data is not equal") diff --git a/internal/pkg/api/handleCheckin_test.go b/internal/pkg/api/handleCheckin_test.go index 07b775f78d..a02bd9c0e7 100644 --- a/internal/pkg/api/handleCheckin_test.go +++ b/internal/pkg/api/handleCheckin_test.go @@ -1001,12 +1001,12 @@ func TestParseComponents(t *testing.T) { agent: &model.Agent{ LastCheckinStatus: FailedStatus, UnhealthyReason: []string{"input"}, - Components: []model.ComponentsItems{{ + Components: requireMarshalJSON(t, []model.ComponentsItems{{ Status: "DEGRADED", Units: []model.UnitsItems{{ Status: "DEGRADED", Type: "input", }}, - }}, + }}), }, req: &CheckinRequest{ Components: degradedInputReqComponents, @@ -1020,12 +1020,40 @@ func TestParseComponents(t *testing.T) { agent: &model.Agent{ LastCheckinStatus: "online", UnhealthyReason: nil, - Components: []model.ComponentsItems{{ + Components: requireMarshalJSON(t, []model.ComponentsItems{{ Status: "HEALTHY", Units: []model.UnitsItems{{ Status: "HEALTHY", Type: "input", }}, - }}, + }}), + }, + req: &CheckinRequest{ + Status: "DEGRADED", + Components: degradedInputReqComponents, + }, + outComponents: degradedInputReqComponents, + unhealthyReason: &[]string{"input"}, + err: nil, + }, { + name: "bad stored components", + agent: &model.Agent{ + LastCheckinStatus: "online", + UnhealthyReason: nil, + Components: requireMarshalJSON(t, "string stored in components incorrectly"), + }, + req: &CheckinRequest{ + Status: "DEGRADED", + Components: degradedInputReqComponents, + }, + outComponents: degradedInputReqComponents, + unhealthyReason: &[]string{"input"}, + err: nil, + }, { + name: "invalid JSON ignored", + agent: &model.Agent{ + LastCheckinStatus: "online", + UnhealthyReason: nil, + Components: json.RawMessage("{s"), }, req: &CheckinRequest{ Status: "DEGRADED", @@ -1046,6 +1074,12 @@ func TestParseComponents(t *testing.T) { } } +func requireMarshalJSON(t *testing.T, obj interface{}) json.RawMessage { + data, err := json.Marshal(obj) + require.NoError(t, err) + return data +} + func TestValidateCheckinRequest(t *testing.T) { verCon := mustBuildConstraints("8.0.0") diff --git a/internal/pkg/model/schema.go b/internal/pkg/model/schema.go index 92e78df86e..f462dbc71d 100644 --- a/internal/pkg/model/schema.go +++ b/internal/pkg/model/schema.go @@ -142,7 +142,7 @@ type Agent struct { AuditUnenrolledTime string `json:"audit_unenrolled_time,omitempty"` // Elastic Agent components detailed status information - Components []ComponentsItems `json:"components,omitempty"` + Components json.RawMessage `json:"components,omitempty"` // Deprecated. Use Outputs instead. API key the Elastic Agent uses to authenticate with elasticsearch DefaultAPIKey string `json:"default_api_key,omitempty"` @@ -335,8 +335,9 @@ type CheckinPolicyInputItems struct { TemplateID string `json:"template_id"` } -// ComponentsItems +// ComponentsItems Elastic Agent component detailed status information type ComponentsItems struct { + ESDocument ID string `json:"id,omitempty"` Message string `json:"message,omitempty"` Status string `json:"status,omitempty"` diff --git a/model/schema.json b/model/schema.json index 754d9c99d9..bc65f19dbd 100644 --- a/model/schema.json +++ b/model/schema.json @@ -477,6 +477,43 @@ } }, + "components_items": { + "title": "Component Items", + "description": "Elastic Agent component detailed status information", + "type": "object", + "properties": { + "id": { + "type": "string" + }, + "status": { + "type": "string" + }, + "message": { + "type": "string" + }, + "units": { + "type": "array", + "items": { + "type": "object", + "properties": { + "id": { + "type": "string" + }, + "type": { + "type": "string" + }, + "status": { + "type": "string" + }, + "message": { + "type": "string" + } + } + } + } + } + }, + "agent": { "title": "Agent", "description": "An Elastic Agent that has enrolled into Fleet", @@ -628,41 +665,7 @@ }, "components": { "description": "Elastic Agent components detailed status information", - "type": "array", - "items": { - "type": "object", - "properties": { - "id": { - "type": "string" - }, - "status": { - "type": "string" - }, - "message": { - "type": "string" - }, - "units": { - "type": "array", - "items": { - "type": "object", - "properties": { - "id": { - "type": "string" - }, - "type": { - "type": "string" - }, - "status": { - "type": "string" - }, - "message": { - "type": "string" - } - } - } - } - } - } + "format": "raw" }, "default_api_key_id": { "description": "Deprecated. Use Outputs instead. ID of the API key the Elastic Agent uses to authenticate with elasticsearch",