From f246c9053f9603e610d98439799bdd2a6b293427 Mon Sep 17 00:00:00 2001 From: Aditya Manthramurthy Date: Wed, 11 Dec 2024 18:09:40 -0800 Subject: [PATCH] fix: Privilege escalation in IAM import API (#20756) This API had missing permissions checking, allowing a user to change their policy mapping by: 1. Craft iam-info.zip file: Update own user permission in user_mappings.json 2. Upload it via `mc admin cluster iam import nobody iam-info.zip` Here `nobody` can be a user with pretty much any kind of permission (but not anonymous) and this ends up working. Some more detailed steps - start from a fresh setup: ``` ./minio server /tmp/d{1...4} & mc alias set myminio http://localhost:9000 minioadmin minioadmin mc admin user add myminio nobody nobody123 mc admin policy attach myminio readwrite nobody nobody123 mc alias set nobody http://localhost:9000 nobody nobody123 mc admin cluster iam export myminio mkdir /tmp/x && mv myminio-iam-info.zip /tmp/x cd /tmp/x unzip myminio-iam-info.zip echo '{"nobody":{"version":1,"policy":"consoleAdmin","updatedAt":"2024-08-13T19:47:10.1Z"}}' > \ iam-assets/user_mappings.json zip -r myminio-iam-info-updated.zip iam-assets/ mc admin cluster iam import nobody ./myminio-iam-info-updated.zip mc admin service restart nobody ``` --- cmd/admin-handlers-users.go | 48 ++++--------------------------------- 1 file changed, 4 insertions(+), 44 deletions(-) diff --git a/cmd/admin-handlers-users.go b/cmd/admin-handlers-users.go index a7439f20b..320b186df 100644 --- a/cmd/admin-handlers-users.go +++ b/cmd/admin-handlers-users.go @@ -2032,6 +2032,7 @@ func (a adminAPIHandlers) ExportIAM(w http.ResponseWriter, r *http.Request) { // Get current object layer instance. objectAPI, _ := validateAdminReq(ctx, w, r, policy.ExportIAMAction) if objectAPI == nil { + writeErrorResponseJSON(ctx, w, errorCodes.ToAPIErr(ErrServerNotInitialized), r.URL) return } // Initialize a zip writer which will provide a zipped content @@ -2254,17 +2255,13 @@ func (a adminAPIHandlers) ImportIAMV2(w http.ResponseWriter, r *http.Request) { func (a adminAPIHandlers) importIAM(w http.ResponseWriter, r *http.Request, apiVer string) { ctx := r.Context() - // Get current object layer instance. - objectAPI := newObjectLayerFn() + // Validate signature, permissions and get current object layer instance. + objectAPI, _ := validateAdminReq(ctx, w, r, policy.ImportIAMAction) if objectAPI == nil || globalNotificationSys == nil { writeErrorResponseJSON(ctx, w, errorCodes.ToAPIErr(ErrServerNotInitialized), r.URL) return } - cred, owner, s3Err := validateAdminSignature(ctx, r, "") - if s3Err != ErrNone { - writeErrorResponseJSON(ctx, w, errorCodes.ToAPIErr(s3Err), r.URL) - return - } + data, err := io.ReadAll(r.Body) if err != nil { writeErrorResponseJSON(ctx, w, errorCodes.ToAPIErr(ErrInvalidRequest), r.URL) @@ -2354,38 +2351,12 @@ func (a adminAPIHandlers) importIAM(w http.ResponseWriter, r *http.Request, apiV return } - if (cred.IsTemp() || cred.IsServiceAccount()) && cred.ParentUser == accessKey { - // Incoming access key matches parent user then we should - // reject password change requests. - writeErrorResponseJSON(ctx, w, importErrorWithAPIErr(ctx, ErrAddUserInvalidArgument, err, allUsersFile, accessKey), r.URL) - return - } - // Check if accessKey has beginning and end space characters, this only applies to new users. if !exists && hasSpaceBE(accessKey) { writeErrorResponseJSON(ctx, w, importErrorWithAPIErr(ctx, ErrAdminResourceInvalidArgument, err, allUsersFile, accessKey), r.URL) return } - checkDenyOnly := false - if accessKey == cred.AccessKey { - // Check that there is no explicit deny - otherwise it's allowed - // to change one's own password. - checkDenyOnly = true - } - - if !globalIAMSys.IsAllowed(policy.Args{ - AccountName: cred.AccessKey, - Groups: cred.Groups, - Action: policy.CreateUserAdminAction, - ConditionValues: getConditionValues(r, "", cred), - IsOwner: owner, - Claims: cred.Claims, - DenyOnly: checkDenyOnly, - }) { - writeErrorResponseJSON(ctx, w, importErrorWithAPIErr(ctx, ErrAccessDenied, err, allUsersFile, accessKey), r.URL) - return - } if _, err = globalIAMSys.CreateUser(ctx, accessKey, ureq); err != nil { failed.Users = append(failed.Users, madmin.IAMErrEntity{Name: accessKey, Error: err}) } else { @@ -2485,17 +2456,6 @@ func (a adminAPIHandlers) importIAM(w http.ResponseWriter, r *http.Request, apiV writeErrorResponseJSON(ctx, w, errorCodes.ToAPIErr(ErrAdminResourceInvalidArgument), r.URL) return } - if !globalIAMSys.IsAllowed(policy.Args{ - AccountName: cred.AccessKey, - Groups: cred.Groups, - Action: policy.CreateServiceAccountAdminAction, - ConditionValues: getConditionValues(r, "", cred), - IsOwner: owner, - Claims: cred.Claims, - }) { - writeErrorResponseJSON(ctx, w, importErrorWithAPIErr(ctx, ErrAccessDenied, err, allSvcAcctsFile, user), r.URL) - return - } updateReq := true _, _, err = globalIAMSys.GetServiceAccount(ctx, svcAcctReq.AccessKey) if err != nil {