Skip to content

Commit

Permalink
Allow pubkey recovery for all-zero messages
Browse files Browse the repository at this point in the history
After https://github.com/openethereum/openethereum/pull/11406 it is no longer possible to to public key recovery from messages that are all-zero. This create issues when using the `ecrecover` builtin because externally produced signatures may well provide a message (i.e. a preimage) that is all-zeroes.
This works around the problem at the cost of cloning the incoming message and create a `ZeroesAllowedMessage` wrapper around it. The `ZeroesAllowedMessage` implements the `ThirtyTwoByteHash` trait from `rust-secp256k1` which circumvents the zero-check.

In a follow-up PR we'll likely change the interface of `recover()` to take a `ZeroesAllowedMessage` directly, thus removing the unneeded clone.
  • Loading branch information
dvdplm committed Apr 9, 2020
1 parent dd89c9a commit 6a14615
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 4 deletions.
30 changes: 26 additions & 4 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, SECP256K1, ZeroesAllowedMessage};
use ethereum_types::{H256, H520};
use rustc_hex::{FromHex, ToHex};
use secp256k1::key::{PublicKey, SecretKey};
Expand Down Expand Up @@ -247,8 +247,9 @@ 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 secp_msg = ZeroesAllowedMessage(message.clone());
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 = context.recover(&secp_msg.into(), &rsig)?;
let serialized = pubkey.serialize_uncompressed();

let mut public = Public::default();
Expand All @@ -258,8 +259,9 @@ pub fn recover(signature: &Signature, message: &Message) -> Result<Public, Error

#[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, sign, verify_address, verify_public, Signature, ZeroesAllowedMessage};
use secp256k1::SecretKey;
use std::str::FromStr;

#[test]
Expand Down Expand Up @@ -295,6 +297,26 @@ mod tests {
assert_eq!(keypair.public(), &recover(&signature, &message).unwrap());
}

#[test]
fn sign_and_recover_public_works_with_zeroed_messages() {
let keypair = Random.generate();
let zero_message = Message::zero();
let signature = {
let context = &SECP256K1;
let sec = SecretKey::from_slice(keypair.secret().as_ref()).unwrap();
let secp_msg = ZeroesAllowedMessage(zero_message);
let s = context.sign_recoverable(&secp_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)
};
assert_eq!(keypair.public(), &recover(&signature, &zero_message).unwrap());
}

#[test]
fn sign_and_verify_public() {
let keypair = Random.generate();
Expand Down
8 changes: 8 additions & 0 deletions parity-crypto/src/publickey/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,14 @@ use lazy_static::lazy_static;
pub use ethereum_types::{Address, Public};
pub type Message = H256;

use secp256k1::ThirtyTwoByteHash;
pub struct ZeroesAllowedMessage(pub H256);
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

0 comments on commit 6a14615

Please sign in to comment.