From b2f6639c01d099cb6a4ba82cbf817cec9506c95f Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Wed, 10 Feb 2021 16:17:29 +0100 Subject: [PATCH] libgit2: use provided host to validate public key The callback from libgit2 only provides a hostname (without the port), but the `known_hosts` file indexes the public keys based on the full host (e.g. `[localhost]:123` for a host behind a specific port). As a result, it was unable to find the correct public key for the hostname when it was added to the `known_hosts` file with the port. To work around this, we add the user provided host that includes the port to the `PublicKeyAuth` strategy, and use this to find the right entry in the `known_hosts` file, after having validated that the hostname provided to the callback matches the hostname of the host provided by the user. Signed-off-by: Hidde Beydals --- pkg/git/libgit2/transport.go | 42 +++++++++++++++++++++++-------- pkg/git/libgit2/transport_test.go | 4 +-- 2 files changed, 34 insertions(+), 12 deletions(-) diff --git a/pkg/git/libgit2/transport.go b/pkg/git/libgit2/transport.go index 67f29d349..74fd317e4 100644 --- a/pkg/git/libgit2/transport.go +++ b/pkg/git/libgit2/transport.go @@ -22,12 +22,13 @@ import ( "crypto/sha1" "crypto/x509" "fmt" + "net" "net/url" "strings" - "golang.org/x/crypto/ssh" - git2go "github.com/libgit2/git2go/v31" + "golang.org/x/crypto/ssh" + "golang.org/x/crypto/ssh/knownhosts" corev1 "k8s.io/api/core/v1" "github.com/fluxcd/source-controller/pkg/git" @@ -43,7 +44,7 @@ func AuthSecretStrategyForURL(URL string) (git.AuthSecretStrategy, error) { case u.Scheme == "http", u.Scheme == "https": return &BasicAuth{}, nil case u.Scheme == "ssh": - return &PublicKeyAuth{user: u.User.Username()}, nil + return &PublicKeyAuth{user: u.User.Username(), host: u.Host}, nil default: return nil, fmt.Errorf("no auth secret strategy for scheme %s", u.Scheme) } @@ -62,7 +63,7 @@ func (s *BasicAuth) Method(secret corev1.Secret) (*git.Auth, error) { password = string(d) } if username != "" && password != "" { - credCallback = func(url string, username_from_url string, allowed_types git2go.CredType) (*git2go.Cred, error) { + credCallback = func(url string, usernameFromURL string, allowedTypes git2go.CredType) (*git2go.Cred, error) { cred, err := git2go.NewCredUserpassPlaintext(username, password) if err != nil { return nil, err @@ -97,11 +98,12 @@ func (s *BasicAuth) Method(secret corev1.Secret) (*git.Auth, error) { type PublicKeyAuth struct { user string + host string } func (s *PublicKeyAuth) Method(secret corev1.Secret) (*git.Auth, error) { if _, ok := secret.Data[git.CAFile]; ok { - return nil, fmt.Errorf("found caFile key in secret '%s' but libgit2 SSH transport does not support custom certificates", secret.Name) + return nil, fmt.Errorf("found %s key in secret '%s' but libgit2 SSH transport does not support custom certificates", git.CAFile, secret.Name) } identity := secret.Data["identity"] knownHosts := secret.Data["known_hosts"] @@ -126,7 +128,7 @@ func (s *PublicKeyAuth) Method(secret corev1.Secret) (*git.Auth, error) { user = git.DefaultPublicKeyAuthUser } - credCallback := func(url string, username_from_url string, allowed_types git2go.CredType) (*git2go.Cred, error) { + credCallback := func(url string, usernameFromURL string, allowedTypes git2go.CredType) (*git2go.Cred, error) { cred, err := git2go.NewCredSshKeyFromMemory(user, "", string(identity), "") if err != nil { return nil, err @@ -134,12 +136,32 @@ func (s *PublicKeyAuth) Method(secret corev1.Secret) (*git.Auth, error) { return cred, nil } certCallback := func(cert *git2go.Certificate, valid bool, hostname string) git2go.ErrorCode { + // First, attempt to split the configured host and port to validate + // the port-less hostname given to the callback. + host, _, err := net.SplitHostPort(s.host) + if err != nil { + // SplitHostPort returns an error if the host is missing + // a port, assume the host has no port. + host = s.host + } + + // Check if the configured host matches the hostname given to + // the callback. + if host != hostname { + return git2go.ErrUser + } + + // 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. + host = knownhosts.Normalize(s.host) for _, k := range kk { - if k.matches(hostname, cert.Hostkey.HashSHA1[:]) { + if k.matches(host, cert.Hostkey.HashSHA1[:]) { return git2go.ErrOk } } - return git2go.ErrGeneric + return git2go.ErrCertificate } return &git.Auth{CredCallback: credCallback, CertCallback: certCallback}, nil @@ -151,7 +173,7 @@ type knownKey struct { } func parseKnownHosts(s string) ([]knownKey, error) { - knownHosts := []knownKey{} + var knownHosts []knownKey scanner := bufio.NewScanner(strings.NewReader(s)) for scanner.Scan() { _, hosts, pubKey, _, _, err := ssh.ParseKnownHosts(scanner.Bytes()) @@ -178,7 +200,7 @@ func (k knownKey) matches(host string, key []byte) bool { return false } - hash := sha1.Sum([]byte(k.key.Marshal())) + hash := sha1.Sum(k.key.Marshal()) if bytes.Compare(hash[:], key) != 0 { return false } diff --git a/pkg/git/libgit2/transport_test.go b/pkg/git/libgit2/transport_test.go index 2897e92d2..7a2dcd310 100644 --- a/pkg/git/libgit2/transport_test.go +++ b/pkg/git/libgit2/transport_test.go @@ -73,8 +73,8 @@ func TestAuthSecretStrategyForURL(t *testing.T) { }{ {"HTTP", "http://git.example.com/org/repo.git", &BasicAuth{}, false}, {"HTTPS", "https://git.example.com/org/repo.git", &BasicAuth{}, false}, - {"SSH", "ssh://git.example.com:2222/org/repo.git", &PublicKeyAuth{}, false}, - {"SSH with username", "ssh://example@git.example.com:2222/org/repo.git", &PublicKeyAuth{user: "example"}, false}, + {"SSH", "ssh://git.example.com:2222/org/repo.git", &PublicKeyAuth{host: "git.example.com:2222"}, false}, + {"SSH with username", "ssh://example@git.example.com:2222/org/repo.git", &PublicKeyAuth{user: "example", host: "git.example.com:2222"}, false}, {"unsupported", "protocol://example.com", nil, true}, } for _, tt := range tests {