Skip to content

Commit 3f09fbd

Browse files
committed
feat(blobmanager/s3accesspoint): optional managed session policy via PolicyArns
Add an optional Credentials.SessionPolicyARN field. When set, the per-request sts:AssumeRole call references a customer-managed IAM policy via PolicyArns instead of inlining a JSON policy document. Only the ARN counts against STS's packed-policy budget that way, leaving more headroom for session tags inherited from the caller principal (IRSA / Pod Identity). The field is optional and backwards-compatible: when empty, the backend continues to send the slim inline session policy. A missing ARN must NOT degrade to an unscoped session that inherits the full BaseRoleARN permissions, so the inline fallback is always present. Validation rejects values that are not IAM managed policy ARNs (e.g. S3 ARNs, role ARNs) at credential-load time rather than surfacing as opaque STS errors deep in the upload path. The sts.Client field on sessionCredentialsProvider is narrowed to a small stsAssumer interface so tests can lock down the AssumeRole call shape (inline Policy vs PolicyArns) without spinning up a real AWS config. No production behavior change. Assisted-by: Claude Signed-off-by: Jose I. Paris <jiparis@chainloop.dev> Chainloop-Trace-Sessions: ee4385fa-8011-4750-87cd-502097dda8b9
1 parent 7334f48 commit 3f09fbd

4 files changed

Lines changed: 165 additions & 14 deletions

File tree

‎pkg/blobmanager/s3accesspoint/backend.go‎

