diff --git a/tlsutil/config.go b/tlsutil/config.go index 6abe92329..55c8aa225 100644 --- a/tlsutil/config.go +++ b/tlsutil/config.go @@ -893,6 +893,10 @@ func (c *Configurator) wrapALPNTLSClient(dc, nodeName, alpnProto string, conn ne return tlsConn, nil } +type TLSConn interface { + ConnectionState() tls.ConnectionState +} + // AuthorizeServerConn is used to validate that the connection is being established // by a Consul server in the same datacenter. // @@ -902,7 +906,7 @@ func (c *Configurator) wrapALPNTLSClient(dc, nodeName, alpnProto string, conn ne // // Note this check is only performed if VerifyServerHostname is enabled, otherwise // it does no authorization. -func (c *Configurator) AuthorizeServerConn(dc string, conn *tls.Conn) error { +func (c *Configurator) AuthorizeServerConn(dc string, conn TLSConn) error { if !c.VerifyServerHostname() { return nil } @@ -931,7 +935,7 @@ func (c *Configurator) AuthorizeServerConn(dc string, conn *tls.Conn) error { if err == nil { return nil } - multierror.Append(errs, err) + errs = multierror.Append(errs, err) } return fmt.Errorf("AuthorizeServerConn failed certificate validation for certificate with a SAN.DNSName of %v: %w", expected, errs) diff --git a/tlsutil/config_test.go b/tlsutil/config_test.go index f1cb58bca..2452933a6 100644 --- a/tlsutil/config_test.go +++ b/tlsutil/config_test.go @@ -7,6 +7,7 @@ import ( "io" "io/ioutil" "net" + "path/filepath" "reflect" "strings" "testing" @@ -1270,6 +1271,125 @@ func TestConfigurator_AutoEncryptCert(t *testing.T) { require.Equal(t, int64(4679716209), c.AutoEncryptCert().NotAfter.Unix()) } +func TestConfigurator_AuthorizeServerConn_Error(t *testing.T) { + caPEM, caPK, err := GenerateCA(CAOpts{Days: 5, Domain: "consul"}) + require.NoError(t, err) + + dir := testutil.TempDir(t, "ca") + caPath := filepath.Join(dir, "ca.pem") + err = ioutil.WriteFile(caPath, []byte(caPEM), 0600) + require.NoError(t, err) + + cfg := Config{ + VerifyServerHostname: true, + Domain: "consul", + CAFile: caPath, + } + c, err := NewConfigurator(cfg, hclog.New(nil)) + require.NoError(t, err) + + t.Run("wrong DNSName", func(t *testing.T) { + signer, err := ParseSigner(caPK) + require.NoError(t, err) + + pem, _, err := GenerateCert(CertOpts{ + Signer: signer, + CA: caPEM, + Name: "server.dc1.consul", + Days: 5, + DNSNames: []string{"this-name-is-wrong", "localhost"}, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth}, + }) + require.NoError(t, err) + + s := fakeTLSConn{ + state: tls.ConnectionState{ + VerifiedChains: [][]*x509.Certificate{certChain(t, pem, caPEM)}, + PeerCertificates: certChain(t, pem, caPEM), + }, + } + err = c.AuthorizeServerConn("dc1", s) + testutil.RequireErrorContains(t, err, "is valid for this-name-is-wrong, localhost, not server.dc1.consul") + }) + + t.Run("wrong CA", func(t *testing.T) { + caPEM, caPK, err := GenerateCA(CAOpts{Days: 5, Domain: "consul"}) + require.NoError(t, err) + + dir := testutil.TempDir(t, "other") + caPath := filepath.Join(dir, "ca.pem") + err = ioutil.WriteFile(caPath, []byte(caPEM), 0600) + require.NoError(t, err) + + signer, err := ParseSigner(caPK) + require.NoError(t, err) + + pem, _, err := GenerateCert(CertOpts{ + Signer: signer, + CA: caPEM, + Name: "server.dc1.consul", + Days: 5, + DNSNames: []string{"server.dc1.consul", "localhost"}, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth}, + }) + require.NoError(t, err) + + s := fakeTLSConn{ + state: tls.ConnectionState{ + VerifiedChains: [][]*x509.Certificate{certChain(t, pem, caPEM)}, + PeerCertificates: certChain(t, pem, caPEM), + }, + } + err = c.AuthorizeServerConn("dc1", s) + testutil.RequireErrorContains(t, err, "signed by unknown authority") + }) + + t.Run("missing ext key usage", func(t *testing.T) { + signer, err := ParseSigner(caPK) + require.NoError(t, err) + + pem, _, err := GenerateCert(CertOpts{ + Signer: signer, + CA: caPEM, + Name: "server.dc1.consul", + Days: 5, + DNSNames: []string{"server.dc1.consul", "localhost"}, + ExtKeyUsage: []x509.ExtKeyUsage{x509.ExtKeyUsageEmailProtection}, + }) + require.NoError(t, err) + + s := fakeTLSConn{ + state: tls.ConnectionState{ + VerifiedChains: [][]*x509.Certificate{certChain(t, pem, caPEM)}, + PeerCertificates: certChain(t, pem, caPEM), + }, + } + err = c.AuthorizeServerConn("dc1", s) + testutil.RequireErrorContains(t, err, "certificate specifies an incompatible key usage") + }) +} + +type fakeTLSConn struct { + state tls.ConnectionState +} + +func (f fakeTLSConn) ConnectionState() tls.ConnectionState { + return f.state +} + +func certChain(t *testing.T, certs ...string) []*x509.Certificate { + t.Helper() + + result := make([]*x509.Certificate, 0, len(certs)) + + for i, c := range certs { + cert, err := parseCert(c) + require.NoError(t, err, "cert %d", i) + result = append(result, cert) + } + return result +} + func TestConfig_tlsVersions(t *testing.T) { require.Equal(t, []string{"tls10", "tls11", "tls12", "tls13"}, tlsVersions()) expected := "tls10, tls11, tls12, tls13"