From 236692cfd5cba39dd700676705c2bc9b02a4f0b2 Mon Sep 17 00:00:00 2001 From: Toralf Wittner Date: Wed, 14 Mar 2018 15:41:35 +0100 Subject: [PATCH] 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 | 11 +++++------ ethcrypto/Cargo.toml | 2 +- ethcrypto/src/lib.rs | 2 +- ethstore/Cargo.toml | 4 ++++ ethstore/src/account/crypto.rs | 8 ++++---- ethstore/src/lib.rs | 5 +++++ 6 files changed, 20 insertions(+), 12 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ecbc10ac3..3759a6359 100644 --- a/Cargo.lock +++ b/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" diff --git a/ethcrypto/Cargo.toml b/ethcrypto/Cargo.toml index 2344f59d0..13b098fb3 100644 --- a/ethcrypto/Cargo.toml +++ b/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" diff --git a/ethcrypto/src/lib.rs b/ethcrypto/src/lib.rs index 8f00004ab..a09f087e0 100644 --- a/ethcrypto/src/lib.rs +++ b/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); } diff --git a/ethstore/Cargo.toml b/ethstore/Cargo.toml index 1f821cb1f..a504c7f2a 100755 --- a/ethstore/Cargo.toml +++ b/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] diff --git a/ethstore/src/account/crypto.rs b/ethstore/src/account/crypto.rs index cc7489514..967c92306 100755 --- a/ethstore/src/account/crypto.rs +++ b/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] diff --git a/ethstore/src/lib.rs b/ethstore/src/lib.rs index 0f6094ce3..3422a5ff7 100755 --- a/ethstore/src/lib.rs +++ b/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;