mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-09-08 15:41:15 +02:00
iam: bind CreateServiceAccount ParentUser to the caller (#11218)
* iam: bind CreateServiceAccount target to caller in AuthorizeIamAction A non-admin holding iam:CreateServiceAccount could pass an arbitrary ParentUser and mint a service account for any identity, inheriting that identity permissions. Add a self-target category so a granted non-admin may only target their own identity; admins remain unrestricted. * iam: authorize CreateServiceAccount against its ParentUser target AuthIamManagement passed UserName as the authorization target for every action, so CreateServiceAccount was authorized with an empty target and the self-target binding never saw the caller-supplied ParentUser. Pass ParentUser for that action so the binding takes effect on the live path. * iam: test CreateServiceAccount binds target to caller Regression test: a non-admin holding iam:CreateServiceAccount may target itself but is denied targeting another identity; admins remain unrestricted. * iam: authorize CreateServiceAccount against ParentUser on the S3 port UnifiedPostHandler passed UserName as the authorization target for every IAM action, so CreateServiceAccount was authorized with an empty target on the S3-port route and the self-target binding never saw the caller ParentUser. Extract iamTargetUserName (ParentUser for CreateServiceAccount, UserName otherwise) and use it from both IAM dispatch surfaces so the binding applies on the live S3-port path as well as the standalone iam server. * iam: test CreateServiceAccount ParentUser binding on the S3 port End-to-end regression test through UnifiedPostHandler: a non-admin holding iam:CreateServiceAccount is denied (403) when targeting another identity and passes authorization when targeting itself.
This commit is contained in:
@@ -2474,8 +2474,31 @@ func iamRequiresAdminForOthers(action string) bool {
|
||||
return iamSelfServiceActions[action]
|
||||
}
|
||||
|
||||
// iamSelfTargetActions require an explicit iam:<Action> grant, but a non-admin
|
||||
// grant holder may only target their own identity. The target parameter is
|
||||
// action-specific (UserName for most, ParentUser for CreateServiceAccount).
|
||||
var iamSelfTargetActions = map[string]bool{
|
||||
"CreateServiceAccount": true,
|
||||
}
|
||||
|
||||
func iamRequiresSelfTarget(action string) bool {
|
||||
return iamSelfTargetActions[action]
|
||||
}
|
||||
|
||||
// iamTargetUserName returns the request's target identity for authorization.
|
||||
// Most IAM actions target UserName; CreateServiceAccount targets ParentUser.
|
||||
// Both IAM dispatch surfaces (AuthIamManagement and UnifiedPostHandler) use
|
||||
// this so the authorized target and the acted-on target cannot differ.
|
||||
func iamTargetUserName(action string, r *http.Request) string {
|
||||
if action == "CreateServiceAccount" {
|
||||
return r.PostForm.Get("ParentUser")
|
||||
}
|
||||
return r.PostForm.Get("UserName")
|
||||
}
|
||||
|
||||
// AuthorizeIamAction authorizes an IAM management action for identity, with
|
||||
// targetUserName taken from the request's UserName parameter.
|
||||
// targetUserName taken from the request's target parameter (UserName, or
|
||||
// ParentUser for CreateServiceAccount).
|
||||
//
|
||||
// IAM management is not part of the S3 data plane, so the grant is checked as
|
||||
// iam:<Action>. A coarse S3 action would instead be matched by an ordinary
|
||||
@@ -2496,7 +2519,13 @@ func (iam *IdentityAccessManagement) AuthorizeIamAction(r *http.Request, identit
|
||||
if identity.isAdmin() {
|
||||
return s3err.ErrNone
|
||||
}
|
||||
return iam.VerifyActionPermission(r, identity, Action("iam:"+action), "arn:aws:iam:::*", "")
|
||||
if errCode := iam.VerifyActionPermission(r, identity, Action("iam:"+action), "arn:aws:iam:::*", ""); errCode != s3err.ErrNone {
|
||||
return errCode
|
||||
}
|
||||
if iamRequiresSelfTarget(action) && targetUserName != "" && targetUserName != identity.Name {
|
||||
return s3err.ErrAccessDenied
|
||||
}
|
||||
return s3err.ErrNone
|
||||
}
|
||||
|
||||
// AuthIamManagement authenticates an IAM management request and authorizes the
|
||||
@@ -2530,7 +2559,8 @@ func (iam *IdentityAccessManagement) AuthIamManagement(f http.HandlerFunc) http.
|
||||
|
||||
// UserName comes from the body only, the same place the handlers read it
|
||||
// from, so the authorized target and the acted-on target cannot differ.
|
||||
if errCode := iam.AuthorizeIamAction(r, identity, r.Form.Get("Action"), r.PostForm.Get("UserName")); errCode != s3err.ErrNone {
|
||||
action := r.Form.Get("Action")
|
||||
if errCode := iam.AuthorizeIamAction(r, identity, action, iamTargetUserName(action, r)); errCode != s3err.ErrNone {
|
||||
s3err.WriteErrorResponse(w, r, errCode)
|
||||
return
|
||||
}
|
||||
|
||||
@@ -12,8 +12,9 @@ import (
|
||||
)
|
||||
|
||||
const (
|
||||
dataPlanePolicy = `{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":"s3:*","Resource":"*"}]}`
|
||||
iamAdminPolicy = `{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":"iam:*","Resource":"*"}]}`
|
||||
dataPlanePolicy = `{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":"s3:*","Resource":"*"}]}`
|
||||
iamAdminPolicy = `{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":"iam:*","Resource":"*"}]}`
|
||||
iamCreateSvcAcctPolicy = `{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Action":["iam:CreateServiceAccount"],"Resource":["*"]}]}`
|
||||
)
|
||||
|
||||
func newIamAuthzTestIam(t *testing.T) *IdentityAccessManagement {
|
||||
@@ -93,3 +94,23 @@ func TestAuthorizeIamActionDeniesAnonymous(t *testing.T) {
|
||||
assert.Equal(t, s3err.ErrAccessDenied,
|
||||
iam.AuthorizeIamAction(iamPostRequest(""), anonymous, "CreateUser", "victim"))
|
||||
}
|
||||
|
||||
// CreateServiceAccount takes its target from the ParentUser parameter, not
|
||||
// UserName. A non-admin holding only iam:CreateServiceAccount must not be able
|
||||
// to mint a service account for another identity (privilege escalation to that
|
||||
// identity's permissions).
|
||||
func TestAuthorizeIamActionCreateServiceAccountBindsTargetToCaller(t *testing.T) {
|
||||
iam := newIamAuthzTestIam(t)
|
||||
require.NoError(t, iam.PutPolicy("CreateSvcAcctPolicy", iamCreateSvcAcctPolicy))
|
||||
dev := &Identity{Name: "dev", PolicyNames: []string{"CreateSvcAcctPolicy"}}
|
||||
admin := &Identity{Name: "admin", Actions: []Action{s3_constants.ACTION_ADMIN}}
|
||||
|
||||
assert.Equal(t, s3err.ErrNone,
|
||||
iam.AuthorizeIamAction(iamPostRequest(""), dev, "CreateServiceAccount", "dev"))
|
||||
assert.Equal(t, s3err.ErrNone,
|
||||
iam.AuthorizeIamAction(iamPostRequest(""), dev, "CreateServiceAccount", ""))
|
||||
assert.Equal(t, s3err.ErrAccessDenied,
|
||||
iam.AuthorizeIamAction(iamPostRequest(""), dev, "CreateServiceAccount", "admin"))
|
||||
assert.Equal(t, s3err.ErrNone,
|
||||
iam.AuthorizeIamAction(iamPostRequest(""), admin, "CreateServiceAccount", "victim"))
|
||||
}
|
||||
|
||||
@@ -742,7 +742,7 @@ func (s3a *S3ApiServer) UnifiedPostHandler(w http.ResponseWriter, r *http.Reques
|
||||
|
||||
// UserName comes from the body only, the same place DoActions reads it
|
||||
// from, so the authorized target and the acted-on target cannot differ.
|
||||
if s3a.iam.AuthorizeIamAction(r, identity, action, r.PostForm.Get("UserName")) != s3err.ErrNone {
|
||||
if s3a.iam.AuthorizeIamAction(r, identity, action, iamTargetUserName(action, r)) != s3err.ErrNone {
|
||||
s3err.WriteErrorResponse(w, r, s3err.ErrAccessDenied)
|
||||
return
|
||||
}
|
||||
|
||||
@@ -205,6 +205,72 @@ func TestRouting_AuthenticatedIAM(t *testing.T) {
|
||||
assert.Contains(t, []int{http.StatusBadRequest, http.StatusForbidden}, rr.Code, "Should route to IAM handler (400/403 due to invalid signature)")
|
||||
}
|
||||
|
||||
// setupRoutingTestServerWithCreator seeds a non-admin identity (creator) that
|
||||
// holds only the iam:CreateServiceAccount action, plus a victim identity, so a
|
||||
// SigV4-signed CreateServiceAccount request can exercise UnifiedPostHandler's
|
||||
// authorization against the ParentUser target.
|
||||
func setupRoutingTestServerWithCreator(t *testing.T) *S3ApiServer {
|
||||
s3a := setupRoutingTestServer(t)
|
||||
const creatorAK, creatorSK = "creator-ak", "creator-sk"
|
||||
creator := &Identity{
|
||||
Name: "creator",
|
||||
Actions: []Action{Action("iam:CreateServiceAccount")},
|
||||
IsStatic: true,
|
||||
Credentials: []*Credential{{
|
||||
AccessKey: creatorAK,
|
||||
SecretKey: creatorSK,
|
||||
}},
|
||||
}
|
||||
victim := &Identity{Name: "victim", IsStatic: true}
|
||||
s3a.iam.m.Lock()
|
||||
s3a.iam.identities = append(s3a.iam.identities, creator, victim)
|
||||
s3a.iam.accessKeyIdent[creatorAK] = creator
|
||||
s3a.iam.nameToIdentity["creator"] = creator
|
||||
s3a.iam.nameToIdentity["victim"] = victim
|
||||
s3a.iam.m.Unlock()
|
||||
s3a.cb = NewCircuitBreaker(s3a.option)
|
||||
return s3a
|
||||
}
|
||||
|
||||
// TestRouting_CreateServiceAccountBindsParentUser verifies that on the S3-port
|
||||
// IAM route a non-admin holding iam:CreateServiceAccount cannot mint a service
|
||||
// account for another identity (ParentUser=victim), and can for itself.
|
||||
func TestRouting_CreateServiceAccountBindsParentUser(t *testing.T) {
|
||||
router := mux.NewRouter()
|
||||
s3a := setupRoutingTestServerWithCreator(t)
|
||||
s3a.registerRouter(router)
|
||||
|
||||
signCreator := func(t *testing.T, req *http.Request, body string) {
|
||||
t.Helper()
|
||||
creds := credentials.NewStaticCredentials("creator-ak", "creator-sk", "")
|
||||
if _, err := v4.NewSigner(creds).Sign(req, strings.NewReader(body), "iam", "us-east-1", time.Now()); err != nil {
|
||||
t.Fatalf("sign request: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
makeReq := func(parentUser string) *http.Request {
|
||||
data := url.Values{}
|
||||
data.Set("Action", "CreateServiceAccount")
|
||||
data.Set("Version", "2010-05-08")
|
||||
data.Set("ParentUser", parentUser)
|
||||
body := data.Encode()
|
||||
req, _ := http.NewRequest("POST", "http://localhost/", strings.NewReader(body))
|
||||
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
|
||||
signCreator(t, req, body)
|
||||
return req
|
||||
}
|
||||
|
||||
// Targeting another identity must be denied at authorization.
|
||||
rr := httptest.NewRecorder()
|
||||
router.ServeHTTP(rr, makeReq("victim"))
|
||||
assert.Equal(t, http.StatusForbidden, rr.Code, "cross-identity ParentUser must be denied; got body=%s", rr.Body.String())
|
||||
|
||||
// Targeting self must pass authorization (handler may still error, but not 403).
|
||||
rr2 := httptest.NewRecorder()
|
||||
router.ServeHTTP(rr2, makeReq("creator"))
|
||||
assert.NotEqual(t, http.StatusForbidden, rr2.Code, "self ParentUser must pass authorization; got body=%s", rr2.Body.String())
|
||||
}
|
||||
|
||||
// TestRouting_IAMMatcherLogic verifies the iamMatcher correctly distinguishes auth types
|
||||
func TestRouting_IAMMatcherLogic(t *testing.T) {
|
||||
tests := []struct {
|
||||
|
||||
Reference in New Issue
Block a user