Merge pull request #1594 from nats-io/fix_websocket_auth

Added an allowed connection type filter for users

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-20200827232814-292806fa48ba
+	github.com/nats-io/jwt/v2 v2.0.0-20200916203241-1f8ce17dff02
 	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

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-20200827232814-292806fa48ba h1:1sdQj5FtOMA3AWDyDYXwFNfLiOqMX0/u8EeyzBxHCRo=
-github.com/nats-io/jwt/v2 v2.0.0-20200827232814-292806fa48ba/go.mod h1:vs+ZEjP+XKy8szkBmQwCB7RjYdIlMaPsFPs4VdS4bTQ=
+github.com/nats-io/jwt/v2 v2.0.0-20200916203241-1f8ce17dff02 h1:WloZv3SCb55D/rOHYy1rWBXLrj3BYc9zw8VIq6X54lI=
+github.com/nats-io/jwt/v2 v2.0.0-20200916203241-1f8ce17dff02/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=

server/accounts.go

@@ -2764,8 +2764,8 @@ func buildPermissionsFromJwt(uc *jwt.Permissions) *Permissions {
 }
 
 // Helper to build internal NKeyUser.
-func buildInternalNkeyUser(uc *jwt.UserClaims, acc *Account) *NkeyUser {
-	nu := &NkeyUser{Nkey: uc.Subject, Account: acc}
+func buildInternalNkeyUser(uc *jwt.UserClaims, acts map[string]struct{}, acc *Account) *NkeyUser {
+	nu := &NkeyUser{Nkey: uc.Subject, Account: acc, AllowedConnectionTypes: acts}
 	if uc.IssuerAccount != "" {
 		nu.SigningKey = uc.Issuer
 	}

server/auth.go

@@ -50,18 +50,20 @@ type ClientAuthentication interface {
 
 // NkeyUser is for multiple nkey based users
 type NkeyUser struct {
-	Nkey        string       `json:"user"`
-	Permissions *Permissions `json:"permissions,omitempty"`
-	Account     *Account     `json:"account,omitempty"`
-	SigningKey  string       `json:"signing_key,omitempty"`
+	Nkey                   string              `json:"user"`
+	Permissions            *Permissions        `json:"permissions,omitempty"`
+	Account                *Account            `json:"account,omitempty"`
+	SigningKey             string              `json:"signing_key,omitempty"`
+	AllowedConnectionTypes map[string]struct{} `json:"connection_types,omitempty"`
 }
 
 // User is for multiple accounts/users.
 type User struct {
-	Username    string       `json:"user"`
-	Password    string       `json:"password"`
-	Permissions *Permissions `json:"permissions,omitempty"`
-	Account     *Account     `json:"account,omitempty"`
+	Username               string              `json:"user"`
+	Password               string              `json:"password"`
+	Permissions            *Permissions        `json:"permissions,omitempty"`
+	Account                *Account            `json:"account,omitempty"`
+	AllowedConnectionTypes map[string]struct{} `json:"connection_types,omitempty"`
 }
 
 // clone performs a deep copy of the User struct, returning a new clone with
@@ -356,8 +358,6 @@ func (s *Server) processClientOrLeafAuthentication(c *client, opts *Options) boo
 		password   string
 		token      string
 		noAuthUser string
-		users      map[string]*User
-		nkusers    map[string]*NkeyUser
 	)
 	tlsMap := opts.TLSMap
 	if c.ws != nil {
@@ -371,8 +371,6 @@ func (s *Server) processClientOrLeafAuthentication(c *client, opts *Options) boo
 			username = wo.Username
 			password = wo.Password
 			token = wo.Token
-			users = s.websocket.users
-			nkusers = s.websocket.nkeys
 			ao = true
 		}
 	} else if c.kind == LEAF {
@@ -383,8 +381,6 @@ func (s *Server) processClientOrLeafAuthentication(c *client, opts *Options) boo
 		username = opts.Username
 		password = opts.Password
 		token = opts.Authorization
-		users = s.users
-		nkusers = s.nkeys
 	}
 
 	// Check if we have trustedKeys defined in the server. If so we require a user jwt.
@@ -411,11 +407,11 @@ func (s *Server) processClientOrLeafAuthentication(c *client, opts *Options) boo
 	}
 
 	// Check if we have nkeys or users for client.
