ssh/knownhosts: add IsHostAuthority.

This is a breaking change.

This adds a new hostkey callback which takes the hostname field
restrictions into account when validating host certificates.

Prior to this, a known_hosts file with the following entry

  @cert-authority *.example.com ssh-rsa <example.com public key>

would, when passed to knownhosts.New() generate an ssh.HostKeyCallback
that would accept all host certificates signed by the example.com public
key, no matter what host the client was connecting to.

After this change, that known_hosts entry can only be used to validate
host certificates presented when connecting to hosts under *.example.com

This also renames IsAuthority to IsUserAuthority to make its intended
purpose more clear.

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

ssh/certs.go

@@ -251,10 +251,18 @@ type CertChecker struct {
 	// for user certificates.
 	SupportedCriticalOptions []string
 
-	// IsAuthority should return true if the key is recognized as
-	// an authority. This allows for certificates to be signed by other
-	// certificates.
-	IsAuthority func(auth PublicKey) bool
+	// IsUserAuthority should return true if the key is recognized as an
+	// authority for the given user certificate. This allows for
+	// certificates to be signed by other certificates. This must be set
+	// if this CertChecker will be checking user certificates.
+	IsUserAuthority func(auth PublicKey) bool
+
+	// IsHostAuthority should report whether the key is recognized as
+	// an authority for this host. This allows for certificates to be
+	// signed by other keys, and for those other keys to only be valid
+	// signers for particular hostnames. This must be set if this
+	// CertChecker will be checking host certificates.
+	IsHostAuthority func(auth PublicKey, address string) bool
 
 	// Clock is used for verifying time stamps. If nil, time.Now
 	// is used.
@@ -356,7 +364,13 @@ func (c *CertChecker) CheckCert(principal string, cert *Certificate) error {
 		}
 	}
 
-	if !c.IsAuthority(cert.SignatureKey) {
+	// 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")
 	}
 

ssh/certs_test.go

@@ -104,7 +104,7 @@ func TestValidateCert(t *testing.T) {
 		t.Fatalf("got %v (%T), want *Certificate", key, key)
 	}
 	checker := CertChecker{}
-	checker.IsAuthority = func(k PublicKey) bool {
+	checker.IsUserAuthority = func(k PublicKey) bool {
 		return bytes.Equal(k.Marshal(), validCert.SignatureKey.Marshal())
 	}
 
@@ -142,7 +142,7 @@ func TestValidateCertTime(t *testing.T) {
 		checker := CertChecker{
 			Clock: func() time.Time { return time.Unix(ts, 0) },
 		}
-		checker.IsAuthority = func(k PublicKey) bool {
+		checker.IsUserAuthority = func(k PublicKey) bool {
 			return bytes.Equal(k.Marshal(),
 				testPublicKeys["ecdsa"].Marshal())
 		}
@@ -160,16 +160,16 @@ func TestValidateCertTime(t *testing.T) {
 
 func TestHostKeyCert(t *testing.T) {
 	cert := &Certificate{
-		ValidPrincipals: []string{"hostname", "hostname.domain"},
+		ValidPrincipals: []string{"hostname", "hostname.domain", "otherhost"},
 		Key:             testPublicKeys["rsa"],
 		ValidBefore:     CertTimeInfinity,
 		CertType:        HostCert,
 	}
 	cert.SignCert(rand.Reader, testSigners["ecdsa"])
 
 	checker := &CertChecker{
-		IsAuthority: func(p PublicKey) bool {
-			return bytes.Equal(testPublicKeys["ecdsa"].Marshal(), p.Marshal())
+		IsHostAuthority: func(p PublicKey, h string) bool {
+			return h == "hostname" && bytes.Equal(testPublicKeys["ecdsa"].Marshal(), p.Marshal())
 		},
 	}
 
@@ -178,7 +178,7 @@ func TestHostKeyCert(t *testing.T) {
 		t.Errorf("NewCertSigner: %v", err)
 	}
 
-	for _, name := range []string{"hostname", "otherhost"} {
+	for _, name := range []string{"hostname", "otherhost", "lasthost"} {
 		c1, c2, err := netPipe()
 		if err != nil {
 			t.Fatalf("netPipe: %v", err)

ssh/client_auth_test.go

@@ -38,7 +38,7 @@ func tryAuth(t *testing.T, config *ClientConfig) error {
 	defer c2.Close()
 
 	certChecker := CertChecker{
-		IsAuthority: func(k PublicKey) bool {
+		IsUserAuthority: func(k PublicKey) bool {
 			return bytes.Equal(k.Marshal(), testPublicKeys["ecdsa"].Marshal())
 		},
 		UserKeyFallback: func(conn ConnMetadata, key PublicKey) (*Permissions, error) {

ssh/knownhosts/knownhosts.go

@@ -144,11 +144,16 @@ func keyEq(a, b ssh.PublicKey) bool {
 	return bytes.Equal(a.Marshal(), b.Marshal())
 }
 
-// IsAuthority can be used as a callback in ssh.CertChecker
-func (db *hostKeyDB) IsAuthority(remote ssh.PublicKey) bool {
+// IsAuthorityForHost can be used as a callback in ssh.CertChecker
+func (db *hostKeyDB) IsHostAuthority(remote ssh.PublicKey, address string) bool {
+	h, p, err := net.SplitHostPort(address)
+	if err != nil {
+		return false
+	}
+	a := addr{host: h, port: p}
+
 	for _, l := range db.lines {
-		// TODO(hanwen): should we check the hostname against host pattern?
-		if l.cert && keyEq(l.knownKey.Key, remote) {
+		if l.cert && keyEq(l.knownKey.Key, remote) && l.match([]addr{a}) {
 			return true
 		}
 	}
@@ -409,9 +414,7 @@ func (db *hostKeyDB) Read(r io.Reader, filename string) error {
 
 // New creates a host key callback from the given OpenSSH host key
 // files. The returned callback is for use in
-// ssh.ClientConfig.HostKeyCallback. Hostnames are ignored for
-// certificates, ie. any certificate authority is assumed to be valid
-// for all remote hosts.  Hashed hostnames are not supported.
+// ssh.ClientConfig.HostKeyCallback. Hashed hostnames are not supported.
 func New(files ...string) (ssh.HostKeyCallback, error) {
 	db := newHostKeyDB()
 	for _, fn := range files {
@@ -425,12 +428,8 @@ func New(files ...string) (ssh.HostKeyCallback, error) {
 		}
 	}
 
-	// TODO(hanwen): properly supporting certificates requires an
-	// API change in the SSH library: IsAuthority should provide
-	// the address too?
-
 	var certChecker ssh.CertChecker
-	certChecker.IsAuthority = db.IsAuthority
+	certChecker.IsHostAuthority = db.IsHostAuthority
 	certChecker.IsRevoked = db.IsRevoked
 	certChecker.HostKeyFallback = db.check