Merge pull request #10614 from jmillikin-stripe/cert-allowed-san-flags

etcdmain, pkg: Support peer and client TLS auth based on SAN fields.

Documentation/op-guide/authentication.md

@@ -171,6 +171,6 @@ Otherwise, all `etcdctl` commands remain the same. Users and roles can still be
 ## Using TLS Common Name
 As of version v3.2 if an etcd server is launched with the option `--client-cert-auth=true`, the field of Common Name (CN) in the client's TLS cert will be used as an etcd user. In this case, the common name authenticates the user and the client does not need a password. Note that if both of 1. `--client-cert-auth=true` is passed and CN is provided by the client, and 2. username and password are provided by the client, the username and password based authentication is prioritized. Note that this feature cannot be used with gRPC-proxy and gRPC-gateway. This is because gRPC-proxy terminates TLS from its client so all the clients share a cert of the proxy. gRPC-gateway uses a TLS connection internally for transforming HTTP request to gRPC request so it shares the same limitation. Therefore the clients cannot provide their CN to the server correctly. gRPC-proxy will cause an error and stop if a given cert has non empty CN. gRPC-proxy returns an error which indicates that the client has an non empty CN in its cert.
 
-As of version v3.3 if an etcd server is launched with the option `--peer-cert-allowed-cn` filtering of CN inter-peer connections is enabled.  Nodes can only join the etcd cluster if their CN match the allowed one.
+As of version v3.3 if an etcd server is launched with the option `--peer-cert-allowed-cn` or `--peer-cert-allowed-hostname` filtering of inter-peer connections is enabled.  Nodes can only join the etcd cluster if their TLS certificate identity match the allowed one.
 See [etcd security page](https://github.com/etcd-io/etcd/blob/master/Documentation/op-guide/security.md) for more details.
 

Documentation/op-guide/configuration.md

@@ -270,6 +270,11 @@ The security flags help to [build a secure etcd cluster][security].
 + default: ""
 + env variable: ETCD_CLIENT_CRL_FILE
 
+### --client-cert-allowed-hostname
++ Allowed Allowed TLS name for client cert authentication.
++ default: ""
++ env variable: ETCD_CLIENT_CERT_ALLOWED_HOSTNAME
+
 ### --trusted-ca-file
 + Path to the client server TLS trusted CA cert file.
 + default: ""
@@ -320,9 +325,14 @@ The security flags help to [build a secure etcd cluster][security].
 
 ### --peer-cert-allowed-cn
 + Allowed CommonName for inter peer authentication.
-+ default: none
++ default: ""
 + env variable: ETCD_PEER_CERT_ALLOWED_CN
 
+### --peer-cert-allowed-hostname
++ Allowed TLS certificate name for inter peer authentication.
++ default: ""
++ env variable: ETCD_PEER_CERT_ALLOWED_HOSTNAME
+
 ### --cipher-suites
 + Comma-separated list of supported TLS cipher suites between server/client and peers.
 + default: ""

etcdmain/config.go

@@ -200,6 +200,7 @@ func newConfig() *config {
 	fs.StringVar(&cfg.ec.ClientTLSInfo.KeyFile, "key-file", "", "Path to the client server TLS key file.")
 	fs.BoolVar(&cfg.ec.ClientTLSInfo.ClientCertAuth, "client-cert-auth", false, "Enable client cert authentication.")
 	fs.StringVar(&cfg.ec.ClientTLSInfo.CRLFile, "client-crl-file", "", "Path to the client certificate revocation list file.")
+	fs.StringVar(&cfg.ec.ClientTLSInfo.AllowedHostname, "client-cert-allowed-hostname", "", "Allowed TLS hostname for client cert authentication.")
 	fs.StringVar(&cfg.ec.ClientTLSInfo.TrustedCAFile, "trusted-ca-file", "", "Path to the client server TLS trusted CA cert file.")
 	fs.BoolVar(&cfg.ec.ClientAutoTLS, "auto-tls", false, "Client TLS using generated certificates")
 	fs.StringVar(&cfg.ec.PeerTLSInfo.CertFile, "peer-cert-file", "", "Path to the peer server TLS cert file.")
@@ -209,6 +210,7 @@ func newConfig() *config {
 	fs.BoolVar(&cfg.ec.PeerAutoTLS, "peer-auto-tls", false, "Peer TLS using generated certificates")
 	fs.StringVar(&cfg.ec.PeerTLSInfo.CRLFile, "peer-crl-file", "", "Path to the peer certificate revocation list file.")
 	fs.StringVar(&cfg.ec.PeerTLSInfo.AllowedCN, "peer-cert-allowed-cn", "", "Allowed CN for inter peer authentication.")
+	fs.StringVar(&cfg.ec.PeerTLSInfo.AllowedHostname, "peer-cert-allowed-hostname", "", "Allowed TLS hostname for inter peer authentication.")
 	fs.Var(flags.NewStringsValue(""), "cipher-suites", "Comma-separated list of supported TLS cipher suites between client/server and peers (empty will be auto-populated by Go).")
 
 	fs.Var(

etcdmain/help.go

@@ -128,6 +128,8 @@ Security:
     Enable client cert authentication.
   --client-crl-file ''
     Path to the client certificate revocation list file.
+  --client-cert-allowed-hostname ''
+    Allowed TLS hostname for client cert authentication.
   --trusted-ca-file ''
     Path to the client server TLS trusted CA cert file.
   --auto-tls 'false'
@@ -142,6 +144,8 @@ Security:
     Path to the peer server TLS trusted CA file.
   --peer-cert-allowed-cn ''
     Required CN for client certs connecting to the peer endpoint.
+  --peer-cert-allowed-hostname ''
+    Allowed TLS hostname for inter peer authentication.
   --peer-auto-tls 'false'
     Peer TLS using self-generated certificates if --peer-key-file and --peer-cert-file are not provided.
   --peer-crl-file ''

pkg/transport/listener.go

@@ -88,6 +88,10 @@ type TLSInfo struct {
 	// AllowedCN is a CN which must be provided by a client.
 	AllowedCN string
 
+	// AllowedHostname is an IP address or hostname that must match the TLS
+	// certificate provided by a client.
+	AllowedHostname string
+
 	// Logger logs TLS errors.
 	// If nil, all logs are discarded.
 	Logger *zap.Logger
@@ -256,16 +260,32 @@ func (info TLSInfo) baseConfig() (*tls.Config, error) {
 		cfg.CipherSuites = info.CipherSuites
 	}
 
+	// Client certificates may be verified by either an exact match on the CN,
+	// or a more general check of the CN and SANs.
+	var verifyCertificate func(*x509.Certificate) bool
 	if info.AllowedCN != "" {
+		if info.AllowedHostname != "" {
+			return nil, fmt.Errorf("AllowedCN and AllowedHostname are mutually exclusive (cn=%q, hostname=%q)", info.AllowedCN, info.AllowedHostname)
+		}
+		verifyCertificate = func(cert *x509.Certificate) bool {
+			return info.AllowedCN == cert.Subject.CommonName
+		}
+	}
+	if info.AllowedHostname != "" {
+		verifyCertificate = func(cert *x509.Certificate) bool {
+			return cert.VerifyHostname(info.AllowedHostname) == nil
+		}
+	}
+	if verifyCertificate != nil {
 		cfg.VerifyPeerCertificate = func(rawCerts [][]byte, verifiedChains [][]*x509.Certificate) error {
 			for _, chains := range verifiedChains {
 				if len(chains) != 0 {
-					if info.AllowedCN == chains[0].Subject.CommonName {
+					if verifyCertificate(chains[0]) {
 						return nil
 					}
 				}
 			}
-			return errors.New("CommonName authentication failed")
+			return errors.New("client certificate authentication failed")
 		}
 	}
 

tests/e2e/etcd_config_test.go

@@ -191,6 +191,83 @@ func TestEtcdPeerCNAuth(t *testing.T) {
 	}
 }
 
+// TestEtcdPeerNameAuth checks that the inter peer auth based on cert name validation is working correctly.
+func TestEtcdPeerNameAuth(t *testing.T) {
+	peers, tmpdirs := make([]string, 3), make([]string, 3)
+	for i := range peers {
+		peers[i] = fmt.Sprintf("e%d=https://127.0.0.1:%d", i, etcdProcessBasePort+i)
+		d, err := ioutil.TempDir("", fmt.Sprintf("e%d.etcd", i))
+		if err != nil {
+			t.Fatal(err)
+		}
+		tmpdirs[i] = d
+	}
+	ic := strings.Join(peers, ",")
+
+	procs := make([]*expect.ExpectProcess, len(peers))
+	defer func() {
+		for i := range procs {
+			if procs[i] != nil {
+				procs[i].Stop()
+			}
+			os.RemoveAll(tmpdirs[i])
+		}
+	}()
+
+	// node 0 and 1 have a cert with the correct certificate name, node 2 doesn't
+	for i := range procs {
+		commonArgs := []string{
+			binDir + "/etcd",
+			"--name", fmt.Sprintf("e%d", i),
+			"--listen-client-urls", "http://0.0.0.0:0",
+			"--data-dir", tmpdirs[i],
+			"--advertise-client-urls", "http://0.0.0.0:0",
+			"--listen-peer-urls", fmt.Sprintf("https://127.0.0.1:%d,https://127.0.0.1:%d", etcdProcessBasePort+i, etcdProcessBasePort+len(peers)+i),
+			"--initial-advertise-peer-urls", fmt.Sprintf("https://127.0.0.1:%d", etcdProcessBasePort+i),
+			"--initial-cluster", ic,
+		}
+
+		var args []string
+		if i <= 1 {
+			args = []string{
+				"--peer-cert-file", certPath,
+				"--peer-key-file", privateKeyPath,
+				"--peer-trusted-ca-file", caPath,
+				"--peer-client-cert-auth",
+				"--peer-cert-allowed-hostname", "localhost",
+			}
+		} else {
+			args = []string{
+				"--peer-cert-file", certPath2,
+				"--peer-key-file", privateKeyPath2,
+				"--peer-trusted-ca-file", caPath,
+				"--peer-client-cert-auth",
+				"--peer-cert-allowed-hostname", "example2.com",
+			}
+		}
+
+		commonArgs = append(commonArgs, args...)
+
+		p, err := spawnCmd(commonArgs)
+		if err != nil {
+			t.Fatal(err)
+		}
+		procs[i] = p
+	}
+
+	for i, p := range procs {
+		var expect []string
+		if i <= 1 {
+			expect = etcdServerReadyLines
+		} else {
+			expect = []string{"client certificate authentication failed"}
+		}
+		if err := waitReadyExpectProc(p, expect); err != nil {
+			t.Fatal(err)
+		}
+	}
+}
+
 func TestGrpcproxyAndCommonName(t *testing.T) {
 	argsWithNonEmptyCN := []string{
 		binDir + "/etcd",