-	hasNkeys := len(nkusers) > 0
-	hasUsers := len(users) > 0
+	hasNkeys := len(s.nkeys) > 0
+	hasUsers := len(s.users) > 0
 	if hasNkeys && c.opts.Nkey != "" {
-		nkey, ok = nkusers[c.opts.Nkey]
-		if !ok {
+		nkey, ok = s.nkeys[c.opts.Nkey]
+		if !ok || !c.connectionTypeAllowed(nkey.AllowedConnectionTypes) {
 			s.mu.Unlock()
 			return false
 		}
@@ -426,8 +422,8 @@ func (s *Server) processClientOrLeafAuthentication(c *client, opts *Options) boo
 				// First do literal lookup using the resulting string representation
 				// of RDNSequence as implemented by the pkix package from Go.
 				if u != "" {
-					usr, ok := users[u]
-					if !ok {
+					usr, ok := s.users[u]
+					if !ok || !c.connectionTypeAllowed(usr.AllowedConnectionTypes) {
 						return "", ok
 					}
 					user = usr
@@ -440,7 +436,10 @@ func (s *Server) processClientOrLeafAuthentication(c *client, opts *Options) boo
 
 				// Look through the accounts for an RDN that is equal to the one
 				// presented by the certificate.
-				for _, usr := range users {
+				for _, usr := range s.users {
+					if !c.connectionTypeAllowed(usr.AllowedConnectionTypes) {
+						continue
+					}
 					// TODO: Use this utility to make a full validation pass
 					// on start in case tlsmap feature is being used.
 					inputRDN, err := ldap.ParseDN(usr.Username)
@@ -466,14 +465,14 @@ func (s *Server) processClientOrLeafAuthentication(c *client, opts *Options) boo
 			c.opts.Username = user.Username
 		} else {
 			if c.kind == CLIENT && c.opts.Username == "" && noAuthUser != "" {
-				if u, exists := users[noAuthUser]; exists {
+				if u, exists := s.users[noAuthUser]; exists {
 					c.opts.Username = u.Username
 					c.opts.Password = u.Password
 				}
 			}
 			if c.opts.Username != "" {
-				user, ok = users[c.opts.Username]
-				if !ok {
+				user, ok = s.users[c.opts.Username]
+				if !ok || !c.connectionTypeAllowed(user.AllowedConnectionTypes) {
 					s.mu.Unlock()
 					return false
 				}
@@ -485,6 +484,34 @@ func (s *Server) processClientOrLeafAuthentication(c *client, opts *Options) boo
 	// If we have a jwt and a userClaim, make sure we have the Account, etc associated.
 	// We need to look up the account. This will use an account resolver if one is present.
 	if juc != nil {
+		allowedConnTypes, err := convertAllowedConnectionTypes(juc.AllowedConnectionTypes)
+		if err != nil {
+			// We got an error, which means some connection types were unknown. As long as
+			// a valid one is returned, we proceed with auth. If not, we have to reject.
+			// In other words, suppose that JWT allows "WEBSOCKET" in the array. No error
+			// is returned and allowedConnTypes will contain "WEBSOCKET" only.
+			// Client will be rejected if not a websocket client, or proceed with rest of
+			// auth if it is.
+			// Now suppose JWT allows "WEBSOCKET, MQTT" and say MQTT is not known by this
+			// server. In this case, allowedConnTypes would contain "WEBSOCKET" and we
+			// would get `err` indicating that "MQTT" is an unknown connection type.
+			// If a websocket client connects, it should still be allowed, since after all
+			// the admin wanted to allow websocket and mqtt connection types.
+			// However, say that the JWT only allows "MQTT" (and again suppose this server
+			// does not know about MQTT connection type), then since the allowedConnTypes
+			// map would be empty (no valid types found), and since empty means allow-all,
+			// then we should reject because the intent was to allow connections for this
+			// user only as an MQTT client.
+			c.Debugf("%v", err)
+			if len(allowedConnTypes) == 0 {
+				return false
+			}
+			err = nil
+		}
+		if !c.connectionTypeAllowed(allowedConnTypes) {
+			c.Debugf("Connection type not allowed")
+			return false
+		}
 		issuer := juc.Issuer
 		if juc.IssuerAccount != "" {
 			issuer = juc.IssuerAccount
@@ -546,7 +573,7 @@ func (s *Server) processClientOrLeafAuthentication(c *client, opts *Options) boo
 			return false
 		}
 
-		nkey = buildInternalNkeyUser(juc, acc)
+		nkey = buildInternalNkeyUser(juc, allowedConnTypes, acc)
 		if err := c.RegisterNkeyUser(nkey); err != nil {
 			return false
 		}
@@ -895,21 +922,51 @@ func comparePasswords(serverPassword, clientPassword string) bool {
 }
 
 func validateAuth(o *Options) error {
-	if o.NoAuthUser == "" {
+	for _, u := range o.Users {
+		if err := validateAllowedConnectionTypes(u.AllowedConnectionTypes); err != nil {
+			return err
+		}
+	}
+	for _, u := range o.Nkeys {
+		if err := validateAllowedConnectionTypes(u.AllowedConnectionTypes); err != nil {
+			return err
+		}
+	}
+	return validateNoAuthUser(o, o.NoAuthUser)
+}
+
+func validateAllowedConnectionTypes(m map[string]struct{}) error {
+	for ct := range m {
+		ctuc := strings.ToUpper(ct)
+		switch ctuc {
+		case jwt.ConnectionTypeStandard, jwt.ConnectionTypeWebsocket, jwt.ConnectionTypeLeafnode:
+		default:
+			return fmt.Errorf("unknown connection type %q", ct)
+		}
+		if ctuc != ct {
+			delete(m, ct)
+			m[ctuc] = struct{}{}
+		}
+	}
+	return nil
+}
+
+func validateNoAuthUser(o *Options, noAuthUser string) error {
+	if noAuthUser == "" {
 		return nil
 	}
 	if len(o.TrustedOperators) > 0 {
 		return fmt.Errorf("no_auth_user not compatible with Trusted Operator")
 	}
 	if o.Users == nil {
-		return fmt.Errorf(`no_auth_user: "%s" present, but users are not defined`, o.NoAuthUser)
+		return fmt.Errorf(`no_auth_user: "%s" present, but users are not defined`, noAuthUser)
 	}
 	for _, u := range o.Users {
-		if u.Username == o.NoAuthUser {
+		if u.Username == noAuthUser {
 			return nil
 		}
 	}
 	return fmt.Errorf(
 		`no_auth_user: "%s" not present as user in authorization block or account configuration`,
-		o.NoAuthUser)
+		noAuthUser)
 }

server/auth_test.go

@@ -15,7 +15,10 @@ package server
 
 import (
 	"reflect"
+	"strings"
 	"testing"
+
+	"github.com/nats-io/jwt/v2"
 )
 
 func TestUserCloneNilPermissions(t *testing.T) {
@@ -101,3 +104,53 @@ func TestUserCloneNil(t *testing.T) {
 		t.Fatalf("Expected nil, got: %+v", clone)
 	}
 }
+
+func TestUserUnknownAllowedConnectionType(t *testing.T) {
+	o := DefaultOptions()
+	o.Users = []*User{&User{
+		Username:               "user",
+		Password:               "pwd",
+		AllowedConnectionTypes: testCreateAllowedConnectionTypes([]string{jwt.ConnectionTypeStandard, "someNewType"}),
+	}}
+	_, err := NewServer(o)
+	if err == nil || !strings.Contains(err.Error(), "connection type") {
+		t.Fatalf("Expected error about unknown connection type, got %v", err)
+	}
+
+	o.Users[0].AllowedConnectionTypes = testCreateAllowedConnectionTypes([]string{"websocket"})
+	s, err := NewServer(o)
+	if err != nil {
+		t.Fatalf("Unexpected error: %v", err)
+	}
+	s.mu.Lock()
+	user := s.opts.Users[0]
+	s.mu.Unlock()
+	for act := range user.AllowedConnectionTypes {
+		if act != jwt.ConnectionTypeWebsocket {
+			t.Fatalf("Expected map to have been updated with proper case, got %v", act)
+		}
+	}
+	// Same with NKey user now.
+	o.Users = nil
+	o.Nkeys = []*NkeyUser{&NkeyUser{
+		Nkey:                   "somekey",
+		AllowedConnectionTypes: testCreateAllowedConnectionTypes([]string{jwt.ConnectionTypeStandard, "someNewType"}),
+	}}
+	_, err = NewServer(o)
+	if err == nil || !strings.Contains(err.Error(), "connection type") {
+		t.Fatalf("Expected error about unknown connection type, got %v", err)
+	}
+	o.Nkeys[0].AllowedConnectionTypes = testCreateAllowedConnectionTypes([]string{"websocket"})
+	s, err = NewServer(o)
+	if err != nil {
+		t.Fatalf("Unexpected error: %v", err)
+	}
+	s.mu.Lock()
+	nkey := s.opts.Nkeys[0]
+	s.mu.Unlock()
+	for act := range nkey.AllowedConnectionTypes {
+		if act != jwt.ConnectionTypeWebsocket {
+			t.Fatalf("Expected map to have been updated with proper case, got %v", act)
+		}
+	}
+}

server/client.go

@@ -4395,6 +4395,55 @@ func (c *client) getAuthUser() string {
 	}
 }
 
+// Given an array of strings, this function converts it to a map as long
+// as all the content (converted to upper-case) matches some constants.
+
+// Converts the given array of strings to a map of string.
+// The strings are converted to upper-case and added to the map only
+// if the server recognize them as valid connection types.
+// If there are unknown connection types, the map of valid ones is returned
+// along with an error that contains the name of the unknown.
+func convertAllowedConnectionTypes(cts []string) (map[string]struct{}, error) {
+	var unknown []string
+	m := make(map[string]struct{}, len(cts))
+	for _, i := range cts {
+		i = strings.ToUpper(i)
+		switch i {
+		case jwt.ConnectionTypeStandard, jwt.ConnectionTypeWebsocket, jwt.ConnectionTypeLeafnode:
+			m[i] = struct{}{}
+		default:
+			unknown = append(unknown, i)
+		}
+	}
+	var err error
+	// We will still return the map of valid ones.
+	if len(unknown) != 0 {
+		err = fmt.Errorf("invalid connection types %q", unknown)
+	}
+	return m, err
+}
+
+// This will return true if the connection is of a type present in the given `acts` map.
+// Note that so far this is used only for CLIENT or LEAF connections.
+// But a CLIENT can be standard or websocket (and other types in the future).
+func (c *client) connectionTypeAllowed(acts map[string]struct{}) bool {
+	// Empty means all type of clients are allowed
+	if len(acts) == 0 {
+		return true
+	}
+	// Assume standard client, then update based on presence of websocket
+	// or other type.
+	want := jwt.ConnectionTypeStandard
+	if c.kind == LEAF {
+		want = jwt.ConnectionTypeLeafnode
+	}
+	if c.ws != nil {
+		want = jwt.ConnectionTypeWebsocket
+	}
+	_, ok := acts[want]
+	return ok
+}
+
 // isClosed returns true if either closeConnection or connMarkedClosed
 // flag have been set, or if `nc` is nil, which may happen in tests.
 func (c *client) isClosed() bool {