amadan
The principal inventory becomes a report: one listing cannot support the ledger's claim, and it no longer offers to
Astra's first review rejected d61f4f27 on three P1s, each right: a listing of an eventually consistent, paged account is not an instant; a role set aside as the platform's sets aside everyone its trust policy lets become it; and an absent bucket is not a bucket this grant creates. None is fixed by reading more carefully. - The inventory is PrincipalReport.BucketPrincipals: a report for a person of which users, roles and groups reach a name, through which policies, and who each role's trust policy lets assume it. It is not a GranteeProber, and a test says so. The principals a deployment names as its own are a label in the report and exempt nothing. - The same principal, managed policy or inline policy name returned twice is an error, whichever copy reaches (P2). - The identity-policy grammar is checked before anything is evaluated: a null or empty Statement, an empty or null value, a Principal, an unknown element or version is an error, not a statement that reaches nothing (P2). - The pagination refusals are asserted by their reason and their call count, and the page bound against a page that must not be asked for (P3). The note says what a completeness claim needs instead (a rule the account enforces, and a bucket only its own grant could have made), as proposals for the owner. Forty mutants in a tree copy; one is equivalent. Nothing was run against AWS; nothing is wired.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
5 files changed,
+619
−382
docs/superpowers/plans/2026-10-02-aws-principal-inventory.md+30 −32internal/objstore/principals.go+358 −236internal/objstore/principals_export_test.go+9 −0internal/objstore/principals_test.go+213 −107internal/objstore/provisioner.go+9 −7
CI passed — run details
diff --git a/docs/superpowers/plans/2026-10-02-aws-principal-inventory.md b/docs/superpowers/plans/2026-10-02-aws-principal-inventory.md| index a9ce70afb..b2565f663 100644 |
| --- a/docs/superpowers/plans/2026-10-02-aws-principal-inventory.md |
| +++ b/docs/superpowers/plans/2026-10-02-aws-principal-inventory.md |
| @@ -1,54 +1,52 @@ |
| -# The AWS principal inventory |
| +# The AWS principal report, and what a completeness claim needs |
| -Date: 2026-10-02. Status: built on `feat/carloku-principal-inventory-20261002`, **unwired**: no console constructs it, and nothing in production behaves differently. It is the first step of `2026-10-01-bucket-ownership-attestation.md` and of the store-transfer meter (`OUTSTANDING.md` item 2). It needs one decision and one permission before it can be switched on, both below. Nothing was run against AWS. |
| +Date: 2026-10-02. Revised the same day after its first review (Astra: reject; three P1s, two P2s, one P3; the Opus review has not run). Status: a report for a person, built on `feat/carloku-principal-inventory-20261002`. **It is not wired, and it is no longer offered as the proof a bucket's ledger needs.** Nothing was run against AWS. |
| -## What it is for |
| +## Why this exists |
| -A bucket's ownership ledger may claim it names everyone who ever had the bucket only when the grant that minted the bucket could show two things: no bucket held the name, and no principal anywhere had a policy reaching the name (`objstore.GranteeProber`). The second needs a list of principals. The AWS provisioner had none, so on AWS no ledger claims completeness, no charged store transfer can be attributed, and the month holds. |
| +Object-store transfer is billed to a tenant only if the bucket's ownership ledger can claim it names everyone who could ever reach the bucket (`objstore.GranteeProber`, `OwnerAt`). On AWS nothing can make that claim today, so charged store transfer holds the month (`OUTSTANDING.md` item 2). |
| -## What is built |
| +The first build of this branch tried to make the claim by reading the account: list every principal, evaluate every policy, and call an empty answer "nobody else". The review showed that an empty answer from one listing is not that claim, for three reasons that no amount of care in the reading removes: |
| -`internal/objstore/principals.go`. One listing of the account (`iam:GetAccountAuthorizationDetails`, every page), every policy document decoded and evaluated here against one bucket name. |
| +1. **A listing is not an instant.** IAM is eventually consistent: a policy attached before the listing began can be missing from it. The listing is paged: a principal on page one can be given access before the last page is read. Nothing in the answer says it is complete. |
| +2. **A principal's policy is not everyone who can act as it.** A role can be assumed by whoever its trust policy names, another account included, and by whoever may pass it to a service. Setting a role aside as "the platform's" sets aside everyone who can become it. |
| +3. **An absent bucket is not a bucket this grant creates.** Another actor in the account can create the name, with a policy of its own, between any probe and the walk's own create; AWS answers the walk's create as a success. |
| -- **Covers:** users, roles and groups; inline and attached managed policies (the default version); a user's groups; resource patterns with `*`, `?` and policy variables; the bucket ARN and any object ARN under it; `NotAction` and `NotResource`. |
| -- **Over-reports, never under-reports.** A Deny, a condition, a permissions boundary or an organization policy only narrows access, and none is evaluated: a principal they keep out is still listed. A listed principal costs a claim. An omitted one would bill a tenant for bytes somebody else could move. |
| -- **Errors rather than guesses.** A document it cannot decode, an effect or statement shape it does not know, a managed policy attached and missing from the listing, a truncated or endless listing, a refused listing. An error claims nothing. |
| -- **Answers only for a name no bucket holds.** A bucket's own policy, its ACL, and access points exist only while the bucket does, and none is read. For a name a bucket holds the inventory returns an error. |
| -- **Inert by default.** The plain AWS provisioner is unchanged and is not a `GranteeProber`. The inventory is a separate constructor (`NewAWSProvisionerWithInventory`) that nothing calls. |
| +So the branch now does less, and says so. |
| -Tests: `go test ./internal/objstore -run 'TestPolicyPatterns|TestInventory|TestPlatformPrincipals|TestThePlainAWS'`, against a stub of the IAM listing. No test reads a real account. |
| +## What is built |
| -Mutation checks, in a tree copy, twenty-six: `NotResource` and `NotAction` not reaching; actions compared with case; a policy variable taken literally; object ARNs, and the bucket ARN, not matched; the empty key counted; a missing managed policy skipped; group membership ignored; the platform counted among grantees; a name a bucket holds answered for; only the first page read; any version of a managed policy taken as the default; an unknown effect passed over quietly; a pattern accepted as a platform principal; a question mark taken literally; attached policies not read; the listing filtered; roles not evaluated; a reaching group not listed; an Allow with no resource passed over quietly; the partition fixed; a failed bucket probe read as no bucket; a service pattern compared as text; a plus sign read as a space; the page limit not enforced. One survived the first run (the partition was never derived in a test) and has a test now. The page-limit mutant dies by the test never ending, not by an assertion. |
| +`internal/objstore/principals.go`: `PrincipalReport.BucketPrincipals(name)`. One listing of the account (`iam:GetAccountAuthorizationDetails`, every page), every identity policy decoded and evaluated against one bucket name, and a report: which users, roles and groups reach the name, through which policies, and for each role, who its trust policy lets assume it. Principals the deployment names as its own are set apart in the report. That is a label for the reader and exempts nothing. |
| -## What it does not do |
| +- It is **not** a `GranteeProber`, and a test says so. The grant walk on AWS claims nothing, as before. |
| +- Within the documents it is given it leans toward listing: a Deny, a condition or a boundary is not evaluated; `NotAction`, `NotResource` and policy variables are taken to reach. |
| +- What it cannot read is an error, never a shorter list: an undecodable document; a statement, element or version outside the identity-policy grammar (a null or empty `Statement`, an empty or null value, a `Principal`, an unknown element); the same principal, managed policy or inline policy name returned twice; an attached policy the listing did not return; a truncated or endless listing. |
| +- It does not read: the root user; a bucket's own policy, ACL or access points; grants through other services' resources (access points, Object Lambda, Batch Operations, replication jobs); who inside the account may assume a role (`sts:AssumeRole` in their own policies); anything in another account. |
| -- **Existing and adopted buckets.** The four stores already on the flagship, and any adopted bucket, need the resource side (bucket policy, ACL, access points, grants to other accounts) and the attestation door. Not built. |
| -- **Anything after the instant of the listing.** IAM is eventually consistent, and a policy attached by hand later is not known to the ledger. The claim is dated. Nothing re-checks it. Whether a periodic re-check should exist, and what a new grantee found later does to months already billed, is not designed. |
| -- **The root user.** No policy lists it. |
| -- **A real listing.** The shape of a real account's documents (encodings, service-linked roles, Identity Center roles, AWS managed policies with constructs this parser refuses) is unverified. The first real run may error, which holds as today. |
| +Use: a person preparing the attestation of the four existing stores, or checking a deployment, can see who reaches a store name today. It needs `iam:GetAccountAuthorizationDetails`, which the console's role does not have and this branch does not add; it can be run with an operator's own credentials. |
| -## Decision for Paul: which principals are the platform's own |
| +Tests: `go test ./internal/objstore -run 'TestPolicyPatterns|TestInventory|TestPlatformPrincipals|TestNothingOnAWS'`, against a stub of the listing. No test reads a real account, and the stub is built from SDK structs, not from IAM's wire format. |
| -In a real account some principals reach every store bucket name. The host role provisions them (`CreateBucket`, `DeleteBucket`, `ListBucket`, `DeleteObject` on the store prefix; no `GetObject`). `OrganizationAccountAccessRole` and any administrator role reach everything. Counted as grantees, they make every name "already granted", and no ledger ever claims completeness. |
| +Mutation checks, in a tree copy, forty. Thirty-nine die. One is equivalent: dropping the check that a default version has a document changes nothing, because an absent document then fails to decode. One mutant did not compile as first written and was rerun. Output: `inventory-mutants-round1.txt` in the run directory's logs. |
| -The inventory therefore takes a list of principals declared the platform's own, by exact ARN (no patterns). They are reported apart and are not grantees. **The list is empty and nothing sets it.** |
| +## What a completeness claim needs instead (not built; needs the owner) |
| -The question is not technical. AWS charges transfer by bucket and the report does not say who moved the bytes. If an administrator downloads from a tenant's bucket, that transfer is on the tenant's bucket and would be billed to the tenant. |
| +Looking cannot establish it. A rule the account enforces can. Two parts, each a design question: |
| -Proposed, for review: |
| +**A. Nobody else can be granted a store bucket.** Proposed for review: an organization policy (or, failing that, a permissions boundary required on every principal the platform creates plus an account rule) that denies S3 actions on the store bucket prefix to every principal except the store users under the store path and a short list named by ARN. With that in force, "who can reach this bucket" is the store user and the named list by construction, and the report above becomes the check that the rule is in place, not the proof. Who is on the named list, and who can assume each role on it, is decision 14. |
| -1. Declared by ARN in the deployment's configuration, and written into each ledger's evidence so a statement can show what the claim rested on. |
| -2. Only two kinds may be declared: the deployment's own host role, and roles only a person with account administration can assume. |
| -3. Staff do not read tenant store buckets. If support ever requires it, the bytes are small and the platform absorbs them by credit, case by case. |
| +**B. The bucket is the one this grant made.** Proposed for review: the grant mints a bucket name with a random part recorded in its grant block before any provider call, so no other actor can know the name to create it first; and the walk refuses a bucket that already exists under that name for any reason, including AWS's "already owned by you". |
| -Without an answer the inventory stays unwired. |
| +Neither is designed in detail, and both change infrastructure or the grant walk, which need their own review. |
| -## Approval needed: one IAM permission |
| +## Decision for Paul (item 14, reworded) |
| -`iam:GetAccountAuthorizationDetails` on the console's role, account 271376211898. It reads every policy document in the account. The host module today deliberately withholds policy reads from that role (`infra/modules/region-host/main.tf`, the inventory page's grant). This is a wider read than anything the console has. The module is **not changed** on this branch. |
| +Which principals may reach tenant store buckets besides the tenant's own store user, and who can act as each. Today the host role (it creates and deletes store buckets; it cannot read objects) and any administrator role reach every store bucket. AWS charges transfer by bucket and does not say who moved the bytes: if staff download from a tenant's bucket, that transfer lands on the tenant's bill. |
| -Alternative: run the listing from an operator's own credentials at attestation time, through a verb, and hand the console the result. The console's role gains nothing, at the cost of a person in the loop for every new store. Not built. |
| +Proposed: only the deployment's host role and roles that need account administration to assume; named one by one, by ARN, with each role's trust policy part of the record; staff do not read tenant buckets, and if support ever requires it the platform credits the bytes. And approve the direction in A and B above, or say what should stand in their place. |
| -## After both |
| +## Before anything here is wired |
| -Wire the constructor in `cmd/console` behind the declaration, run it read-only against the flagship, and compare its answer for one new name with the account by hand. Then the resource side and the attestation door for existing buckets. |
| +1. The owner's answers to item 14 and to A and B. |
| +2. A real listing, read by a person: encodings, service-linked roles, Identity Center roles, AWS managed policies with constructs this parser refuses. The first real run may error. |
| +3. The resource side for existing buckets (bucket policy, ACL, access points) and the attestation door. |
diff --git a/internal/objstore/principals.go b/internal/objstore/principals.go| index b7ebbd6af..b41ce9ef1 100644 |
| --- a/internal/objstore/principals.go |
| +++ b/internal/objstore/principals.go |
| @@ -11,81 +11,80 @@ import ( |
| "github.com/aws/aws-sdk-go-v2/aws" |
| "github.com/aws/aws-sdk-go-v2/service/iam" |
| + iamtypes "github.com/aws/aws-sdk-go-v2/service/iam/types" |
| ) |
| -// The AWS principal inventory: who in the account has a policy that |
| -// reaches a bucket name (GranteeProber, provisioner.go). |
| +// The AWS principal inventory: which users, roles and groups of the |
| +// account carry an identity policy that reaches a bucket name. |
| // |
| -// It answers one question, for a name NO BUCKET HOLDS: which users, roles |
| -// and groups of this account carry an identity policy, inline or attached, |
| -// directly or through a group, with an Allow statement whose resource |
| -// matches the bucket's ARN or an object ARN under it and whose action can |
| -// be an S3 action. It reads the whole account's authorization details in |
| -// one listing (iam:GetAccountAuthorizationDetails) and evaluates every |
| -// document itself. |
| +// It is a report for a person, and it is NOT a GranteeProber |
| +// (provisioner.go): it claims nothing the ownership ledger could rest on. |
| +// Its first review said why, and was right: |
| // |
| -// It over-reports and never under-reports, because a principal wrongly |
| -// listed costs a claim (the ledger claims nothing, the meter holds) and a |
| -// principal wrongly omitted bills a tenant for bytes somebody else could |
| -// move: |
| +// - A listing is not an instant. IAM is eventually consistent, so a |
| +// policy attached before the listing began can be missing from it; |
| +// and a principal returned on the first page can be given access |
| +// before the last page is read. Nothing in the answer says it is |
| +// complete, and reading it twice does not make it so. |
| +// - A principal's policy is not everyone who can act as it. A role can |
| +// be assumed by whoever its trust policy names, another account's |
| +// principals included, and by whoever may pass it to a service. The |
| +// trust policy's principals are reported for each role that reaches |
| +// the name; who in turn may assume it from inside this account |
| +// (sts:AssumeRole in their own policies) is not traced. |
| +// - An absent bucket now is not a bucket this grant creates. Another |
| +// actor in the account can create the name, with a policy of its own, |
| +// between any probe and the walk's own create, and AWS answers the |
| +// walk's create as a success. |
| +// |
| +// So a claim that nobody else can reach a store bucket cannot be read off |
| +// the account. It needs a basis the account enforces (a rule that stops |
| +// such a grant existing, not a look that failed to find one) and a bucket |
| +// only its own grant could have created. Neither exists; both are design |
| +// questions put to the owner |
| +// (docs/superpowers/plans/2026-10-02-aws-principal-inventory.md). Until |
| +// then no ledger on AWS claims completeness and store transfer holds. |
| +// |
| +// What the report does. It reads the whole account's authorization |
| +// details (iam:GetAccountAuthorizationDetails, every page) and evaluates |
| +// every identity policy itself: users, roles and groups; inline and |
| +// attached managed policies (the default version); a user's groups. |
| +// Within the documents it is given it leans one way, toward listing: |
| // |
| // - Deny statements, conditions, permissions boundaries, session |
| // policies and organization policies only narrow access. None is |
| // evaluated. A principal an explicit Deny keeps out is still listed. |
| // - NotAction and NotResource on an Allow are taken to reach the bucket. |
| -// - A policy variable (${aws:username}) is taken to expand to anything. |
| +// - A policy variable (${aws:username}), and its escapes, are taken to |
| +// expand to anything. |
| // - A group whose policy reaches the bucket is listed, with or without |
| // members, and so is each of its members. |
| // |
| -// What it cannot answer is an error, and an error claims nothing: |
| -// |
| -// - a document it cannot decode, or a statement that is not the IAM |
| -// grammar it knows (an Effect that is neither Allow nor Deny; an Allow |
| -// with no resource element); |
| -// - an attached managed policy the listing did not return; |
| -// - a listing that is truncated without a marker, or does not end; |
| -// - a name a bucket holds. Resource-side grants (a bucket policy, an |
| -// ACL, an access point and its policy, a grant to another account) |
| -// exist only while a bucket does, and none of them is read here. For |
| -// an existing bucket this inventory is half an answer, so it gives |
| -// none. The attestation of buckets that already exist needs that |
| -// other half; it is not built. |
| -// |
| -// What it does not see, by construction, and what follows from that: |
| +// What it cannot read is an error, never a shorter list: a document it |
| +// cannot decode; a statement or an element that is not the identity |
| +// policy grammar it knows; the same principal, policy or inline policy |
| +// name returned twice; an attached managed policy the listing did not |
| +// return; a listing truncated without a marker, or that does not end. |
| // |
| -// - The account's root user, which no policy lists. It is the |
| -// platform's. |
| -// - Anything that changes after the listing. The answer is true of one |
| -// instant; IAM is also eventually consistent, so a policy attached |
| -// seconds before may be missing. A ledger's claim built on it is |
| -// dated, and a principal given the name later, outside the platform's |
| -// own grant, is not known to the ledger. |
| +// What it does not read at all: the root user; a bucket's own policy, ACL |
| +// and access points; grants made through other services' resources |
| +// (access points, Object Lambda, Batch Operations and replication jobs); |
| +// anything in another account. |
| // |
| -// Platform principals. In any real account some principals reach every |
| -// store bucket name: the console's own role provisions them, and an |
| -// administrator role reaches everything. With those counted no name is |
| -// ever free of grantees. The deployment may therefore declare principals |
| -// that are its own, by exact ARN; they are reported apart and are not |
| -// grantees. Which principals may be declared, and whether the bytes they |
| -// move are the tenant's to pay for, is a decision that has not been made |
| -// (docs/superpowers/plans/2026-10-02-aws-principal-inventory.md): nothing |
| -// in the console declares any, and nothing in the console constructs this |
| -// inventory. Until both are decided and the console's role may make the |
| -// listing, the AWS provisioner answers as it always has. |
| - |
| -// iamInventoryAPI is the one IAM read the inventory makes. |
| -type iamInventoryAPI interface { |
| +// The platform's own. A deployment may name principals that are its own, |
| +// by exact ARN; the report sets them apart. That is a label for the |
| +// reader. It exempts nothing from anything. |
| + |
| +// IAMAuthorizationLister is the one IAM read the inventory makes. |
| +// *iam.Client implements it. |
| +type IAMAuthorizationLister interface { |
| GetAccountAuthorizationDetails(ctx context.Context, in *iam.GetAccountAuthorizationDetailsInput, optFns ...func(*iam.Options)) (*iam.GetAccountAuthorizationDetailsOutput, error) |
| } |
| // inventoryMaxPages bounds the listing: a listing that has not ended by |
| // then is not trusted to end. At the service's page size of a hundred |
| // entities this is far more than an account can hold. |
| -const inventoryMaxPages = 2000 |
| - |
| -// ErrBucketExistsNoInventory is the inventory's answer for a name a bucket |
| -// holds: its resource-side grants are not read, so nothing is claimed. |
| -var ErrBucketExistsNoInventory = errors.New("objstore: the bucket exists, and its bucket policy, ACL and access points are not inventoried") |
| +var inventoryMaxPages = 2000 |
| // PrincipalGrant is one principal whose identity policies reach a bucket |
| // name, and the policies through which they do. |
| @@ -96,64 +95,52 @@ type PrincipalGrant struct { |
| // "managed:<policy arn>", and for a user "group:<group arn>" for each |
| // of its groups that does. Sorted. |
| Via []string `json:"via"` |
| + // AssumableBy, for a role, is every principal its trust policy's |
| + // Allow statements name ("AWS:<arn or account>", "Service:<name>", |
| + // "Federated:<provider>", or "*"), sorted: who can become this role |
| + // as far as the role itself says. It is not who can in fact. |
| + AssumableBy []string `json:"assumable_by,omitempty"` |
| } |
| -// PrincipalInventory is everything the listing showed about one name. |
| +// PrincipalInventory is everything one listing showed about one name. It |
| +// is an observation, not a claim of completeness. |
| type PrincipalInventory struct { |
| Bucket string `json:"bucket"` |
| - // Grantees reach the name and are not declared the platform's own. |
| - Grantees []PrincipalGrant `json:"grantees"` |
| - // Platform reach the name and were declared the platform's own. |
| + // Reaching are the principals whose identity policies reach the name |
| + // and that the deployment did not name as its own. |
| + Reaching []PrincipalGrant `json:"reaching"` |
| + // Platform are those it did name. |
| Platform []PrincipalGrant `json:"platform"` |
| // What was read, so an empty answer can be told from an empty account. |
| Users, Roles, Groups, ManagedPolicies int |
| } |
| -// awsInventoryProvisioner is the AWS provisioner with the principal |
| -// inventory: the only AWS provisioner that is a GranteeProber. |
| -type awsInventoryProvisioner struct { |
| - *awsProvisioner |
| - inv iamInventoryAPI |
| +// PrincipalReport reads the account and reports on bucket names. |
| +type PrincipalReport struct { |
| + api IAMAuthorizationLister |
| platform map[string]bool |
| } |
| -var ( |
| - _ Provisioner = (*awsInventoryProvisioner)(nil) |
| - _ BucketProber = (*awsInventoryProvisioner)(nil) |
| - _ GranteeProber = (*awsInventoryProvisioner)(nil) |
| -) |
| - |
| -// NewAWSProvisionerWithInventory builds the AWS provisioner with the |
| -// principal inventory. platform is the deployment's own principals, by |
| -// exact ARN (see the note at the top of this file: nothing decides yet |
| -// which may be declared, and no console calls this). |
| -func NewAWSProvisionerWithInventory(ctx context.Context, region, endpoint, boundary string, platform []string) (Provisioner, error) { |
| - p, err := NewAWSProvisioner(ctx, region, endpoint, boundary) |
| - if err != nil { |
| - return nil, err |
| - } |
| - base := p.(*awsProvisioner) |
| - inv, ok := base.iam.(iamInventoryAPI) |
| - if !ok { |
| - return nil, errors.New("objstore: the IAM client cannot list the account's authorization details") |
| - } |
| - return withInventory(base, inv, platform) |
| -} |
| - |
| -func withInventory(base *awsProvisioner, inv iamInventoryAPI, platform []string) (*awsInventoryProvisioner, error) { |
| - declared := map[string]bool{} |
| +// NewPrincipalReport builds the report over an IAM client. platform is |
| +// the principals the deployment names as its own, each by its exact ARN. |
| +// Nothing in the console calls this. |
| +func NewPrincipalReport(api IAMAuthorizationLister, platform []string) (*PrincipalReport, error) { |
| + if api == nil { |
| + return nil, errors.New("objstore: no IAM client") |
| + } |
| + named := map[string]bool{} |
| for _, arn := range platform { |
| if err := validPlatformPrincipal(arn); err != nil { |
| return nil, err |
| } |
| - declared[arn] = true |
| + named[arn] = true |
| } |
| - return &awsInventoryProvisioner{awsProvisioner: base, inv: inv, platform: declared}, nil |
| + return &PrincipalReport{api: api, platform: named}, nil |
| } |
| // validPlatformPrincipal accepts the ARN of one user, role or group, |
| -// spelled out: no wildcard, no pattern. A declaration is a list a person |
| -// can read, each entry one principal. |
| +// spelled out: no wildcard, no pattern. The list is one a person can |
| +// read, each entry one principal. |
| func validPlatformPrincipal(arn string) error { |
| parts := strings.SplitN(arn, ":", 6) |
| if len(parts) != 6 || parts[0] != "arn" || parts[1] == "" || parts[2] != "iam" || parts[3] != "" || len(parts[4]) != 12 || strings.Trim(parts[4], "0123456789") != "" { |
| @@ -164,29 +151,14 @@ func validPlatformPrincipal(arn string) error { |
| return fmt.Errorf("objstore: platform principal %q is not a user, role or group", arn) |
| } |
| if strings.ContainsAny(arn, "*?$ \t\n") { |
| - return fmt.Errorf("objstore: platform principal %q is a pattern; each principal is declared by its own ARN", arn) |
| + return fmt.Errorf("objstore: platform principal %q is a pattern; each principal is named by its own ARN", arn) |
| } |
| return nil |
| } |
| -// BucketGrantees lists the principals, other than the platform's declared |
| -// own, whose identity policies reach the name (GranteeProber). It answers |
| -// only for a name no bucket holds. |
| -func (p *awsInventoryProvisioner) BucketGrantees(ctx context.Context, name string) ([]string, error) { |
| - inv, err := p.BucketPrincipals(ctx, name) |
| - if err != nil { |
| - return nil, err |
| - } |
| - out := make([]string, 0, len(inv.Grantees)) |
| - for _, g := range inv.Grantees { |
| - out = append(out, g.ARN) |
| - } |
| - return out, nil |
| -} |
| - |
| -// BucketPrincipals is the inventory in full, for a caller that records |
| -// what it rested on. |
| -func (p *awsInventoryProvisioner) BucketPrincipals(ctx context.Context, name string) (*PrincipalInventory, error) { |
| +// BucketPrincipals lists the account once and reports which principals' |
| +// identity policies reach the name. |
| +func (p *PrincipalReport) BucketPrincipals(ctx context.Context, name string) (*PrincipalInventory, error) { |
| if name == "" || strings.ContainsAny(name, "*?/:${} ") { |
| return nil, fmt.Errorf("objstore: %q is not a bucket name", name) |
| } |
| @@ -194,23 +166,17 @@ func (p *awsInventoryProvisioner) BucketPrincipals(ctx context.Context, name str |
| if err != nil { |
| return nil, err |
| } |
| - inv, err := account.reaching(name, p.platform) |
| - if err != nil { |
| - return nil, err |
| - } |
| - // Last, so the window between this answer and the caller's use of it |
| - // is as short as this function can make it: a name a bucket holds has |
| - // grants this inventory did not read. |
| - exists, err := p.BucketExists(ctx, name) |
| - if err != nil { |
| - return nil, err |
| - } |
| - if exists { |
| - return nil, fmt.Errorf("%w: %s", ErrBucketExistsNoInventory, name) |
| - } |
| - return inv, nil |
| + return account.reaching(name, p.platform) |
| } |
| +// Errors a caller can tell apart: each is a listing that is not trusted, |
| +// and none is an empty answer. |
| +var ( |
| + errInventoryTruncated = errors.New("objstore: the account's authorization details are truncated with no way on") |
| + errInventoryEndless = errors.New("objstore: the account's authorization details did not end") |
| + errInventoryRepeated = errors.New("objstore: the account's authorization details return the same thing twice") |
| +) |
| + |
| // accountPolicies is the account's authorization details, decoded. |
| type accountPolicies struct { |
| users, roles, groups []inventoryPrincipal |
| @@ -222,75 +188,107 @@ type inventoryPrincipal struct { |
| inline map[string]*policyDoc // by policy name |
| attached []string // managed policy ARNs |
| groups []string // group names, for a user |
| + assumableBy []string // a role's trust policy principals |
| +} |
| + |
| +// inlinePolicies decodes a principal's inline policies. The same name |
| +// twice is refused: the second would replace the first, and one of them |
| +// might be the one that reaches. |
| +func inlinePolicies(arn string, list []iamtypes.PolicyDetail) (map[string]*policyDoc, error) { |
| + out := map[string]*policyDoc{} |
| + for _, d := range list { |
| + name := aws.ToString(d.PolicyName) |
| + if _, twice := out[name]; twice || name == "" { |
| + return nil, fmt.Errorf("%w: inline policy %q of %s", errInventoryRepeated, name, arn) |
| + } |
| + doc, err := decodePolicy(aws.ToString(d.PolicyDocument)) |
| + if err != nil { |
| + return nil, fmt.Errorf("objstore: policy %q of %s: %w", name, arn, err) |
| + } |
| + out[name] = doc |
| + } |
| + return out, nil |
| } |
| -func (p *awsInventoryProvisioner) listAccount(ctx context.Context) (*accountPolicies, error) { |
| +func attachedPolicies(list []iamtypes.AttachedPolicy) []string { |
| + var out []string |
| + for _, a := range list { |
| + out = append(out, aws.ToString(a.PolicyArn)) |
| + } |
| + return out |
| +} |
| + |
| +func (p *PrincipalReport) listAccount(ctx context.Context) (*accountPolicies, error) { |
| acct := &accountPolicies{managed: map[string]*policyDoc{}} |
| var marker *string |
| - seen := map[string]bool{} |
| + markers := map[string]bool{} |
| + principals := map[string]bool{} |
| + add := func(list *[]inventoryPrincipal, pr inventoryPrincipal) error { |
| + if pr.arn == "" || pr.name == "" { |
| + return errors.New("objstore: the account's authorization details name a principal with no ARN or name") |
| + } |
| + // The same principal twice, on one page or two, is not put |
| + // together or chosen between: either copy might be the stale one. |
| + if principals[pr.arn] { |
| + return fmt.Errorf("%w: %s", errInventoryRepeated, pr.arn) |
| + } |
| + principals[pr.arn] = true |
| + *list = append(*list, pr) |
| + return nil |
| + } |
| for page := 0; ; page++ { |
| if page >= inventoryMaxPages { |
| - return nil, fmt.Errorf("objstore: the account's authorization details did not end within %d pages", inventoryMaxPages) |
| + return nil, fmt.Errorf("%w within %d pages", errInventoryEndless, inventoryMaxPages) |
| } |
| - out, err := p.inv.GetAccountAuthorizationDetails(ctx, &iam.GetAccountAuthorizationDetailsInput{Marker: marker}) |
| + out, err := p.api.GetAccountAuthorizationDetails(ctx, &iam.GetAccountAuthorizationDetailsInput{Marker: marker}) |
| if err != nil { |
| return nil, fmt.Errorf("objstore: list the account's authorization details: %w", err) |
| } |
| for _, u := range out.UserDetailList { |
| - pr := inventoryPrincipal{arn: aws.ToString(u.Arn), kind: "user", name: aws.ToString(u.UserName), groups: u.GroupList, inline: map[string]*policyDoc{}} |
| - for _, d := range u.UserPolicyList { |
| - doc, err := decodePolicy(aws.ToString(d.PolicyDocument)) |
| - if err != nil { |
| - return nil, fmt.Errorf("objstore: policy %q of %s: %w", aws.ToString(d.PolicyName), pr.arn, err) |
| - } |
| - pr.inline[aws.ToString(d.PolicyName)] = doc |
| + pr := inventoryPrincipal{arn: aws.ToString(u.Arn), kind: "user", name: aws.ToString(u.UserName), groups: u.GroupList, attached: attachedPolicies(u.AttachedManagedPolicies)} |
| + if pr.inline, err = inlinePolicies(pr.arn, u.UserPolicyList); err != nil { |
| + return nil, err |
| } |
| - for _, a := range u.AttachedManagedPolicies { |
| - pr.attached = append(pr.attached, aws.ToString(a.PolicyArn)) |
| + if err := add(&acct.users, pr); err != nil { |
| + return nil, err |
| } |
| - acct.users = append(acct.users, pr) |
| } |
| for _, g := range out.GroupDetailList { |
| - pr := inventoryPrincipal{arn: aws.ToString(g.Arn), kind: "group", name: aws.ToString(g.GroupName), inline: map[string]*policyDoc{}} |
| - for _, d := range g.GroupPolicyList { |
| - doc, err := decodePolicy(aws.ToString(d.PolicyDocument)) |
| - if err != nil { |
| - return nil, fmt.Errorf("objstore: policy %q of %s: %w", aws.ToString(d.PolicyName), pr.arn, err) |
| - } |
| - pr.inline[aws.ToString(d.PolicyName)] = doc |
| + pr := inventoryPrincipal{arn: aws.ToString(g.Arn), kind: "group", name: aws.ToString(g.GroupName), attached: attachedPolicies(g.AttachedManagedPolicies)} |
| + if pr.inline, err = inlinePolicies(pr.arn, g.GroupPolicyList); err != nil { |
| + return nil, err |
| } |
| - for _, a := range g.AttachedManagedPolicies { |
| - pr.attached = append(pr.attached, aws.ToString(a.PolicyArn)) |
| + if err := add(&acct.groups, pr); err != nil { |
| + return nil, err |
| } |
| - acct.groups = append(acct.groups, pr) |
| } |
| for _, r := range out.RoleDetailList { |
| - pr := inventoryPrincipal{arn: aws.ToString(r.Arn), kind: "role", name: aws.ToString(r.RoleName), inline: map[string]*policyDoc{}} |
| - for _, d := range r.RolePolicyList { |
| - doc, err := decodePolicy(aws.ToString(d.PolicyDocument)) |
| - if err != nil { |
| - return nil, fmt.Errorf("objstore: policy %q of %s: %w", aws.ToString(d.PolicyName), pr.arn, err) |
| - } |
| - pr.inline[aws.ToString(d.PolicyName)] = doc |
| + pr := inventoryPrincipal{arn: aws.ToString(r.Arn), kind: "role", name: aws.ToString(r.RoleName), attached: attachedPolicies(r.AttachedManagedPolicies)} |
| + if pr.inline, err = inlinePolicies(pr.arn, r.RolePolicyList); err != nil { |
| + return nil, err |
| + } |
| + if pr.assumableBy, err = trustPrincipals(aws.ToString(r.AssumeRolePolicyDocument)); err != nil { |
| + return nil, fmt.Errorf("objstore: trust policy of %s: %w", pr.arn, err) |
| } |
| - for _, a := range r.AttachedManagedPolicies { |
| - pr.attached = append(pr.attached, aws.ToString(a.PolicyArn)) |
| + if err := add(&acct.roles, pr); err != nil { |
| + return nil, err |
| } |
| - acct.roles = append(acct.roles, pr) |
| } |
| for _, m := range out.Policies { |
| arn := aws.ToString(m.Arn) |
| + if _, twice := acct.managed[arn]; twice || arn == "" { |
| + return nil, fmt.Errorf("%w: managed policy %q", errInventoryRepeated, arn) |
| + } |
| + defaults := 0 |
| var current *string |
| for _, v := range m.PolicyVersionList { |
| if v.IsDefaultVersion { |
| - if current != nil { |
| - return nil, fmt.Errorf("objstore: managed policy %s has two default versions", arn) |
| - } |
| + defaults++ |
| current = v.Document |
| } |
| } |
| - if current == nil { |
| - return nil, fmt.Errorf("objstore: managed policy %s has no default version in the listing", arn) |
| + if defaults != 1 || current == nil { |
| + return nil, fmt.Errorf("objstore: managed policy %s has %d default versions with a document in the listing; want one", arn, defaults) |
| } |
| doc, err := decodePolicy(aws.ToString(current)) |
| if err != nil { |
| @@ -302,25 +300,18 @@ func (p *awsInventoryProvisioner) listAccount(ctx context.Context) (*accountPoli |
| break |
| } |
| next := aws.ToString(out.Marker) |
| - if next == "" || seen[next] { |
| - return nil, errors.New("objstore: the account's authorization details are truncated with no way on") |
| + if next == "" || markers[next] { |
| + return nil, errInventoryTruncated |
| } |
| - seen[next] = true |
| + markers[next] = true |
| marker = out.Marker |
| } |
| - for _, list := range [][]inventoryPrincipal{acct.users, acct.roles, acct.groups} { |
| - for _, pr := range list { |
| - if pr.arn == "" || pr.name == "" { |
| - return nil, errors.New("objstore: the account's authorization details name a principal with no ARN or name") |
| - } |
| - } |
| - } |
| return acct, nil |
| } |
| // reaching evaluates every principal against one bucket name. |
| func (a *accountPolicies) reaching(bucket string, platform map[string]bool) (*PrincipalInventory, error) { |
| - inv := &PrincipalInventory{Bucket: bucket, Grantees: []PrincipalGrant{}, Platform: []PrincipalGrant{}, Users: len(a.users), Roles: len(a.roles), Groups: len(a.groups), ManagedPolicies: len(a.managed)} |
| + inv := &PrincipalInventory{Bucket: bucket, Reaching: []PrincipalGrant{}, Platform: []PrincipalGrant{}, Users: len(a.users), Roles: len(a.roles), Groups: len(a.groups), ManagedPolicies: len(a.managed)} |
| via := func(pr inventoryPrincipal) ([]string, error) { |
| partition := "aws" |
| if parts := strings.SplitN(pr.arn, ":", 3); len(parts) == 3 && parts[0] == "arn" && parts[1] != "" { |
| @@ -350,11 +341,11 @@ func (a *accountPolicies) reaching(bucket string, platform map[string]bool) (*Pr |
| return |
| } |
| sort.Strings(through) |
| - g := PrincipalGrant{ARN: pr.arn, Kind: pr.kind, Via: through} |
| + g := PrincipalGrant{ARN: pr.arn, Kind: pr.kind, Via: through, AssumableBy: pr.assumableBy} |
| if platform[pr.arn] { |
| inv.Platform = append(inv.Platform, g) |
| } else { |
| - inv.Grantees = append(inv.Grantees, g) |
| + inv.Reaching = append(inv.Reaching, g) |
| } |
| } |
| for _, g := range a.groups { |
| @@ -363,7 +354,7 @@ func (a *accountPolicies) reaching(bucket string, platform map[string]bool) (*Pr |
| return nil, err |
| } |
| if _, twice := groupARN[g.name]; twice { |
| - return nil, fmt.Errorf("objstore: the listing names the group %q twice", g.name) |
| + return nil, fmt.Errorf("%w: the group %q", errInventoryRepeated, g.name) |
| } |
| groupVia[g.name], groupARN[g.name] = through, g.arn |
| add(g, through) |
| @@ -391,7 +382,7 @@ func (a *accountPolicies) reaching(bucket string, platform map[string]bool) (*Pr |
| } |
| add(r, through) |
| } |
| - for _, list := range []*[]PrincipalGrant{&inv.Grantees, &inv.Platform} { |
| + for _, list := range []*[]PrincipalGrant{&inv.Reaching, &inv.Platform} { |
| sort.Slice(*list, func(i, j int) bool { return (*list)[i].ARN < (*list)[j].ARN }) |
| } |
| return inv, nil |
| @@ -404,31 +395,43 @@ type policyDoc struct { |
| type policyStmt struct { |
| allow bool |
| - actions, notActions []string |
| - resources, notResources []string |
| + actions []string |
| + resources []string |
| hasNotAction, hasNotResource bool |
| } |
| -// oneOrMany decodes an IAM element that is a string or a list of strings. |
| -type oneOrMany []string |
| - |
| -func (o *oneOrMany) UnmarshalJSON(raw []byte) error { |
| - var one string |
| +// stringOrList decodes an IAM element that is a string or a list of |
| +// strings, and refuses anything else: null, a number, an empty list, an |
| +// empty string, a list holding something that is not a string. |
| +func stringOrList(raw json.RawMessage) ([]string, error) { |
| + var one *string |
| if err := json.Unmarshal(raw, &one); err == nil { |
| - *o = []string{one} |
| - return nil |
| + if one == nil || *one == "" { |
| + return nil, errors.New("null or empty where a value is required") |
| + } |
| + return []string{*one}, nil |
| } |
| - var many []string |
| + var many []*string |
| if err := json.Unmarshal(raw, &many); err != nil { |
| - return errors.New("neither a string nor a list of strings") |
| + return nil, errors.New("neither a string nor a list of strings") |
| } |
| - *o = many |
| - return nil |
| + if len(many) == 0 { |
| + return nil, errors.New("an empty list where a value is required") |
| + } |
| + out := make([]string, 0, len(many)) |
| + for _, v := range many { |
| + if v == nil || *v == "" { |
| + return nil, errors.New("null or empty in a list of values") |
| + } |
| + out = append(out, *v) |
| + } |
| + return out, nil |
| } |
| -// decodePolicy reads a policy document as IAM returns it: percent-encoded |
| -// JSON (RFC 3986; a plus sign is a plus sign), or plain JSON. |
| -func decodePolicy(raw string) (*policyDoc, error) { |
| +// policyText turns a policy document as IAM returns it (percent-encoded |
| +// JSON, RFC 3986: a plus sign is a plus sign; or plain JSON) into its |
| +// top-level elements, and checks them against the ones a policy has. |
| +func policyText(raw string) (map[string]json.RawMessage, error) { |
| text := strings.TrimSpace(raw) |
| if text == "" { |
| return nil, errors.New("an empty policy document") |
| @@ -440,64 +443,183 @@ func decodePolicy(raw string) (*policyDoc, error) { |
| } |
| text = decoded |
| } |
| - var doc struct { |
| - Statement json.RawMessage `json:"Statement"` |
| + var doc map[string]json.RawMessage |
| + if err := json.Unmarshal([]byte(text), &doc); err != nil || doc == nil { |
| + return nil, errors.New("policy document is not a JSON object") |
| + } |
| + for key := range doc { |
| + switch key { |
| + case "Version", "Id", "Statement": |
| + default: |
| + return nil, fmt.Errorf("policy document has the element %q", key) |
| + } |
| } |
| - if err := json.Unmarshal([]byte(text), &doc); err != nil { |
| - return nil, fmt.Errorf("policy document is not JSON: %w", err) |
| + if v, ok := doc["Version"]; ok { |
| + var version string |
| + if err := json.Unmarshal(v, &version); err != nil || (version != "2012-10-17" && version != "2008-10-17") { |
| + return nil, fmt.Errorf("policy document has the version %s", string(v)) |
| + } |
| } |
| - if len(doc.Statement) == 0 { |
| + return doc, nil |
| +} |
| + |
| +// policyStatements gives a document's statements, each as its elements: |
| +// one statement or a list of at least one, never null. |
| +func policyStatements(doc map[string]json.RawMessage) ([]map[string]json.RawMessage, error) { |
| + raw, ok := doc["Statement"] |
| + if !ok { |
| return nil, errors.New("policy document has no Statement") |
| } |
| - type stmt struct { |
| - Effect string `json:"Effect"` |
| - Action *oneOrMany `json:"Action"` |
| - NotAction *oneOrMany `json:"NotAction"` |
| - Resource *oneOrMany `json:"Resource"` |
| - NotResource *oneOrMany `json:"NotResource"` |
| + var list []map[string]json.RawMessage |
| + if err := json.Unmarshal(raw, &list); err != nil { |
| + var one map[string]json.RawMessage |
| + if err := json.Unmarshal(raw, &one); err != nil { |
| + return nil, errors.New("policy Statement is neither a statement nor a list of them") |
| + } |
| + list = []map[string]json.RawMessage{one} |
| + } |
| + if len(list) == 0 { |
| + return nil, errors.New("policy Statement is null or empty") |
| } |
| - var list []stmt |
| - if err := json.Unmarshal(doc.Statement, &list); err != nil { |
| - var one stmt |
| - if err := json.Unmarshal(doc.Statement, &one); err != nil { |
| - return nil, fmt.Errorf("policy Statement is neither a statement nor a list of them: %w", err) |
| + for i, st := range list { |
| + if st == nil { |
| + return nil, fmt.Errorf("statement %d is null", i) |
| } |
| - list = []stmt{one} |
| + } |
| + return list, nil |
| +} |
| + |
| +func statementEffect(i int, st map[string]json.RawMessage) (allow bool, err error) { |
| + var effect string |
| + if raw, ok := st["Effect"]; !ok || json.Unmarshal(raw, &effect) != nil || (effect != "Allow" && effect != "Deny") { |
| + return false, fmt.Errorf("statement %d has the effect %s", i, string(st["Effect"])) |
| + } |
| + return effect == "Allow", nil |
| +} |
| + |
| +// decodePolicy reads an identity policy. It checks the grammar before it |
| +// evaluates anything: a shape it does not know is an error, and is never |
| +// a statement that reaches nothing. |
| +func decodePolicy(raw string) (*policyDoc, error) { |
| + doc, err := policyText(raw) |
| + if err != nil { |
| + return nil, err |
| + } |
| + list, err := policyStatements(doc) |
| + if err != nil { |
| + return nil, err |
| } |
| out := &policyDoc{} |
| - for i, s := range list { |
| - var ps policyStmt |
| - switch s.Effect { |
| - case "Allow": |
| - ps.allow = true |
| - case "Deny": |
| - // Read no further: a Deny only narrows, and is not evaluated. |
| - out.statements = append(out.statements, ps) |
| - continue |
| - default: |
| - return nil, fmt.Errorf("statement %d has the effect %q", i, s.Effect) |
| + for i, st := range list { |
| + for key := range st { |
| + switch key { |
| + case "Sid", "Effect", "Action", "NotAction", "Resource", "NotResource", "Condition": |
| + case "Principal", "NotPrincipal": |
| + return nil, fmt.Errorf("statement %d has %s, which an identity policy does not", i, key) |
| + default: |
| + return nil, fmt.Errorf("statement %d has the element %q", i, key) |
| + } |
| + } |
| + allow, err := statementEffect(i, st) |
| + if err != nil { |
| + return nil, err |
| } |
| - if (s.Action == nil) == (s.NotAction == nil) { |
| + ps := policyStmt{allow: allow} |
| + _, hasAction := st["Action"] |
| + _, ps.hasNotAction = st["NotAction"] |
| + _, hasResource := st["Resource"] |
| + _, ps.hasNotResource = st["NotResource"] |
| + if hasAction == ps.hasNotAction { |
| return nil, fmt.Errorf("statement %d does not have exactly one of Action and NotAction", i) |
| } |
| - if (s.Resource == nil) == (s.NotResource == nil) { |
| + if hasResource == ps.hasNotResource { |
| return nil, fmt.Errorf("statement %d does not have exactly one of Resource and NotResource", i) |
| } |
| - if s.Action != nil { |
| - ps.actions = *s.Action |
| - } else { |
| - ps.hasNotAction = true |
| - } |
| - if s.Resource != nil { |
| - ps.resources = *s.Resource |
| - } else { |
| - ps.hasNotResource = true |
| + for _, key := range []string{"Action", "NotAction", "Resource", "NotResource"} { |
| + raw, ok := st[key] |
| + if !ok { |
| + continue |
| + } |
| + values, err := stringOrList(raw) |
| + if err != nil { |
| + return nil, fmt.Errorf("statement %d %s: %w", i, key, err) |
| + } |
| + switch key { |
| + case "Action": |
| + ps.actions = values |
| + case "Resource": |
| + ps.resources = values |
| + } |
| } |
| + // A Deny is validated like an Allow and then not evaluated: it |
| + // only narrows. |
| out.statements = append(out.statements, ps) |
| } |
| return out, nil |
| } |
| +// trustPrincipals reads a role's trust policy and gives every principal |
| +// its Allow statements name, sorted. NotPrincipal on an Allow is "*". |
| +func trustPrincipals(raw string) ([]string, error) { |
| + doc, err := policyText(raw) |
| + if err != nil { |
| + return nil, err |
| + } |
| + list, err := policyStatements(doc) |
| + if err != nil { |
| + return nil, err |
| + } |
| + seen := map[string]bool{} |
| + for i, st := range list { |
| + allow, err := statementEffect(i, st) |
| + if err != nil { |
| + return nil, err |
| + } |
| + if !allow { |
| + continue |
| + } |
| + if _, not := st["NotPrincipal"]; not { |
| + seen["*"] = true |
| + continue |
| + } |
| + rawPrincipal, ok := st["Principal"] |
| + if !ok { |
| + return nil, fmt.Errorf("statement %d names no principal", i) |
| + } |
| + var star string |
| + if json.Unmarshal(rawPrincipal, &star) == nil { |
| + if star != "*" { |
| + return nil, fmt.Errorf("statement %d has the principal %q", i, star) |
| + } |
| + seen["*"] = true |
| + continue |
| + } |
| + var kinds map[string]json.RawMessage |
| + if err := json.Unmarshal(rawPrincipal, &kinds); err != nil || len(kinds) == 0 { |
| + return nil, fmt.Errorf("statement %d has a principal that is not a map of kinds", i) |
| + } |
| + for kind, rawValues := range kinds { |
| + values, err := stringOrList(rawValues) |
| + if err != nil { |
| + return nil, fmt.Errorf("statement %d principal %s: %w", i, kind, err) |
| + } |
| + for _, v := range values { |
| + if v == "*" { |
| + seen["*"] = true |
| + } else { |
| + seen[kind+":"+v] = true |
| + } |
| + } |
| + } |
| + } |
| + out := make([]string, 0, len(seen)) |
| + for v := range seen { |
| + out = append(out, v) |
| + } |
| + sort.Strings(out) |
| + return out, nil |
| +} |
| + |
| // reaches reports whether any Allow statement of the document can apply |
| // an S3 action to the bucket or to an object under it. |
| func (d *policyDoc) reaches(partition, bucket string) bool { |
diff --git a/internal/objstore/principals_export_test.go b/internal/objstore/principals_export_test.go| new file mode 100644 |
| index 000000000..d45e255c9 |
| --- /dev/null |
| +++ b/internal/objstore/principals_export_test.go |
| @@ -0,0 +1,9 @@ |
| +package objstore |
| + |
| +// inventoryMaxPagesForTest sets the listing's page bound and returns the |
| +// one it replaced. |
| +func inventoryMaxPagesForTest(n int) int { |
| + old := inventoryMaxPages |
| + inventoryMaxPages = n |
| + return old |
| +} |
diff --git a/internal/objstore/principals_test.go b/internal/objstore/principals_test.go| index 151321b71..c83194d33 100644 |
| --- a/internal/objstore/principals_test.go |
| +++ b/internal/objstore/principals_test.go |
| @@ -12,7 +12,6 @@ import ( |
| "github.com/aws/aws-sdk-go-v2/aws" |
| "github.com/aws/aws-sdk-go-v2/service/iam" |
| iamtypes "github.com/aws/aws-sdk-go-v2/service/iam/types" |
| - awss3 "github.com/aws/aws-sdk-go-v2/service/s3" |
| ) |
| // stubInventory answers GetAccountAuthorizationDetails with fixed pages. |
| @@ -57,8 +56,10 @@ func invUser(name string, groups []string, inline map[string]string, attached .. |
| return u |
| } |
| +const trustEC2 = `{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Principal":{"Service":"ec2.amazonaws.com"},"Action":"sts:AssumeRole"}]}` |
| + |
| func invRole(name string, inline map[string]string, attached ...string) iamtypes.RoleDetail { |
| - r := iamtypes.RoleDetail{Arn: aws.String(invAccount + "role/" + name), RoleName: aws.String(name)} |
| + r := iamtypes.RoleDetail{Arn: aws.String(invAccount + "role/" + name), RoleName: aws.String(name), AssumeRolePolicyDocument: encoded(trustEC2)} |
| for n, d := range inline { |
| r.RolePolicyList = append(r.RolePolicyList, iamtypes.PolicyDetail{PolicyName: aws.String(n), PolicyDocument: encoded(d)}) |
| } |
| @@ -86,18 +87,11 @@ func invManaged(arn, doc string) iamtypes.ManagedPolicyDetail { |
| }} |
| } |
| -// inventoryFor builds the provisioner over fixed pages and a bucket that |
| -// does or does not exist. |
| -func inventoryFor(t *testing.T, exists bool, platform []string, pages ...*iam.GetAccountAuthorizationDetailsOutput) (*awsInventoryProvisioner, *stubInventory) { |
| +// inventoryFor builds the report over fixed pages. |
| +func inventoryFor(t *testing.T, platform []string, pages ...*iam.GetAccountAuthorizationDetailsOutput) (*PrincipalReport, *stubInventory) { |
| t.Helper() |
| - s3 := &stubS3{t: t, listObjectsV2: func(*awss3.ListObjectsV2Input) (*awss3.ListObjectsV2Output, error) { |
| - if exists { |
| - return &awss3.ListObjectsV2Output{}, nil |
| - } |
| - return nil, &smithyAPIErrorStub{code: "NoSuchBucket"} |
| - }} |
| inv := &stubInventory{pages: pages} |
| - p, err := withInventory(&awsProvisioner{s3: s3, iam: &stubIAM{t: t}, region: "eu-west-1"}, inv, platform) |
| + p, err := NewPrincipalReport(inv, platform) |
| if err != nil { |
| t.Fatal(err) |
| } |
| @@ -147,8 +141,6 @@ func TestPolicyPatternsThatReachABucket(t *testing.T) { |
| {"another service on everything", `"ec2:*"`, `"*"`, false}, |
| {"another service whose name begins s3", `"s3-object-lambda:*"`, `"*"`, false}, |
| {"another partition", `"s3:*"`, `"arn:aws-cn:s3:::carlos-store-bac-blog"`, false}, |
| - {"no actions", `[]`, `"*"`, false}, |
| - {"no resources", `"s3:*"`, `[]`, false}, |
| } { |
| doc, err := decodePolicy(allow(tc.action, tc.resource)) |
| if err != nil { |
| @@ -170,6 +162,9 @@ func TestPolicyPatternsThatReachABucket(t *testing.T) { |
| "an Allow that a Deny takes back": {`{"Statement":[{"Effect":"Allow","Action":"s3:*","Resource":"*"},{"Effect":"Deny","Action":"s3:*","Resource":"*"}]}`, true}, |
| "an Allow under a condition": {`{"Statement":[{"Effect":"Allow","Action":"s3:*","Resource":"*","Condition":{"Bool":{"aws:MultiFactorAuthPresent":"true"}}}]}`, true}, |
| "one statement, not a list": {`{"Statement":{"Effect":"Allow","Action":"s3:*","Resource":"*"}}`, true}, |
| + "the older version": {`{"Version":"2008-10-17","Statement":[{"Effect":"Allow","Action":"s3:*","Resource":"*"}]}`, true}, |
| + "a statement id and a policy id": {`{"Version":"2012-10-17","Id":"p","Statement":[{"Sid":"one","Effect":"Allow","Action":"s3:*","Resource":"*"}]}`, true}, |
| + "an escaped star, taken as any": {`{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":"s3:GetObject","Resource":"arn:aws:s3:::carlos-store-bac-${*}/*"}]}`, true}, |
| "the second of two statements": {`{"Statement":[{"Effect":"Allow","Action":"ec2:*","Resource":"*"},{"Effect":"Allow","Action":"s3:ListBucket","Resource":"arn:aws:s3:::carlos-store-bac-blog"}]}`, true}, |
| } { |
| doc, err := decodePolicy(tc.doc) |
| @@ -183,17 +178,34 @@ func TestPolicyPatternsThatReachABucket(t *testing.T) { |
| } |
| // What it cannot read is an error, never "does not reach". |
| for name, doc := range map[string]string{ |
| - "an unknown effect": `{"Statement":[{"Effect":"Permit","Action":"s3:*","Resource":"*"}]}`, |
| - "an effect in the wrong case": `{"Statement":[{"Effect":"allow","Action":"s3:*","Resource":"*"}]}`, |
| - "an Allow with no resource": `{"Statement":[{"Effect":"Allow","Action":"s3:*"}]}`, |
| - "an Allow with no action": `{"Statement":[{"Effect":"Allow","Resource":"*"}]}`, |
| - "both Resource and NotResource": `{"Statement":[{"Effect":"Allow","Action":"s3:*","Resource":"*","NotResource":"*"}]}`, |
| - "a resource that is a number": `{"Statement":[{"Effect":"Allow","Action":"s3:*","Resource":7}]}`, |
| - "no statement": `{"Version":"2012-10-17"}`, |
| - "not JSON": `Statement: none`, |
| - "an empty document": ``, |
| - "a broken percent-encoding": `%7B%2`, |
| - "a statement that is a string": `{"Statement":"Allow"}`, |
| + "an unknown effect": `{"Statement":[{"Effect":"Permit","Action":"s3:*","Resource":"*"}]}`, |
| + "an effect in the wrong case": `{"Statement":[{"Effect":"allow","Action":"s3:*","Resource":"*"}]}`, |
| + "an Allow with no resource": `{"Statement":[{"Effect":"Allow","Action":"s3:*"}]}`, |
| + "an Allow with no action": `{"Statement":[{"Effect":"Allow","Resource":"*"}]}`, |
| + "both Resource and NotResource": `{"Statement":[{"Effect":"Allow","Action":"s3:*","Resource":"*","NotResource":"*"}]}`, |
| + "a resource that is a number": `{"Statement":[{"Effect":"Allow","Action":"s3:*","Resource":7}]}`, |
| + "a statement that is null": `{"Statement":null}`, |
| + "no statements": `{"Statement":[]}`, |
| + "a null among the statements": `{"Statement":[null]}`, |
| + "no actions": `{"Statement":[{"Effect":"Allow","Action":[],"Resource":"*"}]}`, |
| + "no resources": `{"Statement":[{"Effect":"Allow","Action":"s3:*","Resource":[]}]}`, |
| + "an action that is null": `{"Statement":[{"Effect":"Allow","Action":null,"Resource":"*"}]}`, |
| + "a null among the resources": `{"Statement":[{"Effect":"Allow","Action":"s3:*","Resource":["arn:aws:s3:::x",null]}]}`, |
| + "an empty resource": `{"Statement":[{"Effect":"Allow","Action":"s3:*","Resource":""}]}`, |
| + "a principal, as a bucket policy has": `{"Statement":[{"Effect":"Allow","Principal":"*","Action":"s3:*","Resource":"*"}]}`, |
| + "a NotPrincipal": `{"Statement":[{"Effect":"Deny","NotPrincipal":{"AWS":"*"},"Action":"s3:*","Resource":"*"}]}`, |
| + "an element it does not know": `{"Statement":[{"Effect":"Allow","Action":"s3:*","Resource":"*","Scope":"all"}]}`, |
| + "a version it does not know": `{"Version":"2030-01-01","Statement":[{"Effect":"Allow","Action":"s3:*","Resource":"*"}]}`, |
| + "a version that is a number": `{"Version":2012,"Statement":[{"Effect":"Allow","Action":"s3:*","Resource":"*"}]}`, |
| + "a top-level element it does not know": `{"Statement":[{"Effect":"Allow","Action":"s3:*","Resource":"*"}],"Extra":1}`, |
| + "a Deny with no resource": `{"Statement":[{"Effect":"Deny","Action":"s3:*"}]}`, |
| + "a document that is a list": `[{"Effect":"Allow","Action":"s3:*","Resource":"*"}]`, |
| + "null": `null`, |
| + "no statement": `{"Version":"2012-10-17"}`, |
| + "not JSON": `Statement: none`, |
| + "an empty document": ``, |
| + "a broken percent-encoding": `%7B%2`, |
| + "a statement that is a string": `{"Statement":"Allow"}`, |
| } { |
| if _, err := decodePolicy(doc); err == nil { |
| t.Errorf("%s: decoded; want an error", name) |
| @@ -241,7 +253,7 @@ func TestInventoryListsEveryPrincipalThatReachesTheName(t *testing.T) { |
| invManaged(awsAdmin, allow(`"*"`, `"*"`)), |
| }, |
| } |
| - p, stub := inventoryFor(t, false, nil, page1, page2) |
| + p, stub := inventoryFor(t, nil, page1, page2) |
| inv, err := p.BucketPrincipals(context.Background(), bucket) |
| if err != nil { |
| t.Fatal(err) |
| @@ -254,8 +266,8 @@ func TestInventoryListsEveryPrincipalThatReachesTheName(t *testing.T) { |
| invAccount + "user/in-a-reading-group", |
| invAccount + "user/with-a-managed-policy", |
| } |
| - if got := arns(inv.Grantees); !reflect.DeepEqual(got, want) { |
| - t.Fatalf("grantees\n %v\nwant\n %v", got, want) |
| + if got := arns(inv.Reaching); !reflect.DeepEqual(got, want) { |
| + t.Fatalf("reaching\n %v\nwant\n %v", got, want) |
| } |
| if len(inv.Platform) != 0 || inv.Users != 6 || inv.Groups != 2 || inv.Roles != 3 || inv.ManagedPolicies != 3 { |
| t.Fatalf("inventory %+v", inv) |
| @@ -264,8 +276,11 @@ func TestInventoryListsEveryPrincipalThatReachesTheName(t *testing.T) { |
| t.Fatalf("pages asked for: %q; want the first, then the marker", stub.calls) |
| } |
| via := map[string][]string{} |
| - for _, g := range inv.Grantees { |
| + for _, g := range inv.Reaching { |
| via[g.ARN] = g.Via |
| + if (g.Kind == "role") != reflect.DeepEqual(g.AssumableBy, []string{"Service:ec2.amazonaws.com"}) { |
| + t.Errorf("%s: assumable by %v", g.ARN, g.AssumableBy) |
| + } |
| if wantKind := strings.SplitN(strings.TrimPrefix(g.ARN, invAccount), "/", 2)[0]; g.Kind != wantKind { |
| t.Errorf("%s has kind %q", g.ARN, g.Kind) |
| } |
| @@ -282,16 +297,10 @@ func TestInventoryListsEveryPrincipalThatReachesTheName(t *testing.T) { |
| t.Errorf("%s reaches through %v; want %v", arn, via[arn], want) |
| } |
| } |
| - // The GranteeProber's answer is the same list. |
| - p, _ = inventoryFor(t, false, nil, page1, page2) |
| - grantees, err := p.BucketGrantees(context.Background(), bucket) |
| - if err != nil || !reflect.DeepEqual(grantees, want) { |
| - t.Fatalf("BucketGrantees %v %v", grantees, err) |
| - } |
| - // With the platform's own declared, they are reported apart, and a |
| - // name only they reach has no grantees. |
| + // With the platform's own named, they are set apart; that is a label, |
| + // and they are still in the report. |
| platform := []string{invAccount + "role/carlos-flagship-host", invAccount + "role/OrganizationAccountAccessRole", invAccount + "role/not-in-the-account"} |
| - p, _ = inventoryFor(t, false, platform, page1, page2) |
| + p, _ = inventoryFor(t, platform, page1, page2) |
| inv, err = p.BucketPrincipals(context.Background(), bucket) |
| if err != nil { |
| t.Fatal(err) |
| @@ -299,16 +308,70 @@ func TestInventoryListsEveryPrincipalThatReachesTheName(t *testing.T) { |
| if got := arns(inv.Platform); !reflect.DeepEqual(got, []string{platform[1], platform[0]}) { |
| t.Fatalf("platform %v", got) |
| } |
| - if got := arns(inv.Grantees); !reflect.DeepEqual(got, []string{want[0], want[3], want[4], want[5]}) { |
| - t.Fatalf("grantees with the platform declared: %v", got) |
| + if got := arns(inv.Reaching); !reflect.DeepEqual(got, []string{want[0], want[3], want[4], want[5]}) { |
| + t.Fatalf("reaching with the platform named: %v", got) |
| } |
| - p, _ = inventoryFor(t, false, platform, page1, page2) |
| - if grantees, err := p.BucketGrantees(context.Background(), "carlos-store-new-name"); err != nil || !reflect.DeepEqual(grantees, []string{invAccount + "group/readers", invAccount + "user/in-a-reading-group", invAccount + "user/with-a-managed-policy"}) { |
| - t.Fatalf("a new name under the prefix: %v %v; want the prefix readers, who are nobody's declared own", grantees, err) |
| + p, _ = inventoryFor(t, platform, page1, page2) |
| + if inv, err := p.BucketPrincipals(context.Background(), "carlos-store-new-name"); err != nil || !reflect.DeepEqual(arns(inv.Reaching), []string{invAccount + "group/readers", invAccount + "user/in-a-reading-group", invAccount + "user/with-a-managed-policy"}) { |
| + t.Fatalf("a new name under the prefix: %v %v; want the prefix readers, who are nobody's named own", inv, err) |
| } |
| - p, _ = inventoryFor(t, false, platform, page1, page2) |
| - if grantees, err := p.BucketGrantees(context.Background(), "elsewhere-entirely"); err != nil || grantees == nil || len(grantees) != 0 { |
| - t.Fatalf("a name only the administrator reaches: %v %v; want none, and a list", grantees, err) |
| + p, _ = inventoryFor(t, platform, page1, page2) |
| + if inv, err := p.BucketPrincipals(context.Background(), "elsewhere-entirely"); err != nil || len(inv.Reaching) != 0 || len(inv.Platform) != 1 { |
| + t.Fatalf("a name only the administrator reaches: %+v %v", inv, err) |
| + } |
| +} |
| + |
| +// A role is reported with who its trust policy lets become it: another |
| +// account, a federation, anyone. |
| +func TestInventoryReportsWhoARoleTrusts(t *testing.T) { |
| + role := func(name, trust string) iamtypes.RoleDetail { |
| + r := invRole(name, map[string]string{"p": allow(`"s3:GetObject"`, `"arn:aws:s3:::carlos-store-bac-blog/*"`)}) |
| + r.AssumeRolePolicyDocument = encoded(trust) |
| + return r |
| + } |
| + trust := func(principal string) string { |
| + return `{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Principal":` + principal + `,"Action":"sts:AssumeRole"}]}` |
| + } |
| + p, _ := inventoryFor(t, nil, &iam.GetAccountAuthorizationDetailsOutput{RoleDetailList: []iamtypes.RoleDetail{ |
| + role("a-another-account", trust(`{"AWS":"arn:aws:iam::999999999999:root"}`)), |
| + role("b-several", trust(`{"AWS":["arn:aws:iam::123456789012:user/ops","111111111111"],"Service":"lambda.amazonaws.com"}`)), |
| + role("c-anyone", trust(`"*"`)), |
| + role("d-anyone-in-a-map", trust(`{"AWS":"*"}`)), |
| + role("e-federated", trust(`{"Federated":"arn:aws:iam::123456789012:saml-provider/idp"}`)), |
| + role("f-not-principal", `{"Statement":[{"Effect":"Allow","NotPrincipal":{"AWS":"arn:aws:iam::123456789012:user/x"},"Action":"sts:AssumeRole"},{"Effect":"Deny","Principal":"*","Action":"sts:AssumeRole"}]}`), |
| + }}) |
| + inv, err := p.BucketPrincipals(context.Background(), "carlos-store-bac-blog") |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + want := [][]string{ |
| + {"AWS:arn:aws:iam::999999999999:root"}, |
| + {"AWS:111111111111", "AWS:arn:aws:iam::123456789012:user/ops", "Service:lambda.amazonaws.com"}, |
| + {"*"}, {"*"}, |
| + {"Federated:arn:aws:iam::123456789012:saml-provider/idp"}, |
| + {"*"}, |
| + } |
| + if len(inv.Reaching) != len(want) { |
| + t.Fatalf("reaching %v", arns(inv.Reaching)) |
| + } |
| + for i, g := range inv.Reaching { |
| + if !reflect.DeepEqual(g.AssumableBy, want[i]) { |
| + t.Errorf("%s is assumable by %v; want %v", g.ARN, g.AssumableBy, want[i]) |
| + } |
| + } |
| + // A trust policy it cannot read is an error, not a role with nobody. |
| + for name, doc := range map[string]string{ |
| + "no principal": `{"Statement":[{"Effect":"Allow","Action":"sts:AssumeRole"}]}`, |
| + "a principal that is a word": `{"Statement":[{"Effect":"Allow","Principal":"everyone","Action":"sts:AssumeRole"}]}`, |
| + "an empty principal map": `{"Statement":[{"Effect":"Allow","Principal":{},"Action":"sts:AssumeRole"}]}`, |
| + "a null principal value": `{"Statement":[{"Effect":"Allow","Principal":{"AWS":null},"Action":"sts:AssumeRole"}]}`, |
| + "no trust policy": ``, |
| + "an unknown effect": `{"Statement":[{"Effect":"Maybe","Principal":"*"}]}`, |
| + } { |
| + p, _ := inventoryFor(t, nil, &iam.GetAccountAuthorizationDetailsOutput{RoleDetailList: []iamtypes.RoleDetail{role("r", doc)}}) |
| + if inv, err := p.BucketPrincipals(context.Background(), "carlos-store-bac-blog"); err == nil { |
| + t.Errorf("%s: %+v with no error", name, inv) |
| + } |
| } |
| } |
| @@ -316,80 +379,114 @@ func TestInventoryListsEveryPrincipalThatReachesTheName(t *testing.T) { |
| // bucket ARN is spelled with it. |
| func TestInventoryMatchesInThePrincipalsPartition(t *testing.T) { |
| const bucket = "carlos-store-bac-blog" |
| - cn := iamtypes.RoleDetail{Arn: aws.String("arn:aws-cn:iam::123456789012:role/reader"), RoleName: aws.String("reader"), |
| + cn := iamtypes.RoleDetail{Arn: aws.String("arn:aws-cn:iam::123456789012:role/reader"), RoleName: aws.String("reader"), AssumeRolePolicyDocument: encoded(trustEC2), |
| RolePolicyList: []iamtypes.PolicyDetail{{PolicyName: aws.String("p"), PolicyDocument: encoded(allow(`"s3:GetObject"`, `"arn:aws-cn:s3:::carlos-store-bac-blog/*"`))}}} |
| other := invRole("spelled-for-another-partition", map[string]string{"p": allow(`"s3:GetObject"`, `"arn:aws-cn:s3:::carlos-store-bac-blog/*"`)}) |
| - p, _ := inventoryFor(t, false, nil, &iam.GetAccountAuthorizationDetailsOutput{RoleDetailList: []iamtypes.RoleDetail{cn, other}}) |
| - grantees, err := p.BucketGrantees(context.Background(), bucket) |
| - if err != nil || !reflect.DeepEqual(grantees, []string{"arn:aws-cn:iam::123456789012:role/reader"}) { |
| - t.Fatalf("grantees %v %v; want the role whose partition its policy is spelled in, and not the one it is not", grantees, err) |
| + p, _ := inventoryFor(t, nil, &iam.GetAccountAuthorizationDetailsOutput{RoleDetailList: []iamtypes.RoleDetail{cn, other}}) |
| + inv, err := p.BucketPrincipals(context.Background(), bucket) |
| + if err != nil || !reflect.DeepEqual(arns(inv.Reaching), []string{"arn:aws-cn:iam::123456789012:role/reader"}) { |
| + t.Fatalf("reaching %v %v; want the role whose partition its policy is spelled in, and not the one it is not", inv, err) |
| } |
| } |
| -// What the inventory cannot answer is an error: it never answers "nobody" |
| -// from a listing it could not read to the end. |
| +// What the report cannot read is an error with its reason, never a |
| +// shorter list, and it stops asking where it should. |
| func TestInventoryClaimsNothingItCannotRead(t *testing.T) { |
| ctx := context.Background() |
| const bucket = "carlos-store-bac-blog" |
| - good := &iam.GetAccountAuthorizationDetailsOutput{UserDetailList: []iamtypes.UserDetail{invUser("someone", nil, map[string]string{"p": allow(`"ec2:*"`, `"*"`)})}} |
| + good := func() *iam.GetAccountAuthorizationDetailsOutput { |
| + return &iam.GetAccountAuthorizationDetailsOutput{UserDetailList: []iamtypes.UserDetail{invUser("someone", nil, map[string]string{"p": allow(`"ec2:*"`, `"*"`)})}} |
| + } |
| + reaches := allow(`"s3:*"`, `"*"`) |
| + harmless := allow(`"ec2:*"`, `"*"`) |
| // The control: the same account, read to the end, reaches nothing. |
| - p, _ := inventoryFor(t, false, nil, good) |
| - if grantees, err := p.BucketGrantees(ctx, bucket); err != nil || len(grantees) != 0 { |
| - t.Fatalf("control: %v %v", grantees, err) |
| - } |
| - // A name a bucket holds: its resource-side grants are not read. |
| - p, _ = inventoryFor(t, true, nil, good) |
| - if _, err := p.BucketGrantees(ctx, bucket); !errors.Is(err, ErrBucketExistsNoInventory) { |
| - t.Fatalf("a name a bucket holds: %v; want ErrBucketExistsNoInventory", err) |
| - } |
| - // The probe failing is an error too. |
| - p, _ = inventoryFor(t, false, nil, good) |
| - p.s3 = &stubS3{t: t, listObjectsV2: func(*awss3.ListObjectsV2Input) (*awss3.ListObjectsV2Output, error) { |
| - return nil, errors.New("timeout") |
| - }} |
| - if _, err := p.BucketGrantees(ctx, bucket); err == nil { |
| - t.Fatal("a failed bucket probe: no error") |
| + p, stub := inventoryFor(t, nil, good()) |
| + if inv, err := p.BucketPrincipals(ctx, bucket); err != nil || len(inv.Reaching) != 0 || inv.Users != 1 || len(stub.calls) != 1 { |
| + t.Fatalf("control: %+v %v after %d calls", inv, err, len(stub.calls)) |
| } |
| // The listing refused (the role may not make it). |
| - p, stub := inventoryFor(t, false, nil, good) |
| + p, stub = inventoryFor(t, nil, good()) |
| stub.err = &smithyAPIErrorStub{code: "AccessDenied"} |
| - if grantees, err := p.BucketGrantees(ctx, bucket); err == nil || grantees != nil { |
| - t.Fatalf("a refused listing: %v %v", grantees, err) |
| + if inv, err := p.BucketPrincipals(ctx, bucket); err == nil || inv != nil { |
| + t.Fatalf("a refused listing: %v %v", inv, err) |
| + } |
| + // Truncated with no marker: refused at the first page, for that |
| + // reason, with no second call. |
| + p, stub = inventoryFor(t, nil, &iam.GetAccountAuthorizationDetailsOutput{IsTruncated: true}, good()) |
| + if _, err := p.BucketPrincipals(ctx, bucket); !errors.Is(err, errInventoryTruncated) || len(stub.calls) != 1 { |
| + t.Fatalf("truncated with no marker: %v after %d calls; want errInventoryTruncated after one", err, len(stub.calls)) |
| + } |
| + // A marker that repeats: refused when it repeats, with no third call |
| + // (the third page, which would end the listing cleanly, is never |
| + // asked for). |
| + again := func() *iam.GetAccountAuthorizationDetailsOutput { |
| + return &iam.GetAccountAuthorizationDetailsOutput{IsTruncated: true, Marker: aws.String("m")} |
| + } |
| + p, stub = inventoryFor(t, nil, again(), again(), good()) |
| + if _, err := p.BucketPrincipals(ctx, bucket); !errors.Is(err, errInventoryTruncated) || len(stub.calls) != 2 { |
| + t.Fatalf("a marker that repeats: %v after %d calls; want errInventoryTruncated after two", err, len(stub.calls)) |
| + } |
| + // A listing that does not end is given up on at the bound, exactly. |
| + saved := inventoryMaxPagesForTest(5) |
| + endless := &endlessInventory{} |
| + rep, _ := NewPrincipalReport(endless, nil) |
| + _, err := rep.BucketPrincipals(ctx, bucket) |
| + inventoryMaxPagesForTest(saved) |
| + if !errors.Is(err, errInventoryEndless) || endless.calls != 5 { |
| + t.Fatalf("an endless listing: %v after %d pages; want errInventoryEndless after five", err, endless.calls) |
| + } |
| + |
| + // The same thing returned twice is refused, whichever copy reaches. |
| + twoPages := func(first, second *iam.GetAccountAuthorizationDetailsOutput) []*iam.GetAccountAuthorizationDetailsOutput { |
| + first.IsTruncated, first.Marker = true, aws.String("m1") |
| + return []*iam.GetAccountAuthorizationDetailsOutput{first, second} |
| + } |
| + managed := "arn:aws:iam::123456789012:policy/p" |
| + for name, pages := range map[string][]*iam.GetAccountAuthorizationDetailsOutput{ |
| + "a managed policy on two pages, the first reaching": twoPages( |
| + &iam.GetAccountAuthorizationDetailsOutput{Policies: []iamtypes.ManagedPolicyDetail{invManaged(managed, reaches)}, RoleDetailList: []iamtypes.RoleDetail{invRole("r", nil, managed)}}, |
| + &iam.GetAccountAuthorizationDetailsOutput{Policies: []iamtypes.ManagedPolicyDetail{invManaged(managed, harmless)}}), |
| + "a role on two pages, the first reaching": twoPages( |
| + &iam.GetAccountAuthorizationDetailsOutput{RoleDetailList: []iamtypes.RoleDetail{invRole("r", map[string]string{"p": reaches})}}, |
| + &iam.GetAccountAuthorizationDetailsOutput{RoleDetailList: []iamtypes.RoleDetail{invRole("r", map[string]string{"p": harmless})}}), |
| + "a user twice on one page": {{UserDetailList: []iamtypes.UserDetail{invUser("u", nil, map[string]string{"p": reaches}), invUser("u", nil, map[string]string{"p": harmless})}}}, |
| + "a group twice": {{GroupDetailList: []iamtypes.GroupDetail{invGroup("g", nil), invGroup("g", nil)}}}, |
| + "an inline policy name twice": {{UserDetailList: []iamtypes.UserDetail{{Arn: aws.String(invAccount + "user/u"), UserName: aws.String("u"), UserPolicyList: []iamtypes.PolicyDetail{ |
| + {PolicyName: aws.String("p"), PolicyDocument: encoded(reaches)}, {PolicyName: aws.String("p"), PolicyDocument: encoded(harmless)}}}}}}, |
| + } { |
| + p, _ := inventoryFor(t, nil, pages...) |
| + if inv, err := p.BucketPrincipals(ctx, bucket); !errors.Is(err, errInventoryRepeated) { |
| + t.Errorf("%s: %+v %v; want errInventoryRepeated", name, inv, err) |
| + } |
| } |
| for name, pages := range map[string][]*iam.GetAccountAuthorizationDetailsOutput{ |
| - "truncated with no marker": {{IsTruncated: true}}, |
| - "a marker that repeats": {{IsTruncated: true, Marker: aws.String("m")}, {IsTruncated: true, Marker: aws.String("m")}, {IsTruncated: true, Marker: aws.String("m")}}, |
| "a managed policy attached and not listed": {{RoleDetailList: []iamtypes.RoleDetail{invRole("r", nil, "arn:aws:iam::123456789012:policy/missing")}}}, |
| - "a managed policy with no default version": {{Policies: []iamtypes.ManagedPolicyDetail{{Arn: aws.String("arn:aws:iam::123456789012:policy/p"), PolicyVersionList: []iamtypes.PolicyVersion{{VersionId: aws.String("v1"), Document: encoded(allow(`"ec2:*"`, `"*"`))}}}}}}, |
| - "a managed policy with two default versions": {{Policies: []iamtypes.ManagedPolicyDetail{{Arn: aws.String("arn:aws:iam::123456789012:policy/p"), PolicyVersionList: []iamtypes.PolicyVersion{ |
| - {VersionId: aws.String("v1"), IsDefaultVersion: true, Document: encoded(allow(`"ec2:*"`, `"*"`))}, {VersionId: aws.String("v2"), IsDefaultVersion: true, Document: encoded(allow(`"ec2:*"`, `"*"`))}}}}}}, |
| + "a managed policy with no default version": {{Policies: []iamtypes.ManagedPolicyDetail{{Arn: aws.String(managed), PolicyVersionList: []iamtypes.PolicyVersion{{VersionId: aws.String("v1"), Document: encoded(harmless)}}}}}}, |
| + "a managed policy with two default versions": {{Policies: []iamtypes.ManagedPolicyDetail{{Arn: aws.String(managed), PolicyVersionList: []iamtypes.PolicyVersion{ |
| + {VersionId: aws.String("v1"), IsDefaultVersion: true, Document: encoded(harmless)}, {VersionId: aws.String("v2"), IsDefaultVersion: true, Document: encoded(harmless)}}}}}}, |
| + "two default versions, the first with no document": {{Policies: []iamtypes.ManagedPolicyDetail{{Arn: aws.String(managed), PolicyVersionList: []iamtypes.PolicyVersion{ |
| + {VersionId: aws.String("v1"), IsDefaultVersion: true}, {VersionId: aws.String("v2"), IsDefaultVersion: true, Document: encoded(harmless)}}}}}}, |
| + "a default version with no document": {{Policies: []iamtypes.ManagedPolicyDetail{{Arn: aws.String(managed), PolicyVersionList: []iamtypes.PolicyVersion{{VersionId: aws.String("v1"), IsDefaultVersion: true}}}}}}, |
| "an inline policy that does not decode": {{UserDetailList: []iamtypes.UserDetail{invUser("u", nil, map[string]string{"p": `{"Statement":[{"Effect":"Allow","Action":"s3:*"}]}`})}}}, |
| + "an inline policy whose statement is null": {{UserDetailList: []iamtypes.UserDetail{invUser("u", nil, map[string]string{"p": `{"Statement":null}`})}}}, |
| "a role's inline policy that does not decode": {{RoleDetailList: []iamtypes.RoleDetail{invRole("r", map[string]string{"p": `not a policy`})}}}, |
| "a group's inline policy that does not decode": {{GroupDetailList: []iamtypes.GroupDetail{invGroup("g", map[string]string{"p": `not a policy`})}}}, |
| - "a managed policy that does not decode": {{Policies: []iamtypes.ManagedPolicyDetail{invManaged("arn:aws:iam::123456789012:policy/p", `not a policy`)}}}, |
| + "a managed policy that does not decode": {{Policies: []iamtypes.ManagedPolicyDetail{invManaged(managed, `not a policy`)}}}, |
| "a user in a group the listing did not return": {{UserDetailList: []iamtypes.UserDetail{invUser("u", []string{"ghosts"}, nil)}}}, |
| - "a group listed twice": {{GroupDetailList: []iamtypes.GroupDetail{invGroup("g", nil), invGroup("g", nil)}}}, |
| - "a principal with no ARN": {{RoleDetailList: []iamtypes.RoleDetail{{RoleName: aws.String("r")}}}}, |
| + "a principal with no ARN": {{RoleDetailList: []iamtypes.RoleDetail{{RoleName: aws.String("r"), AssumeRolePolicyDocument: encoded(trustEC2)}}}}, |
| } { |
| - p, _ := inventoryFor(t, false, nil, pages...) |
| - if grantees, err := p.BucketGrantees(ctx, bucket); err == nil { |
| - t.Errorf("%s: %v with no error; want an error", name, grantees) |
| + p, _ := inventoryFor(t, nil, pages...) |
| + if inv, err := p.BucketPrincipals(ctx, bucket); err == nil || errors.Is(err, errInventoryRepeated) { |
| + t.Errorf("%s: %+v %v; want an error of its own", name, inv, err) |
| } |
| } |
| - // A listing that never ends is given up on. |
| - p, stub = inventoryFor(t, false, nil) |
| - endless := &endlessInventory{} |
| - p.inv = endless |
| - if _, err := p.BucketGrantees(ctx, bucket); err == nil || endless.calls != inventoryMaxPages { |
| - t.Fatalf("an endless listing: %v after %d pages", err, endless.calls) |
| - } |
| - _ = stub |
| - // A name that is a pattern is not a name. |
| + // A name that is a pattern is not a name, and the account is not read |
| + // for it. |
| for _, name := range []string{"", "carlos-store-*", "a/b", "arn:aws:s3:::b", "a?b", "a${x}"} { |
| - p, _ := inventoryFor(t, false, nil, good) |
| - if _, err := p.BucketGrantees(ctx, name); err == nil { |
| - t.Errorf("the name %q was answered for", name) |
| + p, stub := inventoryFor(t, nil, good()) |
| + if _, err := p.BucketPrincipals(ctx, name); err == nil || len(stub.calls) != 0 { |
| + t.Errorf("the name %q: %v after %d calls", name, err, len(stub.calls)) |
| } |
| } |
| } |
| @@ -398,6 +495,12 @@ type endlessInventory struct{ calls int } |
| func (e *endlessInventory) GetAccountAuthorizationDetails(context.Context, *iam.GetAccountAuthorizationDetailsInput, ...func(*iam.Options)) (*iam.GetAccountAuthorizationDetailsOutput, error) { |
| e.calls++ |
| + if e.calls > 10 { |
| + // Past the bound the test sets: a page that must never be asked |
| + // for. It ends the listing, so a report without the bound would |
| + // answer. |
| + return &iam.GetAccountAuthorizationDetailsOutput{}, nil |
| + } |
| return &iam.GetAccountAuthorizationDetailsOutput{IsTruncated: true, Marker: aws.String(fmt.Sprint("m", e.calls))}, nil |
| } |
| @@ -431,26 +534,29 @@ func TestPlatformPrincipalsAreDeclaredOneByOne(t *testing.T) { |
| t.Errorf("%q was accepted as a platform principal", arn) |
| } |
| } |
| - if _, err := withInventory(&awsProvisioner{}, &stubInventory{}, []string{"arn:aws:iam::123456789012:role/*"}); err == nil { |
| + if _, err := NewPrincipalReport(&stubInventory{}, []string{"arn:aws:iam::123456789012:role/*"}); err == nil { |
| t.Error("a pattern was accepted by the constructor") |
| } |
| + if _, err := NewPrincipalReport(nil, nil); err == nil { |
| + t.Error("a report was built over no client") |
| + } |
| } |
| -// The plain AWS provisioner is not a GranteeProber: until the inventory is |
| -// constructed on purpose, the grant walk on AWS claims nothing, as before. |
| -func TestThePlainAWSProvisionerHasNoInventory(t *testing.T) { |
| +// Nothing here is a GranteeProber: a listing of the account is a report |
| +// for a person, and the grant walk on AWS claims nothing, as before. |
| +func TestNothingOnAWSAnswersForGrantees(t *testing.T) { |
| var plain Provisioner = &awsProvisioner{} |
| if _, ok := plain.(GranteeProber); ok { |
| - t.Fatal("the plain AWS provisioner answers for grantees") |
| + t.Fatal("the AWS provisioner answers for grantees") |
| } |
| if _, ok := plain.(BucketProber); !ok { |
| - t.Fatal("the plain AWS provisioner lost its bucket probe") |
| + t.Fatal("the AWS provisioner lost its bucket probe") |
| } |
| - var with Provisioner = &awsInventoryProvisioner{awsProvisioner: &awsProvisioner{}} |
| - if _, ok := with.(GranteeProber); !ok { |
| - t.Fatal("the inventory provisioner does not answer for grantees") |
| + report, err := NewPrincipalReport(&stubInventory{}, nil) |
| + if err != nil { |
| + t.Fatal(err) |
| } |
| - if _, ok := with.(BucketProber); !ok { |
| - t.Fatal("the inventory provisioner lost the bucket probe") |
| + if _, ok := any(report).(GranteeProber); ok { |
| + t.Fatal("the principal report answers for grantees: one listing cannot support the ledger's claim") |
| } |
| } |
diff --git a/internal/objstore/provisioner.go b/internal/objstore/provisioner.go| index 36ed050c2..5ad371362 100644 |
| --- a/internal/objstore/provisioner.go |
| +++ b/internal/objstore/provisioner.go |
| @@ -61,13 +61,15 @@ type BucketProber interface { |
| // An implementation that cannot answer for all of these returns an error, |
| // and an error claims nothing. |
| // |
| -// The AWS provisioner the console constructs does not implement it. An |
| -// implementation for identity policies exists (principals.go: every user, |
| -// role and group, for a name no bucket holds) and is constructed by |
| -// nothing: it needs a listing the console's role is not granted, and a |
| -// decision on which principals are the platform's own. Until both, no |
| -// bucket's ownership ledger claims completeness on AWS, and store transfer |
| -// holds. |
| +// Nothing on AWS implements it. A report of who reaches a name exists |
| +// (principals.go) and is deliberately not this interface: one listing of |
| +// an eventually consistent account, of principals whose trust policies |
| +// let others act as them, about a name another actor can create first, |
| +// cannot support the claim. That needs a rule the account enforces and a |
| +// bucket only its own grant could have made |
| +// (docs/superpowers/plans/2026-10-02-aws-principal-inventory.md). Until |
| +// then no bucket's ownership ledger claims completeness on AWS, and store |
| +// transfer holds. |
| type GranteeProber interface { |
| BucketGrantees(ctx context.Context, name string) ([]string, error) |
| } |