From 90f76b8ffe1453c472892d338785687e9727bcc0 Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Sun, 19 Jul 2026 21:07:53 +0200 Subject: [PATCH] ssh: reject certificate signature keys before recursing parseCert parsed a certificate's SignatureKey by calling ParsePublicKey again, and only afterwards rejected a certificate used as a signing key. For a chain of nested certificates this recursed once per level, so a key with enough nesting exhausted the goroutine stack and crashed with a fatal stack overflow that recover cannot catch. Reject the signature key by its declared algorithm before parsing it, which bounds parsing. Fixes golang/go#80414 Change-Id: Id51401aae15bfbec006c03ab97635a3b68bd6058 Reviewed-on: https://go-review.googlesource.com/c/crypto/+/802145 Auto-Submit: Nicola Murino Reviewed-by: Carlos Amedee LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com Reviewed-by: Cherry Mui Reviewed-by: Filippo Valsorda --- ssh/certs.go | 15 ++++++++++----- ssh/certs_test.go | 29 +++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/ssh/certs.go b/ssh/certs.go index 6f75d77..fa848f5 100644 --- a/ssh/certs.go +++ b/ssh/certs.go @@ -229,15 +229,20 @@ func parseCert(in []byte, privAlgo string) (*Certificate, error) { return nil, err } c.Reserved = g.Reserved + // Reject a certificate whose signature key is itself a certificate before + // parsing it. Certificates signed by certificates are not supported (see + // PROTOCOL.certkeys), and rejecting after ParsePublicKey returns would allow + // a chain of nested certificates to recurse once per level, exhausting the + // goroutine stack. + if sigAlgo, _, ok := parseString(g.SignatureKey); !ok { + return nil, errShortRead + } else if _, ok := certKeyAlgoNames[string(sigAlgo)]; ok { + return nil, fmt.Errorf("ssh: the signature key type %q is invalid for certificates", sigAlgo) + } k, err := ParsePublicKey(g.SignatureKey) if err != nil { return nil, 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[k.Type()]; ok { - return nil, fmt.Errorf("ssh: the signature key type %q is invalid for certificates", k.Type()) - } c.SignatureKey = k c.Signature, rest, ok = parseSignatureBody(g.Signature) if !ok || len(rest) > 0 { diff --git a/ssh/certs_test.go b/ssh/certs_test.go index 43280e3..8358b33 100644 --- a/ssh/certs_test.go +++ b/ssh/certs_test.go @@ -15,6 +15,7 @@ import ( "math/big" "net" "reflect" + "strings" "testing" "time" @@ -45,6 +46,34 @@ func TestParseCert(t *testing.T) { } } +func TestParseCertNestedSignatureKey(t *testing.T) { + signer, err := NewSignerFromKey(testPrivateKeys["ed25519"]) + if err != nil { + t.Fatal(err) + } + + cert := &Certificate{ + Key: signer.PublicKey(), + CertType: UserCert, + ValidBefore: CertTimeInfinity, + } + if err := cert.SignCert(rand.Reader, signer); err != nil { + t.Fatal(err) + } + + inner := *cert + cert.SignatureKey = &inner + blob := cert.Marshal() + + _, err = ParsePublicKey(blob) + if err == nil { + t.Fatal("ParsePublicKey: expected error for certificate signed by a certificate, got nil") + } + if !strings.Contains(err.Error(), "invalid for certificates") { + t.Errorf("ParsePublicKey: got error %q, want it to mention the signature key is invalid for certificates", err) + } +} + // Cert generated by ssh-keygen OpenSSH_6.8p1 OS X 10.10.3 // % ssh-keygen -s ca -I testcert -O source-address=192.168.1.0/24 -O force-command=/bin/sleep user.pub // user.pub key: ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABAQDACh1rt2DXfV3hk6fszSQcQ/rueMId0kVD9U7nl8cfEnFxqOCrNT92g4laQIGl2mn8lsGZfTLg8ksHq3gkvgO3oo/0wHy4v32JeBOHTsN5AL4gfHNEhWeWb50ev47hnTsRIt9P4dxogeUo/hTu7j9+s9lLpEQXCvq6xocXQt0j8MV9qZBBXFLXVT3cWIkSqOdwt/5ZBg+1GSrc7WfCXVWgTk4a20uPMuJPxU4RQwZW6X3+O8Pqo8C3cW0OzZRFP6gUYUKUsTI5WntlS+LAxgw1mZNsozFGdbiOPRnEryE3SRldh9vjDR3tin1fGpA5P7+CEB/bqaXtG3V+F2OkqaMN