dsa: handle NonZero/Odd conversions on BoxedUint internally (#998)

Makes `BoxedUint` the one unsigned integer type in the public API,
converting to `NonZero<BoxedUint>` and `Odd<BoxedUint>` as needed.

This makes the API simpler to use for end-users, with fewer types for
them to manage.

Closes #987

Author: tarcieri

Cargo.lock

@@ -15,6 +15,8 @@ members = [
 opt-level = 2
 
 [patch.crates-io]
+elliptic-curve = { git = "https://github.com/RustCrypto/traits.git" }
+
 # A global patch crates-io block is used to avoid duplicate dependencies
 # when pulling a member crate through git
 dsa = { path = "./dsa" }

Cargo.toml

@@ -17,7 +17,7 @@ rust-version = "1.85"
 
 [dependencies]
 digest = "0.11.0-rc.0"
-crypto-bigint = { version = "=0.7.0-pre.4", default-features = false, features = ["alloc", "zeroize"] }
+crypto-bigint = { version = "=0.7.0-pre.5", default-features = false, features = ["alloc", "zeroize"] }
 crypto-primes = { version = "=0.7.0-pre.1", default-features = false }
 pkcs8 = { version = "0.11.0-rc.1", default-features = false, features = ["alloc"] }
 rfc6979 = { version = "0.5.0-rc.0" }

dsa/Cargo.toml

@@ -30,11 +30,17 @@ pub struct Components {
 
 impl Components {
     /// Construct the common components container from its inner values (p, q and g)
-    pub fn from_components(
-        p: Odd<BoxedUint>,
-        q: NonZero<BoxedUint>,
-        g: NonZero<BoxedUint>,
-    ) -> signature::Result<Self> {
+    pub fn from_components(p: BoxedUint, q: BoxedUint, g: BoxedUint) -> signature::Result<Self> {
+        let p = Odd::new(p)
+            .into_option()
+            .ok_or_else(signature::Error::new)?;
+        let q = NonZero::new(q)
+            .into_option()
+            .ok_or_else(signature::Error::new)?;
+        let g = NonZero::new(g)
+            .into_option()
+            .ok_or_else(signature::Error::new)?;
+
         if *p < two() || *q < two() || *g > *p {
             return Err(signature::Error::new());
         }
@@ -54,7 +60,8 @@ impl Components {
     /// Generate a new pair of common components
     pub fn generate<R: CryptoRng + ?Sized>(rng: &mut R, key_size: KeySize) -> Self {
         let (p, q, g) = crate::generate::common_components(rng, key_size);
-        Self::from_components(p, q, g).expect("[Bug] Newly generated components considered invalid")
+        Self::from_components(p.get(), q.get(), g.get())
+            .expect("[Bug] Newly generated components considered invalid")
     }
 
     /// DSA prime p
@@ -84,22 +91,9 @@ impl<'a> DecodeValue<'a> for Components {
         let q = reader.decode::<UintRef<'_>>()?;
         let g = reader.decode::<UintRef<'_>>()?;
 
-        let p = BoxedUint::from_be_slice(p.as_bytes(), (p.as_bytes().len() * 8) as u32)
-            .expect("invariant violation");
-        let q = BoxedUint::from_be_slice(q.as_bytes(), (q.as_bytes().len() * 8) as u32)
-            .expect("invariant violation");
-        let g = BoxedUint::from_be_slice(g.as_bytes(), (g.as_bytes().len() * 8) as u32)
-            .expect("invariant violation");
-
-        let p = Odd::new(p)
-            .into_option()
-            .ok_or(Tag::Integer.value_error())?;
-        let q = NonZero::new(q)
-            .into_option()
-            .ok_or(Tag::Integer.value_error())?;
-        let g = NonZero::new(g)
-            .into_option()
-            .ok_or(Tag::Integer.value_error())?;
+        let p = BoxedUint::from_be_slice_vartime(p.as_bytes());
+        let q = BoxedUint::from_be_slice_vartime(q.as_bytes());
+        let g = BoxedUint::from_be_slice_vartime(g.as_bytes());
 
         Self::from_components(p, q, g).map_err(|_| Tag::Integer.value_error())
     }

dsa/src/components.rs

@@ -39,7 +39,7 @@ pub fn keypair<R: CryptoRng + ?Sized>(rng: &mut R, components: Components) -> Si
 
     let (x, y) = find_components(rng, &components);
 
-    VerifyingKey::from_components(components, y)
-        .and_then(|verifying_key| SigningKey::from_components(verifying_key, x))
+    VerifyingKey::from_components(components, y.get())
+        .and_then(|verifying_key| SigningKey::from_components(verifying_key, x.get()))
         .expect("[Bug] Newly generated keypair considered invalid")
 }

dsa/src/generate/keypair.rs

@@ -29,12 +29,12 @@
 //! # use crypto_bigint::{BoxedUint, NonZero, Odd};
 //! # let read_common_parameters = ||
 //! #     (
-//! #          Odd::new(BoxedUint::one()).unwrap(),
-//! #          NonZero::new(BoxedUint::one()).unwrap(),
-//! #          NonZero::new(BoxedUint::one()).unwrap(),
+//! #          BoxedUint::one(),
+//! #          BoxedUint::one(),
+//! #          BoxedUint::one(),
 //! #     );
-//! # let read_public_component = || NonZero::new(BoxedUint::one()).unwrap();
-//! # let read_private_component = || NonZero::new(BoxedUint::one()).unwrap();
+//! # let read_public_component = || BoxedUint::one();
+//! # let read_private_component = || BoxedUint::one();
 //! # || -> signature::Result<()> {
 //! let (p, q, g) = read_common_parameters();
 //! let components = Components::from_components(p, q, g)?;
@@ -57,10 +57,11 @@ pub use crate::signing_key::SigningKey;
 
 pub use crate::{components::Components, size::KeySize, verifying_key::VerifyingKey};
 
-pub use crypto_bigint::{BoxedUint, NonZero, Odd};
+pub use crypto_bigint::BoxedUint;
 pub use pkcs8;
 pub use signature;
 
+use crypto_bigint::NonZero;
 use pkcs8::spki::ObjectIdentifier;
 
 mod components;
@@ -94,8 +95,10 @@ pub struct Signature {
 
 impl Signature {
     /// Create a new Signature container from its components
-    pub fn from_components(r: NonZero<BoxedUint>, s: NonZero<BoxedUint>) -> Self {
-        Self { r, s }
+    pub fn from_components(r: BoxedUint, s: BoxedUint) -> Option<Self> {
+        let r = NonZero::new(r).into_option()?;
+        let s = NonZero::new(s).into_option()?;
+        Some(Self { r, s })
     }
 
     /// Signature part r
@@ -124,14 +127,7 @@ impl<'a> DecodeValue<'a> for Signature {
             let s = BoxedUint::from_be_slice(s.as_bytes(), s.as_bytes().len() as u32 * 8)
                 .map_err(|_| UintRef::TAG.value_error())?;
 
-            let r = NonZero::new(r)
-                .into_option()
-                .ok_or(UintRef::TAG.value_error())?;
-            let s = NonZero::new(s)
-                .into_option()
-                .ok_or(UintRef::TAG.value_error())?;
-
-            Ok(Self::from_components(r, s))
+            Self::from_components(r, s).ok_or_else(|| UintRef::TAG.value_error())
         })
     }
 }

dsa/src/lib.rs

@@ -44,10 +44,11 @@ pub struct SigningKey {
 
 impl SigningKey {
     /// Construct a new private key from the public key and private component
-    pub fn from_components(
-        verifying_key: VerifyingKey,
-        x: NonZero<BoxedUint>,
-    ) -> signature::Result<Self> {
+    pub fn from_components(verifying_key: VerifyingKey, x: BoxedUint) -> signature::Result<Self> {
+        let x = NonZero::new(x)
+            .into_option()
+            .ok_or_else(signature::Error::new)?;
+
         if x > *verifying_key.components().q() {
             return Err(signature::Error::new());
         }
@@ -116,26 +117,19 @@ impl SigningKey {
         debug_assert_eq!(key_size.l_aligned(), r.bits_precision());
 
         let r_short = r.clone().resize(key_size.n_aligned());
-        let r_short = NonZero::new(r_short)
-            .expect("[bug] invalid value of k used here, the secret number computed was invalid");
-        let r = NonZero::new(r)
-            .expect("[bug] invalid value of k used here, the secret number computed was invalid");
-
         let n = q.bits() / 8;
         let block_size = hash.len(); // Hash function output size
 
         let z_len = min(n as usize, block_size);
         let z = BoxedUint::from_be_slice(&hash[..z_len], z_len as u32 * 8)
             .expect("invariant violation");
 
-        let s = inv_k.mul_mod(&(z + &**x * &*r), &q.resize(key_size.l_aligned()));
+        let s = inv_k.mul_mod(&(z + &**x * &r), &q.resize(key_size.l_aligned()));
         let s = s.resize(key_size.n_aligned());
-        let s = NonZero::new(s)
-            .expect("[bug] invalid value of k used here, the secret number computed was invalid");
 
         debug_assert_eq!(key_size.n_aligned(), r_short.bits_precision());
         debug_assert_eq!(key_size.n_aligned(), s.bits_precision());
-        let signature = Signature::from_components(r_short, s);
+        let signature = Signature::from_components(r_short, s).ok_or_else(signature::Error::new)?;
 
         if signature.r() < q && signature.s() < q {
             Ok(signature)
@@ -260,21 +254,19 @@ impl<'a> TryFrom<PrivateKeyInfoRef<'a>> for SigningKey {
 
         let y = if let Some(y_bytes) = value.public_key.as_ref().and_then(|bs| bs.as_bytes()) {
             let y = UintRef::from_der(y_bytes)?;
-            let y = BoxedUint::from_be_slice(y.as_bytes(), precision)
-                .map_err(|_| pkcs8::Error::KeyMalformed)?;
-            NonZero::new(y)
-                .into_option()
-                .ok_or(pkcs8::Error::KeyMalformed)?
+            BoxedUint::from_be_slice(y.as_bytes(), precision)
+                .map_err(|_| pkcs8::Error::KeyMalformed)?
         } else {
             crate::generate::public_component(&components, &x)
                 .into_option()
                 .ok_or(pkcs8::Error::KeyMalformed)?
+                .get()
         };
 
         let verifying_key =
             VerifyingKey::from_components(components, y).map_err(|_| pkcs8::Error::KeyMalformed)?;
 
-        SigningKey::from_components(verifying_key, x).map_err(|_| pkcs8::Error::KeyMalformed)
+        SigningKey::from_components(verifying_key, x.get()).map_err(|_| pkcs8::Error::KeyMalformed)
     }
 }
 

dsa/src/signing_key.rs

@@ -32,10 +32,11 @@ pub struct VerifyingKey {
 
 impl VerifyingKey {
     /// Construct a new public key from the common components and the public component
-    pub fn from_components(
-        components: Components,
-        y: NonZero<BoxedUint>,
-    ) -> signature::Result<Self> {
+    pub fn from_components(components: Components, y: BoxedUint) -> signature::Result<Self> {
+        let y = NonZero::new(y)
+            .into_option()
+            .ok_or_else(signature::Error::new)?;
+
         let params = BoxedMontyParams::new_vartime(components.p().clone());
         let form = BoxedMontyForm::new((*y).clone(), params);
 
@@ -192,13 +193,7 @@ impl<'a> TryFrom<SubjectPublicKeyInfoRef<'a>> for VerifyingKey {
                 .as_bytes()
                 .ok_or(spki::Error::KeyMalformed)?,
         )?;
-
-        let y = NonZero::new(
-            BoxedUint::from_be_slice(y.as_bytes(), y.as_bytes().len() as u32 * 8)
-                .expect("invariant violation"),
-        )
-        .into_option()
-        .ok_or(spki::Error::KeyMalformed)?;
+        let y = BoxedUint::from_be_slice_vartime(y.as_bytes());
 
         Self::from_components(components, y).map_err(|_| spki::Error::KeyMalformed)
     }

dsa/src/verifying_key.rs

@@ -1,5 +1,5 @@
 #![cfg(feature = "hazmat")]
-use crypto_bigint::{BoxedUint, NonZero, Odd};
+use crypto_bigint::BoxedUint;
 use digest::{Digest, FixedOutputReset, block_api::BlockSizeUser};
 use dsa::{Components, Signature, SigningKey, VerifyingKey};
 use sha1::Sha1;
@@ -39,14 +39,6 @@ fn dsa_1024_signing_key() -> SigningKey {
             82F65CBDC4FAE93C2EA212390E54905A86E2223170B44EAA7DA5DD9FFCFB7F3B";
     let y = decode_hex_number(y_str, 1024);
 
-    let (p, q, g, y, x) = (
-        Odd::new(p).unwrap(),
-        NonZero::new(q).unwrap(),
-        NonZero::new(g).unwrap(),
-        NonZero::new(y).unwrap(),
-        NonZero::new(x).unwrap(),
-    );
-
     let components = Components::from_components(p, q, g).expect("Invalid components");
     let verifying_key =
         VerifyingKey::from_components(components, y).expect("Invalid verifying key");
@@ -98,14 +90,6 @@ fn dsa_2048_signing_key() -> SigningKey {
         2048,
     );
 
-    let (p, q, g, y, x) = (
-        Odd::new(p).unwrap(),
-        NonZero::new(q).unwrap(),
-        NonZero::new(g).unwrap(),
-        NonZero::new(y).unwrap(),
-        NonZero::new(x).unwrap(),
-    );
-
     let components = Components::from_components(p, q, g).expect("Invalid components");
     let verifying_key =
         VerifyingKey::from_components(components, y).expect("Invalid verifying key");
@@ -144,9 +128,10 @@ fn from_str_signature(r: &str, s: &str) -> Signature {
     let precision = (r.len() * 8) as u32;
 
     Signature::from_components(
-        NonZero::new(decode_hex_number(r, precision)).unwrap(),
-        NonZero::new(decode_hex_number(s, precision)).unwrap(),
+        decode_hex_number(r, precision),
+        decode_hex_number(s, precision),
     )
+    .unwrap()
 }
 
 /// Return the RFC 6979 test cases