dsa: enable and fix workspace-level lints (#1406)

Author: tarcieri

dsa/Cargo.toml

@@ -45,9 +45,6 @@ default = ["pkcs8"]
 getrandom = ["crypto-common/getrandom"]
 hazmat = []
 
-[package.metadata.docs.rs]
-all-features = true
-
 [[example]]
 name = "sign"
 required-features = ["hazmat", "pkcs8"]
@@ -59,3 +56,9 @@ required-features = ["hazmat"]
 [[example]]
 name = "export"
 required-features = ["hazmat", "pkcs8"]
+
+[lints]
+workspace = true
+
+[package.metadata.docs.rs]
+all-features = true

dsa/examples/export.rs

@@ -1,4 +1,7 @@
+//! Example for serializing keys in SPKI/PKCS#8 format.
+
 #![cfg(feature = "hazmat")]
+#![allow(clippy::unwrap_used, reason = "tests")]
 
 use dsa::{Components, KeySize, SigningKey};
 use getrandom::SysRng;

dsa/examples/generate.rs

@@ -1,3 +1,5 @@
+//! Key generation example.
+
 #![cfg(feature = "hazmat")]
 
 use dsa::{Components, KeySize, SigningKey};

dsa/examples/sign.rs

@@ -1,3 +1,7 @@
+//! Signing example.
+
+#![allow(clippy::unwrap_used, reason = "tests")]
+
 use digest::Digest;
 use dsa::{Components, KeySize, SigningKey};
 use getrandom::{SysRng, rand_core::UnwrapErr};

dsa/src/components.rs

@@ -30,7 +30,17 @@ pub struct Components {
 }
 
 impl Components {
-    /// Construct the common components container from its inner values (p, q and g)
+    /// Construct the common components container from its inner values (`p`, `q`, and `g`).
+    ///
+    /// - `p` must be odd
+    /// - `g` must less than `p`
+    ///
+    /// # Errors
+    /// Returns [`signature::Error`] if any of the following occur:
+    /// - components aren't sized appropriately for a 1024-bit, 2048-bit, or 3072-bit key
+    /// - any of `p`, `q`, or `g` are less than `2`
+    /// - `p` is even instead of odd
+    /// - `g > p`
     pub fn from_components(p: BoxedUint, q: BoxedUint, g: BoxedUint) -> signature::Result<Self> {
         let (p, q, g) = Self::adapt_components(p, q, g)?;
 
@@ -49,9 +59,14 @@ impl Components {
     /// Construct the common components container from its inner values (p, q and g)
     ///
     /// # Safety
-    ///
     /// Any length of keys may be used, no checks are to be performed. You are responsible for
     /// checking the key strengths.
+    ///
+    /// # Errors
+    /// Returns [`signature::Error`] if any of the following occur:
+    /// - any of `p`, `q`, or `g` are less than `2`
+    /// - `p` is even instead of odd
+    /// - `g > p`
     #[cfg(feature = "hazmat")]
     pub fn from_components_unchecked(
         p: BoxedUint,
@@ -60,11 +75,10 @@ impl Components {
     ) -> signature::Result<Self> {
         let (p, q, g) = Self::adapt_components(p, q, g)?;
         let key_size = KeySize::other(p.bits_precision(), q.bits_precision());
-
         Ok(Self { p, q, g, key_size })
     }
 
-    /// Helper method to build a [`Components`]
+    /// Helper method to build [`Components`].
     fn adapt_components(
         p: BoxedUint,
         q: BoxedUint,
@@ -80,14 +94,18 @@ impl Components {
             .into_option()
             .ok_or_else(signature::Error::new)?;
 
-        if *p < two() || *q < two() || *g > *p {
+        if *p < two() || *q < two() || *g >= *p {
             return Err(signature::Error::new());
         }
 
         Ok((p, q, g))
     }
 
-    /// Generate a new pair of common components
+    /// Generate a new pair of common components.
+    ///
+    /// # Errors
+    /// Propagates errors from `R`.
+    #[allow(clippy::missing_panics_doc, reason = "shouldn't panic in practice")]
     pub fn try_generate_from_rng_with_key_size<R: TryCryptoRng + ?Sized>(
         rng: &mut R,
         key_size: KeySize,

dsa/src/generate/secret_number.rs

@@ -7,7 +7,6 @@ use alloc::vec;
 use core::cmp::min;
 use crypto_bigint::{BoxedUint, NonZero, NonZeroBoxedUint, RandomBits, Resize};
 use digest::{Digest, common::BlockSizeUser};
-use rfc6979::KGenerator;
 use signature::rand_core::TryCryptoRng;
 use zeroize::Zeroizing;
 
@@ -44,7 +43,7 @@ fn init_kgen<'a, D: BlockSizeUser + Digest>(
     x: &NonZeroBoxedUint,
     z: &[u8],
     q: &'a NonZeroBoxedUint,
-) -> KGenerator<'a, D, BoxedUint> {
+) -> rfc6979::KGenerator<'a, D, BoxedUint> {
     // Truncate to the right `size` most bytes
     fn truncate(b: &[u8], size: usize) -> &[u8] {
         &b[(b.len() - size)..]
@@ -66,6 +65,7 @@ fn bytes2uint(b: &[u8], q: &NonZeroBoxedUint) -> BoxedUint {
     BoxedUint::from_be_slice_truncated(b, q.bits_precision())
 }
 
+#[allow(clippy::as_conversions)]
 fn qlen(q: &NonZeroBoxedUint) -> usize {
     q.bits().div_ceil(8) as usize
 }

dsa/src/key_size.rs

@@ -2,7 +2,7 @@ use core::cmp::Ordering;
 use crypto_bigint::Limb;
 
 /// DSA key size
-#[derive(Clone, Debug, Copy)]
+#[derive(Clone, Copy, Debug)]
 pub struct KeySize {
     /// Bit size of p
     pub(crate) l: u32,
@@ -35,15 +35,15 @@ impl KeySize {
         Self { l, n }
     }
 
-    pub(crate) fn l_aligned(&self) -> u32 {
+    pub(crate) fn l_aligned(self) -> u32 {
         self.l.div_ceil(Limb::BITS) * Limb::BITS
     }
 
-    pub(crate) fn n_aligned(&self) -> u32 {
+    pub(crate) fn n_aligned(self) -> u32 {
         self.n.div_ceil(Limb::BITS) * Limb::BITS
     }
 
-    pub(crate) fn matches(&self, l: u32, n: u32) -> bool {
+    pub(crate) fn matches(self, l: u32, n: u32) -> bool {
         l == self.l_aligned() && n == self.n_aligned()
     }
 }

dsa/src/lib.rs

@@ -1,6 +1,5 @@
 #![no_std]
-#![forbid(unsafe_code)]
-#![warn(missing_docs, rust_2018_idioms, unreachable_pub)]
+#![cfg_attr(docsrs, feature(doc_cfg))]
 #![doc = include_str!("../README.md")]
 #![doc(
     html_logo_url = "https://raw.githubusercontent.com/RustCrypto/media/8f1a9894/logo.svg",
@@ -105,6 +104,7 @@ pub struct Signature {
 
 impl Signature {
     /// Create a new Signature container from its components
+    #[must_use]
     pub fn from_components(r: BoxedUint, s: BoxedUint) -> Option<Self> {
         let r = NonZero::new(r).into_option()?;
         let s = NonZero::new(s).into_option()?;
@@ -127,6 +127,11 @@ impl Signature {
 impl<'a> DecodeValue<'a> for Signature {
     type Error = der::Error;
 
+    #[allow(
+        clippy::as_conversions,
+        clippy::cast_possible_truncation,
+        reason = "TODO"
+    )]
     fn decode_value<R: Reader<'a>>(reader: &mut R, _header: der::Header) -> der::Result<Self> {
         let r = UintRef::decode(reader)?;
         let s = UintRef::decode(reader)?;

dsa/src/signing_key.rs

@@ -50,13 +50,16 @@ pub struct SigningKey {
 }
 
 impl SigningKey {
-    /// Construct a new private key from the public key and private component
+    /// Construct a new private key from the public key and private component.
+    ///
+    /// # Errors
+    /// Returns an error in the event `x` is zero or equal to or larger than `q`.
     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() {
+        if x >= *verifying_key.components().q() {
             return Err(signature::Error::new());
         }
 
@@ -66,7 +69,10 @@ impl SigningKey {
         })
     }
 
-    /// Generate a new DSA keypair
+    /// Generate a new DSA keypair.
+    ///
+    /// # Errors
+    /// Propagates errors from `R`.
     #[cfg(feature = "hazmat")]
     #[inline]
     pub fn try_generate_from_rng_with_components<R: TryCryptoRng + ?Sized>(
@@ -81,9 +87,17 @@ impl SigningKey {
         &self.verifying_key
     }
 
-    /// DSA private component
+    /// DSA private component.
+    ///
+    /// <div class="warning">
+    /// <b>Security Warning</b>
+    ///
+    /// This value is key material. Please treat it with care!
     ///
-    /// If you decide to clone this value, please consider using [`Zeroize::zeroize`](::zeroize::Zeroize::zeroize()) to zero out the memory after you're done using the clone
+    /// If you decide to clone this value, please consider using
+    /// [`Zeroize::zeroize`](::zeroize::Zeroize::zeroize) to zero out the memory after you're done
+    /// using the clone.
+    /// </div>
     #[must_use]
     pub fn x(&self) -> &NonZero<BoxedUint> {
         &self.x
@@ -94,15 +108,26 @@ impl SigningKey {
     ///
     /// [RFC6979]: https://datatracker.ietf.org/doc/html/rfc6979
     #[cfg(feature = "hazmat")]
+    #[allow(
+        clippy::missing_errors_doc,
+        reason = "errors shouldn't occur in practice"
+    )]
     pub fn sign_prehashed_rfc6979<D>(&self, prehash: &[u8]) -> Result<Signature, signature::Error>
     where
         D: BlockSizeUser + Digest,
     {
+        // TODO(tarcieri): make this operation infallible by retrying with a different `k`
         let k_kinv = generate::secret_number_rfc6979::<D>(self, prehash);
         self.sign_prehashed(k_kinv, prehash)
     }
 
-    /// Sign some pre-hashed data
+    /// Sign some pre-hashed data.
+    #[allow(
+        clippy::as_conversions,
+        clippy::cast_possible_truncation,
+        clippy::integer_division_remainder_used,
+        reason = "TODO"
+    )]
     fn sign_prehashed(
         &self,
         (k, inv_k): (BoxedUint, BoxedUint),
@@ -285,7 +310,11 @@ impl<'a> TryFrom<PrivateKeyInfoRef<'a>> for SigningKey {
             .into_option()
             .ok_or(pkcs8::KeyError::Invalid)?;
 
-        let y = if let Some(y_bytes) = value.public_key.as_ref().and_then(|bs| bs.as_bytes()) {
+        let y = if let Some(y_bytes) = value
+            .public_key
+            .as_ref()
+            .and_then(der::asn1::BitStringRef::as_bytes)
+        {
             let y = UintRef::from_der(y_bytes)?;
             BoxedUint::from_be_slice(y.as_bytes(), precision)
                 .map_err(|_| pkcs8::KeyError::Invalid)?

dsa/src/verifying_key.rs

@@ -36,7 +36,10 @@ pub struct VerifyingKey {
 }
 
 impl VerifyingKey {
-    /// Construct a new public key from the common components and the public component
+    /// Construct a new public key from the common components and the public component.
+    ///
+    /// # Errors
+    /// Returns an error if `y < 2`.
     pub fn from_components(components: Components, y: BoxedUint) -> signature::Result<Self> {
         let params = BoxedMontyParams::new_vartime(components.p().clone());
         let form = BoxedMontyForm::new(y.clone(), &params);
@@ -65,6 +68,12 @@ impl VerifyingKey {
 
     /// Verify some prehashed data
     #[must_use]
+    #[allow(
+        clippy::as_conversions,
+        clippy::cast_possible_truncation,
+        clippy::integer_division_remainder_used,
+        reason = "TODO"
+    )]
     fn verify_prehashed(&self, hash: &[u8], signature: &Signature) -> Option<bool> {
         let components = self.components();
         let (p, q, g) = (components.p(), components.q(), components.g());