Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

libgit2: fix ssh host key verification regression #771

Merged
merged 1 commit into from
Jun 8, 2022
Merged

libgit2: fix ssh host key verification regression #771

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
6 changes: 1 addition & 5 deletions pkg/git/libgit2/managed/transport.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
package managed

import (
"crypto/sha256"
"fmt"
"hash"
"net"

pkgkh "github.com/fluxcd/pkg/ssh/knownhosts"
Expand Down Expand Up @@ -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")
}
Expand All @@ -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
}
}
Expand Down
27 changes: 15 additions & 12 deletions pkg/git/libgit2/managed/transport_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
package managed

import (
"crypto/x509"
"encoding/base64"
"encoding/pem"
"errors"
"fmt"
"testing"

Expand All @@ -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
`
Comment on lines +14 to +17
Copy link
Member

@pjbgf pjbgf Jun 8, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could fail to simulate the scenario based on external factors that may change in the future. This is OK for now, but we should consider depending on a test server when refactoring/consolidating the Git implementation.


func TestKnownHostsCallback(t *testing.T) {
tests := []struct {
Expand All @@ -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",
Expand Down Expand Up @@ -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)
}