Merge pull request etcd-io#18160 from lhy1024/pick-cn

[3.5] Support multiple values for allowed client and peer TLS identities

client/pkg/transport/listener.go

@@ -180,12 +180,23 @@ type TLSInfo struct {
 	parseFunc func([]byte, []byte) (tls.Certificate, error)
 
 	// AllowedCN is a CN which must be provided by a client.
+	//
+	// Deprecated: use AllowedCNs instead.
 	AllowedCN string
 
 	// AllowedHostname is an IP address or hostname that must match the TLS
 	// certificate provided by a client.
+	//
+	// Deprecated: use AllowedHostnames instead.
 	AllowedHostname string
 
+	// AllowedCNs is a list of acceptable CNs which must be provided by a client.
+	AllowedCNs []string
+
+	// AllowedHostnames is a list of acceptable IP addresses or hostnames that must match the
+	// TLS certificate provided by a client.
+	AllowedHostnames []string
+
 	// Logger logs TLS errors.
 	// If nil, all logs are discarded.
 	Logger *zap.Logger
@@ -407,19 +418,52 @@ func (info TLSInfo) baseConfig() (*tls.Config, error) {
 	// 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 != "" && len(info.AllowedCNs) > 0 {
+		return nil, fmt.Errorf("AllowedCN and AllowedCNs are mutually exclusive (cn=%q, cns=%q)", info.AllowedCN, info.AllowedCNs)
+	}
+	if info.AllowedHostname != "" && len(info.AllowedHostnames) > 0 {
+		return nil, fmt.Errorf("AllowedHostname and AllowedHostnames are mutually exclusive (hostname=%q, hostnames=%q)", info.AllowedHostname, info.AllowedHostnames)
+	}
+	if info.AllowedCN != "" && info.AllowedHostname != "" {
+		return nil, fmt.Errorf("AllowedCN and AllowedHostname are mutually exclusive (cn=%q, hostname=%q)", info.AllowedCN, info.AllowedHostname)
+	}
+	if len(info.AllowedCNs) > 0 && len(info.AllowedHostnames) > 0 {
+		return nil, fmt.Errorf("AllowedCNs and AllowedHostnames are mutually exclusive (cns=%q, hostnames=%q)", info.AllowedCNs, info.AllowedHostnames)
+	}
+
 	if info.AllowedCN != "" {
-		if info.AllowedHostname != "" {
-			return nil, fmt.Errorf("AllowedCN and AllowedHostname are mutually exclusive (cn=%q, hostname=%q)", info.AllowedCN, info.AllowedHostname)
-		}
+		info.Logger.Warn("AllowedCN is deprecated, use AllowedCNs instead")
 		verifyCertificate = func(cert *x509.Certificate) bool {
 			return info.AllowedCN == cert.Subject.CommonName
 		}
 	}
 	if info.AllowedHostname != "" {
+		info.Logger.Warn("AllowedHostname is deprecated, use AllowedHostnames instead")
 		verifyCertificate = func(cert *x509.Certificate) bool {
 			return cert.VerifyHostname(info.AllowedHostname) == nil
 		}
 	}
+	if len(info.AllowedCNs) > 0 {
+		verifyCertificate = func(cert *x509.Certificate) bool {
+			for _, allowedCN := range info.AllowedCNs {
+				if allowedCN == cert.Subject.CommonName {
+					return true
+				}
+			}
+			return false
+		}
+	}
+	if len(info.AllowedHostnames) > 0 {
+		verifyCertificate = func(cert *x509.Certificate) bool {
+			for _, allowedHostname := range info.AllowedHostnames {
+				if cert.VerifyHostname(allowedHostname) == nil {
+					return true
+				}
+			}
+			return false
+		}
+	}
 	if verifyCertificate != nil {
 		cfg.VerifyPeerCertificate = func(rawCerts [][]byte, verifiedChains [][]*x509.Certificate) error {
 			for _, chains := range verifiedChains {

server/embed/config.go

@@ -443,15 +443,15 @@ type configJSON struct {
 }
 
 type securityConfig struct {
-	CertFile        string `json:"cert-file"`
-	KeyFile         string `json:"key-file"`
-	ClientCertFile  string `json:"client-cert-file"`
-	ClientKeyFile   string `json:"client-key-file"`
-	CertAuth        bool   `json:"client-cert-auth"`
-	TrustedCAFile   string `json:"trusted-ca-file"`
-	AutoTLS         bool   `json:"auto-tls"`
-	AllowedCN       string `json:"allowed-cn"`
-	AllowedHostname string `json:"allowed-hostname"`
+	CertFile         string   `json:"cert-file"`
+	KeyFile          string   `json:"key-file"`
+	ClientCertFile   string   `json:"client-cert-file"`
+	ClientKeyFile    string   `json:"client-key-file"`
+	CertAuth         bool     `json:"client-cert-auth"`
+	TrustedCAFile    string   `json:"trusted-ca-file"`
+	AutoTLS          bool     `json:"auto-tls"`
+	AllowedCNs       []string `json:"allowed-cn"`
+	AllowedHostnames []string `json:"allowed-hostname"`
 }
 
 // NewConfig creates a new Config populated with default values.
@@ -631,8 +631,8 @@ func (cfg *configYAML) configFromFile(path string) error {
 		tls.ClientKeyFile = ysc.ClientKeyFile
 		tls.ClientCertAuth = ysc.CertAuth
 		tls.TrustedCAFile = ysc.TrustedCAFile
-		tls.AllowedCN = ysc.AllowedCN
-		tls.AllowedHostname = ysc.AllowedHostname
+		tls.AllowedCNs = ysc.AllowedCNs
+		tls.AllowedHostnames = ysc.AllowedHostnames
 	}
 	copySecurityDetails(&cfg.ClientTLSInfo, &cfg.ClientSecurityJSON)
 	copySecurityDetails(&cfg.PeerTLSInfo, &cfg.PeerSecurityJSON)

server/embed/config_test.go

@@ -41,7 +41,7 @@ func notFoundErr(service, domain string) error {
 func TestConfigFileOtherFields(t *testing.T) {
 	ctls := securityConfig{TrustedCAFile: "cca", CertFile: "ccert", KeyFile: "ckey"}
 	// Note AllowedCN and AllowedHostname are mutually exclusive, this test is just to verify the fields can be correctly marshalled & unmarshalled.
-	ptls := securityConfig{TrustedCAFile: "pca", CertFile: "pcert", KeyFile: "pkey", AllowedCN: "etcd", AllowedHostname: "whatever.example.com"}
+	ptls := securityConfig{TrustedCAFile: "pca", CertFile: "pcert", KeyFile: "pkey", AllowedCNs: []string{"etcd"}, AllowedHostnames: []string{"whatever.example.com"}}
 	yc := struct {
 		ClientSecurityCfgFile securityConfig       `json:"client-transport-security"`
 		PeerSecurityCfgFile   securityConfig       `json:"peer-transport-security"`
@@ -160,8 +160,20 @@ func (s *securityConfig) equals(t *transport.TLSInfo) bool {
 		s.ClientCertFile == t.ClientCertFile &&
 		s.ClientKeyFile == t.ClientKeyFile &&
 		s.KeyFile == t.KeyFile &&
-		s.AllowedCN == t.AllowedCN &&
-		s.AllowedHostname == t.AllowedHostname
+		compareSlices(s.AllowedCNs, t.AllowedCNs) &&
+		compareSlices(s.AllowedHostnames, t.AllowedHostnames)
+}
+
+func compareSlices(slice1, slice2 []string) bool {
+	if len(slice1) != len(slice2) {
+		return false
+	}
+	for i, v := range slice1 {
+		if v != slice2[i] {
+			return false
+		}
+	}
+	return true
 }
 
 func mustCreateCfgFile(t *testing.T, b []byte) *os.File {

server/etcdmain/config.go

@@ -226,7 +226,7 @@ func newConfig() *config {
 	fs.StringVar(&cfg.ec.ClientTLSInfo.ClientKeyFile, "client-key-file", "", "Path to an explicit peer client TLS key file otherwise key file will be used when client auth is required.")
 	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.Var(flags.NewStringsValue(""), "client-cert-allowed-hostname", "Comma-separated list of allowed SAN hostnames 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.")
@@ -238,8 +238,8 @@ func newConfig() *config {
 	fs.BoolVar(&cfg.ec.PeerAutoTLS, "peer-auto-tls", false, "Peer TLS using generated certificates")
 	fs.UintVar(&cfg.ec.SelfSignedCertValidity, "self-signed-cert-validity", 1, "The validity period of the client and peer certificates, unit is year")
 	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(""), "peer-cert-allowed-cn", "Comma-separated list of allowed CNs for inter-peer TLS authentication.")
+	fs.Var(flags.NewStringsValue(""), "peer-cert-allowed-hostname", "Comma-separated list of allowed SAN hostnames for inter-peer TLS 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.BoolVar(&cfg.ec.PeerTLSInfo.SkipClientSANVerify, "experimental-peer-skip-client-san-verification", false, "Skip verification of SAN field in client certificate for peer connections.")
 	fs.StringVar(&cfg.ec.TlsMinVersion, "tls-min-version", string(tlsutil.TLSVersion12), "Minimum TLS version supported by etcd. Possible values: TLS1.2, TLS1.3.")
@@ -409,6 +409,10 @@ func (cfg *config) configFromCmdLine() error {
 	cfg.ec.CORS = flags.UniqueURLsMapFromFlag(cfg.cf.flagSet, "cors")
 	cfg.ec.HostWhitelist = flags.UniqueStringsMapFromFlag(cfg.cf.flagSet, "host-whitelist")
 
+	cfg.ec.ClientTLSInfo.AllowedHostnames = flags.StringsFromFlag(cfg.cf.flagSet, "client-cert-allowed-hostname")
+	cfg.ec.PeerTLSInfo.AllowedCNs = flags.StringsFromFlag(cfg.cf.flagSet, "peer-cert-allowed-cn")
+	cfg.ec.PeerTLSInfo.AllowedHostnames = flags.StringsFromFlag(cfg.cf.flagSet, "peer-cert-allowed-hostname")
+
 	cfg.ec.CipherSuites = flags.StringsFromFlag(cfg.cf.flagSet, "cipher-suites")
 
 	cfg.ec.MaxConcurrentStreams = flags.Uint32FromFlag(cfg.cf.flagSet, "max-concurrent-streams")

server/etcdmain/help.go

@@ -159,7 +159,7 @@ Security:
   --client-crl-file ''
     Path to the client certificate revocation list file.
   --client-cert-allowed-hostname ''
-    Allowed TLS hostname for client cert authentication.
+    Comma-separated list of SAN hostnames for client cert authentication.
   --trusted-ca-file ''
     Path to the client server TLS trusted CA cert file.
   --auto-tls 'false'
@@ -173,9 +173,9 @@ Security:
   --peer-trusted-ca-file ''
     Path to the peer server TLS trusted CA file.
   --peer-cert-allowed-cn ''
-    Required CN for client certs connecting to the peer endpoint.
+    Comma-separated list of allowed CNs for inter-peer TLS authentication.
   --peer-cert-allowed-hostname ''
-    Allowed TLS hostname for inter peer authentication.
+    Comma-separated list of allowed SAN hostnames for inter-peer TLS authentication.
   --peer-auto-tls 'false'
     Peer TLS using self-generated certificates if --peer-key-file and --peer-cert-file are not provided.
   --peer-client-cert-file ''

tests/e2e/etcd_config_test.go

@@ -205,6 +205,96 @@ func TestEtcdPeerCNAuth(t *testing.T) {
 	}
 }
 
+// TestEtcdPeerMultiCNAuth checks that the inter peer auth based on CN of cert is working correctly
+// when there are multiple allowed values for the CN.
+func TestEtcdPeerMultiCNAuth(t *testing.T) {
+	e2e.SkipInShortMode(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, e2e.EtcdProcessBasePort+i)
+		tmpdirs[i] = t.TempDir()
+	}
+	ic := strings.Join(peers, ",")
+	procs := make([]*expect.ExpectProcess, len(peers))
+	defer func() {
+		for i := range procs {
+			if procs[i] != nil {
+				procs[i].Stop()
+				procs[i].Close()
+			}
+		}
+	}()
+
+	// all nodes have unique certs with different CNs
+	// node 0 and 1 have a cert with one of the correct CNs, node 2 doesn't
+	for i := range procs {
+		commonArgs := []string{
+			e2e.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", e2e.EtcdProcessBasePort+i, e2e.EtcdProcessBasePort+len(peers)+i),
+			"--initial-advertise-peer-urls", fmt.Sprintf("https://127.0.0.1:%d", e2e.EtcdProcessBasePort+i),
+			"--initial-cluster", ic,
+		}
+
+		var args []string
+		switch i {
+		case 0:
+			args = []string{
+				"--peer-cert-file", e2e.CertPath, // server.crt has CN "example.com".
+				"--peer-key-file", e2e.PrivateKeyPath,
+				"--peer-client-cert-file", e2e.CertPath,
+				"--peer-client-key-file", e2e.PrivateKeyPath,
+				"--peer-trusted-ca-file", e2e.CaPath,
+				"--peer-client-cert-auth",
+				"--peer-cert-allowed-cn", "example.com,example2.com",
+			}
+		case 1:
+			args = []string{
+				"--peer-cert-file", e2e.CertPath2, // server2.crt has CN "example2.com".
+				"--peer-key-file", e2e.PrivateKeyPath2,
+				"--peer-client-cert-file", e2e.CertPath2,
+				"--peer-client-key-file", e2e.PrivateKeyPath2,
+				"--peer-trusted-ca-file", e2e.CaPath,
+				"--peer-client-cert-auth",
+				"--peer-cert-allowed-cn", "example.com,example2.com",
+			}
+		default:
+			args = []string{
+				"--peer-cert-file", e2e.CertPath3, // server3.crt has CN "ca".
+				"--peer-key-file", e2e.PrivateKeyPath3,
+				"--peer-client-cert-file", e2e.CertPath3,
+				"--peer-client-key-file", e2e.PrivateKeyPath3,
+				"--peer-trusted-ca-file", e2e.CaPath,
+				"--peer-client-cert-auth",
+				"--peer-cert-allowed-cn", "example.com,example2.com",
+			}
+		}
+
+		commonArgs = append(commonArgs, args...)
+		p, err := e2e.SpawnCmd(commonArgs, nil)
+		if err != nil {
+			t.Fatal(err)
+		}
+		procs[i] = p
+	}
+
+	for i, p := range procs {
+		var expect []string
+		if i <= 1 {
+			expect = e2e.EtcdServerReadyLines
+		} else {
+			expect = []string{"remote error: tls: bad certificate"}
+		}
+		if err := e2e.WaitReadyExpectProc(p, expect); err != nil {
+			t.Fatal(err)
+		}
+	}
+}
+
 // TestEtcdPeerNameAuth checks that the inter peer auth based on cert name validation is working correctly.
 func TestEtcdPeerNameAuth(t *testing.T) {
 	e2e.SkipInShortMode(t)