Merge pull request #573 from pjbgf/bump-libgit2

hiddeco · web-flow · commit 2c4818fda562 · 2022-02-16T13:55:42.000+01:00
diff --git a/.github/workflows/e2e.yaml b/.github/workflows/e2e.yaml
@@ -57,11 +57,7 @@ jobs:
         with:
           go-version: 1.17.x
       - name: Run tests
-        run: |
-          mkdir tmp-download; cd tmp-download; go mod init go-download;
-          GOBIN="${GITHUB_WORKSPACE}/build/gobin" go install sigs.k8s.io/controller-runtime/tools/setup-envtest@latest
-          cd ..; rm -rf tmp-download
-          make test
+        run: make test
       - name: Prepare
         id: prep
         run: |
diff --git a/ATTRIBUTIONS.md b/ATTRIBUTIONS.md
@@ -1203,6 +1203,18 @@ STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
 ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED
 OF THE POSSIBILITY OF SUCH DAMAGE.
 
+----------------------------------------------------------------------
+
+The xoroshiro256** implementation is licensed in the public domain:
+
+Written in 2018 by David Blackman and Sebastiano Vigna (vigna@acm.org)
+
+To the extent possible under law, the author has dedicated all copyright
+and related and neighboring rights to this software to the public domain
+worldwide. This software is distributed without any warranty.
+
+See <http://creativecommons.org/publicdomain/zero/1.0/>.
+
 ***
 
 ## zlib
diff --git 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
@@ -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:"; \
@@ -224,7 +231,7 @@ TMP_DIR=$$(mktemp -d) ;\
 cd $$TMP_DIR ;\
 go mod init tmp ;\
 echo "Downloading $(2)" ;\
-go install $(2) ;\
+env -i bash -c "GOBIN=$(GOBIN) PATH=$(PATH) GOPATH=$(shell go env GOPATH) GOCACHE=$(shell go env GOCACHE) go install $(2)" ;\
 rm -rf $$TMP_DIR ;\
 }
 endef
diff --git a/go.mod b/go.mod
@@ -23,7 +23,7 @@ require (
 	github.com/go-git/go-billy/v5 v5.3.1
 	github.com/go-git/go-git/v5 v5.4.2
 	github.com/go-logr/logr v1.2.2
-	github.com/libgit2/git2go/v31 v31.7.6
+	github.com/libgit2/git2go/v33 v33.0.6
 	github.com/minio/minio-go/v7 v7.0.15
 	github.com/onsi/ginkgo v1.16.5
 	github.com/onsi/gomega v1.17.0
diff --git a/go.sum b/go.sum
@@ -622,8 +622,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.7.6 h1:jg/pNomrQULnafmfF6XTkozPX5ypyELoWErWkJuYPcI=
-github.com/libgit2/git2go/v31 v31.7.6/go.mod h1:c/rkJcBcUFx6wHaT++UwNpKvIsmPNqCeQ/vzO4DrEec=
+github.com/libgit2/git2go/v33 v33.0.6 h1:F//bA3/pgSTVq2hLNahhnof9NxyCzFF/c3MB6lb93Qo=
+github.com/libgit2/git2go/v33 v33.0.6/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/lithammer/dedent v1.1.0/go.mod h1:jrXYCQtgg0nJiN+StA2KgR7w6CiQNv9Fd/Z9BP0jIOc=
diff --git 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.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(ctx, opts),
 			ProxyOptions:    git2go.ProxyOptions{Type: git2go.ProxyTypeAuto},
@@ -91,7 +91,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(ctx, opts),
 			ProxyOptions:    git2go.ProxyOptions{Type: git2go.ProxyTypeAuto},
@@ -115,7 +115,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(ctx, opts),
 			ProxyOptions:    git2go.ProxyOptions{Type: git2go.ProxyTypeAuto},
@@ -147,7 +147,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(ctx, opts),
 			ProxyOptions:    git2go.ProxyOptions{Type: git2go.ProxyTypeAuto},
diff --git a/pkg/git/libgit2/checkout_test.go b/pkg/git/libgit2/checkout_test.go
@@ -20,13 +20,20 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"net/url"
 	"os"
 	"path/filepath"
 	"testing"
 	"time"
 
-	git2go "github.com/libgit2/git2go/v31"
+	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"
 )
 
 func TestCheckoutBranch_Checkout(t *testing.T) {
@@ -444,3 +451,68 @@ func mockSignature(time time.Time) *git2go.Signature {
 		When:  time,
 	}
 }
