@@ -25,16 +25,6 @@ internal static void ReadDsaPrivateKey(
2525 throw new CryptographicException ( SR . Cryptography_Der_Invalid_Encoding ) ;
2626 }
2727
28- DssParms parms = DssParms . Decode ( algId . Parameters . Value , AsnEncodingRules . BER ) ;
29-
30- ret = new DSAParameters
31- {
32- P = parms . P . ToByteArray ( isUnsigned : true , isBigEndian : true ) ,
33- Q = parms . Q . ToByteArray ( isUnsigned : true , isBigEndian : true ) ,
34- } ;
35-
36- ret . G = parms . G . ExportKeyParameter ( ret . P . Length ) ;
37-
3828 BigInteger x ;
3929
4030 try
@@ -57,6 +47,34 @@ internal static void ReadDsaPrivateKey(
5747 throw new CryptographicException ( SR . Cryptography_Der_Invalid_Encoding , e ) ;
5848 }
5949
50+ DssParms parms = DssParms . Decode ( algId . Parameters . Value , AsnEncodingRules . BER ) ;
51+
52+ // Sanity checks from FIPS 186-4 4.1/4.2. Since FIPS 186-5 withdrew DSA/DSS
53+ // these will never change again.
54+ //
55+ // This technically allows a non-standard combination of 1024-bit P and 256-bit Q,
56+ // but that will get filtered out by the underlying provider.
57+ // These checks just prevent obviously bad data from wasting work on reinterpretation.
58+
59+ if ( parms . P . Sign < 0 ||
60+ parms . Q . Sign < 0 ||
61+ ! IsValidPLength ( parms . P . GetBitLength ( ) ) ||
62+ ! IsValidQLength ( parms . Q . GetBitLength ( ) ) ||
63+ parms . G <= 1 ||
64+ parms . G >= parms . P ||
65+ x <= 1 ||
66+ x >= parms . Q )
67+ {
68+ throw new CryptographicException ( SR . Cryptography_Der_Invalid_Encoding ) ;
69+ }
70+
71+ ret = new DSAParameters
72+ {
73+ P = parms . P . ToByteArray ( isUnsigned : true , isBigEndian : true ) ,
74+ Q = parms . Q . ToByteArray ( isUnsigned : true , isBigEndian : true ) ,
75+ } ;
76+
77+ ret . G = parms . G . ExportKeyParameter ( ret . P . Length ) ;
6078 ret . X = x . ExportKeyParameter ( ret . Q . Length ) ;
6179
6280 // The public key is not contained within the format, calculate it.
@@ -69,6 +87,11 @@ internal static void ReadDsaPublicKey(
6987 in AlgorithmIdentifierAsn algId ,
7088 out DSAParameters ret )
7189 {
90+ if ( ! algId . Parameters . HasValue )
91+ {
92+ throw new CryptographicException ( SR . Cryptography_Der_Invalid_Encoding ) ;
93+ }
94+
7295 BigInteger y ;
7396
7497 try
@@ -88,13 +111,27 @@ internal static void ReadDsaPublicKey(
88111 throw new CryptographicException ( SR . Cryptography_Der_Invalid_Encoding , e ) ;
89112 }
90113
91- if ( ! algId . Parameters . HasValue )
114+ DssParms parms = DssParms . Decode ( algId . Parameters . Value , AsnEncodingRules . BER ) ;
115+
116+ // Sanity checks from FIPS 186-4 4.1/4.2. Since FIPS 186-5 withdrew DSA/DSS
117+ // these will never change again.
118+ //
119+ // This technically allows a non-standard combination of 1024-bit P and 256-bit Q,
120+ // but that will get filtered out by the underlying provider.
121+ // These checks just prevent obviously bad data from wasting work on reinterpretation.
122+
123+ if ( parms . P . Sign < 0 ||
124+ parms . Q . Sign < 0 ||
125+ ! IsValidPLength ( parms . P . GetBitLength ( ) ) ||
126+ ! IsValidQLength ( parms . Q . GetBitLength ( ) ) ||
127+ parms . G <= 1 ||
128+ parms . G >= parms . P ||
129+ y <= 1 ||
130+ y >= parms . P )
92131 {
93132 throw new CryptographicException ( SR . Cryptography_Der_Invalid_Encoding ) ;
94133 }
95134
96- DssParms parms = DssParms . Decode ( algId . Parameters . Value , AsnEncodingRules . BER ) ;
97-
98135 ret = new DSAParameters
99136 {
100137 P = parms . P . ToByteArray ( isUnsigned : true , isBigEndian : true ) ,
@@ -105,6 +142,25 @@ internal static void ReadDsaPublicKey(
105142 ret . Y = y . ExportKeyParameter ( ret . P . Length ) ;
106143 }
107144
145+ private static bool IsValidPLength ( long pBitLength )
146+ {
147+ return pBitLength switch
148+ {
149+ // FIPS 186-3/186-4
150+ 1024 or 2048 or 3072 => true ,
151+ // FIPS 186-1/186-2
152+ >= 512 and < 1024 => pBitLength % 64 == 0 ,
153+ _ => false ,
154+ } ;
155+ }
156+
157+ private static bool IsValidQLength ( long qBitLength )
158+ {
159+ // FIPS 196-1/186-2 only allows 160
160+ // FIPS 186-3/186-4 allow 160/224/256
161+ return qBitLength is 160 or 224 or 256 ;
162+ }
163+
108164 internal static void ReadSubjectPublicKeyInfo (
109165 ReadOnlySpan < byte > source ,
110166 out int bytesRead ,
0 commit comments