diff --git a/go.mod b/go.mod index 0af0e89a..9290366a 100644 --- a/go.mod +++ b/go.mod @@ -2,7 +2,7 @@ module github.com/nats-io/nats-server/v2 require ( github.com/minio/highwayhash v1.0.0 - github.com/nats-io/jwt/v2 v2.0.0-20200930010033-c4fd08d85545 + github.com/nats-io/jwt/v2 v2.0.0-20201006231922-e00ffcea7738 github.com/nats-io/nats.go v1.10.1-0.20200606002146-fc6fed82929a github.com/nats-io/nkeys v0.2.0 github.com/nats-io/nuid v1.0.1 diff --git a/go.sum b/go.sum index ffe56d43..51f2f862 100644 --- a/go.sum +++ b/go.sum @@ -14,8 +14,8 @@ github.com/minio/highwayhash v1.0.0/go.mod h1:xQboMTeM9nY9v/LlAOxFctujiv5+Aq2hR5 github.com/nats-io/jwt v0.3.2/go.mod h1:/euKqTS1ZD+zzjYrY7pseZrTtWQSjujC7xjPc8wL6eU= github.com/nats-io/jwt v0.3.3-0.20200519195258-f2bf5ce574c7 h1:RnGotxlghqR5D2KDAu4TyuLqyjuylOsJiAFhXvMvQIc= github.com/nats-io/jwt v0.3.3-0.20200519195258-f2bf5ce574c7/go.mod h1:n3cvmLfBfnpV4JJRN7lRYCyZnw48ksGsbThGXEk4w9M= -github.com/nats-io/jwt/v2 v2.0.0-20200930010033-c4fd08d85545 h1:RhEai4U9Ax2izzNupSdputRLZcJU1fpP1CE2zgwuTRI= -github.com/nats-io/jwt/v2 v2.0.0-20200930010033-c4fd08d85545/go.mod h1:vs+ZEjP+XKy8szkBmQwCB7RjYdIlMaPsFPs4VdS4bTQ= +github.com/nats-io/jwt/v2 v2.0.0-20201006231922-e00ffcea7738 h1:MlwwastrhUZSIvSs4M70vT0fOWTCF6WxOu9S4/NtY9U= +github.com/nats-io/jwt/v2 v2.0.0-20201006231922-e00ffcea7738/go.mod h1:vs+ZEjP+XKy8szkBmQwCB7RjYdIlMaPsFPs4VdS4bTQ= github.com/nats-io/nats-server/v2 v2.1.8-0.20200524125952-51ebd92a9093/go.mod h1:rQnBf2Rv4P9adtAs/Ti6LfFmVtFG6HLhl/H7cVshcJU= github.com/nats-io/nats-server/v2 v2.1.8-0.20200601203034-f8d6dd992b71/go.mod h1:Nan/1L5Sa1JRW+Thm4HNYcIDcVRFc5zK9OpSZeI2kk4= github.com/nats-io/nats.go v1.10.0/go.mod h1:AjGArbfyR50+afOUotNX2Xs5SYHf+CoOa5HH1eEl2HE= diff --git a/server/accounts.go b/server/accounts.go index 1d1a16aa..e1a7d02e 100644 --- a/server/accounts.go +++ b/server/accounts.go @@ -2253,13 +2253,13 @@ func (a *Account) clearExpirationTimer() bool { } // checkUserRevoked will check if a user has been revoked. -func (a *Account) checkUserRevoked(nkey string) bool { +func (a *Account) checkUserRevoked(nkey string, issuedAt int64) bool { a.mu.RLock() defer a.mu.RUnlock() if a.usersRevoked == nil { return false } - if t, ok := a.usersRevoked[nkey]; !ok || t > time.Now().Unix() { + if t, ok := a.usersRevoked[nkey]; !ok || t < issuedAt { return false } return true @@ -2595,6 +2595,8 @@ func (s *Server) updateAccountClaimsWithRefresh(a *Account, ac *jwt.AccountClaim for pk, t := range ac.Revocations { a.usersRevoked[pk] = t } + } else { + a.usersRevoked = nil } a.defaultPerms = buildPermissionsFromJwt(&ac.DefaultPermissions) a.incomplete = len(incompleteImports) != 0 @@ -2639,7 +2641,6 @@ func (s *Server) updateAccountClaimsWithRefresh(a *Account, ac *jwt.AccountClaim } } - now := time.Now().Unix() for i, c := range clients { a.mu.RLock() exceeded := a.mconns != jwt.NoLimit && i >= int(a.mconns) @@ -2650,17 +2651,15 @@ func (s *Server) updateAccountClaimsWithRefresh(a *Account, ac *jwt.AccountClaim } c.mu.Lock() c.applyAccountLimits() - // Check for being revoked here. We use ac one to avoid - // the account lock. - var nkey string - if c.user != nil { - nkey = c.user.Nkey - } + theJWT := c.opts.JWT c.mu.Unlock() - - // Check if we have been revoked. + // Check for being revoked here. We use ac one to avoid the account lock. if ac.Revocations != nil { - if t, ok := ac.Revocations[nkey]; ok && now >= t { + if juc, err := jwt.DecodeUserClaims(theJWT); err != nil { + c.Debugf("User JWT not valid: %v", err) + c.authViolation() + continue + } else if ok := ac.IsClaimRevoked(juc); ok { c.sendErrAndDebug("User Authentication Revoked") c.closeConnection(Revocation) continue diff --git a/server/auth.go b/server/auth.go index 14eff310..601e28a4 100644 --- a/server/auth.go +++ b/server/auth.go @@ -559,7 +559,7 @@ func (s *Server) processClientOrLeafAuthentication(c *client, opts *Options) boo return false } } - if acc.checkUserRevoked(juc.Subject) { + if acc.checkUserRevoked(juc.Subject, juc.IssuedAt) { c.Debugf("User authentication revoked") return false } diff --git a/server/jwt_test.go b/server/jwt_test.go index da15039d..3148e1ec 100644 --- a/server/jwt_test.go +++ b/server/jwt_test.go @@ -4268,3 +4268,93 @@ func TestJWTJetStreamLimits(t *testing.T) { expect_JSDisabledForAccount(c) c.Close() } + +func TestJWTUserRevocation(t *testing.T) { + createAccountAndUser := func(done chan struct{}, pubKey, jwt1, jwt2, creds1, creds2 *string) { + t.Helper() + kp, _ := nkeys.CreateAccount() + *pubKey, _ = kp.PublicKey() + claim := jwt.NewAccountClaims(*pubKey) + var err error + *jwt1, err = claim.Encode(oKp) + require_NoError(t, err) + + ukp, _ := nkeys.CreateUser() + seed, _ := ukp.Seed() + upub, _ := ukp.PublicKey() + uclaim := newJWTTestUserClaims() + uclaim.Subject = upub + + ujwt1, err := uclaim.Encode(kp) + require_NoError(t, err) + *creds1 = genCredsFile(t, ujwt1, seed) + + // create updated claim need to assure that issue time differs + claim.Revoke(upub) // revokes all jwt from now on + time.Sleep(time.Millisecond * 1100) + *jwt2, err = claim.Encode(oKp) + require_NoError(t, err) + + ujwt2, err := uclaim.Encode(kp) + require_NoError(t, err) + *creds2 = genCredsFile(t, ujwt2, seed) + + done <- struct{}{} + } + // Create Accounts and corresponding revoked and non revoked user creds. Do so concurrently to speed up the test + doneChan := make(chan struct{}, 2) + defer close(doneChan) + var syspub, sysjwt, dummy1, sysCreds, dummyCreds string + go createAccountAndUser(doneChan, &syspub, &sysjwt, &dummy1, &sysCreds, &dummyCreds) + var apub, ajwt1, ajwt2, aCreds1, aCreds2 string + go createAccountAndUser(doneChan, &apub, &ajwt1, &ajwt2, &aCreds1, &aCreds2) + for i := 0; i < cap(doneChan); i++ { + <-doneChan + } + defer os.Remove(sysCreds) + defer os.Remove(dummyCreds) + defer os.Remove(aCreds1) + defer os.Remove(aCreds2) + dirSrv := createDir(t, "srv") + defer os.RemoveAll(dirSrv) + conf := createConfFile(t, []byte(fmt.Sprintf(` + listen: -1 + operator: %s + system_account: %s + resolver: { + type: full + dir: %s + } + `, ojwt, syspub, dirSrv))) + defer os.Remove(conf) + srv, _ := RunServerWithConfig(conf) + defer srv.Shutdown() + updateJwt(t, srv.ClientURL(), sysCreds, syspub, sysjwt, 1) // update system account jwt + updateJwt(t, srv.ClientURL(), sysCreds, apub, ajwt1, 1) // set account jwt without revocation + // use credentials that will be revoked ans assure that the connection will be disconnected + nc := natsConnect(t, srv.ClientURL(), nats.UserCredentials(aCreds1), + nats.DisconnectErrHandler(func(conn *nats.Conn, err error) { + if lErr := conn.LastError(); lErr != nil && strings.Contains(lErr.Error(), "Authentication Revoked") { + doneChan <- struct{}{} + } + })) + defer nc.Close() + // update account jwt to contain revocation + if passCnt := updateJwt(t, srv.ClientURL(), sysCreds, apub, ajwt2, 1); passCnt != 1 { + t.Fatalf("Expected jwt update to pass") + } + // assure that nc got disconnected due to the revocation + select { + case <-doneChan: + case <-time.After(time.Second): + t.Fatalf("Expected connection to have failed") + } + // try again with old credentials. Expected to fail + if nc1, err := nats.Connect(srv.ClientURL(), nats.UserCredentials(aCreds1)); err == nil { + nc1.Close() + t.Fatalf("Expected revoked credentials to fail") + } + // Assure new creds pass + nc2 := natsConnect(t, srv.ClientURL(), nats.UserCredentials(aCreds2)) + defer nc2.Close() +} diff --git a/vendor/github.com/nats-io/jwt/v2/account_claims.go b/vendor/github.com/nats-io/jwt/v2/account_claims.go index 0a9b9615..4105dd7f 100644 --- a/vendor/github.com/nats-io/jwt/v2/account_claims.go +++ b/vendor/github.com/nats-io/jwt/v2/account_claims.go @@ -235,7 +235,8 @@ func (a *AccountClaims) Revoke(pubKey string) { a.RevokeAt(pubKey, time.Now()) } -// RevokeAt enters a revocation by public key and timestamp into this export +// RevokeAt enters a revocation by public key and timestamp into this account +// This will revoke all jwt issued for pubKey, prior to timestamp // If there is already a revocation for this public key that is newer, it is kept. func (a *AccountClaims) RevokeAt(pubKey string, timestamp time.Time) { if a.Revocations == nil { @@ -250,14 +251,18 @@ func (a *AccountClaims) ClearRevocation(pubKey string) { a.Revocations.ClearRevocation(pubKey) } -// IsRevokedAt checks if the public key is in the revoked list with a timestamp later than -// the one passed in. Generally this method is called with time.Now() but other time's can -// be used for testing. -func (a *AccountClaims) IsRevokedAt(pubKey string, timestamp time.Time) bool { - return a.Revocations.IsRevoked(pubKey, timestamp) +// isRevoked checks if the public key is in the revoked list with a timestamp later than the one passed in. +// Generally this method is called with the subject and issue time of the jwt to be tested. +// DO NOT pass time.Now(), it will not produce a stable/expected response. +func (a *AccountClaims) isRevoked(pubKey string, claimIssuedAt time.Time) bool { + return a.Revocations.IsRevoked(pubKey, claimIssuedAt) } -// IsRevoked checks if the public key is in the revoked list with time.Now() -func (a *AccountClaims) IsRevoked(pubKey string) bool { - return a.Revocations.IsRevoked(pubKey, time.Now()) +// IsClaimRevoked checks if the account revoked the claim passed in. +// Invalid claims (nil, no Subject or IssuedAt) will return true. +func (a *AccountClaims) IsClaimRevoked(claim *UserClaims) bool { + if claim == nil || claim.IssuedAt == 0 || claim.Subject == "" { + return true + } + return a.isRevoked(claim.Subject, time.Unix(claim.IssuedAt, 0)) } diff --git a/vendor/github.com/nats-io/jwt/v2/revocation_list.go b/vendor/github.com/nats-io/jwt/v2/revocation_list.go index b9c38373..9de30b11 100644 --- a/vendor/github.com/nats-io/jwt/v2/revocation_list.go +++ b/vendor/github.com/nats-io/jwt/v2/revocation_list.go @@ -39,9 +39,9 @@ func (r RevocationList) ClearRevocation(pubKey string) { } // IsRevoked checks if the public key is in the revoked list with a timestamp later than -// the one passed in. Generally this method is called with time.Now() but other time's can +// the one passed in. Generally this method is called with an issue time but other time's can // be used for testing. func (r RevocationList) IsRevoked(pubKey string, timestamp time.Time) bool { ts, ok := r[pubKey] - return ok && ts > timestamp.Unix() + return ok && ts >= timestamp.Unix() } diff --git a/vendor/github.com/nats-io/jwt/v2/validation.go b/vendor/github.com/nats-io/jwt/v2/validation.go index 4625efd7..afaed57f 100644 --- a/vendor/github.com/nats-io/jwt/v2/validation.go +++ b/vendor/github.com/nats-io/jwt/v2/validation.go @@ -105,3 +105,14 @@ func (v *ValidationResults) Errors() []error { } return errs } + +// Warnings returns only non blocking issues as strings +func (v *ValidationResults) Warnings() []string { + var errs []string + for _, v := range v.Issues { + if !v.Blocking { + errs = append(errs, v.Description) + } + } + return errs +} diff --git a/vendor/modules.txt b/vendor/modules.txt index e596228a..2bfcf382 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -1,6 +1,6 @@ # github.com/minio/highwayhash v1.0.0 github.com/minio/highwayhash -# github.com/nats-io/jwt/v2 v2.0.0-20200930010033-c4fd08d85545 +# github.com/nats-io/jwt/v2 v2.0.0-20201006231922-e00ffcea7738 github.com/nats-io/jwt/v2 # github.com/nats-io/nats.go v1.10.1-0.20200606002146-fc6fed82929a github.com/nats-io/nats.go