ssh/agent: bound RSA modulus and DSA parameters in key parsers

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(&params); 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(&params); 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,
 	}

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)
+		}
+	}
+}