fix: enforce policy-based access control on artifact downloads

changelog/fragments/1778540235-fix-artifact-access-control.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: security
+
+# Change summary; a 80ish characters long description of the change.
+summary: Enforce policy-based access control on artifact downloads
+
+# 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

internal/pkg/api/error.go

@@ -142,6 +142,24 @@ func NewHTTPErrResp(err error) HTTPErrResp {
 				zerolog.WarnLevel,
 			},
 		},
+		{
+			ErrUnauthorizedArtifact,
+			HTTPErrResp{
+				http.StatusForbidden,
+				"Forbidden",
+				"agent not authorized for artifact",
+				zerolog.WarnLevel,
+			},
+		},
+		{
+			ErrAgentPolicyIDMissing,
+			HTTPErrResp{
+				http.StatusForbidden,
+				"Forbidden",
+				"agent has no policy ID",
+				zerolog.WarnLevel,
+			},
+		},
 		{
 			ErrorThrottle,
 			HTTPErrResp{

internal/pkg/api/handleArtifacts.go

@@ -25,6 +25,7 @@ import (
 	"github.com/elastic/fleet-server/v7/internal/pkg/dl"
 	"github.com/elastic/fleet-server/v7/internal/pkg/logger"
 	"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/throttle"
 
 	"github.com/rs/zerolog"
@@ -36,23 +37,27 @@ const (
 )
 
 var (
-	ErrorThrottle     = errors.New("cannot acquire throttle token")
-	ErrorBadSha2      = errors.New("malformed sha256")
-	ErrorRecord       = errors.New("artifact record mismatch")
-	ErrorMismatchSha2 = errors.New("mismatched sha256")
+	ErrorThrottle           = errors.New("cannot acquire throttle token")
+	ErrorBadSha2            = errors.New("malformed sha256")
+	ErrorRecord             = errors.New("artifact record mismatch")
+	ErrorMismatchSha2       = errors.New("mismatched sha256")
+	ErrUnauthorizedArtifact = errors.New("agent not authorized for artifact")
+	ErrAgentPolicyIDMissing = errors.New("agent has no policy ID")
 )
 
 type ArtifactT struct {
 	bulker     bulk.Bulk
 	cache      cache.Cache
 	esThrottle *throttle.Throttle
+	pm         policy.Monitor
 }
 
-func NewArtifactT(cfg *config.Server, bulker bulk.Bulk, cache cache.Cache) *ArtifactT {
+func NewArtifactT(cfg *config.Server, bulker bulk.Bulk, cache cache.Cache, pm policy.Monitor) *ArtifactT {
 	return &ArtifactT{
 		bulker:     bulker,
 		cache:      cache,
 		esThrottle: throttle.NewThrottle(defaultMaxParallel),
+		pm:         pm,
 	}
 }
 
@@ -138,19 +143,73 @@ func (at ArtifactT) processRequest(ctx context.Context, zlog zerolog.Logger, age
 	return rdr, nil
 }
 
-// TODO: Pull the policy record for this agent and validate that the
-// requested artifact is assigned to this policy.  This will prevent
-// agents from retrieving artifacts that they do not have access to.
-// Note that this is racy, the policy could have changed to allow an
-// artifact before this instantiation of FleetServer has its local
-// copy updated.  Take the race conditions into consideration.
+// authorizeArtifact checks that the requested artifact is listed in the agent's
+// assigned policy. The policy is read from the in-memory cache maintained by the
+// policy monitor, which introduces a staleness window between ES updates and
+// cache refresh. This creates two race conditions:
 //
-// Initial implementation is dependent on security by obscurity; ie.
-// it should be difficult for an attacker to guess a guid.
-func (at ArtifactT) authorizeArtifact(ctx context.Context, _ *model.Agent, _, _ string) error {
-	span, _ := apm.StartSpan(ctx, "authorizeArtifacts", "auth") // TODO return and use span ctx if this is ever not a nop
+//   - False negative (deny when should allow): an artifact was just added to a
+//     policy but the cache hasn't refreshed yet. This is self-healing — the agent
+//     will retry and succeed once the cache catches up.
+//
+//   - False positive (allow when should deny): an artifact was just removed from
+//     a policy but the stale cache still lists it. The agent can download the
+//     artifact until the cache refreshes. This is the security-sensitive direction.
+//     A future improvement could verify against ES when the cache says "allow".
+func (at ArtifactT) authorizeArtifact(ctx context.Context, agent *model.Agent, id, sha2 string) error {
+	span, ctx := apm.StartSpan(ctx, "authorizeArtifacts", "auth")
 	defer span.End()
-	return nil // TODO
+
+	// AgentPolicyID (agent_policy_id) is set at first checkin; PolicyID (policy_id) is set
+	// at enrollment. Fall back to PolicyID so newly enrolled agents that have not yet
+	// checked in can still download artifacts for their assigned policy.
+	policyID := agent.AgentPolicyID
+	if policyID == "" {
+		policyID = agent.PolicyID
+	}
+	if policyID == "" {
+		return ErrAgentPolicyIDMissing
+	}
+
+	p, err := at.pm.GetPolicy(ctx, policyID)
+	if errors.Is(err, policy.ErrPolicyNotFound) {
+		return ErrUnauthorizedArtifact
+	}
+	if err != nil {
+		return fmt.Errorf("authorizeArtifact: %w", err)
+	}
+
+	if p.Data != nil && policyHasArtifact(p.Data, id, sha2) {
+		return nil
+	}
+
+	return ErrUnauthorizedArtifact
+}
+
+func policyHasArtifact(pd *model.PolicyData, id, sha2 string) bool {
+	for _, input := range pd.Inputs {
+		if inputHasArtifact(input, id, sha2) {
+			return true
+		}
+	}
+	return false
+}
+
+func inputHasArtifact(input map[string]any, id, sha2 string) bool {
+	manifestRaw, ok := input[dl.FieldArtifactManifest].(map[string]any)
+	if !ok {
+		return false
+	}
+	artifacts, ok := manifestRaw[dl.FieldArtifacts].(map[string]any)
+	if !ok {
+		return false
+	}
+	entry, ok := artifacts[id].(map[string]any)
+	if !ok {
+		return false
+	}
+	sha, _ := entry[dl.FieldDecodedSha256].(string)
+	return sha == sha2
 }
 
 // Return artifact from cache by sha2 or fetch directly from Elastic.