[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 <ivan@synadia.com>

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,18 +173,20 @@ 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 {
+	return akp
+}
+
+func createAccount(t *testing.T, s *server.Server) (acc *server.Account, akp nkeys.KeyPair) {
+	t.Helper()
+	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 createAccount(t *testing.T, s *server.Server) (*server.Account, nkeys.KeyPair) {
-	t.Helper()
-	return createAccountForOperatorKey(t, s, oSeed)
-}
-
 func createUserCreds(t *testing.T, s *server.Server, akp nkeys.KeyPair) nats.Option {
 	t.Helper()
 	kp, _ := nkeys.CreateUser()
@@ -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)
 	}
 

vendor/github.com/nats-io/jwt/.travis.yml

@@ -1,8 +1,7 @@
 language: go
-sudo: false
 go:
+- 1.14.x
 - 1.13.x
-- 1.12.x
 
 install:
 - go get -t ./...
@@ -19,4 +18,4 @@ before_script:
 
 script:
 - go test -v -race ./...
-- if [[ "$TRAVIS_GO_VERSION" =~ 1.12 ]]; then ./scripts/cov.sh TRAVIS; fi
+- if [[ "$TRAVIS_GO_VERSION" =~ 1.13 ]]; then ./scripts/cov.sh TRAVIS; fi

vendor/github.com/nats-io/jwt/account_claims.go

@@ -194,7 +194,8 @@ func (a *AccountClaims) Revoke(pubKey string) {
 	a.RevokeAt(pubKey, time.Now())
 }
 
-// RevokeAt enters a revocation by publickey 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 {
@@ -209,14 +210,23 @@ 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.
+// 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 (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 time.Now()
-func (a *AccountClaims) IsRevoked(pubKey string) bool {
-	return a.Revocations.IsRevoked(pubKey, time.Now())
+// IsRevoked does not perform a valid check. Use IsRevokedAt instead.
+func (a *AccountClaims) IsRevoked(_ string) bool {
+	return true
+}
+
+// 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.Revocations.IsRevoked(claim.Subject, time.Unix(claim.IssuedAt, 0))
 }

vendor/github.com/nats-io/jwt/creds_utils.go

@@ -82,7 +82,7 @@ NKEYs are sensitive and should be treated as secrets.
 	return w.Bytes(), nil
 }
 
-var userConfigRE = regexp.MustCompile(`\s*(?:(?:[-]{3,}[^\n]*[-]{3,}\n)(.+)(?:\n\s*[-]{3,}[^\n]*[-]{3,}\n))`)
+var userConfigRE = regexp.MustCompile(`\s*(?:(?:[-]{3,}.*[-]{3,}\r?\n)([\w\-.=]+)(?:\r?\n[-]{3,}.*[-]{3,}\r?\n))`)
 
 // An user config file looks like this:
 //  -----BEGIN NATS USER JWT-----

vendor/github.com/nats-io/jwt/exports.go

