diff --git a/cmd/auth-handler.go b/cmd/auth-handler.go index d6b7ba884..4e1206a21 100644 --- a/cmd/auth-handler.go +++ b/cmd/auth-handler.go @@ -238,20 +238,6 @@ func getClaimsFromToken(token string) (map[string]interface{}, error) { claims.MapClaims[iampolicy.SessionPolicyName] = string(spBytes) } - // If LDAP claim key is set, return here. - if _, ok := claims.MapClaims[ldapUser]; ok { - return claims.Map(), nil - } - - // Session token must have a policy, reject requests without policy - // claim. - _, pokOpenIDClaimName := claims.MapClaims[iamPolicyClaimNameOpenID()] - _, pokOpenIDRoleArn := claims.MapClaims[roleArnClaim] - _, pokSA := claims.MapClaims[iamPolicyClaimNameSA()] - if !pokOpenIDClaimName && !pokOpenIDRoleArn && !pokSA { - return nil, errAuthentication - } - return claims.Map(), nil } diff --git a/cmd/iam-store.go b/cmd/iam-store.go index 3e2c48809..6bb971149 100644 --- a/cmd/iam-store.go +++ b/cmd/iam-store.go @@ -1392,9 +1392,11 @@ func (store *IAMStoreSys) DeleteUser(ctx context.Context, accessKey string, user return err } -// SetTempUser - saves temporary credential to storage and cache. +// SetTempUser - saves temporary (STS) credential to storage and cache. If a +// policy name is given, it is associated with the parent user specified in the +// credential. func (store *IAMStoreSys) SetTempUser(ctx context.Context, accessKey string, cred auth.Credentials, policyName string) error { - if accessKey == "" || !cred.IsTemp() || cred.IsExpired() { + if accessKey == "" || !cred.IsTemp() || cred.IsExpired() || cred.ParentUser == "" { return errInvalidArgument } @@ -1411,26 +1413,12 @@ func (store *IAMStoreSys) SetTempUser(ctx context.Context, accessKey string, cre return fmt.Errorf("specified policy %s, not found %w", policyName, errNoSuchPolicy) } - err := store.saveMappedPolicy(ctx, accessKey, stsUser, false, mp, options{ttl: ttl}) + err := store.saveMappedPolicy(ctx, cred.ParentUser, stsUser, false, mp, options{ttl: ttl}) if err != nil { return err } - cache.iamUserPolicyMap[accessKey] = mp - - // We are on purpose not persisting the policy map for parent - // user, although this is a hack, it is a good enough 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.ParentUser != "" && cred.ParentUser != globalActiveCred.AccessKey { - if _, ok := cache.iamUserPolicyMap[cred.ParentUser]; !ok { - cache.iamUserPolicyMap[cred.ParentUser] = mp - } - } + cache.iamUserPolicyMap[cred.ParentUser] = mp } u := newUserIdentity(cred) diff --git a/cmd/iam.go b/cmd/iam.go index 9249047b5..07debbe6c 100644 --- a/cmd/iam.go +++ b/cmd/iam.go @@ -568,7 +568,45 @@ func (sys *IAMSys) notifyForUser(ctx context.Context, accessKey string, isTemp b } } -// SetTempUser - set temporary user credentials, these credentials have an expiry. +// SetTempUser - set temporary user credentials, these credentials have an +// expiry. The permissions for these STS credentials is determined in one of the +// following ways: +// +// - RoleARN - if a role-arn is specified in the request, the STS credential's +// policy is the role's policy. +// +// - inherited from parent - this is the case for AssumeRole API, where the +// parent user is an actual real user with their own (permanent) credentials and +// policy association. +// +// - inherited from "virtual" parent - this is the case for AssumeRoleWithLDAP +// where the parent user is the DN of the actual LDAP user. The parent user +// itself cannot login, but the policy associated with them determines the base +// policy for the STS credential. The policy mapping can be updated by the +// administrator. +// +// - from `Subject.CommonName` field from the STS request for +// AssumeRoleWithCertificate. In this case, the policy for the STS credential +// has the same name as the value of this field. +// +// - from special JWT claim from STS request for AssumeRoleWithOIDC API (when +// not using RoleARN). The claim value can be a string or a list and refers to +// the names of access policies. +// +// For all except the RoleARN case, the implementation is the same - the policy +// for the STS credential is associated with a parent user. For the +// AssumeRoleWithCertificate case, the "virtual" parent user is the value of the +// `Subject.CommonName` field. For the OIDC (without RoleARN) case the "virtual" +// parent is derived as a concatenation of the `sub` and `iss` fields. The +// policies applicable to the STS credential are associated with this "virtual" +// parent. +// +// When a policyName is given to this function, the policy association is +// created and stored in the IAM store. Thus, it should NOT be given for the +// role-arn case (because the role-to-policy mapping is separately stored +// elsewhere), the AssumeRole case (because the parent user is real and their +// policy is associated via policy-set API) and the AssumeRoleWithLDAP case +// (because the policy association is made via policy-set API). func (sys *IAMSys) SetTempUser(ctx context.Context, accessKey string, cred auth.Credentials, policyName string) error { if !sys.Initialized() { return errServerNotInitialized @@ -1388,34 +1426,26 @@ func (sys *IAMSys) IsAllowedSTS(args iampolicy.Args, parentUser string) bool { } policies = newMappedPolicy(sys.rolesMap[arn]).toSlice() } else { - // If roleArn is not used, we fall back to using policy claim - // from JWT. - policySet, ok := args.GetPolicies(iamPolicyClaimNameOpenID()) + // Lookup the parent user's mapping if there's no role-ARN. + mp, ok := sys.store.GetMappedPolicy(parentUser, false) if !ok { - // When claims are set, it should have a policy claim field. - return false - } - // When claims are set, it should have policies as claim. - if policySet.IsEmpty() { - // No policy, no access! - return false + // TODO (deprecated in Dec 2021): Only need to handle + // behavior for STS credentials created in older + // releases. Otherwise, reject such cases, once older + // behavior is deprecated. + + // If there is no parent policy mapping, we fall back to + // using policy claim from JWT. + policySet, ok := args.GetPolicies(iamPolicyClaimNameOpenID()) + if !ok { + // When claims are set, it should have a policy claim field. + return false + } + policies = policySet.ToSlice() + } else { + policies = mp.toSlice() } - // If policy is available for given user, check the policy. - mp, ok := sys.store.GetMappedPolicy(args.AccountName, false) - if !ok { - // No policy set for the user that we can find, no access! - return false - } - - if !policySet.Equals(mp.policySet()) { - // When claims has a policy, it should match the - // policy of args.AccountName which server remembers. - // if not reject such requests. - return false - } - - policies = policySet.ToSlice() } combinedPolicy, err := sys.store.GetPolicy(strings.Join(policies, ",")) diff --git a/cmd/sts-handlers.go b/cmd/sts-handlers.go index 59afcc362..1aa2cf185 100644 --- a/cmd/sts-handlers.go +++ b/cmd/sts-handlers.go @@ -20,6 +20,7 @@ package cmd import ( "bytes" "context" + "crypto/sha256" "crypto/x509" "encoding/base64" "errors" @@ -245,22 +246,18 @@ func (sts *stsAPIHandlers) AssumeRole(w http.ResponseWriter, r *http.Request) { } m := map[string]interface{}{ - expClaim: UTCNow().Add(duration).Unix(), + expClaim: UTCNow().Add(duration).Unix(), + parentClaim: user.AccessKey, } - policies, err := globalIAMSys.PolicyDBGet(user.AccessKey, false) + // Validate that user.AccessKey's policies can be retrieved - it may not + // be in case the user is disabled. + _, err = globalIAMSys.PolicyDBGet(user.AccessKey, false) if err != nil { writeSTSErrorResponse(ctx, w, true, ErrSTSInvalidParameterValue, err) return } - policyName := strings.Join(policies, ",") - - // This policy is the policy associated with the user - // requesting for temporary credentials. The temporary - // credentials will inherit the same policy requirements. - m[iamPolicyClaimNameOpenID()] = policyName - if len(sessionPolicyStr) > 0 { m[iampolicy.SessionPolicyName] = base64.StdEncoding.EncodeToString([]byte(sessionPolicyStr)) } @@ -272,12 +269,12 @@ 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. + // Set the parent of the temporary access key, so that it's access + // policy is inherited from `user.AccessKey`. cred.ParentUser = user.AccessKey // Set the newly generated credentials. - if err = globalIAMSys.SetTempUser(ctx, cred.AccessKey, cred, policyName); err != nil { + if err = globalIAMSys.SetTempUser(ctx, cred.AccessKey, cred, ""); err != nil { writeSTSErrorResponse(ctx, w, true, ErrSTSInternalError, err) return } @@ -483,7 +480,17 @@ func (sts *stsAPIHandlers) AssumeRoleWithSSO(w http.ResponseWriter, r *http.Requ issFromToken, _ = v.(string) } - cred.ParentUser = "openid:" + subFromToken + ":" + issFromToken + // Since issFromToken can have `/` characters (it is typically the + // provider URL), we hash and encode it to base64 here. This is needed + // because there will be a policy mapping stored on drives whose + // filename is this parentUser: therefore, it needs to have only valid + // filename characters and needs to have bounded length. + { + h := sha256.New() + h.Write([]byte("openid:" + subFromToken + ":" + issFromToken)) + bs := h.Sum(nil) + cred.ParentUser = base64.RawURLEncoding.EncodeToString(bs) + } // Set the newly generated credentials. if err = globalIAMSys.SetTempUser(ctx, cred.AccessKey, cred, policyName); err != nil { @@ -787,12 +794,11 @@ func (sts *stsAPIHandlers) AssumeRoleWithCertificate(w http.ResponseWriter, r *h parentUser := "tls:" + certificate.Subject.CommonName tmpCredentials, err := auth.GetNewCredentialsWithMetadata(map[string]interface{}{ - expClaim: UTCNow().Add(expiry).Unix(), - parentClaim: parentUser, - subClaim: certificate.Subject.CommonName, - audClaim: certificate.Subject.Organization, - issClaim: certificate.Issuer.CommonName, - iamPolicyClaimNameOpenID(): certificate.Subject.CommonName, + expClaim: UTCNow().Add(expiry).Unix(), + parentClaim: parentUser, + subClaim: certificate.Subject.CommonName, + audClaim: certificate.Subject.Organization, + issClaim: certificate.Issuer.CommonName, }, globalActiveCred.SecretKey) if err != nil { writeSTSErrorResponse(ctx, w, true, ErrSTSInternalError, err)