Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

libgit2: update to v1.3.0 #465

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/actions/run-tests/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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'.
Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion controllers/gitrepository_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
10 changes: 5 additions & 5 deletions pkg/git/libgit2/checkout.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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),
},
Expand Down Expand Up @@ -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),
},
Expand All @@ -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),
},
Expand Down Expand Up @@ -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),
},
Expand Down
2 changes: 1 addition & 1 deletion pkg/git/libgit2/checkout_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import (
"testing"
"time"

git2go "github.com/libgit2/git2go/v31"
git2go "github.com/libgit2/git2go/v33"
. "github.com/onsi/gomega"
)

Expand Down
34 changes: 12 additions & 22 deletions pkg/git/libgit2/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,10 @@ import (
"crypto/x509"
"fmt"
"hash"
"net"
"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"

Expand Down Expand Up @@ -100,10 +99,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{
Expand All @@ -112,48 +111,39 @@ 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
}
}

// 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
}

// 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
return fmt.Errorf("failed to parse known_hosts: %w", err)
}

// Check if the configured host matches the hostname given to
// the callback.
if h != hostname {
return git2go.ErrorCodeUser
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 git2go.ErrorCodeOK
return nil
}
}
return git2go.ErrorCodeCertificate
return fmt.Errorf("failed to lookup known_hosts entry for '%s'", hostname)
}
}

Expand Down
39 changes: 24 additions & 15 deletions pkg/git/libgit2/transport_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import (
"testing"
"time"

git2go "github.com/libgit2/git2go/v31"
git2go "github.com/libgit2/git2go/v33"
. "github.com/onsi/gomega"
)

Expand Down Expand Up @@ -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 {
Expand All @@ -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())
})
}
}
Expand All @@ -205,39 +210,37 @@ func Test_knownHostsCallback(t *testing.T) {
expectedHost string
knownHosts []byte
hostkey git2go.HostkeyCertificate
want git2go.ErrorCode
wantErr string
}{
{
name: "Match",
host: "github.com",
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",
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",
want: git2go.ErrorCodeOK,
},
{
name: "Hostname mismatch",
host: "github.com",
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",
host: "github.com",
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 {
Expand All @@ -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())
})
}
}
Expand Down