ssh: reject certificate keys used as signature keys for SSH certs

As specified in draft-miller-ssh-cert-01, Section 2.1.1:
Implementations MUST NOT accept certificate keys as CA keys.

Change-Id: I2e559a8a58b7bceccd0d8c6b80803abdbe281067
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/678715
Reviewed-by: Filippo Valsorda <filippo@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Nicola Murino <nicola.murino@gmail.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: David Chase <drchase@google.com>

3 files changed, 38 insertions, 1 deletions

diff --git a/ssh/certs.go b/ssh/certs.go
index a3dc629..4f535eb 100644
--- a/ssh/certs.go
+++ b/ssh/certs.go
@@ -447,12 +447,19 @@ func (c *CertChecker) CheckCert(principal string, cert *Certificate) error {
 // SignCert signs the certificate with an authority, setting the Nonce,
 // SignatureKey, and Signature fields. If the authority implements the
 // MultiAlgorithmSigner interface the first algorithm in the list is used. This
-// is useful if you want to sign with a specific algorithm.
+// is useful if you want to sign with a specific algorithm. As specified in
+// [SSH-CERTS], Section 2.1.1, authority can't be a [Certificate].
 func (c *Certificate) SignCert(rand io.Reader, authority Signer) error {
 	c.Nonce = make([]byte, 32)
 	if _, err := io.ReadFull(rand, c.Nonce); err != nil {
 		return err
 	}
+	// The Type() function is intended to return only certificate key types, but
+	// we use certKeyAlgoNames anyway for safety, to match [Certificate.Type].
+	if _, ok := certKeyAlgoNames[authority.PublicKey().Type()]; ok {
+		return fmt.Errorf("ssh: certificates cannot be used as authority (public key type %q)",
+			authority.PublicKey().Type())
+	}
 	c.SignatureKey = authority.PublicKey()
 
 	if v, ok := authority.(MultiAlgorithmSigner); ok {
diff --git a/ssh/certs_test.go b/ssh/certs_test.go
index 1a4b499..e2a6fed 100644
--- a/ssh/certs_test.go
+++ b/ssh/certs_test.go
@@ -404,3 +404,32 @@ func TestCertSignWithMultiAlgorithmSigner(t *testing.T) {
 		})
 	}
 }
+
+func TestCertSignWithCertificate(t *testing.T) {
+	cert := &Certificate{
+		Key:         testPublicKeys["rsa"],
+		ValidBefore: CertTimeInfinity,
+		CertType:    UserCert,
+	}
+	if err := cert.SignCert(rand.Reader, testSigners["ecdsa"]); err != nil {
+		t.Fatalf("SignCert: %v", err)
+	}
+	signer, err := NewSignerWithAlgorithms(testSigners["rsa"].(AlgorithmSigner), []string{KeyAlgoRSASHA256})
+	if err != nil {
+		t.Fatal(err)
+	}
+	certSigner, err := NewCertSigner(cert, signer)
+	if err != nil {
+		t.Fatalf("NewCertSigner: %v", err)
+	}
+
+	cert1 := &Certificate{
+		Key:         testPublicKeys["ecdsa"],
+		ValidBefore: CertTimeInfinity,
+		CertType:    UserCert,
+	}
+
+	if err := cert1.SignCert(rand.Reader, certSigner); err == nil {
+		t.Fatal("successfully signed a certificate using another certificate, it is expected to fail")
+	}
+}
diff --git a/ssh/doc.go b/ssh/doc.go
index f5d352f..04ccce3 100644
--- a/ssh/doc.go
+++ b/ssh/doc.go
@@ -16,6 +16,7 @@ References:
 	[PROTOCOL]: https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/ssh/PROTOCOL?rev=HEAD
 	[PROTOCOL.certkeys]: http://cvsweb.openbsd.org/cgi-bin/cvsweb/src/usr.bin/ssh/PROTOCOL.certkeys?rev=HEAD
 	[SSH-PARAMETERS]:    http://www.iana.org/assignments/ssh-parameters/ssh-parameters.xml#ssh-parameters-1
+	[SSH-CERTS]:	https://datatracker.ietf.org/doc/html/draft-miller-ssh-cert-01
 
 This package does not fall under the stability promise of the Go language itself,
 so its API may be changed when pressing needs arise.