Const time comparison (#8113)

* Use `subtle::slices_equal` for constant time comparison.

Also update the existing version of subtle in `ethcrypto` from
0.1 to 0.5

* Test specifically for InvalidPassword error.

Cargo.lock

@@ -754,7 +754,7 @@ dependencies = [
  "ethereum-types 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)",
  "ethkey 0.3.0",
  "rust-crypto 0.2.36 (registry+https://github.com/rust-lang/crates.io-index)",
- "subtle 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
+ "subtle 0.5.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "tiny-keccak 1.4.1 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
@@ -834,6 +834,7 @@ dependencies = [
  "itertools 0.5.10 (registry+https://github.com/rust-lang/crates.io-index)",
  "libc 0.2.36 (registry+https://github.com/rust-lang/crates.io-index)",
  "log 0.3.9 (registry+https://github.com/rust-lang/crates.io-index)",
+ "matches 0.1.6 (registry+https://github.com/rust-lang/crates.io-index)",
  "parity-wordlist 1.2.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "parking_lot 0.5.4 (registry+https://github.com/rust-lang/crates.io-index)",
  "rand 0.4.1 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -843,6 +844,7 @@ dependencies = [
  "serde_derive 1.0.29 (registry+https://github.com/rust-lang/crates.io-index)",
  "serde_json 1.0.9 (registry+https://github.com/rust-lang/crates.io-index)",
  "smallvec 0.4.3 (registry+https://github.com/rust-lang/crates.io-index)",
+ "subtle 0.5.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "tempdir 0.3.5 (registry+https://github.com/rust-lang/crates.io-index)",
  "time 0.1.38 (registry+https://github.com/rust-lang/crates.io-index)",
  "tiny-keccak 1.4.1 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -2970,11 +2972,8 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 
 [[package]]
 name = "subtle"
-version = "0.1.0"
+version = "0.5.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-dependencies = [
- "num-traits 0.1.40 (registry+https://github.com/rust-lang/crates.io-index)",
-]
 
 [[package]]
 name = "syn"
@@ -3801,7 +3800,7 @@ dependencies = [
 "checksum spmc 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)" = "cd1f11d1fb5fd41834e55ce0b85a186efbf2f2afd9fdb09e2c8d72f9bff1ad1a"
 "checksum stable_deref_trait 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "15132e0e364248108c5e2c02e3ab539be8d6f5d52a01ca9bbf27ed657316f02b"
 "checksum strsim 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)" = "b4d15c810519a91cf877e7e36e63fe068815c678181439f2f29e2562147c3694"
-"checksum subtle 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)" = "7b811576c12506ff3f6da145585dc833edc32ee34c9fc021127d90e8134cc05c"
+"checksum subtle 0.5.1 (registry+https://github.com/rust-lang/crates.io-index)" = "dc7f6353c2ee5407358d063a14cccc1630804527090a6fb5a9489ce4924280fb"
 "checksum syn 0.12.14 (registry+https://github.com/rust-lang/crates.io-index)" = "8c5bc2d6ff27891209efa5f63e9de78648d7801f085e4653701a692ce938d6fd"
 "checksum syntex 0.58.1 (registry+https://github.com/rust-lang/crates.io-index)" = "a8f5e3aaa79319573d19938ea38d068056b826db9883a5d47f86c1cecc688f0e"
 "checksum syntex_errors 0.58.1 (registry+https://github.com/rust-lang/crates.io-index)" = "867cc5c2d7140ae7eaad2ae9e8bf39cb18a67ca651b7834f88d46ca98faadb9c"

ethcrypto/Cargo.toml

@@ -9,4 +9,4 @@ tiny-keccak = "1.3"
 eth-secp256k1 = { git = "https://github.com/paritytech/rust-secp256k1" }
 ethkey = { path = "../ethkey" }
 ethereum-types = "0.2"
-subtle = "0.1"
+subtle = "0.5"

ethcrypto/src/lib.rs

@@ -308,7 +308,7 @@ pub mod ecies {
 		hmac.raw_result(&mut mac);
 
 		// constant time compare to avoid timing attack.
-		if ::subtle::arrays_equal(&mac[..], msg_mac) != 1 {
+		if ::subtle::slices_equal(&mac[..], msg_mac) != 1 {
 			return Err(Error::InvalidMessage);
 		}
 

ethstore/Cargo.toml

@@ -22,6 +22,10 @@ ethereum-types = "0.2"
 dir = { path = "../util/dir" }
 smallvec = "0.4"
 parity-wordlist = "1.0"
+subtle = "0.5"
 tempdir = "0.3"
 
+[dev-dependencies]
+matches = "0.1"
+
 [lib]

ethstore/src/account/crypto.rs

@@ -21,6 +21,7 @@ use crypto::Keccak256;
 use random::Random;
 use smallvec::SmallVec;
 use account::{Cipher, Kdf, Aes128Ctr, Pbkdf2, Prf};
+use subtle;
 
 /// Encrypted data
 #[derive(Debug, PartialEq, Clone)]
@@ -136,7 +137,7 @@ impl Crypto {
 
 		let mac = crypto::derive_mac(&derived_right_bits, &self.ciphertext).keccak256();
 
-		if mac != self.mac {
+		if subtle::slices_equal(&mac, &self.mac) == 0 {
 			return Err(Error::InvalidPassword);
 		}
 
@@ -158,7 +159,7 @@ impl Crypto {
 #[cfg(test)]
 mod tests {
 	use ethkey::{Generator, Random};
-	use super::Crypto;
+	use super::{Crypto, Error};
 
 	#[test]
 	fn crypto_with_secret_create() {
@@ -169,11 +170,10 @@ mod tests {
 	}
 
 	#[test]
-	#[should_panic]
 	fn crypto_with_secret_invalid_password() {
 		let keypair = Random.generate().unwrap();
 		let crypto = Crypto::with_secret(keypair.secret(), "this is sparta", 10240);
-		let _ = crypto.secret("this is sparta!").unwrap();
+		assert_matches!(crypto.secret("this is sparta!"), Err(Error::InvalidPassword))
 	}
 
 	#[test]

ethstore/src/lib.rs

@@ -28,6 +28,7 @@ extern crate rustc_hex;
 extern crate serde;
 extern crate serde_json;
 extern crate smallvec;
+extern crate subtle;
 extern crate time;
 extern crate tiny_keccak;
 extern crate tempdir;
@@ -42,6 +43,10 @@ extern crate log;
 #[macro_use]
 extern crate serde_derive;
 
+#[cfg(test)]
+#[macro_use]
+extern crate matches;
+
 pub mod accounts_dir;
 pub mod ethkey;