ssh: enforce strict limits on DSA key parameters The parseDSA function previously accepted DSA keys with arbitrary values for the sub-prime Q and did not validate that group elements G and Y were within the modulus P. Malicious actors could provide a key with a massively large Q (e.g., millions of bits), leading to excessive CPU consumption during signature verification. This change restricts the sub-prime Q to exactly 160 bits, as required by FIPS 186-2, and ensures that G and Y are strictly less than P. This issue was found during a security audit by NCC Group Cryptography Services, sponsored by Teleport. Fixes golang/go#79565 Fixes CVE-2026-39829 Change-Id: I526118d94684076088d0625178844f64c1303ec8 Reviewed-on: https://go-review.googlesource.com/c/crypto/+/781661 Reviewed-by: Roland Shoemaker <roland@golang.org> LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Neal Patel <nealpatel@google.com>
diff --git a/ssh/keys.go b/ssh/keys.go index 47b2c45..2b53d95 100644 --- a/ssh/keys.go +++ b/ssh/keys.go
@@ -580,6 +580,24 @@ return fmt.Errorf("ssh: unsupported DSA key size %d", l) } + // FIPS 186-2 specifies that Q must be exactly 160 bits. We must enforce + // this to prevent DoS attacks where an attacker sends a huge Q which makes + // verification slow. + if l := param.Q.BitLen(); l != 160 { + return fmt.Errorf("ssh: unsupported DSA sub-prime size %d", l) + } + + // The generator G is an element of the group, so it must be strictly less + // than the modulus P. + if param.G.Cmp(param.P) >= 0 { + return errors.New("ssh: DSA generator larger than modulus") + } + + // G must be positive. + if param.G.Sign() <= 0 { + return errors.New("ssh: DSA generator must be positive") + } + return nil } @@ -602,6 +620,14 @@ return nil, nil, err } + // The public value Y must be a non-zero element of the group, i.e. + // strictly between 0 and P. crypto/dsa.Verify does not range-check Y, + // so we reject out-of-range values here to prevent a maliciously + // oversized Y from slowing verification. + if w.Y.Sign() <= 0 || w.Y.Cmp(w.P) >= 0 { + return nil, nil, errors.New("ssh: DSA public value Y out of range") + } + key := &dsaPublicKey{ Parameters: param, Y: w.Y,
diff --git a/ssh/keys_test.go b/ssh/keys_test.go index 704ad43..6fd5a4f 100644 --- a/ssh/keys_test.go +++ b/ssh/keys_test.go
@@ -19,6 +19,7 @@ "errors" "fmt" "io" + "math/big" "reflect" "strings" "testing" @@ -370,6 +371,72 @@ } } +func TestParseDSAHugeQ(t *testing.T) { + P := new(big.Int).Lsh(big.NewInt(1), 1023) + Q := new(big.Int).Lsh(big.NewInt(1), 20000) // very large + // G and Y: Dummy values, just needs to be < P to pass that specific check + G := big.NewInt(2) + Y := big.NewInt(5) + + rawKey := struct { + P, Q, G, Y *big.Int + }{ + P: P, + Q: Q, + G: G, + Y: Y, + } + + inputBytes := Marshal(&rawKey) + + _, _, err := parseDSA(inputBytes) + if err == nil { + t.Fatal("parseDSA accepted a DSA key with large Q") + } + + expectedError := "ssh: unsupported DSA sub-prime size" + if !strings.Contains(err.Error(), expectedError) { + t.Errorf("unexpected error message: got %q, want substring %q", err.Error(), expectedError) + } +} + +func TestParseDSAYOutOfRange(t *testing.T) { + // Valid 1024/160 parameters (values don't need to be a real DSA group, + // they only need to pass the checkDSAParams bit-length checks and the + // G < P / G > 0 checks). + P := new(big.Int).Lsh(big.NewInt(1), 1023) + P.SetBit(P, 0, 1) // make P odd so it can pass as a prime candidate shape + Q := new(big.Int).Lsh(big.NewInt(1), 159) + Q.SetBit(Q, 0, 1) + G := big.NewInt(2) + + for _, tc := range []struct { + name string + Y *big.Int + }{ + {"Y_zero", big.NewInt(0)}, + {"Y_negative", big.NewInt(-1)}, + {"Y_equals_P", new(big.Int).Set(P)}, + {"Y_greater_than_P", new(big.Int).Add(P, big.NewInt(1))}, + {"Y_much_greater_than_P", new(big.Int).Lsh(big.NewInt(1), 20000)}, + } { + t.Run(tc.name, func(t *testing.T) { + rawKey := struct { + P, Q, G, Y *big.Int + }{P: P, Q: Q, G: G, Y: tc.Y} + + _, _, err := parseDSA(Marshal(&rawKey)) + if err == nil { + t.Fatalf("parseDSA accepted a DSA key with Y=%s (P=%s)", tc.Y, P) + } + expectedError := "DSA public value Y out of range" + if !strings.Contains(err.Error(), expectedError) { + t.Errorf("unexpected error message: got %q, want substring %q", err.Error(), expectedError) + } + }) + } +} + func TestMarshalPrivateKey(t *testing.T) { tests := []struct { name string