Skip to content

Commit

Permalink
Address review
Browse files Browse the repository at this point in the history
  • Loading branch information
Nicolas Stalder committed May 20, 2022
1 parent 699fda0 commit 0822d78
Show file tree
Hide file tree
Showing 2 changed files with 94 additions and 59 deletions.
6 changes: 1 addition & 5 deletions k256/src/arithmetic/affine.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,6 @@ impl DecompressPoint<Secp256k1> for AffinePoint {
/// [BIP 340]: https://github.com/bitcoin/bips/blob/master/bip-0340.mediawiki
impl DecompactPoint<Secp256k1> for AffinePoint {
fn decompact(x_bytes: &FieldBytes) -> CtOption<Self> {
// TODO(tarcieri): implement full `lift_x` algorithm as described in BIP 340
Self::decompress(x_bytes, Choice::from(0))
}
}
Expand Down Expand Up @@ -250,10 +249,7 @@ impl FromEncodedPoint<Secp256k1> for AffinePoint {
fn from_encoded_point(encoded_point: &EncodedPoint) -> CtOption<Self> {
match encoded_point.coordinates() {
sec1::Coordinates::Identity => CtOption::new(Self::IDENTITY, 1.into()),
sec1::Coordinates::Compact { .. } => {
// TODO(tarcieri): add decompaction support
CtOption::new(Self::default(), 0.into())
}
sec1::Coordinates::Compact { x } => Self::decompact(x),
sec1::Coordinates::Compressed { x, y_is_odd } => {
AffinePoint::decompress(x, Choice::from(y_is_odd as u8))
}
Expand Down
147 changes: 93 additions & 54 deletions k256/src/schnorr.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
#![allow(non_snake_case, clippy::many_single_char_names)]

use core::{cmp, fmt};

use crate::{
arithmetic::FieldElement, AffinePoint, FieldBytes, NonZeroScalar, ProjectivePoint, PublicKey,
Scalar,
Expand All @@ -25,15 +27,40 @@ const CHALLENGE_TAG: &[u8] = b"BIP0340/challenge";
/// Taproot Schnorr signature as defined in [BIP340].
///
/// [BIP340]: https://github.com/bitcoin/bips/blob/master/bip-0340.mediawiki
#[derive(Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Ord)]
#[derive(Copy, Clone)]
pub struct Signature {
// TODO(nickray): maybe store (FieldElement, NonScalar), avoiding the unwraps.
// However, this
// a) contradicts tarcieri's philosophy of storing "bag of bytes"
// b) makes the Debug, Eq, PartialEq, PartialOrd, Ord impls tricky,
// as FieldElement + NonZeroScalar don't implement them, so we'd have to
// either modify upstream or implement in terms of the calculated `to_bytes`.
bytes: [u8; 64],

// for efficiency
r: FieldElement,
s: NonZeroScalar,
}

impl fmt::Debug for Signature {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
self.bytes.fmt(f)
}
}

impl PartialEq for Signature {
fn eq(&self, other: &Self) -> bool {
self.bytes == other.bytes
}
}

impl Eq for Signature {}

impl PartialOrd for Signature {
fn partial_cmp(&self, other: &Self) -> Option<cmp::Ordering> {
Some(self.cmp(other))
}
}

// useful e.g. for BTreeMaps
impl Ord for Signature {
fn cmp(&self, other: &Self) -> cmp::Ordering {
self.bytes.cmp(&other.bytes)
}
}

impl Signature {
Expand All @@ -42,12 +69,21 @@ impl Signature {
let bytes: [u8; 64] = bytes.try_into()
.map_err(|_| Error::new())?;

let _: FieldElement = Option::from(FieldElement::from_bytes(FieldBytes::from_slice(&bytes[..32])))
.ok_or_else(Error::new)?;
let _: NonZeroScalar = Option::from(NonZeroScalar::from_repr(*FieldBytes::from_slice(&bytes[32..])))
let r: FieldElement = Option::from(
FieldElement::from_bytes(FieldBytes::from_slice(&bytes[..32]))
)
.ok_or_else(Error::new)?;

Ok(Self { bytes })
// one of the rules for valid signatures: !is_infinite(R);
// don't have a NonZeroFieldElement type.
if r == FieldElement::ZERO {
return Err(Error::new());
}

let s = NonZeroScalar::try_from(&bytes[32..])
.map_err(|_| Error::new())?;

Ok(Self { bytes, r, s })
}

/// Borrow the serialized signature as bytes.
Expand All @@ -56,17 +92,17 @@ impl Signature {
}

/// Get the `r` component of this signature.
fn r(&self) -> FieldElement {
FieldElement::from_bytes(FieldBytes::from_slice(&self.bytes[..32])).unwrap()
fn r(&self) -> &FieldElement {
&self.r
}

/// Get the `s` component of this signature.
fn s(&self) -> NonZeroScalar {
NonZeroScalar::from_repr(*FieldBytes::from_slice(&self.bytes[32..])).unwrap()
fn s(&self) -> &NonZeroScalar {
&self.s
}

/// Split this signature into its `r` and `s` components.
fn split(&self) -> (FieldElement, NonZeroScalar) {
fn split(&self) -> (&FieldElement, &NonZeroScalar) {
(self.r(), self.s())
}
}
Expand Down Expand Up @@ -97,7 +133,21 @@ impl SigningKey {
/// Parse signing key from big endian-encoded bytes.
pub fn from_bytes(bytes: &[u8]) -> Result<Self> {

let trial_secret_key = NonZeroScalar::try_from(bytes).map_err(|_| Error::new())?;
let (secret_key, verifying_point) = Self::raw_from_bytes(bytes)?;

let verifying_key = VerifyingKey { inner:
PublicKey::from_affine(verifying_point)
.map_err(|_| Error::new())?
};

Ok(Self { secret_key, verifying_key })
}

// a little type dance for use in SigningKey's `from_bytes` and `try_sign`.
fn raw_from_bytes(bytes: &[u8]) -> Result<(NonZeroScalar, AffinePoint)> {

let trial_secret_key = NonZeroScalar::try_from(bytes)
.map_err(|_| Error::new())?;

let even = (ProjectivePoint::GENERATOR * *trial_secret_key)
.to_affine()
Expand All @@ -110,18 +160,12 @@ impl SigningKey {
even,
);

let verifying_key = VerifyingKey { inner:
PublicKey::from_affine((ProjectivePoint::GENERATOR * *secret_key).to_affine())
.map_err(|_| Error::new())?
};

Ok(Self {
secret_key,
verifying_key,
})
let verifying_point = (ProjectivePoint::GENERATOR * *secret_key).to_affine();

Ok((secret_key, verifying_point))
}


/// Serialize as bytes.
pub fn to_bytes(&self) -> FieldBytes {
self.secret_key.to_bytes()
Expand Down Expand Up @@ -151,25 +195,28 @@ impl SigningKey {
.chain_update(msg_digest)
.finalize();

let k = SigningKey::from_bytes(&rand)?;
// the ephemeral key "k"
let (secret_key, verifying_point) = SigningKey::raw_from_bytes(&rand)?;

let r = k.verifying_key().to_bytes();
let r = verifying_point.x.normalize();

let e = <Scalar as Reduce<U256>>::from_be_bytes_reduced(
tagged_hash(CHALLENGE_TAG)
.chain_update(&r)
.chain_update(&r.to_bytes())
.chain_update(&self.verifying_key.to_bytes())
.chain_update(msg_digest)
.finalize(),
);

let s = *k.secret_key + e * *self.secret_key;
let s = *secret_key + e * *self.secret_key;
let s: NonZeroScalar = Option::from(NonZeroScalar::new(s))
.ok_or_else(Error::new)?;

let mut bytes = [0u8; 64];
bytes[..32].copy_from_slice(&r);
bytes[..32].copy_from_slice(&r.to_bytes());
bytes[32..].copy_from_slice(&s.to_bytes());

let sig = Signature { bytes };
let sig = Signature { bytes, r, s };

#[cfg(debug_assertions)]
self.verifying_key.verify_raw_digest(msg_digest, &sig)?;
Expand All @@ -191,10 +238,6 @@ impl VerifyingKey {
pub fn verify_raw_digest(&self, msg_digest: &[u8; 32], sig: &Signature) -> Result<()> {
let (r, s) = sig.split();

if r == FieldElement::ZERO {
return Err(Error::new());
}

let e = <Scalar as Reduce<U256>>::from_be_bytes_reduced(
tagged_hash(CHALLENGE_TAG)
.chain_update(&sig.bytes[..32])
Expand All @@ -211,7 +254,7 @@ impl VerifyingKey {
)
.to_affine();

if R.y.normalize().is_odd().into() || R.x.normalize() != r {
if R.y.normalize().is_odd().into() || R.x.normalize() != *r {
return Err(Error::new());
}

Expand Down Expand Up @@ -278,6 +321,9 @@ mod tests {

/// Signing test vector
struct SignVector {
/// Index of test case
index: u8,

/// Signing key
secret_key: [u8; 32],

Expand All @@ -296,8 +342,8 @@ mod tests {

/// BIP340 signing test vectors: index 0-3
const BIP340_SIGN_VECTORS: &[SignVector] = &[
// index 0
SignVector {
index: 0,
secret_key: hex!("0000000000000000000000000000000000000000000000000000000000000003"),
public_key: hex!("F9308A019258C31049344F85F89D5229B531C845836F99B08601F113BCE036F9"),
aux_rand: hex!("0000000000000000000000000000000000000000000000000000000000000000"),
Expand All @@ -307,8 +353,8 @@ mod tests {
25F66A4A85EA8B71E482A74F382D2CE5EBEEE8FDB2172F477DF4900D310536C0"
),
},
// index 1
SignVector {
index: 1,
secret_key: hex!("B7E151628AED2A6ABF7158809CF4F3C762E7160F38B4DA56A784D9045190CFEF"),
public_key: hex!("DFF1D77F2A671C5F36183726DB2341BE58FEAE1DA2DECED843240F7B502BA659"),
aux_rand: hex!("0000000000000000000000000000000000000000000000000000000000000001"),
Expand All @@ -318,8 +364,8 @@ mod tests {
8906D11AC976ABCCB20B091292BFF4EA897EFCB639EA871CFA95F6DE339E4B0A"
),
},
// index 2
SignVector {
index: 2,
secret_key: hex!("C90FDAA22168C234C4C6628B80DC1CD129024E088A67CC74020BBEA63B14E5C9"),
public_key: hex!("DD308AFEC5777E13121FA72B9CC1B7CC0139715309B086C960E18FD969774EB8"),
aux_rand: hex!("C87AA53824B4D7AE2EB035A2B5BBBCCC080E76CDC6D1692C4B0B62D798E6D906"),
Expand All @@ -329,9 +375,9 @@ mod tests {
AB745879A5AD954A72C45A91C3A51D3C7ADEA98D82F8481E0E1E03674A6F3FB7"
),
},
// index 3
// test fails if msg is reduced modulo p or n
SignVector {
index: 3,
secret_key: hex!("0B432B2677937381AEF05BB02A66ECD012773062CF3FA2549E44F58ED2401710"),
public_key: hex!("25D1DFF95105F5253C4022F628A996AD3A0D95FBF21D468A1B33F8C160D8F517"),
aux_rand: hex!("FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFF"),
Expand All @@ -351,9 +397,13 @@ mod tests {

let sig = sk
.try_sign_raw_digest(&vector.message, &vector.aux_rand)
.expect("low-level Schnorr signing failure");
.unwrap_or_else( |_| panic!(
"low-level Schnorr signing failure for index {}",
vector.index
));

assert_eq!(&vector.signature, sig.as_ref());
assert_eq!(&vector.signature, sig.as_ref(),
"wrong signature for index {}", vector.index);
}
}

Expand All @@ -377,7 +427,6 @@ mod tests {

/// BIP340 verification test vectors: index 4-14
const BIP340_VERIFY_VECTORS: &[VerifyVector] = &[
// index 4
VerifyVector {
index: 4,
public_key: hex!("D69C3509BB99E412E68B0FE8544E72837DFA30746D8BE2AA65975F29D22DC7B9"),
Expand All @@ -388,7 +437,6 @@ mod tests {
),
valid: true,
},
// index 5
// public key not on curve
VerifyVector {
index: 5,
Expand All @@ -399,7 +447,6 @@ mod tests {
69E89B4C5564D00349106B8497785DD7D1D713A8AE82B32FA79D5F7FC407D39B"),
valid: false,
},
// index 6
// has_even_y(R) is false
VerifyVector {
index: 6,
Expand All @@ -410,7 +457,6 @@ mod tests {
3CC27944640AC607CD107AE10923D9EF7A73C643E166BE5EBEAFA34B1AC553E2"),
valid: false,
},
// index 7
// negated message
VerifyVector {
index: 7,
Expand All @@ -421,7 +467,6 @@ mod tests {
28890B3EDB6E7189B630448B515CE4F8622A954CFE545735AAEA5134FCCDB2BD"),
valid: false,
},
// index 8
// negated s value
VerifyVector {
index: 8,
Expand All @@ -432,7 +477,6 @@ mod tests {
961764B3AA9B2FFCB6EF947B6887A226E8D7C93E00C5ED0C1834FF0D0C2E6DA6"),
valid: false,
},
// index 9
// sG - eP is infinite. Test fails in single verification if has_even_y(inf) is defined as true and x(inf) as 0
VerifyVector {
index: 9,
Expand All @@ -443,7 +487,6 @@ mod tests {
123DDA8328AF9C23A94C1FEECFD123BA4FB73476F0D594DCB65C6425BD186051"),
valid: false,
},
// index 10
// sG - eP is infinite. Test fails in single verification if has_even_y(inf) is defined as true and x(inf) as 1
VerifyVector {
index: 10,
Expand All @@ -454,7 +497,6 @@ mod tests {
7615FBAF5AE28864013C099742DEADB4DBA87F11AC6754F93780D5A1837CF197"),
valid: false,
},
// index 11
// sig[0:32] is not an X coordinate on the curve
VerifyVector {
index: 11,
Expand All @@ -465,7 +507,6 @@ mod tests {
69E89B4C5564D00349106B8497785DD7D1D713A8AE82B32FA79D5F7FC407D39B"),
valid: false,
},
// index 12
// sig[0:32] is equal to field size
VerifyVector {
index: 12,
Expand All @@ -476,7 +517,6 @@ mod tests {
69E89B4C5564D00349106B8497785DD7D1D713A8AE82B32FA79D5F7FC407D39B"),
valid: false,
},
// index 13
// sig[32:64] is equal to curve order
VerifyVector {
index: 13,
Expand All @@ -487,7 +527,6 @@ mod tests {
FFFFFFFFFFFFFFFFFFFFFFFFFFFFFFFEBAAEDCE6AF48A03BBFD25E8CD0364141"),
valid: false,
},
// index 14
// public key is not a valid X coordinate because it exceeds the field size
VerifyVector {
index: 14,
Expand Down

0 comments on commit 0822d78

Please sign in to comment.