From fa432f9a6a75427054a854c104048a84d9d431ff Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Fri, 31 Jul 2026 19:50:59 -0700 Subject: [PATCH] s3: keep an admin's role session scoped to the role (#10520) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * s3: keep an admin's role session scoped to the role AssumeRole copied the caller's admin standing into the minted session as the is_admin claim, which short-circuits base policy evaluation. An admin assuming a scoped-down role therefore kept full access and the role's attached policies, explicit denies included, were never evaluated. Only a session the caller assumed for itself carries the claim now — a legacy static admin has no IAM policies for such a session to inherit. * s3: name the caller when it assumes a session for itself An identity that carries no principal ARN left the self-assumed session with an empty role name in its assumed-role ARN. callerPrincipalArn synthesizes the canonical user ARN for that case. --- weed/s3api/s3api_sts.go | 13 +- .../s3api_sts_assume_role_admin_scope_test.go | 138 ++++++++++++++++++ 2 files changed, 148 insertions(+), 3 deletions(-) create mode 100644 weed/s3api/s3api_sts_assume_role_admin_scope_test.go diff --git a/weed/s3api/s3api_sts.go b/weed/s3api/s3api_sts.go index 5e217eede..c2f68ef8d 100644 --- a/weed/s3api/s3api_sts.go +++ b/weed/s3api/s3api_sts.go @@ -388,6 +388,8 @@ func (h *STSHandlers) handleAssumeRole(w http.ResponseWriter, r *http.Request) { glog.V(2).Infof("AssumeRole: caller identity=%s, roleArn=%s, sessionName=%s", identity.Name, roleArn, roleSessionName) + assumesSelf := roleArn == "" + // A named role is authorized by its trust policy, which declares which // principals may assume it, so no separate identity-side sts:AssumeRole allow // is required. An explicit identity-side deny still wins (deny-always-wins). @@ -415,7 +417,9 @@ func (h *STSHandlers) handleAssumeRole(w http.ResponseWriter, r *http.Request) { return } } else { - roleArn = identity.PrincipalArn + // Synthesize the caller ARN when the identity carries none, else the + // session ends up with an empty role name in its assumed-role ARN. + roleArn = h.callerPrincipalArn(identity) glog.V(2).Infof("AssumeRole: no RoleArn provided, defaulting to caller identity: %s", roleArn) if authErr := h.iam.VerifyActionPermission(r, identity, Action(sts.ActionAssumeRole), "", ""); authErr != s3err.ErrNone { glog.Warningf("AssumeRole: caller %s attempted to assume role without RoleArn and lacks global sts:AssumeRole permission", identity.Name) @@ -431,9 +435,12 @@ func (h *STSHandlers) handleAssumeRole(w http.ResponseWriter, r *http.Request) { return } - // Prepare custom claims for the session + // is_admin lets the session bypass base policy evaluation, so it may only + // travel into a session the caller assumed for itself — a legacy static admin + // carries no IAM policies for such a session to inherit. Assuming a named role + // scopes the session to that role's policies, admin caller or not. var modifyClaims func(claims *sts.STSSessionClaims) - if identity.isAdmin() { + if assumesSelf && identity.isAdmin() { modifyClaims = func(claims *sts.STSSessionClaims) { if claims.RequestContext == nil { claims.RequestContext = make(map[string]interface{}) diff --git a/weed/s3api/s3api_sts_assume_role_admin_scope_test.go b/weed/s3api/s3api_sts_assume_role_admin_scope_test.go new file mode 100644 index 000000000..2551054f7 --- /dev/null +++ b/weed/s3api/s3api_sts_assume_role_admin_scope_test.go @@ -0,0 +1,138 @@ +package s3api + +import ( + "context" + "encoding/xml" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/seaweedfs/seaweedfs/weed/iam/integration" + "github.com/seaweedfs/seaweedfs/weed/iam/policy" + "github.com/seaweedfs/seaweedfs/weed/pb/iam_pb" +) + +// An admin that assumes a named role gets the role's permissions, not its own: +// the is_admin claim bypasses base policy evaluation, so carrying it into a +// role session would silently ignore the role's attached policies. +func TestAssumeRole_AdminCallerScopedToRolePolicies(t *testing.T) { + ctx := context.Background() + manager := newTestSTSIntegrationManager(t) + manager.SetSessionRevocationStore(integration.NewMemorySessionRevocationStore()) + + require.NoError(t, manager.CreatePolicy(ctx, "", "ReadOnlyPolicy", &policy.PolicyDocument{ + Version: "2012-10-17", + Statement: []policy.Statement{{ + Effect: "Allow", + Action: []string{"s3:GetObject"}, + Resource: []string{"arn:aws:s3:::*/*"}, + }}, + })) + require.NoError(t, manager.CreateRole(ctx, "", "ReadOnlyRole", &integration.RoleDefinition{ + RoleName: "ReadOnlyRole", + TrustPolicy: &policy.PolicyDocument{ + Version: "2012-10-17", + Statement: []policy.Statement{{Effect: "Allow", Principal: "*", Action: []string{"sts:AssumeRole"}}}, + }, + AttachedPolicies: []string{"ReadOnlyPolicy"}, + })) + + const accessKey, secretKey = "adminkey", "adminsecret" + iam := &IdentityAccessManagement{iamIntegration: NewS3IAMIntegration(manager, "")} + require.NoError(t, iam.loadS3ApiConfiguration(&iam_pb.S3ApiConfiguration{ + Identities: []*iam_pb.Identity{{ + Name: "admin", + Credentials: []*iam_pb.Credential{{AccessKey: accessKey, SecretKey: secretKey}}, + Actions: []string{"Admin"}, + }}, + })) + + body := url.Values{ + "Action": {"AssumeRole"}, + "Version": {"2011-06-15"}, + "RoleArn": {"arn:aws:iam::" + defaultAccountID + ":role/ReadOnlyRole"}, + "RoleSessionName": {"admin-session"}, + }.Encode() + req, err := newTestRequest(http.MethodPost, "http://sts.seaweedfs.test/", int64(len(body)), strings.NewReader(body)) + require.NoError(t, err) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + require.NoError(t, signRequestV4(req, accessKey, secretKey)) + + rec := httptest.NewRecorder() + NewSTSHandlers(manager.GetSTSService(), iam).handleAssumeRole(rec, req) + require.Equal(t, http.StatusOK, rec.Code, rec.Body.String()) + + var resp AssumeRoleResponse + require.NoError(t, xml.Unmarshal(rec.Body.Bytes(), &resp)) + sessionToken := resp.Result.Credentials.SessionToken + require.NotEmpty(t, sessionToken) + + session, err := manager.GetSTSService().ValidateSessionToken(ctx, sessionToken) + require.NoError(t, err) + assert.NotContains(t, session.RequestContext, "is_admin", "a role session must not inherit the caller's admin standing") + + allowed := func(action, resource string) bool { + t.Helper() + ok, err := manager.IsActionAllowed(ctx, &integration.ActionRequest{ + Principal: session.Principal, + Action: action, + Resource: resource, + SessionToken: sessionToken, + RequestContext: session.RequestContext, + PolicyNames: session.Policies, + }) + require.NoError(t, err) + return ok + } + + assert.True(t, allowed("s3:GetObject", "arn:aws:s3:::bucket/key"), "the role's own policy still applies") + assert.False(t, allowed("s3:DeleteBucket", "arn:aws:s3:::bucket"), "the role's policy bounds the session") +} + +// Assuming a session for oneself is the one path that keeps the caller's admin +// standing, and it has to name the caller even when the identity carries no +// principal ARN of its own — otherwise the session ARN has an empty role name. +func TestAssumeRole_SelfAssumptionWithoutPrincipalArn(t *testing.T) { + ctx := context.Background() + manager := newTestSTSIntegrationManager(t) + + const accessKey, secretKey = "adminkey", "adminsecret" + iam := &IdentityAccessManagement{iamIntegration: NewS3IAMIntegration(manager, "")} + require.NoError(t, iam.loadS3ApiConfiguration(&iam_pb.S3ApiConfiguration{ + Identities: []*iam_pb.Identity{{ + Name: "admin", + Credentials: []*iam_pb.Credential{{AccessKey: accessKey, SecretKey: secretKey}}, + Actions: []string{"Admin"}, + }}, + })) + for _, ident := range iam.identities { + ident.PrincipalArn = "" + } + + body := url.Values{ + "Action": {"AssumeRole"}, + "Version": {"2011-06-15"}, + "RoleSessionName": {"self-session"}, + }.Encode() + req, err := newTestRequest(http.MethodPost, "http://sts.seaweedfs.test/", int64(len(body)), strings.NewReader(body)) + require.NoError(t, err) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + require.NoError(t, signRequestV4(req, accessKey, secretKey)) + + rec := httptest.NewRecorder() + NewSTSHandlers(manager.GetSTSService(), iam).handleAssumeRole(rec, req) + require.Equal(t, http.StatusOK, rec.Code, rec.Body.String()) + + var resp AssumeRoleResponse + require.NoError(t, xml.Unmarshal(rec.Body.Bytes(), &resp)) + assert.Equal(t, "arn:aws:sts::"+defaultAccountID+":assumed-role/admin/self-session", resp.Result.AssumedRoleUser.Arn) + + session, err := manager.GetSTSService().ValidateSessionToken(ctx, resp.Result.Credentials.SessionToken) + require.NoError(t, err) + assert.Equal(t, true, session.RequestContext["is_admin"], "a self-assumed session keeps the caller's admin standing") +}