[changed] pinned certs to check the server connected to as well (#2247)

* [changed] pinned certs to check the server connected to as well

on reload clients with removed pinned certs will be disconnected.
The check happens only on tls handshake now.

Signed-off-by: Matthias Hanel <mh@synadia.com>
This commit is contained in:
Matthias Hanel
2021-05-24 17:28:32 -04:00
committed by GitHub
parent f5c3da32ff
commit b1dee292e6
10 changed files with 136 additions and 54 deletions

View File

@@ -187,19 +187,6 @@ func (s *Server) checkAuthforWarnings() {
// Warning about using plaintext passwords.
s.Warnf("Plaintext passwords detected, use nkeys or bcrypt")
}
// only warn about client connections others use bidirectional TLS
if len(s.opts.TLSPinnedCerts) > 0 && !(s.opts.TLSVerify || s.opts.TLSMap) {
s.Warnf("Pinned Certs specified but no verify or verify_and_map that would require presenting a client cert")
}
if len(s.opts.Websocket.TLSPinnedCerts) > 0 && !(s.opts.Websocket.TLSMap) {
s.Warnf("Websocket Pinned Certs specified but no verify_and_map that would require presenting a client cert")
}
if len(s.opts.MQTT.TLSPinnedCerts) > 0 && !(s.opts.MQTT.TLSMap) {
s.Warnf("MQTT Pinned Certs specified but no verify_and_map that would require presenting a client cert")
}
if len(s.opts.LeafNode.TLSPinnedCerts) > 0 && !(s.opts.LeafNode.TLSMap) {
s.Warnf("Leaf Pinned Certs specified but verify_and_map that would require presenting a client cert")
}
}
// If Users or Nkeys options have definitions without an account defined,
@@ -360,7 +347,7 @@ func (s *Server) isClientAuthorized(c *client) bool {
}
// returns false if the client needs to be disconnected
func (s *Server) matchesPinnedCert(c *client, tlsPinnedCerts PinnedCertSet) bool {
func (c *client) matchesPinnedCert(tlsPinnedCerts PinnedCertSet) bool {
if tlsPinnedCerts == nil {
return true
}
@@ -400,6 +387,11 @@ func (s *Server) processClientOrLeafAuthentication(c *client, opts *Options) boo
authRequired = s.websocket.authOverride
}
}
if !authRequired {
// TODO(dlc) - If they send us credentials should we fail?
s.mu.Unlock()
return true
}
var (
username string
password string
@@ -407,14 +399,12 @@ func (s *Server) processClientOrLeafAuthentication(c *client, opts *Options) boo
noAuthUser string
)
tlsMap := opts.TLSMap
tlsPinnedCerts := opts.TLSPinnedCerts
if c.kind == CLIENT {
switch c.clientType() {
case MQTT:
mo := &opts.MQTT
// Always override TLSMap.
tlsMap = mo.TLSMap
tlsPinnedCerts = mo.TLSPinnedCerts
// The rest depends on if there was any auth override in
// the mqtt's config.
if s.mqtt.authOverride {
@@ -428,7 +418,6 @@ func (s *Server) processClientOrLeafAuthentication(c *client, opts *Options) boo
wo := &opts.Websocket
// Always override TLSMap.
tlsMap = wo.TLSMap
tlsPinnedCerts = wo.TLSPinnedCerts
// The rest depends on if there was any auth override in
// the websocket's config.
if s.websocket.authOverride {
@@ -441,16 +430,6 @@ func (s *Server) processClientOrLeafAuthentication(c *client, opts *Options) boo
}
} else {
tlsMap = opts.LeafNode.TLSMap
tlsPinnedCerts = opts.LeafNode.TLSPinnedCerts
}
if !s.matchesPinnedCert(c, tlsPinnedCerts) {
s.mu.Unlock()
return false
}
if !authRequired {
// TODO(dlc) - If they send us credentials should we fail?
s.mu.Unlock()
return true
}
if !ao {
@@ -915,10 +894,6 @@ func (s *Server) isRouterAuthorized(c *client) bool {
return s.opts.CustomRouterAuthentication.Check(c)
}
if !s.matchesPinnedCert(c, opts.Cluster.TLSPinnedCerts) {
return false
}
if opts.Cluster.TLSMap || opts.Cluster.TLSCheckKnownURLs {
return checkClientTLSCertSubject(c, func(user string, _ *ldap.DN, isDNSAltName bool) (string, bool) {
if user == "" {
@@ -954,10 +929,6 @@ func (s *Server) isGatewayAuthorized(c *client) bool {
// Snapshot server options.
opts := s.getOpts()
if !s.matchesPinnedCert(c, opts.Gateway.TLSPinnedCerts) {
return false
}
// Check whether TLS map is enabled, otherwise use single user/pass.
if opts.Gateway.TLSMap || opts.Gateway.TLSCheckKnownURLs {
return checkClientTLSCertSubject(c, func(user string, _ *ldap.DN, isDNSAltName bool) (string, bool) {
@@ -1020,10 +991,6 @@ func (s *Server) isLeafNodeAuthorized(c *client) bool {
return s.registerLeafWithAccount(c, account)
}
if !s.matchesPinnedCert(c, opts.LeafNode.TLSPinnedCerts) {
return false
}
// If leafnodes config has an authorization{} stanza, this takes precedence.
// The user in CONNECT must match. We will bind to the account associated
// with that user (from the leafnode's authorization{} config).

View File

@@ -4929,21 +4929,21 @@ func (c *client) getClientInfo(detailed bool) *ClientInfo {
return &ci
}
func (c *client) doTLSServerHandshake(typ string, tlsConfig *tls.Config, timeout float64) error {
_, err := c.doTLSHandshake(typ, false, nil, tlsConfig, _EMPTY_, timeout)
func (c *client) doTLSServerHandshake(typ string, tlsConfig *tls.Config, timeout float64, pCerts PinnedCertSet) error {
_, err := c.doTLSHandshake(typ, false, nil, tlsConfig, _EMPTY_, timeout, pCerts)
return err
}
func (c *client) doTLSClientHandshake(typ string, url *url.URL, tlsConfig *tls.Config, tlsName string, timeout float64) (bool, error) {
return c.doTLSHandshake(typ, true, url, tlsConfig, tlsName, timeout)
func (c *client) doTLSClientHandshake(typ string, url *url.URL, tlsConfig *tls.Config, tlsName string, timeout float64, pCerts PinnedCertSet) (bool, error) {
return c.doTLSHandshake(typ, true, url, tlsConfig, tlsName, timeout, pCerts)
}
// Performs eithe server or client side (if solicit is true) TLS Handshake.
// Performs either server or client side (if solicit is true) TLS Handshake.
// On error, the TLS handshake error has been logged and the connection
// has been closed.
//
// Lock is held on entry.
func (c *client) doTLSHandshake(typ string, solicit bool, url *url.URL, tlsConfig *tls.Config, tlsName string, timeout float64) (bool, error) {
func (c *client) doTLSHandshake(typ string, solicit bool, url *url.URL, tlsConfig *tls.Config, tlsName string, timeout float64, pCerts PinnedCertSet) (bool, error) {
var host string
var resetTLSName bool
var err error
@@ -4992,6 +4992,11 @@ func (c *client) doTLSHandshake(typ string, solicit bool, url *url.URL, tlsConfi
}
}
}
} else if !c.matchesPinnedCert(pCerts) {
err = ErrCertNotPinned
}
if err != nil {
if kind == CLIENT {
c.Errorf("TLS handshake error: %v", err)
} else {

View File

@@ -211,6 +211,9 @@ var (
// ErrReplicasNotSupported is returned when a stream with replicas > 1 in non-clustered mode.
ErrReplicasNotSupported = errors.New("replicas > 1 not supported in non-clustered mode")
// ErrCertNotPinned is returned when pinned certs are set and the certificate is not in it
ErrCertNotPinned = errors.New("certificate not pinned")
)
// configErr is a configuration error.

View File

@@ -801,7 +801,7 @@ func (s *Server) createGateway(cfg *gatewayCfg, url *url.URL, conn net.Conn) {
}
// Perform (either server or client side) TLS handshake.
if resetTLSName, err := c.doTLSHandshake("gateway", solicit, url, tlsConfig, tlsName, timeout); err != nil {
if resetTLSName, err := c.doTLSHandshake("gateway", solicit, url, tlsConfig, tlsName, timeout, opts.Gateway.TLSPinnedCerts); err != nil {
if resetTLSName {
cfg.Lock()
cfg.tlsName = _EMPTY_

View File

@@ -898,7 +898,7 @@ func (s *Server) createLeafNode(conn net.Conn, rURL *url.URL, remote *leafNodeCf
// Check to see if we need to spin up TLS.
if !c.isWebsocket() && info.TLSRequired {
// Perform server-side TLS handshake.
if err := c.doTLSServerHandshake("leafnode", opts.LeafNode.TLSConfig, opts.LeafNode.TLSTimeout); err != nil {
if err := c.doTLSServerHandshake("leafnode", opts.LeafNode.TLSConfig, opts.LeafNode.TLSTimeout, opts.LeafNode.TLSPinnedCerts); err != nil {
c.mu.Unlock()
return nil
}
@@ -2220,7 +2220,7 @@ func (c *client) leafNodeSolicitWSConnection(opts *Options, rURL *url.URL, remot
// Do TLS here as needed.
if tlsRequired {
// Perform the client-side TLS handshake.
if resetTLSName, err := c.doTLSClientHandshake("leafnode", rURL, tlsConfig, tlsName, tlsTimeout); err != nil {
if resetTLSName, err := c.doTLSClientHandshake("leafnode", rURL, tlsConfig, tlsName, tlsTimeout, opts.LeafNode.TLSPinnedCerts); err != nil {
// Check if we need to reset the remote's TLS name.
if resetTLSName {
remote.Lock()
@@ -2360,7 +2360,7 @@ func (s *Server) leafNodeResumeConnectProcess(c *client) {
rURL := remote.getCurrentURL()
// Perform the client-side TLS handshake.
if resetTLSName, err := c.doTLSClientHandshake("leafnode", rURL, tlsConfig, tlsName, tlsTimeout); err != nil {
if resetTLSName, err := c.doTLSClientHandshake("leafnode", rURL, tlsConfig, tlsName, tlsTimeout, c.srv.getOpts().LeafNode.TLSPinnedCerts); err != nil {
// Check if we need to reset the remote's TLS name.
if resetTLSName {
remote.Lock()

View File

@@ -472,7 +472,7 @@ func (s *Server) createMQTTClient(conn net.Conn) *client {
}
// Perform server-side TLS handshake.
if err := c.doTLSServerHandshake("mqtt", opts.MQTT.TLSConfig, opts.MQTT.TLSTimeout); err != nil {
if err := c.doTLSServerHandshake("mqtt", opts.MQTT.TLSConfig, opts.MQTT.TLSTimeout, opts.MQTT.TLSPinnedCerts); err != nil {
c.mu.Unlock()
return nil
}

View File

@@ -628,6 +628,59 @@ func (o *mqttMaxAckPendingReload) Apply(s *Server) {
s.Noticef("Reloaded: MQTT max_ack_pending = %v", o.newValue)
}
// Compares options and disconnects clients that are no longer listed in pinned certs. Lock must not be held.
func (s *Server) recheckPinnedCerts(curOpts *Options, newOpts *Options) {
s.mu.Lock()
disconnectClients := []*client{}
protoToPinned := map[int]PinnedCertSet{}
if !reflect.DeepEqual(newOpts.TLSPinnedCerts, curOpts.TLSPinnedCerts) {
protoToPinned[NATS] = curOpts.TLSPinnedCerts
}
if !reflect.DeepEqual(newOpts.MQTT.TLSPinnedCerts, curOpts.MQTT.TLSPinnedCerts) {
protoToPinned[MQTT] = curOpts.MQTT.TLSPinnedCerts
}
if !reflect.DeepEqual(newOpts.Websocket.TLSPinnedCerts, curOpts.Websocket.TLSPinnedCerts) {
protoToPinned[WS] = curOpts.Websocket.TLSPinnedCerts
}
for _, c := range s.clients {
if c.kind != CLIENT {
continue
}
if pinned, ok := protoToPinned[c.clientType()]; ok {
if !c.matchesPinnedCert(pinned) {
disconnectClients = append(disconnectClients, c)
}
}
}
checkClients := func(kind int, clients map[uint64]*client, set PinnedCertSet) {
for _, c := range clients {
if c.kind == kind && !c.matchesPinnedCert(set) {
disconnectClients = append(disconnectClients, c)
}
}
}
if !reflect.DeepEqual(newOpts.LeafNode.TLSPinnedCerts, curOpts.LeafNode.TLSPinnedCerts) {
checkClients(LEAF, s.leafs, newOpts.LeafNode.TLSPinnedCerts)
}
if !reflect.DeepEqual(newOpts.Cluster.TLSPinnedCerts, curOpts.Cluster.TLSPinnedCerts) {
checkClients(ROUTER, s.routes, newOpts.Cluster.TLSPinnedCerts)
}
if reflect.DeepEqual(newOpts.Gateway.TLSPinnedCerts, curOpts.Gateway.TLSPinnedCerts) {
for _, c := range s.remotes {
if !c.matchesPinnedCert(newOpts.Gateway.TLSPinnedCerts) {
disconnectClients = append(disconnectClients, c)
}
}
}
s.mu.Unlock()
if len(disconnectClients) > 0 {
s.Noticef("Disconnect %d clients due to pinned certs reload", len(disconnectClients))
for _, c := range disconnectClients {
c.closeConnection(TLSHandshakeError)
}
}
}
// Reload reads the current configuration file and applies any supported
// changes. This returns an error if the server was not started with a config
// file or an option which doesn't support hot-swapping was changed.
@@ -705,6 +758,9 @@ func (s *Server) Reload() error {
if err := s.reloadOptions(curOpts, newOpts); err != nil {
return err
}
s.recheckPinnedCerts(curOpts, newOpts)
s.mu.Lock()
s.configTime = time.Now().UTC()
s.updateVarzConfigReloadableFields(s.varz)

View File

@@ -1332,8 +1332,7 @@ func (s *Server) createRoute(conn net.Conn, rURL *url.URL) *client {
tlsConfig = tlsConfig.Clone()
}
// Perform (server or client side) TLS handshake.
if _, err := c.doTLSHandshake("route", didSolicit, rURL, tlsConfig, _EMPTY_, opts.Cluster.TLSTimeout); err != nil {
c.mu.Unlock()
if _, err := c.doTLSHandshake("route", didSolicit, rURL, tlsConfig, _EMPTY_, opts.Cluster.TLSTimeout, opts.Cluster.TLSPinnedCerts); err != nil {
return nil
}
}

View File

@@ -2353,7 +2353,7 @@ func (s *Server) createClient(conn net.Conn) *client {
pre = nil
}
// Performs server-side TLS handshake.
if err := c.doTLSServerHandshake(_EMPTY_, opts.TLSConfig, opts.TLSTimeout); err != nil {
if err := c.doTLSServerHandshake(_EMPTY_, opts.TLSConfig, opts.TLSTimeout, opts.TLSPinnedCerts); err != nil {
c.mu.Unlock()
return nil
}

View File

@@ -1936,7 +1936,7 @@ func TestTLSClientSVIDAuth(t *testing.T) {
}
}
func TestTLSPinnedCerts(t *testing.T) {
func TestTLSPinnedCertsClient(t *testing.T) {
tmpl := `
host: localhost
port: -1
@@ -1980,3 +1980,55 @@ func TestTLSPinnedCerts(t *testing.T) {
}
nc.Close()
}
func TestTLSPinnedCertsRoute(t *testing.T) {
tmplSeed := `
host: localhost
port: -1
cluster {
port: -1
tls {
ca_file: "configs/certs/ca.pem"
cert_file: "configs/certs/server-cert.pem"
key_file: "configs/certs/server-key.pem"
}
}`
// this server connects to seed, but is set up to not trust seeds cert
tmplSrv := `
host: localhost
port: -1
cluster {
port: -1
routes = [nats-route://localhost:%d]
tls {
ca_file: "configs/certs/ca.pem"
cert_file: "configs/certs/server-cert.pem"
key_file: "configs/certs/server-key.pem"
# Require a client certificate and map user id from certificate
verify: true
# expected to fail the seed server
pinned_certs: ["%s"]
}
}`
confSeed := createConfFile(t, []byte(tmplSeed))
defer removeFile(t, confSeed)
srvSeed, o := RunServerWithConfig(confSeed)
defer srvSeed.Shutdown()
confSrv := createConfFile(t, []byte(fmt.Sprintf(tmplSrv, o.Cluster.Port, "89386860ea1222698ea676fc97310bdf2bff6f7e2b0420fac3b3f8f5a08fede5")))
defer removeFile(t, confSrv)
srv, _ := RunServerWithConfig(confSrv)
defer srv.Shutdown()
checkClusterFormed(t, srvSeed, srv)
// this change will result in the server being and remaining disconnected
ioutil.WriteFile(confSrv, []byte(fmt.Sprintf(tmplSrv, o.Cluster.Port, "aaaaaaaa09fde09451411ba3b42c0f74727d61a974c69fd3cf5257f39c75f0e9")), 0660)
if err := srv.Reload(); err != nil {
t.Fatalf("on Reload got %v", err)
}
checkNumRoutes(t, srvSeed, 0)
checkNumRoutes(t, srv, 0)
}