Merge pull request #288 from fluxcd/libgit2-hostkey

pkg/git/libgit2/transport.go

@@ -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"
@@ -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)
 	}
@@ -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
@@ -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"]
@@ -126,7 +128,7 @@ 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
@@ -134,12 +136,32 @@ func (s *PublicKeyAuth) Method(secret corev1.Secret) (*git.Auth, error) {
 		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
@@ -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())
@@ -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
 	}

pkg/git/libgit2/transport_test.go

@@ -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", "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"}, 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 {