support service accounts for OpenID connect properly (#12178)

OpenID connect generated service accounts do not work
properly after console logout, since the parentUser state
is lost - instead use sub+iss claims for parentUser, according
to OIDC spec both the claims provide the necessary stability
across logins etc.
This commit is contained in:
Harshavardhana 2021-04-29 13:01:42 -07:00 committed by GitHub
parent 8cd89e10ea
commit c5a80ca5d5
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 53 additions and 11 deletions

View file

@ -529,11 +529,14 @@ func (a adminAPIHandlers) AddServiceAccount(w http.ResponseWriter, r *http.Reque
}
// targerUser is set to bindDN at this point in time.
} else {
if targetUser == "" {
targetUser = cred.AccessKey
}
if cred.ParentUser != "" {
if cred.IsServiceAccount() || cred.IsTemp() {
if cred.ParentUser == "" {
writeErrorResponseJSON(ctx, w, toAdminAPIErr(ctx, errors.New("service accounts cannot be generated for temporary credentials without parent")), r.URL)
return
}
targetUser = cred.ParentUser
} else {
targetUser = cred.AccessKey
}
targetGroups = cred.Groups
}
@ -985,7 +988,7 @@ func (a adminAPIHandlers) AccountInfoHandler(w http.ResponseWriter, r *http.Requ
}
policies, err = globalIAMSys.PolicyDBGet(parentUser, false, cred.Groups...)
default:
err = errors.New("should not happen!")
err = errors.New("should never happen")
}
if err != nil {
logger.LogIf(ctx, err)

View file

@ -872,6 +872,24 @@ func (sys *IAMSys) SetTempUser(accessKey string, cred auth.Credentials, policyNa
sys.store.lock()
defer sys.store.unlock()
// We are on purpose not persisting the policy map for parent
// user, although this is a hack, it is a good enoug hack
// at this point in time - we need to overhaul our OIDC
// usage with service accounts with a more cleaner implementation
//
// This mapping is necessary to ensure that valid credentials
// have necessary ParentUser present - this is mainly for only
// webIdentity based STS tokens.
if cred.IsTemp() && cred.ParentUser != "" {
if _, ok := sys.iamUserPolicyMap[cred.ParentUser]; !ok {
if err := sys.store.saveMappedPolicy(context.Background(), accessKey, stsUser, false, mp, options{ttl: ttl}); err != nil {
sys.store.unlock()
return err
}
sys.iamUserPolicyMap[cred.ParentUser] = mp
}
}
if err := sys.store.saveMappedPolicy(context.Background(), accessKey, stsUser, false, mp, options{ttl: ttl}); err != nil {
sys.store.unlock()
return err
@ -1458,8 +1476,10 @@ func (sys *IAMSys) GetUser(accessKey string) (cred auth.Credentials, ok bool) {
defer sys.store.runlock()
if ok && cred.IsValid() {
if cred.ParentUser != "" && sys.usersSysType == MinIOUsersSysType {
_, ok = sys.iamUsersMap[cred.ParentUser]
if cred.IsServiceAccount() || cred.IsTemp() {
// temporary credentials or service accounts
// must have their parent in UsersMap
_, ok = sys.iamUserPolicyMap[cred.ParentUser]
}
// for LDAP service accounts with ParentUser set
// we have no way to validate, either because user

View file

@ -21,6 +21,7 @@ import (
"bytes"
"context"
"encoding/base64"
"errors"
"fmt"
"net/http"
"strings"
@ -57,6 +58,7 @@ const (
// JWT claim keys
expClaim = "exp"
subClaim = "sub"
issClaim = "iss"
// JWT claim to check the parent user
parentClaim = "parent"
@ -322,6 +324,21 @@ func (sts *stsAPIHandlers) AssumeRoleWithSSO(w http.ResponseWriter, r *http.Requ
return
}
var subFromToken string
if v, ok := m[subClaim]; ok {
subFromToken, _ = v.(string)
}
if subFromToken == "" {
writeSTSErrorResponse(ctx, w, true, ErrSTSInvalidParameterValue, errors.New("STS JWT Token has `sub` claim missing, `sub` claim is mandatory"))
return
}
var issFromToken string
if v, ok := m[issClaim]; ok {
issFromToken, _ = v.(string)
}
// JWT has requested a custom claim with policy value set.
// This is a MinIO STS API specific value, this value should
// be set and configured on your identity provider as part of
@ -371,10 +388,12 @@ func (sts *stsAPIHandlers) AssumeRoleWithSSO(w http.ResponseWriter, r *http.Requ
return
}
var subFromToken string
if v, ok := m[subClaim]; ok {
subFromToken, _ = v.(string)
}
// https://openid.net/specs/openid-connect-core-1_0.html#ClaimStability
// claim is only considered stable when subject and iss are used together
// this is to ensure that ParentUser doesn't change and we get to use
// parentUser as per the requirements for service accounts for OpenID
// based logins.
cred.ParentUser = "jwt:" + subFromToken + ":" + issFromToken
// Set the newly generated credentials.
if err = globalIAMSys.SetTempUser(cred.AccessKey, cred, policyName); err != nil {