diff --git a/cmd/auth-handler.go b/cmd/auth-handler.go index 4e1206a21..5bfb332b3 100644 --- a/cmd/auth-handler.go +++ b/cmd/auth-handler.go @@ -197,13 +197,7 @@ func mustGetClaimsFromToken(r *http.Request) map[string]interface{} { return claims } -// Fetch claims in the security token returned by the client. -func getClaimsFromToken(token string) (map[string]interface{}, error) { - if token == "" { - claims := xjwt.NewMapClaims() - return claims.Map(), nil - } - +func getClaimsFromTokenWithSecret(token, secret string) (map[string]interface{}, error) { // JWT token for x-amz-security-token is signed with admin // secret key, temporary credentials become invalid if // server admin credentials change. This is done to ensure @@ -212,9 +206,15 @@ func getClaimsFromToken(token string) (map[string]interface{}, error) { // hijacking the policies. We need to make sure that this is // based an admin credential such that token cannot be decoded // on the client side and is treated like an opaque value. - claims, err := auth.ExtractClaims(token, globalActiveCred.SecretKey) + claims, err := auth.ExtractClaims(token, secret) if err != nil { - return nil, errAuthentication + if subtle.ConstantTimeCompare([]byte(secret), []byte(globalActiveCred.SecretKey)) == 1 { + return nil, errAuthentication + } + claims, err = auth.ExtractClaims(token, globalActiveCred.SecretKey) + if err != nil { + return nil, errAuthentication + } } // If OPA is set, return without any further checks. @@ -241,23 +241,50 @@ func getClaimsFromToken(token string) (map[string]interface{}, error) { return claims.Map(), nil } +// Fetch claims in the security token returned by the client. +func getClaimsFromToken(token string) (map[string]interface{}, error) { + return getClaimsFromTokenWithSecret(token, globalActiveCred.SecretKey) +} + // Fetch claims in the security token returned by the client and validate the token. func checkClaimsFromToken(r *http.Request, cred auth.Credentials) (map[string]interface{}, APIErrorCode) { token := getSessionToken(r) if token != "" && cred.AccessKey == "" { + // x-amz-security-token is not allowed for anonymous access. return nil, ErrNoAccessKey } - if cred.IsServiceAccount() && token == "" { - token = cred.SessionToken - } - if subtle.ConstantTimeCompare([]byte(token), []byte(cred.SessionToken)) != 1 { + + if token == "" && cred.IsTemp() { + // Temporary credentials should always have x-amz-security-token return nil, ErrInvalidToken } - claims, err := getClaimsFromToken(token) - if err != nil { - return nil, toAPIErrorCode(r.Context(), err) + + if token != "" && !cred.IsTemp() { + // x-amz-security-token should not present for static credentials. + return nil, ErrInvalidToken } - return claims, ErrNone + + if cred.IsTemp() && subtle.ConstantTimeCompare([]byte(token), []byte(cred.SessionToken)) != 1 { + // validate token for temporary credentials only. + return nil, ErrInvalidToken + } + + secret := globalActiveCred.SecretKey + if cred.IsServiceAccount() { + token = cred.SessionToken + secret = cred.SecretKey + } + + if token != "" { + claims, err := getClaimsFromTokenWithSecret(token, secret) + if err != nil { + return nil, toAPIErrorCode(r.Context(), err) + } + return claims, ErrNone + } + + claims := xjwt.NewMapClaims() + return claims.Map(), ErrNone } // Check request auth type verifies the incoming http request diff --git a/cmd/iam-store.go b/cmd/iam-store.go index 1e719afca..5c126ff32 100644 --- a/cmd/iam-store.go +++ b/cmd/iam-store.go @@ -1456,13 +1456,28 @@ func (store *IAMStoreSys) GetAllParentUsers() map[string]string { res := map[string]string{} for _, cred := range cache.iamUsersMap { - if cred.IsServiceAccount() || cred.IsTemp() { + if (cred.IsServiceAccount() || cred.IsTemp()) && cred.SessionToken != "" { parentUser := cred.ParentUser - if cred.SessionToken != "" { - claims, err := getClaimsFromToken(cred.SessionToken) + + var ( + err error + claims map[string]interface{} + ) + + if cred.IsServiceAccount() { + claims, err = getClaimsFromTokenWithSecret(cred.SessionToken, cred.SecretKey) if err != nil { - continue + claims, err = getClaimsFromTokenWithSecret(cred.SessionToken, globalActiveCred.SecretKey) } + } else if cred.IsTemp() { + claims, err = getClaimsFromTokenWithSecret(cred.SessionToken, globalActiveCred.SecretKey) + } + + if err != nil { + continue + } + + if len(claims) > 0 { if v, ok := claims[subClaim]; ok { subFromToken, ok := v.(string) if ok { @@ -1470,6 +1485,11 @@ func (store *IAMStoreSys) GetAllParentUsers() map[string]string { } } } + + if parentUser == "" { + continue + } + if _, ok := res[parentUser]; !ok { res[parentUser] = cred.ParentUser } @@ -1561,6 +1581,7 @@ func (store *IAMStoreSys) UpdateServiceAccount(ctx context.Context, accessKey st return errNoSuchServiceAccount } + currentSecretKey := cr.SecretKey if opts.secretKey != "" { if !auth.IsSecretKeyValid(opts.secretKey) { return auth.ErrInvalidSecretKeyLength @@ -1582,20 +1603,21 @@ func (store *IAMStoreSys) UpdateServiceAccount(ctx context.Context, accessKey st return errors.New("unknown account status value") } - if opts.sessionPolicy != nil { - m, err := getClaimsFromToken(cr.SessionToken) - if err != nil { - return fmt.Errorf("unable to get svc acc claims: %v", err) - } + m, err := getClaimsFromTokenWithSecret(cr.SessionToken, currentSecretKey) + if err != nil { + return fmt.Errorf("unable to get svc acc claims: %v", err) + } - err = opts.sessionPolicy.Validate() - if err != nil { + if opts.sessionPolicy != nil { + if err := opts.sessionPolicy.Validate(); err != nil { return err } + policyBuf, err := json.Marshal(opts.sessionPolicy) if err != nil { return err } + if len(policyBuf) > 16*humanize.KiByte { return fmt.Errorf("Session policy should not exceed 16 KiB characters") } @@ -1603,10 +1625,11 @@ func (store *IAMStoreSys) UpdateServiceAccount(ctx context.Context, accessKey st // Overwrite session policy claims. m[iampolicy.SessionPolicyName] = base64.StdEncoding.EncodeToString(policyBuf) m[iamPolicyClaimNameSA()] = "embedded-policy" - cr.SessionToken, err = auth.JWTSignWithAccessKey(accessKey, m, globalActiveCred.SecretKey) - if err != nil { - return err - } + } + + cr.SessionToken, err = auth.JWTSignWithAccessKey(accessKey, m, cr.SecretKey) + if err != nil { + return err } u := newUserIdentity(cr) diff --git a/cmd/iam.go b/cmd/iam.go index f8a83dc12..0a14c2059 100644 --- a/cmd/iam.go +++ b/cmd/iam.go @@ -37,6 +37,7 @@ import ( "github.com/minio/minio/internal/arn" "github.com/minio/minio/internal/auth" "github.com/minio/minio/internal/color" + "github.com/minio/minio/internal/jwt" "github.com/minio/minio/internal/logger" iampolicy "github.com/minio/pkg/iam/policy" etcd "go.etcd.io/etcd/client/v3" @@ -799,14 +800,17 @@ func (sys *IAMSys) NewServiceAccount(ctx context.Context, parentUser string, gro } } - var cred auth.Credentials - + var accessKey, secretKey string var err error if len(opts.accessKey) > 0 { - cred, err = auth.CreateNewCredentialsWithMetadata(opts.accessKey, opts.secretKey, m, globalActiveCred.SecretKey) + accessKey, secretKey = opts.accessKey, opts.secretKey } else { - cred, err = auth.GetNewCredentialsWithMetadata(m, globalActiveCred.SecretKey) + accessKey, secretKey, err = auth.GenerateCredentials() + if err != nil { + return auth.Credentials{}, err + } } + cred, err := auth.CreateNewCredentialsWithMetadata(accessKey, secretKey, m, secretKey) if err != nil { return auth.Credentials{}, err } @@ -880,9 +884,12 @@ func (sys *IAMSys) getServiceAccount(ctx context.Context, accessKey string) (aut var embeddedPolicy *iampolicy.Policy - jwtClaims, err := auth.ExtractClaims(sa.SessionToken, globalActiveCred.SecretKey) + jwtClaims, err := auth.ExtractClaims(sa.SessionToken, sa.SecretKey) if err != nil { - return auth.Credentials{}, nil, err + jwtClaims, err = auth.ExtractClaims(sa.SessionToken, globalActiveCred.SecretKey) + if err != nil { + return auth.Credentials{}, nil, err + } } pt, ptok := jwtClaims.Lookup(iamPolicyClaimNameSA()) sp, spok := jwtClaims.Lookup(iampolicy.SessionPolicyName) @@ -915,9 +922,12 @@ func (sys *IAMSys) GetClaimsForSvcAcc(ctx context.Context, accessKey string) (ma return nil, errNoSuchServiceAccount } - jwtClaims, err := auth.ExtractClaims(sa.SessionToken, globalActiveCred.SecretKey) + jwtClaims, err := auth.ExtractClaims(sa.SessionToken, sa.SecretKey) if err != nil { - return nil, err + jwtClaims, err = auth.ExtractClaims(sa.SessionToken, globalActiveCred.SecretKey) + if err != nil { + return nil, err + } } return jwtClaims.Map(), nil } @@ -1063,12 +1073,28 @@ func (sys *IAMSys) updateGroupMembershipsForLDAP(ctx context.Context) { if _, ok := parentUserToCredsMap[cred.ParentUser]; !ok { // Try to find the ldapUsername for this // parentUser by extracting JWT claims - jwtClaims, err := auth.ExtractClaims(cred.SessionToken, globalActiveCred.SecretKey) - if err != nil { - // skip this cred - session token seems - // invalid + var ( + jwtClaims *jwt.MapClaims + err error + ) + + if cred.SessionToken == "" { continue } + + if cred.IsServiceAccount() { + jwtClaims, err = auth.ExtractClaims(cred.SessionToken, cred.SecretKey) + if err != nil { + jwtClaims, err = auth.ExtractClaims(cred.SessionToken, globalActiveCred.SecretKey) + } + } else { + jwtClaims, err = auth.ExtractClaims(cred.SessionToken, globalActiveCred.SecretKey) + } + if err != nil { + // skip this cred - session token seems invalid + continue + } + ldapUsername, ok := jwtClaims.Lookup(ldapUserN) if !ok { // skip this cred - we dont have the diff --git a/cmd/jwt.go b/cmd/jwt.go index f2b18d19f..1df051733 100644 --- a/cmd/jwt.go +++ b/cmd/jwt.go @@ -132,7 +132,7 @@ func authenticateURL(accessKey, secretKey string) (string, error) { // Check if the request is authenticated. // Returns nil if the request is authenticated. errNoAuthToken if token missing. // Returns errAuthentication for all other errors. -func webRequestAuthenticate(req *http.Request) (*xjwt.MapClaims, []string, bool, error) { +func metricsRequestAuthenticate(req *http.Request) (*xjwt.MapClaims, []string, bool, error) { token, err := jwtreq.AuthorizationHeaderExtractor.ExtractToken(req) if err != nil { if err == jwtreq.ErrNoTokenInRequest { diff --git a/cmd/jwt_test.go b/cmd/jwt_test.go index 8a6b09be2..0774839f5 100644 --- a/cmd/jwt_test.go +++ b/cmd/jwt_test.go @@ -149,7 +149,7 @@ func TestWebRequestAuthenticate(t *testing.T) { } for i, testCase := range testCases { - _, _, _, gotErr := webRequestAuthenticate(testCase.req) + _, _, _, gotErr := metricsRequestAuthenticate(testCase.req) if testCase.expectedErr != gotErr { t.Errorf("Test %d, expected err %s, got %s", i+1, testCase.expectedErr, gotErr) } diff --git a/cmd/metrics.go b/cmd/metrics.go index e626eddaa..761d59777 100644 --- a/cmd/metrics.go +++ b/cmd/metrics.go @@ -674,7 +674,7 @@ func metricsHandler() http.Handler { // AuthMiddleware checks if the bearer token is valid and authorized. func AuthMiddleware(h http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - claims, groups, owner, authErr := webRequestAuthenticate(r) + claims, groups, owner, authErr := metricsRequestAuthenticate(r) if authErr != nil || !claims.VerifyIssuer("prometheus", true) { w.WriteHeader(http.StatusForbidden) return