fix: allow STS credentials with dynamic policies (#12681)

- ParentUser for OIDC auth changed to `openid:`
  instead of `jwt:` to avoid clashes with variable
  substitution

- Do not pass in random parents into IsAllowed()
  policy evaluation as it can change the behavior
  of looking for correct policies underneath.

fixes #12676
fixes #12680
This commit is contained in:
Harshavardhana 2021-07-11 17:39:52 -07:00 committed by GitHub
parent e4c3953947
commit cd36019450
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 23 additions and 17 deletions

View File

@ -984,18 +984,13 @@ func (a adminAPIHandlers) AccountInfoHandler(w http.ResponseWriter, r *http.Requ
// Set delimiter value for "s3:delimiter" policy conditionals. // Set delimiter value for "s3:delimiter" policy conditionals.
r.Header.Set("delimiter", SlashSeparator) r.Header.Set("delimiter", SlashSeparator)
parentUser := cred.AccessKey
if cred.ParentUser != "" {
parentUser = cred.ParentUser
}
isAllowedAccess := func(bucketName string) (rd, wr bool) { isAllowedAccess := func(bucketName string) (rd, wr bool) {
if globalIAMSys.IsAllowed(iampolicy.Args{ if globalIAMSys.IsAllowed(iampolicy.Args{
AccountName: parentUser, AccountName: cred.AccessKey,
Groups: cred.Groups, Groups: cred.Groups,
Action: iampolicy.ListBucketAction, Action: iampolicy.ListBucketAction,
BucketName: bucketName, BucketName: bucketName,
ConditionValues: getConditionValues(r, "", parentUser, claims), ConditionValues: getConditionValues(r, "", cred.AccessKey, claims),
IsOwner: owner, IsOwner: owner,
ObjectName: "", ObjectName: "",
Claims: claims, Claims: claims,
@ -1004,11 +999,11 @@ func (a adminAPIHandlers) AccountInfoHandler(w http.ResponseWriter, r *http.Requ
} }
if globalIAMSys.IsAllowed(iampolicy.Args{ if globalIAMSys.IsAllowed(iampolicy.Args{
AccountName: parentUser, AccountName: cred.AccessKey,
Groups: cred.Groups, Groups: cred.Groups,
Action: iampolicy.PutObjectAction, Action: iampolicy.PutObjectAction,
BucketName: bucketName, BucketName: bucketName,
ConditionValues: getConditionValues(r, "", parentUser, claims), ConditionValues: getConditionValues(r, "", cred.AccessKey, claims),
IsOwner: owner, IsOwner: owner,
ObjectName: "", ObjectName: "",
Claims: claims, Claims: claims,

View File

@ -2042,7 +2042,7 @@ func (sys *IAMSys) policyDBGet(name string, isGroup bool) (policies []string, er
// IsAllowedServiceAccount - checks if the given service account is allowed to perform // IsAllowedServiceAccount - checks if the given service account is allowed to perform
// actions. The permission of the parent user is checked first // actions. The permission of the parent user is checked first
func (sys *IAMSys) IsAllowedServiceAccount(args iampolicy.Args, parent string) bool { func (sys *IAMSys) IsAllowedServiceAccount(args iampolicy.Args, parentUser string) bool {
// Now check if we have a subject claim // Now check if we have a subject claim
p, ok := args.Claims[parentClaim] p, ok := args.Claims[parentClaim]
if ok { if ok {
@ -2053,7 +2053,7 @@ func (sys *IAMSys) IsAllowedServiceAccount(args iampolicy.Args, parent string) b
} }
// The parent claim in the session token should be equal // The parent claim in the session token should be equal
// to the parent detected in the backend // to the parent detected in the backend
if parentInClaim != parent { if parentInClaim != parentUser {
return false return false
} }
} else { } else {
@ -2064,7 +2064,7 @@ func (sys *IAMSys) IsAllowedServiceAccount(args iampolicy.Args, parent string) b
} }
// Check policy for this service account. // Check policy for this service account.
svcPolicies, err := sys.PolicyDBGet(parent, false, args.Groups...) svcPolicies, err := sys.PolicyDBGet(parentUser, false, args.Groups...)
if err != nil { if err != nil {
logger.LogIf(GlobalContext, err) logger.LogIf(GlobalContext, err)
return false return false
@ -2097,7 +2097,10 @@ func (sys *IAMSys) IsAllowedServiceAccount(args iampolicy.Args, parent string) b
} }
parentArgs := args parentArgs := args
parentArgs.AccountName = parent parentArgs.AccountName = parentUser
// These are dynamic values set them appropriately.
parentArgs.ConditionValues["username"] = []string{parentUser}
parentArgs.ConditionValues["userid"] = []string{parentUser}
saPolicyClaim, ok := args.Claims[iamPolicyClaimNameSA()] saPolicyClaim, ok := args.Claims[iamPolicyClaimNameSA()]
if !ok { if !ok {
@ -2136,7 +2139,11 @@ func (sys *IAMSys) IsAllowedServiceAccount(args iampolicy.Args, parent string) b
return false return false
} }
// Policy without Version string value reject it. // This can only happen if policy was set but with an empty JSON.
if subPolicy.Version == "" && len(subPolicy.Statements) == 0 {
return combinedPolicy.IsAllowed(parentArgs)
}
if subPolicy.Version == "" { if subPolicy.Version == "" {
return false return false
} }
@ -2255,6 +2262,10 @@ func (sys *IAMSys) IsAllowedSTS(args iampolicy.Args, parentUser string) bool {
availablePolicies[i].Statements...) availablePolicies[i].Statements...)
} }
// These are dynamic values set them appropriately.
args.ConditionValues["username"] = []string{parentUser}
args.ConditionValues["userid"] = []string{parentUser}
// Now check if we have a sessionPolicy. // Now check if we have a sessionPolicy.
spolicy, ok := args.Claims[iampolicy.SessionPolicyName] spolicy, ok := args.Claims[iampolicy.SessionPolicyName]
if ok { if ok {

View File

@ -69,8 +69,8 @@ const (
) )
func parseOpenIDParentUser(parentUser string) (userID string, err error) { func parseOpenIDParentUser(parentUser string) (userID string, err error) {
if strings.HasPrefix(parentUser, "jwt:") { if strings.HasPrefix(parentUser, "openid:") {
tokens := strings.SplitN(strings.TrimPrefix(parentUser, "jwt:"), ":", 2) tokens := strings.SplitN(strings.TrimPrefix(parentUser, "openid:"), ":", 2)
if len(tokens) == 2 { if len(tokens) == 2 {
return tokens[0], nil return tokens[0], nil
} }
@ -408,7 +408,7 @@ func (sts *stsAPIHandlers) AssumeRoleWithSSO(w http.ResponseWriter, r *http.Requ
// this is to ensure that ParentUser doesn't change and we get to use // this is to ensure that ParentUser doesn't change and we get to use
// parentUser as per the requirements for service accounts for OpenID // parentUser as per the requirements for service accounts for OpenID
// based logins. // based logins.
cred.ParentUser = "jwt:" + subFromToken + ":" + issFromToken cred.ParentUser = "openid:" + subFromToken + ":" + issFromToken
// Set the newly generated credentials. // Set the newly generated credentials.
if err = globalIAMSys.SetTempUser(cred.AccessKey, cred, policyName); err != nil { if err = globalIAMSys.SetTempUser(cred.AccessKey, cred, policyName); err != nil {