k256: impl ecdsa::hazmat::RecoverableSignPrimitive

Impls the `RecoverableSignPrimitive` trait introduced in
RustCrypto/signatures#111 which returns the necessary information
(even-or-odd) about the ECDSA signature's `R` y-coordinate to be able to
compute the recovery ID without using brute force trial recovery.

This implementation leaks some details the existing `ecdsa` crate APIs
are trying to abstract over, but hopefully can feed into the
requirements for what those APIs should actually look like.

k256/src/ecdsa/recoverable.rs

@@ -240,7 +240,7 @@ impl From<Signature> for super::Signature {
 /// of contrived examples, so for simplicity's sake handling these values
 /// is unsupported and will return an `Error` when parsing the `Id`.
 #[derive(Copy, Clone, Debug)]
-pub struct Id(u8);
+pub struct Id(pub(super) u8);
 
 impl Id {
     /// Create a new [`Id`] from the given byte value

k256/src/ecdsa/signer.rs

@@ -3,20 +3,24 @@
 use super::{recoverable, Error, Signature};
 use crate::{ProjectivePoint, PublicKey, Scalar, ScalarBytes, Secp256k1, SecretKey};
 use core::borrow::Borrow;
-use ecdsa_core::{hazmat::SignPrimitive, signature::RandomizedSigner};
+use ecdsa_core::{hazmat::RecoverableSignPrimitive, signature::RandomizedSigner};
 use elliptic_curve::{
     ops::Invert,
     rand_core::{CryptoRng, RngCore},
+    secret_key::FromSecretKey,
+    zeroize::Zeroizing,
+    Generate,
 };
+use sha2::{Digest, Sha256};
 
 #[cfg(debug_assertions)]
 use crate::{ecdsa::signature::Verifier as _, ecdsa::Verifier};
 
 /// ECDSA/secp256k1 signer
 #[cfg_attr(docsrs, doc(cfg(feature = "ecdsa")))]
 pub struct Signer {
-    /// Core ECDSA signer
-    signer: ecdsa_core::Signer<Secp256k1>,
+    /// Secret scalar value
+    secret_key: SecretKey,
 
     /// Public key
     public_key: PublicKey,
@@ -25,9 +29,11 @@ pub struct Signer {
 impl Signer {
     /// Create a new signer
     pub fn new(secret_key: &SecretKey) -> Result<Self, Error> {
-        let signer = ecdsa_core::Signer::new(secret_key)?;
         let public_key = PublicKey::from_secret_key(secret_key, true).map_err(|_| Error::new())?;
-        Ok(Self { signer, public_key })
+        Ok(Self {
+            secret_key: secret_key.clone(),
+            public_key,
+        })
     }
 
     /// Get the public key for this signer
@@ -42,8 +48,9 @@ impl RandomizedSigner<Signature> for Signer {
         rng: impl CryptoRng + RngCore,
         msg: &[u8],
     ) -> Result<Signature, Error> {
-        let signature = self
-            .signer
+        let signer = ecdsa_core::Signer::new(&self.secret_key)?;
+
+        let signature = signer
             .try_sign_with_rng(rng, msg)
             .and_then(|sig| super::normalize_s(&sig))?;
 
@@ -63,10 +70,18 @@ impl RandomizedSigner<recoverable::Signature> for Signer {
         rng: impl CryptoRng + RngCore,
         msg: &[u8],
     ) -> Result<recoverable::Signature, Error> {
-        let signature = self.try_sign_with_rng(rng, msg)?;
-
-        // TODO(tarcieri): more efficient method of computing recovery ID
-        recoverable::Signature::from_trial_recovery(&self.public_key, msg, &signature)
+        let d = Scalar::from_secret_key(&self.secret_key).unwrap();
+        let k = Zeroizing::new(Scalar::generate(rng));
+        let z = Sha256::new().chain(msg).finalize();
+        let (signature, is_r_odd) = d.try_sign_recoverable_prehashed(&*k, &z)?;
+        let normalized_signature = super::normalize_s(&signature)?;
+        let is_s_high = &normalized_signature != &signature;
+        let recovery_id = recoverable::Id((is_r_odd ^ is_s_high) as u8);
+
+        Ok(recoverable::Signature::new(
+            &normalized_signature,
+            recovery_id,
+        ))
     }
 }
 
@@ -76,13 +91,13 @@ impl From<&Signer> for PublicKey {
     }
 }
 
-impl SignPrimitive<Secp256k1> for Scalar {
-    #[allow(clippy::many_single_char_names)]
-    fn try_sign_prehashed<K>(
+impl RecoverableSignPrimitive<Secp256k1> for Scalar {
+    #[allow(non_snake_case, clippy::many_single_char_names)]
+    fn try_sign_recoverable_prehashed<K>(
         &self,
         ephemeral_scalar: &K,
         hashed_msg: &ScalarBytes,
-    ) -> Result<Signature, Error>
+    ) -> Result<(Signature, bool), Error>
     where
         K: Borrow<Scalar> + Invert<Output = Scalar>,
     {
@@ -95,12 +110,12 @@ impl SignPrimitive<Secp256k1> for Scalar {
 
         let k_inverse = k_inverse.unwrap();
 
-        // Compute `x`-coordinate of affine point 𝑘×𝑮
-        let x = (ProjectivePoint::generator() * k).to_affine().unwrap().x;
+        // Compute 𝐑 = 𝑘×𝑮
+        let R = (ProjectivePoint::generator() * k).to_affine().unwrap();
 
-        // Lift `x` (element of base field) to serialized big endian integer,
-        // then reduce it to an element of the scalar field
-        let r = Scalar::from_bytes_reduced(&x.to_bytes());
+        // Lift x-coordinate of 𝐑 (element of base field) into a serialized big
+        // integer, then reduce it to an element of the scalar field
+        let r = Scalar::from_bytes_reduced(&R.x.to_bytes());
 
         // Reduce message hash to an element of the scalar field
         let z = Scalar::from_bytes_reduced(hashed_msg.as_ref());
@@ -112,13 +127,16 @@ impl SignPrimitive<Secp256k1> for Scalar {
             return Err(Error::new());
         }
 
-        Ok(Signature::from_scalars(&r.into(), &s.into()))
+        let signature = Signature::from_scalars(&r.into(), &s.into());
+        let r_is_odd = R.y.is_odd();
+        Ok((signature, r_is_odd.into()))
     }
 }
 
 #[cfg(test)]
 mod tests {
     use super::*;
     use crate::test_vectors::ecdsa::ECDSA_TEST_VECTORS;
+    use ecdsa_core::hazmat::SignPrimitive;
     ecdsa_core::new_signing_test!(ECDSA_TEST_VECTORS);
 }