Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow pubkey recovery for all-zero messages #369

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 56 additions & 6 deletions parity-crypto/src/publickey/ecdsa_signature.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

//! Signature based on ECDSA, algorithm's description: https://en.wikipedia.org/wiki/Elliptic_Curve_Digital_Signature_Algorithm

use super::{public_to_address, Address, Error, Message, Public, Secret, SECP256K1};
use super::{public_to_address, Address, Error, Message, Public, Secret, ZeroesAllowedMessage, SECP256K1};
use ethereum_types::{H256, H520};
use rustc_hex::{FromHex, ToHex};
use secp256k1::key::{PublicKey, SecretKey};
Expand Down Expand Up @@ -246,22 +246,56 @@ pub fn verify_address(address: &Address, signature: &Signature, message: &Messag

/// Recovers the public key from the signature for the message
pub fn recover(signature: &Signature, message: &Message) -> Result<Public, Error> {
let context = &SECP256K1;
let rsig = RecoverableSignature::from_compact(&signature[0..64], RecoveryId::from_i32(signature[64] as i32)?)?;
let pubkey = context.recover(&SecpMessage::from_slice(&message[..])?, &rsig)?;
let pubkey = &SECP256K1.recover(&SecpMessage::from_slice(&message[..])?, &rsig)?;
let serialized = pubkey.serialize_uncompressed();

let mut public = Public::default();
public.as_bytes_mut().copy_from_slice(&serialized[1..65]);
Ok(public)
}

/// Recovers the public key from the signature for the given message.
/// This version of `recover()` allows for all-zero messages, which is necessary
/// for ethereum but is otherwise highly discouraged. Use with caution.
pub fn recover_allowing_all_zero_message(
signature: &Signature,
message: ZeroesAllowedMessage,
) -> Result<Public, Error> {
let rsig = RecoverableSignature::from_compact(&signature[0..64], RecoveryId::from_i32(signature[64] as i32)?)?;
let pubkey = &SECP256K1.recover(&message.into(), &rsig)?;
let serialized = pubkey.serialize_uncompressed();
let mut public = Public::zero();
public.as_bytes_mut().copy_from_slice(&serialized[1..65]);
Ok(public)
}

#[cfg(test)]
mod tests {
use super::super::{Generator, Message, Random};
use super::{recover, sign, verify_address, verify_public, Signature};
use super::super::{Generator, Message, Random, SECP256K1};
use super::{
recover, recover_allowing_all_zero_message, sign, verify_address, verify_public, Secret, Signature,
ZeroesAllowedMessage,
};
use secp256k1::SecretKey;
use std::str::FromStr;

// Copy of `sign()` that allows signing all-zero Messages.
// Note: this is for *tests* only. DO NOT USE UNLESS YOU NEED IT.
fn sign_zero_message(secret: &Secret) -> Signature {
let context = &SECP256K1;
let sec = SecretKey::from_slice(secret.as_ref()).unwrap();
// force an all-zero message into a secp `Message` bypassing the validity check.
let zero_msg = ZeroesAllowedMessage(Message::zero());
let s = context.sign_recoverable(&zero_msg.into(), &sec);
let (rec_id, data) = s.serialize_compact();
let mut data_arr = [0; 65];

// no need to check if s is low, it always is
data_arr[0..64].copy_from_slice(&data[0..64]);
data_arr[64] = rec_id.to_i32() as u8;
Signature(data_arr)
}

#[test]
fn vrs_conversion() {
// given
Expand Down Expand Up @@ -295,6 +329,22 @@ mod tests {
assert_eq!(keypair.public(), &recover(&signature, &message).unwrap());
}

#[test]
fn sign_and_recover_public_fails_with_zeroed_messages() {
let keypair = Random.generate();
let signature = sign_zero_message(keypair.secret());
let zero_message = Message::zero();
assert!(&recover(&signature, &zero_message).is_err());
}

#[test]
fn recover_allowing_all_zero_message_can_recover_from_all_zero_messages() {
let keypair = Random.generate();
let signature = sign_zero_message(keypair.secret());
let zero_message = ZeroesAllowedMessage(Message::zero());
assert_eq!(keypair.public(), &recover_allowing_all_zero_message(&signature, zero_message).unwrap())
}

#[test]
fn sign_and_verify_public() {
let keypair = Random.generate();
Expand Down
20 changes: 19 additions & 1 deletion parity-crypto/src/publickey/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,9 @@ pub mod ecdh;
pub mod ecies;
pub mod error;

pub use self::ecdsa_signature::{recover, sign, verify_address, verify_public, Signature};
pub use self::ecdsa_signature::{
recover, recover_allowing_all_zero_message, sign, verify_address, verify_public, Signature,
};
pub use self::error::Error;
pub use self::extended_keys::{Derivation, DerivationError, ExtendedKeyPair, ExtendedPublic, ExtendedSecret};
pub use self::keypair::{public_to_address, KeyPair};
Expand All @@ -33,6 +35,22 @@ use lazy_static::lazy_static;
pub use ethereum_types::{Address, Public};
pub type Message = H256;

use secp256k1::ThirtyTwoByteHash;

/// In ethereum we allow public key recovery from a signature + message pair
/// where the message is all-zeroes. This conflicts with the best practise of
/// not allowing such values and so in order to avoid breaking consensus we need
/// this to work around it. The `ZeroesAllowedType` wraps an `H256` that can be
/// converted to a `[u8; 32]` which in turn can be cast to a
/// `secp256k1::Message` by the `ThirtyTwoByteHash` and satisfy the API for
/// `recover()`.
pub struct ZeroesAllowedMessage(H256);
ordian marked this conversation as resolved.
Show resolved Hide resolved
impl ThirtyTwoByteHash for ZeroesAllowedMessage {
fn into_32(self) -> [u8; 32] {
self.0.to_fixed_bytes()
}
}

/// The number -1 encoded as a secret key
const MINUS_ONE_KEY: &'static [u8] = &[
0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xfe, 0xba, 0xae, 0xdc,
Expand Down