tls: consider presented intermediates during server connection tls handshake. (#10964)

* use intermediates when verifying

* extract connection state

* remove useless import

* add changelog entry

* golint

* better error

* wording

* collect errors

* use SAN.DNSName instead of CommonName

* Add test for unknown intermediate

* improve changelog entry

.changelog/10964.txt

@@ -0,0 +1,3 @@
+```release-note:bug
+tls: consider presented intermediates during server connection tls handshake.
+```

agent/consul/rpc_test.go

@@ -1072,13 +1072,22 @@ func (r isReadRequest) HasTimedOut(since time.Time, rpcHoldTimeout, maxQueryTime
 }
 
 func TestRPC_AuthorizeRaftRPC(t *testing.T) {
-	caPEM, pk, err := tlsutil.GenerateCA(tlsutil.CAOpts{Days: 5, Domain: "consul"})
+	caPEM, caPK, err := tlsutil.GenerateCA(tlsutil.CAOpts{Days: 5, Domain: "consul"})
+	require.NoError(t, err)
+
+	caSigner, err := tlsutil.ParseSigner(caPK)
 	require.NoError(t, err)
 
 	dir := testutil.TempDir(t, "certs")
 	err = ioutil.WriteFile(filepath.Join(dir, "ca.pem"), []byte(caPEM), 0600)
 	require.NoError(t, err)
 
+	intermediatePEM, intermediatePK, err := tlsutil.GenerateCert(tlsutil.CertOpts{IsCA: true, CA: caPEM, Signer: caSigner, Days: 5})
+	require.NoError(t, err)
+
+	err = ioutil.WriteFile(filepath.Join(dir, "intermediate.pem"), []byte(intermediatePEM), 0600)
+	require.NoError(t, err)
+
 	newCert := func(t *testing.T, caPEM, pk, node, name string) {
 		t.Helper()
 
@@ -1101,7 +1110,7 @@ func TestRPC_AuthorizeRaftRPC(t *testing.T) {
 		require.NoError(t, err)
 	}
 
-	newCert(t, caPEM, pk, "srv1", "server.dc1.consul")
+	newCert(t, caPEM, caPK, "srv1", "server.dc1.consul")
 
 	_, connectCApk, err := connect.GeneratePrivateKey()
 	require.NoError(t, err)
@@ -1113,7 +1122,7 @@ func TestRPC_AuthorizeRaftRPC(t *testing.T) {
 		c.TLSConfig.KeyFile = filepath.Join(dir, "srv1-server.dc1.consul.key")
 		c.TLSConfig.VerifyIncoming = true
 		c.TLSConfig.VerifyServerHostname = true
-		// Enable Auto-Encrypt so that Conenct CA roots are added to the
+		// Enable Auto-Encrypt so that Connect CA roots are added to the
 		// tlsutil.Configurator.
 		c.AutoEncryptAllowTLS = true
 		c.CAConfig = &structs.CAConfiguration{
@@ -1159,11 +1168,29 @@ func TestRPC_AuthorizeRaftRPC(t *testing.T) {
 
 	setupAgentTLSCert := func(name string) func(t *testing.T) string {
 		return func(t *testing.T) string {
-			newCert(t, caPEM, pk, "node1", name)
+			newCert(t, caPEM, caPK, "node1", name)
 			return filepath.Join(dir, "node1-"+name)
 		}
 	}
 
+	setupAgentTLSCertWithIntermediate := func(name string) func(t *testing.T) string {
+		return func(t *testing.T) string {
+			newCert(t, intermediatePEM, intermediatePK, "node1", name)
+			certPrefix := filepath.Join(dir, "node1-"+name)
+			f, err := os.OpenFile(certPrefix+".pem", os.O_APPEND|os.O_CREATE|os.O_WRONLY, 0644)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if _, err := f.Write([]byte(intermediatePEM)); err != nil {
+				t.Fatal(err)
+			}
+			if err := f.Close(); err != nil {
+				t.Fatal(err)
+			}
+			return certPrefix
+		}
+	}
+
 	setupConnectCACert := func(name string) func(t *testing.T) string {
 		return func(t *testing.T) string {
 			_, caRoot := srv.caManager.getCAProvider()
@@ -1221,6 +1248,11 @@ func TestRPC_AuthorizeRaftRPC(t *testing.T) {
 			setupCert: setupAgentTLSCert("server.dc1.consul"),
 			conn:      useTLSByte,
 		},
+		{
+			name:      "TLS byte with server cert in same DC and with unknown intermediate",
+			setupCert: setupAgentTLSCertWithIntermediate("server.dc1.consul"),
+			conn:      useTLSByte,
+		},
 		{
 			name:        "TLS byte with ConnectCA leaf cert",
 			setupCert:   setupConnectCACert("server.dc1.consul"),

tlsutil/config.go

@@ -15,6 +15,7 @@ import (
 	"time"
 
 	"github.com/hashicorp/go-hclog"
+	"github.com/hashicorp/go-multierror"
 
 	"github.com/hashicorp/consul/logging"
 )
@@ -842,15 +843,11 @@ func (c *Configurator) wrapTLSClient(dc string, conn net.Conn) (net.Conn, error)
 		Intermediates: x509.NewCertPool(),
 	}
 
-	certs := tlsConn.ConnectionState().PeerCertificates
+	cs := tlsConn.ConnectionState()
-	for i, cert := range certs {
+	for _, cert := range cs.PeerCertificates[1:] {
-		if i == 0 {
-			continue
-		}
 		opts.Intermediates.AddCert(cert)
 	}
-
+	_, err = cs.PeerCertificates[0].Verify(opts)
-	_, err = certs[0].Verify(opts)
 	if err != nil {
 		tlsConn.Close()
 		return nil, err
@@ -915,22 +912,29 @@ func (c *Configurator) AuthorizeServerConn(dc string, conn *tls.Conn) error {
 	c.lock.RUnlock()
 
 	expected := c.ServerSNI(dc, "")
-	for _, chain := range conn.ConnectionState().VerifiedChains {
+	cs := conn.ConnectionState()
+	var errs error
+	for _, chain := range cs.VerifiedChains {
 		if len(chain) == 0 {
 			continue
 		}
-		clientCert := chain[0]
+		opts := x509.VerifyOptions{
-		_, err := clientCert.Verify(x509.VerifyOptions{
 			DNSName:       expected,
+			Intermediates: x509.NewCertPool(),
 			Roots:         caPool,
 			KeyUsages:     []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth},
-		})
+		}
+		for _, cert := range cs.PeerCertificates[1:] {
+			opts.Intermediates.AddCert(cert)
+		}
+		_, err := cs.PeerCertificates[0].Verify(opts)
 		if err == nil {
 			return nil
 		}
-		c.logger.Debug("AuthorizeServerConn failed certificate validation", "error", err)
+		multierror.Append(errs, err)
 	}
-	return fmt.Errorf("a TLS certificate with a CommonName of %v is required for this operation", expected)
+	return fmt.Errorf("AuthorizeServerConn failed certificate validation for certificate with a SAN.DNSName of %v: %w", expected, errs)
+
 }
 
 // ParseCiphers parse ciphersuites from the comma-separated string into

tlsutil/generate.go

@@ -54,6 +54,7 @@ type CertOpts struct {
 	DNSNames    []string
 	IPAddresses []net.IP
 	ExtKeyUsage []x509.ExtKeyUsage
+	IsCA        bool
 }
 
 // GenerateCA generates a new CA for agent TLS (not to be confused with Connect TLS)
@@ -177,6 +178,10 @@ func GenerateCert(opts CertOpts) (string, string, error) {
 		DNSNames:              opts.DNSNames,
 		IPAddresses:           opts.IPAddresses,
 	}
+	if opts.IsCA {
+		template.IsCA = true
+		template.KeyUsage = x509.KeyUsageCertSign | x509.KeyUsageCRLSign | x509.KeyUsageDigitalSignature
+	}
 
 	bs, err := x509.CreateCertificate(rand.Reader, &template, parent, signee.Public(), opts.Signer)
 	if err != nil {