From b490a6aae8620f3b978c8fc641d02b5ebcc5addf Mon Sep 17 00:00:00 2001 From: Paulo Gomes Date: Thu, 16 Jun 2022 11:07:44 +0100 Subject: [PATCH] libgit2: improve known_hosts error messages Known hosts can be a difficult problem to troubleshoot. To make it easier for end users, the generic message has now been changed with a much more user friendly one. Now if a known_host is not set, an error message will be returned, instead of it simply being ignored. Signed-off-by: Paulo Gomes --- pkg/git/libgit2/managed/ssh.go | 17 +-------- pkg/git/libgit2/managed/ssh_test.go | 13 ++++++- pkg/git/libgit2/managed/transport.go | 44 ++++++++++++++-------- pkg/git/libgit2/managed/transport_test.go | 45 ++++++++++++++++++++++- 4 files changed, 86 insertions(+), 33 deletions(-) diff --git a/pkg/git/libgit2/managed/ssh.go b/pkg/git/libgit2/managed/ssh.go index 1c11afe86..5081241bc 100644 --- a/pkg/git/libgit2/managed/ssh.go +++ b/pkg/git/libgit2/managed/ssh.go @@ -191,21 +191,8 @@ func (t *sshSmartSubtransport) Action(transportOptionsURL string, action git2go. } sshConfig.HostKeyCallback = func(hostname string, remote net.Addr, key ssh.PublicKey) error { - marshaledKey := key.Marshal() - cert := &git2go.Certificate{ - Kind: git2go.CertificateHostkey, - Hostkey: git2go.HostkeyCertificate{ - Kind: git2go.HostkeySHA256 | git2go.HostkeyRaw, - HashSHA256: sha256.Sum256(marshaledKey), - Hostkey: marshaledKey, - SSHPublicKey: key, - }, - } - - if len(opts.AuthOpts.KnownHosts) > 0 { - return KnownHostsCallback(hostname, opts.AuthOpts.KnownHosts)(cert, true, hostname) - } - return nil + keyHash := sha256.Sum256(key.Marshal()) + return CheckKnownHost(hostname, opts.AuthOpts.KnownHosts, keyHash[:]) } if t.connected { diff --git a/pkg/git/libgit2/managed/ssh_test.go b/pkg/git/libgit2/managed/ssh_test.go index 0d18c1a83..a6e0fd4c4 100644 --- a/pkg/git/libgit2/managed/ssh_test.go +++ b/pkg/git/libgit2/managed/ssh_test.go @@ -17,9 +17,11 @@ limitations under the License. package managed import ( + "net/url" "os" "path/filepath" "testing" + "time" "github.com/fluxcd/pkg/ssh" "github.com/fluxcd/source-controller/pkg/git" @@ -97,13 +99,20 @@ func TestSSHManagedTransport_E2E(t *testing.T) { err = server.InitRepo("../../testdata/git/repo", git.DefaultBranch, repoPath) g.Expect(err).ToNot(HaveOccurred()) + u, err := url.Parse(server.SSHAddress()) + g.Expect(err).NotTo(HaveOccurred()) + g.Expect(u.Host).ToNot(BeEmpty()) + knownhosts, err := ssh.ScanHostKey(u.Host, 5*time.Second, git.HostKeyAlgos, false) + g.Expect(err).NotTo(HaveOccurred()) + transportOptsURL := "ssh://git@fake-url" sshAddress := server.SSHAddress() + "/" + repoPath AddTransportOptions(transportOptsURL, TransportOptions{ TargetURL: sshAddress, AuthOpts: &git.AuthOptions{ - Username: "user", - Identity: kp.PrivateKey, + Username: "user", + Identity: kp.PrivateKey, + KnownHosts: knownhosts, }, }) diff --git a/pkg/git/libgit2/managed/transport.go b/pkg/git/libgit2/managed/transport.go index 5f6202366..ba4c5b338 100644 --- a/pkg/git/libgit2/managed/transport.go +++ b/pkg/git/libgit2/managed/transport.go @@ -1,6 +1,7 @@ package managed import ( + "encoding/base64" "fmt" "net" @@ -14,11 +15,6 @@ import ( // git.SSH Transports. func KnownHostsCallback(host string, knownHosts []byte) git2go.CertificateCheckCallback { return func(cert *git2go.Certificate, valid bool, hostname string) error { - kh, err := pkgkh.ParseKnownHosts(string(knownHosts)) - if err != nil { - 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. hostWithoutPort, _, err := net.SplitHostPort(host) @@ -47,18 +43,36 @@ func KnownHostsCallback(host string, knownHosts []byte) git2go.CertificateCheckC return fmt.Errorf("invalid host key kind, expected to be of kind SHA256") } - // 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) - for _, k := range kh { - if k.Matches(h, fingerprint) { - return nil - } + return CheckKnownHost(host, knownHosts, fingerprint) + } +} + +// CheckKnownHost checks whether the host being connected to is +// part of the known_hosts, and if so, it ensures the host +// fingerprint matches the fingerprint of the known host with +// the same name. +func CheckKnownHost(host string, knownHosts []byte, fingerprint []byte) error { + kh, err := pkgkh.ParseKnownHosts(string(knownHosts)) + if err != nil { + return fmt.Errorf("failed to parse known_hosts: %w", err) + } + + if len(kh) == 0 { + return fmt.Errorf("hostkey verification aborted: no known_hosts found") + } + + // 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) + for _, k := range kh { + if k.Matches(h, fingerprint) { + return nil } - return fmt.Errorf("hostkey could not be verified") } + return fmt.Errorf("no entries in known_hosts match host '%s' with fingerprint '%s'", + h, base64.RawStdEncoding.EncodeToString(fingerprint)) } // RemoteCallbacks constructs git2go.RemoteCallbacks with dummy callbacks. diff --git a/pkg/git/libgit2/managed/transport_test.go b/pkg/git/libgit2/managed/transport_test.go index 59dfe3bd6..7e68cd4d0 100644 --- a/pkg/git/libgit2/managed/transport_test.go +++ b/pkg/git/libgit2/managed/transport_test.go @@ -16,6 +16,17 @@ github.com ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAA github.com ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIOMqqnkVzrm0SdG6UOoqKLsabgH5C9okWi0dh2l9GKJl ` +// To fetch latest knownhosts for source.developers.google.com run: +// ssh-keyscan -p 2022 source.developers.google.com +// +// Expected hash (used in the cases) can get found with: +// ssh-keyscan -p 2022 source.developers.google.com | ssh-keygen -l -f - +var knownHostsFixtureWithPort = `[source.developers.google.com]:2022 ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBB5Iy4/cq/gt/fPqe3uyMy4jwv1Alc94yVPxmnwNhBzJqEV5gRPiRk5u4/JJMbbu9QUVAguBABxL7sBZa5PH/xY=` + +// This is an incorrect known hosts entry, that does not aligned with +// the normalized format and therefore won't match. +var knownHostsFixtureUnormalized = `source.developers.google.com:2022 ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBB5Iy4/cq/gt/fPqe3uyMy4jwv1Alc94yVPxmnwNhBzJqEV5gRPiRk5u4/JJMbbu9QUVAguBABxL7sBZa5PH/xY=` + func TestKnownHostsCallback(t *testing.T) { tests := []struct { name string @@ -25,6 +36,38 @@ func TestKnownHostsCallback(t *testing.T) { hostkey git2go.HostkeyCertificate want error }{ + { + name: "Empty", + host: "source.developers.google.com", + knownHosts: []byte(""), + hostkey: git2go.HostkeyCertificate{Kind: git2go.HostkeySHA256, HashSHA256: sha256Fingerprint("AGvEpqYNMqsRNIviwyk4J4HM0lEylomDBKOWZsBn434")}, + expectedHost: "source.developers.google.com:2022", + want: fmt.Errorf("hostkey verification aborted: no known_hosts found"), + }, + { + name: "Mismatch incorrect known_hosts", + host: "source.developers.google.com", + knownHosts: []byte(knownHostsFixtureUnormalized), + hostkey: git2go.HostkeyCertificate{Kind: git2go.HostkeySHA256, HashSHA256: sha256Fingerprint("AGvEpqYNMqsRNIviwyk4J4HM0lEylomDBKOWZsBn434")}, + expectedHost: "source.developers.google.com:2022", + want: fmt.Errorf("no entries in known_hosts match host '[source.developers.google.com]:2022' with fingerprint 'AGvEpqYNMqsRNIviwyk4J4HM0lEylomDBKOWZsBn434'"), + }, + { + name: "Match when host has port", + host: "source.developers.google.com:2022", + knownHosts: []byte(knownHostsFixtureWithPort), + hostkey: git2go.HostkeyCertificate{Kind: git2go.HostkeySHA256, HashSHA256: sha256Fingerprint("AGvEpqYNMqsRNIviwyk4J4HM0lEylomDBKOWZsBn434")}, + expectedHost: "source.developers.google.com:2022", + want: nil, + }, + { + name: "Match even when host does not have port", + host: "source.developers.google.com", + knownHosts: []byte(knownHostsFixtureWithPort), + hostkey: git2go.HostkeyCertificate{Kind: git2go.HostkeySHA256, HashSHA256: sha256Fingerprint("AGvEpqYNMqsRNIviwyk4J4HM0lEylomDBKOWZsBn434")}, + expectedHost: "source.developers.google.com:2022", + want: nil, + }, { name: "Match", host: "github.com", @@ -66,7 +109,7 @@ func TestKnownHostsCallback(t *testing.T) { knownHosts: []byte(knownHostsFixture), hostkey: git2go.HostkeyCertificate{Kind: git2go.HostkeySHA256, HashSHA256: sha256Fingerprint("ROQFvPThGrW4RuWLoL9tq9I9zJ42fK4XywyRtbOz/EQ")}, expectedHost: "github.com", - want: fmt.Errorf("hostkey could not be verified"), + want: fmt.Errorf("no entries in known_hosts match host 'github.com' with fingerprint 'ROQFvPThGrW4RuWLoL9tq9I9zJ42fK4XywyRtbOz/EQ'"), }, } for _, tt := range tests {