Merge pull request #7023 from hashicorp/b-tls-validation

Validate role and region for mTLS

Author: schmichael

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

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)
+	}
+}

website/source/guides/upgrade/upgrade-specific.html.md

@@ -17,6 +17,25 @@ 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.<region>.nomad`, and that the common name or SAN of all Nomad server
+node certs is `server.<region>.nomad`.
+
 ### Connection Limits Added
 
 Nomad 0.10.3 introduces the [limits][limits] agent configuration parameters for
@@ -419,3 +438,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