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>
diff --git a/ssh/certs.go b/ssh/certs.go
index 2fc8af1..b1f0220 100644
--- a/ssh/certs.go
+++ b/ssh/certs.go
@@ -298,8 +298,17 @@
if cert.CertType != HostCert {
return fmt.Errorf("ssh: certificate presented as a host key has type %d", cert.CertType)
}
+ if !c.IsHostAuthority(cert.SignatureKey, addr) {
+ return fmt.Errorf("ssh: no authorities for hostname: %v", addr)
+ }
- return c.CheckCert(addr, cert)
+ hostname, _, err := net.SplitHostPort(addr)
+ if err != nil {
+ return err
+ }
+
+ // Pass hostname only as principal for host certificates (consistent with OpenSSH)
+ return c.CheckCert(hostname, cert)
}
// Authenticate checks a user certificate. Authenticate can be used as
@@ -316,6 +325,9 @@
if cert.CertType != UserCert {
return nil, fmt.Errorf("ssh: cert has type %d", cert.CertType)
}
+ if !c.IsUserAuthority(cert.SignatureKey) {
+ return nil, fmt.Errorf("ssh: certificate signed by unrecognized authority")
+ }
if err := c.CheckCert(conn.User(), cert); err != nil {
return nil, err
@@ -364,16 +376,6 @@
}
}
- // if this is a host cert, principal is the remote hostname as passed
- // to CheckHostCert.
- if cert.CertType == HostCert && !c.IsHostAuthority(cert.SignatureKey, principal) {
- return fmt.Errorf("ssh: no authorities for hostname: %v", principal)
- }
-
- if cert.CertType == UserCert && !c.IsUserAuthority(cert.SignatureKey) {
- return fmt.Errorf("ssh: certificate signed by unrecognized authority")
- }
-
clock := c.Clock
if clock == nil {
clock = time.Now
diff --git a/ssh/certs_test.go b/ssh/certs_test.go
index fba6310..0200531 100644
--- a/ssh/certs_test.go
+++ b/ssh/certs_test.go
@@ -168,8 +168,8 @@
cert.SignCert(rand.Reader, testSigners["ecdsa"])
checker := &CertChecker{
- IsHostAuthority: func(p PublicKey, h string) bool {
- return h == "hostname" && bytes.Equal(testPublicKeys["ecdsa"].Marshal(), p.Marshal())
+ IsHostAuthority: func(p PublicKey, addr string) bool {
+ return addr == "hostname:22" && bytes.Equal(testPublicKeys["ecdsa"].Marshal(), p.Marshal())
},
}
@@ -178,7 +178,14 @@
t.Errorf("NewCertSigner: %v", err)
}
- for _, name := range []string{"hostname", "otherhost", "lasthost"} {
+ for _, test := range []struct {
+ addr string
+ succeed bool
+ }{
+ {addr: "hostname:22", succeed: true},
+ {addr: "otherhost:22", succeed: false}, // The certificate is valid for 'otherhost' as hostname, but we only recognize the authority of the signer for the address 'hostname:22'
+ {addr: "lasthost:22", succeed: false},
+ } {
c1, c2, err := netPipe()
if err != nil {
t.Fatalf("netPipe: %v", err)
@@ -201,16 +208,15 @@
User: "user",
HostKeyCallback: checker.CheckHostKey,
}
- _, _, _, err = NewClientConn(c2, name, config)
+ _, _, _, err = NewClientConn(c2, test.addr, config)
- succeed := name == "hostname"
- if (err == nil) != succeed {
- t.Fatalf("NewClientConn(%q): %v", name, err)
+ if (err == nil) != test.succeed {
+ t.Fatalf("NewClientConn(%q): %v", test.addr, err)
}
err = <-errc
- if (err == nil) != succeed {
- t.Fatalf("NewServerConn(%q): %v", name, err)
+ if (err == nil) != test.succeed {
+ t.Fatalf("NewServerConn(%q): %v", test.addr, err)
}
}
}
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 @@
}
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 @@
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 @@
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 (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 @@
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 @@
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]))
diff --git a/ssh/testdata/keys.go b/ssh/testdata/keys.go
index 0be2e7e..3b3d26c 100644
--- a/ssh/testdata/keys.go
+++ b/ssh/testdata/keys.go
@@ -70,6 +70,47 @@
PLL8IEwvYu2wq+lpXfGQnNMbzYf9gspG0w==
-----END EC PRIVATE KEY-----
`),
+ "ca": []byte(`-----BEGIN RSA PRIVATE KEY-----
+MIIEpAIBAAKCAQEAvg9dQ9IRG59lYJb+GESfKWTch4yBpr7Ydw1jkK6vvtrx9jLo
+5hkA8X6+ElRPRqTAZSlN5cBm6YCAcQIOsmXDUn6Oj1lVPQAoOjTBTvsjM3NjGhvv
+52kHTY0nsMsBeY9q5DTtlzmlYkVUq2a6Htgf2mNi01dIw5fJ7uTTo8EbNf7O0i3u
+c9a8P19HaZl5NKiWN4EIZkfB2WdXYRJCVBsGgQj3dE/GrEmH9QINq1A+GkNvK96u
+vZm8H1jjmuqzHplWa7lFeXcx8FTVTbVb/iJrZ2Lc/JvIPitKZWhqbR59yrGjpwEp
+Id7bo4WhO5L3OB0fSIJYvfu+o4WYnt4f3UzecwIDAQABAoIBABRD9yHgKErVuC2Q
+bA+SYZY8VvdtF/X7q4EmQFORDNRA7EPgMc03JU6awRGbQ8i4kHs46EFzPoXvWcKz
+AXYsO6N0Myc900Tp22A5d9NAHATEbPC/wdje7hRq1KyZONMJY9BphFv3nZbY5apR
+Dc90JBFZP5RhXjTc3n9GjvqLAKfFEKVmPRCvqxCOZunw6XR+SgIQLJo36nsIsbhW
+QUXIVaCI6cXMN8bRPm8EITdBNZu06Fpu4ZHm6VaxlXN9smERCDkgBSNXNWHKxmmA
+c3Glo2DByUr2/JFBOrLEe9fkYgr24KNCQkHVcSaFxEcZvTggr7StjKISVHlCNEaB
+7Q+kPoECgYEA3zE9FmvFGoQCU4g4Nl3dpQHs6kaAW8vJlrmq3xsireIuaJoa2HMe
+wYdIvgCnK9DIjyxd5OWnE4jXtAEYPsyGD32B5rSLQrRO96lgb3f4bESCLUb3Bsn/
+sdgeE3p1xZMA0B59htqCrvVgN9k8WxyevBxYl3/gSBm/p8OVH1RTW/ECgYEA2f9Z
+95OLj0KQHQtxQXf+I3VjhCw3LkLW39QZOXVI0QrCJfqqP7uxsJXH9NYX0l0GFTcR
+kRrlyoaSU1EGQosZh+n1MvplGBTkTSV47/bPsTzFpgK2NfEZuFm9RoWgltS+nYeH
+Y2k4mnAN3PhReCMwuprmJz8GRLsO3Cs2s2YylKMCgYEA2UX+uO/q7jgqZ5UJW+ue
+1H5+W0aMuFA3i7JtZEnvRaUVFqFGlwXin/WJ2+WY1++k/rPrJ+Rk9IBXtBUIvEGw
+FC5TIfsKQsJyyWgqx/jbbtJ2g4s8+W/1qfTAuqeRNOg5d2DnRDs90wJuS4//0JaY
+9HkHyVwkQyxFxhSA/AHEMJECgYA2MvyFR1O9bIk0D3I7GsA+xKLXa77Ua53MzIjw
+9i4CezBGDQpjCiFli/fI8am+jY5DnAtsDknvjoG24UAzLy5L0mk6IXMdB6SzYYut
+7ak5oahqW+Y9hxIj+XvLmtGQbphtxhJtLu35x75KoBpxSh6FZpmuTEccs31AVCYn
+eFM/DQKBgQDOPUwbLKqVi6ddFGgrV9MrWw+SWsDa43bPuyvYppMM3oqesvyaX1Dt
+qDvN7owaNxNM4OnfKcZr91z8YPVCFo4RbBif3DXRzjNNBlxEjHBtuMOikwvsmucN
+vIrbeEpjTiUMTEAr6PoTiVHjsfS8WAM6MDlF5M+2PNswDsBpa2yLgA==
+-----END RSA PRIVATE KEY-----
+`),
+}
+
+var SSHCertificates = map[string][]byte{
+ // The following are corresponding certificates for the private keys above, signed by the CA key
+ // Generated by the following commands:
+ //
+ // 1. Assumes "rsa" key above in file named "rsa", write out the public key to "rsa.pub":
+ // ssh-keygen -y -f rsa > rsa.pu
+ //
+ // 2. Assumes "ca" key above in file named "ca", sign a cert for "rsa.pub":
+ // ssh-keygen -s ca -h -n host.example.com -V +500w -I host.example.com-key rsa.pub
+ "rsa": []byte(`ssh-rsa-cert-v01@openssh.com AAAAHHNzaC1yc2EtY2VydC12MDFAb3BlbnNzaC5jb20AAAAgLjYqmmuTSEmjVhSfLQphBSTJMLwIZhRgmpn8FHKLiEIAAAADAQABAAAAgQC8A6FGHDiWCSREAXCq6yBfNVr0xCVG2CzvktFNRpue+RXrGs/2a6ySEJQb3IYquw7HlJgu6fg3WIWhOmHCjfpG0PrL4CRwbqQ2LaPPXhJErWYejcD8Di00cF3677+G10KMZk9RXbmHtuBFZT98wxg8j+ZsBMqGM1+7yrWUvynswQAAAAAAAAAAAAAAAgAAABRob3N0LmV4YW1wbGUuY29tLWtleQAAABQAAAAQaG9zdC5leGFtcGxlLmNvbQAAAABZHN8UAAAAAGsjIYUAAAAAAAAAAAAAAAAAAAEXAAAAB3NzaC1yc2EAAAADAQABAAABAQC+D11D0hEbn2Vglv4YRJ8pZNyHjIGmvth3DWOQrq++2vH2MujmGQDxfr4SVE9GpMBlKU3lwGbpgIBxAg6yZcNSfo6PWVU9ACg6NMFO+yMzc2MaG+/naQdNjSewywF5j2rkNO2XOaViRVSrZroe2B/aY2LTV0jDl8nu5NOjwRs1/s7SLe5z1rw/X0dpmXk0qJY3gQhmR8HZZ1dhEkJUGwaBCPd0T8asSYf1Ag2rUD4aQ28r3q69mbwfWOOa6rMemVZruUV5dzHwVNVNtVv+ImtnYtz8m8g+K0plaGptHn3KsaOnASkh3tujhaE7kvc4HR9Igli9+76jhZie3h/dTN5zAAABDwAAAAdzc2gtcnNhAAABALeDea+60H6xJGhktAyosHaSY7AYzLocaqd8hJQjEIDifBwzoTlnBmcK9CxGhKuaoJFThdCLdaevCeOSuquh8HTkf+2ebZZc/G5T+2thPvPqmcuEcmMosWo+SIjYhbP3S6KD49aLC1X0kz8IBQeauFvURhkZ5ZjhA1L4aQYt9NjL73nqOl8PplRui+Ov5w8b4ldul4zOvYAFrzfcP6wnnXk3c1Zzwwf5wynD5jakO8GpYKBuhM7Z4crzkKSQjU3hla7xqgfomC5Gz4XbR2TNjcQiRrJQ0UlKtX3X3ObRCEhuvG0Kzjklhv+Ddw6txrhKjMjiSi/Yyius/AE8TmC1p4U= host.example.com
+`),
}
var PEMEncryptedKeys = []struct {
