luxolus/open-consul
2
0
Fork 0
Code Issues 1 Pull requests Packages Releases Wiki Activity

rpc: include error for AuthorizeServerConn failures

Browse source 
The errs were not being captured because the value of append was not being assigned.
This commit is contained in:
Daniel Nephin 2021-10-04 13:22:30 -04:00
parent 3c8ca0dbd2
commit 24df1392b3
2 changed files with 126 additions and 2 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
tlsutil/config.go
View file

@ -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)

120
tlsutil/config_test.go
View file

@ -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"