From 3589d4c4e3c946092b86609e4a746f9abf04129a Mon Sep 17 00:00:00 2001 From: Mahmood Ali Date: Tue, 28 Jan 2020 12:58:38 -0500 Subject: [PATCH] Merge pull request #160 from hashicorp/b-mtls-hostname server: validate role and region for RPC w/ mTLS --- nomad/server.go | 74 +++++++++++++++---- nomad/server_test.go | 27 +++++++ .../guides/upgrade/upgrade-specific.html.md | 23 ++++++ 3 files changed, 111 insertions(+), 13 deletions(-) diff --git a/nomad/server.go b/nomad/server.go index 6b7765edfa1..7d69ec57b5c 100644 --- a/nomad/server.go +++ b/nomad/server.go @@ -3,6 +3,8 @@ package nomad import ( "context" "crypto/tls" + "crypto/x509" + "errors" "fmt" "io/ioutil" "net" @@ -11,6 +13,7 @@ import ( "path/filepath" "sort" "strconv" + "strings" "sync" "sync/atomic" "time" @@ -279,7 +282,7 @@ func NewServer(config *Config, consulCatalog consul.CatalogAPI) (*Server, error) if err != nil { return nil, err } - incomingTLS, tlsWrap, err := getTLSConf(config.TLSConfig.EnableRPC, tlsConf) + incomingTLS, tlsWrap, err := getTLSConf(config.TLSConfig.EnableRPC, tlsConf, config.Region) if err != nil { return nil, err } @@ -447,25 +450,70 @@ func (s *Server) createRPCListener() (*net.TCPListener, error) { // getTLSConf gets the server's TLS configuration based on the config supplied // by the operator -func getTLSConf(enableRPC bool, tlsConf *tlsutil.Config) (*tls.Config, tlsutil.RegionWrapper, error) { +func getTLSConf(enableRPC bool, tlsConf *tlsutil.Config, region string) (*tls.Config, tlsutil.RegionWrapper, error) { var tlsWrap tlsutil.RegionWrapper var incomingTLS *tls.Config - if enableRPC { - tw, err := tlsConf.OutgoingTLSWrapper() - if err != nil { - return nil, nil, err - } - tlsWrap = tw + if !enableRPC { + return incomingTLS, tlsWrap, nil + } - itls, err := tlsConf.IncomingTLSConfig() - if err != nil { - return nil, nil, err - } + tlsWrap, err := tlsConf.OutgoingTLSWrapper() + if err != nil { + return nil, nil, err + } + + itls, err := tlsConf.IncomingTLSConfig() + if err != nil { + return nil, nil, err + } + + if tlsConf.VerifyServerHostname { + incomingTLS = itls.Clone() + incomingTLS.VerifyPeerCertificate = rpcNameAndRegionValidator(region) + } else { incomingTLS = itls } return incomingTLS, tlsWrap, nil } +// implements signature of tls.Config.VerifyPeerCertificate which is called +// after the certs have been verified. We'll ignore the raw certs and only +// check the verified certs. +func rpcNameAndRegionValidator(region string) func([][]byte, [][]*x509.Certificate) error { + return func(_ [][]byte, certificates [][]*x509.Certificate) error { + if len(certificates) > 0 && len(certificates[0]) > 0 { + cert := certificates[0][0] + for _, dnsName := range cert.DNSNames { + if validateRPCRegionPeer(dnsName, region) { + return nil + } + } + if validateRPCRegionPeer(cert.Subject.CommonName, region) { + return nil + } + } + return errors.New("invalid role or region for certificate") + } +} + +func validateRPCRegionPeer(name, region string) bool { + parts := strings.Split(name, ".") + if len(parts) < 3 { + // Invalid SAN + return false + } + if parts[len(parts)-1] != "nomad" { + // Incorrect service + return false + } + if parts[0] == "client" { + // Clients may only connect to servers in their region + return name == "client."+region+".nomad" + } + // Servers may connect to any Nomad RPC service for federation. + return parts[0] == "server" +} + // reloadTLSConnections updates a server's TLS configuration and reloads RPC // connections. func (s *Server) reloadTLSConnections(newTLSConfig *config.TLSConfig) error { @@ -483,7 +531,7 @@ func (s *Server) reloadTLSConnections(newTLSConfig *config.TLSConfig) error { return err } - incomingTLS, tlsWrap, err := getTLSConf(newTLSConfig.EnableRPC, tlsConf) + incomingTLS, tlsWrap, err := getTLSConf(newTLSConfig.EnableRPC, tlsConf, s.config.Region) if err != nil { s.logger.Error("unable to reset TLS context", "error", err) return err diff --git a/nomad/server_test.go b/nomad/server_test.go index 226d6bfc2db..797b57e62eb 100644 --- a/nomad/server_test.go +++ b/nomad/server_test.go @@ -558,3 +558,30 @@ func TestServer_InvalidSchedulers(t *testing.T) { require.NotNil(err) require.Contains(err.Error(), "foo") } + +func TestServer_RPCNameAndRegionValidation(t *testing.T) { + t.Parallel() + for _, tc := range []struct { + name string + region string + expected bool + }{ + // OK + {name: "client.global.nomad", region: "global", expected: true}, + {name: "server.global.nomad", region: "global", expected: true}, + {name: "server.other.nomad", region: "global", expected: true}, + {name: "server.other.region.nomad", region: "other.region", expected: true}, + + // Bad + {name: "client.other.nomad", region: "global", expected: false}, + {name: "client.global.nomad.other", region: "global", expected: false}, + {name: "server.global.nomad.other", region: "global", expected: false}, + {name: "other.global.nomad", region: "global", expected: false}, + {name: "server.nomad", region: "global", expected: false}, + {name: "localhost", region: "global", expected: false}, + } { + assert.Equal(t, tc.expected, validateRPCRegionPeer(tc.name, tc.region), + "expected %q in region %q to validate as %v", + tc.name, tc.region, tc.expected) + } +} diff --git a/website/source/guides/upgrade/upgrade-specific.html.md b/website/source/guides/upgrade/upgrade-specific.html.md index cad2335d41d..227694108ee 100644 --- a/website/source/guides/upgrade/upgrade-specific.html.md +++ b/website/source/guides/upgrade/upgrade-specific.html.md @@ -15,6 +15,27 @@ details provided for their upgrades as a result of new features or changed behavior. This page is used to document those details separately from the standard upgrade flow. +## Nomad 0.10.3 + +### mTLS Certificate Validation + +Nomad 0.10.3 includes a fix for a privilege escalation vulnerability in +validating TLS certificates for RPC with mTLS. Nomad RPC endpoints validated +that TLS client certificates had not expired and were signed by the same CA as +the Nomad node, but did not correctly check the certificate's name for the role +and region as described in the [Securing Nomad with TLS][tls-guide] guide. This +allows trusted operators with a client certificate signed by the CA to send RPC +calls as a Nomad client or server node, bypassing access control and accessing +any secrets available to a client. + +Nomad clusters configured for mTLS following the [Securing Nomad with TLS][tls-guide] +guide or the [Vault PKI Secrets Engine Integration][tls-vault-guide] guide +should already have certificates that will pass validation. Before upgrading to +Nomad 0.10.3, operators using mTLS with `verify_server_hostname = true` should +confirm that the common name or SAN of all Nomad client node certs is +`client..nomad`, and that the common name or SAN of all Nomad server +node certs is `server..nomad`. + ## Nomad 0.10.2 ### Preemption Panic Fixed @@ -394,3 +415,5 @@ deleted and then Nomad 0.3.0 can be launched. [task-config]: /docs/job-specification/task.html#config [validate]: /docs/commands/job/validate.html [update]: /docs/job-specification/update.html +[tls-guide]: /guides/security/securing-nomad.html +[tls-vault-guide]: /guides/security/vault-pki-integration.html