From 908bfbfc271b6ea92ca2cf72dc4843f8d7dadfbb Mon Sep 17 00:00:00 2001 From: Josh Siviour Date: Sun, 19 Jul 2026 12:44:11 +1000 Subject: [PATCH] fix(ecr): enforce IAM authorization on OCI registry operations (#548) Treat the ECR registry JWT as a signed identity pointer rather than a permission grant. Every /v2/* request now rehydrates the token's access key against current IAM/STS state, maps the OCI route to an ecr: action and repository ARN, and runs the identity-policy evaluator before dispatch. PullOnly/ReadOnly principals can no longer push, overwrite tags, or delete manifests, and key/session/user/role/account/policy revocation takes effect on the next request instead of at token expiry. Cross-repository blob mounts now require source-read permission and proof the digest is referenced by a manifest in the named source repo, closing an account-wide blob-fishing gap. --- spinifex/gateway/ecr.go | 7 + spinifex/gateway/ecr/authorization.go | 181 +++++++ spinifex/gateway/ecr/authorization_test.go | 216 ++++++++ spinifex/gateway/ecr/registry.go | 53 +- spinifex/gateway/ecr/registry_test.go | 60 ++- spinifex/gateway/ecr_authorization.go | 84 +++ spinifex/gateway/ecr_authorization_test.go | 296 +++++++++++ spinifex/gateway/ecr_getauthorizationtoken.go | 14 +- .../gateway/ecr_getauthorizationtoken_test.go | 4 + spinifex/gateway/ecr_principal.go | 258 +++++++++ spinifex/gateway/ecr_principal_test.go | 492 ++++++++++++++++++ spinifex/gateway/ecr_test.go | 8 + spinifex/gateway/ecr_token.go | 31 +- spinifex/gateway/ecr_token_test.go | 22 +- spinifex/gateway/ecrauth/ecrauth_test.go | 94 +++- spinifex/gateway/ecrauth/token.go | 57 +- spinifex/gateway/ecrauth_middleware.go | 24 +- spinifex/gateway/ecrauth_middleware_test.go | 31 +- spinifex/gateway/gateway.go | 80 ++- tests/e2e/ecr/iam_authorization_test.go | 304 +++++++++++ tests/e2e/harness/oci_raw.go | 53 ++ 21 files changed, 2326 insertions(+), 43 deletions(-) create mode 100644 spinifex/gateway/ecr/authorization.go create mode 100644 spinifex/gateway/ecr/authorization_test.go create mode 100644 spinifex/gateway/ecr_authorization.go create mode 100644 spinifex/gateway/ecr_authorization_test.go create mode 100644 spinifex/gateway/ecr_principal.go create mode 100644 spinifex/gateway/ecr_principal_test.go create mode 100644 tests/e2e/ecr/iam_authorization_test.go create mode 100644 tests/e2e/harness/oci_raw.go diff --git a/spinifex/gateway/ecr.go b/spinifex/gateway/ecr.go index 7d32f73b8..9da9716fd 100644 --- a/spinifex/gateway/ecr.go +++ b/spinifex/gateway/ecr.go @@ -17,6 +17,12 @@ import ( // version-check probe succeeds even before any repository exists. When no // Registry is wired (e.g. unit tests of unrelated routes), the surface falls // back to the 501 stub. +// +// Requests flow hostMatch -> ecrAuthBridge -> ecrOperationAuthorization -> +// Registry: the bridge authenticates the token and rehydrates its principal +// from current IAM/STS state, then authorization classifies the OCI +// operation and evaluates it against that principal's policies before the +// request ever reaches Registry's object/metadata stores. func (gw *GatewayConfig) mountOCIRegistry(r chi.Router) { r.Route("/v2", func(v2 chi.Router) { v2.Use(gw.hostMatch) @@ -27,6 +33,7 @@ func (gw *GatewayConfig) mountOCIRegistry(r chi.Router) { v2.Group(func(reg chi.Router) { reg.Use(gw.ecrAuthBridge) + reg.Use(gw.ecrOperationAuthorization) reg.Get("/", gateway_ecr.APIVersion) if gw.ECRRegistry != nil { reg.HandleFunc("/*", gw.ECRRegistry.ServeHTTP) diff --git a/spinifex/gateway/ecr/authorization.go b/spinifex/gateway/ecr/authorization.go new file mode 100644 index 000000000..fdab10e28 --- /dev/null +++ b/spinifex/gateway/ecr/authorization.go @@ -0,0 +1,181 @@ +package gateway_ecr + +import ( + "net/http" + "net/url" + "strings" +) + +// ECR IAM action names, unprefixed. Combine with policy.IAMAction("ecr", ...) +// to build the "ecr:Action" string the identity-policy evaluator expects. +const ( + ActionDescribeRepositories = "DescribeRepositories" + ActionListImages = "ListImages" + ActionBatchCheckLayerAvailability = "BatchCheckLayerAvailability" + ActionGetDownloadUrlForLayer = "GetDownloadUrlForLayer" + ActionInitiateLayerUpload = "InitiateLayerUpload" + ActionUploadLayerPart = "UploadLayerPart" + ActionCompleteLayerUpload = "CompleteLayerUpload" + ActionBatchGetImage = "BatchGetImage" + ActionPutImage = "PutImage" + ActionBatchDeleteImage = "BatchDeleteImage" +) + +// ResourceScope identifies which repository ARN an ActionRequirement is +// evaluated against. +type ResourceScope int + +const ( + // ScopeNone means the operation needs a validly rehydrated principal but + // no resource-specific policy decision (the bare /v2/ version probe). + ScopeNone ResourceScope = iota + // ScopeAccountWildcard is the account-wide repository/* ARN (catalog listing). + ScopeAccountWildcard + // ScopeDestination is the operation's target repository. + ScopeDestination + // ScopeSource is the cross-repository mount's source repository. + ScopeSource +) + +// ActionRequirement is one ECR IAM action an operation must be allowed +// before dispatch, and the resource scope it is evaluated against. +type ActionRequirement struct { + Action string + Scope ResourceScope +} + +// ClassifiedOperation is the result of mapping an OCI /v2/* request to its +// required ECR authorization. Repo is the destination repository name (empty +// for account-scoped or repo-less operations). Source and MountDigest are +// populated only for a cross-repository blob-mount attempt (?mount=&from=). +type ClassifiedOperation struct { + Repo string + Source string + MountDigest string + // Requirements lists every ActionRequirement that must independently + // evaluate allow before the operation may dispatch. Empty for the bare + // version probe, which still requires a resolved principal but no + // resource-specific decision. + Requirements []ActionRequirement +} + +// ClassifyOperation maps method and urlPath — the request's full path as +// Registry.serve sees it ("/v2/..." including the mount prefix) — to the ECR +// operation it names. ok is false for any method/path pair Registry does not +// implement, so a future dispatch branch added without a matching case here +// fails authorization closed instead of silently dispatching unauthorized. +// query supplies the mount/digest parameters for a blob-upload-start request. +func ClassifyOperation(method, urlPath string, query url.Values) (ClassifiedOperation, bool) { + path := strings.TrimPrefix(urlPath, "/v2/") + if path == urlPath { + path = strings.TrimPrefix(urlPath, "/v2") + } + + if path == "" { + if method == http.MethodGet { + return ClassifiedOperation{}, true + } + return ClassifiedOperation{}, false + } + + if path == "_catalog" { + if method != http.MethodGet { + return ClassifiedOperation{}, false + } + return ClassifiedOperation{ + Requirements: []ActionRequirement{{ActionDescribeRepositories, ScopeAccountWildcard}}, + }, true + } + + name, kind, ref, ok := splitV2Path(path) + if !ok { + return ClassifiedOperation{}, false + } + + switch kind { + case "tags": + return classifyTagsOperation(method, name, ref) + case "blobs": + return classifyBlobOperation(method, name, ref, query) + case "manifests": + return classifyManifestOperation(method, name, ref) + default: + return ClassifiedOperation{}, false + } +} + +func classifyTagsOperation(method, name, ref string) (ClassifiedOperation, bool) { + if method != http.MethodGet || ref != "list" { + return ClassifiedOperation{}, false + } + return ClassifiedOperation{Repo: name, Requirements: []ActionRequirement{{ActionListImages, ScopeDestination}}}, true +} + +// classifyBlobOperation mirrors Registry.routeBlobs' dispatch exactly, so +// every method/ref combination it accepts has a matching authorization case. +func classifyBlobOperation(method, name, ref string, query url.Values) (ClassifiedOperation, bool) { + switch { + case ref == "uploads/" || ref == "uploads": + if method != http.MethodPost { + return ClassifiedOperation{}, false + } + op := ClassifiedOperation{ + Repo: name, + Requirements: []ActionRequirement{{ActionInitiateLayerUpload, ScopeDestination}}, + } + // A cross-repository mount additionally requires source-layer read + // permission. Both mount and from must be present — a mount digest with + // no source (or vice versa) is not a mount attempt, matching Registry's + // own "mount miss falls through to a normal upload" behavior. + mountDigest, from := query.Get("mount"), query.Get("from") + if mountDigest != "" && from != "" { + op.Source = from + op.MountDigest = mountDigest + op.Requirements = append(op.Requirements, ActionRequirement{ActionBatchCheckLayerAvailability, ScopeSource}) + } + return op, true + case strings.HasPrefix(ref, "uploads/"): + if strings.TrimPrefix(ref, "uploads/") == "" { + return ClassifiedOperation{}, false + } + switch method { + case http.MethodPatch: + return ClassifiedOperation{Repo: name, Requirements: []ActionRequirement{{ActionUploadLayerPart, ScopeDestination}}}, true + case http.MethodPut: + return ClassifiedOperation{Repo: name, Requirements: []ActionRequirement{{ActionCompleteLayerUpload, ScopeDestination}}}, true + case http.MethodDelete: + // ECR defines no abort-upload IAM action; UploadLayerPart is the + // permission governing mutation of an in-progress upload, so cancel + // requires it too. Pinned compatibility decision, not an oversight. + return ClassifiedOperation{Repo: name, Requirements: []ActionRequirement{{ActionUploadLayerPart, ScopeDestination}}}, true + default: + return ClassifiedOperation{}, false + } + default: + // ref is a digest. + switch method { + case http.MethodHead: + return ClassifiedOperation{Repo: name, Requirements: []ActionRequirement{{ActionBatchCheckLayerAvailability, ScopeDestination}}}, true + case http.MethodGet: + return ClassifiedOperation{Repo: name, Requirements: []ActionRequirement{{ActionGetDownloadUrlForLayer, ScopeDestination}}}, true + default: + return ClassifiedOperation{}, false + } + } +} + +func classifyManifestOperation(method, name, reference string) (ClassifiedOperation, bool) { + if reference == "" { + return ClassifiedOperation{}, false + } + switch method { + case http.MethodHead, http.MethodGet: + return ClassifiedOperation{Repo: name, Requirements: []ActionRequirement{{ActionBatchGetImage, ScopeDestination}}}, true + case http.MethodPut: + return ClassifiedOperation{Repo: name, Requirements: []ActionRequirement{{ActionPutImage, ScopeDestination}}}, true + case http.MethodDelete: + return ClassifiedOperation{Repo: name, Requirements: []ActionRequirement{{ActionBatchDeleteImage, ScopeDestination}}}, true + default: + return ClassifiedOperation{}, false + } +} diff --git a/spinifex/gateway/ecr/authorization_test.go b/spinifex/gateway/ecr/authorization_test.go new file mode 100644 index 000000000..cebb9be06 --- /dev/null +++ b/spinifex/gateway/ecr/authorization_test.go @@ -0,0 +1,216 @@ +package gateway_ecr + +import ( + "net/http" + "net/url" + "testing" + + "github.com/stretchr/testify/assert" +) + +// TestClassifyOperation_RouteMatrix pins the plan's OCI request -> ECR action +// -> resource table exactly. Every row here must match the plan doc's table; +// changing an entry without a corresponding plan-doc update is a regression. +func TestClassifyOperation_RouteMatrix(t *testing.T) { + cases := []struct { + name string + method string + path string + query url.Values + want ClassifiedOperation + }{ + { + name: "GET /v2/ version probe", + method: http.MethodGet, + path: "/v2/", + want: ClassifiedOperation{}, + }, + { + name: "GET /v2/_catalog", + method: http.MethodGet, + path: "/v2/_catalog", + want: ClassifiedOperation{ + Requirements: []ActionRequirement{{ActionDescribeRepositories, ScopeAccountWildcard}}, + }, + }, + { + name: "GET tags list", + method: http.MethodGet, + path: "/v2/team/app/tags/list", + want: ClassifiedOperation{ + Repo: "team/app", + Requirements: []ActionRequirement{{ActionListImages, ScopeDestination}}, + }, + }, + { + name: "HEAD blob", + method: http.MethodHead, + path: "/v2/team/app/blobs/sha256:abc", + want: ClassifiedOperation{ + Repo: "team/app", + Requirements: []ActionRequirement{{ActionBatchCheckLayerAvailability, ScopeDestination}}, + }, + }, + { + name: "GET blob", + method: http.MethodGet, + path: "/v2/team/app/blobs/sha256:abc", + want: ClassifiedOperation{ + Repo: "team/app", + Requirements: []ActionRequirement{{ActionGetDownloadUrlForLayer, ScopeDestination}}, + }, + }, + { + name: "POST start upload", + method: http.MethodPost, + path: "/v2/team/app/blobs/uploads/", + want: ClassifiedOperation{ + Repo: "team/app", + Requirements: []ActionRequirement{{ActionInitiateLayerUpload, ScopeDestination}}, + }, + }, + { + name: "PATCH upload chunk", + method: http.MethodPatch, + path: "/v2/team/app/blobs/uploads/upload-1", + want: ClassifiedOperation{ + Repo: "team/app", + Requirements: []ActionRequirement{{ActionUploadLayerPart, ScopeDestination}}, + }, + }, + { + name: "PUT finish upload", + method: http.MethodPut, + path: "/v2/team/app/blobs/uploads/upload-1", + want: ClassifiedOperation{ + Repo: "team/app", + Requirements: []ActionRequirement{{ActionCompleteLayerUpload, ScopeDestination}}, + }, + }, + { + name: "DELETE cancel upload uses UploadLayerPart (no abort action exists)", + method: http.MethodDelete, + path: "/v2/team/app/blobs/uploads/upload-1", + want: ClassifiedOperation{ + Repo: "team/app", + Requirements: []ActionRequirement{{ActionUploadLayerPart, ScopeDestination}}, + }, + }, + { + name: "HEAD manifest", + method: http.MethodHead, + path: "/v2/team/app/manifests/latest", + want: ClassifiedOperation{ + Repo: "team/app", + Requirements: []ActionRequirement{{ActionBatchGetImage, ScopeDestination}}, + }, + }, + { + name: "GET manifest", + method: http.MethodGet, + path: "/v2/team/app/manifests/latest", + want: ClassifiedOperation{ + Repo: "team/app", + Requirements: []ActionRequirement{{ActionBatchGetImage, ScopeDestination}}, + }, + }, + { + name: "PUT manifest", + method: http.MethodPut, + path: "/v2/team/app/manifests/latest", + want: ClassifiedOperation{ + Repo: "team/app", + Requirements: []ActionRequirement{{ActionPutImage, ScopeDestination}}, + }, + }, + { + name: "DELETE manifest", + method: http.MethodDelete, + path: "/v2/team/app/manifests/sha256:abc", + want: ClassifiedOperation{ + Repo: "team/app", + Requirements: []ActionRequirement{{ActionBatchDeleteImage, ScopeDestination}}, + }, + }, + { + name: "cross-repo mount requires destination initiate + source read", + method: http.MethodPost, + path: "/v2/team/app/blobs/uploads/", + query: url.Values{"mount": {"sha256:abc"}, "from": {"team/base"}}, + want: ClassifiedOperation{ + Repo: "team/app", + Source: "team/base", + MountDigest: "sha256:abc", + Requirements: []ActionRequirement{ + {ActionInitiateLayerUpload, ScopeDestination}, + {ActionBatchCheckLayerAvailability, ScopeSource}, + }, + }, + }, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + q := c.query + if q == nil { + q = url.Values{} + } + got, ok := ClassifyOperation(c.method, c.path, q) + assert.True(t, ok, "expected %s %s to classify", c.method, c.path) + assert.Equal(t, c.want, got) + }) + } +} + +// TestClassifyOperation_MountRequiresBothParams pins that a mount digest or +// source repo alone is not a mount attempt: it classifies as a plain upload +// start (Registry itself falls through to a normal upload on a mount miss). +func TestClassifyOperation_MountRequiresBothParams(t *testing.T) { + cases := []struct { + name string + query url.Values + }{ + {"digest without source", url.Values{"mount": {"sha256:abc"}}}, + {"source without digest", url.Values{"from": {"team/base"}}}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + got, ok := ClassifyOperation(http.MethodPost, "/v2/team/app/blobs/uploads/", c.query) + assert.True(t, ok) + assert.Empty(t, got.Source) + assert.Empty(t, got.MountDigest) + assert.Equal(t, []ActionRequirement{{ActionInitiateLayerUpload, ScopeDestination}}, got.Requirements) + }) + } +} + +// TestClassifyOperation_UnsupportedNeverClassifies verifies that unsupported +// methods and malformed paths never authorize an operation: ok must be +// false, so no ActionRequirement can leak through unmapped. +func TestClassifyOperation_UnsupportedNeverClassifies(t *testing.T) { + cases := []struct { + name string + method string + path string + }{ + {"POST /v2/ unsupported method", http.MethodPost, "/v2/"}, + {"malformed path, no marker", http.MethodGet, "/v2/team/app"}, + {"empty repo name", http.MethodGet, "/v2//tags/list"}, + {"unknown kind segment", http.MethodGet, "/v2/team/app/unknown/list"}, + {"tags list wrong method", http.MethodPost, "/v2/team/app/tags/list"}, + {"tags wrong ref", http.MethodGet, "/v2/team/app/tags/other"}, + {"blob PUT unsupported", http.MethodPut, "/v2/team/app/blobs/sha256:abc"}, + {"blob upload empty uuid", http.MethodPatch, "/v2/team/app/blobs/uploads/"}, + {"blob upload GET unsupported", http.MethodGet, "/v2/team/app/blobs/uploads/upload-1"}, + {"manifest POST unsupported", http.MethodPost, "/v2/team/app/manifests/latest"}, + {"manifest empty reference", http.MethodGet, "/v2/team/app/manifests/"}, + {"catalog wrong method", http.MethodPost, "/v2/_catalog"}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + got, ok := ClassifyOperation(c.method, c.path, url.Values{}) + assert.False(t, ok, "expected %s %s to be unclassified", c.method, c.path) + assert.Equal(t, ClassifiedOperation{}, got) + }) + } +} diff --git a/spinifex/gateway/ecr/registry.go b/spinifex/gateway/ecr/registry.go index c703c3595..761b443dc 100644 --- a/spinifex/gateway/ecr/registry.go +++ b/spinifex/gateway/ecr/registry.go @@ -12,6 +12,7 @@ import ( "io" "log/slog" "net/http" + "slices" "strconv" "strings" "sync" @@ -100,6 +101,17 @@ func (reg *Registry) ServeHTTP(w http.ResponseWriter, r *http.Request) { // trailing "/blobs", "/manifests" or "/tags" marker. func (reg *Registry) serve(w http.ResponseWriter, r *http.Request) { ctx := r.Context() + + // ClassifyOperation is the same route table the authorization middleware + // consumes to decide required IAM actions. Re-checking it here means a + // request that reaches Registry without having been classified (e.g. the + // authorization middleware disabled, or a future caller of serve that + // forgets to wire it) is refused rather than silently dispatched. + if _, ok := ClassifyOperation(r.Method, r.URL.Path, r.URL.Query()); !ok { + WriteError(w, http.StatusNotFound, "NAME_UNKNOWN", "unrecognized registry operation") + return + } + if err := reg.ensureBucket(ctx); err != nil { reg.internal(ctx, w, "ensure bucket", err) return @@ -251,13 +263,21 @@ func (reg *Registry) startUpload(w http.ResponseWriter, r *http.Request, name st return } + // A mount is only recognised when both mount and from are present, matching + // the authorization middleware's classification: with only one of the two, + // this falls through to a normal upload start. A valid mount additionally + // requires digest to be provably reachable from a manifest stored in the + // source repository — checking only that the blob exists in the + // account-wide pool would let a caller name an unrelated permitted source + // repository to fish out a blob it has no read access to. A miss on any of + // these checks falls through silently: a denied or invalid source must not + // reveal whether an account-level blob exists. q := r.URL.Query() - if mountDigest := q.Get("mount"); mountDigest != "" { - if ecr.ValidateDigest(mountDigest) && reg.blobExists(ctx, mountDigest) { + if mountDigest, source := q.Get("mount"), q.Get("from"); mountDigest != "" && source != "" { + if ecr.ValidateDigest(mountDigest) && reg.blobExists(ctx, mountDigest) && reg.mountedFromValidSource(ctx, source, mountDigest) { reg.writeBlobCreated(w, name, mountDigest) return } - // Mount miss: fall through to a normal upload start per the spec. } uploadID := uuid.NewString() @@ -770,6 +790,33 @@ func (reg *Registry) blobExists(ctx context.Context, digest string) bool { return err == nil } +// mountedFromValidSource reports whether digest is provably reachable from +// source: source must exist, and some manifest stored under source must +// reference digest as a config or layer blob. Blob storage is shared across +// every repository in the account, so checking only that the blob exists in +// the pool would let a caller mount an unrelated account-level blob by +// naming any source repository it happens to have read access to; requiring +// provenance through an actual manifest closes that gap. +func (reg *Registry) mountedFromValidSource(ctx context.Context, source, digest string) bool { + if _, err := reg.Meta.GetRepo(ctx, reg.AccountID, source); err != nil { + return false + } + digests, err := reg.Meta.ListManifests(ctx, reg.AccountID, source) + if err != nil { + return false + } + for _, d := range digests { + meta, err := reg.Meta.GetManifestMeta(ctx, reg.AccountID, source, d) + if err != nil { + continue + } + if slices.Contains(meta.ChildDigests, digest) { + return true + } + } + return false +} + // readUploadBytesAt returns the bytes at key, or nil for an empty key or a miss // (a fresh upload has no committed bytes yet). func (reg *Registry) readUploadBytesAt(ctx context.Context, key string) []byte { diff --git a/spinifex/gateway/ecr/registry_test.go b/spinifex/gateway/ecr/registry_test.go index 21ec15d0c..999560f5e 100644 --- a/spinifex/gateway/ecr/registry_test.go +++ b/spinifex/gateway/ecr/registry_test.go @@ -147,13 +147,28 @@ func TestBlobUpload_Cancel(t *testing.T) { assert.Equal(t, http.StatusNotFound, w.Code) } +// putManifestReferencing PUTs a minimal image manifest into repo that +// references digest as its config blob, so digest is provably reachable +// from repo for mountedFromValidSource. +func putManifestReferencing(t *testing.T, reg *Registry, repo, digest string) { + t.Helper() + manifest := fmt.Appendf(nil, + `{"schemaVersion":2,"mediaType":"%s","config":{"digest":"%s"},"layers":[]}`, + mediaTypeDockerManifest, digest) + w := do(reg, http.MethodPut, "/v2/"+repo+"/manifests/v1", manifest, + map[string]string{"Content-Type": mediaTypeDockerManifest}) + require.Equal(t, http.StatusCreated, w.Code, "body: %s", w.Body.String()) +} + func TestBlobMount_HitAndMiss(t *testing.T) { reg := newTestRegistry() data := []byte("shared-layer") dg := pushBlob(t, reg, "team/source", data) + putManifestReferencing(t, reg, "team/source", dg) seedRepo(t, reg, "team/dest") - // Mount hit: blob already in account pool -> 201 without upload. + // Mount hit: blob in the account pool and reachable via a manifest in the + // source repo -> 201 without upload. w := do(reg, http.MethodPost, "/v2/team/dest/blobs/uploads/?mount="+dg+"&from=team/source", nil, nil) assert.Equal(t, http.StatusCreated, w.Code) @@ -165,6 +180,49 @@ func TestBlobMount_HitAndMiss(t *testing.T) { assert.Equal(t, http.StatusAccepted, w.Code) } +// TestBlobMount_MissingFromFallsThroughToUpload verifies mount is only +// recognised when both mount and from are present; mount alone is treated +// as a plain upload start, matching the authorization middleware's +// classification of the same request. +func TestBlobMount_MissingFromFallsThroughToUpload(t *testing.T) { + reg := newTestRegistry() + data := []byte("shared-layer") + dg := pushBlob(t, reg, "team/source", data) + putManifestReferencing(t, reg, "team/source", dg) + seedRepo(t, reg, "team/dest") + + w := do(reg, http.MethodPost, "/v2/team/dest/blobs/uploads/?mount="+dg, nil, nil) + assert.Equal(t, http.StatusAccepted, w.Code) +} + +// TestBlobMount_UnreferencedDigestFallsThrough verifies a digest that exists +// in the account-wide blob pool but is not referenced by any manifest in the +// named source repository is rejected as a mount — proving digest existence +// alone is not enough to mount cross-repo. +func TestBlobMount_UnreferencedDigestFallsThrough(t *testing.T) { + reg := newTestRegistry() + unreferenced := pushBlob(t, reg, "team/other", []byte("not-in-source")) + seedRepo(t, reg, "team/source") + seedRepo(t, reg, "team/dest") + + w := do(reg, http.MethodPost, + "/v2/team/dest/blobs/uploads/?mount="+unreferenced+"&from=team/source", nil, nil) + assert.Equal(t, http.StatusAccepted, w.Code) +} + +// TestBlobMount_NonexistentSourceFallsThrough verifies naming a source +// repository that does not exist never mounts, and never distinguishes +// itself from an unreferenced-digest miss (both fall through identically). +func TestBlobMount_NonexistentSourceFallsThrough(t *testing.T) { + reg := newTestRegistry() + dg := pushBlob(t, reg, "team/elsewhere", []byte("some-bytes")) + seedRepo(t, reg, "team/dest") + + w := do(reg, http.MethodPost, + "/v2/team/dest/blobs/uploads/?mount="+dg+"&from=team/does-not-exist", nil, nil) + assert.Equal(t, http.StatusAccepted, w.Code) +} + func TestManifest_PutGetHead_ImageManifest(t *testing.T) { reg := newTestRegistry() repo := "team/app" diff --git a/spinifex/gateway/ecr_authorization.go b/spinifex/gateway/ecr_authorization.go new file mode 100644 index 000000000..a8ff9c345 --- /dev/null +++ b/spinifex/gateway/ecr_authorization.go @@ -0,0 +1,84 @@ +package gateway + +import ( + "fmt" + "net/http" + + "github.com/mulgadc/spinifex/spinifex/awserrors" + gateway_ecr "github.com/mulgadc/spinifex/spinifex/gateway/ecr" + "github.com/mulgadc/spinifex/spinifex/gateway/policy" +) + +// ecrOperationAuthorization runs after ecrAuthBridge has rehydrated and +// stashed the current principal: it classifies the request's OCI operation, +// builds the repository ARN(s) the route requires, and evaluates every +// required ecr: action through the same identity-policy evaluator SigV4 +// requests use. The request only reaches Registry once every requirement +// evaluates allow — a token that authenticates a real but under-permissioned +// principal (e.g. AmazonEC2ContainerRegistryPullOnly) must not push, delete, +// or overwrite despite carrying a validly signed token. +// +// A nil ECRTokenVerifier disables ecrAuthBridge upstream (registry mounts +// open in unit tests of unrelated routes); this middleware mirrors that +// bypass so it never blocks those tests. +func (gw *GatewayConfig) ecrOperationAuthorization(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if gw.ECRTokenVerifier == nil { + next.ServeHTTP(w, r) + return + } + + principal, ok := r.Context().Value(ctxECRPrincipal).(principalContext) + if !ok { + // ecrAuthBridge did not stash a principal: it either already rejected + // the request (this handler is never reached) or is disabled in a way + // that skipped rehydration. Fail closed rather than dispatch with no + // principal to evaluate. + gateway_ecr.WriteError(w, http.StatusUnauthorized, "UNAUTHORIZED", "authentication required") + return + } + + op, ok := gateway_ecr.ClassifyOperation(r.Method, r.URL.Path, r.URL.Query()) + if !ok { + // Registry itself would 404 this path/method combination; refusing it + // here too means an operation added to Registry without a matching + // classifier case can never dispatch unauthorized. + gateway_ecr.WriteError(w, http.StatusNotFound, "NAME_UNKNOWN", "unrecognized registry operation") + return + } + + for _, req := range op.Requirements { + resource, err := ecrResourceARN(gw.Region, principal.accountID, req.Scope, op) + if err != nil { + gateway_ecr.WriteError(w, http.StatusServiceUnavailable, "UNKNOWN", "authorization unavailable") + return + } + if err := gw.evaluatePrincipalPolicy(principal, policy.IAMAction("ecr", req.Action), resource); err != nil { + if err.Error() == awserrors.ErrorInternalError { + // IAM/STS/NATS dependency failure: fail closed, never dispatch. + gateway_ecr.WriteError(w, http.StatusServiceUnavailable, "UNKNOWN", "authorization unavailable") + return + } + gateway_ecr.WriteError(w, http.StatusForbidden, "DENIED", "insufficient permissions") + return + } + } + + next.ServeHTTP(w, r) + }) +} + +// ecrResourceARN builds the repository ARN an ActionRequirement's scope +// resolves to, from the gateway's configured region and the token's account. +func ecrResourceARN(region, accountID string, scope gateway_ecr.ResourceScope, op gateway_ecr.ClassifiedOperation) (string, error) { + switch scope { + case gateway_ecr.ScopeAccountWildcard: + return fmt.Sprintf("arn:aws:ecr:%s:%s:repository/*", region, accountID), nil + case gateway_ecr.ScopeDestination: + return fmt.Sprintf("arn:aws:ecr:%s:%s:repository/%s", region, accountID, op.Repo), nil + case gateway_ecr.ScopeSource: + return fmt.Sprintf("arn:aws:ecr:%s:%s:repository/%s", region, accountID, op.Source), nil + default: + return "", fmt.Errorf("unhandled ECR resource scope %d", scope) + } +} diff --git a/spinifex/gateway/ecr_authorization_test.go b/spinifex/gateway/ecr_authorization_test.go new file mode 100644 index 000000000..db323c5bb --- /dev/null +++ b/spinifex/gateway/ecr_authorization_test.go @@ -0,0 +1,296 @@ +package gateway + +import ( + "context" + "errors" + "net/http" + "net/http/httptest" + "testing" + + "github.com/mulgadc/spinifex/spinifex/awserrors" + handlers_iam "github.com/mulgadc/spinifex/spinifex/handlers/iam" + "github.com/stretchr/testify/assert" +) + +const ( + ecrAuthzTestAccount = "000000000077" + ecrAuthzTestUser = "svc" +) + +// ecrAllowPolicy and ecrDenyPolicy build single-statement policy documents for +// the ECR operation-authorization tests below. They are distinct from +// auth_test.go's allowPolicy/allowPolicyResource helpers because those are +// fixed to a single action string; ECR route classification can require +// multiple actions per operation (e.g. cross-repo mount). +func ecrAllowPolicy(actions, resources []string) handlers_iam.PolicyDocument { + return handlers_iam.PolicyDocument{ + Version: "2012-10-17", + Statement: []handlers_iam.Statement{ + {Effect: "Allow", Action: actions, Resource: resources}, + }, + } +} + +func ecrDenyPolicy(actions, resources []string) handlers_iam.PolicyDocument { + return handlers_iam.PolicyDocument{ + Version: "2012-10-17", + Statement: []handlers_iam.Statement{ + {Effect: "Deny", Action: actions, Resource: resources}, + }, + } +} + +// ecrAuthzTestGateway builds a GatewayConfig wired for ecrOperationAuthorization +// tests: a non-nil ECRTokenVerifier (so the middleware is active) and an +// ecrMockIAMService seeded with ecrAuthzTestUser's policy documents. +func ecrAuthzTestGateway(t *testing.T, policies []handlers_iam.PolicyDocument) (*GatewayConfig, *ecrMockIAMService) { + t.Helper() + _, verify := newECRAuth(t) + iamSvc := newECRMockIAMService() + iamSvc.userPolicies[ecrAuthzTestAccount+"|"+ecrAuthzTestUser] = policies + return &GatewayConfig{ + Region: ecrTestRegion, + ECRTokenVerifier: verify, + IAMService: iamSvc, + }, iamSvc +} + +// ecrAuthzRequest builds a request carrying a resolved principal, as +// ecrAuthBridge would stash it, for method/path/query. +func ecrAuthzRequest(method, path string, principal principalContext) *http.Request { + req := httptest.NewRequest(method, path, nil) + ctx := context.WithValue(req.Context(), ctxECRPrincipal, principal) + return req.WithContext(ctx) +} + +func ecrTestUserPrincipal() principalContext { + return principalContext{identity: ecrAuthzTestUser, accountID: ecrAuthzTestAccount, principalType: principalTypeUser} +} + +// TestECROperationAuthorization_PullOnlyMatrix pins the release-gate positive +// and negative matrix for a PullOnly-shaped policy: pull operations succeed, +// every push/tag-overwrite/delete operation is denied. +func TestECROperationAuthorization_PullOnlyMatrix(t *testing.T) { + repoARN := "arn:aws:ecr:" + ecrTestRegion + ":" + ecrAuthzTestAccount + ":repository/team/app" + pullOnly := ecrAllowPolicy( + []string{"ecr:GetDownloadUrlForLayer", "ecr:BatchGetImage", "ecr:BatchCheckLayerAvailability"}, + []string{repoARN}, + ) + gw, _ := ecrAuthzTestGateway(t, []handlers_iam.PolicyDocument{pullOnly}) + + allowed := []struct{ method, path string }{ + {http.MethodHead, "/v2/team/app/blobs/sha256:abc"}, + {http.MethodGet, "/v2/team/app/blobs/sha256:abc"}, + {http.MethodHead, "/v2/team/app/manifests/latest"}, + {http.MethodGet, "/v2/team/app/manifests/latest"}, + } + for _, c := range allowed { + t.Run("allow "+c.method+" "+c.path, func(t *testing.T) { + next, called := okHandler() + w := httptest.NewRecorder() + gw.ecrOperationAuthorization(next).ServeHTTP(w, ecrAuthzRequest(c.method, c.path, ecrTestUserPrincipal())) + assert.True(t, *called) + assert.Equal(t, http.StatusOK, w.Code) + }) + } + + denied := []struct{ method, path string }{ + {http.MethodPost, "/v2/team/app/blobs/uploads/"}, + {http.MethodPatch, "/v2/team/app/blobs/uploads/upload-1"}, + {http.MethodPut, "/v2/team/app/blobs/uploads/upload-1"}, + {http.MethodDelete, "/v2/team/app/blobs/uploads/upload-1"}, + {http.MethodPut, "/v2/team/app/manifests/latest"}, + {http.MethodDelete, "/v2/team/app/manifests/sha256:abc"}, + } + for _, c := range denied { + t.Run("deny "+c.method+" "+c.path, func(t *testing.T) { + next, called := okHandler() + w := httptest.NewRecorder() + gw.ecrOperationAuthorization(next).ServeHTTP(w, ecrAuthzRequest(c.method, c.path, ecrTestUserPrincipal())) + assert.False(t, *called, "denied operation must never dispatch") + assert.Equal(t, http.StatusForbidden, w.Code) + }) + } +} + +// TestECROperationAuthorization_ReadOnlyMatrix pins the ReadOnly release-gate +// matrix: catalog, tags, blob and manifest reads succeed; every write/delete +// is denied. +func TestECROperationAuthorization_ReadOnlyMatrix(t *testing.T) { + repoARN := "arn:aws:ecr:" + ecrTestRegion + ":" + ecrAuthzTestAccount + ":repository/team/app" + wildcardARN := "arn:aws:ecr:" + ecrTestRegion + ":" + ecrAuthzTestAccount + ":repository/*" + readOnly := ecrAllowPolicy( + []string{ + "ecr:GetDownloadUrlForLayer", "ecr:BatchGetImage", "ecr:BatchCheckLayerAvailability", + "ecr:ListImages", "ecr:DescribeRepositories", + }, + []string{repoARN, wildcardARN}, + ) + gw, _ := ecrAuthzTestGateway(t, []handlers_iam.PolicyDocument{readOnly}) + + allowed := []struct{ method, path string }{ + {http.MethodGet, "/v2/_catalog"}, + {http.MethodGet, "/v2/team/app/tags/list"}, + {http.MethodHead, "/v2/team/app/blobs/sha256:abc"}, + {http.MethodGet, "/v2/team/app/manifests/latest"}, + } + for _, c := range allowed { + t.Run("allow "+c.method+" "+c.path, func(t *testing.T) { + next, called := okHandler() + w := httptest.NewRecorder() + gw.ecrOperationAuthorization(next).ServeHTTP(w, ecrAuthzRequest(c.method, c.path, ecrTestUserPrincipal())) + assert.True(t, *called) + assert.Equal(t, http.StatusOK, w.Code) + }) + } + + denied := []struct{ method, path string }{ + {http.MethodPost, "/v2/team/app/blobs/uploads/"}, + {http.MethodPut, "/v2/team/app/manifests/latest"}, + {http.MethodDelete, "/v2/team/app/manifests/sha256:abc"}, + } + for _, c := range denied { + t.Run("deny "+c.method+" "+c.path, func(t *testing.T) { + next, called := okHandler() + w := httptest.NewRecorder() + gw.ecrOperationAuthorization(next).ServeHTTP(w, ecrAuthzRequest(c.method, c.path, ecrTestUserPrincipal())) + assert.False(t, *called) + assert.Equal(t, http.StatusForbidden, w.Code) + }) + } +} + +// TestECROperationAuthorization_RepoScopedAllowDoesNotCrossRepos verifies an +// allow scoped to repository A's ARN does not authorize the identical action +// against repository B. +func TestECROperationAuthorization_RepoScopedAllowDoesNotCrossRepos(t *testing.T) { + repoAARN := "arn:aws:ecr:" + ecrTestRegion + ":" + ecrAuthzTestAccount + ":repository/team/a" + policy := ecrAllowPolicy([]string{"ecr:BatchGetImage"}, []string{repoAARN}) + gw, _ := ecrAuthzTestGateway(t, []handlers_iam.PolicyDocument{policy}) + + t.Run("repo A allowed", func(t *testing.T) { + next, called := okHandler() + w := httptest.NewRecorder() + gw.ecrOperationAuthorization(next).ServeHTTP(w, ecrAuthzRequest(http.MethodGet, "/v2/team/a/manifests/latest", ecrTestUserPrincipal())) + assert.True(t, *called) + assert.Equal(t, http.StatusOK, w.Code) + }) + + t.Run("repo B denied", func(t *testing.T) { + next, called := okHandler() + w := httptest.NewRecorder() + gw.ecrOperationAuthorization(next).ServeHTTP(w, ecrAuthzRequest(http.MethodGet, "/v2/team/b/manifests/latest", ecrTestUserPrincipal())) + assert.False(t, *called) + assert.Equal(t, http.StatusForbidden, w.Code) + }) +} + +// TestECROperationAuthorization_ExplicitDenyWins verifies an explicit Deny +// statement overrides a broader Allow for the same action/resource. +func TestECROperationAuthorization_ExplicitDenyWins(t *testing.T) { + repoARN := "arn:aws:ecr:" + ecrTestRegion + ":" + ecrAuthzTestAccount + ":repository/team/app" + allow := ecrAllowPolicy([]string{"ecr:BatchGetImage"}, []string{"arn:aws:ecr:" + ecrTestRegion + ":" + ecrAuthzTestAccount + ":repository/*"}) + deny := ecrDenyPolicy([]string{"ecr:BatchGetImage"}, []string{repoARN}) + gw, _ := ecrAuthzTestGateway(t, []handlers_iam.PolicyDocument{allow, deny}) + + next, called := okHandler() + w := httptest.NewRecorder() + gw.ecrOperationAuthorization(next).ServeHTTP(w, ecrAuthzRequest(http.MethodGet, "/v2/team/app/manifests/latest", ecrTestUserPrincipal())) + assert.False(t, *called) + assert.Equal(t, http.StatusForbidden, w.Code) +} + +// TestECROperationAuthorization_CrossRepoMount verifies a mount requires both +// destination write and source read permission. +func TestECROperationAuthorization_CrossRepoMount(t *testing.T) { + destARN := "arn:aws:ecr:" + ecrTestRegion + ":" + ecrAuthzTestAccount + ":repository/team/app" + sourceARN := "arn:aws:ecr:" + ecrTestRegion + ":" + ecrAuthzTestAccount + ":repository/team/base" + + t.Run("destination write only: denied, never dispatches", func(t *testing.T) { + gw, _ := ecrAuthzTestGateway(t, []handlers_iam.PolicyDocument{ + ecrAllowPolicy([]string{"ecr:InitiateLayerUpload"}, []string{destARN}), + }) + next, called := okHandler() + w := httptest.NewRecorder() + req := ecrAuthzRequest(http.MethodPost, "/v2/team/app/blobs/uploads/?mount=sha256:abc&from=team/base", ecrTestUserPrincipal()) + gw.ecrOperationAuthorization(next).ServeHTTP(w, req) + assert.False(t, *called) + assert.Equal(t, http.StatusForbidden, w.Code) + }) + + t.Run("destination write + source read: allowed", func(t *testing.T) { + gw, _ := ecrAuthzTestGateway(t, []handlers_iam.PolicyDocument{ + ecrAllowPolicy([]string{"ecr:InitiateLayerUpload"}, []string{destARN}), + ecrAllowPolicy([]string{"ecr:BatchCheckLayerAvailability"}, []string{sourceARN}), + }) + next, called := okHandler() + w := httptest.NewRecorder() + req := ecrAuthzRequest(http.MethodPost, "/v2/team/app/blobs/uploads/?mount=sha256:abc&from=team/base", ecrTestUserPrincipal()) + gw.ecrOperationAuthorization(next).ServeHTTP(w, req) + assert.True(t, *called) + assert.Equal(t, http.StatusOK, w.Code) + }) +} + +// TestECROperationAuthorization_UnclassifiedOperationNeverDispatches verifies +// a method/path combination Registry does not implement is refused before +// dispatch rather than silently forwarded. +func TestECROperationAuthorization_UnclassifiedOperationNeverDispatches(t *testing.T) { + gw, _ := ecrAuthzTestGateway(t, nil) + next, called := okHandler() + w := httptest.NewRecorder() + gw.ecrOperationAuthorization(next).ServeHTTP(w, ecrAuthzRequest(http.MethodPost, "/v2/", ecrTestUserPrincipal())) + assert.False(t, *called) + assert.Equal(t, http.StatusNotFound, w.Code) +} + +// TestECROperationAuthorization_MissingPrincipalUnauthorized verifies the +// middleware fails closed if no principal was stashed on the context (the +// bridge is disabled or was bypassed), rather than dispatching unauthenticated. +func TestECROperationAuthorization_MissingPrincipalUnauthorized(t *testing.T) { + gw, _ := ecrAuthzTestGateway(t, nil) + next, called := okHandler() + w := httptest.NewRecorder() + req := httptest.NewRequest(http.MethodGet, "/v2/team/app/manifests/latest", nil) + gw.ecrOperationAuthorization(next).ServeHTTP(w, req) + assert.False(t, *called) + assert.Equal(t, http.StatusUnauthorized, w.Code) +} + +// TestECROperationAuthorization_DependencyFailureNeverDispatches verifies an +// IAM backend outage fails closed with 503 rather than allowing or denying +// based on incomplete information. +func TestECROperationAuthorization_DependencyFailureNeverDispatches(t *testing.T) { + gw, iamSvc := ecrAuthzTestGateway(t, nil) + iamSvc.genericErr = errors.New(awserrors.ErrorServiceUnavailable) + + next, called := okHandler() + w := httptest.NewRecorder() + req := ecrAuthzRequest(http.MethodGet, "/v2/team/app/manifests/latest", ecrTestUserPrincipal()) + gw.ecrOperationAuthorization(next).ServeHTTP(w, req) + assert.False(t, *called) + assert.Equal(t, http.StatusServiceUnavailable, w.Code) +} + +// TestECROperationAuthorization_NilVerifierPassesThrough mirrors the bridge's +// own nil-verifier bypass, used by unit tests of unrelated routes. +func TestECROperationAuthorization_NilVerifierPassesThrough(t *testing.T) { + gw := &GatewayConfig{} + next, called := okHandler() + w := httptest.NewRecorder() + gw.ecrOperationAuthorization(next).ServeHTTP(w, httptest.NewRequest(http.MethodPost, "/v2/team/app/manifests/latest", nil)) + assert.True(t, *called) + assert.Equal(t, http.StatusOK, w.Code) +} + +// TestECROperationAuthorization_BasePathNoActionRequired verifies GET /v2/ +// dispatches once a principal is resolved, with no resource-specific decision. +func TestECROperationAuthorization_BasePathNoActionRequired(t *testing.T) { + gw, _ := ecrAuthzTestGateway(t, nil) + next, called := okHandler() + w := httptest.NewRecorder() + req := ecrAuthzRequest(http.MethodGet, "/v2/", ecrTestUserPrincipal()) + gw.ecrOperationAuthorization(next).ServeHTTP(w, req) + assert.True(t, *called) + assert.Equal(t, http.StatusOK, w.Code) +} diff --git a/spinifex/gateway/ecr_getauthorizationtoken.go b/spinifex/gateway/ecr_getauthorizationtoken.go index ef86e4011..0db8a41cb 100644 --- a/spinifex/gateway/ecr_getauthorizationtoken.go +++ b/spinifex/gateway/ecr_getauthorizationtoken.go @@ -28,11 +28,23 @@ func (gw *GatewayConfig) handleGetAuthorizationToken(w http.ResponseWriter, r *h return errors.New(awserrors.ErrorServerInternal) } accessKey, _ := ctx.Value(ctxAccessKey).(string) + identity, _ := ctx.Value(ctxIdentity).(string) principalType, _ := ctx.Value(ctxPrincipalType).(string) + assumedRoleARN, _ := ctx.Value(ctxAssumedRoleARN).(string) + + // The minted token names the exact IAM/STS record every /v2/* request will + // later rehydrate against, so its subject must be the same canonical ARN + // buildCallerARN produces everywhere else in the gateway (STS included) — + // not a best-effort approximation that could omit an IAM path. + callerARN, err := buildCallerARN(accountID, identity, principalType, assumedRoleARN) + if err != nil { + slog.Error("GetAuthorizationToken: cannot build canonical caller ARN", "err", err) + return errors.New(awserrors.ErrorServerInternal) + } token, expiresAt, err := gw.ECRTokenIssuer.Mint(gateway_ecrauth.Principal{ AccountID: accountID, - ARN: eksCallerPrincipalARN(r), + ARN: callerARN, Type: principalType, AccessKeyID: accessKey, }) diff --git a/spinifex/gateway/ecr_getauthorizationtoken_test.go b/spinifex/gateway/ecr_getauthorizationtoken_test.go index f0dff1cd3..fd6a8dfc9 100644 --- a/spinifex/gateway/ecr_getauthorizationtoken_test.go +++ b/spinifex/gateway/ecr_getauthorizationtoken_test.go @@ -23,6 +23,8 @@ func TestHandleGetAuthorizationToken_MintsUsableToken(t *testing.T) { req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader("{}")) ctx := context.WithValue(req.Context(), ctxAccountID, ecrTestAccount) ctx = context.WithValue(ctx, ctxPrincipalType, principalTypeUser) + ctx = context.WithValue(ctx, ctxIdentity, "dev") + ctx = context.WithValue(ctx, ctxAccessKey, "AKIAGETAUTHTOKENTEST1") w := httptest.NewRecorder() require.NoError(t, gw.handleGetAuthorizationToken(w, req.WithContext(ctx))) @@ -64,6 +66,8 @@ func TestHandleGetAuthorizationToken_ProxyEndpointCarriesPort(t *testing.T) { req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader("{}")) ctx := context.WithValue(req.Context(), ctxAccountID, ecrTestAccount) ctx = context.WithValue(ctx, ctxPrincipalType, principalTypeUser) + ctx = context.WithValue(ctx, ctxIdentity, "dev") + ctx = context.WithValue(ctx, ctxAccessKey, "AKIAGETAUTHTOKENTEST1") w := httptest.NewRecorder() require.NoError(t, gw.handleGetAuthorizationToken(w, req.WithContext(ctx))) diff --git a/spinifex/gateway/ecr_principal.go b/spinifex/gateway/ecr_principal.go new file mode 100644 index 000000000..780a2e19b --- /dev/null +++ b/spinifex/gateway/ecr_principal.go @@ -0,0 +1,258 @@ +package gateway + +import ( + "errors" + "fmt" + "strings" + "time" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/iam" + "github.com/mulgadc/predastore/auth" + "github.com/mulgadc/spinifex/spinifex/awserrors" + gateway_ecrauth "github.com/mulgadc/spinifex/spinifex/gateway/ecrauth" + handlers_iam "github.com/mulgadc/spinifex/spinifex/handlers/iam" +) + +// An ECR registry token is a signed pointer to an IAM/STS record, not a +// permission grant: possession of a verified, unexpired JWT only proves the +// bearer once authenticated as the access key named in its claims. Every +// /v2/* request must resolve that access key against the *current* IAM/STS +// state before dispatch, so a key deactivation, session expiry/deletion, user +// or role deletion, account suspension, or policy change takes effect on the +// very next request — including one carrying an already-issued 12h token. +// +// resolveECRPrincipal performs that rehydration and returns the same +// principalContext the SigV4 middleware builds, so ECR requests are +// authorized by the identical policy-evaluation path as every other AWS +// action. + +// ecrPrincipalError classifies a principal-rehydration failure so the HTTP +// layer can fail closed with the right status: an invalid/revoked/mismatched +// identity is client-visible (401, with the registry's re-auth challenge), +// while a lookup that could not complete is an operator-visible dependency +// failure (503) that must never be treated as an allow. +type ecrPrincipalError struct { + dependency bool + err error +} + +func (e *ecrPrincipalError) Error() string { return e.err.Error() } +func (e *ecrPrincipalError) Unwrap() error { return e.err } + +// ecrInvalidPrincipal wraps a reason the presented identity pointer is no +// longer valid (revoked, expired, deleted, or inconsistent with the signed claims). +func ecrInvalidPrincipal(format string, args ...any) error { + return &ecrPrincipalError{err: fmt.Errorf(format, args...)} +} + +// ecrDependencyFailure wraps a reason the identity pointer could not be +// checked at all (IAM/STS/NATS unavailable). Never treat this as an allow. +func ecrDependencyFailure(format string, args ...any) error { + return &ecrPrincipalError{dependency: true, err: fmt.Errorf(format, args...)} +} + +// isECRDependencyFailure reports whether err (from resolveECRPrincipal or a +// caller further down the ECR chain) represents a backend outage rather than +// an invalid identity, so the HTTP layer can choose 503 over 401. +func isECRDependencyFailure(err error) bool { + var pe *ecrPrincipalError + return errors.As(err, &pe) && pe.dependency +} + +// classifyIAMLookupErr maps an IAMService lookup error to the right +// ecrPrincipalError kind: a NoSuchEntity-shaped error means the referenced +// record is gone (invalid, 401); anything else is a dependency failure (503), +// since it says nothing about whether the identity is still valid. +func classifyIAMLookupErr(err error, what string) error { + if strings.Contains(err.Error(), awserrors.ErrorIAMNoSuchEntity) { + return ecrInvalidPrincipal("%s not found: %w", what, err) + } + return ecrDependencyFailure("%s lookup failed: %w", what, err) +} + +// resolveECRPrincipal rehydrates claims (already ES256/expiry/issuer/audience +// verified by gateway_ecrauth.Verifier) against current IAM/STS state and +// returns the equivalent of a fresh SigV4 authentication for the same +// identity. It never trusts claims.PrincipalType or claims.Subject on their +// own — both are cross-checked against the freshly resolved record before +// being accepted, so a forged or stale claim cannot widen access beyond what +// the current IAM/STS state actually grants. +func (gw *GatewayConfig) resolveECRPrincipal(claims *gateway_ecrauth.Claims) (principalContext, error) { + switch { + case strings.HasPrefix(claims.AccessKeyID, longLivedAKIDPrefix): + return gw.resolveECRLongLivedPrincipal(claims) + case strings.HasPrefix(claims.AccessKeyID, sessionAKIDPrefix): + return gw.resolveECRSessionPrincipal(claims) + default: + return principalContext{}, ecrInvalidPrincipal("unrecognized access key prefix") + } +} + +// resolveECRLongLivedPrincipal rehydrates an AKIA (IAM user) claim: the +// access key must still be active and belong to the claimed account, the +// user it names must still exist, the account must still be active, and the +// canonical ARN/principalType computed from that current state must match +// what was signed into the token. +func (gw *GatewayConfig) resolveECRLongLivedPrincipal(claims *gateway_ecrauth.Claims) (principalContext, error) { + if gw.IAMService == nil { + return principalContext{}, ecrDependencyFailure("IAM service not available") + } + + ak, err := gw.IAMService.LookupAccessKey(claims.AccessKeyID) + if err != nil { + return principalContext{}, classifyIAMLookupErr(err, "access key") + } + if ak.Status != handlers_iam.AccessKeyStatusActive { + return principalContext{}, ecrInvalidPrincipal("access key %q is inactive", claims.AccessKeyID) + } + if ak.AccountID != claims.AccountID { + return principalContext{}, ecrInvalidPrincipal("access key account does not match token account") + } + + if err := gw.requireActiveECRAccount(ak.AccountID); err != nil { + return principalContext{}, err + } + + userOut, err := gw.IAMService.GetUser(ak.AccountID, &iam.GetUserInput{UserName: aws.String(ak.UserName)}) + if err != nil { + return principalContext{}, classifyIAMLookupErr(err, "user") + } + identity := aws.StringValue(userOut.User.UserName) + + if claims.PrincipalType != principalTypeUser { + return principalContext{}, ecrInvalidPrincipal("principalType claim %q does not match resolved user", claims.PrincipalType) + } + canonicalARN, err := buildCallerARN(ak.AccountID, identity, principalTypeUser, "") + if err != nil { + return principalContext{}, ecrInvalidPrincipal("cannot build canonical ARN: %w", err) + } + if canonicalARN != claims.Subject { + return principalContext{}, ecrInvalidPrincipal("subject claim does not match canonical principal ARN") + } + + return principalContext{ + identity: identity, + accountID: ak.AccountID, + principalType: principalTypeUser, + }, nil +} + +// resolveECRSessionPrincipal rehydrates an ASIA (STS session) claim. A +// GetSessionToken user session resolves like resolveECRLongLivedPrincipal's +// user branch; an assumed-role session additionally re-resolves the +// underlying role and rejects the session if the role was deleted and +// recreated (its RoleID changed) even though the role name and ARN are +// unchanged. +func (gw *GatewayConfig) resolveECRSessionPrincipal(claims *gateway_ecrauth.Claims) (principalContext, error) { + if gw.STSService == nil { + return principalContext{}, ecrDependencyFailure("STS service not available") + } + + cred, err := gw.STSService.LookupSessionCredential(claims.AccessKeyID) + if err != nil { + return principalContext{}, ecrDependencyFailure("session lookup failed: %w", err) + } + if cred == nil { + return principalContext{}, ecrInvalidPrincipal("session %q not found", claims.AccessKeyID) + } + if time.Now().UTC().After(cred.ExpiresAt) { + return principalContext{}, ecrInvalidPrincipal("session %q expired", claims.AccessKeyID) + } + if cred.AccountID != claims.AccountID { + return principalContext{}, ecrInvalidPrincipal("session account does not match token account") + } + + if err := gw.requireActiveECRAccount(cred.AccountID); err != nil { + return principalContext{}, err + } + + if gw.IAMService == nil { + return principalContext{}, ecrDependencyFailure("IAM service not available") + } + + if cred.PrincipalType == principalTypeUser { + // GetSessionToken: the session resolves to the same user identity as a + // long-lived key, so it is authorized as that user. + userOut, err := gw.IAMService.GetUser(cred.AccountID, &iam.GetUserInput{UserName: aws.String(cred.SessionName)}) + if err != nil { + return principalContext{}, classifyIAMLookupErr(err, "user") + } + identity := aws.StringValue(userOut.User.UserName) + + if claims.PrincipalType != principalTypeUser { + return principalContext{}, ecrInvalidPrincipal("principalType claim %q does not match resolved session", claims.PrincipalType) + } + canonicalARN, err := buildCallerARN(cred.AccountID, identity, principalTypeUser, "") + if err != nil { + return principalContext{}, ecrInvalidPrincipal("cannot build canonical ARN: %w", err) + } + if canonicalARN != claims.Subject { + return principalContext{}, ecrInvalidPrincipal("subject claim does not match canonical principal ARN") + } + + return principalContext{ + identity: identity, + accountID: cred.AccountID, + principalType: principalTypeUser, + }, nil + } + + // Assumed-role, or a legacy record whose empty PrincipalType predates the + // field — both mean "assumed-role" (mirrors resolveSessionAKID). + if claims.PrincipalType != principalTypeAssumedRole { + return principalContext{}, ecrInvalidPrincipal("principalType claim %q does not match resolved session", claims.PrincipalType) + } + + // Resolve by the session's underlying role, never by SessionName — the + // caller controls RoleSessionName at AssumeRole time, so trusting it here + // would let an attacker rename their way into another role's ARN shape. + roleAcct, roleName, perr := auth.ParseRoleARN(cred.UnderlyingRoleARN) + if perr != nil || roleAcct != cred.AccountID { + return principalContext{}, ecrInvalidPrincipal("session underlying role ARN is unresolvable or cross-account: %w", perr) + } + roleOut, err := gw.IAMService.GetRole(cred.AccountID, &iam.GetRoleInput{RoleName: aws.String(roleName)}) + if err != nil { + return principalContext{}, classifyIAMLookupErr(err, "role") + } + currentRoleARN := aws.StringValue(roleOut.Role.Arn) + currentRoleID := aws.StringValue(roleOut.Role.RoleId) + if currentRoleARN != cred.UnderlyingRoleARN { + return principalContext{}, ecrInvalidPrincipal("role ARN has changed since the session was minted") + } + if cred.RoleID != "" && currentRoleID != cred.RoleID { + // The role name was deleted and recreated: same ARN, different identity. + return principalContext{}, ecrInvalidPrincipal("role was replaced since the session was minted") + } + + canonicalARN, err := buildCallerARN(cred.AccountID, cred.SessionName, principalTypeAssumedRole, cred.AssumedRoleARN) + if err != nil { + return principalContext{}, ecrInvalidPrincipal("cannot build canonical ARN: %w", err) + } + if canonicalARN != claims.Subject { + return principalContext{}, ecrInvalidPrincipal("subject claim does not match canonical principal ARN") + } + + return principalContext{ + identity: cred.SessionName, + accountID: cred.AccountID, + principalType: principalTypeAssumedRole, + assumedRoleARN: cred.AssumedRoleARN, + assumedRoleID: cred.AssumedRoleID, + underlyingRoleARN: cred.UnderlyingRoleARN, + }, nil +} + +// requireActiveECRAccount fails closed unless accountID resolves to an +// active account, so a suspended account's already-issued tokens stop +// working immediately rather than at their natural 12h expiry. +func (gw *GatewayConfig) requireActiveECRAccount(accountID string) error { + account, err := gw.IAMService.GetAccount(accountID) + if err != nil { + return classifyIAMLookupErr(err, "account") + } + if account.Status != handlers_iam.AccountStatusActive { + return ecrInvalidPrincipal("account %q is not active", accountID) + } + return nil +} diff --git a/spinifex/gateway/ecr_principal_test.go b/spinifex/gateway/ecr_principal_test.go new file mode 100644 index 000000000..d6b809888 --- /dev/null +++ b/spinifex/gateway/ecr_principal_test.go @@ -0,0 +1,492 @@ +package gateway + +import ( + "errors" + "testing" + "time" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/iam" + "github.com/mulgadc/spinifex/spinifex/awserrors" + gateway_ecrauth "github.com/mulgadc/spinifex/spinifex/gateway/ecrauth" + handlers_iam "github.com/mulgadc/spinifex/spinifex/handlers/iam" + handlers_sts "github.com/mulgadc/spinifex/spinifex/handlers/sts" + "github.com/mulgadc/spinifex/spinifex/utils" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// ecrMockIAMService implements handlers_iam.IAMService for ECR principal +// rehydration tests. Only the methods resolveECRPrincipal calls are wired; +// any other method nil-panics on the embedded nil interface, surfacing an +// unexpected call rather than silently returning a zero value. +type ecrMockIAMService struct { + handlers_iam.IAMService + + accessKeys map[string]*handlers_iam.AccessKey + users map[string]*iam.User + roles map[string]*iam.Role + accounts map[string]*handlers_iam.Account + userPolicies map[string][]handlers_iam.PolicyDocument + rolePolicies map[string][]handlers_iam.PolicyDocument + + // genericErr, when set, is returned by LookupAccessKey/GetUser/GetRole/ + // GetAccount/GetUserPolicies/GetRolePolicies instead of the usual + // not-found error, simulating an IAM backend outage rather than a + // missing record. + genericErr error +} + +func newECRMockIAMService() *ecrMockIAMService { + return &ecrMockIAMService{ + accessKeys: map[string]*handlers_iam.AccessKey{}, + users: map[string]*iam.User{}, + roles: map[string]*iam.Role{}, + accounts: map[string]*handlers_iam.Account{}, + userPolicies: map[string][]handlers_iam.PolicyDocument{}, + rolePolicies: map[string][]handlers_iam.PolicyDocument{}, + } +} + +func (m *ecrMockIAMService) LookupAccessKey(accessKeyID string) (*handlers_iam.AccessKey, error) { + if m.genericErr != nil { + return nil, m.genericErr + } + ak, ok := m.accessKeys[accessKeyID] + if !ok { + return nil, errors.New(awserrors.ErrorIAMNoSuchEntity) + } + return ak, nil +} + +func (m *ecrMockIAMService) GetUser(accountID string, input *iam.GetUserInput) (*iam.GetUserOutput, error) { + if m.genericErr != nil { + return nil, m.genericErr + } + u, ok := m.users[accountID+"|"+aws.StringValue(input.UserName)] + if !ok { + return nil, errors.New(awserrors.ErrorIAMNoSuchEntity) + } + return &iam.GetUserOutput{User: u}, nil +} + +func (m *ecrMockIAMService) GetRole(accountID string, input *iam.GetRoleInput) (*iam.GetRoleOutput, error) { + if m.genericErr != nil { + return nil, m.genericErr + } + r, ok := m.roles[accountID+"|"+aws.StringValue(input.RoleName)] + if !ok { + return nil, errors.New(awserrors.ErrorIAMNoSuchEntity) + } + return &iam.GetRoleOutput{Role: r}, nil +} + +func (m *ecrMockIAMService) GetAccount(accountID string) (*handlers_iam.Account, error) { + if m.genericErr != nil { + return nil, m.genericErr + } + a, ok := m.accounts[accountID] + if !ok { + return nil, errors.New(awserrors.ErrorIAMNoSuchEntity) + } + return a, nil +} + +func (m *ecrMockIAMService) GetUserPolicies(accountID, userName string) ([]handlers_iam.PolicyDocument, error) { + if m.genericErr != nil { + return nil, m.genericErr + } + return m.userPolicies[accountID+"|"+userName], nil +} + +func (m *ecrMockIAMService) GetRolePolicies(accountID, roleName string) ([]handlers_iam.PolicyDocument, error) { + if m.genericErr != nil { + return nil, m.genericErr + } + return m.rolePolicies[accountID+"|"+roleName], nil +} + +// ecrMockSTSService implements handlers_sts.STSService for ECR principal +// rehydration tests. Only LookupSessionCredential is wired. +type ecrMockSTSService struct { + handlers_sts.STSService + + sessions map[string]*handlers_sts.SessionCredential +} + +func newECRMockSTSService() *ecrMockSTSService { + return &ecrMockSTSService{sessions: map[string]*handlers_sts.SessionCredential{}} +} + +func (m *ecrMockSTSService) LookupSessionCredential(accessKeyID string) (*handlers_sts.SessionCredential, error) { + return m.sessions[accessKeyID], nil +} + +const ( + ecrPrincipalTestAccount = "000000000042" + ecrPrincipalTestAKID = "AKIATESTLONGLIVED0001" + ecrPrincipalTestASID = "ASIATESTSESSION000001" +) + +func seedECRTestUser(iamSvc *ecrMockIAMService, accountID, userName, akid string) { + iamSvc.accessKeys[akid] = &handlers_iam.AccessKey{ + AccessKeyID: akid, + UserName: userName, + AccountID: accountID, + Status: handlers_iam.AccessKeyStatusActive, + } + iamSvc.users[accountID+"|"+userName] = &iam.User{ + UserName: aws.String(userName), + Arn: aws.String("arn:aws:iam::" + accountID + ":user/" + userName), + } + iamSvc.accounts[accountID] = &handlers_iam.Account{AccountID: accountID, Status: handlers_iam.AccountStatusActive} +} + +func TestResolveECRPrincipal_LongLivedUser(t *testing.T) { + iamSvc := newECRMockIAMService() + seedECRTestUser(iamSvc, ecrPrincipalTestAccount, "dev", ecrPrincipalTestAKID) + gw := &GatewayConfig{IAMService: iamSvc} + + claims := &gateway_ecrauth.Claims{ + AccountID: ecrPrincipalTestAccount, + PrincipalType: principalTypeUser, + AccessKeyID: ecrPrincipalTestAKID, + } + claims.Subject = "arn:aws:iam::" + ecrPrincipalTestAccount + ":user/dev" + + got, err := gw.resolveECRPrincipal(claims) + require.NoError(t, err) + assert.Equal(t, principalContext{identity: "dev", accountID: ecrPrincipalTestAccount, principalType: principalTypeUser}, got) +} + +func TestResolveECRPrincipal_GlobalRoot(t *testing.T) { + iamSvc := newECRMockIAMService() + seedECRTestUser(iamSvc, utils.GlobalAccountID, "root", ecrPrincipalTestAKID) + gw := &GatewayConfig{IAMService: iamSvc} + + claims := &gateway_ecrauth.Claims{ + AccountID: utils.GlobalAccountID, + PrincipalType: principalTypeUser, + AccessKeyID: ecrPrincipalTestAKID, + } + claims.Subject = "arn:aws:iam::" + utils.GlobalAccountID + ":root" + + got, err := gw.resolveECRPrincipal(claims) + require.NoError(t, err) + assert.Equal(t, principalTypeUser, got.principalType) + assert.Equal(t, "root", got.identity) +} + +func TestResolveECRPrincipal_LongLivedUser_Rejections(t *testing.T) { + validClaims := func() *gateway_ecrauth.Claims { + c := &gateway_ecrauth.Claims{ + AccountID: ecrPrincipalTestAccount, + PrincipalType: principalTypeUser, + AccessKeyID: ecrPrincipalTestAKID, + } + c.Subject = "arn:aws:iam::" + ecrPrincipalTestAccount + ":user/dev" + return c + } + + t.Run("unknown access key", func(t *testing.T) { + iamSvc := newECRMockIAMService() + gw := &GatewayConfig{IAMService: iamSvc} + _, err := gw.resolveECRPrincipal(validClaims()) + require.Error(t, err) + assert.False(t, isECRDependencyFailure(err)) + }) + + t.Run("inactive access key", func(t *testing.T) { + iamSvc := newECRMockIAMService() + seedECRTestUser(iamSvc, ecrPrincipalTestAccount, "dev", ecrPrincipalTestAKID) + iamSvc.accessKeys[ecrPrincipalTestAKID].Status = "Inactive" + gw := &GatewayConfig{IAMService: iamSvc} + _, err := gw.resolveECRPrincipal(validClaims()) + require.Error(t, err) + assert.False(t, isECRDependencyFailure(err)) + }) + + t.Run("access key account does not match claim account", func(t *testing.T) { + iamSvc := newECRMockIAMService() + seedECRTestUser(iamSvc, ecrPrincipalTestAccount, "dev", ecrPrincipalTestAKID) + iamSvc.accessKeys[ecrPrincipalTestAKID].AccountID = "000000000099" + gw := &GatewayConfig{IAMService: iamSvc} + _, err := gw.resolveECRPrincipal(validClaims()) + require.Error(t, err) + }) + + t.Run("deleted user", func(t *testing.T) { + iamSvc := newECRMockIAMService() + seedECRTestUser(iamSvc, ecrPrincipalTestAccount, "dev", ecrPrincipalTestAKID) + delete(iamSvc.users, ecrPrincipalTestAccount+"|dev") + gw := &GatewayConfig{IAMService: iamSvc} + _, err := gw.resolveECRPrincipal(validClaims()) + require.Error(t, err) + assert.False(t, isECRDependencyFailure(err)) + }) + + t.Run("suspended account", func(t *testing.T) { + iamSvc := newECRMockIAMService() + seedECRTestUser(iamSvc, ecrPrincipalTestAccount, "dev", ecrPrincipalTestAKID) + iamSvc.accounts[ecrPrincipalTestAccount].Status = handlers_iam.AccountStatusSuspended + gw := &GatewayConfig{IAMService: iamSvc} + _, err := gw.resolveECRPrincipal(validClaims()) + require.Error(t, err) + assert.False(t, isECRDependencyFailure(err)) + }) + + t.Run("principalType claim mismatch", func(t *testing.T) { + iamSvc := newECRMockIAMService() + seedECRTestUser(iamSvc, ecrPrincipalTestAccount, "dev", ecrPrincipalTestAKID) + gw := &GatewayConfig{IAMService: iamSvc} + c := validClaims() + c.PrincipalType = principalTypeAssumedRole + _, err := gw.resolveECRPrincipal(c) + require.Error(t, err) + }) + + t.Run("subject does not match canonical ARN", func(t *testing.T) { + iamSvc := newECRMockIAMService() + seedECRTestUser(iamSvc, ecrPrincipalTestAccount, "dev", ecrPrincipalTestAKID) + gw := &GatewayConfig{IAMService: iamSvc} + c := validClaims() + c.Subject = "arn:aws:iam::" + ecrPrincipalTestAccount + ":user/someone-else" + _, err := gw.resolveECRPrincipal(c) + require.Error(t, err) + }) + + t.Run("nil IAM service is a dependency failure", func(t *testing.T) { + gw := &GatewayConfig{} + _, err := gw.resolveECRPrincipal(validClaims()) + require.Error(t, err) + assert.True(t, isECRDependencyFailure(err)) + }) + + t.Run("IAM backend outage is a dependency failure, not a denial", func(t *testing.T) { + iamSvc := newECRMockIAMService() + iamSvc.genericErr = errors.New("nats: no responders") + gw := &GatewayConfig{IAMService: iamSvc} + _, err := gw.resolveECRPrincipal(validClaims()) + require.Error(t, err) + assert.True(t, isECRDependencyFailure(err)) + }) +} + +func seedECRSessionUser(iamSvc *ecrMockIAMService, stsSvc *ecrMockSTSService, accountID, userName, asid string, expiresAt time.Time) { + iamSvc.users[accountID+"|"+userName] = &iam.User{ + UserName: aws.String(userName), + Arn: aws.String("arn:aws:iam::" + accountID + ":user/" + userName), + } + iamSvc.accounts[accountID] = &handlers_iam.Account{AccountID: accountID, Status: handlers_iam.AccountStatusActive} + stsSvc.sessions[asid] = &handlers_sts.SessionCredential{ + AccessKeyID: asid, + AccountID: accountID, + PrincipalType: principalTypeUser, + SessionName: userName, + ExpiresAt: expiresAt, + } +} + +func TestResolveECRPrincipal_SessionToken_User(t *testing.T) { + iamSvc := newECRMockIAMService() + stsSvc := newECRMockSTSService() + seedECRSessionUser(iamSvc, stsSvc, ecrPrincipalTestAccount, "dev", ecrPrincipalTestASID, time.Now().Add(time.Hour)) + gw := &GatewayConfig{IAMService: iamSvc, STSService: stsSvc} + + claims := &gateway_ecrauth.Claims{ + AccountID: ecrPrincipalTestAccount, + PrincipalType: principalTypeUser, + AccessKeyID: ecrPrincipalTestASID, + } + claims.Subject = "arn:aws:iam::" + ecrPrincipalTestAccount + ":user/dev" + + got, err := gw.resolveECRPrincipal(claims) + require.NoError(t, err) + assert.Equal(t, principalContext{identity: "dev", accountID: ecrPrincipalTestAccount, principalType: principalTypeUser}, got) +} + +func TestResolveECRPrincipal_SessionToken_Expired(t *testing.T) { + iamSvc := newECRMockIAMService() + stsSvc := newECRMockSTSService() + seedECRSessionUser(iamSvc, stsSvc, ecrPrincipalTestAccount, "dev", ecrPrincipalTestASID, time.Now().Add(-time.Hour)) + gw := &GatewayConfig{IAMService: iamSvc, STSService: stsSvc} + + claims := &gateway_ecrauth.Claims{ + AccountID: ecrPrincipalTestAccount, + PrincipalType: principalTypeUser, + AccessKeyID: ecrPrincipalTestASID, + } + claims.Subject = "arn:aws:iam::" + ecrPrincipalTestAccount + ":user/dev" + + _, err := gw.resolveECRPrincipal(claims) + require.Error(t, err) + assert.False(t, isECRDependencyFailure(err)) +} + +func TestResolveECRPrincipal_SessionToken_UnknownSession(t *testing.T) { + gw := &GatewayConfig{IAMService: newECRMockIAMService(), STSService: newECRMockSTSService()} + claims := &gateway_ecrauth.Claims{ + AccountID: ecrPrincipalTestAccount, + PrincipalType: principalTypeAssumedRole, + AccessKeyID: ecrPrincipalTestASID, + } + claims.Subject = "arn:aws:iam::" + ecrPrincipalTestAccount + ":assumed-role/deploy/session" + _, err := gw.resolveECRPrincipal(claims) + require.Error(t, err) + assert.False(t, isECRDependencyFailure(err)) +} + +func seedECRAssumedRole(iamSvc *ecrMockIAMService, stsSvc *ecrMockSTSService, accountID, roleName, roleID, asid, sessionName string, expiresAt time.Time) string { + roleARN := "arn:aws:iam::" + accountID + ":role/" + roleName + iamSvc.roles[accountID+"|"+roleName] = &iam.Role{ + RoleName: aws.String(roleName), + Arn: aws.String(roleARN), + RoleId: aws.String(roleID), + } + iamSvc.accounts[accountID] = &handlers_iam.Account{AccountID: accountID, Status: handlers_iam.AccountStatusActive} + // AssumeRole session ARNs use the "assumed-role" resource type; the + // underlying role (resolved via GetRole) stays a role/ ARN, matching the + // production shape. + assumedRoleARN := "arn:aws:sts::" + accountID + ":assumed-role/" + roleName + "/" + sessionName + stsSvc.sessions[asid] = &handlers_sts.SessionCredential{ + AccessKeyID: asid, + AccountID: accountID, + PrincipalType: principalTypeAssumedRole, + SessionName: sessionName, + AssumedRoleARN: assumedRoleARN, + UnderlyingRoleARN: roleARN, + RoleID: roleID, + ExpiresAt: expiresAt, + } + return assumedRoleARN +} + +func TestResolveECRPrincipal_AssumedRole(t *testing.T) { + iamSvc := newECRMockIAMService() + stsSvc := newECRMockSTSService() + assumedARN := seedECRAssumedRole(iamSvc, stsSvc, ecrPrincipalTestAccount, "deploy", "AROATESTROLE0001", ecrPrincipalTestASID, "session-1", time.Now().Add(time.Hour)) + gw := &GatewayConfig{IAMService: iamSvc, STSService: stsSvc} + + claims := &gateway_ecrauth.Claims{ + AccountID: ecrPrincipalTestAccount, + PrincipalType: principalTypeAssumedRole, + AccessKeyID: ecrPrincipalTestASID, + } + claims.Subject = assumedARN + + got, err := gw.resolveECRPrincipal(claims) + require.NoError(t, err) + assert.Equal(t, principalTypeAssumedRole, got.principalType) + assert.Equal(t, "session-1", got.identity) + assert.Equal(t, assumedARN, got.assumedRoleARN) + assert.Equal(t, "arn:aws:iam::"+ecrPrincipalTestAccount+":role/deploy", got.underlyingRoleARN) +} + +func TestResolveECRPrincipal_AssumedRole_LegacyEmptyPrincipalType(t *testing.T) { + iamSvc := newECRMockIAMService() + stsSvc := newECRMockSTSService() + assumedARN := seedECRAssumedRole(iamSvc, stsSvc, ecrPrincipalTestAccount, "deploy", "AROATESTROLE0001", ecrPrincipalTestASID, "session-1", time.Now().Add(time.Hour)) + stsSvc.sessions[ecrPrincipalTestASID].PrincipalType = "" // pre-PrincipalType legacy record + gw := &GatewayConfig{IAMService: iamSvc, STSService: stsSvc} + + claims := &gateway_ecrauth.Claims{ + AccountID: ecrPrincipalTestAccount, + PrincipalType: principalTypeAssumedRole, + AccessKeyID: ecrPrincipalTestASID, + } + claims.Subject = assumedARN + + got, err := gw.resolveECRPrincipal(claims) + require.NoError(t, err) + assert.Equal(t, principalTypeAssumedRole, got.principalType) +} + +func TestResolveECRPrincipal_AssumedRole_Rejections(t *testing.T) { + baseClaims := func(assumedARN string) *gateway_ecrauth.Claims { + c := &gateway_ecrauth.Claims{ + AccountID: ecrPrincipalTestAccount, + PrincipalType: principalTypeAssumedRole, + AccessKeyID: ecrPrincipalTestASID, + } + c.Subject = assumedARN + return c + } + + t.Run("role deleted", func(t *testing.T) { + iamSvc := newECRMockIAMService() + stsSvc := newECRMockSTSService() + assumedARN := seedECRAssumedRole(iamSvc, stsSvc, ecrPrincipalTestAccount, "deploy", "AROATESTROLE0001", ecrPrincipalTestASID, "session-1", time.Now().Add(time.Hour)) + delete(iamSvc.roles, ecrPrincipalTestAccount+"|deploy") + gw := &GatewayConfig{IAMService: iamSvc, STSService: stsSvc} + _, err := gw.resolveECRPrincipal(baseClaims(assumedARN)) + require.Error(t, err) + assert.False(t, isECRDependencyFailure(err)) + }) + + t.Run("role replaced (RoleId changed, ARN unchanged)", func(t *testing.T) { + iamSvc := newECRMockIAMService() + stsSvc := newECRMockSTSService() + assumedARN := seedECRAssumedRole(iamSvc, stsSvc, ecrPrincipalTestAccount, "deploy", "AROATESTROLE0001", ecrPrincipalTestASID, "session-1", time.Now().Add(time.Hour)) + iamSvc.roles[ecrPrincipalTestAccount+"|deploy"].RoleId = aws.String("AROADIFFERENTROLE002") + gw := &GatewayConfig{IAMService: iamSvc, STSService: stsSvc} + _, err := gw.resolveECRPrincipal(baseClaims(assumedARN)) + require.Error(t, err) + assert.False(t, isECRDependencyFailure(err)) + }) + + t.Run("expired session", func(t *testing.T) { + iamSvc := newECRMockIAMService() + stsSvc := newECRMockSTSService() + assumedARN := seedECRAssumedRole(iamSvc, stsSvc, ecrPrincipalTestAccount, "deploy", "AROATESTROLE0001", ecrPrincipalTestASID, "session-1", time.Now().Add(-time.Minute)) + gw := &GatewayConfig{IAMService: iamSvc, STSService: stsSvc} + _, err := gw.resolveECRPrincipal(baseClaims(assumedARN)) + require.Error(t, err) + }) + + t.Run("attacker-controlled session name cannot rename into another role's ARN shape", func(t *testing.T) { + // resolveECRPrincipal must derive the ARN from the stored underlying + // role, never trust an attacker-influenced session name for anything + // beyond the ARN's trailing session-name segment. + iamSvc := newECRMockIAMService() + stsSvc := newECRMockSTSService() + assumedARN := seedECRAssumedRole(iamSvc, stsSvc, ecrPrincipalTestAccount, "deploy", "AROATESTROLE0001", ecrPrincipalTestASID, "session-1", time.Now().Add(time.Hour)) + gw := &GatewayConfig{IAMService: iamSvc, STSService: stsSvc} + + claims := baseClaims(assumedARN) + // Subject claims a completely different role than the session's stored + // underlying role — must be rejected even though PrincipalType matches. + claims.Subject = "arn:aws:sts::" + ecrPrincipalTestAccount + ":assumed-role/other-role/session-1" + _, err := gw.resolveECRPrincipal(claims) + require.Error(t, err) + }) + + t.Run("cross-account underlying role ARN rejected", func(t *testing.T) { + iamSvc := newECRMockIAMService() + stsSvc := newECRMockSTSService() + assumedARN := seedECRAssumedRole(iamSvc, stsSvc, ecrPrincipalTestAccount, "deploy", "AROATESTROLE0001", ecrPrincipalTestASID, "session-1", time.Now().Add(time.Hour)) + stsSvc.sessions[ecrPrincipalTestASID].UnderlyingRoleARN = "arn:aws:iam::000000000099:role/deploy" + gw := &GatewayConfig{IAMService: iamSvc, STSService: stsSvc} + _, err := gw.resolveECRPrincipal(baseClaims(assumedARN)) + require.Error(t, err) + }) + + t.Run("nil STS service is a dependency failure", func(t *testing.T) { + gw := &GatewayConfig{IAMService: newECRMockIAMService()} + _, err := gw.resolveECRPrincipal(baseClaims("arn:aws:sts::" + ecrPrincipalTestAccount + ":assumed-role/deploy/session-1")) + require.Error(t, err) + assert.True(t, isECRDependencyFailure(err)) + }) +} + +func TestResolveECRPrincipal_UnrecognizedAccessKeyPrefix(t *testing.T) { + gw := &GatewayConfig{IAMService: newECRMockIAMService()} + claims := &gateway_ecrauth.Claims{ + AccountID: ecrPrincipalTestAccount, + PrincipalType: principalTypeUser, + AccessKeyID: "NOTAVALIDPREFIX", + } + claims.Subject = "arn:aws:iam::" + ecrPrincipalTestAccount + ":user/dev" + _, err := gw.resolveECRPrincipal(claims) + require.Error(t, err) + assert.False(t, isECRDependencyFailure(err)) +} diff --git a/spinifex/gateway/ecr_test.go b/spinifex/gateway/ecr_test.go index b9570a90a..06b7d472b 100644 --- a/spinifex/gateway/ecr_test.go +++ b/spinifex/gateway/ecr_test.go @@ -15,6 +15,11 @@ import ( "github.com/stretchr/testify/require" ) +// setupECRRequest builds an ECR control-plane request context. It carries a +// full SigV4-shaped identity (not just accountID) so handlers reached through +// it — including GetAuthorizationToken, which now builds a canonical caller +// ARN from the context rather than a best-effort helper — behave exactly as +// they would for a real authenticated user. func setupECRRequest(target, body string) *http.Request { req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(body)) if target != "" { @@ -22,6 +27,9 @@ func setupECRRequest(target, body string) *http.Request { } ctx := context.WithValue(req.Context(), ctxService, "ecr") ctx = context.WithValue(ctx, ctxAccountID, "123456789012") + ctx = context.WithValue(ctx, ctxIdentity, "dev") + ctx = context.WithValue(ctx, ctxPrincipalType, principalTypeUser) + ctx = context.WithValue(ctx, ctxAccessKey, "AKIAECRTESTCONTROLPL1") return req.WithContext(ctx) } diff --git a/spinifex/gateway/ecr_token.go b/spinifex/gateway/ecr_token.go index 84574629f..5d425b1e5 100644 --- a/spinifex/gateway/ecr_token.go +++ b/spinifex/gateway/ecr_token.go @@ -64,10 +64,35 @@ func (gw *GatewayConfig) handleECRToken(w http.ResponseWriter, r *http.Request) return } + // A refresh must not re-issue a token for an identity that was revoked + // since the presented token was minted: rehydrate against current IAM/STS + // state and mint the replacement from that authoritative record, never + // blindly from the presented claims. + principal, err := gw.resolveECRPrincipal(claims) + if err != nil { + // No Bearer challenge here: this is already the challenge target, and a + // revoked identity retrying the challenge flow would only loop back. + if isECRDependencyFailure(err) { + slog.Error("ECR token endpoint: principal rehydration dependency failure", "err", err) + gateway_ecr.WriteError(w, http.StatusServiceUnavailable, "UNKNOWN", "authorization unavailable") + return + } + slog.Warn("ECR token endpoint: refusing to refresh revoked identity", "err", err) + gateway_ecr.WriteError(w, http.StatusUnauthorized, "UNAUTHORIZED", "invalid credentials") + return + } + + callerARN, err := buildCallerARN(principal.accountID, principal.identity, principal.principalType, principal.assumedRoleARN) + if err != nil { + slog.Error("ECR token endpoint: cannot build canonical caller ARN", "err", err) + gateway_ecr.WriteError(w, http.StatusInternalServerError, "SERVER_ERROR", "token mint failed") + return + } + fresh, expiresAt, err := gw.ECRTokenIssuer.Mint(gateway_ecrauth.Principal{ - AccountID: claims.AccountID, - ARN: claims.Subject, - Type: claims.PrincipalType, + AccountID: principal.accountID, + ARN: callerARN, + Type: principal.principalType, AccessKeyID: claims.AccessKeyID, }) if err != nil { diff --git a/spinifex/gateway/ecr_token_test.go b/spinifex/gateway/ecr_token_test.go index dc584d73b..434e1fcd5 100644 --- a/spinifex/gateway/ecr_token_test.go +++ b/spinifex/gateway/ecr_token_test.go @@ -22,7 +22,10 @@ func decodeTokenResp(t *testing.T, w *httptest.ResponseRecorder) ociTokenRespons func TestECRToken_ExchangesBasicForBearer(t *testing.T) { iss, verify := newECRAuth(t) - gw := &GatewayConfig{Region: ecrTestRegion, InternalSuffix: ecrTestSuffix, ECRTokenIssuer: iss, ECRTokenVerifier: verify} + gw := &GatewayConfig{ + Region: ecrTestRegion, InternalSuffix: ecrTestSuffix, ECRTokenIssuer: iss, ECRTokenVerifier: verify, + IAMService: ecrBridgeTestIAM(ecrTestAccount), + } req := httptest.NewRequest(http.MethodGet, "/v2/token", nil) req.Header.Set("Authorization", mintBasic(t, iss, ecrTestAccount)) @@ -44,9 +47,17 @@ func TestECRToken_ExchangesBasicForBearer(t *testing.T) { func TestECRToken_BearerCredentialAccepted(t *testing.T) { iss, verify := newECRAuth(t) - gw := &GatewayConfig{Region: ecrTestRegion, InternalSuffix: ecrTestSuffix, ECRTokenIssuer: iss, ECRTokenVerifier: verify} + gw := &GatewayConfig{ + Region: ecrTestRegion, InternalSuffix: ecrTestSuffix, ECRTokenIssuer: iss, ECRTokenVerifier: verify, + IAMService: ecrBridgeTestIAM(ecrTestAccount), + } - tok, _, err := iss.Mint(gateway_ecrauth.Principal{AccountID: ecrTestAccount, ARN: "arn:aws:iam::" + ecrTestAccount + ":user/dev"}) + tok, _, err := iss.Mint(gateway_ecrauth.Principal{ + AccountID: ecrTestAccount, + ARN: "arn:aws:iam::" + ecrTestAccount + ":user/dev", + Type: principalTypeUser, + AccessKeyID: ecrBridgeTestAKID, + }) require.NoError(t, err) req := httptest.NewRequest(http.MethodGet, "/v2/token", nil) @@ -113,7 +124,10 @@ func TestECRToken_MultipleAuthHeadersRejected(t *testing.T) { // endpoint (200) rather than the bridge-guarded /* catch-all. func TestOCIRegistry_TokenRouted(t *testing.T) { iss, verify := newECRAuth(t) - gw := &GatewayConfig{DisableLogging: true, Region: ecrTestRegion, InternalSuffix: ecrTestSuffix, ECRTokenIssuer: iss, ECRTokenVerifier: verify} + gw := &GatewayConfig{ + DisableLogging: true, Region: ecrTestRegion, InternalSuffix: ecrTestSuffix, ECRTokenIssuer: iss, ECRTokenVerifier: verify, + IAMService: ecrBridgeTestIAM(ecrTestAccount), + } r := chi.NewRouter() gw.mountOCIRegistry(r) diff --git a/spinifex/gateway/ecrauth/ecrauth_test.go b/spinifex/gateway/ecrauth/ecrauth_test.go index 2ce76b324..3f3ff8458 100644 --- a/spinifex/gateway/ecrauth/ecrauth_test.go +++ b/spinifex/gateway/ecrauth/ecrauth_test.go @@ -23,11 +23,15 @@ var testMasterKey = func() []byte { return k }() +// samplePrincipal returns a Principal shaped like the gateway's user +// principalContext. Type uses the same literal ("user") the gateway package's +// principalTypeUser constant holds; this package can't import gateway to +// reference the constant directly without cycling. func samplePrincipal() Principal { return Principal{ AccountID: "000000000001", ARN: "arn:aws:iam::000000000001:user/dev", - Type: "IAMUser", + Type: "user", AccessKeyID: "AKIAVAOB203DGNBR04XP", } } @@ -80,7 +84,7 @@ func TestIssuerVerifier_RoundTrip(t *testing.T) { require.NoError(t, err) assert.Equal(t, "000000000001", claims.AccountID) assert.Equal(t, "arn:aws:iam::000000000001:user/dev", claims.Subject) - assert.Equal(t, "IAMUser", claims.PrincipalType) + assert.Equal(t, "user", claims.PrincipalType) } func TestIssuer_MintRequiresAccountID(t *testing.T) { @@ -94,6 +98,43 @@ func TestIssuer_MintRequiresAccountID(t *testing.T) { require.Error(t, err) } +// TestMint_RequiresCompleteIdentityPointer pins the plan's claim contract: a +// token minted with any field missing from the identity pointer (ARN, +// accessKeyID, an unsupported principalType) must fail to mint at all, since +// a token that can't name a lookup key can never be safely rehydrated. +func TestMint_RequiresCompleteIdentityPointer(t *testing.T) { + _, _, js := testutil.StartTestJetStream(t) + key, _, err := LoadOrCreateSigningKey(js, testMasterKey, 1) + require.NoError(t, err) + iss := NewIssuer(key, testAudience) + + cases := []struct { + name string + mutate func(p Principal) Principal + }{ + {"missing ARN", func(p Principal) Principal { p.ARN = ""; return p }}, + {"missing accessKeyID", func(p Principal) Principal { p.AccessKeyID = ""; return p }}, + {"unsupported principalType", func(p Principal) Principal { p.Type = "IAMUser"; return p }}, + {"empty principalType", func(p Principal) Principal { p.Type = ""; return p }}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + _, _, err := iss.Mint(c.mutate(samplePrincipal())) + require.Error(t, err) + }) + } +} + +// TestSupportedPrincipalType pins the exact set of principalType claim values +// Verify accepts, mirrored from the gateway package's principalType constants. +func TestSupportedPrincipalType(t *testing.T) { + assert.True(t, SupportedPrincipalType("user")) + assert.True(t, SupportedPrincipalType("assumed-role")) + assert.True(t, SupportedPrincipalType("root")) + assert.False(t, SupportedPrincipalType("IAMUser")) + assert.False(t, SupportedPrincipalType("")) +} + func TestVerifier_RejectsWrongAudience(t *testing.T) { _, _, js := testutil.StartTestJetStream(t) key, verify, err := LoadOrCreateSigningKey(js, testMasterKey, 1) @@ -143,6 +184,55 @@ func TestVerifier_RejectsExpiredToken(t *testing.T) { require.Error(t, err) } +// TestVerifier_RejectsIncompleteIdentityPointer hand-mints tokens that skip +// Mint's own checks, proving Verify itself refuses a claim set missing any +// part of the identity pointer even if it were signed by some other path. +func TestVerifier_RejectsIncompleteIdentityPointer(t *testing.T) { + _, _, js := testutil.StartTestJetStream(t) + key, verify, err := LoadOrCreateSigningKey(js, testMasterKey, 1) + require.NoError(t, err) + + base := func() Claims { + return Claims{ + RegisteredClaims: jwt.RegisteredClaims{ + Issuer: tokenIssuer, + Audience: jwt.ClaimStrings{testAudience}, + Subject: "arn:aws:iam::000000000001:user/dev", + ExpiresAt: jwt.NewNumericDate(time.Now().Add(time.Hour)), + }, + AccountID: "000000000001", + PrincipalType: "user", + AccessKeyID: "AKIAVAOB203DGNBR04XP", + } + } + sign := func(t *testing.T, c Claims) string { + t.Helper() + raw := jwt.NewWithClaims(jwt.SigningMethodES256, c) + raw.Header["kid"] = key.Kid + signed, err := raw.SignedString(key.priv) + require.NoError(t, err) + return signed + } + + cases := []struct { + name string + mutate func(c Claims) Claims + }{ + {"missing subject", func(c Claims) Claims { c.Subject = ""; return c }}, + {"missing accountID", func(c Claims) Claims { c.AccountID = ""; return c }}, + {"missing accessKeyID", func(c Claims) Claims { c.AccessKeyID = ""; return c }}, + {"unsupported principalType", func(c Claims) Claims { c.PrincipalType = "IAMUser"; return c }}, + {"empty principalType", func(c Claims) Claims { c.PrincipalType = ""; return c }}, + } + v := NewVerifier(verify, testAudience) + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + _, err := v.Verify(sign(t, tc.mutate(base()))) + require.Error(t, err) + }) + } +} + func TestVerifier_RejectsNonES256(t *testing.T) { _, _, js := testutil.StartTestJetStream(t) _, verify, err := LoadOrCreateSigningKey(js, testMasterKey, 1) diff --git a/spinifex/gateway/ecrauth/token.go b/spinifex/gateway/ecrauth/token.go index 6772e06e3..70775fb54 100644 --- a/spinifex/gateway/ecrauth/token.go +++ b/spinifex/gateway/ecrauth/token.go @@ -19,9 +19,33 @@ const ( DefaultTokenTTL = 12 * time.Hour ) +// supportedPrincipalTypes are the mulga:principalType claim values a verified +// token may carry. These mirror the gateway package's principalType constants +// (user, assumed-role, root); this package can't import gateway (it would +// cycle), so the strings are pinned here and by the gateway package's own +// constants — a rename on either side is caught by ecrauth_test.go and by +// the gateway ECR principal tests exercising real Mint/Verify round-trips. +var supportedPrincipalTypes = map[string]bool{ + "user": true, + "assumed-role": true, + "root": true, +} + +// SupportedPrincipalType reports whether t is a principalType claim value +// Verify accepts. Exported so callers minting a token (or tests) can validate +// a type before it round-trips through Mint/Verify. +func SupportedPrincipalType(t string) bool { + return supportedPrincipalTypes[t] +} + // Claims is the ECR token claim set: registered OIDC claims plus the mulga -// principal attributes the bridge reads to scope a request. accountID is the -// authority for /v2 account scoping; accessKeyID is audit-only. +// principal attributes the bridge reads to scope a request. The token is a +// signed pointer, not a permission grant: accountID scopes /v2 account +// access, and accessKeyID is the lookup key the gateway uses to rehydrate the +// current IAM/STS record on every request, so a revoked key, session, user, +// role, or policy takes effect on the very next request even against an +// already-issued 12h token. Verify requires all three of these plus Subject +// to be non-empty and PrincipalType to be a supported value. type Claims struct { jwt.RegisteredClaims @@ -84,6 +108,18 @@ func (i *Issuer) Mint(p Principal) (token string, expiresAt time.Time, err error if p.AccountID == "" { return "", time.Time{}, errors.New("ecrauth: mint requires account ID") } + // A registry token is a signed pointer to an IAM/STS record: every field + // the verifier will later demand back must be present at mint time, or + // the token would verify into an unusable/ambiguous identity. + if p.ARN == "" { + return "", time.Time{}, errors.New("ecrauth: mint requires principal ARN") + } + if p.AccessKeyID == "" { + return "", time.Time{}, errors.New("ecrauth: mint requires access key ID") + } + if !SupportedPrincipalType(p.Type) { + return "", time.Time{}, fmt.Errorf("ecrauth: mint requires a supported principal type, got %q", p.Type) + } now := time.Now() exp := now.Add(i.ttl) claims := Claims{ @@ -138,8 +174,12 @@ func (v *Verifier) publicKey(kid string) (*ecdsa.PublicKey, bool) { return pub, ok } -// Verify parses and validates raw: ES256, unexpired, known kid, matching iss and -// aud, non-empty accountID. It returns the claims on success. +// Verify parses and validates raw: ES256, unexpired, known kid, matching iss +// and aud, and a complete identity pointer — non-empty subject, accountID, +// accessKeyID, and a supported principalType. The gateway resolves the +// returned claims against current IAM/STS state before trusting them; Verify +// only proves the token was signed by this issuer and names a well-formed +// identity to look up. func (v *Verifier) Verify(raw string) (*Claims, error) { claims := &Claims{} parser := jwt.NewParser( @@ -163,8 +203,17 @@ func (v *Verifier) Verify(raw string) (*Claims, error) { if !slices.Contains(claims.Audience, v.audience) { return nil, errors.New("audience mismatch") } + if claims.Subject == "" { + return nil, errors.New("missing subject claim") + } if claims.AccountID == "" { return nil, errors.New("missing accountID claim") } + if claims.AccessKeyID == "" { + return nil, errors.New("missing accessKeyID claim") + } + if !SupportedPrincipalType(claims.PrincipalType) { + return nil, fmt.Errorf("unsupported principalType %q", claims.PrincipalType) + } return claims, nil } diff --git a/spinifex/gateway/ecrauth_middleware.go b/spinifex/gateway/ecrauth_middleware.go index 8e0e8a5a6..052297c4e 100644 --- a/spinifex/gateway/ecrauth_middleware.go +++ b/spinifex/gateway/ecrauth_middleware.go @@ -13,8 +13,11 @@ import ( // ecrAuthBridge authenticates /v2/* requests with an ECR token. It accepts a // single Authorization header carrying "Bearer " or "Basic AWS:", // verifies the JWT, enforces that the token account matches the host-derived -// account, and stashes the resolved account/principal on the request context. -// Unauthenticated or invalid requests get a 401 Bearer challenge. +// account, rehydrates the token's principal against current IAM/STS state, +// and stashes the resolved account/principal on the request context. +// Unauthenticated, invalid, or revoked-identity requests get a 401 Bearer +// challenge; a backend outage that prevents rehydration fails closed with 503 +// rather than falling through as an allow. // // A nil verifier disables the bridge (registry mounts open), matching the nil // ECRRegistry fallback used by unit tests of unrelated routes. @@ -54,8 +57,25 @@ func (gw *GatewayConfig) ecrAuthBridge(next http.Handler) http.Handler { return } + // The JWT is a signed pointer, not a permission grant: rehydrate it + // against current IAM/STS state so a revoked key, session, user, role, + // or account takes effect on this request even though the token itself + // is still cryptographically valid and unexpired. + principal, err := gw.resolveECRPrincipal(claims) + if err != nil { + if isECRDependencyFailure(err) { + slog.Error("ECR auth bridge: principal rehydration dependency failure", "err", err) + gateway_ecr.WriteError(w, http.StatusServiceUnavailable, "UNKNOWN", "authorization unavailable") + return + } + slog.Warn("ECR auth bridge: principal rehydration rejected token", "err", err) + gw.writeECRChallenge(w, r) + return + } + ctx := gateway_ecr.WithAuthAccount(r.Context(), claims.AccountID) ctx = context.WithValue(ctx, ctxAuthPrincipal, claims.Subject) + ctx = context.WithValue(ctx, ctxECRPrincipal, principal) next.ServeHTTP(w, r.WithContext(ctx)) }) } diff --git a/spinifex/gateway/ecrauth_middleware_test.go b/spinifex/gateway/ecrauth_middleware_test.go index baa445dc3..a7ea9500c 100644 --- a/spinifex/gateway/ecrauth_middleware_test.go +++ b/spinifex/gateway/ecrauth_middleware_test.go @@ -38,13 +38,37 @@ func newECRAuth(t *testing.T) (*gateway_ecrauth.Issuer, *gateway_ecrauth.Verifie return gateway_ecrauth.NewIssuer(key, ecrTestAudience), gateway_ecrauth.NewVerifier(verify, ecrTestAudience) } +// mintBasic mints a token for account/"dev" using ecrBridgeTestAKID as its +// access key ID. Tests that expect the bridge to pass the request through +// must wire a GatewayConfig whose IAMService resolves that key back to an +// active "dev" user in account (see ecrBridgeTestIAM), since the bridge now +// rehydrates every token against current IAM state rather than trusting the +// claims outright. func mintBasic(t *testing.T, iss *gateway_ecrauth.Issuer, account string) string { t.Helper() - tok, _, err := iss.Mint(gateway_ecrauth.Principal{AccountID: account, ARN: "arn:aws:iam::" + account + ":user/dev"}) + tok, _, err := iss.Mint(gateway_ecrauth.Principal{ + AccountID: account, + ARN: "arn:aws:iam::" + account + ":user/dev", + Type: principalTypeUser, + AccessKeyID: ecrBridgeTestAKID, + }) require.NoError(t, err) return "Basic " + base64.StdEncoding.EncodeToString([]byte("AWS:"+tok)) } +// ecrBridgeTestAKID is the access key ID mintBasic bakes into every token it +// mints. +const ecrBridgeTestAKID = "AKIABRIDGETESTUSER001" + +// ecrBridgeTestIAM returns an ecrMockIAMService that resolves +// ecrBridgeTestAKID to an active "dev" user in account, so a token minted by +// mintBasic rehydrates successfully through ecrAuthBridge. +func ecrBridgeTestIAM(account string) *ecrMockIAMService { + iamSvc := newECRMockIAMService() + seedECRTestUser(iamSvc, account, "dev", ecrBridgeTestAKID) + return iamSvc +} + func okHandler() (http.Handler, *bool) { called := new(bool) h := http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { @@ -110,7 +134,10 @@ func TestECRAuthBridge_NoAuthChallenges401(t *testing.T) { func TestECRAuthBridge_ValidTokenPassesThrough(t *testing.T) { iss, verify := newECRAuth(t) - gw := &GatewayConfig{Region: ecrTestRegion, InternalSuffix: ecrTestSuffix, ECRTokenVerifier: verify} + gw := &GatewayConfig{ + Region: ecrTestRegion, InternalSuffix: ecrTestSuffix, ECRTokenVerifier: verify, + IAMService: ecrBridgeTestIAM(ecrTestAccount), + } next, called := okHandler() req := httptest.NewRequest(http.MethodGet, "/v2/", nil) diff --git a/spinifex/gateway/gateway.go b/spinifex/gateway/gateway.go index 561decc78..404ea755f 100644 --- a/spinifex/gateway/gateway.go +++ b/spinifex/gateway/gateway.go @@ -64,6 +64,11 @@ const ( // The resolved account is stashed via gateway_ecr.WithAuthAccount so the // registry package can read it without sharing this package's key type. ctxAuthPrincipal contextKey = "ecr.authPrincipal" + // ctxECRPrincipal carries the principalContext resolveECRPrincipal rebuilt + // from current IAM/STS state for the request's ECR token. The operation- + // authorization middleware reads this to run the same policy evaluator + // SigV4 requests use; it is never populated for non-ECR routes. + ctxECRPrincipal contextKey = "ecr.principal" ) // Values stored under ctxPrincipalType. Downstream handlers that interpret @@ -404,8 +409,9 @@ func (gw *GatewayConfig) checkPolicy(r *http.Request, service, action string) er } // checkPolicyResource evaluates IAM policies against a specific resource ARN. -// Root users bypass evaluation. Nil IAMService allows (pre-IAM compatibility). -// Used by EC2 paths that enforce iam:PassRole before attaching an instance profile. +// Root users bypass evaluation. Nil IAMService, or a request with no SigV4 +// auth context, allows (pre-IAM compatibility). Used by EC2 paths that +// enforce iam:PassRole before attaching an instance profile. func (gw *GatewayConfig) checkPolicyResource(r *http.Request, service, action, resource string) error { if gw.IAMService == nil { slog.Warn("checkPolicy: IAM service not available, skipping policy check", @@ -423,13 +429,47 @@ func (gw *GatewayConfig) checkPolicyResource(r *http.Request, service, action, r slog.Error("checkPolicy: identity has unexpected type", "type", fmt.Sprintf("%T", identityVal)) return errors.New(awserrors.ErrorInternalError) } + if identity == "" { + // Pre-IAM compatibility: an authenticated request with no identity name + // predates per-principal policy enforcement. + return nil + } accountID, _ := r.Context().Value(ctxAccountID).(string) if accountID == "" { slog.Error("checkPolicy: no account ID in auth context", "user", identity) return errors.New(awserrors.ErrorInternalError) } - iamAction := policy.IAMAction(service, action) + principal := principalContext{ + identity: identity, + accountID: accountID, + principalType: mustCtxString(r, ctxPrincipalType), + assumedRoleARN: mustCtxString(r, ctxAssumedRoleARN), + assumedRoleID: mustCtxString(r, ctxAssumedRoleID), + underlyingRoleARN: mustCtxString(r, ctxUnderlyingRoleARN), + } + return gw.evaluatePrincipalPolicy(principal, policy.IAMAction(service, action), resource) +} + +// mustCtxString reads a string context value, defaulting to "" for an absent +// or wrong-typed key rather than panicking. +func mustCtxString(r *http.Request, key contextKey) string { + v, _ := r.Context().Value(key).(string) + return v +} + +// evaluatePrincipalPolicy is the request-shape-independent core of policy +// enforcement: given an already-resolved principal (from SigV4 or, for /v2/* +// ECR requests, a freshly rehydrated token identity), it resolves the +// principal's current policies and evaluates iamAction against resource. +// Unlike checkPolicyResource, a nil IAMService fails closed here — callers +// that want pre-IAM-compatibility bypass behavior must check for that before +// calling in, exactly as checkPolicyResource does. +func (gw *GatewayConfig) evaluatePrincipalPolicy(principal principalContext, iamAction, resource string) error { + if gw.IAMService == nil { + slog.Error("evaluatePrincipalPolicy: IAM service not available", "action", iamAction) + return errors.New(awserrors.ErrorInternalError) + } // Each branch resolves the policy resolver and log identity for its principal // type. Identity-sensitive decisions (root bypass, resolver selection) are @@ -438,36 +478,34 @@ func (gw *GatewayConfig) checkPolicyResource(r *http.Request, service, action, r var resolve func() ([]handlers_iam.PolicyDocument, error) var logIdentity string - principalType, _ := r.Context().Value(ctxPrincipalType).(string) - switch principalType { + switch principal.principalType { case principalTypeUser: - if identity == "" || (identity == "root" && accountID == utils.GlobalAccountID) { - // root / pre-IAM bypass — user branch only. + if principal.identity == "root" && principal.accountID == utils.GlobalAccountID { + // Global root bypass — user branch only. return nil } resolve = func() ([]handlers_iam.PolicyDocument, error) { - return gw.IAMService.GetUserPolicies(accountID, identity) + return gw.IAMService.GetUserPolicies(principal.accountID, principal.identity) } - logIdentity = identity + logIdentity = principal.identity case principalTypeAssumedRole: // Resolve by the session's underlying role, never by SessionName (attacker-influenced). // A missing/legacy, cross-account, or malformed ARN fails closed with AccessDenied. - underlyingRoleARN, _ := r.Context().Value(ctxUnderlyingRoleARN).(string) - roleAcct, roleName, perr := auth.ParseRoleARN(underlyingRoleARN) - if perr != nil || roleAcct != accountID { - slog.Warn("checkPolicy: unresolvable or cross-account assumed-role principal denied", - "underlyingRoleARN", underlyingRoleARN, - "accountID", accountID, + roleAcct, roleName, perr := auth.ParseRoleARN(principal.underlyingRoleARN) + if perr != nil || roleAcct != principal.accountID { + slog.Warn("evaluatePrincipalPolicy: unresolvable or cross-account assumed-role principal denied", + "underlyingRoleARN", principal.underlyingRoleARN, + "accountID", principal.accountID, "action", iamAction, "err", perr) return errors.New(awserrors.ErrorAccessDenied) } resolve = func() ([]handlers_iam.PolicyDocument, error) { - return gw.IAMService.GetRolePolicies(accountID, roleName) + return gw.IAMService.GetRolePolicies(principal.accountID, roleName) } - logIdentity, _ = r.Context().Value(ctxAssumedRoleARN).(string) + logIdentity = principal.assumedRoleARN default: - slog.Error("checkPolicy: unknown principal type", "principalType", principalType) + slog.Error("evaluatePrincipalPolicy: unknown principal type", "principalType", principal.principalType) return errors.New(awserrors.ErrorInternalError) } @@ -480,18 +518,18 @@ func (gw *GatewayConfig) checkPolicyResource(r *http.Request, service, action, r break } if attempt < 2 { - slog.Debug("checkPolicy: transient NATS error, retrying", + slog.Debug("evaluatePrincipalPolicy: transient NATS error, retrying", "identity", logIdentity, "attempt", attempt+1, "err", err) time.Sleep(time.Duration(attempt+1) * 200 * time.Millisecond) } } if err != nil { - slog.Error("checkPolicy: failed to resolve policies", "identity", logIdentity, "err", err) + slog.Error("evaluatePrincipalPolicy: failed to resolve policies", "identity", logIdentity, "err", err) return errors.New(awserrors.ErrorInternalError) } if iampolicy.Evaluate(iamAction, resource, policies) == iampolicy.Deny { - slog.Info("checkPolicy: access denied", "identity", logIdentity, "action", iamAction, "resource", resource) + slog.Info("evaluatePrincipalPolicy: access denied", "identity", logIdentity, "action", iamAction, "resource", resource) return errors.New(awserrors.ErrorAccessDenied) } diff --git a/tests/e2e/ecr/iam_authorization_test.go b/tests/e2e/ecr/iam_authorization_test.go new file mode 100644 index 000000000..4c87c24b3 --- /dev/null +++ b/tests/e2e/ecr/iam_authorization_test.go @@ -0,0 +1,304 @@ +//go:build e2e + +package ecr + +import ( + "crypto/sha256" + "encoding/hex" + "fmt" + "net/http" + "testing" + "time" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/iam" + "github.com/aws/aws-sdk-go/service/sts" + "github.com/mulgadc/spinifex/tests/e2e/harness" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const ( + pullOnlyPolicyARN = "arn:aws:iam::aws:policy/AmazonEC2ContainerRegistryPullOnly" + readOnlyPolicyARN = "arn:aws:iam::aws:policy/AmazonEC2ContainerRegistryReadOnly" + + // ociManifestMediaType is the content type used for the minimal image + // manifests this suite pushes to seed pull fixtures. + ociManifestMediaType = "application/vnd.docker.distribution.manifest.v2+json" + + // iamTrustAnyPrincipal lets any authenticated caller in the account assume + // the role, matching the pattern used elsewhere in the E2E suite for + // roles that only exist to be assumed by the test's own admin credentials. + iamTrustAnyPrincipal = `{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Principal":{"AWS":"*"},"Action":"sts:AssumeRole"}]}` +) + +func ociDigest(data []byte) string { + sum := sha256.Sum256(data) + return "sha256:" + hex.EncodeToString(sum[:]) +} + +func uniqueName(prefix string) string { + return fmt.Sprintf("%s-%d", prefix, time.Now().UnixNano()) +} + +// seedOCIRepo creates repo as an administrator and pushes one minimal image +// (an empty-layer manifest referencing a single config blob) tagged "v1", +// giving the authorization-matrix subtests something to pull. Returns the +// manifest digest. +func seedOCIRepo(t *testing.T, f *Fixture, host, repo string) string { + t.Helper() + harness.CreateECRRepository(t, f.AWS, repo) + adminBearer := harness.ECRGetLoginPassword(t, f.AWS) + + cfg := []byte("iam-authorization-e2e-config") + cfgDigest := ociDigest(cfg) + + status, hdr, body := harness.OCIRequest(t, f.AWS, http.MethodPost, host, "/v2/"+repo+"/blobs/uploads/", adminBearer, nil) + require.Equal(t, http.StatusAccepted, status, "start upload: %s", body) + loc := hdr.Get("Location") + require.NotEmpty(t, loc, "upload response missing Location header") + + status, _, body = harness.OCIRequest(t, f.AWS, http.MethodPut, host, loc+"?digest="+cfgDigest, adminBearer, cfg) + require.Equal(t, http.StatusCreated, status, "finish upload: %s", body) + + manifest := fmt.Appendf(nil, + `{"schemaVersion":2,"mediaType":"%s","config":{"digest":"%s"},"layers":[]}`, + ociManifestMediaType, cfgDigest) + status, hdr, body = harness.OCIRequest(t, f.AWS, http.MethodPut, host, "/v2/"+repo+"/manifests/v1", adminBearer, manifest) + require.Equal(t, http.StatusCreated, status, "push manifest: %s", body) + return hdr.Get("Docker-Content-Digest") +} + +// newScopedIAMUser creates a fresh IAM user with policyARN attached and one +// active access key, registering teardown. Returns an AWSClient bound to +// that key so callers can mint ECR bearer tokens as this identity. +func newScopedIAMUser(t *testing.T, f *Fixture, policyARN string) *harness.AWSClient { + t.Helper() + userName := uniqueName("ecr-authz-e2e") + + _, err := f.AWS.IAM.CreateUser(&iam.CreateUserInput{UserName: aws.String(userName)}) + require.NoError(t, err, "create-user") + t.Cleanup(func() { + _, _ = f.AWS.IAM.DetachUserPolicy(&iam.DetachUserPolicyInput{ + UserName: aws.String(userName), PolicyArn: aws.String(policyARN), + }) + keys, kerr := f.AWS.IAM.ListAccessKeys(&iam.ListAccessKeysInput{UserName: aws.String(userName)}) + if kerr == nil { + for _, k := range keys.AccessKeyMetadata { + _, _ = f.AWS.IAM.DeleteAccessKey(&iam.DeleteAccessKeyInput{ + UserName: aws.String(userName), AccessKeyId: k.AccessKeyId, + }) + } + } + _, _ = f.AWS.IAM.DeleteUser(&iam.DeleteUserInput{UserName: aws.String(userName)}) + }) + + _, err = f.AWS.IAM.AttachUserPolicy(&iam.AttachUserPolicyInput{ + UserName: aws.String(userName), PolicyArn: aws.String(policyARN), + }) + require.NoError(t, err, "attach-user-policy") + + keyOut, err := f.AWS.IAM.CreateAccessKey(&iam.CreateAccessKeyInput{UserName: aws.String(userName)}) + require.NoError(t, err, "create-access-key") + + return harness.NewAWSClientWithCreds(t, f.Env, + aws.StringValue(keyOut.AccessKey.AccessKeyId), aws.StringValue(keyOut.AccessKey.SecretAccessKey)) +} + +// assertPullAllowedPushDenied runs the read/write half of the release-gate +// matrix common to every scoped-credential subtest below: pulling repo's +// seeded "v1" tag and HEADing its blob must succeed, while every mutating +// operation must return 403 DENIED without ever reaching the object store. +func assertPullAllowedPushDenied(t *testing.T, cli *harness.AWSClient, host, repo, manifestDigest string) { + t.Helper() + bearer := harness.ECRGetLoginPassword(t, cli) + + status, _, body := harness.OCIRequest(t, cli, http.MethodGet, host, "/v2/"+repo+"/manifests/v1", bearer, nil) + assert.Equal(t, http.StatusOK, status, "pull manifest: %s", body) + + status, _, body = harness.OCIRequest(t, cli, http.MethodPost, host, "/v2/"+repo+"/blobs/uploads/", bearer, nil) + assert.Equal(t, http.StatusForbidden, status, "push must be denied: %s", body) + + status, _, body = harness.OCIRequest(t, cli, http.MethodPut, host, "/v2/"+repo+"/manifests/v1", bearer, + []byte(`{"schemaVersion":2,"mediaType":"`+ociManifestMediaType+`","config":{"digest":"sha256:0000000000000000000000000000000000000000000000000000000000000000"},"layers":[]}`)) + assert.Equal(t, http.StatusForbidden, status, "manifest overwrite must be denied: %s", body) + + status, _, body = harness.OCIRequest(t, cli, http.MethodDelete, host, "/v2/"+repo+"/manifests/"+manifestDigest, bearer, nil) + assert.Equal(t, http.StatusForbidden, status, "manifest delete must be denied: %s", body) +} + +// TestECRIAMAuthorization_PullOnly proves a PullOnly-scoped identity can pull +// but not push, overwrite, or delete, and cannot list the account catalog +// (DescribeRepositories is outside PullOnly's grant). +func TestECRIAMAuthorization_PullOnly(t *testing.T) { + f := requireECRFixture(t) + host := harness.ECRRegistryHost(f.Account) + harness.RequireRegistryResolves(t, host) + + repo := uniqueRepo("authz-pullonly") + manifestDigest := seedOCIRepo(t, f, host, repo) + + cli := newScopedIAMUser(t, f, pullOnlyPolicyARN) + assertPullAllowedPushDenied(t, cli, host, repo, manifestDigest) + + bearer := harness.ECRGetLoginPassword(t, cli) + status, _, body := harness.OCIRequest(t, cli, http.MethodGet, host, "/v2/_catalog", bearer, nil) + assert.Equal(t, http.StatusForbidden, status, "catalog listing outside PullOnly's grant must be denied: %s", body) +} + +// TestECRIAMAuthorization_ReadOnly proves a ReadOnly-scoped identity can pull, +// list tags, and list the catalog, while push, overwrite, and delete remain +// denied. +func TestECRIAMAuthorization_ReadOnly(t *testing.T) { + f := requireECRFixture(t) + host := harness.ECRRegistryHost(f.Account) + harness.RequireRegistryResolves(t, host) + + repo := uniqueRepo("authz-readonly") + manifestDigest := seedOCIRepo(t, f, host, repo) + + cli := newScopedIAMUser(t, f, readOnlyPolicyARN) + assertPullAllowedPushDenied(t, cli, host, repo, manifestDigest) + + bearer := harness.ECRGetLoginPassword(t, cli) + status, _, body := harness.OCIRequest(t, cli, http.MethodGet, host, "/v2/_catalog", bearer, nil) + assert.Equal(t, http.StatusOK, status, "catalog listing must be allowed under ReadOnly: %s", body) + + status, _, body = harness.OCIRequest(t, cli, http.MethodGet, host, "/v2/"+repo+"/tags/list", bearer, nil) + assert.Equal(t, http.StatusOK, status, "tag listing must be allowed under ReadOnly: %s", body) +} + +// TestECRIAMAuthorization_AssumedRolePullOnly proves the same pull-allow / +// push-deny matrix holds for a token minted from ASIA (assumed-role) session +// credentials, not just long-lived AKIA users. +func TestECRIAMAuthorization_AssumedRolePullOnly(t *testing.T) { + f := requireECRFixture(t) + host := harness.ECRRegistryHost(f.Account) + harness.RequireRegistryResolves(t, host) + + repo := uniqueRepo("authz-role-pullonly") + manifestDigest := seedOCIRepo(t, f, host, repo) + + roleName := uniqueName("ecr-authz-e2e-role") + roleARN := harness.IAMRoleARN(f.Account, roleName) + _, err := f.AWS.IAM.CreateRole(&iam.CreateRoleInput{ + RoleName: aws.String(roleName), + AssumeRolePolicyDocument: aws.String(iamTrustAnyPrincipal), + }) + require.NoError(t, err, "create-role") + t.Cleanup(func() { harness.IAMDeleteRoleAndProfilesBestEffort(f.AWS, roleName, nil, pullOnlyPolicyARN) }) + + _, err = f.AWS.IAM.AttachRolePolicy(&iam.AttachRolePolicyInput{ + RoleName: aws.String(roleName), PolicyArn: aws.String(pullOnlyPolicyARN), + }) + require.NoError(t, err, "attach-role-policy") + + assumed, err := f.AWS.STS.AssumeRole(&sts.AssumeRoleInput{ + RoleArn: aws.String(roleARN), + RoleSessionName: aws.String(uniqueName("authz-session")), + }) + require.NoError(t, err, "assume-role") + creds := assumed.Credentials + require.NotNil(t, creds, "AssumeRole returned nil Credentials") + cli := harness.NewAWSClientWithSessionCreds(t, f.Env, + aws.StringValue(creds.AccessKeyId), aws.StringValue(creds.SecretAccessKey), aws.StringValue(creds.SessionToken)) + + assertPullAllowedPushDenied(t, cli, host, repo, manifestDigest) +} + +// TestECRIAMAuthorization_DetachPolicyDeniesImmediately proves a JWT minted +// while a policy was attached is denied on its very next request once that +// policy is detached — the token is a pointer re-resolved against live IAM +// state, not a capability baked in at mint time. +func TestECRIAMAuthorization_DetachPolicyDeniesImmediately(t *testing.T) { + f := requireECRFixture(t) + host := harness.ECRRegistryHost(f.Account) + harness.RequireRegistryResolves(t, host) + + repo := uniqueRepo("authz-detach") + _ = seedOCIRepo(t, f, host, repo) + + userName := uniqueName("ecr-authz-e2e-detach") + _, err := f.AWS.IAM.CreateUser(&iam.CreateUserInput{UserName: aws.String(userName)}) + require.NoError(t, err, "create-user") + t.Cleanup(func() { + keys, kerr := f.AWS.IAM.ListAccessKeys(&iam.ListAccessKeysInput{UserName: aws.String(userName)}) + if kerr == nil { + for _, k := range keys.AccessKeyMetadata { + _, _ = f.AWS.IAM.DeleteAccessKey(&iam.DeleteAccessKeyInput{ + UserName: aws.String(userName), AccessKeyId: k.AccessKeyId, + }) + } + } + _, _ = f.AWS.IAM.DeleteUser(&iam.DeleteUserInput{UserName: aws.String(userName)}) + }) + _, err = f.AWS.IAM.AttachUserPolicy(&iam.AttachUserPolicyInput{ + UserName: aws.String(userName), PolicyArn: aws.String(pullOnlyPolicyARN), + }) + require.NoError(t, err, "attach-user-policy") + keyOut, err := f.AWS.IAM.CreateAccessKey(&iam.CreateAccessKeyInput{UserName: aws.String(userName)}) + require.NoError(t, err, "create-access-key") + cli := harness.NewAWSClientWithCreds(t, f.Env, + aws.StringValue(keyOut.AccessKey.AccessKeyId), aws.StringValue(keyOut.AccessKey.SecretAccessKey)) + + bearer := harness.ECRGetLoginPassword(t, cli) + status, _, body := harness.OCIRequest(t, cli, http.MethodGet, host, "/v2/"+repo+"/manifests/v1", bearer, nil) + require.Equal(t, http.StatusOK, status, "pull must succeed while policy is attached: %s", body) + + _, err = f.AWS.IAM.DetachUserPolicy(&iam.DetachUserPolicyInput{ + UserName: aws.String(userName), PolicyArn: aws.String(pullOnlyPolicyARN), + }) + require.NoError(t, err, "detach-user-policy") + + status, _, body = harness.OCIRequest(t, cli, http.MethodGet, host, "/v2/"+repo+"/manifests/v1", bearer, nil) + assert.Equal(t, http.StatusForbidden, status, "same JWT must be denied immediately once the policy is detached: %s", body) +} + +// TestECRIAMAuthorization_DeactivatedKeyReturns401 proves a JWT minted from a +// since-deactivated access key is refused with 401 (invalid principal), not +// 403 — the key itself no longer authenticates, so the request never reaches +// policy evaluation. +func TestECRIAMAuthorization_DeactivatedKeyReturns401(t *testing.T) { + f := requireECRFixture(t) + host := harness.ECRRegistryHost(f.Account) + harness.RequireRegistryResolves(t, host) + + repo := uniqueRepo("authz-deactivate") + _ = seedOCIRepo(t, f, host, repo) + + userName := uniqueName("ecr-authz-e2e-deactivate") + _, err := f.AWS.IAM.CreateUser(&iam.CreateUserInput{UserName: aws.String(userName)}) + require.NoError(t, err, "create-user") + t.Cleanup(func() { + keys, kerr := f.AWS.IAM.ListAccessKeys(&iam.ListAccessKeysInput{UserName: aws.String(userName)}) + if kerr == nil { + for _, k := range keys.AccessKeyMetadata { + _, _ = f.AWS.IAM.DeleteAccessKey(&iam.DeleteAccessKeyInput{ + UserName: aws.String(userName), AccessKeyId: k.AccessKeyId, + }) + } + } + _, _ = f.AWS.IAM.DeleteUser(&iam.DeleteUserInput{UserName: aws.String(userName)}) + }) + _, err = f.AWS.IAM.AttachUserPolicy(&iam.AttachUserPolicyInput{ + UserName: aws.String(userName), PolicyArn: aws.String(pullOnlyPolicyARN), + }) + require.NoError(t, err, "attach-user-policy") + keyOut, err := f.AWS.IAM.CreateAccessKey(&iam.CreateAccessKeyInput{UserName: aws.String(userName)}) + require.NoError(t, err, "create-access-key") + keyID := aws.StringValue(keyOut.AccessKey.AccessKeyId) + cli := harness.NewAWSClientWithCreds(t, f.Env, keyID, aws.StringValue(keyOut.AccessKey.SecretAccessKey)) + + bearer := harness.ECRGetLoginPassword(t, cli) + status, _, body := harness.OCIRequest(t, cli, http.MethodGet, host, "/v2/"+repo+"/manifests/v1", bearer, nil) + require.Equal(t, http.StatusOK, status, "pull must succeed while the key is active: %s", body) + + _, err = f.AWS.IAM.UpdateAccessKey(&iam.UpdateAccessKeyInput{ + UserName: aws.String(userName), AccessKeyId: aws.String(keyID), Status: aws.String(iam.StatusTypeInactive), + }) + require.NoError(t, err, "update-access-key deactivate") + + status, hdr, body := harness.OCIRequest(t, cli, http.MethodGet, host, "/v2/"+repo+"/manifests/v1", bearer, nil) + assert.Equal(t, http.StatusUnauthorized, status, "same JWT must be refused once its key is inactive: %s", body) + assert.NotEmpty(t, hdr.Get("Www-Authenticate"), "a revoked-but-well-formed token still gets the Bearer challenge") +} diff --git a/tests/e2e/harness/oci_raw.go b/tests/e2e/harness/oci_raw.go new file mode 100644 index 000000000..780deb4a0 --- /dev/null +++ b/tests/e2e/harness/oci_raw.go @@ -0,0 +1,53 @@ +//go:build e2e + +package harness + +import ( + "bytes" + "io" + "net/http" + "testing" +) + +// OCIRequest issues a raw HTTP request against an OCI Distribution Spec v2 +// registry host (see ECRRegistryHost), authenticating with bearerToken. It +// reuses c's TLS-configured HTTP client so certificate trust matches the AWS +// SDK clients built alongside it. Unlike the AWS SDK calls in this package, +// the OCI surface is not SigV4-signed — the gateway authenticates it purely +// via the Bearer JWT — so this issues the request directly rather than +// through a signer. +// +// t.Fatal fires only on a transport-level failure (DNS, connection refused, +// TLS handshake); HTTP-level outcomes (401/403/404/...) are returned for the +// caller to assert on, since that is exactly what the IAM authorization +// matrix tests exercise. +func OCIRequest(t *testing.T, c *AWSClient, method, host, path, bearerToken string, body []byte) (status int, headers http.Header, respBody []byte) { + t.Helper() + + var reader io.Reader + if body != nil { + reader = bytes.NewReader(body) + } + req, err := http.NewRequest(method, "https://"+host+path, reader) + if err != nil { + t.Fatalf("OCIRequest: build request: %v", err) + } + if bearerToken != "" { + req.Header.Set("Authorization", "Bearer "+bearerToken) + } + + httpClient := c.EC2Conf.Config.HTTPClient + if httpClient == nil { + httpClient = http.DefaultClient + } + resp, err := httpClient.Do(req) + if err != nil { + t.Fatalf("OCIRequest: %s %s: %v", method, path, err) + } + defer func() { _ = resp.Body.Close() }() + respBody, err = io.ReadAll(resp.Body) + if err != nil { + t.Fatalf("OCIRequest: read body: %v", err) + } + return resp.StatusCode, resp.Header, respBody +}