[parity-crypto] Use upstream secp256k1

parity-crypto/CHANGELOG.md

@@ -5,3 +5,9 @@ The format is based on [Keep a Changelog].
 [Keep a Changelog]: http://keepachangelog.com/en/1.0.0/
 
 ## [Unreleased]
+- Remove `inv()` from `SecretKey` (breaking)
+- `Generate::generate()` does not return error
+- `Secp256k1` is no longer exported 
+- Remove `public_is_valid()` as it is now impossible to create invalid public keys
+- 0-valued `Secp::Message`s are disallowed (signatures on them are forgeable for all keys)
+- updates to upstream `rust-secp256k1` at v0.17.2

parity-crypto/Cargo.toml

@@ -16,13 +16,13 @@ required-features = ["publickey"]
 [dependencies]
 tiny-keccak = { version = "2.0", features = ["keccak"] }
 scrypt = { version = "0.2.0", default-features = false }
-parity-secp256k1 = { version = "0.7.0", optional = true }
+secp256k1 = { version = "0.17.2", optional = true, features = ["recovery", "rand-std"] }
 ethereum-types = { version = "0.8.0", optional = true }
 lazy_static = { version = "1.0", optional = true }
 ripemd160 = "0.8.0"
 sha2 = "0.8.0"
-digest = "0.8.1"
-hmac = "0.7.1"
+digest = "0.8"
+hmac = "0.7"
 aes = "0.3.2"
 aes-ctr = "0.3.0"
 block-modes = "0.3.3"
@@ -40,4 +40,4 @@ hex-literal = "0.2.1"
 default = []
 # public key crypto utils
 # moved from ethkey module in parity ethereum repository
-publickey = ["parity-secp256k1", "lazy_static", "ethereum-types"]
+publickey = ["secp256k1", "lazy_static", "ethereum-types"]

parity-crypto/src/publickey/ec_math_utils.rs

@@ -37,52 +37,46 @@ lazy_static! {
 	pub static ref CURVE_ORDER: U256 = H256::from_slice(&SECP256K1_CURVE_ORDER).into_uint();
 }
 