+
+// This test is specifically to detect regression in libgit2's ED25519 key
+// support for client authentication.
+// Refer: https://github.com/fluxcd/source-controller/issues/399
+func TestCheckout_ED25519(t *testing.T) {
+	g := NewWithT(t)
+	timeout := 5 * time.Second
+
+	// Create a git test server.
+	server, err := gittestserver.NewTempGitServer()
+	g.Expect(err).ToNot(HaveOccurred())
+	defer os.RemoveAll(server.Root())
+	server.Auth("test-user", "test-pswd")
+	server.AutoCreate()
+
+	server.KeyDir(filepath.Join(server.Root(), "keys"))
+	g.Expect(server.ListenSSH()).To(Succeed())
+
+	go func() {
+		server.StartSSH()
+	}()
+	defer server.StopSSH()
+
+	repoPath := "test.git"
+
+	err = server.InitRepo("testdata/git/repo", git.DefaultBranch, repoPath)
+	g.Expect(err).NotTo(HaveOccurred())
+
+	sshURL := server.SSHAddress()
+	repoURL := sshURL + "/" + repoPath
+
+	// Fetch host key.
+	u, err := url.Parse(sshURL)
+	g.Expect(err).NotTo(HaveOccurred())
+	g.Expect(u.Host).ToNot(BeEmpty())
+	knownHosts, err := ssh.ScanHostKey(u.Host, timeout)
+	g.Expect(err).ToNot(HaveOccurred())
+
+	kp, err := ssh.NewEd25519Generator().Generate()
+	g.Expect(err).ToNot(HaveOccurred())
+
+	secret := corev1.Secret{
+		Data: map[string][]byte{
+			"identity":    kp.PrivateKey,
+			"known_hosts": knownHosts,
+		},
+	}
+
+	authOpts, err := git.AuthOptionsFromSecret(repoURL, &secret)
+	g.Expect(err).ToNot(HaveOccurred())
+
+	// Prepare for checkout.
+	branchCheckoutStrat := &CheckoutBranch{Branch: git.DefaultBranch}
+	tmpDir, _ := os.MkdirTemp("", "test")
+	defer os.RemoveAll(tmpDir)
+
+	ctx, cancel := context.WithTimeout(context.TODO(), timeout)
+	defer cancel()
+
+	// Checkout the repo.
+	// This should always fail because the generated key above isn't present in
+	// the git server.
+	_, err = branchCheckoutStrat.Checkout(ctx, tmpDir, repoURL, authOpts)
+	g.Expect(err).To(BeNil())
+}
diff --git a/pkg/git/libgit2/testdata/git/repo/foo.txt b/pkg/git/libgit2/testdata/git/repo/foo.txt
@@ -0,0 +1 @@
+test file
diff --git a/pkg/git/libgit2/transport.go b/pkg/git/libgit2/transport.go
@@ -31,7 +31,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"
 
@@ -61,16 +61,16 @@ func RemoteCallbacks(ctx context.Context, opts *git.AuthOptions) git2go.RemoteCa
 // libgit2 it should stop the transfer when the given context is closed (due to
 // e.g. a timeout).
 func transferProgressCallback(ctx context.Context) git2go.TransferProgressCallback {
-	return func(p git2go.TransferProgress) git2go.ErrorCode {
+	return func(p git2go.TransferProgress) error {
 		// Early return if all the objects have been received.
 		if p.ReceivedObjects == p.TotalObjects {
-			return git2go.ErrorCodeOK
+			return nil
 		}
 		select {
 		case <-ctx.Done():
-			return git2go.ErrorCodeUser
+			return fmt.Errorf("transport close (potentially due to a timeout)")
 		default:
-			return git2go.ErrorCodeOK
+			return nil
 		}
 	}
 }
@@ -79,12 +79,12 @@ func transferProgressCallback(ctx context.Context) git2go.TransferProgressCallba
 // libgit2 it should cancel the network operation when the given context is
 // closed.
 func transportMessageCallback(ctx context.Context) git2go.TransportMessageCallback {
-	return func(_ string) git2go.ErrorCode {
+	return func(_ string) error {
 		select {
 		case <-ctx.Done():
-			return git2go.ErrorCodeUser
+			return fmt.Errorf("transport closed")
 		default:
-			return git2go.ErrorCodeOK
+			return nil
 		}
 	}
 }
@@ -93,16 +93,16 @@ func transportMessageCallback(ctx context.Context) git2go.TransportMessageCallba
 // signals libgit2 it should stop the push transfer when the given context is
 // closed (due to e.g. a timeout).
 func pushTransferProgressCallback(ctx context.Context) git2go.PushTransferProgressCallback {
-	return func(current, total uint32, _ uint) git2go.ErrorCode {
+	return func(current, total uint32, _ uint) error {
 		// Early return if current equals total.
 		if current == total {
-			return git2go.ErrorCodeOK
+			return nil
 		}
 		select {
 		case <-ctx.Done():
-			return git2go.ErrorCodeUser
+			return fmt.Errorf("transport close (potentially due to a timeout)")
 		default:
-			return git2go.ErrorCodeOK
+			return nil
 		}
 	}
 }
@@ -155,10 +155,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("PEM CA bundle could not be appended to x509 certificate pool")
 		}
 
 		opts := x509.VerifyOptions{
@@ -167,20 +167,20 @@ func x509Callback(caBundle []byte) git2go.CertificateCheckCallback {
 			CurrentTime: now(),
 		}
 		if _, err := cert.X509.Verify(opts); err != nil {
-			return git2go.ErrorCodeCertificate
+			return fmt.Errorf("verification failed: %w", err)
 		}
-		return git2go.ErrorCodeOK
+		return nil
 	}
 }
 
 // knownHostCallback returns a CertificateCheckCallback that verifies
 // 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
@@ -200,7 +200,7 @@ func knownHostsCallback(host string, knownHosts []byte) git2go.CertificateCheckC
 		}
 
 		if hostnameWithoutPort != hostWithoutPort {
-			return git2go.ErrorCodeUser
+			return fmt.Errorf("host mismatch: %q %q", hostWithoutPort, hostnameWithoutPort)
 		}
 
 		// We are now certain that the configured host and the hostname
@@ -210,10 +210,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("hostkey could not be verified")
 	}
 }
 
diff --git a/pkg/git/libgit2/transport_test.go b/pkg/git/libgit2/transport_test.go
diff --git a/tests/fuzz/gitrepository_fuzzer.go b/tests/fuzz/gitrepository_fuzzer.go
diff --git a/tests/fuzz/oss_fuzz_build.sh b/tests/fuzz/oss_fuzz_build.sh
