fix: add service account support for AssumeRole/LDAPIdentity creds (#9451)

allow generating service accounts for temporary credentials
which have a designated parent, currently OpenID is not yet
supported.

added checks to ensure that service account cannot generate
further service accounts for itself, service accounts can
never be a parent to any credential.

cmd/admin-handlers-users.go

@@ -397,7 +397,12 @@ func (a adminAPIHandlers) AddServiceAccount(w http.ResponseWriter, r *http.Reque
 		return
 	}
 
-	newCred, err := globalIAMSys.NewServiceAccount(ctx, cred.AccessKey, createReq.Policy)
+	parentUser := cred.AccessKey
+	if cred.ParentUser != "" {
+		parentUser = cred.ParentUser
+	}
+
+	newCred, err := globalIAMSys.NewServiceAccount(ctx, parentUser, createReq.Policy)
 	if err != nil {
 		writeErrorResponseJSON(ctx, w, toAdminAPIErr(ctx, err), r.URL)
 		return
@@ -516,7 +521,7 @@ func (a adminAPIHandlers) DeleteServiceAccount(w http.ResponseWriter, r *http.Re
 		return
 	}
 
-	if cred.AccessKey != user {
+	if cred.AccessKey != user || cred.ParentUser != user {
 		// The service account belongs to another user but return not found error to mitigate brute force attacks.
 		writeErrorResponseJSON(ctx, w, errorCodes.ToAPIErr(ErrServiceAccountNotFound), r.URL)
 		return

cmd/iam.go

@@ -145,8 +145,8 @@ type UserIdentity struct {
 	Credentials auth.Credentials `json:"credentials"`
 }
 
-func newUserIdentity(creds auth.Credentials) UserIdentity {
-	return UserIdentity{Version: 1, Credentials: creds}
+func newUserIdentity(cred auth.Credentials) UserIdentity {
+	return UserIdentity{Version: 1, Credentials: cred}
 }
 
 // GroupInfo contains info about a group
@@ -440,7 +440,7 @@ func (sys *IAMSys) DeletePolicy(policyName string) error {
 				// This case cannot happen
 				return errNoSuchUser
 			}
-			// User is from STS if the creds are temporary
+			// User is from STS if the cred are temporary
 			if cr.IsTemp() {
 				usersType = append(usersType, stsUser)
 			} else {
@@ -553,6 +553,16 @@ func (sys *IAMSys) DeleteUser(accessKey string) error {
 		return errIAMActionNotAllowed
 	}
 
+	// Delete any service accounts if any first.
+	for _, u := range sys.iamUsersMap {
+		if u.IsServiceAccount() {
+			if u.ParentUser == accessKey {
+				_ = sys.store.deleteUserIdentity(u.AccessKey, srvAccUser)
+				delete(sys.iamUsersMap, u.AccessKey)
+			}
+		}
+	}
+
 	// It is ok to ignore deletion error on the mapped policy
 	sys.store.deleteMappedPolicy(accessKey, regularUser, false)
 	err := sys.store.deleteUserIdentity(accessKey, regularUser)
@@ -564,15 +574,6 @@ func (sys *IAMSys) DeleteUser(accessKey string) error {
 	delete(sys.iamUsersMap, accessKey)
 	delete(sys.iamUserPolicyMap, accessKey)
 
-	for _, u := range sys.iamUsersMap {
-		if u.IsServiceAccount() {
-			if u.ParentUser == accessKey {
-				_ = sys.store.deleteUserIdentity(u.AccessKey, srvAccUser)
-				delete(sys.iamUsersMap, u.AccessKey)
-			}
-		}
-	}
-
 	return err
 }
 
@@ -659,12 +660,12 @@ func (sys *IAMSys) IsTempUser(name string) (bool, error) {
 	sys.store.rlock()
 	defer sys.store.runlock()
 
-	creds, found := sys.iamUsersMap[name]
+	cred, found := sys.iamUsersMap[name]
 	if !found {
 		return false, errNoSuchUser
 	}
 
-	return creds.IsTemp(), nil
+	return cred.IsTemp(), nil
 }
 
 // IsServiceAccount - returns if given key is a service account
@@ -677,13 +678,13 @@ func (sys *IAMSys) IsServiceAccount(name string) (bool, string, error) {
 	sys.store.rlock()
 	defer sys.store.runlock()
 
-	creds, found := sys.iamUsersMap[name]
+	cred, found := sys.iamUsersMap[name]
 	if !found {
 		return false, "", errNoSuchUser
 	}
 
-	if creds.IsServiceAccount() {
-		return true, creds.ParentUser, nil
+	if cred.IsServiceAccount() {
+		return true, cred.ParentUser, nil
 	}
 
 	return false, "", nil
@@ -713,19 +714,19 @@ func (sys *IAMSys) GetUserInfo(name string) (u madmin.UserInfo, err error) {
 		}, nil
 	}
 
-	creds, found := sys.iamUsersMap[name]
+	cred, found := sys.iamUsersMap[name]
 	if !found {
 		return u, errNoSuchUser
 	}
 
-	if creds.IsTemp() {
+	if cred.IsTemp() || cred.IsServiceAccount() {
 		return u, errIAMActionNotAllowed
 	}
 
 	u = madmin.UserInfo{
 		PolicyName: sys.iamUserPolicyMap[name].Policy,
 		Status: func() madmin.AccountStatus {
-			if creds.IsValid() {
+			if cred.IsValid() {
 				return madmin.AccountEnabled
 			}
 			return madmin.AccountDisabled
@@ -758,7 +759,7 @@ func (sys *IAMSys) SetUserStatus(accessKey string, status madmin.AccountStatus)
 		return errNoSuchUser
 	}
 
-	if cred.IsTemp() {
+	if cred.IsTemp() || cred.IsServiceAccount() {
 		return errIAMActionNotAllowed
 	}
 
@@ -806,10 +807,6 @@ func (sys *IAMSys) NewServiceAccount(ctx context.Context, parentUser string, ses
 	sys.store.lock()
 	defer sys.store.unlock()
 
-	if sys.usersSysType != MinIOUsersSysType {
-		return auth.Credentials{}, errIAMActionNotAllowed
-	}
-
 	if parentUser == globalActiveCred.AccessKey {
 		return auth.Credentials{}, errIAMActionNotAllowed
 	}
@@ -819,7 +816,16 @@ func (sys *IAMSys) NewServiceAccount(ctx context.Context, parentUser string, ses
 		return auth.Credentials{}, errNoSuchUser
 	}
 
-	if cr.IsTemp() {
+	// Disallow service accounts to further create more service accounts.
+	if cr.IsServiceAccount() {
+		return auth.Credentials{}, errIAMActionNotAllowed
+	}
+
+	// FIXME: Disallow temporary users with no parent, this is most
+	// probably with OpenID which we don't support to provide
+	// any parent user, LDAPUsersType and MinIOUsersType are
+	// currently supported.
+	if cr.ParentUser == "" && cr.IsTemp() {
 		return auth.Credentials{}, errIAMActionNotAllowed
 	}
 
@@ -838,8 +844,8 @@ func (sys *IAMSys) NewServiceAccount(ctx context.Context, parentUser string, ses
 	if err != nil {
 		return auth.Credentials{}, err
 	}
-
 	cred.ParentUser = parentUser
+
 	u := newUserIdentity(cred)
 
 	if err := sys.store.saveUserIdentity(u.Credentials.AccessKey, srvAccUser, u); err != nil {
@@ -861,10 +867,6 @@ func (sys *IAMSys) ListServiceAccounts(ctx context.Context, accessKey string) ([
 	sys.store.rlock()
 	defer sys.store.runlock()
 
-	if sys.usersSysType != MinIOUsersSysType {
-		return nil, errIAMActionNotAllowed
-	}
-
 	var serviceAccounts []string
 
 	for k, v := range sys.iamUsersMap {
@@ -886,10 +888,6 @@ func (sys *IAMSys) GetServiceAccountParent(ctx context.Context, accessKey string
 	sys.store.rlock()
 	defer sys.store.runlock()
 
-	if sys.usersSysType != MinIOUsersSysType {
-		return "", errIAMActionNotAllowed
-	}
-
 	sa, ok := sys.iamUsersMap[accessKey]
 	if !ok || !sa.IsServiceAccount() {
 		return "", errNoSuchServiceAccount
@@ -908,10 +906,6 @@ func (sys *IAMSys) DeleteServiceAccount(ctx context.Context, accessKey string) e
 	sys.store.lock()
 	defer sys.store.unlock()
 
-	if sys.usersSysType != MinIOUsersSysType {
-		return errIAMActionNotAllowed
-	}
-
 	sa, ok := sys.iamUsersMap[accessKey]
 	if !ok || !sa.IsServiceAccount() {
 		return errNoSuchServiceAccount
@@ -1009,6 +1003,11 @@ func (sys *IAMSys) GetUser(accessKey string) (cred auth.Credentials, ok bool) {
 	defer sys.store.runlock()
 
 	cred, ok = sys.iamUsersMap[accessKey]
+	if ok && cred.IsValid() {
+		if cred.ParentUser != "" {
+			_, ok = sys.iamUsersMap[cred.ParentUser]
+		}
+	}
 	return cred, ok && cred.IsValid()
 }
 

cmd/sts-handlers.go

@@ -22,6 +22,7 @@ import (
 	"encoding/base64"
 	"fmt"
 	"net/http"
+	"strings"
 
 	"github.com/gorilla/mux"
 	"github.com/minio/minio/cmd/config/identity/openid"
@@ -135,6 +136,11 @@ func checkAssumeRoleAuth(ctx context.Context, r *http.Request) (user auth.Creden
 		return user, ErrSTSAccessDenied
 	}
 
+	// Temporary credentials or Service accounts cannot generate further temporary credentials.
+	if user.IsTemp() || user.IsServiceAccount() {
+		return user, ErrSTSAccessDenied
+	}
+
 	return user, ErrSTSNone
 }
 
@@ -207,10 +213,7 @@ func (sts *stsAPIHandlers) AssumeRole(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	policyName := ""
-	if len(policies) > 0 {
-		policyName = policies[0]
-	}
+	policyName := strings.Join(policies, ",")
 
 	// This policy is the policy associated with the user
 	// requesting for temporary credentials. The temporary
@@ -228,6 +231,10 @@ func (sts *stsAPIHandlers) AssumeRole(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	// Set the parent of the temporary access key, this is useful
+	// in obtaining service accounts by this cred.
+	cred.ParentUser = user.AccessKey
+
 	// Set the newly generated credentials.
 	if err = globalIAMSys.SetTempUser(cred.AccessKey, cred, policyName); err != nil {
 		writeSTSErrorResponse(ctx, w, ErrSTSInternalError, err)
@@ -500,9 +507,14 @@ func (sts *stsAPIHandlers) AssumeRoleWithLDAPIdentity(w http.ResponseWriter, r *
 		return
 	}
 
-	policyName := ""
-	// Set the newly generated credentials.
-	if err = globalIAMSys.SetTempUser(cred.AccessKey, cred, policyName); err != nil {
+	// Set the parent of the temporary access key, this is useful
+	// in obtaining service accounts by this cred.
+	cred.ParentUser = ldapUsername
+
+	// Set the newly generated credentials, policyName is empty on purpose
+	// LDAP policies are applied automatically using their ldapUser, ldapGroups
+	// mapping.
+	if err = globalIAMSys.SetTempUser(cred.AccessKey, cred, ""); err != nil {
 		writeSTSErrorResponse(ctx, w, ErrSTSInternalError, err)
 		return
 	}

pkg/auth/credentials.go

@@ -120,7 +120,7 @@ func (cred Credentials) IsExpired() bool {
 
 // IsTemp - returns whether credential is temporary or not.
 func (cred Credentials) IsTemp() bool {
-	return cred.SessionToken != "" && cred.ParentUser == "" && !cred.Expiration.IsZero() && !cred.Expiration.Equal(timeSentinel)
+	return cred.SessionToken != "" && !cred.Expiration.IsZero() && !cred.Expiration.Equal(timeSentinel)
 }
 
 // IsServiceAccount - returns whether credential is a service account or not