From cf3470b69ed0ea12363ceb20f17a0b79558fcbf6 Mon Sep 17 00:00:00 2001 From: denyeart Date: Mon, 30 Aug 2021 18:21:56 -0400 Subject: [PATCH] Update x509.CertPool equality checks (#2879) Go 1.16 changed the CertPool implementation to employ functions to lazily acquire certificates. This change effectively breaks `reflect.DeepEqual` used by our test assertions. This commit changes the assertions compare certificate subjects instead of the entire pool. While not the same, it's a close approximation. See https://go-review.googlesource.com/c/go/+/229917 Signed-off-by: Matthew Sykes Co-authored-by: Matthew Sykes --- common/fabhttp/tls_test.go | 6 +++++- internal/pkg/comm/creds_test.go | 29 ++++++++--------------------- 2 files changed, 13 insertions(+), 22 deletions(-) diff --git a/common/fabhttp/tls_test.go b/common/fabhttp/tls_test.go index 09064f81602..f6efe689ebf 100644 --- a/common/fabhttp/tls_test.go +++ b/common/fabhttp/tls_test.go @@ -59,6 +59,11 @@ var _ = Describe("TLS", func() { tlsConfig, err := httpTLS.Config() Expect(err).NotTo(HaveOccurred()) + + // https://go-review.googlesource.com/c/go/+/229917 + Expect(tlsConfig.ClientCAs.Subjects()).To(Equal(clientCAPool.Subjects())) + tlsConfig.ClientCAs = nil + Expect(tlsConfig).To(Equal(&tls.Config{ MinVersion: tls.VersionTLS12, Certificates: []tls.Certificate{cert}, @@ -70,7 +75,6 @@ var _ = Describe("TLS", func() { tls.TLS_RSA_WITH_AES_128_GCM_SHA256, tls.TLS_RSA_WITH_AES_256_GCM_SHA384, }, - ClientCAs: clientCAPool, ClientAuth: tls.RequireAndVerifyClientCert, })) }) diff --git a/internal/pkg/comm/creds_test.go b/internal/pkg/comm/creds_test.go index 68c1e6cbe10..48496f5b753 100644 --- a/internal/pkg/comm/creds_test.go +++ b/internal/pkg/comm/creds_test.go @@ -125,38 +125,25 @@ func TestAddRootCA(t *testing.T) { t.Parallel() caPEM, err := ioutil.ReadFile(filepath.Join("testdata", "certs", "Org1-cert.pem")) - if err != nil { - t.Fatalf("failed to read root certificate: %v", err) - } - - cert := &x509.Certificate{ - EmailAddresses: []string{"test@foobar.com"}, - } + require.NoError(t, err, "failed to read root certificate") expectedCertPool := x509.NewCertPool() ok := expectedCertPool.AppendCertsFromPEM(caPEM) - if !ok { - t.Fatalf("failed to create expected certPool") - } + require.True(t, ok, "failed to create expected certPool") + cert := &x509.Certificate{EmailAddresses: []string{"test@foobar.com"}} expectedCertPool.AddCert(cert) certPool := x509.NewCertPool() ok = certPool.AppendCertsFromPEM(caPEM) - if !ok { - t.Fatalf("failed to create certPool") - } + require.True(t, ok, "failed to create certPool") - tlsConfig := &tls.Config{ - ClientCAs: certPool, - } - config := comm.NewTLSConfig(tlsConfig) - - require.Equal(t, config.Config().ClientCAs, certPool) + config := comm.NewTLSConfig(&tls.Config{ClientCAs: certPool}) + require.Same(t, config.Config().ClientCAs, certPool) + // https://go-review.googlesource.com/c/go/+/229917 config.AddClientRootCA(cert) - - require.Equal(t, config.Config().ClientCAs, expectedCertPool, "The CertPools should be equal") + require.Equal(t, certPool.Subjects(), expectedCertPool.Subjects(), "subjects in the pool should be equal") } func TestSetClientCAs(t *testing.T) {