From 540728db199e62f3385b9370e940dbc817b3229e Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Mon, 25 Oct 2021 13:29:36 +0200 Subject: [PATCH 1/5] libgit2: update to v1.3.0 This commit experimentally updates `libgit2` to `v1.3.0`, which supports Go native transport (without OpenSSL or LibSSH2 as dependencies). Signed-off-by: Hidde Beydals --- Makefile | 4 ++-- go.mod | 2 +- go.sum | 4 ++-- pkg/git/libgit2/checkout.go | 10 +++++----- pkg/git/libgit2/checkout_test.go | 2 +- pkg/git/libgit2/transport.go | 20 ++++++++++---------- pkg/git/libgit2/transport_test.go | 2 +- 7 files changed, 22 insertions(+), 22 deletions(-) diff --git a/Makefile b/Makefile index fb4d99981..250f82616 100644 --- a/Makefile +++ b/Makefile @@ -4,7 +4,7 @@ TAG ?= latest # Base image used to build the Go binary LIBGIT2_IMG ?= ghcr.io/fluxcd/golang-with-libgit2 -LIBGIT2_TAG ?= libgit2-1.1.1-1 +LIBGIT2_TAG ?= main # Allows for defining additional Docker buildx arguments, # e.g. '--push'. @@ -19,7 +19,7 @@ CRD_OPTIONS ?= crd:crdVersions=v1 REPOSITORY_ROOT := $(shell git rev-parse --show-toplevel) # Libgit2 version -LIBGIT2_VERSION ?= 1.1.1 +LIBGIT2_VERSION ?= 1.3.0 # Other dependency versions ENVTEST_BIN_VERSION ?= 1.19.2 diff --git a/go.mod b/go.mod index 9eaaed4ed..a19125552 100644 --- a/go.mod +++ b/go.mod @@ -25,7 +25,7 @@ require ( github.com/go-logr/logr v0.4.0 github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect github.com/googleapis/gax-go/v2 v2.1.0 // indirect - github.com/libgit2/git2go/v31 v31.6.1 + github.com/libgit2/git2go/v33 v33.0.1 github.com/minio/minio-go/v7 v7.0.10 github.com/onsi/ginkgo v1.16.4 github.com/onsi/gomega v1.14.0 diff --git a/go.sum b/go.sum index 9ebb6018b..d4970d8be 100644 --- a/go.sum +++ b/go.sum @@ -613,8 +613,8 @@ github.com/lann/ps v0.0.0-20150810152359-62de8c46ede0/go.mod h1:vmVJ0l/dxyfGW6Fm github.com/lib/pq v1.2.0/go.mod h1:5WUZQaWbwv1U+lTReE5YruASi9Al49XbQIvNi/34Woo= github.com/lib/pq v1.10.0 h1:Zx5DJFEYQXio93kgXnQ09fXNiUKsqv4OUEu2UtGcB1E= github.com/lib/pq v1.10.0/go.mod h1:AlVN5x4E4T544tWzH6hKfbfQvm3HdbOxrmggDNAPY9o= -github.com/libgit2/git2go/v31 v31.6.1 h1:FnKHHDDBgltSsu9RpKuL4rSR8dQ1JTf9dfvFhZ1y7Aw= -github.com/libgit2/git2go/v31 v31.6.1/go.mod h1:c/rkJcBcUFx6wHaT++UwNpKvIsmPNqCeQ/vzO4DrEec= +github.com/libgit2/git2go/v33 v33.0.1 h1:9l8o4o5JhH7ptjvK7fdcIVC8Kwht6V/Hie+Tjac3C9s= +github.com/libgit2/git2go/v33 v33.0.1/go.mod h1:KdpqkU+6+++4oHna/MIOgx4GCQ92IPCdpVRMRI80J+4= github.com/liggitt/tabwriter v0.0.0-20181228230101-89fcab3d43de h1:9TO3cAIGXtEhnIaL+V+BEER86oLrvS+kWobKpbJuye0= github.com/liggitt/tabwriter v0.0.0-20181228230101-89fcab3d43de/go.mod h1:zAbeS9B/r2mtpb6U+EI2rYA5OAXxsYw6wTamcNW+zcE= github.com/lightstep/lightstep-tracer-common/golang/gogo v0.0.0-20190605223551-bc2310a04743/go.mod h1:qklhhLq1aX+mtWk9cPHPzaBjWImj5ULL6C7HFJtXQMM= diff --git a/pkg/git/libgit2/checkout.go b/pkg/git/libgit2/checkout.go index 3d60d9c78..b0fdeb073 100644 --- a/pkg/git/libgit2/checkout.go +++ b/pkg/git/libgit2/checkout.go @@ -25,7 +25,7 @@ import ( "github.com/Masterminds/semver/v3" "github.com/go-logr/logr" - git2go "github.com/libgit2/git2go/v31" + git2go "github.com/libgit2/git2go/v33" "github.com/fluxcd/pkg/gitutil" "github.com/fluxcd/pkg/version" @@ -61,7 +61,7 @@ type CheckoutBranch struct { func (c *CheckoutBranch) Checkout(ctx context.Context, path, url string, opts *git.AuthOptions) (*git.Commit, error) { repo, err := git2go.Clone(url, path, &git2go.CloneOptions{ - FetchOptions: &git2go.FetchOptions{ + FetchOptions: git2go.FetchOptions{ DownloadTags: git2go.DownloadTagsNone, RemoteCallbacks: RemoteCallbacks(opts), }, @@ -90,7 +90,7 @@ type CheckoutTag struct { func (c *CheckoutTag) Checkout(ctx context.Context, path, url string, opts *git.AuthOptions) (*git.Commit, error) { repo, err := git2go.Clone(url, path, &git2go.CloneOptions{ - FetchOptions: &git2go.FetchOptions{ + FetchOptions: git2go.FetchOptions{ DownloadTags: git2go.DownloadTagsAll, RemoteCallbacks: RemoteCallbacks(opts), }, @@ -113,7 +113,7 @@ type CheckoutCommit struct { func (c *CheckoutCommit) Checkout(ctx context.Context, path, url string, opts *git.AuthOptions) (*git.Commit, error) { repo, err := git2go.Clone(url, path, &git2go.CloneOptions{ - FetchOptions: &git2go.FetchOptions{ + FetchOptions: git2go.FetchOptions{ DownloadTags: git2go.DownloadTagsNone, RemoteCallbacks: RemoteCallbacks(opts), }, @@ -144,7 +144,7 @@ func (c *CheckoutSemVer) Checkout(ctx context.Context, path, url string, opts *g } repo, err := git2go.Clone(url, path, &git2go.CloneOptions{ - FetchOptions: &git2go.FetchOptions{ + FetchOptions: git2go.FetchOptions{ DownloadTags: git2go.DownloadTagsAll, RemoteCallbacks: RemoteCallbacks(opts), }, diff --git a/pkg/git/libgit2/checkout_test.go b/pkg/git/libgit2/checkout_test.go index 24ca72b30..2ca991c69 100644 --- a/pkg/git/libgit2/checkout_test.go +++ b/pkg/git/libgit2/checkout_test.go @@ -25,7 +25,7 @@ import ( "testing" "time" - git2go "github.com/libgit2/git2go/v31" + git2go "github.com/libgit2/git2go/v33" . "github.com/onsi/gomega" ) diff --git a/pkg/git/libgit2/transport.go b/pkg/git/libgit2/transport.go index 83d9107ec..434419857 100644 --- a/pkg/git/libgit2/transport.go +++ b/pkg/git/libgit2/transport.go @@ -29,7 +29,7 @@ import ( "strings" "time" - git2go "github.com/libgit2/git2go/v31" + git2go "github.com/libgit2/git2go/v33" "golang.org/x/crypto/ssh" "golang.org/x/crypto/ssh/knownhosts" @@ -100,10 +100,10 @@ func certificateCallback(opts *git.AuthOptions) git2go.CertificateCheckCallback // x509Callback returns a CertificateCheckCallback that verifies the // certificate against the given caBundle for git.HTTPS Transports. func x509Callback(caBundle []byte) git2go.CertificateCheckCallback { - return func(cert *git2go.Certificate, valid bool, hostname string) git2go.ErrorCode { + return func(cert *git2go.Certificate, valid bool, hostname string) error { roots := x509.NewCertPool() if ok := roots.AppendCertsFromPEM(caBundle); !ok { - return git2go.ErrorCodeCertificate + return fmt.Errorf("failed to create cert pool with given CA bundle") } opts := x509.VerifyOptions{ @@ -112,9 +112,9 @@ func x509Callback(caBundle []byte) git2go.CertificateCheckCallback { CurrentTime: now(), } if _, err := cert.X509.Verify(opts); err != nil { - return git2go.ErrorCodeCertificate + return fmt.Errorf("failed to verify certfificate: %w", err) } - return git2go.ErrorCodeOK + return nil } } @@ -122,10 +122,10 @@ func x509Callback(caBundle []byte) git2go.CertificateCheckCallback { // the key of Git server against the given host and known_hosts for // git.SSH Transports. func knownHostsCallback(host string, knownHosts []byte) git2go.CertificateCheckCallback { - return func(cert *git2go.Certificate, valid bool, hostname string) git2go.ErrorCode { + return func(cert *git2go.Certificate, valid bool, hostname string) error { kh, err := parseKnownHosts(string(knownHosts)) if err != nil { - return git2go.ErrorCodeCertificate + return fmt.Errorf("failed to parse known_hosts: %w", err) } // First, attempt to split the configured host and port to validate @@ -140,7 +140,7 @@ func knownHostsCallback(host string, knownHosts []byte) git2go.CertificateCheckC // Check if the configured host matches the hostname given to // the callback. if h != hostname { - return git2go.ErrorCodeUser + return fmt.Errorf("hostname from server '%s' does not match '%s'", hostname, h) } // We are now certain that the configured host and the hostname @@ -150,10 +150,10 @@ func knownHostsCallback(host string, knownHosts []byte) git2go.CertificateCheckC h = knownhosts.Normalize(host) for _, k := range kh { if k.matches(h, cert.Hostkey) { - return git2go.ErrorCodeOK + return nil } } - return git2go.ErrorCodeCertificate + return fmt.Errorf("failed to lookup known_hosts entry for '%s'", hostname) } } diff --git a/pkg/git/libgit2/transport_test.go b/pkg/git/libgit2/transport_test.go index 6f1e9545b..b1a224f2d 100644 --- a/pkg/git/libgit2/transport_test.go +++ b/pkg/git/libgit2/transport_test.go @@ -25,7 +25,7 @@ import ( "testing" "time" - git2go "github.com/libgit2/git2go/v31" + git2go "github.com/libgit2/git2go/v33" . "github.com/onsi/gomega" ) From cc397ddbbe3b3c7d181b64658b1517cf607452aa Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Mon, 25 Oct 2021 14:06:34 +0200 Subject: [PATCH 2/5] libgit2: update tests to check for returned errors Signed-off-by: Hidde Beydals --- pkg/git/libgit2/transport_test.go | 35 +++++++++++++++++++------------ 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/pkg/git/libgit2/transport_test.go b/pkg/git/libgit2/transport_test.go index b1a224f2d..113274f76 100644 --- a/pkg/git/libgit2/transport_test.go +++ b/pkg/git/libgit2/transport_test.go @@ -143,42 +143,41 @@ func Test_x509Callback(t *testing.T) { certificate string host string caBundle []byte - want git2go.ErrorCode + wantErr string }{ { name: "Valid certificate authority bundle", certificate: googleLeafFixture, host: "www.google.com", caBundle: []byte(giag2IntermediateFixture + "\n" + geoTrustRootFixture), - want: git2go.ErrorCodeOK, }, { name: "Invalid certificate", certificate: googleLeafWithInvalidHashFixture, host: "www.google.com", caBundle: []byte(giag2IntermediateFixture + "\n" + geoTrustRootFixture), - want: git2go.ErrorCodeCertificate, + wantErr: "possibly because of \"x509: cannot verify signature: algorithm unimplemented\"", }, { name: "Invalid certificate authority bundle", certificate: googleLeafFixture, host: "www.google.com", caBundle: bytes.Trim([]byte(giag2IntermediateFixture+"\n"+geoTrustRootFixture), "-"), - want: git2go.ErrorCodeCertificate, + wantErr: "failed to create cert pool with given CA bundle", }, { name: "Missing intermediate in bundle", certificate: googleLeafFixture, host: "www.google.com", caBundle: []byte(geoTrustRootFixture), - want: git2go.ErrorCodeCertificate, + wantErr: "failed to verify certfificate: x509: certificate signed by unknown authority", }, { name: "Invalid host", certificate: googleLeafFixture, host: "www.google.co", caBundle: []byte(giag2IntermediateFixture + "\n" + geoTrustRootFixture), - want: git2go.ErrorCodeCertificate, + wantErr: "certificate is valid for www.google.com, not www.google.co", }, } for _, tt := range tests { @@ -193,7 +192,13 @@ func Test_x509Callback(t *testing.T) { } callback := x509Callback(tt.caBundle) - g.Expect(callback(cert, false, tt.host)).To(Equal(tt.want)) + got := callback(cert, false, tt.host) + if tt.wantErr != "" { + g.Expect(got).To(HaveOccurred()) + g.Expect(got.Error()).To(ContainSubstring(tt.wantErr)) + return + } + g.Expect(got).ToNot(HaveOccurred()) }) } } @@ -205,7 +210,7 @@ func Test_knownHostsCallback(t *testing.T) { expectedHost string knownHosts []byte hostkey git2go.HostkeyCertificate - want git2go.ErrorCode + wantErr string }{ { name: "Match", @@ -213,7 +218,6 @@ func Test_knownHostsCallback(t *testing.T) { knownHosts: []byte(knownHostsFixture), hostkey: git2go.HostkeyCertificate{Kind: git2go.HostkeySHA1 | git2go.HostkeyMD5, HashSHA1: sha1Fingerprint("v2toJdKXfFEaR1u++4iq1UqSrHM")}, expectedHost: "github.com", - want: git2go.ErrorCodeOK, }, { name: "Match with port", @@ -221,7 +225,6 @@ func Test_knownHostsCallback(t *testing.T) { knownHosts: []byte(knownHostsFixture), hostkey: git2go.HostkeyCertificate{Kind: git2go.HostkeySHA1 | git2go.HostkeyMD5, HashSHA1: sha1Fingerprint("v2toJdKXfFEaR1u++4iq1UqSrHM")}, expectedHost: "github.com:22", - want: git2go.ErrorCodeOK, }, { name: "Hostname mismatch", @@ -229,7 +232,7 @@ func Test_knownHostsCallback(t *testing.T) { knownHosts: []byte(knownHostsFixture), hostkey: git2go.HostkeyCertificate{Kind: git2go.HostkeySHA1 | git2go.HostkeyMD5, HashSHA1: sha1Fingerprint("v2toJdKXfFEaR1u++4iq1UqSrHM")}, expectedHost: "example.com", - want: git2go.ErrorCodeUser, + wantErr: "hostname from server 'github.com' does not match 'example.com'", }, { name: "Hostkey mismatch", @@ -237,7 +240,7 @@ func Test_knownHostsCallback(t *testing.T) { knownHosts: []byte(knownHostsFixture), hostkey: git2go.HostkeyCertificate{Kind: git2go.HostkeyMD5, HashMD5: md5Fingerprint("\xb6\x03\x0e\x39\x97\x9e\xd0\xe7\x24\xce\xa3\x77\x3e\x01\x42\x09")}, expectedHost: "github.com", - want: git2go.ErrorCodeCertificate, + wantErr: "failed to lookup known_hosts entry for 'github.com'", }, } for _, tt := range tests { @@ -246,7 +249,13 @@ func Test_knownHostsCallback(t *testing.T) { cert := &git2go.Certificate{Hostkey: tt.hostkey} callback := knownHostsCallback(tt.expectedHost, tt.knownHosts) - g.Expect(callback(cert, false, tt.host)).To(Equal(tt.want)) + got := callback(cert, false, tt.host) + if tt.wantErr != "" { + g.Expect(got).To(HaveOccurred()) + g.Expect(got.Error()).To(ContainSubstring(tt.wantErr)) + return + } + g.Expect(got).ToNot(HaveOccurred()) }) } } From 2b0b1e07e91866990bb9284cdbdaf1ee18e9e327 Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Mon, 25 Oct 2021 14:35:42 +0200 Subject: [PATCH 3/5] controllers: check for more informative string Signed-off-by: Hidde Beydals --- controllers/gitrepository_controller_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/controllers/gitrepository_controller_test.go b/controllers/gitrepository_controller_test.go index f2fd6295d..c726f41c1 100644 --- a/controllers/gitrepository_controller_test.go +++ b/controllers/gitrepository_controller_test.go @@ -383,7 +383,7 @@ var _ = Describe("GitRepositoryReconciler", func() { reference: &sourcev1.GitRepositoryRef{Branch: "main"}, waitForReason: sourcev1.GitOperationFailedReason, expectStatus: metav1.ConditionFalse, - expectMessage: "unable to clone: user rejected certificate", + expectMessage: "x509: certificate signed by unknown authority", gitImplementation: sourcev1.LibGit2Implementation, }), Entry("self signed libgit2 with CA", refTestCase{ From c1517d51b792404e94e0c79b55a80fa03113496a Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Mon, 25 Oct 2021 22:08:19 +0200 Subject: [PATCH 4/5] build: ensure CI uses correct libgit2 version Signed-off-by: Hidde Beydals --- .github/actions/run-tests/Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/actions/run-tests/Dockerfile b/.github/actions/run-tests/Dockerfile index ee9bd04eb..fb093a4eb 100644 --- a/.github/actions/run-tests/Dockerfile +++ b/.github/actions/run-tests/Dockerfile @@ -3,7 +3,7 @@ ARG GO_VERSION=1.16.8 ARG XX_VERSION=1.0.0-rc.2 ARG LIBGIT2_IMG=ghcr.io/fluxcd/golang-with-libgit2 -ARG LIBGIT2_TAG=libgit2-1.1.1-1 +ARG LIBGIT2_TAG=main FROM tonistiigi/xx:${XX_VERSION} AS xx FROM ${LIBGIT2_IMG}:${LIBGIT2_TAG} as libgit2 From 15567d0a76a3a848b13946639d250f7f32c4c30e Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Mon, 25 Oct 2021 22:28:39 +0200 Subject: [PATCH 5/5] libgit2: change knownHostsCallback logic With Go backing the transport, the provided hostname to the callback now does include the port, and we no longer need to take the `Host` into account (as this will now result in a mismatch). Signed-off-by: Hidde Beydals --- pkg/git/libgit2/transport.go | 16 +++------------- pkg/git/libgit2/transport_test.go | 2 +- 2 files changed, 4 insertions(+), 14 deletions(-) diff --git a/pkg/git/libgit2/transport.go b/pkg/git/libgit2/transport.go index 434419857..f9bfeaebd 100644 --- a/pkg/git/libgit2/transport.go +++ b/pkg/git/libgit2/transport.go @@ -25,7 +25,6 @@ import ( "crypto/x509" "fmt" "hash" - "net" "strings" "time" @@ -128,26 +127,17 @@ func knownHostsCallback(host string, knownHosts []byte) git2go.CertificateCheckC return fmt.Errorf("failed to parse known_hosts: %w", err) } - // First, attempt to split the configured host and port to validate - // the port-less hostname given to the callback. - h, _, err := net.SplitHostPort(host) - if err != nil { - // SplitHostPort returns an error if the host is missing - // a port, assume the host has no port. - h = host - } - // Check if the configured host matches the hostname given to // the callback. - if h != hostname { - return fmt.Errorf("hostname from server '%s' does not match '%s'", hostname, h) + if host != hostname { + return fmt.Errorf("hostname from server '%s' does not match '%s'", hostname, host) } // We are now certain that the configured host and the hostname // given to the callback match. Use the configured host (that // includes the port), and normalize it, so we can check if there // is an entry for the hostname _and_ port. - h = knownhosts.Normalize(host) + h := knownhosts.Normalize(hostname) for _, k := range kh { if k.matches(h, cert.Hostkey) { return nil diff --git a/pkg/git/libgit2/transport_test.go b/pkg/git/libgit2/transport_test.go index 113274f76..f9646b4f2 100644 --- a/pkg/git/libgit2/transport_test.go +++ b/pkg/git/libgit2/transport_test.go @@ -221,7 +221,7 @@ func Test_knownHostsCallback(t *testing.T) { }, { name: "Match with port", - host: "github.com", + host: "github.com:22", knownHosts: []byte(knownHostsFixture), hostkey: git2go.HostkeyCertificate{Kind: git2go.HostkeySHA1 | git2go.HostkeyMD5, HashSHA1: sha1Fingerprint("v2toJdKXfFEaR1u++4iq1UqSrHM")}, expectedHost: "github.com:22",