From 842970899702a8c2105202a7c3526785b3ed827d Mon Sep 17 00:00:00 2001 From: Paulo Gomes Date: Wed, 16 Feb 2022 11:39:11 +0000 Subject: [PATCH] Upgrade libgit2 to libgit2-1.3.0-2 Signed-off-by: Paulo Gomes --- Dockerfile | 2 +- Makefile | 9 ++++++++- hack/install-libraries.sh | 2 -- pkg/git/libgit2/checkout_test.go | 5 +++-- pkg/git/libgit2/transport.go | 10 +++++----- pkg/git/libgit2/transport_test.go | 20 ++++++++++---------- tests/fuzz/gitrepository_fuzzer.go | 9 +++++---- tests/fuzz/oss_fuzz_build.sh | 20 ++++++++++++++++++-- 8 files changed, 50 insertions(+), 27 deletions(-) diff --git a/Dockerfile b/Dockerfile index acf4f2866..3f9802f1b 100644 --- a/Dockerfile +++ b/Dockerfile @@ -3,7 +3,7 @@ ARG GO_VERSION=1.17 ARG XX_VERSION=1.1.0 ARG LIBGIT2_IMG=ghcr.io/fluxcd/golang-with-libgit2 -ARG LIBGIT2_TAG=libgit2-1.1.1-7 +ARG LIBGIT2_TAG=libgit2-1.3.0-2 FROM ${LIBGIT2_IMG}:${LIBGIT2_TAG} AS libgit2-libs diff --git a/Makefile b/Makefile index 02cd94022..3b0e5b876 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-7 +LIBGIT2_TAG ?= libgit2-1.3.0-2 # Allows for defining additional Docker buildx arguments, # e.g. '--push'. @@ -136,6 +136,7 @@ tidy: ## Run go mod tidy fmt: ## Run go fmt against code go fmt ./... cd api; go fmt ./... + cd tests/fuzz; go fmt . vet: $(LIBGIT2) ## Run go vet against code go vet ./... @@ -208,6 +209,12 @@ ifneq ($(shell grep -o 'LIBGIT2_IMG ?= \w.*' Makefile | cut -d ' ' -f 3):$(shell exit 1; \ } endif +ifneq ($(shell grep -o 'LIBGIT2_TAG ?= \w.*' Makefile | cut -d ' ' -f 3), $(shell grep -o "LIBGIT2_TAG=.*" tests/fuzz/oss_fuzz_build.sh | sed 's;LIBGIT2_TAG="$${LIBGIT2_TAG:-;;g' | sed 's;}";;g')) + @{ \ + echo "LIBGIT2_TAG must match in both Makefile and tests/fuzz/oss_fuzz_build.sh"; \ + exit 1; \ + } +endif ifneq (, $(shell git status --porcelain --untracked-files=no)) @{ \ echo "working directory is dirty:"; \ diff --git a/hack/install-libraries.sh b/hack/install-libraries.sh index 270ce1915..70866eea1 100755 --- a/hack/install-libraries.sh +++ b/hack/install-libraries.sh @@ -45,8 +45,6 @@ function setup_current() { mkdir -p "./build/libgit2" if [[ $OSTYPE == 'darwin'* ]]; then # For MacOS development environments, download the amd64 static libraries released from from golang-with-libgit2. - - #TODO: update URL with official URL + TAG: curl -o output.tar.gz -LO "https://github.com/fluxcd/golang-with-libgit2/releases/download/${TAG}/darwin-libs.tar.gz" DIR=libgit2-darwin diff --git a/pkg/git/libgit2/checkout_test.go b/pkg/git/libgit2/checkout_test.go index 7d96eb1b6..ff2f5ccd5 100644 --- a/pkg/git/libgit2/checkout_test.go +++ b/pkg/git/libgit2/checkout_test.go @@ -26,12 +26,13 @@ import ( "testing" "time" - "github.com/fluxcd/pkg/gittestserver" - "github.com/fluxcd/pkg/ssh" git2go "github.com/libgit2/git2go/v33" . "github.com/onsi/gomega" corev1 "k8s.io/api/core/v1" + "github.com/fluxcd/pkg/gittestserver" + "github.com/fluxcd/pkg/ssh" + "github.com/fluxcd/source-controller/pkg/git" ) diff --git a/pkg/git/libgit2/transport.go b/pkg/git/libgit2/transport.go index 41ea151a4..22efa054a 100644 --- a/pkg/git/libgit2/transport.go +++ b/pkg/git/libgit2/transport.go @@ -68,7 +68,7 @@ func transferProgressCallback(ctx context.Context) git2go.TransferProgressCallba } select { case <-ctx.Done(): - return fmt.Errorf("transport close - potentially due to a timeout") + return fmt.Errorf("transport close (potentially due to a timeout)") default: return nil } @@ -100,7 +100,7 @@ func pushTransferProgressCallback(ctx context.Context) git2go.PushTransferProgre } select { case <-ctx.Done(): - return fmt.Errorf("transport close - potentially due to a timeout") + return fmt.Errorf("transport close (potentially due to a timeout)") default: return nil } @@ -158,7 +158,7 @@ func x509Callback(caBundle []byte) git2go.CertificateCheckCallback { return func(cert *git2go.Certificate, valid bool, hostname string) error { roots := x509.NewCertPool() if ok := roots.AppendCertsFromPEM(caBundle); !ok { - return fmt.Errorf("x509 cert could not be appended") + return fmt.Errorf("PEM CA bundle could not be appended to x509 certificate pool") } opts := x509.VerifyOptions{ @@ -167,7 +167,7 @@ func x509Callback(caBundle []byte) git2go.CertificateCheckCallback { CurrentTime: now(), } if _, err := cert.X509.Verify(opts); err != nil { - return fmt.Errorf("x509 cert could not be verified") + return fmt.Errorf("verification failed: %w", err) } return nil } @@ -200,7 +200,7 @@ func knownHostsCallback(host string, knownHosts []byte) git2go.CertificateCheckC } if hostnameWithoutPort != hostWithoutPort { - return fmt.Errorf("host mismatch: %q %q\n", hostWithoutPort, hostnameWithoutPort) + return fmt.Errorf("host mismatch: %q %q", hostWithoutPort, hostnameWithoutPort) } // We are now certain that the configured host and the hostname diff --git a/pkg/git/libgit2/transport_test.go b/pkg/git/libgit2/transport_test.go index a5b330aeb..0028fad58 100644 --- a/pkg/git/libgit2/transport_test.go +++ b/pkg/git/libgit2/transport_test.go @@ -159,28 +159,28 @@ func Test_x509Callback(t *testing.T) { certificate: googleLeafWithInvalidHashFixture, host: "www.google.com", caBundle: []byte(giag2IntermediateFixture + "\n" + geoTrustRootFixture), - want: fmt.Errorf("x509 cert could not be verified"), + want: fmt.Errorf(`verification failed: x509: certificate signed by unknown authority (possibly because of "x509: cannot verify signature: algorithm unimplemented" while trying to verify candidate authority certificate "Google Internet Authority G2")`), }, { name: "Invalid certificate authority bundle", certificate: googleLeafFixture, host: "www.google.com", caBundle: bytes.Trim([]byte(giag2IntermediateFixture+"\n"+geoTrustRootFixture), "-"), - want: fmt.Errorf("x509 cert could not be appended"), + want: fmt.Errorf("PEM CA bundle could not be appended to x509 certificate pool"), }, { name: "Missing intermediate in bundle", certificate: googleLeafFixture, host: "www.google.com", caBundle: []byte(geoTrustRootFixture), - want: fmt.Errorf("x509 cert could not be verified"), + want: fmt.Errorf("verification failed: x509: certificate signed by unknown authority"), }, { name: "Invalid host", certificate: googleLeafFixture, host: "www.google.co", caBundle: []byte(giag2IntermediateFixture + "\n" + geoTrustRootFixture), - want: fmt.Errorf("x509 cert could not be verified"), + want: fmt.Errorf("verification failed: x509: certificate is valid for www.google.com, not www.google.co"), }, } for _, tt := range tests { @@ -195,11 +195,11 @@ func Test_x509Callback(t *testing.T) { } callback := x509Callback(tt.caBundle) - result := g.Expect(callback(cert, false, tt.host)) + result := callback(cert, false, tt.host) if tt.want == nil { - result.To(BeNil()) + g.Expect(result).To(BeNil()) } else { - result.To(Equal(tt.want)) + g.Expect(result.Error()).To(Equal(tt.want.Error())) } }) } @@ -236,7 +236,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: fmt.Errorf("host mismatch: %q %q\n", "example.com", "github.com"), + want: fmt.Errorf("host mismatch: %q %q", "example.com", "github.com"), }, { name: "Hostkey mismatch", @@ -399,7 +399,7 @@ func Test_transferProgressCallback(t *testing.T) { ReceivedObjects: 21, }, cancelFunc: func(cf context.CancelFunc) { cf() }, - wantErr: fmt.Errorf("transport close - potentially due to a timeout"), + wantErr: fmt.Errorf("transport close (potentially due to a timeout)"), }, } @@ -497,7 +497,7 @@ func Test_pushTransferProgressCallback(t *testing.T) { name: "error - context cancelled", progress: pushProgress{current: 20, total: 25}, cancelFunc: func(cf context.CancelFunc) { cf() }, - wantErr: fmt.Errorf("transport close - potentially due to a timeout"), + wantErr: fmt.Errorf("transport close (potentially due to a timeout)"), }, } diff --git a/tests/fuzz/gitrepository_fuzzer.go b/tests/fuzz/gitrepository_fuzzer.go index 9ccc0fdf0..01c4cc949 100644 --- a/tests/fuzz/gitrepository_fuzzer.go +++ b/tests/fuzz/gitrepository_fuzzer.go @@ -38,10 +38,6 @@ import ( "time" fuzz "github.com/AdaLogics/go-fuzz-headers" - "github.com/fluxcd/pkg/gittestserver" - "github.com/fluxcd/pkg/runtime/testenv" - sourcev1 "github.com/fluxcd/source-controller/api/v1beta1" - "github.com/fluxcd/source-controller/controllers" "github.com/go-git/go-billy/v5" "github.com/go-git/go-billy/v5/memfs" "github.com/go-git/go-git/v5" @@ -61,6 +57,11 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/envtest" "sigs.k8s.io/controller-runtime/pkg/manager" + + "github.com/fluxcd/pkg/gittestserver" + "github.com/fluxcd/pkg/runtime/testenv" + sourcev1 "github.com/fluxcd/source-controller/api/v1beta1" + "github.com/fluxcd/source-controller/controllers" ) var ( diff --git a/tests/fuzz/oss_fuzz_build.sh b/tests/fuzz/oss_fuzz_build.sh index b70ceca2f..2878342a1 100755 --- a/tests/fuzz/oss_fuzz_build.sh +++ b/tests/fuzz/oss_fuzz_build.sh @@ -16,7 +16,7 @@ set -euxo pipefail -LIBGIT2_TAG="${LIBGIT2_TAG:-libgit2-1.1.1-7}" +LIBGIT2_TAG="${LIBGIT2_TAG:-libgit2-1.3.0-2}" GOPATH="${GOPATH:-/root/go}" GO_SRC="${GOPATH}/src" PROJECT_PATH="github.com/fluxcd/source-controller" @@ -54,10 +54,25 @@ export PKG_CONFIG_PATH="${TARGET_DIR}/lib/pkgconfig:${TARGET_DIR}/lib64/pkgconfi export CGO_CFLAGS="-I${TARGET_DIR}/include -I${TARGET_DIR}/include/openssl" export CGO_LDFLAGS="$(pkg-config --libs --static --cflags libssh2 openssl libgit2)" -go mod tidy -compat=1.17 +go mod tidy + +# The implementation of libgit2 is sensitive to the versions of git2go. +# Leaving it to its own devices, the minimum version of git2go used may not +# be compatible with the currently implemented version. Hence the modifications +# of the existing go.mod. +sed "s;\./api;$(/bin/pwd)/api;g" go.mod > tests/fuzz/go.mod +sed -i 's;module github.com/fluxcd/source-controller;module github.com/fluxcd/source-controller/tests/fuzz;g' tests/fuzz/go.mod +echo "replace github.com/fluxcd/source-controller => $(/bin/pwd)/" >> tests/fuzz/go.mod + +cp go.sum tests/fuzz/go.sum pushd "tests/fuzz" +go mod download + +go get -d github.com/AdaLogics/go-fuzz-headers +go get -d github.com/fluxcd/source-controller + # Setup files to be embedded into controllers_fuzzer.go's testFiles variable. mkdir -p testdata/crd cp ../../config/crd/bases/*.yaml testdata/crd/ @@ -89,6 +104,7 @@ go_compile FuzzRandomGitFiles fuzz_gitrepository_fuzzer go_compile FuzzGitResourceObject fuzz_git_resource_object # By now testdata is embedded in the binaries and no longer needed. +# Remove the dir given that it will be owned by root otherwise. rm -rf testdata/ popd