From 6df4d6067580d13ea056e9283fcd4809af2ce5a6 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Tue, 13 Apr 2021 12:06:13 -0400 Subject: [PATCH 1/3] ci: test against Go1.16.3 --- .circleci/config.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 58964ad2d..4945f7a9e 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -13,7 +13,7 @@ parameters: references: images: - go: &GOLANG_IMAGE docker.mirror.hashicorp.services/circleci/golang:1.15.6 + go: &GOLANG_IMAGE docker.mirror.hashicorp.services/circleci/golang:1.16.3 ember: &EMBER_IMAGE docker.mirror.hashicorp.services/circleci/node:12-browsers paths: From 7f6588082976b28356f858307059ea8b79b4f31d Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Tue, 13 Apr 2021 13:25:45 -0400 Subject: [PATCH 2/3] connect: fix test for go1.16 There is no way to compare x509.CertPools now that it has an unexpected function field. This comparison is as close as we can get. See https://github.com/golang/go/issues/26614 for a related issue. --- connect/tls_test.go | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/connect/tls_test.go b/connect/tls_test.go index 0ea3b897d..9da69b3e1 100644 --- a/connect/tls_test.go +++ b/connect/tls_test.go @@ -6,13 +6,15 @@ import ( "encoding/pem" "testing" - "github.com/hashicorp/consul/sdk/testutil" - "github.com/hashicorp/consul/testrpc" + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + "github.com/stretchr/testify/require" "github.com/hashicorp/consul/agent" "github.com/hashicorp/consul/agent/connect" "github.com/hashicorp/consul/api" - "github.com/stretchr/testify/require" + "github.com/hashicorp/consul/sdk/testutil" + "github.com/hashicorp/consul/testrpc" ) func Test_verifyServerCertMatchesURI(t *testing.T) { @@ -266,7 +268,7 @@ func TestServerSideVerifier(t *testing.T) { func requireEqualTLSConfig(t *testing.T, expect, got *tls.Config) { require := require.New(t) require.Equal(expect.RootCAs, got.RootCAs) - require.Equal(expect.ClientCAs, got.ClientCAs) + assertDeepEqual(t, expect.ClientCAs, got.ClientCAs, cmpCertPool) require.Equal(expect.InsecureSkipVerify, got.InsecureSkipVerify) require.Equal(expect.MinVersion, got.MinVersion) require.Equal(expect.CipherSuites, got.CipherSuites) @@ -293,6 +295,19 @@ func requireEqualTLSConfig(t *testing.T, expect, got *tls.Config) { require.Equal(expectLeaf, gotLeaf) } +// lazyCerts has a func field which can't be compared. +var cmpCertPool = cmp.Options{ + cmpopts.IgnoreFields(x509.CertPool{}, "lazyCerts"), + cmp.AllowUnexported(x509.CertPool{}), +} + +func assertDeepEqual(t *testing.T, x, y interface{}, opts ...cmp.Option) { + t.Helper() + if diff := cmp.Diff(x, y, opts...); diff != "" { + t.Fatalf("assertion failed: values are not equal\n--- expected\n+++ actual\n%v", diff) + } +} + // requireCorrectVerifier invokes got.VerifyPeerCertificate and expects the // tls.Config arg to be returned on the provided channel. This ensures the // correct verifier func was attached to got. From 6ee17c15ffd8976f485cfefc26bb3bbf8c805b4f Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Tue, 13 Apr 2021 13:31:20 -0400 Subject: [PATCH 3/3] tlsutil: fix a test for go1.16 Using a TestSigner was causing problems because go1.16 has this change: > CreateCertificate now verifies the generated certificate's signature > using the signer's public key. If the signature is invalid, an error is > returned, instead of a malformed certificate. See https://golang.org/doc/go1.16#crypto/x509 --- tlsutil/generate_test.go | 77 +++++++++++++++++++++------------------- 1 file changed, 40 insertions(+), 37 deletions(-) diff --git a/tlsutil/generate_test.go b/tlsutil/generate_test.go index 974d3548e..5be9f7e2b 100644 --- a/tlsutil/generate_test.go +++ b/tlsutil/generate_test.go @@ -62,52 +62,55 @@ func (s *TestSigner) Sign(rand io.Reader, digest []byte, opts crypto.SignerOpts) } func TestGenerateCA(t *testing.T) { - t.Parallel() - ca, pk, err := GenerateCA(CAOpts{Signer: &TestSigner{}}) - require.Error(t, err) - require.Empty(t, ca) - require.Empty(t, pk) + t.Run("no signer", func(t *testing.T) { + ca, pk, err := GenerateCA(CAOpts{Signer: &TestSigner{}}) + require.Error(t, err) + require.Empty(t, ca) + require.Empty(t, pk) + }) - // test what happens with wrong key - ca, pk, err = GenerateCA(CAOpts{Signer: &TestSigner{public: &rsa.PublicKey{}}}) - require.Error(t, err) - require.Empty(t, ca) - require.Empty(t, pk) + t.Run("wrong key", func(t *testing.T) { + ca, pk, err := GenerateCA(CAOpts{Signer: &TestSigner{public: &rsa.PublicKey{}}}) + require.Error(t, err) + require.Empty(t, ca) + require.Empty(t, pk) + }) - // test what happens with correct key - ca, pk, err = GenerateCA(CAOpts{}) - require.Nil(t, err) - require.NotEmpty(t, ca) - require.NotEmpty(t, pk) + t.Run("valid key", func(t *testing.T) { + ca, pk, err := GenerateCA(CAOpts{}) + require.Nil(t, err) + require.NotEmpty(t, ca) + require.NotEmpty(t, pk) - cert, err := parseCert(ca) - require.Nil(t, err) - require.True(t, strings.HasPrefix(cert.Subject.CommonName, "Consul Agent CA")) - require.Equal(t, true, cert.IsCA) - require.Equal(t, true, cert.BasicConstraintsValid) + cert, err := parseCert(ca) + require.Nil(t, err) + require.True(t, strings.HasPrefix(cert.Subject.CommonName, "Consul Agent CA")) + require.Equal(t, true, cert.IsCA) + require.Equal(t, true, cert.BasicConstraintsValid) - require.WithinDuration(t, cert.NotBefore, time.Now(), time.Minute) - require.WithinDuration(t, cert.NotAfter, time.Now().AddDate(0, 0, 365), time.Minute) + require.WithinDuration(t, cert.NotBefore, time.Now(), time.Minute) + require.WithinDuration(t, cert.NotAfter, time.Now().AddDate(0, 0, 365), time.Minute) - require.Equal(t, x509.KeyUsageCertSign|x509.KeyUsageCRLSign|x509.KeyUsageDigitalSignature, cert.KeyUsage) + require.Equal(t, x509.KeyUsageCertSign|x509.KeyUsageCRLSign|x509.KeyUsageDigitalSignature, cert.KeyUsage) + }) - // Test what happens with a correct RSA Key - s, err := rsa.GenerateKey(rand.Reader, 2048) - require.Nil(t, err) - ca, _, err = GenerateCA(CAOpts{Signer: &TestSigner{public: s.Public()}}) - require.NoError(t, err) - require.NotEmpty(t, ca) + t.Run("RSA key", func(t *testing.T) { + ca, pk, err := GenerateCA(CAOpts{}) + require.NoError(t, err) + require.NotEmpty(t, ca) + require.NotEmpty(t, pk) - cert, err = parseCert(ca) - require.NoError(t, err) - require.True(t, strings.HasPrefix(cert.Subject.CommonName, "Consul Agent CA")) - require.Equal(t, true, cert.IsCA) - require.Equal(t, true, cert.BasicConstraintsValid) + cert, err := parseCert(ca) + require.NoError(t, err) + require.True(t, strings.HasPrefix(cert.Subject.CommonName, "Consul Agent CA")) + require.Equal(t, true, cert.IsCA) + require.Equal(t, true, cert.BasicConstraintsValid) - require.WithinDuration(t, cert.NotBefore, time.Now(), time.Minute) - require.WithinDuration(t, cert.NotAfter, time.Now().AddDate(0, 0, 365), time.Minute) + require.WithinDuration(t, cert.NotBefore, time.Now(), time.Minute) + require.WithinDuration(t, cert.NotAfter, time.Now().AddDate(0, 0, 365), time.Minute) - require.Equal(t, x509.KeyUsageCertSign|x509.KeyUsageCRLSign|x509.KeyUsageDigitalSignature, cert.KeyUsage) + require.Equal(t, x509.KeyUsageCertSign|x509.KeyUsageCRLSign|x509.KeyUsageDigitalSignature, cert.KeyUsage) + }) } func TestGenerateCert(t *testing.T) {