Merge pull request #11195 from andyliuliming/release-3.2-cherry

etcdserver: cherry-pick skip client san verification option for 3.2 version.

Documentation/op-guide/configuration.md

@@ -253,6 +253,11 @@ The security flags help to [build a secure etcd cluster][security].
 + default: false
 + env variable: ETCD_PEER_AUTO_TLS
 
+### --experimental-peer-skip-client-san-verification 
++ Skip verification of SAN field in client certificate for peer connections.
++ default: false
++ env variable: ETCD_EXPERIMENTAL_PEER_SKIP_CLIENT_SAN_VERIFICATION
+
 ## Logging flags
 
 ### --debug

etcdmain/config.go

@@ -193,6 +193,8 @@ func newConfig() *config {
 
 	fs.Var(flags.NewStringsValueV2(""), "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.PeerTLSInfo.SkipClientSANVerify, "experimental-peer-skip-client-san-verification", false, "Skip verification of SAN field in client certificate for peer connections.")
+
 	// logging
 	fs.BoolVar(&cfg.Debug, "debug", false, "Enable debug-level logging for etcd.")
 	fs.StringVar(&cfg.LogPkgLevels, "log-package-levels", "", "Specify a particular log level for each etcd package (eg: 'etcdmain=CRITICAL,etcdserver=DEBUG').")

etcdmain/help.go

@@ -152,6 +152,8 @@ security flags:
 		peer TLS using self-generated certificates if --peer-key-file and --peer-cert-file are not provided.
 	--cipher-suites ''
 		comma-separated list of supported TLS cipher suites between client/server and peers (empty will be auto-populated by Go).
+	--experimental-peer-skip-client-san-verification 'false'
+		Skip verification of SAN field in client certificate for peer connections.
 
 logging flags
 

pkg/transport/listener.go

@@ -52,7 +52,10 @@ func wrapTLS(addr, scheme string, tlsinfo *TLSInfo, l net.Listener) (net.Listene
 	if scheme != "https" && scheme != "unixs" {
 		return l, nil
 	}
-	return newTLSListener(l, tlsinfo)
+	if tlsinfo != nil && tlsinfo.SkipClientSANVerify {
+		return NewTLSListener(l, tlsinfo)
+	}
+	return newTLSListener(l, tlsinfo, checkSAN)
 }
 
 type TLSInfo struct {
@@ -62,6 +65,8 @@ type TLSInfo struct {
 	TrustedCAFile  string
 	ClientCertAuth bool
 
+	SkipClientSANVerify bool
+
 	// ServerName ensures the cert matches the given host in case of discovery / virtual hosting
 	ServerName string
 
@@ -89,7 +94,7 @@ func (info TLSInfo) Empty() bool {
 	return info.CertFile == "" && info.KeyFile == ""
 }
 
-func SelfCert(dirpath string, hosts []string) (info TLSInfo, err error) {
+func SelfCert(dirpath string, hosts []string, additionalUsages ...x509.ExtKeyUsage) (info TLSInfo, err error) {
 	if err = os.MkdirAll(dirpath, 0700); err != nil {
 		return
 	}
@@ -118,7 +123,7 @@ func SelfCert(dirpath string, hosts []string) (info TLSInfo, err error) {
 		NotAfter:     time.Now().Add(365 * (24 * time.Hour)),
 
 		KeyUsage:              x509.KeyUsageKeyEncipherment | x509.KeyUsageDigitalSignature,
-		ExtKeyUsage:           []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth},
+		ExtKeyUsage:           append([]x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}, additionalUsages...),
 		BasicConstraintsValid: true,
 	}
 

pkg/transport/listener_test.go

@@ -16,20 +16,26 @@ package transport
 
 import (
 	"crypto/tls"
+	"crypto/x509"
 	"errors"
 	"io/ioutil"
+	"net"
 	"net/http"
 	"os"
 	"testing"
 	"time"
 )
 
-func createSelfCert() (*TLSInfo, func(), error) {
+func createSelfCert(hosts ...string) (*TLSInfo, func(), error) {
+	return createSelfCertEx("127.0.0.1")
+}
+
+func createSelfCertEx(host string, additionalUsages ...x509.ExtKeyUsage) (*TLSInfo, func(), error) {
 	d, terr := ioutil.TempDir("", "etcd-test-tls-")
 	if terr != nil {
 		return nil, nil, terr
 	}
-	info, err := SelfCert(d, []string{"127.0.0.1"})
+	info, err := SelfCert(d, []string{host + ":0"}, additionalUsages...)
 	if err != nil {
 		return nil, nil, err
 	}
@@ -70,7 +76,103 @@ func testNewListenerTLSInfoAccept(t *testing.T, tlsInfo TLSInfo) {
 	}
 	defer conn.Close()
 	if _, ok := conn.(*tls.Conn); !ok {
-		t.Errorf("failed to accept *tls.Conn")
+		t.Error("failed to accept *tls.Conn")
+	}
+}
+
+func TestNewListenerTLSInfoSkipClientSANVerify(t *testing.T) {
+	tests := []struct {
+		skipClientSANVerify bool
+		goodClientHost      bool
+		acceptExpected      bool
+	}{
+		{false, true, true},
+		{false, false, false},
+		{true, true, true},
+		{true, false, true},
+	}
+	for _, test := range tests {
+		testNewListenerTLSInfoClientCheck(t, test.skipClientSANVerify, test.goodClientHost, test.acceptExpected)
+	}
+}
+
+func testNewListenerTLSInfoClientCheck(t *testing.T, skipClientSANVerify, goodClientHost, acceptExpected bool) {
+	tlsInfo, del, err := createSelfCert()
+	if err != nil {
+		t.Fatalf("unable to create cert: %v", err)
+	}
+	defer del()
+
+	host := "127.0.0.222"
+	if goodClientHost {
+		host = "127.0.0.1"
+	}
+	clientTLSInfo, del2, err := createSelfCertEx(host, x509.ExtKeyUsageClientAuth)
+	if err != nil {
+		t.Fatalf("unable to create cert: %v", err)
+	}
+	defer del2()
+
+	tlsInfo.SkipClientSANVerify = skipClientSANVerify
+	tlsInfo.CAFile = clientTLSInfo.CertFile
+
+	rootCAs := x509.NewCertPool()
+	loaded, err := ioutil.ReadFile(tlsInfo.CertFile)
+	if err != nil {
+		t.Fatalf("unexpected missing certfile: %v", err)
+	}
+	rootCAs.AppendCertsFromPEM(loaded)
+
+	clientCert, err := tls.LoadX509KeyPair(clientTLSInfo.CertFile, clientTLSInfo.KeyFile)
+	if err != nil {
+		t.Fatalf("unable to create peer cert: %v", err)
+	}
+
+	tlsConfig := &tls.Config{}
+	tlsConfig.InsecureSkipVerify = false
+	tlsConfig.Certificates = []tls.Certificate{clientCert}
+	tlsConfig.RootCAs = rootCAs
+
+	ln, err := NewListener("127.0.0.1:0", "https", tlsInfo)
+	if err != nil {
+		t.Fatalf("unexpected NewListener error: %v", err)
+	}
+	defer ln.Close()
+
+	tr := &http.Transport{TLSClientConfig: tlsConfig}
+	cli := &http.Client{Transport: tr}
+	chClientErr := make(chan error)
+	go func() {
+		_, err := cli.Get("https://" + ln.Addr().String())
+		chClientErr <- err
+	}()
+
+	chAcceptErr := make(chan error)
+	chAcceptConn := make(chan net.Conn)
+	go func() {
+		conn, err := ln.Accept()
+		if err != nil {
+			chAcceptErr <- err
+		} else {
+			chAcceptConn <- conn
+		}
+	}()
+
+	select {
+	case <-chClientErr:
+		if acceptExpected {
+			t.Errorf("accepted for good client address: skipClientSANVerify=%v, goodClientHost=%v", skipClientSANVerify, goodClientHost)
+		}
+	case acceptErr := <-chAcceptErr:
+		t.Fatalf("unexpected Accept error: %v", acceptErr)
+	case conn := <-chAcceptConn:
+		defer conn.Close()
+		if _, ok := conn.(*tls.Conn); !ok {
+			t.Errorf("failed to accept *tls.Conn")
+		}
+		if !acceptExpected {
+			t.Errorf("accepted for bad client address: skipClientSANVerify=%v, goodClientHost=%v", skipClientSANVerify, goodClientHost)
+		}
 	}
 }
 

pkg/transport/listener_tls.go

@@ -32,9 +32,18 @@ type tlsListener struct {
 	donec            chan struct{}
 	err              error
 	handshakeFailure func(*tls.Conn, error)
+	check            tlsCheckFunc
 }
 
-func newTLSListener(l net.Listener, tlsinfo *TLSInfo) (net.Listener, error) {
+type tlsCheckFunc func(context.Context, *tls.Conn) error
+
+// NewTLSListener handshakes TLS connections and performs optional CRL checking.
+func NewTLSListener(l net.Listener, tlsinfo *TLSInfo) (net.Listener, error) {
+	check := func(context.Context, *tls.Conn) error { return nil }
+	return newTLSListener(l, tlsinfo, check)
+}
+
+func newTLSListener(l net.Listener, tlsinfo *TLSInfo, check tlsCheckFunc) (net.Listener, error) {
 	if tlsinfo == nil || tlsinfo.Empty() {
 		l.Close()
 		return nil, fmt.Errorf("cannot listen on TLS for %s: KeyFile and CertFile are not presented", l.Addr().String())
@@ -53,6 +62,7 @@ func newTLSListener(l net.Listener, tlsinfo *TLSInfo) (net.Listener, error) {
 		connc:            make(chan net.Conn),
 		donec:            make(chan struct{}),
 		handshakeFailure: hf,
+		check:            check,
 	}
 	go tlsl.acceptLoop()
 	return tlsl, nil
@@ -116,14 +126,9 @@ func (l *tlsListener) acceptLoop() {
 				return
 			}
 
-			st := tlsConn.ConnectionState()
-			if len(st.PeerCertificates) > 0 {
-				cert := st.PeerCertificates[0]
-				addr := tlsConn.RemoteAddr().String()
-				if cerr := checkCert(ctx, cert, addr); cerr != nil {
-					l.handshakeFailure(tlsConn, cerr)
-					return
-				}
+			if err := l.check(ctx, tlsConn); err != nil {
+				l.handshakeFailure(tlsConn, err)
+				return
 			}
 			select {
 			case l.connc <- tlsConn:
@@ -134,11 +139,20 @@ func (l *tlsListener) acceptLoop() {
 	}
 }
 
-func checkCert(ctx context.Context, cert *x509.Certificate, remoteAddr string) error {
-	h, _, herr := net.SplitHostPort(remoteAddr)
+func checkSAN(ctx context.Context, tlsConn *tls.Conn) error {
+	st := tlsConn.ConnectionState()
+	if certs := st.PeerCertificates; len(certs) > 0 {
+		addr := tlsConn.RemoteAddr().String()
+		return checkCertSAN(ctx, certs[0], addr)
+	}
+	return nil
+}
+
+func checkCertSAN(ctx context.Context, cert *x509.Certificate, remoteAddr string) error {
 	if len(cert.IPAddresses) == 0 && len(cert.DNSNames) == 0 {
 		return nil
 	}
+	h, _, herr := net.SplitHostPort(remoteAddr)
 	if herr != nil {
 		return herr
 	}