Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 17 additions & 16 deletions pkg/blobmanager/s3accesspoint/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -296,13 +296,14 @@ func (p *sessionCredentialsProvider) Retrieve(ctx context.Context) (aws.Credenti
return p.ambientCreds.Retrieve(ctx)
}

// Session policy intersects with the base role's permissions; even
// if the role grants accesspoint/*, this session can only touch the
// caller's AP and prefix. The prefix is the requesting-org UUID
// straight from ctx — same source as the session name — so a
// tampered AccessPointARN in the secret blob can't widen the prefix
// scope to escape into another tenant's namespace.
sessionPolicy := buildSessionPolicy(p.creds.AccessPointARN, info.OrgID)
// Session policy intersects with the base role's permissions and
// pins this session to the caller's AP ARN. Cross-tenant defense
// against a tampered AccessPointARN in the secret blob lives in the
// AP's resource policy (aws:userid StringEquals on the role session
// name minted from the request-context org UUID), not here — keeping
// the inline policy small leaves headroom in STS's packed-policy
// budget for tags inherited from the caller principal.
sessionPolicy := buildSessionPolicy(p.creds.AccessPointARN)

out, err := p.stsClient.AssumeRole(ctx, &sts.AssumeRoleInput{
RoleArn: aws.String(p.creds.BaseRoleARN),
Expand Down Expand Up @@ -334,15 +335,15 @@ func roleSessionName(orgUUID string) string {
return "cas-" + orgUUID
}

// buildSessionPolicy returns an IAM policy document that allows only the
// operations the backend actually performs, and only against this
// tenant's AP + key prefix. The Resource ARNs use the AP form
// "${apARN}/object/${keyPrefix}/*".
func buildSessionPolicy(apARN, keyPrefix string) string {
// Minimal, hand-written JSON — keeping it small reduces request
// payload (STS limits session policies to 2048 chars by default).
return fmt.Sprintf(`{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":["s3:GetObject","s3:PutObject","s3:DeleteObject","s3:GetObjectAttributes"],"Resource":"%s/object/%s/*"}]}`,
apARN, keyPrefix)
// buildSessionPolicy returns an IAM policy document allowing only the
// S3 actions the backend actually performs (Get/Head/Put — HeadObject
// is authorized as s3:GetObject) and scoped to this tenant's AP ARN.
// Cross-tenant isolation is enforced by the AP resource policy's
// aws:userid check against the role session name, not by this policy;
// keeping the inline document minimal preserves headroom in the STS
// packed-policy budget against tags inherited from the caller.
func buildSessionPolicy(apARN string) string {
return fmt.Sprintf(`{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":["s3:GetObject","s3:PutObject"],"Resource":"%s/object/*"}]}`, apARN)
}

// hexSha256ToBinaryB64 decodes the hex sha and re-encodes as base64. S3
Expand Down
22 changes: 14 additions & 8 deletions pkg/blobmanager/s3accesspoint/backend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,21 +90,27 @@ func TestBackend_KeyDerivedFromRequestingOrg(t *testing.T) {
require.ErrorIs(t, err, ErrMissingRequestingOrg)
}

// TestSessionPolicy_ScopesToTenantPrefix locks down the session-policy
// generator: the IAM policy minted at AssumeRole time must reference
// both the AP ARN and the tenant key prefix, so a leaked token can't
// touch keys outside its tenant's namespace.
func TestSessionPolicy_ScopesToTenantPrefix(t *testing.T) {
// TestSessionPolicy_ScopesToAccessPoint locks down the session-policy
// generator: the IAM policy minted at AssumeRole time must scope to the
// AP ARN (the tenant boundary lives in the AP resource policy, not
// here), and must not include S3 actions the backend never calls — that
// keeps the inline policy small so STS's packed-policy budget has
// headroom for tags inherited from the caller principal.
func TestSessionPolicy_ScopesToAccessPoint(t *testing.T) {
t.Parallel()

policy := buildSessionPolicy("arn:aws:s3:us-east-1:111:accesspoint/ap-a", "org/A")
policy := buildSessionPolicy("arn:aws:s3:us-east-1:111:accesspoint/ap-a")

assert.Contains(t, policy, `"arn:aws:s3:us-east-1:111:accesspoint/ap-a/object/org/A/*"`,
"policy Resource must be the AP ARN + tenant prefix")
assert.Contains(t, policy, `"arn:aws:s3:us-east-1:111:accesspoint/ap-a/object/*"`,
"policy Resource must be the AP ARN scoped to /object/*")
assert.NotContains(t, policy, `"*"`,
"session policy must not wildcard the Resource")
assert.Contains(t, policy, `"s3:GetObject"`)
assert.Contains(t, policy, `"s3:PutObject"`)
assert.NotContains(t, policy, `"s3:DeleteObject"`,
"backend never deletes — re-adding s3:DeleteObject grows the packed-policy footprint without need")
assert.NotContains(t, policy, `"s3:GetObjectAttributes"`,
"backend never calls GetObjectAttributes — re-adding it grows the packed-policy footprint without need")
}

// TestRoleSessionName_DerivedFromOrg pins the session-name shape that
Expand Down
14 changes: 9 additions & 5 deletions pkg/blobmanager/s3accesspoint/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,16 @@
// resource policy enforces a StringEquals on aws:userid so that a
// session minted for org A cannot read or write to org B's AP — even if
// org A's secret blob has been tampered with to point at org B's ARN.
// This is the actual cross-tenant boundary.
// 3. A per-tenant key prefix derived from the requesting org UUID: every
// object is keyed as <orgUUID>/sha256:<digest> and the AssumeRole
// session policy's Resource is scoped to ${apARN}/object/<orgUUID>/*.
// The prefix shares its source of truth with the session name, so a
// tampered secret cannot reroute a tenant's writes into a different
// namespace.
// object is keyed as <orgUUID>/sha256:<digest>, sharing its source of
// truth (the request-context org claim) with the role session name.
// This avoids cross-tenant collisions at the bucket layer; it is not
// load-bearing for tenant isolation, which is enforced by (1) and (2).
// The AssumeRole session policy's Resource scopes to the AP ARN only
// (${apARN}/object/*), keeping the inline document small so STS's
// packed-policy budget isn't consumed by chainloop before tags
// inherited from the caller principal are even accounted for.
//
// The session name MUST come from the request context, not from the secret
// blob: a secrets-store compromise alone must not let an attacker reroute
Expand Down
Loading