From 6b3d96ef3bc05f6c9875751cf14bc7f2e033141f Mon Sep 17 00:00:00 2001 From: Philip Laine Date: Thu, 11 Feb 2021 11:28:23 +0100 Subject: [PATCH 1/4] Check hostkey type when validating hostkey Signed-off-by: Philip Laine --- pkg/git/libgit2/transport.go | 26 +++++++++++++--- pkg/git/libgit2/transport_test.go | 50 +++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 4 deletions(-) diff --git a/pkg/git/libgit2/transport.go b/pkg/git/libgit2/transport.go index 74fd317e..58c71524 100644 --- a/pkg/git/libgit2/transport.go +++ b/pkg/git/libgit2/transport.go @@ -19,9 +19,12 @@ package libgit2 import ( "bufio" "bytes" + "crypto/md5" "crypto/sha1" + "crypto/sha256" "crypto/x509" "fmt" + "hash" "net" "net/url" "strings" @@ -157,7 +160,7 @@ func (s *PublicKeyAuth) Method(secret corev1.Secret) (*git.Auth, error) { // is an entry for the hostname _and_ port. host = knownhosts.Normalize(s.host) for _, k := range kk { - if k.matches(host, cert.Hostkey.HashSHA1[:]) { + if k.matches(host, cert.Hostkey) { return git2go.ErrOk } } @@ -195,13 +198,28 @@ func parseKnownHosts(s string) ([]knownKey, error) { return knownHosts, nil } -func (k knownKey) matches(host string, key []byte) bool { +func (k knownKey) matches(host string, hostkey git2go.HostkeyCertificate) bool { if !containsHost(k.hosts, host) { return false } - hash := sha1.Sum(k.key.Marshal()) - if bytes.Compare(hash[:], key) != 0 { + var fingerprint []byte + var hasher hash.Hash + switch hostkey.Kind { + case git2go.HostkeyMD5: + fingerprint = hostkey.HashMD5[:] + hasher = md5.New() + case git2go.HostkeySHA1: + fingerprint = hostkey.HashSHA1[:] + hasher = sha1.New() + case git2go.HostkeySHA256: + fingerprint = hostkey.HashSHA256[:] + hasher = sha256.New() + default: + return false + } + hasher.Write(k.key.Marshal()) + if bytes.Compare(hasher.Sum(nil), fingerprint) != 0 { return false } diff --git a/pkg/git/libgit2/transport_test.go b/pkg/git/libgit2/transport_test.go index 7a2dcd31..b22f260f 100644 --- a/pkg/git/libgit2/transport_test.go +++ b/pkg/git/libgit2/transport_test.go @@ -17,9 +17,11 @@ limitations under the License. package libgit2 import ( + "encoding/base64" "reflect" "testing" + git2go "github.com/libgit2/git2go/v31" corev1 "k8s.io/api/core/v1" "github.com/fluxcd/source-controller/pkg/git" @@ -145,3 +147,51 @@ func TestPublicKeyStrategy_Method(t *testing.T) { }) } } + +func TestKnownKeyHash(t *testing.T) { + tests := []struct { + name string + hostkey git2go.HostkeyCertificate + wantMatches bool + }{ + {"good sha256 hostkey", git2go.HostkeyCertificate{Kind: git2go.HostkeySHA256, HashSHA256: sha256Fingerprint("nThbg6kXUpJWGl7E1IGOCspRomTxdCARLviKw6E5SY8")}, true}, + {"bad sha256 hostkey", git2go.HostkeyCertificate{Kind: git2go.HostkeySHA256, HashSHA256: sha256Fingerprint("ROQFvPThGrW4RuWLoL9tq9I9zJ42fK4XywyRtbOz/EQ")}, false}, + {"good sha1 hostkey", git2go.HostkeyCertificate{Kind: git2go.HostkeySHA1, HashSHA1: sha1Fingerprint("v2toJdKXfFEaR1u++4iq1UqSrHM")}, true}, + {"invalid hostkey", git2go.HostkeyCertificate{}, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + knownKeys, err := parseKnownHosts(knownHostsFixture) + if err != nil { + t.Error(err) + return + } + + matches := knownKeys[0].matches("github.com", tt.hostkey) + if matches != tt.wantMatches { + t.Errorf("Method() matches = %v, wantMatches %v", matches, tt.wantMatches) + return + } + }) + } +} + +func sha1Fingerprint(in string) [20]byte { + d, err := base64.RawStdEncoding.DecodeString(in) + if err != nil { + panic(err) + } + var out [20]byte + copy(out[:], d) + return out +} + +func sha256Fingerprint(in string) [32]byte { + d, err := base64.RawStdEncoding.DecodeString(in) + if err != nil { + panic(err) + } + var out [32]byte + copy(out[:], d) + return out +} From f9ddeb06e1de1274e2db57121eeaec0bf76f1cb0 Mon Sep 17 00:00:00 2001 From: Philip Laine Date: Fri, 12 Feb 2021 08:43:40 +0100 Subject: [PATCH 2/4] Fix hash type switch statement Signed-off-by: Philip Laine --- pkg/git/libgit2/transport.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/pkg/git/libgit2/transport.go b/pkg/git/libgit2/transport.go index 58c71524..9e50b628 100644 --- a/pkg/git/libgit2/transport.go +++ b/pkg/git/libgit2/transport.go @@ -205,16 +205,16 @@ func (k knownKey) matches(host string, hostkey git2go.HostkeyCertificate) bool { var fingerprint []byte var hasher hash.Hash - switch hostkey.Kind { - case git2go.HostkeyMD5: - fingerprint = hostkey.HashMD5[:] - hasher = md5.New() - case git2go.HostkeySHA1: - fingerprint = hostkey.HashSHA1[:] - hasher = sha1.New() - case git2go.HostkeySHA256: + switch { + case hostkey.Kind&git2go.HostkeySHA256 > 0: fingerprint = hostkey.HashSHA256[:] hasher = sha256.New() + case hostkey.Kind&git2go.HostkeySHA1 > 0: + fingerprint = hostkey.HashSHA1[:] + hasher = sha1.New() + case hostkey.Kind&git2go.HostkeyMD5 > 0: + fingerprint = hostkey.HashMD5[:] + hasher = md5.New() default: return false } From 73301df0233fac635d0b3f3c64175586ca345a64 Mon Sep 17 00:00:00 2001 From: Philip Laine Date: Fri, 12 Feb 2021 11:16:14 +0100 Subject: [PATCH 3/4] Add md5 test and check priority of hash types Signed-off-by: Philip Laine --- pkg/git/libgit2/transport_test.go | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/pkg/git/libgit2/transport_test.go b/pkg/git/libgit2/transport_test.go index b22f260f..2a1387c1 100644 --- a/pkg/git/libgit2/transport_test.go +++ b/pkg/git/libgit2/transport_test.go @@ -154,9 +154,12 @@ func TestKnownKeyHash(t *testing.T) { hostkey git2go.HostkeyCertificate wantMatches bool }{ - {"good sha256 hostkey", git2go.HostkeyCertificate{Kind: git2go.HostkeySHA256, HashSHA256: sha256Fingerprint("nThbg6kXUpJWGl7E1IGOCspRomTxdCARLviKw6E5SY8")}, true}, - {"bad sha256 hostkey", git2go.HostkeyCertificate{Kind: git2go.HostkeySHA256, HashSHA256: sha256Fingerprint("ROQFvPThGrW4RuWLoL9tq9I9zJ42fK4XywyRtbOz/EQ")}, false}, - {"good sha1 hostkey", git2go.HostkeyCertificate{Kind: git2go.HostkeySHA1, HashSHA1: sha1Fingerprint("v2toJdKXfFEaR1u++4iq1UqSrHM")}, true}, + {"good sha256 hostkey", git2go.HostkeyCertificate{Kind: git2go.HostkeySHA256 | git2go.HostkeySHA1 | git2go.HostkeyMD5, HashSHA256: sha256Fingerprint("nThbg6kXUpJWGl7E1IGOCspRomTxdCARLviKw6E5SY8")}, true}, + {"bad sha256 hostkey", git2go.HostkeyCertificate{Kind: git2go.HostkeySHA256 | git2go.HostkeySHA1 | git2go.HostkeyMD5, HashSHA256: sha256Fingerprint("ROQFvPThGrW4RuWLoL9tq9I9zJ42fK4XywyRtbOz/EQ")}, false}, + {"good sha1 hostkey", git2go.HostkeyCertificate{Kind: git2go.HostkeySHA1 | git2go.HostkeyMD5, HashSHA1: sha1Fingerprint("v2toJdKXfFEaR1u++4iq1UqSrHM")}, true}, + {"bad sha1 hostkey", git2go.HostkeyCertificate{Kind: git2go.HostkeySHA1 | git2go.HostkeyMD5, HashSHA1: sha1Fingerprint("tfpLlQhDDFP3yGdewTvHNxWmAdk")}, false}, + {"good md5 hostkey", git2go.HostkeyCertificate{Kind: git2go.HostkeyMD5, HashMD5: md5Fingerprint("\x16\x27\xac\xa5\x76\x28\x2d\x36\x63\x1b\x56\x4d\xeb\xdf\xa6\x48")}, true}, + {"bad md5 hostkey", git2go.HostkeyCertificate{Kind: git2go.HostkeyMD5, HashMD5: md5Fingerprint("\xb6\x03\x0e\x39\x97\x9e\xd0\xe7\x24\xce\xa3\x77\x3e\x01\x42\x09")}, false}, {"invalid hostkey", git2go.HostkeyCertificate{}, false}, } for _, tt := range tests { @@ -176,6 +179,12 @@ func TestKnownKeyHash(t *testing.T) { } } +func md5Fingerprint(in string) [16]byte { + var out [16]byte + copy(out[:], []byte(in)) + return out +} + func sha1Fingerprint(in string) [20]byte { d, err := base64.RawStdEncoding.DecodeString(in) if err != nil { From 0a1631dc5a08e383e8d616cc06e375f408cd871f Mon Sep 17 00:00:00 2001 From: Philip Laine Date: Fri, 12 Feb 2021 11:18:27 +0100 Subject: [PATCH 4/4] Remove redundant if else Signed-off-by: Philip Laine --- pkg/git/libgit2/transport.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/pkg/git/libgit2/transport.go b/pkg/git/libgit2/transport.go index 9e50b628..f5327356 100644 --- a/pkg/git/libgit2/transport.go +++ b/pkg/git/libgit2/transport.go @@ -219,11 +219,7 @@ func (k knownKey) matches(host string, hostkey git2go.HostkeyCertificate) bool { return false } hasher.Write(k.key.Marshal()) - if bytes.Compare(hasher.Sum(nil), fingerprint) != 0 { - return false - } - - return true + return bytes.Compare(hasher.Sum(nil), fingerprint) == 0 } func containsHost(hosts []string, host string) bool {