Skip to content

Commit

Permalink
libgit2: use provided host to validate public key
Browse files Browse the repository at this point in the history
The callback from libgit2 only provides a hostname (without the port),
but the `known_hosts` file indexes the public keys based on the full
host (e.g. `[localhost]:123` for a host behind a specific port).

As a result, it was unable to find the correct public key for the
hostname when it was added to the `known_hosts` file with the port.

To work around this, we add the user provided host that includes the
port to the `PublicKeyAuth` strategy, and use this to find the right
entry in the `known_hosts` file, after having validated that the
hostname provided to the callback matches the hostname of the host
provided by the user.

Signed-off-by: Hidde Beydals <hello@hidde.co>
  • Loading branch information
hiddeco committed Feb 11, 2021
1 parent c32d11c commit b2f6639
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 12 deletions.
42 changes: 32 additions & 10 deletions pkg/git/libgit2/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,13 @@ import (
"crypto/sha1"
"crypto/x509"
"fmt"
"net"
"net/url"
"strings"

"golang.org/x/crypto/ssh"

git2go "github.com/libgit2/git2go/v31"
"golang.org/x/crypto/ssh"
"golang.org/x/crypto/ssh/knownhosts"
corev1 "k8s.io/api/core/v1"

"github.com/fluxcd/source-controller/pkg/git"
Expand All @@ -43,7 +44,7 @@ func AuthSecretStrategyForURL(URL string) (git.AuthSecretStrategy, error) {
case u.Scheme == "http", u.Scheme == "https":
return &BasicAuth{}, nil
case u.Scheme == "ssh":
return &PublicKeyAuth{user: u.User.Username()}, nil
return &PublicKeyAuth{user: u.User.Username(), host: u.Host}, nil
default:
return nil, fmt.Errorf("no auth secret strategy for scheme %s", u.Scheme)
}
Expand All @@ -62,7 +63,7 @@ func (s *BasicAuth) Method(secret corev1.Secret) (*git.Auth, error) {
password = string(d)
}
if username != "" && password != "" {
credCallback = func(url string, username_from_url string, allowed_types git2go.CredType) (*git2go.Cred, error) {
credCallback = func(url string, usernameFromURL string, allowedTypes git2go.CredType) (*git2go.Cred, error) {
cred, err := git2go.NewCredUserpassPlaintext(username, password)
if err != nil {
return nil, err
Expand Down Expand Up @@ -97,11 +98,12 @@ func (s *BasicAuth) Method(secret corev1.Secret) (*git.Auth, error) {

type PublicKeyAuth struct {
user string
host string
}

func (s *PublicKeyAuth) Method(secret corev1.Secret) (*git.Auth, error) {
if _, ok := secret.Data[git.CAFile]; ok {
return nil, fmt.Errorf("found caFile key in secret '%s' but libgit2 SSH transport does not support custom certificates", secret.Name)
return nil, fmt.Errorf("found %s key in secret '%s' but libgit2 SSH transport does not support custom certificates", git.CAFile, secret.Name)
}
identity := secret.Data["identity"]
knownHosts := secret.Data["known_hosts"]
Expand All @@ -126,20 +128,40 @@ func (s *PublicKeyAuth) Method(secret corev1.Secret) (*git.Auth, error) {
user = git.DefaultPublicKeyAuthUser
}

credCallback := func(url string, username_from_url string, allowed_types git2go.CredType) (*git2go.Cred, error) {
credCallback := func(url string, usernameFromURL string, allowedTypes git2go.CredType) (*git2go.Cred, error) {
cred, err := git2go.NewCredSshKeyFromMemory(user, "", string(identity), "")
if err != nil {
return nil, err
}
return cred, nil
}
certCallback := func(cert *git2go.Certificate, valid bool, hostname string) git2go.ErrorCode {
// First, attempt to split the configured host and port to validate
// the port-less hostname given to the callback.
host, _, err := net.SplitHostPort(s.host)
if err != nil {
// SplitHostPort returns an error if the host is missing
// a port, assume the host has no port.
host = s.host
}

// Check if the configured host matches the hostname given to
// the callback.
if host != hostname {
return git2go.ErrUser
}

// 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.
host = knownhosts.Normalize(s.host)
for _, k := range kk {
if k.matches(hostname, cert.Hostkey.HashSHA1[:]) {
if k.matches(host, cert.Hostkey.HashSHA1[:]) {
return git2go.ErrOk
}
}
return git2go.ErrGeneric
return git2go.ErrCertificate
}

return &git.Auth{CredCallback: credCallback, CertCallback: certCallback}, nil
Expand All @@ -151,7 +173,7 @@ type knownKey struct {
}

func parseKnownHosts(s string) ([]knownKey, error) {
knownHosts := []knownKey{}
var knownHosts []knownKey
scanner := bufio.NewScanner(strings.NewReader(s))
for scanner.Scan() {
_, hosts, pubKey, _, _, err := ssh.ParseKnownHosts(scanner.Bytes())
Expand All @@ -178,7 +200,7 @@ func (k knownKey) matches(host string, key []byte) bool {
return false
}

hash := sha1.Sum([]byte(k.key.Marshal()))
hash := sha1.Sum(k.key.Marshal())
if bytes.Compare(hash[:], key) != 0 {
return false
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/git/libgit2/transport_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,8 @@ func TestAuthSecretStrategyForURL(t *testing.T) {
}{
{"HTTP", "http://git.example.com/org/repo.git", &BasicAuth{}, false},
{"HTTPS", "https://git.example.com/org/repo.git", &BasicAuth{}, false},
{"SSH", "ssh://git.example.com:2222/org/repo.git", &PublicKeyAuth{}, false},
{"SSH with username", "ssh://example@git.example.com:2222/org/repo.git", &PublicKeyAuth{user: "example"}, false},
{"SSH", "ssh://git.example.com:2222/org/repo.git", &PublicKeyAuth{host: "git.example.com:2222"}, false},
{"SSH with username", "ssh://example@git.example.com:2222/org/repo.git", &PublicKeyAuth{user: "example", host: "git.example.com:2222"}, false},
{"unsupported", "protocol://example.com", nil, true},
}
for _, tt := range tests {
Expand Down

0 comments on commit b2f6639

Please sign in to comment.