-/// Whether the public key is valid.
-pub fn public_is_valid(public: &Public) -> bool {
-	to_secp256k1_public(public).ok().map_or(false, |p| p.is_valid())
-}
-
 /// In-place multiply public key by secret key (EC point * scalar)
 pub fn public_mul_secret(public: &mut Public, secret: &Secret) -> Result<(), Error> {
 	let key_secret = secret.to_secp256k1_secret()?;
 	let mut key_public = to_secp256k1_public(public)?;
-	key_public.mul_assign(&SECP256K1, &key_secret)?;
+	key_public.mul_assign(&SECP256K1, &key_secret[..])?;
 	set_public(public, &key_public);
 	Ok(())
 }
 
 /// In-place add one public key to another (EC point + EC point)
 pub fn public_add(public: &mut Public, other: &Public) -> Result<(), Error> {
-	let mut key_public = to_secp256k1_public(public)?;
+	let key_public = to_secp256k1_public(public)?;
 	let other_public = to_secp256k1_public(other)?;
-	key_public.add_assign(&SECP256K1, &other_public)?;
+	let key_public = key_public.combine(&other_public)?;
 	set_public(public, &key_public);
 	Ok(())
 }
 
 /// In-place sub one public key from another (EC point - EC point)
 pub fn public_sub(public: &mut Public, other: &Public) -> Result<(), Error> {
 	let mut key_neg_other = to_secp256k1_public(other)?;
-	key_neg_other.mul_assign(&SECP256K1, &key::MINUS_ONE_KEY)?;
+	key_neg_other.mul_assign(&SECP256K1, super::MINUS_ONE_KEY)?;
 
 	let mut key_public = to_secp256k1_public(public)?;
-	key_public.add_assign(&SECP256K1, &key_neg_other)?;
+	key_public = key_public.combine(&key_neg_other)?;
 	set_public(public, &key_public);
 	Ok(())
 }
 
 /// Replace a public key with its additive inverse (EC point = - EC point)
 pub fn public_negate(public: &mut Public) -> Result<(), Error> {
 	let mut key_public = to_secp256k1_public(public)?;
-	key_public.mul_assign(&SECP256K1, &key::MINUS_ONE_KEY)?;
+	key_public.mul_assign(&SECP256K1, super::MINUS_ONE_KEY)?;
 	set_public(public, &key_public);
 	Ok(())
 }
 
 /// Return the generation point (aka base point) of secp256k1
 pub fn generation_point() -> Public {
-	let public_key =
-		key::PublicKey::from_slice(&SECP256K1, &BASE_POINT_BYTES).expect("constructed using constants; qed");
+	let public_key = key::PublicKey::from_slice(&BASE_POINT_BYTES).expect("constructed using constants; qed");
 	let mut public = Public::default();
 	set_public(&mut public, &public_key);
 	public
@@ -95,24 +89,24 @@ fn to_secp256k1_public(public: &Public) -> Result<key::PublicKey, Error> {
 		temp
 	};
 
-	Ok(key::PublicKey::from_slice(&SECP256K1, &public_data)?)
+	Ok(key::PublicKey::from_slice(&public_data)?)
 }
 
 fn set_public(public: &mut Public, key_public: &key::PublicKey) {
-	let key_public_serialized = key_public.serialize_vec(&SECP256K1, false);
+	let key_public_serialized = key_public.serialize_uncompressed();
 	public.as_bytes_mut().copy_from_slice(&key_public_serialized[1..65]);
 }
 
 #[cfg(test)]
 mod tests {
 	use super::super::{Generator, Random, Secret};
-	use super::{generation_point, public_add, public_is_valid, public_mul_secret, public_negate, public_sub};
+	use super::{generation_point, public_add, public_mul_secret, public_negate, public_sub};
 	use std::str::FromStr;
 
 	#[test]
 	fn public_addition_is_commutative() {
-		let public1 = Random.generate().unwrap().public().clone();
-		let public2 = Random.generate().unwrap().public().clone();
+		let public1 = Random.generate().public().clone();
+		let public2 = Random.generate().public().clone();
 
 		let mut left = public1.clone();
 		public_add(&mut left, &public2).unwrap();
@@ -125,8 +119,8 @@ mod tests {
 
 	#[test]
 	fn public_addition_is_reversible_with_subtraction() {
-		let public1 = Random.generate().unwrap().public().clone();
-		let public2 = Random.generate().unwrap().public().clone();
+		let public1 = Random.generate().public().clone();
+		let public2 = Random.generate().public().clone();
 
 		let mut sum = public1.clone();
 		public_add(&mut sum, &public2).unwrap();
@@ -137,20 +131,14 @@ mod tests {
 
 	#[test]
 	fn public_negation_is_involutory() {
-		let public = Random.generate().unwrap().public().clone();
+		let public = Random.generate().public().clone();
 		let mut negation = public.clone();
 		public_negate(&mut negation).unwrap();
 		public_negate(&mut negation).unwrap();
 
 		assert_eq!(negation, public);
 	}
 
-	#[test]
-	fn known_public_is_valid() {
-		let public = Random.generate().unwrap().public().clone();
-		assert!(public_is_valid(&public));
-	}
-
 	#[test]
 	fn generation_point_expected() {
 		let point = generation_point();

parity-crypto/src/publickey/ecdh.rs

@@ -16,21 +16,37 @@
 
 //! ECDH key agreement scheme implemented as a free function.
 
-use super::{Error, Public, Secret, SECP256K1};
+use super::{Error, Public, Secret};
 use secp256k1::{self, ecdh, key};
 
 /// Agree on a shared secret
 pub fn agree(secret: &Secret, public: &Public) -> Result<Secret, Error> {
-	let context = &SECP256K1;
 	let pdata = {
 		let mut temp = [4u8; 65];
 		(&mut temp[1..65]).copy_from_slice(&public[0..64]);
 		temp
 	};
 
-	let publ = key::PublicKey::from_slice(context, &pdata)?;
-	let sec = key::SecretKey::from_slice(context, secret.as_bytes())?;
-	let shared = ecdh::SharedSecret::new_raw(context, &publ, &sec);
+	let publ = key::PublicKey::from_slice(&pdata)?;
+	let sec = key::SecretKey::from_slice(secret.as_bytes())?;
+	let shared = ecdh::SharedSecret::new_with_hash(&publ, &sec, |x, _| x.into())?;
 
 	Secret::import_key(&shared[0..32]).map_err(|_| Error::Secp(secp256k1::Error::InvalidSecretKey))
 }
+
+#[cfg(test)]
+mod tests {
+	use super::{agree, Public, Secret};
+	use std::str::FromStr;
+
+	#[test]
+	fn test_agree() {
+		// Just some random values for secret/public to check we agree with previous implementation.
+		let secret = Secret::from_str("01a400760945613ff6a46383b250bf27493bfe679f05274916182776f09b28f1").unwrap();
+		let public= Public::from_str("e37f3cbb0d0601dc930b8d8aa56910dd5629f2a0979cc742418960573efc5c0ff96bc87f104337d8c6ab37e597d4f9ffbd57302bc98a825519f691b378ce13f5").unwrap();
+		let shared = agree(&secret, &public);
+
+		assert!(shared.is_ok());
+		assert_eq!(shared.unwrap().to_hex(), "28ab6fad6afd854ff27162e0006c3f6bd2daafc0816c85b5dfb05dbb865fa6ac",);
+	}
+}

parity-crypto/src/publickey/ecdsa_signature.rs

@@ -20,7 +20,10 @@ use super::{public_to_address, Address, Error, Message, Public, Secret, SECP256K
 use ethereum_types::{H256, H520};
 use rustc_hex::{FromHex, ToHex};
 use secp256k1::key::{PublicKey, SecretKey};
-use secp256k1::{Error as SecpError, Message as SecpMessage, RecoverableSignature, RecoveryId};
+use secp256k1::{
+	recovery::{RecoverableSignature, RecoveryId},
+	Error as SecpError, Message as SecpMessage,
+};
 use std::cmp::PartialEq;
 use std::fmt;
 use std::hash::{Hash, Hasher};
@@ -208,12 +211,12 @@ impl DerefMut for Signature {
 }
 
 /// Signs message with the given secret key.
-/// Returns the corresponding signature
+/// Returns the corresponding signature.
 pub fn sign(secret: &Secret, message: &Message) -> Result<Signature, Error> {
 	let context = &SECP256K1;
-	let sec = SecretKey::from_slice(context, secret.as_ref())?;
-	let s = context.sign_recoverable(&SecpMessage::from_slice(&message[..])?, &sec)?;
-	let (rec_id, data) = s.serialize_compact(context);
+	let sec = SecretKey::from_slice(secret.as_ref())?;
+	let s = context.sign_recoverable(&SecpMessage::from_slice(&message[..])?, &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
@@ -225,17 +228,16 @@ pub fn sign(secret: &Secret, message: &Message) -> Result<Signature, Error> {
 /// Performs verification of the signature for the given message with corresponding public key
 pub fn verify_public(public: &Public, signature: &Signature, message: &Message) -> Result<bool, Error> {
 	let context = &SECP256K1;
-	let rsig =
-		RecoverableSignature::from_compact(context, &signature[0..64], RecoveryId::from_i32(signature[64] as i32)?)?;
-	let sig = rsig.to_standard(context);
+	let rsig = RecoverableSignature::from_compact(&signature[0..64], RecoveryId::from_i32(signature[64] as i32)?)?;
+	let sig = rsig.to_standard();
 
 	let pdata: [u8; 65] = {
 		let mut temp = [4u8; 65];
 		temp[1..65].copy_from_slice(public.as_bytes());
 		temp
 	};
 
-	let publ = PublicKey::from_slice(context, &pdata)?;
+	let publ = PublicKey::from_slice(&pdata)?;
 	match context.verify(&SecpMessage::from_slice(&message[..])?, &sig, &publ) {
 		Ok(_) => Ok(true),
 		Err(SecpError::IncorrectSignature) => Ok(false),
@@ -253,10 +255,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 rsig =
-		RecoverableSignature::from_compact(context, &signature[0..64], RecoveryId::from_i32(signature[64] as i32)?)?;
+	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 serialized = pubkey.serialize_vec(context, false);
+	let serialized = pubkey.serialize_uncompressed();
 
 	let mut public = Public::default();
 	public.as_bytes_mut().copy_from_slice(&serialized[1..65]);
@@ -272,9 +273,9 @@ mod tests {
 	#[test]
 	fn vrs_conversion() {
 		// given
-		let keypair = Random.generate().unwrap();
-		let message = Message::default();
-		let signature = sign(keypair.secret(), &message).unwrap();
+		let keypair = Random.generate();
+		let message = Message::from_str("0000000000000000000000000000000000000000000000000000000000000001").unwrap();
+		let signature = sign(keypair.secret(), &message).expect("can sign a non-zero message");
 
 		// when
 		let vrs = signature.clone().into_electrum();
@@ -286,35 +287,35 @@ mod tests {
 
 	#[test]
 	fn signature_to_and_from_str() {
-		let keypair = Random.generate().unwrap();
-		let message = Message::default();
-		let signature = sign(keypair.secret(), &message).unwrap();
+		let keypair = Random.generate();
+		let message = Message::from_str("0000000000000000000000000000000000000000000000000000000000000001").unwrap();
+		let signature = sign(keypair.secret(), &message).expect("can sign a non-zero message");
 		let string = format!("{}", signature);
 		let deserialized = Signature::from_str(&string).unwrap();
 		assert_eq!(signature, deserialized);
 	}
 
 	#[test]
 	fn sign_and_recover_public() {
-		let keypair = Random.generate().unwrap();
-		let message = Message::default();
+		let keypair = Random.generate();
+		let message = Message::from_str("0000000000000000000000000000000000000000000000000000000000000001").unwrap();
 		let signature = sign(keypair.secret(), &message).unwrap();
 		assert_eq!(keypair.public(), &recover(&signature, &message).unwrap());
 	}
 
 	#[test]
 	fn sign_and_verify_public() {
-		let keypair = Random.generate().unwrap();
-		let message = Message::default();
-		let signature = sign(keypair.secret(), &message).unwrap();
+		let keypair = Random.generate();
+		let message = Message::from_str("0000000000000000000000000000000000000000000000000000000000000001").unwrap();
+		let signature = sign(keypair.secret(), &message).expect("can sign a non-zero message");
 		assert!(verify_public(keypair.public(), &signature, &message).unwrap());
 	}
 
 	#[test]
 	fn sign_and_verify_address() {
-		let keypair = Random.generate().unwrap();
-		let message = Message::default();
-		let signature = sign(keypair.secret(), &message).unwrap();
+		let keypair = Random.generate();
+		let message = Message::from_str("0000000000000000000000000000000000000000000000000000000000000001").unwrap();
+		let signature = sign(keypair.secret(), &message).expect("can sign a non-zero message");
 		assert!(verify_address(&keypair.address(), &signature, &message).unwrap());
 	}
 }

parity-crypto/src/publickey/ecies.rs

@@ -27,7 +27,7 @@ const ENC_VERSION: u8 = 0x04;
 ///
 /// Authenticated data may be empty.
 pub fn encrypt(public: &Public, auth_data: &[u8], plain: &[u8]) -> Result<Vec<u8>, Error> {
-	let r = Random.generate()?;
+	let r = Random.generate();
 	let z = ecdh::agree(r.secret(), public)?;
 	let mut key = [0u8; 32];
 	kdf(&z, &[0u8; 0], &mut key);
@@ -122,7 +122,7 @@ mod tests {
 
 	#[test]
 	fn ecies_shared() {
-		let kp = Random.generate().unwrap();
+		let kp = Random.generate();
 		let message = b"So many books, so little time";
 
 		let shared = b"shared";

parity-crypto/src/publickey/error.rs

@@ -14,7 +14,7 @@
 // You should have received a copy of the GNU General Public License
 // along with Parity Ethereum.  If not, see <http://www.gnu.org/licenses/>.
 
-//! Module specific errors
+//! Module specific errors.
 
 use crate::error::SymmError;
 use std::{error::Error as StdError, fmt, result};