@@ -108,6 +108,10 @@ func (e *Export) IsStreamResponse() bool {
 
 // Validate appends validation issues to the passed in results list
 func (e *Export) Validate(vr *ValidationResults) {
+	if e == nil {
+		vr.AddError("null export is not allowed")
+		return
+	}
 	if !e.IsService() && !e.IsStream() {
 		vr.AddError("invalid export type: %q", e.Type)
 	}
@@ -146,16 +150,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
@@ -199,6 +203,10 @@ func (e *Exports) Validate(vr *ValidationResults) error {
 	var streamSubjects []Subject
 
 	for _, v := range *e {
+		if v == nil {
+			vr.AddError("null export is not allowed")
+			continue
+		}
 		if v.IsService() {
 			serviceSubjects = append(serviceSubjects, v.Subject)
 		} else {

vendor/github.com/nats-io/jwt/go.mod

@@ -1,3 +1,5 @@
 module github.com/nats-io/jwt
 
-require github.com/nats-io/nkeys v0.1.3
+require github.com/nats-io/nkeys v0.1.4
+
+go 1.13

vendor/github.com/nats-io/jwt/go.sum

@@ -1,8 +1,8 @@
-github.com/nats-io/nkeys v0.1.3 h1:6JrEfig+HzTH85yxzhSVbjHRJv9cn0p6n3IngIcM5/k=
-github.com/nats-io/nkeys v0.1.3/go.mod h1:xpnFELMwJABBLVhffcfd1MZx6VsNRFpEugbxziKVo7w=
+github.com/nats-io/nkeys v0.1.4 h1:aEsHIssIk6ETN5m2/MD8Y4B2X7FfXrBAUdkyRvbVYzA=
+github.com/nats-io/nkeys v0.1.4/go.mod h1:XdZpAbhgyyODYqjTawOnIOI7VlbKSarI9Gfy1tqEu/s=
 golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
-golang.org/x/crypto v0.0.0-20190701094942-4def268fd1a4 h1:HuIa8hRrWRSrqYzx1qI49NNxhdi2PrY7gxVSq1JjLDc=
-golang.org/x/crypto v0.0.0-20190701094942-4def268fd1a4/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI=
+golang.org/x/crypto v0.0.0-20200323165209-0ec3e9974c59 h1:3zb4D3T4G8jdExgVU/95+vQXfpEPiMdCaZgmGVxjNHM=
+golang.org/x/crypto v0.0.0-20200323165209-0ec3e9974c59/go.mod h1:LzIPMQfyMNhhGPhUkYOs5KpL4U8rLKemX1yGLhDgUto=
 golang.org/x/net v0.0.0-20190404232315-eb5bcb51f2a3/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg=
 golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
 golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=

vendor/github.com/nats-io/jwt/header.go

@@ -23,7 +23,7 @@ import (
 
 const (
 	// Version is semantic version.
-	Version = "0.3.2"
+	Version = "1.1.0"
 
 	// TokenTypeJwt is the JWT token type supported JWT tokens
 	// encoded and decoded by this library

vendor/github.com/nats-io/jwt/imports.go

@@ -53,6 +53,10 @@ func (i *Import) IsStream() bool {
 
 // Validate checks if an import is valid for the wrapping account
 func (i *Import) Validate(actPubKey string, vr *ValidationResults) {
+	if i == nil {
+		vr.AddError("null import is not allowed")
+		return
+	}
 	if !i.IsService() && !i.IsStream() {
 		vr.AddError("invalid import type: %q", i.Type)
 	}
@@ -123,6 +127,10 @@ type Imports []*Import
 func (i *Imports) Validate(acctPubKey string, vr *ValidationResults) {
 	toSet := make(map[Subject]bool, len(*i))
 	for _, v := range *i {
+		if v == nil {
+			vr.AddError("null import is not allowed")
+			continue
+		}
 		if v.Type == Service {
 			if _, ok := toSet[v.To]; ok {
 				vr.AddError("Duplicate To subjects for %q", v.To)

vendor/github.com/nats-io/jwt/operator_claims.go

@@ -40,6 +40,8 @@ type Operator struct {
 	// A list of NATS urls (tls://host:port) where tools can connect to the server
 	// using proper credentials.
 	OperatorServiceURLs StringList `json:"operator_service_urls,omitempty"`
+	// Identity of the system account
+	SystemAccount string `json:"system_account,omitempty"`
 }
 
 // Validate checks the validity of the operators contents
@@ -63,6 +65,12 @@ func (o *Operator) Validate(vr *ValidationResults) {
 			vr.AddError("%s is not an operator public key", k)
 		}
 	}
+
+	if o.SystemAccount != "" {
+		if !nkeys.IsValidPublicAccountKey(o.SystemAccount) {
+			vr.AddError("%s is not an account public key", o.SystemAccount)
+		}
+	}
 }
 
 func (o *Operator) validateAccountServerURL() error {

vendor/github.com/nats-io/jwt/revocation_list.go

@@ -24,9 +24,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()
 }

vendor/github.com/nats-io/jwt/types.go

@@ -241,8 +241,6 @@ type Permissions struct {
 
 // Validate the pub and sub fields in the permissions list
 func (p *Permissions) Validate(vr *ValidationResults) {
-	p.Pub.Validate(vr)
-	p.Sub.Validate(vr)
 	if p.Resp != nil {
 		p.Resp.Validate(vr)
 	}

vendor/github.com/nats-io/jwt/user_claims.go

@@ -25,12 +25,14 @@ import (
 type User struct {
 	Permissions
 	Limits
+	BearerToken bool `json:"bearer_token,omitempty"`
 }
 
 // Validate checks the permissions and limits in a User jwt
 func (u *User) Validate(vr *ValidationResults) {
 	u.Permissions.Validate(vr)
 	u.Limits.Validate(vr)
+	// When BearerToken is true server will ignore any nonce-signing verification
 }
 
 // UserClaims defines a user JWT
@@ -97,3 +99,8 @@ func (u *UserClaims) Payload() interface{} {
 func (u *UserClaims) String() string {
 	return u.ClaimsData.String(u)
 }
+
+// IsBearerToken returns true if nonce-signing requirements should be skipped
+func (u *UserClaims) IsBearerToken() bool {
+	return u.BearerToken
+}

vendor/modules.txt

@@ -1,6 +1,4 @@
-# github.com/golang/protobuf v1.3.5
-## explicit
-# github.com/nats-io/jwt v0.3.2
+# github.com/nats-io/jwt v1.1.0
 ## explicit
 github.com/nats-io/jwt
 # github.com/nats-io/nats.go v1.10.0
@@ -27,3 +25,5 @@ golang.org/x/sys/windows/registry
 golang.org/x/sys/windows/svc
 golang.org/x/sys/windows/svc/eventlog
 golang.org/x/sys/windows/svc/mgr
+# google.golang.org/protobuf v1.22.0
+## explicit