dsa: Update sanity checks

RustCrypto · May 9, 2022 · 6bc6cc1 · 6bc6cc1
1 parent 1c33224
commit 6bc6cc1
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 10 deletions.
diff --git a/dsa/src/components.rs b/dsa/src/components.rs
@@ -7,6 +7,8 @@ use num_traits::One;
 use pkcs8::der::{self, asn1::UIntRef, DecodeValue, Encode, Header, Reader, Sequence};
 use rand::{CryptoRng, RngCore};
 
+use crate::two;
+
 /// The common components of an DSA keypair
 ///
 /// (the prime p, quotient q and generator g)
@@ -63,11 +65,10 @@ impl Components {
     /// Check whether the components are valid
     #[must_use]
     pub fn is_valid(&self) -> bool {
-        if *self.p() <= BigUint::one() || *self.q() <= BigUint::one() {
-            return false;
-        }
-
-        true
+        *self.p() >= two()
+            && *self.q() >= two()
+            && *self.g() >= BigUint::one()
+            && self.g() < self.p()
     }
 }
 

diff --git a/dsa/src/privatekey.rs b/dsa/src/privatekey.rs
@@ -6,6 +6,7 @@ use crate::{Components, PublicKey, Signature, DSA_OID};
 use core::cmp::min;
 use digest::Digest;
 use num_bigint::BigUint;
+use num_traits::One;
 use pkcs8::{
     der::{asn1::UIntRef, AnyRef, Decode, Encode},
     AlgorithmIdentifier, DecodePrivateKey, EncodePrivateKey, PrivateKeyInfo, SecretDocument,
@@ -61,7 +62,11 @@ impl PrivateKey {
     /// Check whether the private key is valid
     #[must_use]
     pub fn is_valid(&self) -> bool {
-        self.public_key().components().is_valid()
+        if !self.public_key().is_valid() {
+            return false;
+        }
+
+        *self.x() >= BigUint::one() && self.x() < self.public_key().components().q()
     }
 
     /// Sign data with the private key
@@ -120,7 +125,11 @@ impl<'a> TryFrom<PrivateKeyInfo<'a>> for PrivateKey {
         value.algorithm.assert_algorithm_oid(DSA_OID)?;
 
         let parameters = value.algorithm.parameters_any()?;
-        let components = parameters.decode_into()?;
+        let components: Components = parameters.decode_into()?;
+
+        if !components.is_valid() {
+            return Err(pkcs8::Error::KeyMalformed);
+        }
 
         let x = UIntRef::from_der(value.private_key)?;
         let x = BigUint::from_bytes_be(x.as_bytes());
@@ -133,7 +142,13 @@ impl<'a> TryFrom<PrivateKeyInfo<'a>> for PrivateKey {
         };
 
         let public_key = PublicKey::from_components(components, y);
-        Ok(PrivateKey::from_components(public_key, x))
+        let private_key = PrivateKey::from_components(public_key, x);
+
+        if !private_key.is_valid() {
+            return Err(pkcs8::Error::KeyMalformed);
+        }
+
+        Ok(private_key)
     }
 }
 

diff --git a/dsa/src/publickey.rs b/dsa/src/publickey.rs
@@ -2,7 +2,7 @@
 //! Module containing the definition of the public key container
 //!
 
-use crate::{Components, Signature, DSA_OID};
+use crate::{two, Components, Signature, DSA_OID};
 use core::cmp::min;
 use digest::Digest;
 use num_bigint::{BigUint, ModInverse};
@@ -52,7 +52,7 @@ impl PublicKey {
             return false;
         }
 
-        self.y().modpow(components.q(), components.p()) == BigUint::one()
+        *self.y() >= two() && self.y().modpow(components.q(), components.p()) == BigUint::one()
     }
 
     /// Verify if the signature matches the provided hash