fix: temp credentials shouldn't allow policy/group changes (#8675)

This PR fixes the issue where we might allow policy changes
for temporary credentials out of band, this situation allows
privilege escalation for those temporary credentials. We
should disallow any external actions on temporary creds
as a practice and we should clearly differentiate which
are static and which are temporary credentials.

Refer #8667
This commit is contained in:
Harshavardhana 2019-12-19 14:21:21 -08:00 committed by kannappanr
parent d140074773
commit 586614c73f
3 changed files with 77 additions and 10 deletions

View File

@ -69,6 +69,16 @@ func (a adminAPIHandlers) RemoveUser(w http.ResponseWriter, r *http.Request) {
vars := mux.Vars(r) vars := mux.Vars(r)
accessKey := vars["accessKey"] accessKey := vars["accessKey"]
ok, err := globalIAMSys.IsTempUser(accessKey)
if err != nil {
writeErrorResponseJSON(ctx, w, toAdminAPIErr(ctx, err), r.URL)
return
}
if ok {
writeErrorResponseJSON(ctx, w, toAdminAPIErr(ctx, errIAMActionNotAllowed), r.URL)
return
}
if err := globalIAMSys.DeleteUser(accessKey); err != nil { if err := globalIAMSys.DeleteUser(accessKey); err != nil {
writeErrorResponseJSON(ctx, w, toAdminAPIErr(ctx, err), r.URL) writeErrorResponseJSON(ctx, w, toAdminAPIErr(ctx, err), r.URL)
return return
@ -518,6 +528,18 @@ func (a adminAPIHandlers) SetPolicyForUserOrGroup(w http.ResponseWriter, r *http
return return
} }
if !isGroup {
ok, err := globalIAMSys.IsTempUser(entityName)
if err != nil {
writeErrorResponseJSON(ctx, w, toAdminAPIErr(ctx, err), r.URL)
return
}
if ok {
writeErrorResponseJSON(ctx, w, toAdminAPIErr(ctx, errIAMActionNotAllowed), r.URL)
return
}
}
if err := globalIAMSys.PolicyDBSet(entityName, policyName, isGroup); err != nil { if err := globalIAMSys.PolicyDBSet(entityName, policyName, isGroup); err != nil {
writeErrorResponseJSON(ctx, w, toAdminAPIErr(ctx, err), r.URL) writeErrorResponseJSON(ctx, w, toAdminAPIErr(ctx, err), r.URL)
return return

View File

@ -600,20 +600,40 @@ func (sys *IAMSys) ListUsers() (map[string]madmin.UserInfo, error) {
} }
for k, v := range sys.iamUsersMap { for k, v := range sys.iamUsersMap {
users[k] = madmin.UserInfo{ if !v.IsTemp() {
PolicyName: sys.iamUserPolicyMap[k].Policy, users[k] = madmin.UserInfo{
Status: func() madmin.AccountStatus { PolicyName: sys.iamUserPolicyMap[k].Policy,
if v.IsValid() { Status: func() madmin.AccountStatus {
return madmin.AccountEnabled if v.IsValid() {
} return madmin.AccountEnabled
return madmin.AccountDisabled }
}(), return madmin.AccountDisabled
}(),
}
} }
} }
return users, nil return users, nil
} }
// IsTempUser - returns if given key is a temporary user.
func (sys *IAMSys) IsTempUser(name string) (bool, error) {
objectAPI := newObjectLayerWithoutSafeModeFn()
if objectAPI == nil {
return false, errServerNotInitialized
}
sys.RLock()
defer sys.RUnlock()
creds, found := sys.iamUsersMap[name]
if !found {
return false, errNoSuchUser
}
return creds.IsTemp(), nil
}
// GetUserInfo - get info on a user. // GetUserInfo - get info on a user.
func (sys *IAMSys) GetUserInfo(name string) (u madmin.UserInfo, err error) { func (sys *IAMSys) GetUserInfo(name string) (u madmin.UserInfo, err error) {
objectAPI := newObjectLayerWithoutSafeModeFn() objectAPI := newObjectLayerWithoutSafeModeFn()
@ -636,6 +656,10 @@ func (sys *IAMSys) GetUserInfo(name string) (u madmin.UserInfo, err error) {
return u, errNoSuchUser return u, errNoSuchUser
} }
if creds.IsTemp() {
return u, errIAMActionNotAllowed
}
u = madmin.UserInfo{ u = madmin.UserInfo{
PolicyName: sys.iamUserPolicyMap[name].Policy, PolicyName: sys.iamUserPolicyMap[name].Policy,
Status: func() madmin.AccountStatus { Status: func() madmin.AccountStatus {
@ -672,6 +696,10 @@ func (sys *IAMSys) SetUserStatus(accessKey string, status madmin.AccountStatus)
return errNoSuchUser return errNoSuchUser
} }
if cred.IsTemp() {
return errIAMActionNotAllowed
}
uinfo := newUserIdentity(auth.Credentials{ uinfo := newUserIdentity(auth.Credentials{
AccessKey: accessKey, AccessKey: accessKey,
SecretKey: cred.SecretKey, SecretKey: cred.SecretKey,
@ -719,9 +747,15 @@ func (sys *IAMSys) SetUser(accessKey string, uinfo madmin.UserInfo) error {
return errServerNotInitialized return errServerNotInitialized
} }
cr, ok := sys.iamUsersMap[accessKey]
if cr.IsTemp() && ok {
return errIAMActionNotAllowed
}
if err := sys.store.saveUserIdentity(accessKey, false, u); err != nil { if err := sys.store.saveUserIdentity(accessKey, false, u); err != nil {
return err return err
} }
sys.iamUsersMap[accessKey] = u.Credentials sys.iamUsersMap[accessKey] = u.Credentials
// Set policy if specified. // Set policy if specified.
@ -794,10 +828,13 @@ func (sys *IAMSys) AddUsersToGroup(group string, members []string) error {
// Validate that all members exist. // Validate that all members exist.
for _, member := range members { for _, member := range members {
_, ok := sys.iamUsersMap[member] cr, ok := sys.iamUsersMap[member]
if !ok { if !ok {
return errNoSuchUser return errNoSuchUser
} }
if cr.IsTemp() {
return errIAMActionNotAllowed
}
} }
gi, ok := sys.iamGroupsMap[group] gi, ok := sys.iamGroupsMap[group]
@ -856,10 +893,13 @@ func (sys *IAMSys) RemoveUsersFromGroup(group string, members []string) error {
// Validate that all members exist. // Validate that all members exist.
for _, member := range members { for _, member := range members {
_, ok := sys.iamUsersMap[member] cr, ok := sys.iamUsersMap[member]
if !ok { if !ok {
return errNoSuchUser return errNoSuchUser
} }
if cr.IsTemp() {
return errIAMActionNotAllowed
}
} }
gi, ok := sys.iamGroupsMap[group] gi, ok := sys.iamGroupsMap[group]

View File

@ -117,6 +117,11 @@ func (cred Credentials) IsExpired() bool {
return cred.Expiration.Before(time.Now().UTC()) return cred.Expiration.Before(time.Now().UTC())
} }
// IsTemp - returns whether credential is temporary or not.
func (cred Credentials) IsTemp() bool {
return cred.SessionToken != ""
}
// IsValid - returns whether credential is valid or not. // IsValid - returns whether credential is valid or not.
func (cred Credentials) IsValid() bool { func (cred Credentials) IsValid() bool {
// Verify credentials if its enabled or not set. // Verify credentials if its enabled or not set.