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>
This commit is contained in:
committed by
Neal Patel
parent
890731877d
commit
9c2cd33e8d
+26
@@ -580,6 +580,24 @@ func checkDSAParams(param *dsa.Parameters) error {
|
||||
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 @@ func parseDSA(in []byte) (out PublicKey, rest []byte, err error) {
|
||||
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,
|
||||
|
||||
@@ -19,6 +19,7 @@ import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
"math/big"
|
||||
"reflect"
|
||||
"strings"
|
||||
"testing"
|
||||
@@ -370,6 +371,72 @@ func TestMarshalParsePublicKey(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
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
|
||||
|
||||
Reference in New Issue
Block a user