Skip to content

Commit

Permalink
Merge pull request #156 from compiler-errors/do-not-rely-on-inference
Browse files Browse the repository at this point in the history
Do not rely on inference behavior for `Scalar::from_*` and `Point::from_*`
  • Loading branch information
LLFourn authored May 8, 2023
2 parents de2f2f3 + ffb6939 commit 79ae667
Show file tree
Hide file tree
Showing 9 changed files with 32 additions and 42 deletions.
4 changes: 2 additions & 2 deletions ecdsa_fun/benches/bench_ecdsa.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use criterion::{criterion_group, criterion_main, Criterion};
use secp256kfun::{marker::*, nonce::Deterministic, secp256k1, Scalar};
use secp256kfun::{nonce::Deterministic, secp256k1, Scalar};
use sha2::Sha256;

const MESSAGE: &[u8; 32] = b"hello world you are beautiful!!!";
Expand Down Expand Up @@ -37,7 +37,7 @@ fn verify_ecdsa(c: &mut Criterion) {
});

{
let signature = signature.clone().set_secrecy::<Secret>();
let signature = signature.clone();
group.bench_function("fun::ecdsa_verify_ct", |b| {
b.iter(|| ECDSA.verify(&pk, MESSAGE, &signature))
});
Expand Down
2 changes: 1 addition & 1 deletion ecdsa_fun/src/adaptor/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ impl<T: Transcript<DLEQ>, NG> Adaptor<T, NG> {
pub fn recover_decryption_key(
&self,
encryption_key: &Point<impl Normalized, impl Secrecy>,
signature: &Signature<impl Secrecy>,
signature: &Signature,
ciphertext: &EncryptedSignature,
) -> Option<Scalar> {
let EncryptedSignature(EncryptedSignatureInternal { s_hat, R, .. }) = ciphertext;
Expand Down
2 changes: 1 addition & 1 deletion ecdsa_fun/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ impl<NG> ECDSA<NG> {
&self,
verification_key: &Point<impl PointType, Public, NonZero>,
message: &[u8; 32],
signature: &Signature<impl Secrecy>,
signature: &Signature,
) -> bool {
let (R_x, s) = signature.as_tuple();
// This ensures that there is only one valid s value per R_x for any given message.
Expand Down
35 changes: 12 additions & 23 deletions ecdsa_fun/src/signature.rs
Original file line number Diff line number Diff line change
@@ -1,52 +1,41 @@
use secp256kfun::{marker::*, Scalar};
/// An ECDSA signature
#[derive(Clone, PartialEq)]
pub struct Signature<S = Public> {
pub R_x: Scalar<Public, NonZero>,
pub s: Scalar<S, NonZero>,
pub struct Signature {
pub R_x: Scalar<Public>,
pub s: Scalar<Public>,
}

impl<S> Signature<S> {
impl Signature {
pub fn to_bytes(&self) -> [u8; 64] {
let mut bytes = [0u8; 64];
bytes[0..32].copy_from_slice(&self.R_x.to_bytes()[..]);
bytes[32..64].copy_from_slice(&self.s.to_bytes()[..]);
bytes
}

pub fn as_tuple(&self) -> (&Scalar<Public, NonZero>, &Scalar<S, NonZero>) {
pub fn as_tuple(&self) -> (&Scalar<Public>, &Scalar<Public>) {
(&self.R_x, &self.s)
}

pub fn set_secrecy<SigSec: Secrecy>(self) -> Signature<SigSec> {
Signature {
R_x: self.R_x,
s: self.s.set_secrecy::<SigSec>(),
}
}
}

impl Signature<Public> {
impl Signature {
pub fn from_bytes(bytes: [u8; 64]) -> Option<Self> {
Scalar::from_slice(&bytes[0..32])
.and_then(|R_x| R_x.public().non_zero())
.and_then(|R_x| {
Scalar::from_slice(&bytes[32..64])
.and_then(|s| s.public().non_zero())
.map(|s| Self { R_x, s })
})
let R_x = Scalar::from_slice(&bytes[0..32])?.non_zero()?;
let s = Scalar::from_slice(&bytes[32..64])?.non_zero()?;
Some(Self { R_x, s })
}
}

secp256kfun::impl_fromstr_deserialize! {
name => "secp256k1 ECDSA signature",
fn from_bytes<S: Secrecy>(bytes: [u8;64]) -> Option<Signature<S>> {
Signature::from_bytes(bytes).map(|signature| signature.set_secrecy::<S>())
fn from_bytes(bytes: [u8;64]) -> Option<Signature> {
Signature::from_bytes(bytes)
}
}

secp256kfun::impl_display_debug_serialize! {
fn to_bytes<S>(sig: &Signature<S>) -> [u8;64] {
fn to_bytes(sig: &Signature) -> [u8;64] {
sig.to_bytes()
}
}
4 changes: 1 addition & 3 deletions ecdsa_fun/tests/against_c_lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ use ecdsa_fun::{
self,
fun::{
hex,
marker::*,
secp256k1::{self, ecdsa, Message, PublicKey, SecretKey},
Point, Scalar,
},
Expand Down Expand Up @@ -69,8 +68,7 @@ fn ecdsa_verify_high_message() {
.unwrap();
let c_message = Message::from_slice(&message[..]).unwrap();
let c_signature = secp.sign_ecdsa(&c_message, &c_secret_key);
let signature =
ecdsa_fun::Signature::<Public>::from_bytes(c_signature.serialize_compact()).unwrap();
let signature = ecdsa_fun::Signature::from_bytes(c_signature.serialize_compact()).unwrap();

assert!(ecdsa.verify(&verification_key, &message, &signature));
}
Expand Down
4 changes: 2 additions & 2 deletions schnorr_fun/src/frost.rs
Original file line number Diff line number Diff line change
Expand Up @@ -979,13 +979,13 @@ impl SignSession {
fn lagrange_lambda(
x_j: Scalar<impl Secrecy>,
x_ms: impl Iterator<Item = Scalar<impl Secrecy>>,
) -> Scalar {
) -> Scalar<Public> {
x_ms.fold(Scalar::one(), |acc, x_m| {
let denominator = s!(x_m - x_j)
.non_zero()
.expect("removed duplicate indexes")
.invert();
s!(acc * x_m * denominator)
s!(acc * x_m * denominator).public()
})
}

Expand Down
15 changes: 9 additions & 6 deletions secp256kfun/src/point.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ impl Point<Normal, Public, NonZero> {
}
}

impl<Z: ZeroChoice> Point<Normal, Public, Z> {
impl<Z: ZeroChoice, S> Point<Normal, S, Z> {
/// Creates a Point the compressed encoding specified in [_Standards for
/// Efficient Cryptography_]. This is the typical encoding used in
/// Bitcoin. The first byte must be `0x02` or `0x03` to indicate that the
Expand Down Expand Up @@ -388,7 +388,10 @@ impl<S, Z, T: Normalized> Point<T, S, Z> {
/// let point = Point::random(&mut rand::thread_rng());
/// let bytes = point.to_bytes();
/// assert!(bytes[0] == 0x02 || bytes[0] == 0x03);
/// assert_eq!(Point::<_, _, NonZero>::from_bytes(bytes).unwrap(), point);
/// assert_eq!(
/// Point::<_, Public, NonZero>::from_bytes(bytes).unwrap(),
/// point
/// );
/// ```
///
/// [_Standards for Efficient Cryptography_]: https://www.secg.org/sec1-v2.pdf
Expand Down Expand Up @@ -543,15 +546,15 @@ crate::impl_display_serialize! {

crate::impl_fromstr_deserialize! {
name => "secp256k1 x-coordinate",
fn from_bytes<S>(bytes: [u8;32]) -> Option<Point<EvenY,S, NonZero>> {
Point::from_xonly_bytes(bytes).map(|p| p.set_secrecy::<S>())
fn from_bytes<S>(bytes: [u8;32]) -> Option<Point<EvenY, S, NonZero>> {
Point::from_xonly_bytes(bytes)
}
}

crate::impl_fromstr_deserialize! {
name => "secp256k1 point",
fn from_bytes<S,Z: ZeroChoice>(bytes: [u8;33]) -> Option<Point<Normal,S, Z>> {
Point::from_bytes(bytes).map(|p| p.set_secrecy::<S>())
Point::from_bytes(bytes)
}
}

Expand Down Expand Up @@ -792,7 +795,7 @@ mod test {

#[test]
fn zero_to_and_from_bytes() {
let zero = Point::zero();
let zero = Point::<_, Public, _>::zero();
assert_eq!(Point::<_, _, Zero>::from_bytes(zero.to_bytes()), Some(zero));
}

Expand Down
4 changes: 2 additions & 2 deletions secp256kfun/src/proptest_impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ impl<S: Secrecy> Arbitrary for Scalar<S, NonZero> {
// insert some pathological cases
1 => Just(Scalar::<S,_>::one()),
1 => Just(Scalar::<S,_>::minus_one()),
18 => any::<[u8;32]>().prop_filter_map("zero bytes not acceptable", |bytes| Some(Scalar::from_bytes_mod_order(bytes).non_zero()?.set_secrecy::<S>())),
18 => any::<[u8;32]>().prop_filter_map("zero bytes not acceptable", |bytes| Scalar::from_bytes_mod_order(bytes).non_zero()),
].boxed()
}
}
Expand All @@ -28,7 +28,7 @@ impl<S: Secrecy> Arbitrary for Scalar<S, Zero> {
1 => Just(Scalar::zero()),
1 => Just(Scalar::one().mark_zero()),
1 => Just(Scalar::minus_one().mark_zero()),
27 => any::<[u8;32]>().prop_map(|bytes| Scalar::from_bytes_mod_order(bytes).set_secrecy::<S>()),
27 => any::<[u8;32]>().prop_map(|bytes| Scalar::from_bytes_mod_order(bytes)),
]
.boxed()
}
Expand Down
4 changes: 2 additions & 2 deletions secp256kfun/src/scalar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -299,7 +299,7 @@ impl<S> From<u32> for Scalar<S, Zero> {
crate::impl_fromstr_deserialize! {
name => "non-zero secp256k1 scalar",
fn from_bytes<S>(bytes: [u8;32]) -> Option<Scalar<S,NonZero>> {
Scalar::from_bytes(bytes).and_then(|scalar| scalar.set_secrecy::<S>().non_zero())
Scalar::from_bytes(bytes)?.non_zero()
}
}

Expand All @@ -312,7 +312,7 @@ crate::impl_display_debug_serialize! {
crate::impl_fromstr_deserialize! {
name => "secp256k1 scalar",
fn from_bytes<S>(bytes: [u8;32]) -> Option<Scalar<S,Zero>> {
Scalar::from_bytes(bytes).map(|scalar| scalar.set_secrecy::<S>())
Scalar::from_bytes(bytes)
}
}

Expand Down

0 comments on commit 79ae667

Please sign in to comment.