From 7b2b0c1335af5cdf915a48b4f638ef5e7bd3b43b Mon Sep 17 00:00:00 2001 From: Harshavardhana Date: Thu, 21 Jul 2022 22:17:10 -0700 Subject: [PATCH] support additional claim info in STS calls Adds a missing AuditLog from AssumeRoleWithCertificate API Fixes #9529 --- cmd/sts-handlers.go | 92 +++++++++++---------- internal/config/identity/openid/jwt.go | 23 +++--- internal/config/identity/openid/jwt_test.go | 7 +- 3 files changed, 63 insertions(+), 59 deletions(-) diff --git a/cmd/sts-handlers.go b/cmd/sts-handlers.go index a335298253c51..0516c224f7022 100644 --- a/cmd/sts-handlers.go +++ b/cmd/sts-handlers.go @@ -183,6 +183,9 @@ func parseForm(r *http.Request) error { func (sts *stsAPIHandlers) AssumeRole(w http.ResponseWriter, r *http.Request) { ctx := newContext(r, w, "AssumeRole") + claims := make(map[string]interface{}) + defer logger.AuditLog(ctx, w, r, claims) + // Check auth here (otherwise r.Form will have unexpected values from // the call to `parseForm` below), but return failure only after we are // able to validate that it is a valid STS request, so that we are able @@ -208,7 +211,6 @@ func (sts *stsAPIHandlers) AssumeRole(w http.ResponseWriter, r *http.Request) { } ctx = newContext(r, w, action) - defer logger.AuditLog(ctx, w, r, nil) // Validate the authentication result here so that failures will be // audit-logged. @@ -246,25 +248,22 @@ func (sts *stsAPIHandlers) AssumeRole(w http.ResponseWriter, r *http.Request) { return } - m := map[string]interface{}{ - expClaim: UTCNow().Add(duration).Unix(), - parentClaim: user.AccessKey, - } + claims[expClaim] = UTCNow().Add(duration).Unix() + claims[parentClaim] = user.AccessKey // 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 { + if _, err = globalIAMSys.PolicyDBGet(user.AccessKey, false); err != nil { writeSTSErrorResponse(ctx, w, true, ErrSTSInvalidParameterValue, err) return } if len(sessionPolicyStr) > 0 { - m[iampolicy.SessionPolicyName] = base64.StdEncoding.EncodeToString([]byte(sessionPolicyStr)) + claims[iampolicy.SessionPolicyName] = base64.StdEncoding.EncodeToString([]byte(sessionPolicyStr)) } secret := globalActiveCred.SecretKey - cred, err := auth.GetNewCredentialsWithMetadata(m, secret) + cred, err := auth.GetNewCredentialsWithMetadata(claims, secret) if err != nil { writeSTSErrorResponse(ctx, w, true, ErrSTSInternalError, err) return @@ -310,6 +309,9 @@ func (sts *stsAPIHandlers) AssumeRole(w http.ResponseWriter, r *http.Request) { func (sts *stsAPIHandlers) AssumeRoleWithSSO(w http.ResponseWriter, r *http.Request) { ctx := newContext(r, w, "AssumeRoleSSOCommon") + claims := make(map[string]interface{}) + defer logger.AuditLog(ctx, w, r, claims) + // Parse the incoming form data. if err := parseForm(r); err != nil { writeSTSErrorResponse(ctx, w, true, ErrSTSInvalidParameterValue, err) @@ -333,7 +335,6 @@ func (sts *stsAPIHandlers) AssumeRoleWithSSO(w http.ResponseWriter, r *http.Requ } ctx = newContext(r, w, action) - defer logger.AuditLog(ctx, w, r, nil) token := r.Form.Get(stsToken) if token == "" { @@ -356,8 +357,7 @@ func (sts *stsAPIHandlers) AssumeRoleWithSSO(w http.ResponseWriter, r *http.Requ } // Validate JWT; check clientID in claims matches the one associated with the roleArn - m, err := globalOpenIDConfig.Validate(roleArn, token, accessToken, r.Form.Get(stsDurationSeconds)) - if err != nil { + if err := globalOpenIDConfig.Validate(roleArn, token, accessToken, r.Form.Get(stsDurationSeconds), claims); err != nil { switch err { case openid.ErrTokenExpired: switch action { @@ -379,13 +379,13 @@ func (sts *stsAPIHandlers) AssumeRoleWithSSO(w http.ResponseWriter, r *http.Requ if globalIAMSys.HasRolePolicy() { // If roleArn is used, we set it as a claim, and use the // associated policy when credentials are used. - m[roleArnClaim] = roleArn.String() + claims[roleArnClaim] = roleArn.String() } else { // If no role policy is configured, then we use claims from the // JWT. This is a MinIO STS API specific value, this value // should be set and configured on your identity provider as // part of JWT custom claims. - policySet, ok := iampolicy.GetPoliciesFromClaims(m, iamPolicyClaimNameOpenID()) + policySet, ok := iampolicy.GetPoliciesFromClaims(claims, iamPolicyClaimNameOpenID()) policies := strings.Join(policySet.ToSlice(), ",") if ok { policyName = globalIAMSys.CurrentPolicies(policies) @@ -402,7 +402,7 @@ func (sts *stsAPIHandlers) AssumeRoleWithSSO(w http.ResponseWriter, r *http.Requ return } } - m[iamPolicyClaimNameOpenID()] = policyName + claims[iamPolicyClaimNameOpenID()] = policyName } sessionPolicyStr := r.Form.Get(stsPolicy) @@ -427,11 +427,11 @@ func (sts *stsAPIHandlers) AssumeRoleWithSSO(w http.ResponseWriter, r *http.Requ return } - m[iampolicy.SessionPolicyName] = base64.StdEncoding.EncodeToString([]byte(sessionPolicyStr)) + claims[iampolicy.SessionPolicyName] = base64.StdEncoding.EncodeToString([]byte(sessionPolicyStr)) } secret := globalActiveCred.SecretKey - cred, err := auth.GetNewCredentialsWithMetadata(m, secret) + cred, err := auth.GetNewCredentialsWithMetadata(claims, secret) if err != nil { writeSTSErrorResponse(ctx, w, true, ErrSTSInternalError, err) return @@ -443,7 +443,7 @@ func (sts *stsAPIHandlers) AssumeRoleWithSSO(w http.ResponseWriter, r *http.Requ // parentUser as per the requirements for service accounts for OpenID // based logins. var subFromToken string - if v, ok := m[subClaim]; ok { + if v, ok := claims[subClaim]; ok { subFromToken, _ = v.(string) } @@ -454,7 +454,7 @@ func (sts *stsAPIHandlers) AssumeRoleWithSSO(w http.ResponseWriter, r *http.Requ } var issFromToken string - if v, ok := m[issClaim]; ok { + if v, ok := claims[issClaim]; ok { issFromToken, _ = v.(string) } @@ -540,7 +540,8 @@ func (sts *stsAPIHandlers) AssumeRoleWithClientGrants(w http.ResponseWriter, r * func (sts *stsAPIHandlers) AssumeRoleWithLDAPIdentity(w http.ResponseWriter, r *http.Request) { ctx := newContext(r, w, "AssumeRoleWithLDAPIdentity") - defer logger.AuditLog(ctx, w, r, nil, stsLDAPPassword) + claims := make(map[string]interface{}) + defer logger.AuditLog(ctx, w, r, claims, stsLDAPPassword) // Parse the incoming form data. if err := parseForm(r); err != nil { @@ -615,18 +616,16 @@ func (sts *stsAPIHandlers) AssumeRoleWithLDAPIdentity(w http.ResponseWriter, r * return } - m := map[string]interface{}{ - expClaim: UTCNow().Add(expiryDur).Unix(), - ldapUser: ldapUserDN, - ldapUserN: ldapUsername, - } + claims[expClaim] = UTCNow().Add(expiryDur).Unix() + claims[ldapUser] = ldapUserDN + claims[ldapUserN] = ldapUsername if len(sessionPolicyStr) > 0 { - m[iampolicy.SessionPolicyName] = base64.StdEncoding.EncodeToString([]byte(sessionPolicyStr)) + claims[iampolicy.SessionPolicyName] = base64.StdEncoding.EncodeToString([]byte(sessionPolicyStr)) } secret := globalActiveCred.SecretKey - cred, err := auth.GetNewCredentialsWithMetadata(m, secret) + cred, err := auth.GetNewCredentialsWithMetadata(claims, secret) if err != nil { writeSTSErrorResponse(ctx, w, true, ErrSTSInternalError, err) return @@ -682,6 +681,9 @@ func (sts *stsAPIHandlers) AssumeRoleWithLDAPIdentity(w http.ResponseWriter, r * func (sts *stsAPIHandlers) AssumeRoleWithCertificate(w http.ResponseWriter, r *http.Request) { ctx := newContext(r, w, "AssumeRoleWithCertificate") + claims := make(map[string]interface{}) + defer logger.AuditLog(ctx, w, r, claims) + if !globalSTSTLSConfig.Enabled { writeSTSErrorResponse(ctx, w, true, ErrSTSNotInitialized, errors.New("STS API 'AssumeRoleWithCertificate' is disabled")) return @@ -789,13 +791,13 @@ func (sts *stsAPIHandlers) AssumeRoleWithCertificate(w http.ResponseWriter, r *h // Associate any service accounts to the certificate CN 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, - }, globalActiveCred.SecretKey) + claims[expClaim] = UTCNow().Add(expiry).Unix() + claims[subClaim] = certificate.Subject.CommonName + claims[audClaim] = certificate.Subject.Organization + claims[issClaim] = certificate.Issuer.CommonName + claims[parentClaim] = parentUser + + tmpCredentials, err := auth.GetNewCredentialsWithMetadata(claims, globalActiveCred.SecretKey) if err != nil { writeSTSErrorResponse(ctx, w, true, ErrSTSInternalError, err) return @@ -838,6 +840,9 @@ func (sts *stsAPIHandlers) AssumeRoleWithCertificate(w http.ResponseWriter, r *h func (sts *stsAPIHandlers) AssumeRoleWithCustomToken(w http.ResponseWriter, r *http.Request) { ctx := newContext(r, w, "AssumeRoleWithCustomToken") + claims := make(map[string]interface{}) + defer logger.AuditLog(ctx, w, r, claims) + if globalAuthNPlugin == nil { writeSTSErrorResponse(ctx, w, true, ErrSTSNotInitialized, errors.New("STS API 'AssumeRoleWithCustomToken' is disabled")) return @@ -849,8 +854,6 @@ func (sts *stsAPIHandlers) AssumeRoleWithCustomToken(w http.ResponseWriter, r *h return } - defer logger.AuditLog(ctx, w, r, nil) - token := r.Form.Get(stsToken) if token == "" { writeSTSErrorResponse(ctx, w, true, ErrSTSInvalidParameterValue, fmt.Errorf("Invalid empty `Token` parameter provided")) @@ -904,21 +907,20 @@ func (sts *stsAPIHandlers) AssumeRoleWithCustomToken(w http.ResponseWriter, r *h parentUser := "custom:" + res.Success.User // metadata map - m := map[string]interface{}{ - expClaim: UTCNow().Add(time.Duration(expiry) * time.Second).Unix(), - parentClaim: parentUser, - subClaim: parentUser, - roleArnClaim: roleArn.String(), - } + claims[expClaim] = UTCNow().Add(time.Duration(expiry) * time.Second).Unix() + claims[subClaim] = parentUser + claims[roleArnClaim] = roleArn.String() + claims[parentClaim] = parentUser + // Add all other claims from the plugin **without** replacing any // existing claims. for k, v := range res.Success.Claims { - if _, ok := m[k]; !ok { - m[k] = v + if _, ok := claims[k]; !ok { + claims[k] = v } } - tmpCredentials, err := auth.GetNewCredentialsWithMetadata(m, globalActiveCred.SecretKey) + tmpCredentials, err := auth.GetNewCredentialsWithMetadata(claims, globalActiveCred.SecretKey) if err != nil { writeSTSErrorResponse(ctx, w, true, ErrSTSInternalError, err) return diff --git a/internal/config/identity/openid/jwt.go b/internal/config/identity/openid/jwt.go index 145fbc07c8a65..6e455184197c8 100644 --- a/internal/config/identity/openid/jwt.go +++ b/internal/config/identity/openid/jwt.go @@ -131,7 +131,7 @@ const ( ) // Validate - validates the id_token. -func (r *Config) Validate(arn arn.ARN, token, accessToken, dsecs string) (map[string]interface{}, error) { +func (r *Config) Validate(arn arn.ARN, token, accessToken, dsecs string, claims jwtgo.MapClaims) error { jp := new(jwtgo.Parser) jp.ValidMethods = []string{ "RS256", "RS384", "RS512", "ES256", "ES384", "ES512", @@ -148,33 +148,32 @@ func (r *Config) Validate(arn arn.ARN, token, accessToken, dsecs string) (map[st pCfg, ok := r.arnProviderCfgsMap[arn] if !ok { - return nil, fmt.Errorf("Role %s does not exist", arn) + return fmt.Errorf("Role %s does not exist", arn) } - var claims jwtgo.MapClaims jwtToken, err := jp.ParseWithClaims(token, &claims, keyFuncCallback) if err != nil { // Re-populate the public key in-case the JWKS // pubkeys are refreshed if err = r.PopulatePublicKey(arn); err != nil { - return nil, err + return err } jwtToken, err = jwtgo.ParseWithClaims(token, &claims, keyFuncCallback) if err != nil { - return nil, err + return err } } if !jwtToken.Valid { - return nil, ErrTokenExpired + return ErrTokenExpired } if err = updateClaimsExpiry(dsecs, claims); err != nil { - return nil, err + return err } if err = r.updateUserinfoClaims(arn, accessToken, claims); err != nil { - return nil, err + return err } // Validate that matching clientID appears in the aud or azp claims. @@ -188,7 +187,7 @@ func (r *Config) Validate(arn arn.ARN, token, accessToken, dsecs string) (map[st // case sensitive audValues, ok := iampolicy.GetValuesFromClaims(claims, audClaim) if !ok { - return nil, errors.New("STS JWT Token has `aud` claim invalid, `aud` must match configured OpenID Client ID") + return errors.New("STS JWT Token has `aud` claim invalid, `aud` must match configured OpenID Client ID") } if !audValues.Contains(pCfg.ClientID) { // if audience claims is missing, look for "azp" claims. @@ -202,14 +201,14 @@ func (r *Config) Validate(arn arn.ARN, token, accessToken, dsecs string) (map[st // string containing a StringOrURI value azpValues, ok := iampolicy.GetValuesFromClaims(claims, azpClaim) if !ok { - return nil, errors.New("STS JWT Token has `azp` claim invalid, `azp` must match configured OpenID Client ID") + return errors.New("STS JWT Token has `azp` claim invalid, `azp` must match configured OpenID Client ID") } if !azpValues.Contains(pCfg.ClientID) { - return nil, errors.New("STS JWT Token has `azp` claim invalid, `azp` must match configured OpenID Client ID") + return errors.New("STS JWT Token has `azp` claim invalid, `azp` must match configured OpenID Client ID") } } - return claims, nil + return nil } func (r *Config) updateUserinfoClaims(arn arn.ARN, accessToken string, claims map[string]interface{}) error { diff --git a/internal/config/identity/openid/jwt_test.go b/internal/config/identity/openid/jwt_test.go index 44425ff110c2e..6a74b6b2b083b 100644 --- a/internal/config/identity/openid/jwt_test.go +++ b/internal/config/identity/openid/jwt_test.go @@ -29,6 +29,7 @@ import ( "time" jwtg "github.com/golang-jwt/jwt/v4" + jwtgo "github.com/golang-jwt/jwt/v4" "github.com/minio/minio/internal/arn" "github.com/minio/minio/internal/config" jwtm "github.com/minio/minio/internal/jwt" @@ -106,7 +107,8 @@ func TestJWTAzureFail(t *testing.T) { }, } - if _, err := cfg.Validate(DummyRoleARN, jwtToken, "", ""); err == nil { + var claims jwtgo.MapClaims + if err = cfg.Validate(DummyRoleARN, jwtToken, "", "", claims); err == nil { // Azure should fail due to non OIDC compliant JWT // generated by Azure AD t.Fatal(err) @@ -159,7 +161,8 @@ func TestJWT(t *testing.T) { t.Fatal(err) } - if _, err := cfg.Validate(DummyRoleARN, u.Query().Get("Token"), "", ""); err == nil { + var claims jwtgo.MapClaims + if err = cfg.Validate(DummyRoleARN, u.Query().Get("Token"), "", "", claims); err == nil { t.Fatal(err) } }