Do exact cert matching for leaf certs (#494)

Given that NSX manager may present TLS cert with mismatched subject,
this change addresses TLS validation as follows:
- If CA cert, native cert verification is done
- if leaf cert or a chain of certs (the first one in the chain will be
  a leaf cert), exact cert matching will be done. This will bypass any
  CN / SAN / expiration / revocation check.

Above logic also aligns with VC certificate validation guideline.

Signed-off-by: Shawn Wang <[email protected]>

pkg/nsx/cluster.go

@@ -6,7 +6,6 @@ package nsx
 import (
 	"context"
 	"crypto/tls"
-	"crypto/x509"
 	"errors"
 	"fmt"
 	"net"
@@ -165,15 +164,10 @@ func (cluster *Cluster) createTransport(idle time.Duration) *Transport {
 					return nil, err
 				}
 
-				certPool := x509.NewCertPool()
-				certPool.AppendCertsFromPEM(caCert)
-				// #nosec G402: ignore insecure options
-				config = &tls.Config{
-					RootCAs: certPool,
-				}
-				// Bypass CN / SAN verification by setting tls config ServerName to the one in cert
-				if cn, err := util.GetCommonNameFromLeafCert(caCert); err != nil {
-					config.ServerName = cn
+				config, err = util.GetTLSConfigForCert(caCert)
+				if err != nil {
+					log.Error(err, "create transport", "get tls config from cert", cafile)
+					return nil, err
 				}
 			} else {
 				thumbprint := cluster.getThumbprint(addr)

pkg/nsx/util/utils.go

@@ -7,6 +7,7 @@ import (
 	"bytes"
 	"crypto/sha1" // #nosec G505: not used for security purposes
 	"crypto/sha256"
+	"crypto/tls"
 	"crypto/x509"
 	"encoding/hex"
 	"encoding/json"
@@ -419,33 +420,61 @@ func VerifyNsxCertWithThumbprint(der []byte, thumbprint string) error {
 	return err
 }
 
-// GetCommonNameFromLeafCert returns the common name of the first (leaf) cert of
-// provided pemCerts. If CA cert is passed, empty CN will be returned.
+// GetTLSConfigForCert returns TLS config based on given pemCerts.
+// If CA cert is passed, TLS config will do native cert check for connection.
+// Otherwise, exact byte-to-byte check will be performed.
 // Error is returned if pem invalid or not a certificate.
-func GetCommonNameFromLeafCert(pemCerts []byte) (string, error) {
+func GetTLSConfigForCert(pemCerts []byte) (*tls.Config, error) {
 	block, _ := pem.Decode(pemCerts)
 	if block == nil {
 		err := errors.New("decode ca file fail")
-		log.Error(err, "failed to get CN from cert", "pem", pemCerts)
-		return "", err
+		log.Error(err, "failed to decode cert", "pem", pemCerts)
+		return nil, err
 	}
 	if block.Type != "CERTIFICATE" || len(block.Headers) != 0 {
 		err := errors.New("pem not certificate or header not found")
-		log.Error(err, "failed to get CN from cert", "pem", pemCerts)
-		return "", err
+		log.Error(err, "failed to decode cert", "pem", pemCerts)
+		return nil, err
 	}
 
 	cert, err := x509.ParseCertificate(block.Bytes)
 	if err != nil {
-		log.Error(err, "failed to get CN from cert", "pem", pemCerts)
-		return "", err
+		log.Error(err, "failed to decode cert", "pem", pemCerts)
+		return nil, err
 	}
 
+	// Native cert verification in case of CA cert
 	if cert.IsCA {
-		return "", nil
+		log.Info("configured CA cert", "subject", cert.Subject)
+		certPool := x509.NewCertPool()
+		certPool.AddCert(cert)
+		// #nosec G402: ignore insecure options
+		config := &tls.Config{
+			RootCAs: certPool,
+		}
+		return config, nil
+	}
+
+	// Exact pem matching for leaf certs (certificate pinning)
+	// #nosec G402: ignore insecure options
+	config := &tls.Config{
+		InsecureSkipVerify: true,
+		VerifyConnection: func(cs tls.ConnectionState) error {
+			if cs.PeerCertificates == nil || cs.PeerCertificates[0] == nil {
+				err := errors.New("server didn't present cert")
+				log.Error(err, "verify cert")
+				return err
+			}
+			if !bytes.Equal(cs.PeerCertificates[0].Raw, cert.Raw) {
+				err := errors.New("server certificate didn't match pinned leaf cert")
+				log.Error(err, "verify cert")
+				return err
+			}
+			return nil
+		},
 	}
-
-	return cert.Subject.CommonName, nil
+	log.Info("configured cert pining", "subject", cert.Subject)
+	return config, nil
 }
 
 func FindTag(tags []model.Tag, tagScope string) string {

pkg/nsx/util/utils_test.go

@@ -5,7 +5,10 @@ package util
 
 import (
 	"bytes"
+	"crypto/tls"
+	"crypto/x509"
 	"encoding/json"
+	"encoding/pem"
 	"errors"
 	"fmt"
 	"io"
@@ -245,15 +248,15 @@ func TestVerifyNsxCertWithThumbprint(t *testing.T) {
 	}
 }
 
-func TestGetCommonNameFromCert(t *testing.T) {
+func TestGetTLSConfigForCert(t *testing.T) {
 	tests := []struct {
 		name    string
 		pem     []byte
-		cn      string
+		isCA    bool
 		wantErr bool
 	}{
 		{
-			name: "One cert",
+			name: "One leaf cert",
 			pem: []byte(`
 -----BEGIN CERTIFICATE-----
 MIID5DCCAsygAwIBAgIJAIJaVMN4AJHVMA0GCSqGSIb3DQEBCwUAMIGIMTQwMgYD
@@ -279,7 +282,7 @@ XkMSQJYdYDsUkiu98jNxh+oT8Cqdruwtg73pw8pP17EPltBABlHkYOEznw3dgDH3
 jSy6ts7e8AND6YWulG9jLmrI1xWwjbVqAoapxJQeSRYQ6Wb/KODPlg==
 -----END CERTIFICATE-----
 `),
-			cn:      "nsxmanager-ob-22945368-1-dev-integ-nsx-9389",
+			isCA:    false,
 			wantErr: false,
 		},
 		{
@@ -379,7 +382,7 @@ nAuknZoh8/CbCzB428Hch0P+vGOaysXCHMnHjf87ElgI5rY97HosTvuDls4MPGmH
 VHOkc8KT/1EQrBVUAdj8BbGJoX90g5pJ19xOe4pIb4tF9g==
 -----END CERTIFICATE-----
 `),
-			cn:      "nsxManager.sddc-10-215-208-250.vmwarevmc.com",
+			isCA:    false,
 			wantErr: false,
 		},
 		{
@@ -416,7 +419,7 @@ exCdtTix9qrKgWRs6PLigVWXUX/hwidQosk8WwBD9lu51aX8/wdQQGcHsFXwt35u
 Lcw=
 -----END CERTIFICATE-----
 `),
-			cn:      "",
+			isCA:    true,
 			wantErr: false,
 		},
 		{
@@ -427,11 +430,26 @@ Lcw=
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			cn, err := GetCommonNameFromLeafCert(tt.pem)
+			config, err := GetTLSConfigForCert(tt.pem)
 			if tt.wantErr {
-				assert.Error(t, err, "GetCommonNameFromCert expected err returned")
+				assert.Error(t, err, "GetTLSConfigForCert expected err returned")
+				return
+			}
+
+			if tt.isCA {
+				assert.False(t, config.InsecureSkipVerify)
+				expected := x509.NewCertPool()
+				expected.AppendCertsFromPEM(tt.pem)
+				assert.True(t, config.RootCAs.Equal(expected))
 			} else {
-				assert.Equal(t, tt.cn, cn)
+				assert.True(t, config.InsecureSkipVerify)
+				assert.Nil(t, config.RootCAs)
+				assert.NotNil(t, config.VerifyConnection)
+				certDer, _ := pem.Decode(tt.pem)
+				cert, _ := x509.ParseCertificate(certDer.Bytes)
+				assert.NoError(t, config.VerifyConnection(tls.ConnectionState{
+					PeerCertificates: []*x509.Certificate{cert},
+				}))
 			}
 		})
 	}