fix: enforce policy-based access control on artifact downloads#7009
Conversation
|
This pull request does not have a backport label. Could you fix it @ycombinator? 🙏
|
This comment has been minimized.
This comment has been minimized.
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.
This comment has been minimized.
This comment has been minimized.
michel-laterman
left a comment
There was a problem hiding this comment.
lgtm, just have nitpicks
…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>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…acts Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
9d7fccf to
5e5192b
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.
This comment has been minimized.
This comment has been minimized.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
TL;DRThe Buildkite failure is a hung E2E test in Remediation
Investigation detailsRoot CauseThe stack trace in the failed job shows the test goroutine waiting in
The collector command is started with context cancellation and a custom cancel handler:
If the child process does not terminate promptly on Evidence
Related flaky-test tracking: #6590 (same test area under Verification
Follow-upIf this test still flakes after adding a forced-stop path, consider explicitly asserting collector shutdown completion (with a bounded timeout) so hangs fail fast with a direct assertion instead of package timeout. Note 🔒 Integrity filter blocked 2 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. |
agent_policy_id is only populated after first checkin; policy_id is set at enrollment. Use policy_id as fallback so newly enrolled agents can download artifacts before their first checkin. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
@Mergifyio backport 9.4 9.3 8.19 |
✅ Backports have been createdDetails
Cherry-pick of caa8b2d has failed: To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally |
* fix: enforce policy-based access control on artifact downloads
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>
* chore: add changelog fragment for artifact access control fix
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* docs: document race condition tradeoffs in authorizeArtifact
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* fix: use any instead of interface{} per Go conventions
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* refactor: add typed ArtifactManifest struct for policy input parsing
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>
---------
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
(cherry picked from commit caa8b2d)
# Conflicts:
# internal/pkg/api/handleCheckin_test.go
# internal/pkg/policy/monitor.go
The mergify backport left unresolved conflict markers committed in monitor.go and handleCheckin_test.go, breaking the build. It also carried over agent.AgentPolicyID, which doesn't exist in 8.19's Agent model (added later by #5501, main-only, backport-skip); use agent.PolicyID directly instead, since on 8.19 it is the sole, server-controlled source of truth for policy assignment throughout the agent lifecycle. Also renamed handleStatus_test.go's mockPolicyMonitor to mockStatusPolicyMonitor to avoid colliding with the mock introduced by this backport, matching main's naming. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
LatestRev predates #7009 on main (added by #5501, a backport-skip, main-only feature) and has no caller on 8.19 since #5501 was never backported here. The mergify conflict resolution pulled it in anyway as collateral from the adjacent Monitor interface block. Drop it from the interface, monitorT, and the test mocks so the backport only carries #7009's actual contribution: GetPolicy. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
#7164) * fix: enforce policy-based access control on artifact downloads 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: https://github.com/elastic/security/issues/8396 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * chore: add changelog fragment for artifact access control fix Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * docs: document race condition tradeoffs in authorizeArtifact Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: use any instead of interface{} per Go conventions Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor: add typed ArtifactManifest struct for policy input parsing 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> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> (cherry picked from commit caa8b2d) # Conflicts: # internal/pkg/api/handleCheckin_test.go # internal/pkg/policy/monitor.go Co-authored-by: Shaunak Kashyap <ycombinator@gmail.com>
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