From 27cda0498b37a69a7fba8e064dc61c4805769222 Mon Sep 17 00:00:00 2001
From: Tony Arcieri <bascule@gmail.com>
Date: Fri, 21 Jul 2023 10:03:04 -0600
Subject: [PATCH] k256: use generic signing implementation from `ecdsa`

Uses `ecdsa::hazmat::sign_prehashed` which provides a generic
implementation of ECDSA signing.

The code is largely identical to what was previously used, that code
having originally been copied from `p256` and updated iteratively to
match the generic implementation in the `ecdsa` crate as it evolved.

The only reason why we can't use the default implementation of
`SignPrimitive::try_sign_prehashed` is to handle low-S normalization,
which is unique to the secp256k1 ecosystem but nearly ubiquitous and
very much expected of ECDSA/secp256k1 implementations.

In the next release this can hopefully be upstreamed to the `ecdsa`
crate as well, which would eliminate the need for the `SignPrimitive`
and `VerifyPrimitive` traits (which pretty much exist exclusively so
`k256` can have custom logic to handle low-S normalization).
---
 Cargo.lock        |  4 ++--
 k256/Cargo.toml   |  2 +-
 k256/src/ecdsa.rs | 45 +++++++--------------------------------------
 3 files changed, 10 insertions(+), 41 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index e2aaaea2..013cd114 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -350,9 +350,9 @@ dependencies = [
 
 [[package]]
 name = "ecdsa"
-version = "0.16.7"
+version = "0.16.8"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "0997c976637b606099b9985693efa3581e84e41f5c11ba5255f88711058ad428"
+checksum = "a4b1e0c257a9e9f25f90ff76d7a68360ed497ee519c8e428d1825ef0000799d4"
 dependencies = [
  "der",
  "digest",
diff --git a/k256/Cargo.toml b/k256/Cargo.toml
index dd3c7335..a6c5afe5 100644
--- a/k256/Cargo.toml
+++ b/k256/Cargo.toml
@@ -23,7 +23,7 @@ elliptic-curve = { version = "0.13", default-features = false, features = ["hazm
 
 # optional dependencies
 once_cell = { version = "1.18", optional = true, default-features = false }
-ecdsa-core = { version = "0.16", package = "ecdsa", optional = true, default-features = false, features = ["der"] }
+ecdsa-core = { version = "0.16.8", package = "ecdsa", optional = true, default-features = false, features = ["der"] }
 hex-literal = { version = "0.4", optional = true }
 serdect = { version = "0.2", optional = true, default-features = false }
 sha2 = { version = "0.10", optional = true, default-features = false }
diff --git a/k256/src/ecdsa.rs b/k256/src/ecdsa.rs
index 3f9049b0..e2811b5c 100644
--- a/k256/src/ecdsa.rs
+++ b/k256/src/ecdsa.rs
@@ -154,14 +154,9 @@ use crate::Secp256k1;
 
 #[cfg(feature = "ecdsa")]
 use {
-    crate::{AffinePoint, FieldBytes, ProjectivePoint, Scalar},
+    crate::{AffinePoint, FieldBytes, Scalar},
     ecdsa_core::hazmat::{SignPrimitive, VerifyPrimitive},
-    elliptic_curve::{
-        bigint::U256,
-        ops::{Invert, MulByGenerator, Reduce},
-        scalar::IsHigh,
-        subtle::CtOption,
-    },
+    elliptic_curve::{ops::Invert, scalar::IsHigh, subtle::CtOption},
 };
 
 /// ECDSA/secp256k1 signature (fixed-size)
@@ -194,37 +189,11 @@ impl SignPrimitive<Secp256k1> for Scalar {
     where
         K: AsRef<Self> + Invert<Output = CtOption<Self>>,
     {
-        if k.as_ref().is_zero().into() {
-            return Err(Error::new());
-        }
-
-        let z = <Self as Reduce<U256>>::reduce_bytes(z);
-
-        // Compute scalar inversion of 𝑘
-        let k_inv = Option::<Scalar>::from(k.invert()).ok_or_else(Error::new)?;
-
-        // Compute 𝑹 = 𝑘×𝑮
-        let R = ProjectivePoint::mul_by_generator(k.as_ref()).to_affine();
-
-        // Lift x-coordinate of 𝑹 (element of base field) into a serialized big
-        // integer, then reduce it into an element of the scalar field
-        let r = <Self as Reduce<U256>>::reduce_bytes(&R.x.to_bytes());
-
-        // Compute 𝒔 as a signature over 𝒓 and 𝒛.
-        let s = k_inv * (z + (r * self));
-
-        if s.is_zero().into() {
-            return Err(Error::new());
-        }
-
-        let signature = Signature::from_scalars(r, s)?;
-        let is_r_odd = R.y.normalize().is_odd();
-        let is_s_high = s.is_high();
-        let is_y_odd = is_r_odd ^ is_s_high;
-        let signature_low = signature.normalize_s().unwrap_or(signature);
-        let recovery_id = RecoveryId::new(is_y_odd.into(), false);
-
-        Ok((signature_low, Some(recovery_id)))
+        let (sig, recid) = hazmat::sign_prehashed::<Secp256k1, K>(&self, k, z)?;
+        let is_y_odd = recid.is_y_odd() ^ bool::from(sig.s().is_high());
+        let sig_low = sig.normalize_s().unwrap_or(sig);
+        let recid = RecoveryId::new(is_y_odd, recid.is_x_reduced());
+        Ok((sig_low, Some(recid)))
     }
 }