Merge pull request #771 from aryan9600/fix-hostcallback

libgit2: fix ssh host key verification regression
fluxcd · Jun 8, 2022 · d48f4b4 · d48f4b4
2 parents 2031c84 + 4769253
commit d48f4b4
Show file tree

Hide file tree

Showing 4 changed files with 19 additions and 20 deletions.
diff --git a/go.mod b/go.mod
@@ -28,7 +28,7 @@ require (
 	github.com/fluxcd/pkg/helmtestserver v0.7.3
 	github.com/fluxcd/pkg/lockedfile v0.1.0
 	github.com/fluxcd/pkg/runtime v0.16.1
-	github.com/fluxcd/pkg/ssh v0.4.1
+	github.com/fluxcd/pkg/ssh v0.5.0
 	github.com/fluxcd/pkg/testserver v0.2.0
 	github.com/fluxcd/pkg/untar v0.1.0
 	github.com/fluxcd/pkg/version v0.1.0

diff --git a/go.sum b/go.sum
@@ -282,8 +282,8 @@ github.com/fluxcd/pkg/lockedfile v0.1.0 h1:YsYFAkd6wawMCcD74ikadAKXA4s2sukdxrn7w
 github.com/fluxcd/pkg/lockedfile v0.1.0/go.mod h1:EJLan8t9MiOcgTs8+puDjbE6I/KAfHbdvIy9VUgIjm8=
 github.com/fluxcd/pkg/runtime v0.16.1 h1:WU1vNZz4TAzmATQ/tl2zB/FX6GIUTgYeBn/G5RuTA2c=
 github.com/fluxcd/pkg/runtime v0.16.1/go.mod h1:cgVJkOXCg9OmrIUGklf/0UtV28MNzkuoBJhaEQICT6E=
-github.com/fluxcd/pkg/ssh v0.4.1 h1:O5FCjb5NIZ9PeRjdF2iL9jaPNM+RL+IjrMBZPkqF9W4=
-github.com/fluxcd/pkg/ssh v0.4.1/go.mod h1:KGgOUOy1uI6RC6+qxIBLvP1AeOOs/nLB25Ca6TZMIXE=
+github.com/fluxcd/pkg/ssh v0.5.0 h1:jE9F2XvUXC2mgseeXMATvO014fLqdB30/VzlPLKsk20=
+github.com/fluxcd/pkg/ssh v0.5.0/go.mod h1:KGgOUOy1uI6RC6+qxIBLvP1AeOOs/nLB25Ca6TZMIXE=
 github.com/fluxcd/pkg/testserver v0.2.0 h1:Mj0TapmKaywI6Fi5wvt1LAZpakUHmtzWQpJNKQ0Krt4=
 github.com/fluxcd/pkg/testserver v0.2.0/go.mod h1:bgjjydkXsZTeFzjz9Cr4heGANr41uTB1Aj1Q5qzuYVk=
 github.com/fluxcd/pkg/untar v0.1.0 h1:k97V/xV5hFrAkIkVPuv5AVhyxh1ZzzAKba/lbDfGo6o=

diff --git a/pkg/git/libgit2/managed/transport.go b/pkg/git/libgit2/managed/transport.go
@@ -1,9 +1,7 @@
 package managed
 
 import (
-	"crypto/sha256"
 	"fmt"
-	"hash"
 	"net"
 
 	pkgkh "github.com/fluxcd/pkg/ssh/knownhosts"
@@ -42,11 +40,9 @@ func KnownHostsCallback(host string, knownHosts []byte) git2go.CertificateCheckC
 		}
 
 		var fingerprint []byte
-		var hasher hash.Hash
 		switch {
 		case cert.Hostkey.Kind&git2go.HostkeySHA256 > 0:
 			fingerprint = cert.Hostkey.HashSHA256[:]
-			hasher = sha256.New()
 		default:
 			return fmt.Errorf("invalid host key kind, expected to be of kind SHA256")
 		}
@@ -57,7 +53,7 @@ func KnownHostsCallback(host string, knownHosts []byte) git2go.CertificateCheckC
 		// is an entry for the hostname _and_ port.
 		h := knownhosts.Normalize(host)
 		for _, k := range kh {
-			if k.Matches(h, fingerprint, hasher) {
+			if k.Matches(h, fingerprint) {
 				return nil
 			}
 		}

diff --git a/pkg/git/libgit2/managed/transport_test.go b/pkg/git/libgit2/managed/transport_test.go
@@ -1,10 +1,7 @@
 package managed
 
 import (
-	"crypto/x509"
 	"encoding/base64"
-	"encoding/pem"
-	"errors"
 	"fmt"
 	"testing"
 
@@ -14,7 +11,10 @@ import (
 
 // knownHostsFixture is known_hosts fixture in the expected
 // format.
-var knownHostsFixture = `github.com ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEAq2A7hRGmdnm9tUDbO9IDSwBK6TbQa+PXYPCPy6rbTrTtw7PHkccKrpp0yVhp5HdEIcKr6pLlVDBfOLX9QUsyCOV0wzfjIJNlGEYsdlLJizHhbn2mUjvSAHQqZETYP81eFzLQNnPHt4EVVUh7VfDESU84KezmD5QlWpXLmvU31/yMf+Se8xhHTvKSCZIFImWwoG6mbUoWf9nzpIoaSjB+weqqUUmpaaasXVal72J+UX2B+2RPW3RcT0eOzQgqlJL3RKrTJvdsjE3JEAvGq3lGHSZXy28G3skua2SmVi/w4yCE6gbODqnTWlg7+wC604ydGXA8VJiS5ap43JXiUFFAaQ==`
+var knownHostsFixture = `github.com ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEAq2A7hRGmdnm9tUDbO9IDSwBK6TbQa+PXYPCPy6rbTrTtw7PHkccKrpp0yVhp5HdEIcKr6pLlVDBfOLX9QUsyCOV0wzfjIJNlGEYsdlLJizHhbn2mUjvSAHQqZETYP81eFzLQNnPHt4EVVUh7VfDESU84KezmD5QlWpXLmvU31/yMf+Se8xhHTvKSCZIFImWwoG6mbUoWf9nzpIoaSjB+weqqUUmpaaasXVal72J+UX2B+2RPW3RcT0eOzQgqlJL3RKrTJvdsjE3JEAvGq3lGHSZXy28G3skua2SmVi/w4yCE6gbODqnTWlg7+wC604ydGXA8VJiS5ap43JXiUFFAaQ==
+github.com ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBEmKSENjQEezOmxkZMy7opKgwFB9nkt5YRrYMjNuG5N87uRgg6CLrbo5wAdT/y6v0mKV0U2w0WZ2YB/++Tpockg=
+github.com ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIOMqqnkVzrm0SdG6UOoqKLsabgH5C9okWi0dh2l9GKJl
+`
 
 func TestKnownHostsCallback(t *testing.T) {
 	tests := []struct {
@@ -41,6 +41,17 @@ func TestKnownHostsCallback(t *testing.T) {
 			expectedHost: "github.com:22",
 			want:         nil,
 		},
+		{
+			// Test case to specifically detect a regression introduced in v0.25.0
+			// Ref: https://github.com/fluxcd/image-automation-controller/issues/378
+			name:       "Match regardless of order of known_hosts",
+			host:       "github.com",
+			knownHosts: []byte(knownHostsFixture),
+			// Use ecdsa-sha2-nistp256 instead of ssh-rsa
+			hostkey:      git2go.HostkeyCertificate{Kind: git2go.HostkeySHA256, HashSHA256: sha256Fingerprint("p2QAMXNIC1TJYWeIOttrVc98/R1BUFWu3/LiyKgUfQM")},
+			expectedHost: "github.com:22",
+			want:         nil,
+		},
 		{
 			name:         "Hostname mismatch",
 			host:         "github.com",
@@ -83,11 +94,3 @@ func sha256Fingerprint(in string) [32]byte {
 	copy(out[:], d)
 	return out
 }
-
-func certificateFromPEM(pemBytes string) (*x509.Certificate, error) {
-	block, _ := pem.Decode([]byte(pemBytes))
-	if block == nil {
-		return nil, errors.New("failed to decode PEM")
-	}
-	return x509.ParseCertificate(block.Bytes)
-}