Skip to content

Commit 3fb7441

Browse files
committed
fix ssh host key verification regression
Earlier, host key verification could potentially fail if there were multiple entries in the known_hosts file and if the intended encryption algorithm wasn't the first entry. This happened because we used the same hasher object to compute the sum of all the public keys present in the known_hosts file, which led to invalid hashes, resulting in a mismatch when compared with the hash of the advertised public key. This is fixed, by not creating the hasher ourselves and instead delegating that to the function actually doing the matching, ensuring that a new hasher is used for each comparison. Regression introduced in v0.25.0 and reported in fluxcd/image-automation-controller#378 Signed-off-by: Sanskar Jaiswal <jaiswalsanskar078@gmail.com>
1 parent e55c0ce commit 3fb7441

File tree

4 files changed

+19
-20
lines changed

4 files changed

+19
-20
lines changed

go.mod

+1-1
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ require (
2828
github.com/fluxcd/pkg/helmtestserver v0.7.3
2929
github.com/fluxcd/pkg/lockedfile v0.1.0
3030
github.com/fluxcd/pkg/runtime v0.16.1
31-
github.com/fluxcd/pkg/ssh v0.4.1
31+
github.com/fluxcd/pkg/ssh v0.5.0
3232
github.com/fluxcd/pkg/testserver v0.2.0
3333
github.com/fluxcd/pkg/untar v0.1.0
3434
github.com/fluxcd/pkg/version v0.1.0

go.sum

+2-2
Original file line numberDiff line numberDiff line change
@@ -282,8 +282,8 @@ github.com/fluxcd/pkg/lockedfile v0.1.0 h1:YsYFAkd6wawMCcD74ikadAKXA4s2sukdxrn7w
282282
github.com/fluxcd/pkg/lockedfile v0.1.0/go.mod h1:EJLan8t9MiOcgTs8+puDjbE6I/KAfHbdvIy9VUgIjm8=
283283
github.com/fluxcd/pkg/runtime v0.16.1 h1:WU1vNZz4TAzmATQ/tl2zB/FX6GIUTgYeBn/G5RuTA2c=
284284
github.com/fluxcd/pkg/runtime v0.16.1/go.mod h1:cgVJkOXCg9OmrIUGklf/0UtV28MNzkuoBJhaEQICT6E=
285-
github.com/fluxcd/pkg/ssh v0.4.1 h1:O5FCjb5NIZ9PeRjdF2iL9jaPNM+RL+IjrMBZPkqF9W4=
286-
github.com/fluxcd/pkg/ssh v0.4.1/go.mod h1:KGgOUOy1uI6RC6+qxIBLvP1AeOOs/nLB25Ca6TZMIXE=
285+
github.com/fluxcd/pkg/ssh v0.5.0 h1:jE9F2XvUXC2mgseeXMATvO014fLqdB30/VzlPLKsk20=
286+
github.com/fluxcd/pkg/ssh v0.5.0/go.mod h1:KGgOUOy1uI6RC6+qxIBLvP1AeOOs/nLB25Ca6TZMIXE=
287287
github.com/fluxcd/pkg/testserver v0.2.0 h1:Mj0TapmKaywI6Fi5wvt1LAZpakUHmtzWQpJNKQ0Krt4=
288288
github.com/fluxcd/pkg/testserver v0.2.0/go.mod h1:bgjjydkXsZTeFzjz9Cr4heGANr41uTB1Aj1Q5qzuYVk=
289289
github.com/fluxcd/pkg/untar v0.1.0 h1:k97V/xV5hFrAkIkVPuv5AVhyxh1ZzzAKba/lbDfGo6o=

pkg/git/libgit2/managed/transport.go

+1-5
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,7 @@
11
package managed
22

33
import (
4-
"crypto/sha256"
54
"fmt"
6-
"hash"
75
"net"
86

97
pkgkh "github.com/fluxcd/pkg/ssh/knownhosts"
@@ -42,11 +40,9 @@ func KnownHostsCallback(host string, knownHosts []byte) git2go.CertificateCheckC
4240
}
4341

4442
var fingerprint []byte
45-
var hasher hash.Hash
4643
switch {
4744
case cert.Hostkey.Kind&git2go.HostkeySHA256 > 0:
4845
fingerprint = cert.Hostkey.HashSHA256[:]
49-
hasher = sha256.New()
5046
default:
5147
return fmt.Errorf("invalid host key kind, expected to be of kind SHA256")
5248
}
@@ -57,7 +53,7 @@ func KnownHostsCallback(host string, knownHosts []byte) git2go.CertificateCheckC
5753
// is an entry for the hostname _and_ port.
5854
h := knownhosts.Normalize(host)
5955
for _, k := range kh {
60-
if k.Matches(h, fingerprint, hasher) {
56+
if k.Matches(h, fingerprint) {
6157
return nil
6258
}
6359
}

pkg/git/libgit2/managed/transport_test.go

+15-12
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,7 @@
11
package managed
22

33
import (
4-
"crypto/x509"
54
"encoding/base64"
6-
"encoding/pem"
7-
"errors"
85
"fmt"
96
"testing"
107

@@ -14,7 +11,10 @@ import (
1411

1512
// knownHostsFixture is known_hosts fixture in the expected
1613
// format.
17-
var knownHostsFixture = `github.com ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEAq2A7hRGmdnm9tUDbO9IDSwBK6TbQa+PXYPCPy6rbTrTtw7PHkccKrpp0yVhp5HdEIcKr6pLlVDBfOLX9QUsyCOV0wzfjIJNlGEYsdlLJizHhbn2mUjvSAHQqZETYP81eFzLQNnPHt4EVVUh7VfDESU84KezmD5QlWpXLmvU31/yMf+Se8xhHTvKSCZIFImWwoG6mbUoWf9nzpIoaSjB+weqqUUmpaaasXVal72J+UX2B+2RPW3RcT0eOzQgqlJL3RKrTJvdsjE3JEAvGq3lGHSZXy28G3skua2SmVi/w4yCE6gbODqnTWlg7+wC604ydGXA8VJiS5ap43JXiUFFAaQ==`
14+
var knownHostsFixture = `github.com ssh-rsa AAAAB3NzaC1yc2EAAAABIwAAAQEAq2A7hRGmdnm9tUDbO9IDSwBK6TbQa+PXYPCPy6rbTrTtw7PHkccKrpp0yVhp5HdEIcKr6pLlVDBfOLX9QUsyCOV0wzfjIJNlGEYsdlLJizHhbn2mUjvSAHQqZETYP81eFzLQNnPHt4EVVUh7VfDESU84KezmD5QlWpXLmvU31/yMf+Se8xhHTvKSCZIFImWwoG6mbUoWf9nzpIoaSjB+weqqUUmpaaasXVal72J+UX2B+2RPW3RcT0eOzQgqlJL3RKrTJvdsjE3JEAvGq3lGHSZXy28G3skua2SmVi/w4yCE6gbODqnTWlg7+wC604ydGXA8VJiS5ap43JXiUFFAaQ==
15+
github.com ecdsa-sha2-nistp256 AAAAE2VjZHNhLXNoYTItbmlzdHAyNTYAAAAIbmlzdHAyNTYAAABBBEmKSENjQEezOmxkZMy7opKgwFB9nkt5YRrYMjNuG5N87uRgg6CLrbo5wAdT/y6v0mKV0U2w0WZ2YB/++Tpockg=
16+
github.com ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIOMqqnkVzrm0SdG6UOoqKLsabgH5C9okWi0dh2l9GKJl
17+
`
1818

1919
func TestKnownHostsCallback(t *testing.T) {
2020
tests := []struct {
@@ -41,6 +41,17 @@ func TestKnownHostsCallback(t *testing.T) {
4141
expectedHost: "github.com:22",
4242
want: nil,
4343
},
44+
{
45+
// Test case to specifically detect a regression introduced in v0.25.0
46+
// Ref: https://github.com/fluxcd/image-automation-controller/issues/378
47+
name: "Match regardless of order of known_hosts",
48+
host: "github.com",
49+
knownHosts: []byte(knownHostsFixture),
50+
// Use ecdsa-sha2-nistp256 instead of ssh-rsa
51+
hostkey: git2go.HostkeyCertificate{Kind: git2go.HostkeySHA256, HashSHA256: sha256Fingerprint("p2QAMXNIC1TJYWeIOttrVc98/R1BUFWu3/LiyKgUfQM")},
52+
expectedHost: "github.com:22",
53+
want: nil,
54+
},
4455
{
4556
name: "Hostname mismatch",
4657
host: "github.com",
@@ -83,11 +94,3 @@ func sha256Fingerprint(in string) [32]byte {
8394
copy(out[:], d)
8495
return out
8596
}
86-
87-
func certificateFromPEM(pemBytes string) (*x509.Certificate, error) {
88-
block, _ := pem.Decode([]byte(pemBytes))
89-
if block == nil {
90-
return nil, errors.New("failed to decode PEM")
91-
}
92-
return x509.ParseCertificate(block.Bytes)
93-
}

0 commit comments

Comments
 (0)