From cde0ba3655fa07de85232a27cb1546ad13442130 Mon Sep 17 00:00:00 2001 From: tonghuaroot Date: Fri, 29 May 2026 09:14:20 +0800 Subject: [PATCH] ssh/agent: bound RSA modulus and DSA parameters in key parsers The agent server's key parsers accept attacker-controlled RSA moduli and DSA P/Q/G parameters of arbitrary size from SSH_AGENTC_ADD_IDENTITY and SSH2_AGENTC_ADD_ID_CONSTRAINED messages, then run expensive crypto such as rsa.PrivateKey.Precompute with no size bound. A single oversized key (e.g. a 16384-bit RSA modulus) can cost hundreds of milliseconds to process, giving large CPU amplification per packet and allowing a malicious remote server reached over agent forwarding to peg a Go-based ssh-agent. Bound the sizes in parseRSAKey, parseRSACert, parseDSAKey, and parseDSACert, mirroring the limits the same package already enforces on the SSH-server side in ssh/keys.go: an 8192-bit RSA modulus cap and the FIPS 186-2 DSA parameter checks (1024-bit P, 160-bit Q, and a generator in range). Fixes golang/go#79725 --- ssh/agent/server.go | 64 ++++++++++++++++++++++++++++++++-------- ssh/agent/server_test.go | 35 ++++++++++++++++++++++ 2 files changed, 87 insertions(+), 12 deletions(-) diff --git a/ssh/agent/server.go b/ssh/agent/server.go index 248ab6a9f9..9e08e61861 100644 --- a/ssh/agent/server.go +++ b/ssh/agent/server.go @@ -240,6 +240,32 @@ func setConstraints(key *AddedKey, constraintBytes []byte) error { return nil } +// maxRSAKeyBits is the maximum supported RSA modulus size in bits. It mirrors +// the bound enforced by ssh/keys.go on the SSH-server side and prevents an +// attacker from supplying a huge modulus that makes key precomputation +// (rsa.PrivateKey.Precompute) and later operations prohibitively expensive. +const maxRSAKeyBits = 8192 + +// checkDSAParams enforces the same FIPS 186-2 DSA parameter bounds as +// ssh/keys.go. SSH specifies a single 1024-bit P with a 160-bit Q, and +// bounding these prevents a DoS where an attacker supplies oversized +// parameters that make verification slow. +func checkDSAParams(param *dsa.Parameters) error { + if l := param.P.BitLen(); l != 1024 { + return fmt.Errorf("agent: unsupported DSA key size %d", l) + } + if l := param.Q.BitLen(); l != 160 { + return fmt.Errorf("agent: unsupported DSA sub-prime size %d", l) + } + if param.G.Cmp(param.P) >= 0 { + return errors.New("agent: DSA generator larger than modulus") + } + if param.G.Sign() <= 0 { + return errors.New("agent: DSA generator must be positive") + } + return nil +} + func parseRSAKey(req []byte) (*AddedKey, error) { var k rsaKeyMsg if err := ssh.Unmarshal(req, &k); err != nil { @@ -248,6 +274,9 @@ func parseRSAKey(req []byte) (*AddedKey, error) { if k.E.BitLen() > 30 { return nil, errors.New("agent: RSA public exponent too large") } + if k.N.BitLen() > maxRSAKeyBits { + return nil, errors.New("agent: RSA modulus too large") + } priv := &rsa.PrivateKey{ PublicKey: rsa.PublicKey{ E: int(k.E.Int64()), @@ -287,14 +316,18 @@ func parseDSAKey(req []byte) (*AddedKey, error) { if err := ssh.Unmarshal(req, &k); err != nil { return nil, err } + params := dsa.Parameters{ + P: k.P, + Q: k.Q, + G: k.G, + } + if err := checkDSAParams(¶ms); err != nil { + return nil, err + } priv := &dsa.PrivateKey{ PublicKey: dsa.PublicKey{ - Parameters: dsa.Parameters{ - P: k.P, - Q: k.Q, - G: k.G, - }, - Y: k.Y, + Parameters: params, + Y: k.Y, }, X: k.X, } @@ -402,6 +435,9 @@ func parseRSACert(req []byte) (*AddedKey, error) { if rsaPub.E.BitLen() > 30 { return nil, errors.New("agent: RSA public exponent too large") } + if rsaPub.N.BitLen() > maxRSAKeyBits { + return nil, errors.New("agent: RSA modulus too large") + } priv := rsa.PrivateKey{ PublicKey: rsa.PublicKey{ @@ -443,14 +479,18 @@ func parseDSACert(req []byte) (*AddedKey, error) { return nil, fmt.Errorf("agent: Unmarshal failed to parse public key: %v", err) } + params := dsa.Parameters{ + P: w.P, + Q: w.Q, + G: w.G, + } + if err := checkDSAParams(¶ms); err != nil { + return nil, err + } priv := &dsa.PrivateKey{ PublicKey: dsa.PublicKey{ - Parameters: dsa.Parameters{ - P: w.P, - Q: w.Q, - G: w.G, - }, - Y: w.Y, + Parameters: params, + Y: w.Y, }, X: k.X, } diff --git a/ssh/agent/server_test.go b/ssh/agent/server_test.go index 9e790eb428..28fe2aa48b 100644 --- a/ssh/agent/server_test.go +++ b/ssh/agent/server_test.go @@ -9,6 +9,7 @@ import ( "crypto/rand" "fmt" "io" + "math/big" pseudorand "math/rand" "reflect" "strings" @@ -303,3 +304,37 @@ func TestParseEd25519KeyShortPanic(t *testing.T) { t.Error("short ed25519 key was accepted into keyring") } } + +func TestParseRSAKeyModulusBound(t *testing.T) { + // An oversized modulus must be rejected before the expensive + // rsa.PrivateKey.Precompute call, while an in-bounds key still parses. + for _, tc := range []struct { + bits int + wantError bool + }{ + {bits: maxRSAKeyBits, wantError: false}, + {bits: maxRSAKeyBits + 1, wantError: true}, + {bits: 16384, wantError: true}, + } { + // Construct a modulus with exactly tc.bits bits without generating a + // real (expensive) key: setting the top bit suffices for BitLen. + n := new(big.Int).Lsh(big.NewInt(1), uint(tc.bits-1)) + msg := ssh.Marshal(rsaKeyMsg{ + Type: ssh.KeyAlgoRSA, + N: n, + E: big.NewInt(65537), + D: big.NewInt(1), + Iqmp: big.NewInt(1), + P: big.NewInt(1), + Q: big.NewInt(1), + }) + _, err := parseRSAKey(msg) + if tc.wantError { + if err == nil || !strings.Contains(err.Error(), "RSA modulus too large") { + t.Errorf("%d-bit modulus: got error %v, want \"RSA modulus too large\"", tc.bits, err) + } + } else if err != nil { + t.Errorf("%d-bit modulus: unexpected error: %v", tc.bits, err) + } + } +}