Skip to content

Commit 9c2cd33

Browse files
drakkanthatnealpatel
authored andcommitted
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>
1 parent 8907318 commit 9c2cd33

2 files changed

Lines changed: 93 additions & 0 deletions

File tree

ssh/keys.go

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -580,6 +580,24 @@ func checkDSAParams(param *dsa.Parameters) error {
580580
return fmt.Errorf("ssh: unsupported DSA key size %d", l)
581581
}
582582

583+
// FIPS 186-2 specifies that Q must be exactly 160 bits. We must enforce
584+
// this to prevent DoS attacks where an attacker sends a huge Q which makes
585+
// verification slow.
586+
if l := param.Q.BitLen(); l != 160 {
587+
return fmt.Errorf("ssh: unsupported DSA sub-prime size %d", l)
588+
}
589+
590+
// The generator G is an element of the group, so it must be strictly less
591+
// than the modulus P.
592+
if param.G.Cmp(param.P) >= 0 {
593+
return errors.New("ssh: DSA generator larger than modulus")
594+
}
595+
596+
// G must be positive.
597+
if param.G.Sign() <= 0 {
598+
return errors.New("ssh: DSA generator must be positive")
599+
}
600+
583601
return nil
584602
}
585603

@@ -602,6 +620,14 @@ func parseDSA(in []byte) (out PublicKey, rest []byte, err error) {
602620
return nil, nil, err
603621
}
604622

623+
// The public value Y must be a non-zero element of the group, i.e.
624+
// strictly between 0 and P. crypto/dsa.Verify does not range-check Y,
625+
// so we reject out-of-range values here to prevent a maliciously
626+
// oversized Y from slowing verification.
627+
if w.Y.Sign() <= 0 || w.Y.Cmp(w.P) >= 0 {
628+
return nil, nil, errors.New("ssh: DSA public value Y out of range")
629+
}
630+
605631
key := &dsaPublicKey{
606632
Parameters: param,
607633
Y: w.Y,

ssh/keys_test.go

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import (
1919
"errors"
2020
"fmt"
2121
"io"
22+
"math/big"
2223
"reflect"
2324
"strings"
2425
"testing"
@@ -370,6 +371,72 @@ func TestMarshalParsePublicKey(t *testing.T) {
370371
}
371372
}
372373

374+
func TestParseDSAHugeQ(t *testing.T) {
375+
P := new(big.Int).Lsh(big.NewInt(1), 1023)
376+
Q := new(big.Int).Lsh(big.NewInt(1), 20000) // very large
377+
// G and Y: Dummy values, just needs to be < P to pass that specific check
378+
G := big.NewInt(2)
379+
Y := big.NewInt(5)
380+
381+
rawKey := struct {
382+
P, Q, G, Y *big.Int
383+
}{
384+
P: P,
385+
Q: Q,
386+
G: G,
387+
Y: Y,
388+
}
389+
390+
inputBytes := Marshal(&rawKey)
391+
392+
_, _, err := parseDSA(inputBytes)
393+
if err == nil {
394+
t.Fatal("parseDSA accepted a DSA key with large Q")
395+
}
396+
397+
expectedError := "ssh: unsupported DSA sub-prime size"
398+
if !strings.Contains(err.Error(), expectedError) {
399+
t.Errorf("unexpected error message: got %q, want substring %q", err.Error(), expectedError)
400+
}
401+
}
402+
403+
func TestParseDSAYOutOfRange(t *testing.T) {
404+
// Valid 1024/160 parameters (values don't need to be a real DSA group,
405+
// they only need to pass the checkDSAParams bit-length checks and the
406+
// G < P / G > 0 checks).
407+
P := new(big.Int).Lsh(big.NewInt(1), 1023)
408+
P.SetBit(P, 0, 1) // make P odd so it can pass as a prime candidate shape
409+
Q := new(big.Int).Lsh(big.NewInt(1), 159)
410+
Q.SetBit(Q, 0, 1)
411+
G := big.NewInt(2)
412+
413+
for _, tc := range []struct {
414+
name string
415+
Y *big.Int
416+
}{
417+
{"Y_zero", big.NewInt(0)},
418+
{"Y_negative", big.NewInt(-1)},
419+
{"Y_equals_P", new(big.Int).Set(P)},
420+
{"Y_greater_than_P", new(big.Int).Add(P, big.NewInt(1))},
421+
{"Y_much_greater_than_P", new(big.Int).Lsh(big.NewInt(1), 20000)},
422+
} {
423+
t.Run(tc.name, func(t *testing.T) {
424+
rawKey := struct {
425+
P, Q, G, Y *big.Int
426+
}{P: P, Q: Q, G: G, Y: tc.Y}
427+
428+
_, _, err := parseDSA(Marshal(&rawKey))
429+
if err == nil {
430+
t.Fatalf("parseDSA accepted a DSA key with Y=%s (P=%s)", tc.Y, P)
431+
}
432+
expectedError := "DSA public value Y out of range"
433+
if !strings.Contains(err.Error(), expectedError) {
434+
t.Errorf("unexpected error message: got %q, want substring %q", err.Error(), expectedError)
435+
}
436+
})
437+
}
438+
}
439+
373440
func TestMarshalPrivateKey(t *testing.T) {
374441
tests := []struct {
375442
name string

0 commit comments

Comments
 (0)