amadan
objstore: the grant that provisions a store, and the ceiling that contains it
The host role has never been able to provision. region-host grants Get/Put/ Delete on the deployment and data buckets and a read-only Describe set for the architecture page; the provisioner calls s3:CreateBucket and the whole iam:CreateUser/PutUserPolicy/CreateAccessKey lifecycle. So every deployment that set CARLOS_STORE_BACKEND and nothing else would have failed at its first grant, deep in the walk, against live AWS. That is #368's other half, and it is why the flagship has never had object storage switched on. The grant cannot be added on its own. A role holding CreateUser and PutUserPolicy can mint a principal wider than itself, and scoping those calls to an IAM path does not help: a path says which user is made, never what that user may do. So the module ships a permissions boundary — S3 only, on this deployment's own bucket prefix — conditions BOTH calls on it, and objstore stamps it on CreateUser itself rather than attaching it afterwards, where a crash in between leaves an unbounded user holding a live access key. PutUserPolicy is conditioned as well as CreateUser deliberately. Condition only the create and the console can still attach a wide inline policy to some pre-existing unbounded user under the path. There is an explicit Deny on the deployment and data buckets, and it is not belt-and-braces. The obvious prefix "carlos-" also matches carlos-<deployment>-<account>, so the natural configuration would have handed the box DeleteBucket on the control plane it runs from. An explicit Deny beats every Allow, so the guard holds whatever prefix an operator picks. Not granted, and each for a reason: PutUserPermissionsBoundary and DeleteUserPermissionsBoundary would let the console remove its own ceiling; AttachUserPolicy would let it bypass the inline document. CARLOS_STORE_USER_BOUNDARY joins region and bucket prefix as boot-fatal when a backend is configured. Without it every grant fails at CreateUser anyway; failing at boot says why once instead of once per tenant. All of it is behind store_bucket_prefix being set, so a deployment that has not switched object storage on gets no new IAM at all.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AuiZoEUxEooEfFBAs8XPLB
6 files changed,
+234
−11
cmd/console/main.go+16 −4infra/modules/region-host/main.tf+114 −0infra/modules/region-host/outputs.tf+5 −0infra/modules/region-host/variables.tf+12 −0internal/objstore/aws.go+28 −4internal/objstore/aws_test.go+59 −3
CI passed — run details
diff --git a/cmd/console/main.go b/cmd/console/main.go| index bda5c2259..95e4206da 100644 |
| --- a/cmd/console/main.go |
| +++ b/cmd/console/main.go |
| @@ -41,9 +41,9 @@ |
| // platform.carlosframework.com for CLI tokens, is simply left off). |
| // |
| // Object storage (issue #55) is off unless CARLOS_STORE_BACKEND is set, in |
| -// which case it must be "aws" and CARLOS_STORE_REGION and |
| -// CARLOS_STORE_BUCKET_PREFIX are required (CARLOS_STORE_ENDPOINT is optional |
| -// — set it for an S3-compatible backend). CARLOS_STORE_OPERATORS |
| +// which case it must be "aws" and CARLOS_STORE_REGION, |
| +// CARLOS_STORE_BUCKET_PREFIX and CARLOS_STORE_USER_BOUNDARY are required |
| +// (CARLOS_STORE_ENDPOINT is optional — set it for an S3-compatible backend). CARLOS_STORE_OPERATORS |
| // (comma-separated addresses) gates the GRANT door only — rotate is the |
| // member's own verb and destroy is the account owner's browser ceremony; |
| // it is read whether or not a backend is configured. |
| @@ -515,8 +515,20 @@ func storeBackend() (objstore.Provisioner, objstore.Deployment) { |
| if !objstore.ValidBucketName(prefix + "aaaa-a") { |
| log.Fatalf("console: CARLOS_STORE_BUCKET_PREFIX %q cannot start an S3 bucket name — lowercase letters, digits, dots and hyphens only, starting alphanumeric (e.g. carlos-)", prefix) |
| } |
| + // The permissions boundary is fatal for the hardest reason on this list. |
| + // Provisioning a store means CreateUser and PutUserPolicy, and a role |
| + // holding those two without a boundary can mint a principal wider than |
| + // itself — the console box's own credentials become a route to the whole |
| + // cloud account. The deployment's IAM conditions both calls on this ARN |
| + // (infra/modules/region-host publishes it as store_user_boundary_arn), so |
| + // a console booted without it would fail every grant at the CreateUser |
| + // step anyway. Failing here says why, once, instead of once per tenant. |
| + boundary := strings.TrimSpace(os.Getenv("CARLOS_STORE_USER_BOUNDARY")) |
| + if boundary == "" { |
| + log.Fatal("console: CARLOS_STORE_BACKEND is set — also set CARLOS_STORE_USER_BOUNDARY (the permissions-boundary policy ARN store users are created under; infra/modules/region-host outputs it as store_user_boundary_arn)") |
| + } |
| endpoint := strings.TrimSpace(os.Getenv("CARLOS_STORE_ENDPOINT")) |
| - p, err := objstore.NewAWSProvisioner(context.Background(), region, endpoint) |
| + p, err := objstore.NewAWSProvisioner(context.Background(), region, endpoint, boundary) |
| if err != nil { |
| log.Fatal(err) |
| } |
diff --git a/infra/modules/region-host/main.tf b/infra/modules/region-host/main.tf| index deb71e96f..cf4b7fd27 100644 |
| --- a/infra/modules/region-host/main.tf |
| +++ b/infra/modules/region-host/main.tf |
| @@ -430,3 +430,117 @@ resource "aws_cloudwatch_metric_alarm" "host_memory_low" { |
| } |
| tags = { Name = "${var.name}-memory-low" } |
| } |
| + |
| +# ---- object storage provisioning (issue #368) ------------------------------ |
| +# |
| +# Everything below exists only when store_bucket_prefix is set. A deployment |
| +# that has not switched object storage on gets none of it, which is why the |
| +# console's own storeBackend() treats an unset CARLOS_STORE_BACKEND as a legal |
| +# deployment rather than a misconfiguration. |
| +# |
| +# The shape is two halves that only make sense together: a grant wide enough |
| +# to provision, and a ceiling narrow enough that the grant cannot be turned |
| +# into anything else. |
| + |
| +data "aws_caller_identity" "current" {} |
| + |
| +# The ceiling. Every store user is created under this boundary (objstore's |
| +# EnsureUser stamps it, and the role policy below REFUSES CreateUser without |
| +# it), so a store credential can never do more than S3 on this deployment's |
| +# own store buckets — no matter what inline policy the console writes. |
| +# |
| +# This is the answer to the escalation the grant would otherwise open: a role |
| +# that may CreateUser and PutUserPolicy can mint a principal wider than |
| +# itself, and scoping those calls to an IAM path does not help, because a path |
| +# says which user is made and nothing about what it may do. |
| +resource "aws_iam_policy" "store_user_boundary" { |
| + count = var.store_bucket_prefix == "" ? 0 : 1 |
| + name = "${var.iam_name}-store-boundary" |
| + description = "Permissions ceiling for CARLOS object-store users. Intersected with each user's inline policy, which narrows it to that user's one bucket." |
| + policy = jsonencode({ |
| + Version = "2012-10-17" |
| + Statement = [ |
| + { |
| + Effect = "Allow" |
| + Action = ["s3:ListBucket", "s3:GetBucketLocation"] |
| + Resource = "arn:aws:s3:::${var.store_bucket_prefix}*" |
| + }, |
| + { |
| + Effect = "Allow" |
| + Action = ["s3:GetObject", "s3:PutObject", "s3:DeleteObject"] |
| + Resource = "arn:aws:s3:::${var.store_bucket_prefix}*/*" |
| + }, |
| + ] |
| + }) |
| +} |
| + |
| +resource "aws_iam_role_policy" "host_store" { |
| + count = var.store_bucket_prefix == "" ? 0 : 1 |
| + name = "${var.iam_name}-store" |
| + role = aws_iam_role.host.id |
| + policy = jsonencode({ |
| + Version = "2012-10-17" |
| + Statement = [ |
| + # Bucket lifecycle, prefix-scoped. EmptyBucket's paginated delete needs |
| + # ListBucket on the bucket and DeleteObject under it; teardown needs |
| + # DeleteBucket. No GetObject: the console provisions stores, it does not |
| + # read tenant data, and the economics sweep sizes buckets with ListBucket. |
| + { |
| + Effect = "Allow" |
| + Action = ["s3:CreateBucket", "s3:DeleteBucket", "s3:ListBucket", "s3:GetBucketLocation"] |
| + Resource = "arn:aws:s3:::${var.store_bucket_prefix}*" |
| + }, |
| + { |
| + Effect = "Allow" |
| + Action = ["s3:DeleteObject"] |
| + Resource = "arn:aws:s3:::${var.store_bucket_prefix}*/*" |
| + }, |
| + # A deployment whose store prefix also matches its OWN buckets — the |
| + # obvious "carlos-" does exactly that against carlos-<deployment>-<acct> |
| + # — would otherwise hand the box DeleteBucket on the control plane it |
| + # runs from. An explicit Deny beats every Allow in IAM, so this holds |
| + # whatever prefix an operator chooses, including a careless one. |
| + { |
| + Effect = "Deny" |
| + Action = ["s3:DeleteBucket", "s3:DeleteObject", "s3:CreateBucket"] |
| + Resource = [ |
| + var.deployment_bucket_arn, |
| + "${var.deployment_bucket_arn}/*", |
| + var.data_bucket_arn, |
| + "${var.data_bucket_arn}/*", |
| + ] |
| + }, |
| + # The two calls that could escalate, each refused unless the user being |
| + # made or modified carries the boundary above. Conditioning PutUserPolicy |
| + # as well as CreateUser matters: without it the console could attach a |
| + # wide inline policy to some pre-existing unbounded user under the path. |
| + { |
| + Effect = "Allow" |
| + Action = ["iam:CreateUser", "iam:PutUserPolicy"] |
| + Resource = "arn:aws:iam::${data.aws_caller_identity.current.account_id}:user${var.store_user_path}*" |
| + Condition = { |
| + StringEquals = { "iam:PermissionsBoundary" = aws_iam_policy.store_user_boundary[0].arn } |
| + } |
| + }, |
| + # The rest of the user lifecycle. None of these can widen a principal, |
| + # so none is boundary-conditioned — but all stay pinned to the store |
| + # user path, so the box can never touch a role or a person's user. |
| + # Deliberately absent: iam:PutUserPermissionsBoundary and |
| + # iam:DeleteUserPermissionsBoundary, which would let the console remove |
| + # the ceiling, and iam:AttachUserPolicy, which would let it bypass the |
| + # inline document entirely. |
| + { |
| + Effect = "Allow" |
| + Action = [ |
| + "iam:GetUser", |
| + "iam:DeleteUser", |
| + "iam:DeleteUserPolicy", |
| + "iam:CreateAccessKey", |
| + "iam:DeleteAccessKey", |
| + "iam:ListAccessKeys", |
| + ] |
| + Resource = "arn:aws:iam::${data.aws_caller_identity.current.account_id}:user${var.store_user_path}*" |
| + }, |
| + ] |
| + }) |
| +} |
diff --git a/infra/modules/region-host/outputs.tf b/infra/modules/region-host/outputs.tf| index 0468a3a3e..0436ff382 100644 |
| --- a/infra/modules/region-host/outputs.tf |
| +++ b/infra/modules/region-host/outputs.tf |
| @@ -27,3 +27,8 @@ output "ami_id" { |
| description = "Resolved AMI id (most recent matching image) — reusable by sibling boxes in the same root." |
| value = data.aws_ami.debian_arm64.id |
| } |
| + |
| +output "store_user_boundary_arn" { |
| + description = "ARN of the permissions boundary store users are created under. Set it as CARLOS_STORE_USER_BOUNDARY on the console, which refuses to boot with a storage backend and no boundary. Null when store_bucket_prefix is empty." |
| + value = var.store_bucket_prefix == "" ? null : aws_iam_policy.store_user_boundary[0].arn |
| +} |
diff --git a/infra/modules/region-host/variables.tf b/infra/modules/region-host/variables.tf| index b508af535..3218de034 100644 |
| --- a/infra/modules/region-host/variables.tf |
| +++ b/infra/modules/region-host/variables.tf |
| @@ -120,3 +120,15 @@ variable "sibling_staging_arns" { |
| type = list(string) |
| default = [] |
| } |
| + |
| +variable "store_bucket_prefix" { |
| + description = "Bucket-name prefix this deployment mints object stores under (CARLOS_STORE_BUCKET_PREFIX, e.g. \"carlos-\"). Set it and the host role gains the object-storage provisioning grant: bucket lifecycle plus the IAM user lifecycle, both scoped to this prefix and to the store user path, and the module creates the permissions boundary those users are made under. Empty (the default): no store grant and no boundary, which is every deployment that has not switched object storage on." |
| + type = string |
| + default = "" |
| +} |
| + |
| +variable "store_user_path" { |
| + description = "IAM path every store principal lives under (objstore.UserPath). It is what scopes the role's user-lifecycle grant to store users and nothing else, so it must match the platform constant exactly — change it here without changing it there and the console can create a user it then cannot give a key to." |
| + type = string |
| + default = "/carlos-store/" |
| +} |
diff --git a/internal/objstore/aws.go b/internal/objstore/aws.go| index bfaaa5993..e494cc360 100644 |
| --- a/internal/objstore/aws.go |
| +++ b/internal/objstore/aws.go |
| @@ -51,6 +51,10 @@ type awsProvisioner struct { |
| s3 s3API |
| iam iamAPI |
| region string |
| + // boundary is the permissions-boundary policy ARN stamped on every store |
| + // user this provisioner creates. See EnsureUser for why it is not |
| + // optional in any deployment whose console role can reach IAM. |
| + boundary string |
| } |
| var _ Provisioner = (*awsProvisioner)(nil) |
| @@ -73,7 +77,7 @@ var _ Provisioner = (*awsProvisioner)(nil) |
| // The IAM client never takes an endpoint override: IAM has no S3-compatible |
| // twin, so a non-AWS backend (endpoint set, "aws" backend) still means |
| // IAM-against-real-AWS in v1 — there is nothing else for it to point at. |
| -func NewAWSProvisioner(ctx context.Context, region, endpoint string) (Provisioner, error) { |
| +func NewAWSProvisioner(ctx context.Context, region, endpoint, boundary string) (Provisioner, error) { |
| s3Client, err := S3ClientFor(ctx, region, endpoint) |
| if err != nil { |
| return nil, err |
| @@ -83,7 +87,7 @@ func NewAWSProvisioner(ctx context.Context, region, endpoint string) (Provisione |
| return nil, fmt.Errorf("objstore: load aws config: %w", err) |
| } |
| iamClient := iam.NewFromConfig(cfg) |
| - return &awsProvisioner{s3: s3Client, iam: iamClient, region: region}, nil |
| + return &awsProvisioner{s3: s3Client, iam: iamClient, region: region, boundary: boundary}, nil |
| } |
| // S3ClientFor builds a bare *s3.Client the same way NewAWSProvisioner builds |
| @@ -279,12 +283,32 @@ func (p *awsProvisioner) DeleteBucket(ctx context.Context, name string) error { |
| // a crash between CreateUser and PutUserPolicy) is resolved to its ARN via |
| // GetUser rather than treated as an error; PutUserPolicy is an idempotent |
| // overwrite, so it always runs regardless of which branch produced the ARN. |
| +// The user is created UNDER A PERMISSIONS BOUNDARY, and that is the load |
| +// bearing half of this function rather than a hardening extra. |
| +// |
| +// A role that may call CreateUser and PutUserPolicy can, without a boundary, |
| +// mint a principal carrying any policy it likes — including one far wider |
| +// than the role itself. Scoping those two actions to a path is not a fix: |
| +// the path constrains WHICH user is made, never what that user may do. So |
| +// the deployment's own IAM conditions both calls on this exact boundary ARN |
| +// (infra/modules/region-host), and the boundary permits nothing but S3 on |
| +// the deployment's own bucket prefix. The inline policy below then narrows |
| +// that ceiling to one bucket; effective permissions are the intersection, |
| +// and the ceiling holds even if this code one day writes the wrong document. |
| +// |
| +// An empty boundary is legal here only because MemProvisioner and the |
| +// contract tests construct provisioners with no IAM at all. Every real |
| +// console refuses to boot without one (cmd/console/main.go, storeBackend). |
| func (p *awsProvisioner) EnsureUser(ctx context.Context, name, path, bucketARN string) (string, error) { |
| var arn string |
| - out, err := p.iam.CreateUser(ctx, &iam.CreateUserInput{ |
| + in := &iam.CreateUserInput{ |
| UserName: aws.String(name), |
| Path: aws.String(path), |
| - }) |
| + } |
| + if p.boundary != "" { |
| + in.PermissionsBoundary = aws.String(p.boundary) |
| + } |
| + out, err := p.iam.CreateUser(ctx, in) |
| if err != nil { |
| var exists *iamtypes.EntityAlreadyExistsException |
| if !errors.As(err, &exists) { |
diff --git a/internal/objstore/aws_test.go b/internal/objstore/aws_test.go| index 20269f4d2..7a762c630 100644 |
| --- a/internal/objstore/aws_test.go |
| +++ b/internal/objstore/aws_test.go |
| @@ -17,7 +17,7 @@ import ( |
| // default credential chain loads without ever making a call), and the |
| // returned value must satisfy Provisioner. |
| func TestNewAWSProvisioner(t *testing.T) { |
| - p, err := NewAWSProvisioner(context.Background(), "eu-west-1", "") |
| + p, err := NewAWSProvisioner(context.Background(), "eu-west-1", "", "") |
| if err != nil { |
| t.Fatalf("NewAWSProvisioner: %v", err) |
| } |
| @@ -35,7 +35,7 @@ func TestNewAWSProvisioner(t *testing.T) { |
| // the unexported s3 field because this is the only seam that observes what |
| // awss3.NewFromConfig actually built, short of a live request. |
| func TestNewAWSProvisionerEndpointOptions(t *testing.T) { |
| - prov, err := NewAWSProvisioner(context.Background(), "eu-west-1", "http://127.0.0.1:9000") |
| + prov, err := NewAWSProvisioner(context.Background(), "eu-west-1", "http://127.0.0.1:9000", "") |
| if err != nil { |
| t.Fatal(err) |
| } |
| @@ -66,7 +66,7 @@ func TestNewAWSProvisionerEndpointOptions(t *testing.T) { |
| // endpoint) leaves those four options untouched, matching s3compat.Options' |
| // own no-op guarantee for a client that talks to real S3. |
| func TestNewAWSProvisionerNoEndpointOptions(t *testing.T) { |
| - prov, err := NewAWSProvisioner(context.Background(), "eu-west-1", "") |
| + prov, err := NewAWSProvisioner(context.Background(), "eu-west-1", "", "") |
| if err != nil { |
| t.Fatal(err) |
| } |
| @@ -572,3 +572,59 @@ func (e *smithyAPIErrorStub) ErrorMessage() string { return e.msg } |
| func (e *smithyAPIErrorStub) ErrorFault() smithy.ErrorFault { return smithy.FaultUnknown } |
| var _ smithy.APIError = (*smithyAPIErrorStub)(nil) |
| + |
| +// TestEnsureUserStampsThePermissionsBoundary: the boundary is the only thing |
| +// standing between "this role may create IAM users" and "this role may create |
| +// an IAM user with any policy at all", so it has to reach CreateUser on the |
| +// exact call that makes the principal — not be attached afterwards, where a |
| +// crash in between leaves an unbounded user holding a live access key. |
| +func TestEnsureUserStampsThePermissionsBoundary(t *testing.T) { |
| + const boundary = "arn:aws:iam::1:policy/carlos-store-boundary" |
| + var seen *string |
| + iamStub := &stubIAM{ |
| + t: t, |
| + createUser: func(in *iam.CreateUserInput) (*iam.CreateUserOutput, error) { |
| + seen = in.PermissionsBoundary |
| + return &iam.CreateUserOutput{User: &iamtypes.User{ |
| + Arn: aws.String("arn:aws:iam::1:user/carlos-store/carlos-store-bab-titogo"), |
| + }}, nil |
| + }, |
| + putUserPolicy: func(*iam.PutUserPolicyInput) (*iam.PutUserPolicyOutput, error) { |
| + return &iam.PutUserPolicyOutput{}, nil |
| + }, |
| + } |
| + p := &awsProvisioner{iam: iamStub, region: "eu-west-1", boundary: boundary} |
| + if _, err := p.EnsureUser(context.Background(), "carlos-store-bab-titogo", UserPath, "arn:aws:s3:::carlos-bab-titogo"); err != nil { |
| + t.Fatalf("EnsureUser: %v", err) |
| + } |
| + if seen == nil { |
| + t.Fatal("CreateUser was called with NO permissions boundary — the deployment's IAM will refuse this call, and if it ever stops refusing, the user is unbounded") |
| + } |
| + if *seen != boundary { |
| + t.Fatalf("PermissionsBoundary = %q, want %q", *seen, boundary) |
| + } |
| +} |
| + |
| +// An empty boundary must send no field at all rather than an empty string: |
| +// MemProvisioner and the contract tests construct provisioners with no IAM, |
| +// and CreateUser rejects an empty PermissionsBoundary outright. |
| +func TestEnsureUserOmitsAnEmptyBoundary(t *testing.T) { |
| + sent := true |
| + iamStub := &stubIAM{ |
| + t: t, |
| + createUser: func(in *iam.CreateUserInput) (*iam.CreateUserOutput, error) { |
| + sent = in.PermissionsBoundary != nil |
| + return &iam.CreateUserOutput{User: &iamtypes.User{Arn: aws.String("arn:aws:iam::1:user/x")}}, nil |
| + }, |
| + putUserPolicy: func(*iam.PutUserPolicyInput) (*iam.PutUserPolicyOutput, error) { |
| + return &iam.PutUserPolicyOutput{}, nil |
| + }, |
| + } |
| + p := &awsProvisioner{iam: iamStub, region: "eu-west-1"} |
| + if _, err := p.EnsureUser(context.Background(), "u", UserPath, "arn:aws:s3:::b"); err != nil { |
| + t.Fatalf("EnsureUser: %v", err) |
| + } |
| + if sent { |
| + t.Fatal("PermissionsBoundary was set from an empty boundary; want the field omitted") |
| + } |
| +} |