Skip to content

Commit

Permalink
support additional claim info in STS calls
Browse files Browse the repository at this point in the history 
Adds a missing AuditLog from AssumeRoleWithCertificate API

Fixes minio#9529
  • Loading branch information
@harshavardhana @minio-trusted
harshavardhana authored and minio-trusted committed Jul 22, 2022
1 parent 2393a13 commit 7b2b0c1
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 59 deletions.
92 changes: 47 additions & 45 deletions cmd/sts-handlers.go
View file
Expand Up @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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 == "" {
Expand All @@ -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 {
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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
Expand All @@ -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)
}

Expand All @@ -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)
}

Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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"))
Expand Down Expand Up @@ -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
Expand Down
23 changes: 11 additions & 12 deletions internal/config/identity/openid/jwt.go
View file
Expand Up @@ -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",
Expand All @@ -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.
Expand All @@ -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.
Expand All @@ -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 {
Expand Down
7 changes: 5 additions & 2 deletions internal/config/identity/openid/jwt_test.go
View file
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
}
}
Expand Down

0 comments on commit 7b2b0c1

Please sign in to comment.