fix: enforce policy-based access control on artifact downloads#7009
fix: enforce policy-based access control on artifact downloads#7009ycombinator wants to merge 10 commits into
Conversation
The artifact download endpoint (/api/fleet/artifacts/{id}/{sha256})
previously only validated the agent's API key but never checked whether
the requested artifact belonged to the agent's assigned policy. This
allowed an agent enrolled under one policy to download artifacts from
a different policy if it knew the artifact ID and SHA256 hash.
Add authorizeArtifact implementation that fetches the agent's policy
from the in-memory policy monitor cache and verifies the requested
artifact appears in the policy's artifact_manifest before serving it.
Returns 403 Forbidden if the artifact is not in the agent's policy.
Resolves: elastic/security#8396
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
This pull request does not have a backport label. Could you fix it @ycombinator? 🙏
|
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
| if !ok { | ||
| continue | ||
| } | ||
| amMap, ok := am.(map[string]interface{}) |
There was a problem hiding this comment.
go's newer conventions prefers any over interface{}
this is being enforced by the go fix check that can be ran with mage check:fix
|
|
||
| func policyHasArtifact(pd *model.PolicyData, id, sha2 string) bool { | ||
| for _, input := range pd.Inputs { | ||
| am, ok := input["artifact_manifest"] |
There was a problem hiding this comment.
Should we define this artifact_manifest as a struct somewhere?
There was a problem hiding this comment.
Added model.ArtifactManifest and model.ManifestEntry structs in 3e57a6e.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Defines model.ArtifactManifest and model.ManifestEntry structs so policyHasArtifact no longer navigates untyped map[string]any chains. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
schema.go is code-generated and gets overwritten by mage generate. Moving ArtifactManifest and ManifestEntry to ext.go keeps them stable. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ArtifactManifest and ManifestEntry are not ES document types and only exist to support parsing within handleArtifacts.go, so they belong there rather than in the model package. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Agents enrolled under dummy-policy cannot download Elastic Defend artifacts because that policy has no artifact_manifest. Enroll the test agent under security-policy (which has the Elastic Defend integration) instead. Add FleetPolicyHasArtifact scaffold helper that polls .fleet-policies until the policy document references the artifact, ensuring fleet-server's policy monitor cache is up-to-date before the download attempt. Also retry the download on 403 to tolerate any remaining cache propagation lag. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
| if !ok { | ||
| return nil, nil | ||
| } | ||
| data, err := json.Marshal(raw) |
There was a problem hiding this comment.
Marshalling to then unmarshal feels a bit weird, there's no way to just unmarshal this? Maybe an intermediate type that has artifact_manifest as json.RawMessage instead of an any?
There was a problem hiding this comment.
Removed the marshal-then-unmarshal; agree, it was awkward. Just went with directly accessing map fields, with type assertions for safety: 535b14f
…checks PolicyData.Inputs is already decoded as []map[string]any, so marshaling back to JSON just to unmarshal again is unnecessary. Use type assertions directly on the decoded map in both policyHasArtifact (production) and policyInputHasArtifact (e2e scaffold), removing the artifactManifest and manifestEntry types along with parseArtifactManifest. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
2e1103d to
535b14f
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
…ntext The ES global checkpoint monitor can hold up to 4 minutes before the policy cache refreshes. On slow CI the setup steps (FleetHasArtifacts + FleetPolicyHasArtifact) could exhaust the 3-minute budget, leaving the retry loop to fail with a misleading "context deadline exceeded" from the HTTP call. Raise the budget to 5 minutes and add an explicit ctx.Err() check at the top of the retry loop so expiry surfaces a clear message. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
TL;DRBuildkite failed in Remediation
Investigation detailsRoot Cause
The Buildkite log shows a diff for // ... Licensed under the Elastic License;
// you may not use this file except in compliance with the Elastic License.to: // ... Licensed under the Elastic License 2.0;
// you may not use this file except in compliance with the Elastic License 2.0.That mutation made the repo dirty and triggered: Evidence
Verification
Follow-up
Note 🔒 Integrity filter blocked 4 itemsThe following items were blocked because they don't meet the GitHub integrity level.
To allow these resources, lower tools:
github:
min-integrity: approved # merged | approved | unapproved | noneWhat is this? | From workflow: PR Buildkite Detective Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not. |
What is the problem this PR solves?
The artifact download endpoint (
/api/fleet/artifacts/{id}/{sha256}) only validates the agent's API key but never checks whether the requested artifact belongs to the agent's assigned policy. This means an agent enrolled under one policy can download artifacts belonging to a different policy if it knows the artifact ID and SHA256 hash. For example, an agent enrolled under a policy with no integrations can retrieve Elastic Defend trust lists, exception lists, and other security artifacts from another policy.How does this PR solve the problem?
Implements the
authorizeArtifact()function (previously a no-op that returnednil) to enforce policy-based access control:GetPolicy(ctx, policyID)method to thepolicy.Monitorinterface that returns the cached policy for a given ID (reloads from ES on cache miss).authorizeArtifact, fetches the agent's policy via the monitor usingagent.AgentPolicyIDand verifies that the requested artifact (identifier+decoded_sha256) appears in the policy'sinputs[].artifact_manifest.artifacts.ErrUnauthorizedArtifact) if the artifact is not listed in the agent's assigned policy.How to test this PR locally
Design Checklist
Checklist
./changelog/fragmentsusing the changelog tool