k256: use Decompress to impl ECDSA pubkey recovery (#158)

Uses the point decompression trait rather than duplicating the point
decompression logic.

Author: tarcieri

Cargo.lock

@@ -10,8 +10,8 @@
 //!   [`signature::Signer`] trait. Example use cases for this include other
 //!   software implementations of ECDSA/secp256k1 and wrappers for cloud KMS
 //!   services or hardware devices (HSM or crypto hardware wallet).
-//! - `ecdsa`: provides the [`Signature`], [`Signer`], and [`Verifier`] types
-//!   which natively implement ECDSA/secp256k1 signing and verification.
+//! - `ecdsa`: provides `ecdsa-core` plus the [`SigningKey`] and [`VerifyKey`]
+//!   types which natively implement ECDSA/secp256k1 signing and verification.
 //!
 //! ## Signing/Verification Example
 //!

k256/src/ecdsa.rs

@@ -18,16 +18,16 @@
 //!
 //! // Signing
 //! let signing_key = SigningKey::generate(&mut OsRng); // Serialize with `::to_bytes()`
-//! let public_key = EncodedPoint::from(&signing_key.verify_key());
+//! let verify_key = signing_key.verify_key();
 //! let message = b"ECDSA proves knowledge of a secret number in the context of a single message";
 //!
 //! // Note: the signature type must be annotated or otherwise inferrable as
 //! // `Signer` has many impls of the `Signer` trait (for both regular and
 //! // recoverable signature types).
 //! let signature: recoverable::Signature = signing_key.sign(message);
-//! let recovered_pubkey = signature.recover_public_key(message).expect("couldn't recover pubkey");
+//! let recovered_key = signature.recover_verify_key(message).expect("couldn't recover pubkey");
 //!
-//! assert_eq!(&public_key, &recovered_pubkey);
+//! assert_eq!(&verify_key, &recovered_key);
 //! # }
 //! ```
 
@@ -39,12 +39,10 @@ use ecdsa_core::{signature::Signature as _, Error};
 
 #[cfg(feature = "ecdsa")]
 use crate::{
-    arithmetic::{FieldElement, CURVE_EQUATION_B},
+    ecdsa::VerifyKey,
     elliptic_curve::{
-        consts::U32,
-        ops::Invert,
-        subtle::{Choice, ConditionallySelectable},
-        Digest, FromBytes, FromDigest,
+        consts::U32, ops::Invert, subtle::Choice, weierstrass::point::Decompress, Digest,
+        FromBytes, FromDigest,
     },
     AffinePoint, NonZeroScalar, ProjectivePoint, Scalar,
 };
@@ -108,8 +106,8 @@ impl Signature {
 
         for recovery_id in 0..=1 {
             if let Ok(recoverable_signature) = Signature::new(&signature, Id(recovery_id)) {
-                if let Ok(recovered_key) = recoverable_signature.recover_public_key(msg) {
-                    if public_key == &recovered_key {
+                if let Ok(recovered_key) = recoverable_signature.recover_verify_key(msg) {
+                    if public_key == &EncodedPoint::from(&recovered_key) {
                         return Ok(recoverable_signature);
                     }
                 }
@@ -123,52 +121,38 @@ impl Signature {
     /// [`EncodedPoint`].
     #[cfg(all(feature = "ecdsa", feature = "keccak256"))]
     #[cfg_attr(docsrs, doc(cfg(feature = "ecdsa")), doc(cfg(feature = "keccak256")))]
-    pub fn recover_public_key(&self, msg: &[u8]) -> Result<EncodedPoint, Error> {
-        self.recover_public_key_from_prehash(Keccak256::new().chain(msg))
+    pub fn recover_verify_key(&self, msg: &[u8]) -> Result<VerifyKey, Error> {
+        self.recover_verify_key_from_digest(Keccak256::new().chain(msg))
     }
 
     /// Recover the public key used to create the given signature as an
     /// [`EncodedPoint`] from the provided precomputed [`Digest`].
     #[cfg(feature = "ecdsa")]
     #[cfg_attr(docsrs, doc(cfg(feature = "ecdsa")))]
     #[allow(non_snake_case, clippy::many_single_char_names)]
-    pub fn recover_public_key_from_prehash<D>(&self, msg_prehash: D) -> Result<EncodedPoint, Error>
+    pub fn recover_verify_key_from_digest<D>(&self, msg_prehash: D) -> Result<VerifyKey, Error>
     where
         D: Digest<OutputSize = U32>,
     {
         let r = self.r();
         let s = self.s();
         let z = Scalar::from_digest(msg_prehash);
-        let x = FieldElement::from_bytes(&r.to_bytes());
-
-        let pk = x.and_then(|x| {
-            let alpha = (x * &x * &x) + &CURVE_EQUATION_B;
-            let beta = alpha.sqrt().unwrap();
-
-            let y = FieldElement::conditional_select(
-                &beta.negate(1),
-                &beta,
-                // beta.is_odd() == recovery_id.is_y_odd()
-                !(beta.normalize().is_odd() ^ self.recovery_id().is_y_odd()),
-            );
-
-            let R = ProjectivePoint::from(AffinePoint {
-                x,
-                y: y.normalize(),
-            });
+        let R = AffinePoint::decompress(&r.to_bytes(), self.recovery_id().is_y_odd());
 
+        // TODO(tarcieri): replace with into conversion when available (see subtle#73)
+        if R.is_some().into() {
+            let R = ProjectivePoint::from(R.unwrap());
             let r_inv = r.invert().unwrap();
             let u1 = -(r_inv * &z);
-            let u2 = r_inv * s.as_ref();
-            ((&ProjectivePoint::generator() * &u1) + &(R * &u2)).to_affine()
-        });
+            let u2 = r_inv * &*s;
+            let pk = ((&ProjectivePoint::generator() * &u1) + &(R * &u2)).to_affine();
 
-        // TODO(tarcieri): replace with into conversion when available (see subtle#73)
-        if pk.is_some().into() {
-            Ok(pk.unwrap().into())
-        } else {
-            Err(Error::new())
+            if pk.is_some().into() {
+                return Ok(VerifyKey::from(&pk.unwrap()));
+            }
         }
+
+        Err(Error::new())
     }
 
     /// Parse the `r` component of this signature to a [`Scalar`]
@@ -298,6 +282,7 @@ impl From<Id> for u8 {
 #[cfg(all(test, feature = "ecdsa", feature = "sha256"))]
 mod tests {
     use super::Signature;
+    use crate::EncodedPoint;
     use core::convert::TryFrom;
     use hex_literal::hex;
     use sha2::{Digest, Sha256};
@@ -335,8 +320,8 @@ mod tests {
         for vector in VECTORS {
             let sig = Signature::try_from(&vector.sig[..]).unwrap();
             let prehash = Sha256::new().chain(vector.msg);
-            let pk = sig.recover_public_key_from_prehash(prehash).unwrap();
-            assert_eq!(&vector.pk[..], pk.as_bytes());
+            let pk = sig.recover_verify_key_from_digest(prehash).unwrap();
+            assert_eq!(&vector.pk[..], EncodedPoint::from(&pk).as_bytes());
         }
     }
 }

k256/src/ecdsa/recoverable.rs

@@ -1,13 +1,17 @@
 //! ECDSA verifier
 
 use super::{recoverable, Error, Signature};
-use crate::{AffinePoint, EncodedPoint, NonZeroScalar, ProjectivePoint, Scalar, Secp256k1};
+use crate::{
+    AffinePoint, CompressedPoint, EncodedPoint, NonZeroScalar, ProjectivePoint, Scalar, Secp256k1,
+};
+use core::convert::TryFrom;
 use ecdsa_core::{hazmat::VerifyPrimitive, signature};
 use elliptic_curve::{consts::U32, ops::Invert, sec1::ToEncodedPoint, FromBytes};
 use signature::{digest::Digest, DigestVerifier, PrehashSignature};
 
 /// ECDSA/secp256k1 verification key (i.e. public key)
 #[cfg_attr(docsrs, doc(cfg(feature = "ecdsa")))]
+#[derive(Clone, Debug, Eq, PartialEq)]
 pub struct VerifyKey {
     /// Core ECDSA verify key
     pub(super) key: ecdsa_core::VerifyKey<Secp256k1>,
@@ -19,15 +23,17 @@ impl VerifyKey {
         ecdsa_core::VerifyKey::new(bytes).map(|key| VerifyKey { key })
     }
 
-    /// Initialize [`VerifyKey`] from an [`EncodedPoint`].
+    /// Initialize [`VerifyKey`] from a SEC1 [`EncodedPoint`].
     pub fn from_encoded_point(public_key: &EncodedPoint) -> Result<Self, Error> {
         ecdsa_core::VerifyKey::from_encoded_point(public_key).map(|key| VerifyKey { key })
     }
 
-    /// Serialize this [`VerifyKey`] as a SEC1 [`EncodedPoint`], optionally
-    /// applying point compression.
-    pub fn to_encoded_point(&self, compress: bool) -> EncodedPoint {
-        self.key.to_encoded_point(compress)
+    /// Serialize this [`VerifyKey`] as a SEC1-encoded bytestring
+    /// (with point compression applied)
+    pub fn to_bytes(&self) -> CompressedPoint {
+        let mut result = [0u8; 33];
+        result.copy_from_slice(EncodedPoint::from(self).as_ref());
+        result
     }
 }
 
@@ -67,7 +73,15 @@ impl From<&AffinePoint> for VerifyKey {
 
 impl From<&VerifyKey> for EncodedPoint {
     fn from(verify_key: &VerifyKey) -> EncodedPoint {
-        verify_key.to_encoded_point(true)
+        verify_key.key.to_encoded_point(true)
+    }
+}
+
+impl TryFrom<&EncodedPoint> for VerifyKey {
+    type Error = Error;
+
+    fn try_from(encoded_point: &EncodedPoint) -> Result<Self, Error> {
+        Self::from_encoded_point(encoded_point)
     }
 }
 

k256/src/ecdsa/verify.rs

@@ -104,13 +104,16 @@ impl elliptic_curve::Identifier for Secp256k1 {
     const OID: ObjectIdentifier = ObjectIdentifier::new(&[1, 3, 132, 0, 10]);
 }
 
-/// K-256 Serialized Field Element.
+/// Compressed SEC1-encoded secp256k1 (K-256) point (i.e. public key)
+pub type CompressedPoint = [u8; 33];
+
+/// secp256k1 (K-256) serialized field element.
 ///
 /// Byte array containing a serialized field element value (base field or scalar).
 pub type ElementBytes = elliptic_curve::ElementBytes<Secp256k1>;
 
-/// K-256 (secp256k1) SEC1 Encoded Point.
+/// SEC1-encoded secp256k1 (K-256) curve point.
 pub type EncodedPoint = elliptic_curve::sec1::EncodedPoint<Secp256k1>;
 
-/// K-256 (secp256k1) Secret Key.
+/// secp256k1 (K-256) secret key.
 pub type SecretKey = elliptic_curve::SecretKey<Secp256k1>;