diff --git a/server/jwt_test.go b/server/jwt_test.go index 0c7bdb64..69d0cb07 100644 --- a/server/jwt_test.go +++ b/server/jwt_test.go @@ -37,6 +37,8 @@ import ( var ( // This matches ./configs/nkeys_jwts/test.seed oSeed = []byte("SOAFYNORQLQFJYBYNUGC5D7SH2MXMUX5BFEWWGHN3EK4VGG5TPT5DZP7QU") + // This matches ./configs/nkeys/op.jwt + ojwt = "eyJ0eXAiOiJqd3QiLCJhbGciOiJlZDI1NTE5In0.eyJhdWQiOiJURVNUUyIsImV4cCI6MTg1OTEyMTI3NSwianRpIjoiWE5MWjZYWVBIVE1ESlFSTlFPSFVPSlFHV0NVN01JNVc1SlhDWk5YQllVS0VRVzY3STI1USIsImlhdCI6MTU0Mzc2MTI3NSwiaXNzIjoiT0NBVDMzTVRWVTJWVU9JTUdOR1VOWEo2NkFIMlJMU0RBRjNNVUJDWUFZNVFNSUw2NU5RTTZYUUciLCJuYW1lIjoiU3luYWRpYSBDb21tdW5pY2F0aW9ucyBJbmMuIiwibmJmIjoxNTQzNzYxMjc1LCJzdWIiOiJPQ0FUMzNNVFZVMlZVT0lNR05HVU5YSjY2QUgyUkxTREFGM01VQkNZQVk1UU1JTDY1TlFNNlhRRyIsInR5cGUiOiJvcGVyYXRvciIsIm5hdHMiOnsic2lnbmluZ19rZXlzIjpbIk9EU0tSN01ZRlFaNU1NQUo2RlBNRUVUQ1RFM1JJSE9GTFRZUEpSTUFWVk40T0xWMllZQU1IQ0FDIiwiT0RTS0FDU1JCV1A1MzdEWkRSVko2NTdKT0lHT1BPUTZLRzdUNEhONk9LNEY2SUVDR1hEQUhOUDIiLCJPRFNLSTM2TFpCNDRPWTVJVkNSNlA1MkZaSlpZTVlXWlZXTlVEVExFWjVUSzJQTjNPRU1SVEFCUiJdfX0.hyfz6E39BMUh0GLzovFfk3wT4OfualftjdJ_eYkLfPvu5tZubYQ_Pn9oFYGCV_6yKy3KMGhWGUCyCdHaPhalBw" ) func opTrustBasicSetup() *Server { @@ -1558,13 +1560,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) @@ -1643,10 +1646,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) @@ -1668,7 +1672,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) @@ -1848,6 +1852,65 @@ func TestAccountURLResolverFetchFailureInCluster(t *testing.T) { } } +func TestAccountURLResolverReturnDifferentOperator(t *testing.T) { + // Create a valid chain of op/acc/usr using a different operator + // This is so we can test if the server rejects this chain. + // Create Operator + op, _ := nkeys.CreateOperator() + // Create Account, this account is the one returned by the resolver + akp, _ := nkeys.CreateAccount() + apub, _ := akp.PublicKey() + nac := jwt.NewAccountClaims(apub) + ajwt, err := nac.Encode(op) + if err != nil { + t.Fatalf("Error generating account JWT: %v", err) + } + // Create User + nkp, _ := nkeys.CreateUser() + uSeed, _ := nkp.Seed() + upub, _ := nkp.PublicKey() + nuc := newJWTTestUserClaims() + nuc.Subject = upub + uJwt, err := nuc.Encode(akp) + if err != nil { + t.Fatalf("Error generating user JWT: %v", err) + } + creds := genCredsFile(t, uJwt, uSeed) + defer os.Remove(creds) + // Simulate an account server that was hijacked/mis configured + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Write([]byte(ajwt)) + })) + defer ts.Close() + // Create Server + confA := createConfFile(t, []byte(fmt.Sprintf(` + listen: -1 + operator: %s + resolver: URL("%s/A/") + `, ojwt, ts.URL))) + defer os.Remove(confA) + sA, _ := RunServerWithConfig(confA) + defer sA.Shutdown() + // Create first client, directly connects to A + urlA := fmt.Sprintf("nats://%s:%d", sA.opts.Host, sA.opts.Port) + if _, err := nats.Connect(urlA, nats.UserCredentials(creds), + nats.DisconnectErrHandler(func(_ *nats.Conn, err error) { + if err != nil { + t.Fatal("error not expected in this test", err) + } + }), + nats.ErrorHandler(func(_ *nats.Conn, _ *nats.Subscription, err error) { + t.Fatal("error not expected in this test", err) + }), + ); err == nil { + t.Fatal("Expected connect to fail") + } + // Test if the server has the account in memory. (shouldn't) + if v, ok := sA.accounts.Load(apub); ok { + t.Fatalf("Expected account to NOT be in memory: %v", v.(*Account)) + } +} + func TestJWTUserSigningKey(t *testing.T) { s := opTrustBasicSetup() defer s.Shutdown() diff --git a/server/server.go b/server/server.go index 30212bc9..3acc46bc 100644 --- a/server/server.go +++ b/server/server.go @@ -1235,6 +1235,9 @@ func (s *Server) verifyAccountClaims(claimJWT string) (*jwt.AccountClaims, strin if vr.IsBlocking(true) { return nil, _EMPTY_, ErrAccountValidation } + if !s.isTrustedIssuer(accClaims.Issuer) { + return nil, _EMPTY_, ErrAccountValidation + } return accClaims, claimJWT, nil } @@ -1242,32 +1245,32 @@ func (s *Server) verifyAccountClaims(claimJWT string) (*jwt.AccountClaims, strin // Lock is NOT held upon entry. func (s *Server) fetchAccount(name string) (*Account, error) { accClaims, claimJWT, err := s.fetchAccountClaims(name) - if accClaims != nil { - acc := s.buildInternalAccount(accClaims) - acc.claimJWT = claimJWT - // Due to possible race, if registerAccount() returns a non - // nil account, it means the same account was already - // registered and we should use this one. - if racc := s.registerAccount(acc); racc != nil { - // Update with the new claims in case they are new. - // Following call will ignore ErrAccountResolverSameClaims - // if claims are the same. - err = s.updateAccountWithClaimJWT(racc, claimJWT) - if err != nil && err != ErrAccountResolverSameClaims { - return nil, err - } - return racc, nil - } - // The sub imports may have been setup but will not have had their - // subscriptions properly setup. Do that here. - if len(acc.imports.services) > 0 && acc.ic == nil { - acc.ic = s.createInternalAccountClient() - acc.ic.acc = acc - acc.addAllServiceImportSubs() - } - return acc, nil + if accClaims == nil { + return nil, err } - return nil, err + acc := s.buildInternalAccount(accClaims) + acc.claimJWT = claimJWT + // Due to possible race, if registerAccount() returns a non + // nil account, it means the same account was already + // registered and we should use this one. + if racc := s.registerAccount(acc); racc != nil { + // Update with the new claims in case they are new. + // Following call will ignore ErrAccountResolverSameClaims + // if claims are the same. + err = s.updateAccountWithClaimJWT(racc, claimJWT) + if err != nil && err != ErrAccountResolverSameClaims { + return nil, err + } + return racc, nil + } + // The sub imports may have been setup but will not have had their + // subscriptions properly setup. Do that here. + if len(acc.imports.services) > 0 && acc.ic == nil { + acc.ic = s.createInternalAccountClient() + acc.ic.acc = acc + acc.addAllServiceImportSubs() + } + return acc, nil } // Start up the server, this will block. diff --git a/test/operator_test.go b/test/operator_test.go index 2ee585c3..814bad62 100644 --- a/test/operator_test.go +++ b/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() opt, _ := createUserCredsOption(t, s, akp) @@ -221,11 +231,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) { @@ -235,15 +249,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) } @@ -257,10 +271,9 @@ 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) }