x/crypto/ssh: fix host certificate principal evaluation to check for hostname only

SSH host certificates are expected to contain hostnames only,
not "host:port" format.

This change allows Go clients to connect to OpenSSH servers that
use host certificates.

Note, this change will break any clients that use ssh.NewClientConn()
with an `addr` that is not in `host:port` format (they will see a
"missing port in address" error).

Fixes bug 20273.

Change-Id: I5a306c6b7b419a737e1f0f9c5ca8c585e21a45a4
Reviewed-on: https://go-review.googlesource.com/43475
Reviewed-by: Han-Wen Nienhuys <hanwen@google.com>
Run-TryBot: Han-Wen Nienhuys <hanwen@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>

2 files changed, 53 insertions, 7 deletions

diff --git a/ssh/test/cert_test.go b/ssh/test/cert_test.go
index bc83e4f..b231dd8 100644
--- a/ssh/test/cert_test.go
+++ b/ssh/test/cert_test.go
@@ -7,12 +7,14 @@
 package test
 
 import (
+	"bytes"
 	"crypto/rand"
 	"testing"
 
 	"golang.org/x/crypto/ssh"
 )
 
+// Test both logging in with a cert, and also that the certificate presented by an OpenSSH host can be validated correctly
 func TestCertLogin(t *testing.T) {
 	s := newServer(t)
 	defer s.Shutdown()
@@ -36,13 +38,40 @@ func TestCertLogin(t *testing.T) {
 	}
 
 	conf := &ssh.ClientConfig{
-		User:            username(),
-		HostKeyCallback: ssh.InsecureIgnoreHostKey(),
+		User: username(),
+		HostKeyCallback: (&ssh.CertChecker{
+			IsHostAuthority: func(pk ssh.PublicKey, addr string) bool {
+				return bytes.Equal(pk.Marshal(), testPublicKeys["ca"].Marshal())
+			},
+		}).CheckHostKey,
 	}
 	conf.Auth = append(conf.Auth, ssh.PublicKeys(certSigner))
-	client, err := s.TryDial(conf)
-	if err != nil {
-		t.Fatalf("TryDial: %v", err)
+
+	for _, test := range []struct {
+		addr    string
+		succeed bool
+	}{
+		{addr: "host.example.com:22", succeed: true},
+		{addr: "host.example.com:10000", succeed: true}, // non-standard port must be OK
+		{addr: "host.example.com", succeed: false},      // port must be specified
+		{addr: "host.ex4mple.com:22", succeed: false},   // wrong host
+	} {
+		client, err := s.TryDialWithAddr(conf, test.addr)
+
+		// Always close client if opened successfully
+		if err == nil {
+			client.Close()
+		}
+
+		// Now evaluate whether the test failed or passed
+		if test.succeed {
+			if err != nil {
+				t.Fatalf("TryDialWithAddr: %v", err)
+			}
+		} else {
+			if err == nil {
+				t.Fatalf("TryDialWithAddr, unexpected success")
+			}
+		}
 	}
-	client.Close()
 }
diff --git a/ssh/test/test_unix_test.go b/ssh/test/test_unix_test.go
index dd9ff40..e673536 100644
--- a/ssh/test/test_unix_test.go
+++ b/ssh/test/test_unix_test.go
@@ -30,6 +30,7 @@ Protocol 2
 HostKey {{.Dir}}/id_rsa
 HostKey {{.Dir}}/id_dsa
 HostKey {{.Dir}}/id_ecdsa
+HostCertificate {{.Dir}}/id_rsa-cert.pub
 Pidfile {{.Dir}}/sshd.pid
 #UsePrivilegeSeparation no
 KeyRegenerationInterval 3600
@@ -119,6 +120,11 @@ func clientConfig() *ssh.ClientConfig {
 			ssh.PublicKeys(testSigners["user"]),
 		},
 		HostKeyCallback: hostKeyDB().Check,
+		HostKeyAlgorithms: []string{ // by default, don't allow certs as this affects the hostKeyDB checker
+			ssh.KeyAlgoECDSA256, ssh.KeyAlgoECDSA384, ssh.KeyAlgoECDSA521,
+			ssh.KeyAlgoRSA, ssh.KeyAlgoDSA,
+			ssh.KeyAlgoED25519,
+		},
 	}
 	return config
 }
@@ -154,6 +160,12 @@ func unixConnection() (*net.UnixConn, *net.UnixConn, error) {
 }
 
 func (s *server) TryDial(config *ssh.ClientConfig) (*ssh.Client, error) {
+	return s.TryDialWithAddr(config, "")
+}
+
+// addr is the user specified host:port. While we don't actually dial it,
+// we need to know this for host key matching
+func (s *server) TryDialWithAddr(config *ssh.ClientConfig, addr string) (*ssh.Client, error) {
 	sshd, err := exec.LookPath("sshd")
 	if err != nil {
 		s.t.Skipf("skipping test: %v", err)
@@ -179,7 +191,7 @@ func (s *server) TryDial(config *ssh.ClientConfig) (*ssh.Client, error) {
 		s.t.Fatalf("s.cmd.Start: %v", err)
 	}
 	s.clientConn = c1
-	conn, chans, reqs, err := ssh.NewClientConn(c1, "", config)
+	conn, chans, reqs, err := ssh.NewClientConn(c1, addr, config)
 	if err != nil {
 		return nil, err
 	}
@@ -250,6 +262,11 @@ func newServer(t *testing.T) *server {
 		writeFile(filepath.Join(dir, filename+".pub"), ssh.MarshalAuthorizedKey(testPublicKeys[k]))
 	}
 
+	for k, v := range testdata.SSHCertificates {
+		filename := "id_" + k + "-cert.pub"
+		writeFile(filepath.Join(dir, filename), v)
+	}
+
 	var authkeys bytes.Buffer
 	for k, _ := range testdata.PEMBytes {
 		authkeys.Write(ssh.MarshalAuthorizedKey(testPublicKeys[k]))