[FIXED] Possible panic if server receives a maliciously crafted JWT

This addresses [CVE-2020-26521](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2020-26521)

This is mainly a port of #1624 with some other updates related
to tests.

Signed-off-by: Ivan Kozlovic <[email protected]>

Author: kozlovic

go.mod

@@ -1,7 +1,7 @@
 module github.com/nats-io/nats-server/v2
 
 require (
-	github.com/nats-io/jwt v0.3.2
+	github.com/nats-io/jwt v1.1.0
 	github.com/nats-io/nats.go v1.10.0
 	github.com/nats-io/nkeys v0.1.4
 	github.com/nats-io/nuid v1.0.1

go.sum

@@ -10,6 +10,8 @@ github.com/google/go-cmp v0.4.0 h1:xsAVV57WRhGj6kEIi8ReJzQlHHqcBYCElAvkovg3B/4=
 github.com/google/go-cmp v0.4.0/go.mod h1:v8dTdLbMG2kIc/vJvl+f65V22dbkXbowE6jgT/gNBxE=
 github.com/nats-io/jwt v0.3.2 h1:+RB5hMpXUUA2dfxuhBTEkMOrYmM+gKIZYS1KjSostMI=
 github.com/nats-io/jwt v0.3.2/go.mod h1:/euKqTS1ZD+zzjYrY7pseZrTtWQSjujC7xjPc8wL6eU=
+github.com/nats-io/jwt v1.1.0 h1:+vOlgtM0ZsF46GbmUoadq0/2rChNS45gtxHEa3H1gqM=
+github.com/nats-io/jwt v1.1.0/go.mod h1:n3cvmLfBfnpV4JJRN7lRYCyZnw48ksGsbThGXEk4w9M=
 github.com/nats-io/nats.go v1.10.0 h1:L8qnKaofSfNFbXg0C5F71LdjPRnmQwSsA4ukmkt1TvY=
 github.com/nats-io/nats.go v1.10.0/go.mod h1:AjGArbfyR50+afOUotNX2Xs5SYHf+CoOa5HH1eEl2HE=
 github.com/nats-io/nkeys v0.1.3/go.mod h1:xpnFELMwJABBLVhffcfd1MZx6VsNRFpEugbxziKVo7w=

server/accounts.go

@@ -1546,14 +1546,14 @@ func (a *Account) checkActivation(importAcc *Account, claim *jwt.Import, expTime
 	if err != nil {
 		return false
 	}
+	if !a.isIssuerClaimTrusted(act) {
+		return false
+	}
 	vr = jwt.CreateValidationResults()
 	act.Validate(vr)
 	if vr.IsBlocking(true) {
 		return false
 	}
-	if !a.isIssuerClaimTrusted(act) {
-		return false
-	}
 	if act.Expires != 0 {
 		tn := time.Now().Unix()
 		if act.Expires <= tn {

server/jwt_test.go

@@ -1575,13 +1575,14 @@ func TestAccountURLResolver(t *testing.T) {
 			defer ts.Close()
 
 			confTemplate := `
+				operator: %s
 				listen: -1
 				resolver: URL("%s/ngs/v1/accounts/jwt/")
 				resolver_tls {
 					insecure: true
 				}
 			`
-			conf := createConfFile(t, []byte(fmt.Sprintf(confTemplate, ts.URL)))
+			conf := createConfFile(t, []byte(fmt.Sprintf(confTemplate, ojwt, ts.URL)))
 			defer os.Remove(conf)
 
 			s, opts := RunServerWithConfig(conf)
@@ -1660,10 +1661,11 @@ func TestAccountURLResolverNoFetchOnReload(t *testing.T) {
 	defer ts.Close()
 
 	confTemplate := `
+		operator: %s
 		listen: -1
 		resolver: URL("%s/ngs/v1/accounts/jwt/")
     `
-	conf := createConfFile(t, []byte(fmt.Sprintf(confTemplate, ts.URL)))
+	conf := createConfFile(t, []byte(fmt.Sprintf(confTemplate, ojwt, ts.URL)))
 	defer os.Remove(conf)
 
 	s, _ := RunServerWithConfig(conf)
@@ -1685,7 +1687,7 @@ func TestAccountURLResolverNoFetchOnReload(t *testing.T) {
 	}))
 	defer ts.Close()
 
-	changeCurrentConfigContentWithNewContent(t, conf, []byte(fmt.Sprintf(confTemplate, ts.URL)))
+	changeCurrentConfigContentWithNewContent(t, conf, []byte(fmt.Sprintf(confTemplate, ojwt, ts.URL)))
 
 	if err := s.Reload(); err != nil {
 		t.Fatalf("Error on reload: %v", err)

server/server.go

@@ -1083,6 +1083,9 @@ func (s *Server) verifyAccountClaims(claimJWT string) (*jwt.AccountClaims, strin
 	if err != nil {
 		return nil, _EMPTY_, err
 	}
+	if !s.isTrustedIssuer(accClaims.Issuer) {
+		return nil, _EMPTY_, ErrAccountValidation
+	}
 	vr := jwt.CreateValidationResults()
 	accClaims.Validate(vr)
 	if vr.IsBlocking(true) {

test/operator_test.go

@@ -155,7 +155,15 @@ func runOperatorServer(t *testing.T) (*server.Server, *server.Options) {
 	return RunServerWithConfig(testOpConfig)
 }
 
-func createAccountForOperatorKey(t *testing.T, s *server.Server, seed []byte) (*server.Account, nkeys.KeyPair) {
+func publicKeyFromKeyPair(t *testing.T, pair nkeys.KeyPair) (pkey string) {
+	var err error
+	if pkey, err = pair.PublicKey(); err != nil {
+		t.Fatalf("Expected no error %v", err)
+	}
+	return
+}
+
+func createAccountForOperatorKey(t *testing.T, s *server.Server, seed []byte) nkeys.KeyPair {
 	t.Helper()
 	okp, _ := nkeys.FromSeed(seed)
 	akp, _ := nkeys.CreateAccount()
@@ -165,16 +173,18 @@ func createAccountForOperatorKey(t *testing.T, s *server.Server, seed []byte) (*
 	if err := s.AccountResolver().Store(pub, jwt); err != nil {
 		t.Fatalf("Account Resolver returned an error: %v", err)
 	}
-	acc, err := s.LookupAccount(pub)
-	if err != nil {
-		t.Fatalf("Error looking up account: %v", err)
-	}
-	return acc, akp
+	return akp
 }
 
-func createAccount(t *testing.T, s *server.Server) (*server.Account, nkeys.KeyPair) {
+func createAccount(t *testing.T, s *server.Server) (acc *server.Account, akp nkeys.KeyPair) {
 	t.Helper()
-	return createAccountForOperatorKey(t, s, oSeed)
+	akp = createAccountForOperatorKey(t, s, oSeed)
+	if pub, err := akp.PublicKey(); err != nil {
+		t.Fatalf("Expected this to pass, got %v", err)
+	} else if acc, err = s.LookupAccount(pub); err != nil {
+		t.Fatalf("Error looking up account: %v", err)
+	}
+	return acc, akp
 }
 
 func createUserCreds(t *testing.T, s *server.Server, akp nkeys.KeyPair) nats.Option {
@@ -215,11 +225,15 @@ func TestOperatorServer(t *testing.T) {
 	// Now create an account from another operator, this should fail.
 	okp, _ := nkeys.CreateOperator()
 	seed, _ := okp.Seed()
-	_, akp = createAccountForOperatorKey(t, s, seed)
+	akp = createAccountForOperatorKey(t, s, seed)
 	_, err = nats.Connect(url, createUserCreds(t, s, akp))
 	if err == nil {
 		t.Fatalf("Expected error on connect")
 	}
+	// The account should not be in memory either
+	if v, err := s.LookupAccount(publicKeyFromKeyPair(t, akp)); err == nil {
+		t.Fatalf("Expected account to NOT be in memory: %v", v)
+	}
 }
 
 func TestOperatorSystemAccount(t *testing.T) {
@@ -229,15 +243,15 @@ func TestOperatorSystemAccount(t *testing.T) {
 	// Create an account from another operator, this should fail if used as a system account.
 	okp, _ := nkeys.CreateOperator()
 	seed, _ := okp.Seed()
-	acc, _ := createAccountForOperatorKey(t, s, seed)
-	if err := s.SetSystemAccount(acc.Name); err == nil {
+	akp := createAccountForOperatorKey(t, s, seed)
+	if err := s.SetSystemAccount(publicKeyFromKeyPair(t, akp)); err == nil {
 		t.Fatalf("Expected this to fail")
 	}
 	if acc := s.SystemAccount(); acc != nil {
 		t.Fatalf("Expected no account to be set for system account")
 	}
 
-	acc, _ = createAccount(t, s)
+	acc, _ := createAccount(t, s)
 	if err := s.SetSystemAccount(acc.Name); err != nil {
 		t.Fatalf("Expected this succeed, got %v", err)
 	}
@@ -251,10 +265,10 @@ func TestOperatorSigningKeys(t *testing.T) {
 	defer s.Shutdown()
 
 	// Create an account with a signing key, not the master key.
-	acc, akp := createAccountForOperatorKey(t, s, skSeed)
+	akp := createAccountForOperatorKey(t, s, skSeed)
 
 	// Make sure we can set system account.
-	if err := s.SetSystemAccount(acc.Name); err != nil {
+	if err := s.SetSystemAccount(publicKeyFromKeyPair(t, akp)); err != nil {
 		t.Fatalf("Expected this succeed, got %v", err)
 	}