Lines changed: 35 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import (
3030
"github.com/aws/aws-sdk-go-v2/service/s3"
3131
s3types "github.com/aws/aws-sdk-go-v2/service/s3/types"
3232
"github.com/aws/aws-sdk-go-v2/service/sts"
33+
ststypes "github.com/aws/aws-sdk-go-v2/service/sts/types"
3334
"github.com/aws/smithy-go"
3435
pb "github.com/chainloop-dev/chainloop/app/artifact-cas/api/cas/v1"
3536
robotaccount "github.com/chainloop-dev/chainloop/internal/robotaccount/cas"
@@ -47,6 +48,13 @@ const (
4748
// be useless against an AP policy condition.
4849
var ErrMissingRequestingOrg = errors.New("s3accesspoint: requesting org missing from claims")
4950

51+
// stsAssumer is the subset of *sts.Client that the credentials provider
52+
// actually uses. Keeping the dependency at interface-level lets tests
53+
// inject a fake without spinning up a real AWS config.
54+
type stsAssumer interface {
55+
AssumeRole(ctx context.Context, in *sts.AssumeRoleInput, optFns ...func(*sts.Options)) (*sts.AssumeRoleOutput, error)
56+
}
57+
5058
// Backend is the per-tenant uploader/downloader. One *Backend instance is
5159
// bound to one access point; the actual AWS credentials are minted
5260
// per-request via STS using the org UUID found in the request context.
@@ -56,7 +64,7 @@ type Backend struct {
5664
// stsClient is built once at construction using the pod's ambient
5765
// IAM identity. The credential chain (IRSA → IMDS → env →
5866
// ~/.aws/credentials) picks up the identity automatically.
59-
stsClient *sts.Client
67+
stsClient stsAssumer
6068

6169
// s3Client uses a custom CredentialsProvider that mints a scoped
6270
// session per request (cached in-process per requesting-org so back-
@@ -261,7 +269,7 @@ func (b *Backend) CheckWritePermissions(ctx context.Context) error {
261269
// reusing the temporary credentials across consecutive calls until the
262270
// expiration window approaches.
263271
type sessionCredentialsProvider struct {
264-
stsClient *sts.Client
272+
stsClient stsAssumer
265273

266274
// ambientCreds is the SDK-default credentials provider captured from
267275
// awsCfg at construction time. Only consulted when
@@ -296,21 +304,34 @@ func (p *sessionCredentialsProvider) Retrieve(ctx context.Context) (aws.Credenti
296304
return p.ambientCreds.Retrieve(ctx)
297305
}
298306

299-
// Session policy intersects with the base role's permissions and
300-
// pins this session to the caller's AP ARN. Cross-tenant defense
301-
// against a tampered AccessPointARN in the secret blob lives in the
302-
// AP's resource policy (aws:userid StringEquals on the role session
303-
// name minted from the request-context org UUID), not here — keeping
304-
// the inline policy small leaves headroom in STS's packed-policy
305-
// budget for tags inherited from the caller principal.
306-
sessionPolicy := buildSessionPolicy(p.creds.AccessPointARN)
307-
308-
out, err := p.stsClient.AssumeRole(ctx, &sts.AssumeRoleInput{
307+
// The session policy intersects with the base role's permissions and
308+
// pins this session to the caller's AP. Cross-tenant defense against
309+
// a tampered AccessPointARN in the secret blob lives in the AP's
310+
// resource policy (aws:userid StringEquals on the role session name
311+
// minted from the request-context org UUID), not here.
312+
//
313+
// When the operator has provisioned a managed IAM policy and
314+
// recorded its ARN in SessionPolicyARN, reference it via PolicyArns
315+
// instead of inlining a JSON document. Only the ARN counts against
316+
// STS's packed-policy budget that way, leaving more headroom for
317+
// session tags inherited from the caller principal (IRSA / Pod
318+
// Identity). When SessionPolicyARN is empty we fall back to the
319+
// inline default — a missing ARN must NOT degrade to an unscoped
320+
// session that inherits the full BaseRoleARN permissions.
321+
input := &sts.AssumeRoleInput{
309322
RoleArn: aws.String(p.creds.BaseRoleARN),
310323
RoleSessionName: aws.String(roleSessionName(info.OrgID)),
311-
Policy: aws.String(sessionPolicy),
312324
DurationSeconds: aws.Int32(int32(SessionDuration.Seconds())),
313-
})
325+
}
326+
if p.creds.SessionPolicyARN != "" {
327+
input.PolicyArns = []ststypes.PolicyDescriptorType{
328+
{Arn: aws.String(p.creds.SessionPolicyARN)},
329+
}
330+
} else {
331+
input.Policy = aws.String(buildSessionPolicy(p.creds.AccessPointARN))
332+
}
333+
334+
out, err := p.stsClient.AssumeRole(ctx, input)
314335
if err != nil {
315336
return aws.Credentials{}, fmt.Errorf("sts:AssumeRole for org %s: %w", info.OrgID, err)
316337
}

‎pkg/blobmanager/s3accesspoint/backend_test.go‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,11 @@ import (
1919
"bytes"
2020
"context"
2121
"testing"
22+
"time"
2223

2324
"github.com/aws/aws-sdk-go-v2/aws"
25+
"github.com/aws/aws-sdk-go-v2/service/sts"
26+
ststypes "github.com/aws/aws-sdk-go-v2/service/sts/types"
2427
pb "github.com/chainloop-dev/chainloop/app/artifact-cas/api/cas/v1"
2528
robotaccount "github.com/chainloop-dev/chainloop/internal/robotaccount/cas"
2629
jwtmiddleware "github.com/go-kratos/kratos/v2/middleware/auth/jwt"
@@ -174,6 +177,83 @@ func (c *countingCredsProvider) Retrieve(_ context.Context) (aws.Credentials, er
174177
return c.creds, nil
175178
}
176179

180+
// fakeSTSAssumer captures the AssumeRoleInput passed by the credentials
181+
// provider so tests can lock down the AssumeRole call shape (inline
182+
// policy vs PolicyArns) without making a real AWS call.
183+
type fakeSTSAssumer struct {
184+
lastInput *sts.AssumeRoleInput
185+
}
186+
187+
func (f *fakeSTSAssumer) AssumeRole(_ context.Context, in *sts.AssumeRoleInput, _ ...func(*sts.Options)) (*sts.AssumeRoleOutput, error) {
188+
f.lastInput = in
189+
return &sts.AssumeRoleOutput{
190+
Credentials: &ststypes.Credentials{
191+
AccessKeyId: aws.String("AKFAKE"),
192+
SecretAccessKey: aws.String("secret"),
193+
SessionToken: aws.String("token"),
194+
Expiration: aws.Time(time.Now().Add(time.Hour)),
195+
},
196+
}, nil
197+
}
198+
199+
// TestAssumeRoleInput_DefaultsToInlinePolicy locks down the fallback
200+
// path: with an empty SessionPolicyARN the AssumeRole call must carry an
201+
// inline Policy and must NOT pass PolicyArns — otherwise an upgrade
202+
// that forgets to set the new field would silently degrade to a session
203+
// scoped only by the BaseRoleARN's identity policies.
204+
func TestAssumeRoleInput_DefaultsToInlinePolicy(t *testing.T) {
205+
t.Parallel()
206+
207+
fake := &fakeSTSAssumer{}
208+
p := &sessionCredentialsProvider{
209+
stsClient: fake,
210+
creds: &Credentials{
211+
AccessPointARN: "arn:aws:s3:us-east-1:111:accesspoint/ap-a",
212+
BaseRoleARN: "arn:aws:iam::111:role/r",
213+
},
214+
}
215+
ctx := jwtmiddleware.NewContext(context.Background(),
216+
&robotaccount.Claims{OrgID: "org-A", StoredSecretID: "foo", BackendType: "BT", Role: robotaccount.Uploader})
217+
218+
_, err := p.Retrieve(ctx)
219+
require.NoError(t, err)
220+
require.NotNil(t, fake.lastInput, "stub STS must have been invoked")
221+
222+
require.NotNil(t, fake.lastInput.Policy, "empty SessionPolicyARN must produce an inline Policy")
223+
assert.Contains(t, *fake.lastInput.Policy, "arn:aws:s3:us-east-1:111:accesspoint/ap-a/object/*")
224+
assert.Empty(t, fake.lastInput.PolicyArns, "inline path must not also pass PolicyArns")
225+
}
226+
227+
// TestAssumeRoleInput_UsesPolicyArnsWhenSet locks down the opt-in path:
228+
// a configured SessionPolicyARN must reach STS via PolicyArns, and the
229+
// inline Policy must be omitted so we don't double-count against the
230+
// packed-policy budget.
231+
func TestAssumeRoleInput_UsesPolicyArnsWhenSet(t *testing.T) {
232+
t.Parallel()
233+
234+
const managedARN = "arn:aws:iam::111:policy/chainloop-cas-session"
235+
fake := &fakeSTSAssumer{}
236+
p := &sessionCredentialsProvider{
237+
stsClient: fake,
238+
creds: &Credentials{
239+
AccessPointARN: "arn:aws:s3:us-east-1:111:accesspoint/ap-a",
240+
BaseRoleARN: "arn:aws:iam::111:role/r",
241+
SessionPolicyARN: managedARN,
242+
},
243+
}
244+
ctx := jwtmiddleware.NewContext(context.Background(),
245+
&robotaccount.Claims{OrgID: "org-A", StoredSecretID: "foo", BackendType: "BT", Role: robotaccount.Uploader})
246+
247+
_, err := p.Retrieve(ctx)
248+
require.NoError(t, err)
249+
require.NotNil(t, fake.lastInput, "stub STS must have been invoked")
250+
251+
assert.Nil(t, fake.lastInput.Policy, "PolicyArns path must NOT also send an inline Policy")
252+
require.Len(t, fake.lastInput.PolicyArns, 1, "expected exactly one PolicyArn descriptor")
253+
require.NotNil(t, fake.lastInput.PolicyArns[0].Arn)
254+
assert.Equal(t, managedARN, *fake.lastInput.PolicyArns[0].Arn)
255+
}
256+
177257
// --- helpers -----------------------------------------------------------
178258

179259
// newTestBackend constructs a fully wired *Backend that uses static dummy

‎pkg/blobmanager/s3accesspoint/provider.go‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,13 @@
3434
// (${apARN}/object/*), keeping the inline document small so STS's
3535
// packed-policy budget isn't consumed by chainloop before tags
3636
// inherited from the caller principal are even accounted for.
37+
// 4. Optionally, a customer-managed IAM policy referenced via
38+
// Credentials.SessionPolicyARN. When set, the per-request AssumeRole
39+
// call passes that ARN through PolicyArns instead of an inline Policy
40+
// document; only the ~60-char ARN counts against the packed-policy
41+
// budget, leaving more headroom for caller-inherited session tags.
42+
// The managed policy is the action allowlist in that mode, so it
43+
// MUST be at least as restrictive as the inline default.
3744
//
3845
// The session name MUST come from the request context, not from the secret
3946
// blob: a secrets-store compromise alone must not let an attacker reroute
@@ -113,6 +120,15 @@ type Credentials struct {
113120
// accounts without a config change. Required unless DevModeEnvVar is
114121
// set on the running binary.
115122
BaseRoleARN string
123+
// SessionPolicyARN is optional. When non-empty, the per-request
124+
// AssumeRole call passes this ARN via PolicyArns instead of an
125+
// inline Policy document, trimming chainloop's contribution to
126+
// STS's packed-policy budget down to the ARN string itself. The
127+
// IAM policy at this ARN MUST be at least as restrictive as the
128+
// inline default (s3:GetObject + s3:PutObject scoped to the AP's
129+
// /object/* prefix). When empty, the backend falls back to the
130+
// inline session policy.
131+
SessionPolicyARN string
116132
}
117133

118134
func (c *Credentials) Validate() error {
@@ -136,6 +152,15 @@ func (c *Credentials) Validate() error {
136152
return fmt.Errorf("%w: base_role_arn %q is not a valid IAM role ARN", backend.ErrValidation, c.BaseRoleARN)
137153
}
138154
}
155+
// SessionPolicyARN is optional. When set it must look like a managed
156+
// IAM policy ARN — anything else (an S3 ARN, a role ARN) would be
157+
// silently rejected by STS at request time, surfacing as opaque
158+
// errors deep in the upload path. Fail loudly here instead.
159+
if c.SessionPolicyARN != "" {
160+
if !strings.HasPrefix(c.SessionPolicyARN, "arn:aws:iam::") || !strings.Contains(c.SessionPolicyARN, ":policy/") {
161+
return fmt.Errorf("%w: session_policy_arn %q is not a valid IAM managed policy ARN", backend.ErrValidation, c.SessionPolicyARN)
162+
}
163+
}
139164
return nil
140165
}
141166

‎pkg/blobmanager/s3accesspoint/provider_test.go‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,31 @@ func TestCredentials_Validate(t *testing.T) {
6767
mutate: func(c *Credentials) { c.BaseRoleARN = "not-an-arn" },
6868
wantErr: "not a valid IAM role ARN",
6969
},
70+
{
71+
name: "session policy arn empty is allowed",
72+
mutate: func(c *Credentials) { c.SessionPolicyARN = "" },
73+
wantErr: "",
74+
},
75+
{
76+
name: "session policy arn valid",
77+
mutate: func(c *Credentials) { c.SessionPolicyARN = "arn:aws:iam::123456789012:policy/chainloop-cas-session" },
78+
wantErr: "",
79+
},
80+
{
81+
name: "session policy arn pointing at s3 ARN rejected",
82+
mutate: func(c *Credentials) { c.SessionPolicyARN = "arn:aws:s3:::some-bucket" },
83+
wantErr: "not a valid IAM managed policy ARN",
84+
},
85+
{
86+
name: "session policy arn pointing at role rejected",
87+
mutate: func(c *Credentials) { c.SessionPolicyARN = "arn:aws:iam::123456789012:role/some-role" },
88+
wantErr: "not a valid IAM managed policy ARN",
89+
},
90+
{
91+
name: "session policy arn garbage rejected",
92+
mutate: func(c *Credentials) { c.SessionPolicyARN = "not-an-arn-at-all" },
93+
wantErr: "not a valid IAM managed policy ARN",
94+
},
7095
}
7196
for _, tc := range tests {
7297
t.Run(tc.name, func(t *testing.T) {

0 commit comments

Comments
 (0)