From a2a886d202d0efd257a930d8eaabb6d1f1fc41b7 Mon Sep 17 00:00:00 2001 From: Srdjan S Date: Thu, 10 Sep 2026 18:07:40 +0200 Subject: [PATCH] feat(iam): enforce the effective action set on the fast path A key granted only s3:PutObject could read and abort on the same bucket: s3perm maps that permission to a command set containing every command s3:GetObject, s3:ListBucket and s3:AbortMultipartUpload need, and the fast path decided on a chain probe alone. The authorize response's effective action set is now cached under the bucket hilt named, with the same expiry as the derived keys and the tenant. authorizeLocal reports authorized, denied or undecided: it refuses an action the cached set excludes with AccessDenied, and defers to hilt whenever no set is cached or the operation maps to no Forge command. Co-Authored-By: Claude Fable 5.1 --- docs/diagrams.md | 30 ++++- iam/service.go | 105 ++++++++++++----- iam/service_internal_test.go | 217 ++++++++++++++++++++++++++++++++--- iam/service_test.go | 62 ++++++++++ 4 files changed, 362 insertions(+), 52 deletions(-) diff --git a/docs/diagrams.md b/docs/diagrams.md index 47ac2c9c..a00f61a5 100644 --- a/docs/diagrams.md +++ b/docs/diagrams.md @@ -659,7 +659,7 @@ flowchart TB hilt["hilt did:web:hilt"] -->|"/s3/request/authorize and
/s3/bucket/info responses"| kp subgraph stores["proof stores"] - kp["iam.KeyProofs: one DelegationCache
per access key, 24h idle eviction"] + kp["iam.KeyProofs: one DelegationCache
per access key, 24h idle eviction;
also the key's effective S3 action set per bucket"] ship["uploader.Forge shipProofs:
per-space store, 1h TTL"] static["AuthServiceProofs:
static container from config"] tok["tokenstore (tokens.cbor):
empty; dormant login paths only"] @@ -684,6 +684,13 @@ flowchart TB re-capture it. - Each access key gets its own `DelegationCache`, so a proof chain can never assemble across keys. +- The same store holds the effective S3 action set hilt reported for the key + on each bucket, expiring with the keys and the tenant (next UTC midnight + plus clock skew). One invalidation therefore drops the chains and the set + together. The set is what the fast path enforces: several S3 actions map to + the same Forge commands, and `s3:PutObject`'s commands are a superset of + `s3:GetObject`'s, `s3:ListBucket`'s and `s3:AbortMultipartUpload`'s, so a + chain probe alone would let a put-only key read and abort on the bucket. Cross-references: [`architecture.md` §9](./architecture.md#9-the-system-contract-piri--sprue--indexer). @@ -717,11 +724,15 @@ sequenceDiagram G->>I: GetUserAccountForRequest I->>I: access key ID parsed as a did:key alt local fast path (authorizeLocal) - I->>K: cached derived key verifies SigV4,
every command chains to the agent + I->>K: cached derived key verifies SigV4,
cached action set permits the action,
every command chains to the agent + break action outside the cached set + I-->>G: AccessDenied, hilt not consulted + G-->>C: 403 + end else hilt authorize I->>H: /s3/request/authorize (the signed request) - H-->>I: account, derived SigV4 key, fresh delegations - I->>K: cacheProofs (re-delegations) + H-->>I: account, derived SigV4 key,
effective action set, fresh delegations + I->>K: cacheProofs (re-delegations)
+ the action set, keyed by the bucket hilt named opt chain incomplete I->>H: /s3/bucket/info I->>K: cache the bucket chain @@ -735,14 +746,21 @@ sequenceDiagram - `RoleAdmin` is deliberate: authorization already happened (at hilt or the fast path), so versitygw's role and ACL layers must defer entirely. +- The fast path has three outcomes, not two: authorized, refused, or + undecided. It refuses only on a cached action set that excludes the + action — hilt's own answer for that key and bucket, so re-asking would + repeat it. Everything else undecided (no cached set, an expired one, an + operation that maps to no Forge command) goes to hilt, whose response + refills the caches. - Authorize failures map to S3 errors in `mapAuthError`; unrecognized errors stay 500-class on purpose. - The signing key never leaves hilt as a secret: ingot receives a derived SigV4 key per request (the versitygw fork's `auth.Account.SigningKey`). Sources: `iam/service.go` (GetUserAccountForRequest, authorizeLocal, -cacheProofs, mapAuthError), `server.go` (buildS3API middleware). Review when -`iam/` or the hilt client changes. +cacheProofs, mapAuthError), `iam/proofcache.go` (PutPermissions, Permits), +`server.go` (buildS3API middleware). Review when `iam/` or the hilt client +changes. ## Postgres schema as migrated diff --git a/iam/service.go b/iam/service.go index aa6bfc50..cf36d148 100644 --- a/iam/service.go +++ b/iam/service.go @@ -27,14 +27,17 @@ // // A local fast path (the RFC's "local cache" authorization) avoids the Hilt // round-trip when everything needed is already cached, mirroring Hilt's own -// verification order: the request parses as HMAC SigV4 and a cached derived -// key verifies its signature (hilt/pkg/sigv4.VerifyWithKey) and time bounds; -// the request's S3 action maps to Forge commands (hilt/pkg/s3perm); and the -// key's own store holds a chain to this instance's agent for every such -// command (which, being the key's own store, necessarily carries the key's -// grant too). Anything less falls through to /s3/request/authorize, whose -// response replenishes the caches — so expiry (next UTC midnight, when SigV4 -// scope dates roll over) self-heals. +// verification and authorization order: the request parses as HMAC SigV4 and +// a cached derived key verifies its signature (hilt/pkg/sigv4.VerifyWithKey) +// and time bounds; the request's S3 action maps to Forge commands +// (hilt/pkg/s3perm); the key's cached effective action set for the addressed +// bucket contains the action; and the key's own store holds a chain to this +// instance's agent for every such command (which, being the key's own store, +// necessarily carries the key's grant too). Anything less falls through to +// /s3/request/authorize, whose response replenishes the caches — so expiry +// (next UTC midnight, when SigV4 scope dates roll over) self-heals. An action +// the cached set excludes is refused outright: the set is Hilt's own answer +// for this key and bucket, so re-asking would only repeat it. package iam import ( @@ -170,12 +173,20 @@ func (s *Service) GetUserAccountForRequest(ctx fiber.Ctx, accessKeyStr string) ( store := s.proofs.For(accessKeyID) ctx.Locals(reqscope.ProofStoreKey(), ucanlib.ProofStore(store)) - // Local fast path: with a cached verification key and cached delegation - // chains covering the request's Forge commands, Hilt is not consulted. - // The tenant travels with the request too (the write path encrypts to - // its wrap key); a verified request whose tenant has fallen out of the - // cache takes the Hilt path, whose response refills every cache. - if account, ok := s.authorizeLocal(reqCtx, req, accessKeyStr, store); ok { + // Local fast path: with a cached verification key, a cached action set + // permitting the request's S3 action and cached delegation chains + // covering its Forge commands, Hilt is not consulted. The tenant travels + // with the request too (the write path encrypts to its wrap key); a + // verified request whose tenant has fallen out of the cache takes the + // Hilt path, whose response refills every cache. + if account, ok, err := s.authorizeLocal(reqCtx, req, accessKeyStr, store); err != nil { + // A refusal off the cached action set: that set is Hilt's own answer + // for this key and bucket, so asking again would return the same + // refusal. Returned verbatim (not wrapped) so versitygw's renderer + // type-asserts it, matching how mapAuthError returns Hilt's + // OperationNotPermitted. + return auth.Account{}, err + } else if ok { if tenant, found := s.tenants.Get(accessKeyStr); found { ctx.Locals(reqscope.TenantKey(), tenant) return account, nil @@ -224,6 +235,13 @@ func (s *Service) GetUserAccountForRequest(ctx fiber.Ctx, accessKeyStr string) ( // and stashed on this request for the write path. s.tenants.Put(accessKeyStr, ttl, ok.Tenant) ctx.Locals(reqscope.TenantKey(), ok.Tenant) + // The key's effective action set on this bucket is cached to the same + // horizon, in the key's own proof store so a revocation drops the set + // with the chains. Hilt names the bucket; a result without one + // (ListBuckets, CreateBucket) scopes no set. + if ok.Bucket != nil { + store.PutPermissions(*ok.Bucket, ttl, ok.Permissions.Entries[accessKeyID]) + } // Bucket is nil for bucket-level operations (CreateBucket, ListBuckets), // which authorize without addressing an existing bucket. @@ -293,53 +311,80 @@ func mapAuthError(err error) (error, bool) { } } -// authorizeLocal is the fast path, mirroring Hilt's own verification -// order over THIS key's proof store. It reports ok=false whenever anything -// needed isn't cached or doesn't check out; the caller then takes the Hilt -// path, whose response replenishes the caches. -func (s *Service) authorizeLocal(ctx context.Context, req s3.Request, access string, store *DelegationCache) (auth.Account, bool) { +// authorizeLocal is the fast path, mirroring Hilt's own verification and +// authorization order over THIS key's proof store: signature, then the +// effective action set, then the proof chains. It reports ok=true when the +// request checks out end to end. A non-nil error is a refusal: the key's +// cached action set for this bucket excludes the request's action, and that +// set is Hilt's own answer, so the caller refuses rather than asking again. +// Both zero means the fast path could not decide — something it needs isn't +// cached or doesn't check out — and the caller takes the Hilt path, whose +// response replenishes the caches. +// +// The action set is what makes the chain probe safe to trust. Several S3 +// actions share Forge commands and s3:PutObject's command set is a superset +// of s3:GetObject's, s3:ListBucket's and s3:AbortMultipartUpload's, so a +// probe alone would let a put-only key read and abort on the same bucket. +func (s *Service) authorizeLocal(ctx context.Context, req s3.Request, access string, store *DelegationCache) (auth.Account, bool, error) { if s.buckets == nil || !s.agent.Defined() { - return auth.Account{}, false + return auth.Account{}, false, nil } // 1. Parse the signature envelope. Only HMAC SigV4 is fast-pathed: the // gateway re-verifies with the returned key and only speaks HMAC. sr, err := sigv4.Parse(sigv4.Request{Method: req.Method, Headers: req.Headers, URL: req.URL}) if err != nil || sr.Scheme != sigv4.SchemeV4 { - return auth.Account{}, false + return auth.Account{}, false, nil } // 2. Cached key verifies the signature and the request is in time // bounds. A scope-rolled or rotated key simply fails here. key, ok := s.keys.Get(access, s3.KeyKindSigV4) if !ok { - return auth.Account{}, false + return auth.Account{}, false, nil } if sigv4.VerifyWithKey(sr, key) != nil { - return auth.Account{}, false + return auth.Account{}, false, nil } if sigv4.ValidateTimeBounds(sr, time.Now()) != nil { - return auth.Account{}, false + return auth.Account{}, false, nil } // 3. The S3 action must map to Forge commands. Bucket-level operations // (create/delete/list-buckets) map to none — they go to Hilt regardless. op, err := hiltauth.OperationFor(req) if err != nil { - return auth.Account{}, false + return auth.Account{}, false, nil } cmds := s3perm.CommandsFor(op.Permission()) if len(cmds) == 0 { - return auth.Account{}, false + return auth.Account{}, false, nil } // 4. The delegation subject is the bucket's space. st, err := s.buckets.Get(ctx, bucketFromURL(req.URL)) if err != nil || !st.Space.Defined() { - return auth.Account{}, false + return auth.Account{}, false, nil + } + + // 5. The key's cached effective action set for this bucket decides + // whether the action is permitted at all. No set cached means Hilt has + // not answered for this bucket (or the answer expired, or the local + // registry's space disagrees with the bucket Hilt named) — ask Hilt. A + // cached set that excludes the action is a refusal. + if allowed, known := store.Permits(st.Space, op.Permission()); !known { + return auth.Account{}, false, nil + } else if !allowed { + s.logger.Debug("hilt/iam: request denied by cached action set", + zap.String("access", access), + zap.String("operation", op.String()), + zap.String("action", op.Permission()), + zap.Stringer("bucket", st.Space), + ) + return auth.Account{}, false, s3err.GetAPIError(s3err.ErrAccessDenied) } - // 5. Every command must be covered by a chain to this instance's agent + // 6. Every command must be covered by a chain to this instance's agent // in THIS key's store. Because the store holds only keyDID's delegations, // a resolving agent chain is necessarily space→…→keyDID→agent — it // carries both keyDID's own grant (Hilt's permission + bucket scoping) @@ -347,7 +392,7 @@ func (s *Service) authorizeLocal(ctx context.Context, req s3.Request, access str // impossible, so one probe suffices. for _, cmd := range cmds { if chain, _, err := store.ProofChain(ctx, s.agent, cmd, st.Space); err != nil || len(chain) == 0 { - return auth.Account{}, false + return auth.Account{}, false, nil } } @@ -364,7 +409,7 @@ func (s *Service) authorizeLocal(ctx context.Context, req s3.Request, access str // ownership for hilt-managed keys (buckets default to root-owned) and // its admin APIs are not mounted. Role: auth.RoleAdmin, - }, true + }, true, nil } // untilNextUTCMidnight is the cache TTL horizon for verification keys, diff --git a/iam/service_internal_test.go b/iam/service_internal_test.go index 6d2b4b90..369ca334 100644 --- a/iam/service_internal_test.go +++ b/iam/service_internal_test.go @@ -17,6 +17,7 @@ import ( hiltclient "github.com/fil-forge/hilt/pkg/client" hiltauth "github.com/fil-forge/hilt/pkg/rpc/service/auth" + "github.com/fil-forge/hilt/pkg/s3perm" "github.com/fil-forge/hilt/pkg/sigv4" "github.com/fil-forge/ingot/internal/reqscope" "github.com/fil-forge/ingot/registry" @@ -37,7 +38,15 @@ import ( // sigv4.VerifyWithKey accepts it. func signedGet(t *testing.T, host, rawPath, accessKeyID, secret string, when time.Time) s3.Request { t.Helper() - req, err := http.NewRequest(http.MethodGet, "http://"+host+rawPath, nil) + return signedRequest(t, http.MethodGet, host, rawPath, accessKeyID, secret, when) +} + +// signedRequest is signedGet for any method, so the operation classifier sees +// the shapes it distinguishes (a DELETE with ?uploadId is an abort, not an +// object delete). +func signedRequest(t *testing.T, method, host, rawPath, accessKeyID, secret string, when time.Time) s3.Request { + t.Helper() + req, err := http.NewRequest(method, "http://"+host+rawPath, nil) require.NoError(t, err) req.Header.Set("X-Amz-Content-Sha256", "UNSIGNED-PAYLOAD") @@ -52,7 +61,7 @@ func signedGet(t *testing.T, host, rawPath, accessKeyID, secret string, when tim for k := range req.Header { headers[k] = req.Header.Get(k) } - return s3.Request{Method: http.MethodGet, Headers: headers, URL: req.URL.RequestURI()} + return s3.Request{Method: method, Headers: headers, URL: req.URL.RequestURI()} } // fixedResolver returns one bucket→space mapping (or not-found for others). @@ -71,17 +80,30 @@ func (r fixedResolver) Get(_ context.Context, name string) (*registry.State, err // retrieveChain mints the RFC chain space→tenant→accessKey→agent for // /content/retrieve, all subject = space; returns the delegations. func retrieveChain(t *testing.T, spaceIssuer, accessKey, agent ucan.Issuer) []ucan.Delegation { + t.Helper() + return chainFor(t, spaceIssuer, accessKey, agent, contentcmds.Retrieve.Command) +} + +// chainFor mints the RFC chain space→tenant→accessKey→agent for each command, +// all subject = space, through one tenant. It stands in for the delegations +// Hilt issues when it grants an access key a permission: the whole command set +// that permission maps to (see hilt/pkg/s3perm). +func chainFor(t *testing.T, spaceIssuer, accessKey, agent ucan.Issuer, cmds ...ucan.Command) []ucan.Delegation { t.Helper() space := spaceIssuer.DID() tenant, err := ed25519.GenerateIssuer() require.NoError(t, err) - root, err := contentcmds.Retrieve.Delegate(spaceIssuer, tenant.DID(), space, delegation.WithNoExpiration()) - require.NoError(t, err) - mid, err := contentcmds.Retrieve.Delegate(tenant, accessKey.DID(), space, delegation.WithNoExpiration()) - require.NoError(t, err) - leaf, err := contentcmds.Retrieve.Delegate(accessKey, agent.DID(), space, delegation.WithNoExpiration()) - require.NoError(t, err) - return []ucan.Delegation{root, mid, leaf} + var dlgs []ucan.Delegation + for _, cmd := range cmds { + root, err := delegation.Delegate(spaceIssuer, tenant.DID(), space, cmd, delegation.WithNoExpiration()) + require.NoError(t, err) + mid, err := delegation.Delegate(tenant, accessKey.DID(), space, cmd, delegation.WithNoExpiration()) + require.NoError(t, err) + leaf, err := delegation.Delegate(accessKey, agent.DID(), space, cmd, delegation.WithNoExpiration()) + require.NoError(t, err) + dlgs = append(dlgs, root, mid, leaf) + } + return dlgs } // localService builds a Service with the fast path enabled and the given @@ -115,17 +137,29 @@ func TestAuthorizeLocal(t *testing.T) { // authLocal mirrors GetUserAccountForRequest: resolve the requesting // key's own store and hand it to the fast path. - authLocal := func(s *Service) (auth.Account, bool) { + authLocal := func(s *Service) (auth.Account, bool, error) { store := s.proofs.For(accessKey.DID()) return s.authorizeLocal(context.Background(), req, accessKeyID, store) } + // permit caches the effective action set Hilt would have returned for + // this key on the bucket. Without it the fast path has no answer and + // defers to Hilt, so every case that expects a decision seeds one. + permit := func(s *Service, actions ...string) { + if actions == nil { + actions = []string{} // an answer of "nothing", not the absence of one + } + s.proofs.For(accessKey.DID()).PutPermissions(space, time.Hour, actions) + } + t.Run("all cached: authorized locally", func(t *testing.T) { s := localService(agent.DID(), resolver) s.keys.Put(accessKeyID, time.Hour, s3.VerificationKey{Kind: s3.KeyKindSigV4, Data: key}) s.proofs.Deposit(accessKey.DID(), retrieveChain(t, spaceIssuer, accessKey, agent)...) + permit(s, "s3:GetObject") - acct, ok := authLocal(s) + acct, ok, err := authLocal(s) + require.NoError(t, err) require.True(t, ok, "should authorize without Hilt") require.Equal(t, accessKeyID, acct.Access) require.Equal(t, key, acct.SigningKey) @@ -134,7 +168,9 @@ func TestAuthorizeLocal(t *testing.T) { t.Run("no cached key: fall through", func(t *testing.T) { s := localService(agent.DID(), resolver) s.proofs.Deposit(accessKey.DID(), retrieveChain(t, spaceIssuer, accessKey, agent)...) - _, ok := authLocal(s) + permit(s, "s3:GetObject") + _, ok, err := authLocal(s) + require.NoError(t, err) require.False(t, ok) }) @@ -142,15 +178,19 @@ func TestAuthorizeLocal(t *testing.T) { s := localService(agent.DID(), resolver) s.keys.Put(accessKeyID, time.Hour, s3.VerificationKey{Kind: s3.KeyKindSigV4, Data: []byte("wrong-key-32-bytes-......xxxxxxx")}) s.proofs.Deposit(accessKey.DID(), retrieveChain(t, spaceIssuer, accessKey, agent)...) - _, ok := authLocal(s) + permit(s, "s3:GetObject") + _, ok, err := authLocal(s) + require.NoError(t, err) require.False(t, ok) }) t.Run("missing chain: fall through", func(t *testing.T) { s := localService(agent.DID(), resolver) s.keys.Put(accessKeyID, time.Hour, s3.VerificationKey{Kind: s3.KeyKindSigV4, Data: key}) + permit(s, "s3:GetObject") // No delegations cached for this key. - _, ok := authLocal(s) + _, ok, err := authLocal(s) + require.NoError(t, err) require.False(t, ok) }) @@ -158,8 +198,46 @@ func TestAuthorizeLocal(t *testing.T) { s := localService(agent.DID(), fixedResolver{name: "other"}) s.keys.Put(accessKeyID, time.Hour, s3.VerificationKey{Kind: s3.KeyKindSigV4, Data: key}) s.proofs.Deposit(accessKey.DID(), retrieveChain(t, spaceIssuer, accessKey, agent)...) - _, ok := authLocal(s) + permit(s, "s3:GetObject") + _, ok, err := authLocal(s) + require.NoError(t, err) + require.False(t, ok) + }) + + t.Run("no cached action set: fall through", func(t *testing.T) { + // Everything else checks out; without Hilt's answer on what this + // key may do here, the request goes to Hilt rather than through. + s := localService(agent.DID(), resolver) + s.keys.Put(accessKeyID, time.Hour, s3.VerificationKey{Kind: s3.KeyKindSigV4, Data: key}) + s.proofs.Deposit(accessKey.DID(), retrieveChain(t, spaceIssuer, accessKey, agent)...) + + _, ok, err := authLocal(s) + require.NoError(t, err) + require.False(t, ok) + }) + + t.Run("empty action set: denied", func(t *testing.T) { + // A principal whose policy grants nothing on this bucket. The empty + // set is an answer, so the request is refused, not re-asked. + s := localService(agent.DID(), resolver) + s.keys.Put(accessKeyID, time.Hour, s3.VerificationKey{Kind: s3.KeyKindSigV4, Data: key}) + s.proofs.Deposit(accessKey.DID(), retrieveChain(t, spaceIssuer, accessKey, agent)...) + permit(s) + + _, ok, err := authLocal(s) + require.False(t, ok) + require.Equal(t, s3err.GetAPIError(s3err.ErrAccessDenied), err) + }) + + t.Run("action outside the set: denied", func(t *testing.T) { + s := localService(agent.DID(), resolver) + s.keys.Put(accessKeyID, time.Hour, s3.VerificationKey{Kind: s3.KeyKindSigV4, Data: key}) + s.proofs.Deposit(accessKey.DID(), retrieveChain(t, spaceIssuer, accessKey, agent)...) + permit(s, "s3:DeleteObject") + + _, ok, err := authLocal(s) require.False(t, ok) + require.Equal(t, s3err.GetAPIError(s3err.ErrAccessDenied), err) }) // Isolation: a full chain deposited under a DIFFERENT access key (even @@ -172,12 +250,94 @@ func TestAuthorizeLocal(t *testing.T) { s := localService(agent.DID(), resolver) s.keys.Put(accessKeyID, time.Hour, s3.VerificationKey{Kind: s3.KeyKindSigV4, Data: key}) s.proofs.Deposit(otherKey.DID(), retrieveChain(t, spaceIssuer, otherKey, agent)...) + permit(s, "s3:GetObject") - _, ok := authLocal(s) + _, ok, err := authLocal(s) + require.NoError(t, err) require.False(t, ok, "must not see another key's store") }) } +// TestPutOnlyKeyIsDeniedOnReads pins the escalation the action set closes. +// s3perm maps s3:PutObject to a command set that contains every command +// s3:GetObject, s3:ListBucket and s3:AbortMultipartUpload need, so a key +// granted only s3:PutObject satisfies the fast path's chain probe for all +// three. The cached action set is what refuses them. +func TestPutOnlyKeyIsDeniedOnReads(t *testing.T) { + accessKey, err := ed25519.GenerateIssuer() + require.NoError(t, err) + spaceIssuer, err := ed25519.GenerateIssuer() + require.NoError(t, err) + agent, err := ed25519.GenerateIssuer() + require.NoError(t, err) + + accessKeyID := accessKey.DID().Identifier() + const secret = "test-secret-access-key" + space := spaceIssuer.DID() + resolver := fixedResolver{name: "bkt", state: ®istry.State{Name: "bkt", Space: space}} + + // The delegations Hilt issues for a key granted only s3:PutObject. + putCommands := s3perm.CommandsFor("s3:PutObject") + require.NotEmpty(t, putCommands) + putChains := chainFor(t, spaceIssuer, accessKey, agent, putCommands...) + + // authorize builds a service holding those chains, the derived key for + // the request, and the put-only action set, then runs the fast path. + authorize := func(t *testing.T, req s3.Request) (bool, error) { + t.Helper() + sr, err := sigv4.Parse(sigv4.Request{Method: req.Method, Headers: req.Headers, URL: req.URL}) + require.NoError(t, err) + key, err := sigv4.DeriveKey(sr, secret) + require.NoError(t, err) + + s := localService(agent.DID(), resolver) + s.keys.Put(accessKeyID, time.Hour, s3.VerificationKey{Kind: s3.KeyKindSigV4, Data: key}) + s.proofs.Deposit(accessKey.DID(), putChains...) + s.proofs.For(accessKey.DID()).PutPermissions(space, time.Hour, []string{"s3:PutObject"}) + + _, ok, err := s.authorizeLocal(context.Background(), req, accessKeyID, s.proofs.For(accessKey.DID())) + return ok, err + } + + now := time.Now() + denied := map[string]s3.Request{ + "GetObject": signedRequest(t, http.MethodGet, "s3.example", "/bkt/obj", accessKeyID, secret, now), + "ListBucket": signedRequest(t, http.MethodGet, "s3.example", "/bkt", accessKeyID, secret, now), + "AbortMultipartUpload": signedRequest(t, http.MethodDelete, "s3.example", "/bkt/obj?uploadId=abc", accessKeyID, secret, now), + } + for name, req := range denied { + t.Run(name+" is denied", func(t *testing.T) { + // The chains alone would let it through: the operation's + // commands are a subset of s3:PutObject's. + op, err := hiltauth.OperationFor(req) + require.NoError(t, err) + require.Equal(t, name, op.String()) + require.Subset(t, commandStrings(putCommands), commandStrings(s3perm.CommandsFor(op.Permission())), + "the defect only exists while the commands are a subset") + + ok, err := authorize(t, req) + require.False(t, ok) + require.Equal(t, s3err.GetAPIError(s3err.ErrAccessDenied), err) + }) + } + + t.Run("PutObject is still authorized", func(t *testing.T) { + req := signedRequest(t, http.MethodPut, "s3.example", "/bkt/obj", accessKeyID, secret, now) + ok, err := authorize(t, req) + require.NoError(t, err) + require.True(t, ok) + }) +} + +// commandStrings renders commands for set comparison in assertions. +func commandStrings(cmds []ucan.Command) []string { + out := make([]string, 0, len(cmds)) + for _, c := range cmds { + out = append(out, c.String()) + } + return out +} + // hiltErr reproduces the wrapping depth a real authorize failure arrives with: // binding.Unpack wraps the decoded ErrorModel, the Hilt client wraps that, and // GetUserAccountForRequest wraps once more — mapAuthError must see through all @@ -312,11 +472,18 @@ func TestFastPathTenant(t *testing.T) { return acct, stashed, authErr } + // permit caches the effective action set for this key on the bucket, as + // an authorize response would. + permit := func(s *Service, actions ...string) { + s.proofs.For(accessKey.DID()).PutPermissions(spaceIssuer.DID(), time.Hour, actions) + } + t.Run("cached tenant is stashed without consulting Hilt", func(t *testing.T) { s := localService(agent.DID(), resolver) s.keys.Put(accessKeyID, time.Hour, s3.VerificationKey{Kind: s3.KeyKindSigV4, Data: key}) s.tenants.Put(accessKeyID, time.Hour, tenant) s.proofs.Deposit(accessKey.DID(), retrieveChain(t, spaceIssuer, accessKey, agent)...) + permit(s, "s3:GetObject") acct, stashed, err := drive(t, s) require.NoError(t, err) @@ -329,9 +496,27 @@ func TestFastPathTenant(t *testing.T) { s := localService(agent.DID(), resolver) s.keys.Put(accessKeyID, time.Hour, s3.VerificationKey{Kind: s3.KeyKindSigV4, Data: key}) s.proofs.Deposit(accessKey.DID(), retrieveChain(t, spaceIssuer, accessKey, agent)...) + permit(s, "s3:GetObject") _, _, err := drive(t, s) require.ErrorContains(t, err, "hilt consulted") require.Equal(t, 1, s.authorizer.(*refusingAuthorizer).calls) }) + + t.Run("action outside the cached set is refused without consulting Hilt", func(t *testing.T) { + s := localService(agent.DID(), resolver) + s.keys.Put(accessKeyID, time.Hour, s3.VerificationKey{Kind: s3.KeyKindSigV4, Data: key}) + s.tenants.Put(accessKeyID, time.Hour, tenant) + s.proofs.Deposit(accessKey.DID(), retrieveChain(t, spaceIssuer, accessKey, agent)...) + permit(s, "s3:PutObject") + + _, stashed, err := drive(t, s) + // s3err.APIError arrives verbatim so versitygw renders AccessDenied. + var s3e s3err.S3Error + require.ErrorAs(t, err, &s3e) + require.Equal(t, "AccessDenied", s3e.BaseError().Code) + require.Equal(t, 403, s3e.StatusCode()) + require.Zero(t, s.authorizer.(*refusingAuthorizer).calls, "a cached refusal must not re-ask Hilt") + require.Nil(t, stashed, "a refused request must not carry a tenant onward") + }) } diff --git a/iam/service_test.go b/iam/service_test.go index 5c8ac22d..360a6ca8 100644 --- a/iam/service_test.go +++ b/iam/service_test.go @@ -249,6 +249,68 @@ func TestProofChainCapture(t *testing.T) { }) } +// TestPermissionCapture covers the other half of enforcement: the effective +// action set an authorize response carries is cached in the key's own proof +// store, keyed by the bucket Hilt named, so the fast path can decide the +// key's next request without another round trip. +func TestPermissionCapture(t *testing.T) { + access, keyDID := newAccessKey(t) + sigv4 := s3.VerificationKey{Kind: s3.KeyKindSigV4, Data: []byte("dk")} + + t.Run("the effective set is cached under the bucket", func(t *testing.T) { + res := authorizeOK(t, keyDID, sigv4) + res.Permissions = s3.PermissionSet{Entries: map[did.DID][]string{keyDID: {"s3:PutObject"}}} + proofs := iam.NewKeyProofs() + svc := iam.New(&fakeAuthorizer{res: res}, proofs, iam.NewVerificationKeyCache(), iam.NewTenantCache()) + + _, err := resolveForRequest(t, svc, access, + httptest.NewRequest(http.MethodPut, "http://example.com/bkt/key", nil)) + require.NoError(t, err) + + store := proofs.For(keyDID) + allowed, known := store.Permits(*res.Bucket, "s3:PutObject") + require.True(t, known, "the authorize answer must be cached") + require.True(t, allowed) + + allowed, known = store.Permits(*res.Bucket, "s3:GetObject") + require.True(t, known) + require.False(t, allowed, "an action the key does not hold must be denied from cache") + }) + + t.Run("a result with no bucket caches no set", func(t *testing.T) { + // ListBuckets and CreateBucket address no existing bucket, so there + // is nothing to scope a set to. + res := authorizeOK(t, keyDID, sigv4) + bucket := *res.Bucket + res.Bucket = nil + proofs := iam.NewKeyProofs() + svc := iam.New(&fakeAuthorizer{res: res}, proofs, iam.NewVerificationKeyCache(), iam.NewTenantCache()) + + _, err := resolveForRequest(t, svc, access, + httptest.NewRequest(http.MethodGet, "http://example.com/", nil)) + require.NoError(t, err) + + _, known := proofs.For(keyDID).Permits(bucket, "s3:GetObject") + require.False(t, known) + }) + + t.Run("a result with no permissions for the key caches no set", func(t *testing.T) { + // A Hilt that reports nothing leaves the bucket unknown, so requests + // keep going to Hilt instead of being refused locally. + res := authorizeOK(t, keyDID, sigv4) + res.Permissions = s3.PermissionSet{} + proofs := iam.NewKeyProofs() + svc := iam.New(&fakeAuthorizer{res: res}, proofs, iam.NewVerificationKeyCache(), iam.NewTenantCache()) + + _, err := resolveForRequest(t, svc, access, + httptest.NewRequest(http.MethodGet, "http://example.com/bkt/key", nil)) + require.NoError(t, err) + + _, known := proofs.For(keyDID).Permits(*res.Bucket, "s3:GetObject") + require.False(t, known) + }) +} + // TestBaseIAMServiceParity pins the non-request IAMService surface to // IAMServiceSingle's behavior: account management belongs to Hilt. func TestBaseIAMServiceParity(t *testing.T) {