Skip to content

Commit

Permalink
ecdsa: fix RecoveryId computation in SignPrimitive (#702)
Browse files Browse the repository at this point in the history
The implementation was adapted from `k256`, which produces signatures
with low-S normalization.

However, the provided implementation does not low-S normalize
signatures, so it should not be considered in the `RecoveryId`
computation.
  • Loading branch information
tarcieri authored Apr 5, 2023
1 parent 55fbdcd commit 621a847
Showing 1 changed file with 2 additions and 7 deletions.
9 changes: 2 additions & 7 deletions ecdsa/src/hazmat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ where
// 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::reduce_bytes(&R.x());
let x_reduced = r.to_repr() != R.x();
let x_is_reduced = r.to_repr() != R.x();

// Compute 𝒔 as a signature over 𝒓 and 𝒛.
let s = k_inv * (z + (r * self));
Expand All @@ -105,12 +105,7 @@ where
}

let signature = Signature::from_scalars(r, s)?;
let is_r_odd = R.y_is_odd();
let is_s_high = s.is_high();
let is_y_odd = is_r_odd ^ is_s_high;

let recovery_id = RecoveryId::new(is_y_odd.into(), x_reduced);

let recovery_id = RecoveryId::new(R.y_is_odd().into(), x_is_reduced);
Ok((signature, Some(recovery_id)))
}

Expand Down

0 comments on commit 621a847

Please sign in to comment.