Impl ecdsa::CheckSignatureBytes-related changes (#159)

Corresponding `ecdsa` crate PR is RustCrypto/signatures#151

This implements the changes which eagerly validate that ECDSA signature
`r` and `s` components are in-range `NonZeroScalar` values.

This eliminates the need to do these checks as part of each ECDSA
verification implementation.

Author: tarcieri

Cargo.lock

@@ -71,6 +71,9 @@ use core::convert::TryInto;
 /// ECDSA/secp256k1 signature (fixed-size)
 pub type Signature = ecdsa_core::Signature<Secp256k1>;
 
+#[cfg(not(feature = "ecdsa"))]
+impl ecdsa_core::CheckSignatureBytes for Secp256k1 {}
+
 #[cfg(all(feature = "ecdsa", feature = "sha256"))]
 impl ecdsa_core::hazmat::DigestPrimitive for Secp256k1 {
     type Digest = sha2::Sha256;

k256/src/ecdsa.rs

@@ -186,7 +186,7 @@ impl Scalar {
             return Err(Error::new());
         }
 
-        let mut signature = Signature::from_scalars(&r.into(), &s.into());
+        let mut signature = Signature::from_scalars(r, s)?;
         let is_r_odd = bool::from(R.y.normalize().is_odd());
         let is_s_high = signature.normalize_s()?;
         let recovery_id = recoverable::Id((is_r_odd ^ is_s_high) as u8);

k256/src/ecdsa/sign.rs

@@ -1,12 +1,10 @@
 //! ECDSA verifier
 
 use super::{recoverable, Error, Signature};
-use crate::{
-    AffinePoint, CompressedPoint, EncodedPoint, NonZeroScalar, ProjectivePoint, Scalar, Secp256k1,
-};
+use crate::{AffinePoint, CompressedPoint, EncodedPoint, ProjectivePoint, Scalar, Secp256k1};
 use core::convert::TryFrom;
 use ecdsa_core::{hazmat::VerifyPrimitive, signature};
-use elliptic_curve::{consts::U32, ops::Invert, sec1::ToEncodedPoint, FromBytes};
+use elliptic_curve::{consts::U32, ops::Invert, sec1::ToEncodedPoint};
 use signature::{digest::Digest, DigestVerifier, PrehashSignature};
 
 /// ECDSA/secp256k1 verification key (i.e. public key)
@@ -87,15 +85,8 @@ impl TryFrom<&EncodedPoint> for VerifyKey {
 
 impl VerifyPrimitive<Secp256k1> for AffinePoint {
     fn verify_prehashed(&self, z: &Scalar, signature: &Signature) -> Result<(), Error> {
-        let maybe_r = NonZeroScalar::from_bytes(signature.r());
-        let maybe_s = NonZeroScalar::from_bytes(signature.s());
-
-        // TODO(tarcieri): replace with into conversion when available (see subtle#73)
-        let (r, s) = if maybe_r.is_some().into() && maybe_s.is_some().into() {
-            (maybe_r.unwrap(), maybe_s.unwrap())
-        } else {
-            return Err(Error::new());
-        };
+        let r = signature.r();
+        let s = signature.s();
 
         // Ensure signature is "low S" normalized ala BIP 0062
         if s.is_high().into() {

k256/src/ecdsa/verify.rs

@@ -48,7 +48,7 @@ use {
     crate::{AffinePoint, ProjectivePoint, Scalar},
     core::borrow::Borrow,
     ecdsa_core::hazmat::{SignPrimitive, VerifyPrimitive},
-    elliptic_curve::{ops::Invert, subtle::CtOption, FromBytes},
+    elliptic_curve::ops::Invert,
 };
 
 /// ECDSA/P-256 signature (fixed-size)
@@ -64,6 +64,9 @@ pub type SigningKey = ecdsa_core::SigningKey<NistP256>;
 #[cfg_attr(docsrs, doc(cfg(feature = "ecdsa")))]
 pub type VerifyKey = ecdsa_core::VerifyKey<NistP256>;
 
+#[cfg(not(feature = "ecdsa"))]
+impl ecdsa_core::CheckSignatureBytes for NistP256 {}
+
 #[cfg(all(feature = "ecdsa", feature = "sha256"))]
 impl ecdsa_core::hazmat::DigestPrimitive for NistP256 {
     type Digest = sha2::Sha256;
@@ -99,36 +102,25 @@ impl SignPrimitive<NistP256> for Scalar {
             return Err(Error::new());
         }
 
-        Ok(Signature::from_scalars(&r.into(), &s.into()))
+        Signature::from_scalars(r, s)
     }
 }
 
 #[cfg(feature = "ecdsa")]
 impl VerifyPrimitive<NistP256> for AffinePoint {
     fn verify_prehashed(&self, z: &Scalar, signature: &Signature) -> Result<(), Error> {
-        let maybe_r =
-            Scalar::from_bytes(signature.r()).and_then(|r| CtOption::new(r, !r.is_zero()));
-
-        let maybe_s =
-            Scalar::from_bytes(signature.s()).and_then(|s| CtOption::new(s, !s.is_zero()));
-
-        // TODO(tarcieri): replace with into conversion when available (see subtle#73)
-        let (r, s) = if maybe_r.is_some().into() && maybe_s.is_some().into() {
-            (maybe_r.unwrap(), maybe_s.unwrap())
-        } else {
-            return Err(Error::new());
-        };
-
+        let r = signature.r();
+        let s = signature.s();
         let s_inv = s.invert().unwrap();
         let u1 = z * &s_inv;
-        let u2 = r * &s_inv;
+        let u2 = *r * &s_inv;
 
         let x = ((&ProjectivePoint::generator() * &u1) + &(ProjectivePoint::from(*self) * &u2))
             .to_affine()
             .unwrap()
             .x;
 
-        if Scalar::from_bytes_reduced(&x.to_bytes()) == r {
+        if Scalar::from_bytes_reduced(&x.to_bytes()) == *r {
             Ok(())
         } else {
             Err(Error::new())
@@ -177,8 +169,8 @@ mod tests {
             let z = Scalar::from_bytes(vector.m.try_into().unwrap()).unwrap();
             let sig = d.try_sign_prehashed(&k_blinded, &z).unwrap();
 
-            assert_eq!(vector.r, sig.r().as_slice());
-            assert_eq!(vector.s, sig.s().as_slice());
+            assert_eq!(vector.r, sig.r().to_bytes().as_slice());
+            assert_eq!(vector.s, sig.s().to_bytes().as_slice());
         }
     }
 

p256/src/ecdsa.rs

@@ -5,6 +5,8 @@ pub use crate::NistP384;
 /// ECDSA/P-384 signature (fixed-size)
 pub type Signature = ecdsa::Signature<NistP384>;
 
+impl ecdsa::CheckSignatureBytes for NistP384 {}
+
 #[cfg(feature = "sha384")]
 #[cfg_attr(docsrs, doc(cfg(feature = "sha384")))]
 impl ecdsa::hazmat::DigestPrimitive for NistP384 {