CVE-2020-26892

@@ -1525,6 +1525,16 @@ func (a *Account) activationExpired(exportAcc *Account, subject string, kind jwt
 	}
 }
 
+func isRevoked(revocations map[string]int64, subject string, issuedAt int64) bool {
+	if revocations == nil {
+		return false
+	}
+	if t, ok := revocations[subject]; !ok || t < issuedAt {
+		return false
+	}
+	return true
+}
+
 // checkActivation will check the activation token for validity.
 func (a *Account) checkActivation(importAcc *Account, claim *jwt.Import, expTimer bool) bool {
 	if claim == nil || claim.Token == "" {
@@ -1567,13 +1577,7 @@ func (a *Account) checkActivation(importAcc *Account, claim *jwt.Import, expTime
 		}
 	}
 	// Check for token revocation..
-	if a.actsRevoked != nil {
-		if t, ok := a.actsRevoked[act.Subject]; ok && t <= time.Now().Unix() {
-			return false
-		}
-	}
-
-	return true
+	return !isRevoked(a.actsRevoked, act.Subject, act.IssuedAt)
 }
 
 // Returns true if the activation claim is trusted. That is the issuer matches
@@ -1710,16 +1714,10 @@ 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() {
-		return false
-	}
-	return true
+	return isRevoked(a.usersRevoked, nkey, issuedAt)
 }
 
 // Check expiration and set the proper state as needed.
@@ -2010,6 +2008,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.incomplete = len(incompleteImports) != 0
 	for _, i := range incompleteImports {
@@ -2024,7 +2024,7 @@ func (s *Server) updateAccountClaimsWithRefresh(a *Account, ac *jwt.AccountClaim
 			return clients[i].start.After(clients[j].start)
 		})
 	}
-	now := time.Now().Unix()
+
 	for i, c := range clients {
 		a.mu.RLock()
 		exceeded := a.mconns != jwt.NoLimit && i >= int(a.mconns)
@@ -2035,17 +2035,16 @@ 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.
-		if ac.Revocations != nil {
-			if t, ok := ac.Revocations[nkey]; ok && now >= t {
+		// Check for being revoked here. We use ac one to avoid the account lock.
+		if ac.Revocations != nil && theJWT != "" {
+			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

@@ -474,7 +474,7 @@ func (s *Server) processClientOrLeafAuthentication(c *client) bool {
 			c.Debugf("Signature not verified")
 			return false
 		}
-		if acc.checkUserRevoked(juc.Subject) {
+		if acc.checkUserRevoked(juc.Subject, juc.IssuedAt) {
 			c.Debugf("User authentication revoked")
 			return false
 		}

@@ -151,16 +151,16 @@ func (e *Export) ClearRevocation(pubKey string) {
 	e.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.
+// 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 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 (e *Export) IsRevokedAt(pubKey string, timestamp time.Time) bool {
 	return e.Revocations.IsRevoked(pubKey, timestamp)
 }
 
-// IsRevoked checks if the public key is in the revoked list with time.Now()
-func (e *Export) IsRevoked(pubKey string) bool {
-	return e.Revocations.IsRevoked(pubKey, time.Now())
+// IsRevoked does not perform a valid check. Use IsRevokedAt instead.
+func (e *Export) IsRevoked(_ string) bool {
+	return true
 }
 
 // Exports is a slice of exports

@@ -175,16 +175,20 @@ func (e *Export) ClearRevocation(pubKey string) {
 	e.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 (e *Export) IsRevokedAt(pubKey string, timestamp time.Time) bool {
-	return e.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 (e *Export) isRevoked(pubKey string, claimIssuedAt time.Time) bool {
+	return e.Revocations.IsRevoked(pubKey, claimIssuedAt)
 }
 
-// IsRevoked checks if the public key is in the revoked list with time.Now()
-func (e *Export) IsRevoked(pubKey string) bool {
-	return e.Revocations.IsRevoked(pubKey, time.Now())
+// IsClaimRevoked checks if the activation revoked the claim passed in.
+// Invalid claims (nil, no Subject or IssuedAt) will return true.
+func (e *Export) IsClaimRevoked(claim *ActivationClaims) bool {
+	if claim == nil || claim.IssuedAt == 0 || claim.Subject == "" {
+		return true
+	}
+	return e.isRevoked(claim.Subject, time.Unix(claim.IssuedAt, 0))
 }
 
 // Exports is a slice of exports

@@ -187,48 +187,61 @@ func TestExportRevocation(t *testing.T) {
 
 	account.Exports.Add(e)
 
-	pubKey := "bar"
+	ikp := createAccountNKey(t)
+	pubKey := publicKey(ikp, t)
+
+	ac := NewActivationClaims(pubKey)
+	ac.IssuerAccount = apk
+	ac.Name = "foo"
+	ac.Activation.ImportSubject = "foo"
+	ac.Activation.ImportType = Stream
+	aJwt, _ := ac.Encode(akp)
+	ac, err := DecodeActivationClaims(aJwt)
+	if err != nil {
+		t.Errorf("Failed to decode activation claim: %v", err)
+	}
+
 	now := time.Now()
 
 	// test that clear is safe before we add any
 	e.ClearRevocation(pubKey)
 
-	if e.IsRevokedAt(pubKey, now) {
+	if e.isRevoked(pubKey, now) {
 		t.Errorf("no revocation was added so is revoked should be false")
 	}
 
 	e.RevokeAt(pubKey, now.Add(time.Second*100))
 
-	if !e.IsRevokedAt(pubKey, now) {
+	if !e.isRevoked(pubKey, now) {
 		t.Errorf("revocation should hold when timestamp is in the future")
 	}
 
-	if e.IsRevokedAt(pubKey, now.Add(time.Second*150)) {
+	if e.isRevoked(pubKey, now.Add(time.Second*150)) {
 		t.Errorf("revocation should time out")
 	}
 
 	e.RevokeAt(pubKey, now.Add(time.Second*50)) // shouldn't change the revocation, you can't move it in
 
-	if !e.IsRevokedAt(pubKey, now.Add(time.Second*60)) {
+	if !e.isRevoked(pubKey, now.Add(time.Second*60)) {
 		t.Errorf("revocation should hold, 100 > 50")
 	}
 
 	encoded, _ := account.Encode(akp)
 	decoded, _ := DecodeAccountClaims(encoded)
 
-	if !decoded.Exports[0].IsRevokedAt(pubKey, now.Add(time.Second*60)) {
+	if !decoded.Exports[0].isRevoked(pubKey, now.Add(time.Second*60)) {
 		t.Errorf("revocation should last across encoding")
 	}
 
 	e.ClearRevocation(pubKey)
 
-	if e.IsRevokedAt(pubKey, now) {
+	if e.IsClaimRevoked(ac) {
 		t.Errorf("revocations should be cleared")
 	}
 
-	e.RevokeAt(pubKey, now.Add(time.Second*1000))
+	e.RevokeAt(pubKey, now)
 
-	if !e.IsRevoked(pubKey) {
+	if !e.IsClaimRevoked(ac) {
 		t.Errorf("revocation be true we revoked in the future")
 	}
 }