diff --git a/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightwallet.rs b/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightwallet.rs index 276e9fd..060464f 100644 --- a/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightwallet.rs +++ b/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightwallet.rs @@ -975,7 +975,9 @@ impl LightWallet { } pub fn get_seed_phrase(&self) -> String { - if !self.unlocked { + // An encrypted wallet with a zero seed is locked, or was left half-locked by an older build: never + // return the all-zero-entropy phrase ("abandon ... art") as if it were the backup. + if !self.unlocked || (self.encrypted && self.seed == [0u8; 32]) { return "".to_string(); } @@ -1025,8 +1027,14 @@ impl LightWallet { return Err(io::Error::new(ErrorKind::AlreadyExists, "Wallet is already locked")); } - // Empty the seed and the secret keys - self.seed.copy_from_slice(&[0u8; 32]); + // Check every key before changing anything. A key with no encrypted copy can't be locked, and + // failing halfway used to leave the seed zeroed while the wallet still reported unlocked. + let t_ok = self.tkeys.read().unwrap().iter().all(|tk| tk.can_lock()); + let z_ok = self.zkeys.read().unwrap().iter().all(|zk| zk.can_lock()); + if !t_ok || !z_ok { + return Err(io::Error::new(ErrorKind::InvalidInput, + "Can't lock: the wallet holds a private key that is not encrypted")); + } // Remove all the private key from the tkeys self.tkeys.write().unwrap().iter_mut().map(|tk| { @@ -1038,6 +1046,9 @@ impl LightWallet { zk.lock() }).collect::>>()?; + // Empty the seed last + self.seed.copy_from_slice(&[0u8; 32]); + self.unlocked = false; Ok(()) @@ -1056,7 +1067,10 @@ impl LightWallet { // Get the doublesha256 of the password, which is the right length let key = secretbox::Key::from_slice(&double_sha256(passwd.as_bytes())).unwrap(); - let nonce = secretbox::Nonce::from_slice(&self.nonce).unwrap(); + let nonce = match secretbox::Nonce::from_slice(&self.nonce) { + Some(n) => n, + None => return Err(io::Error::new(ErrorKind::InvalidData, "Wallet encryption data is corrupt")), + }; let seed = match secretbox::open(&self.enc_seed, &nonce, &key) { Ok(s) => s, @@ -1071,14 +1085,23 @@ impl LightWallet { let bip39_seed = bip39::Seed::new(&Mnemonic::from_entropy(&seed, Language::English).unwrap(), ""); // Go over the tkeys, and add the keys again - self.tkeys.write().unwrap().iter_mut().map(|tk| { + let t = self.tkeys.write().unwrap().iter_mut().map(|tk| { tk.unlock(&key) - }).collect::>>()?; + }).collect::>>(); // Go over the zkeys, and add the spending keys again - self.zkeys.write().unwrap().iter_mut().map(|zk| { + let z = t.and_then(|_| self.zkeys.write().unwrap().iter_mut().map(|zk| { zk.unlock(&self.config, bip39_seed.as_bytes(), &key) - }).collect::>>()?; + }).collect::>>()); + + // A key failed (a zaddress/fvk mismatch, corrupt encryption data): wipe the keys decrypted so far + // before returning. The wallet stays locked, and write() saves a locked wallet, so they would + // otherwise reach the encrypted file in plaintext. + if let Err(e) = z { + for tk in self.tkeys.write().unwrap().iter_mut() { if tk.can_lock() { let _ = tk.lock(); } } + for zk in self.zkeys.write().unwrap().iter_mut() { if zk.can_lock() { let _ = zk.lock(); } } + return Err(e); + } // Everything checks out, so we'll update our wallet with the decrypted values self.seed.copy_from_slice(&seed); @@ -1096,6 +1119,19 @@ impl LightWallet { return Err(Error::new(ErrorKind::AlreadyExists, "Wallet is not encrypted")); } + // Always prove the passphrase, also when the wallet is already unlocked: from the next save on, the + // seed and every key are written in plaintext. + { + let key = secretbox::Key::from_slice(&double_sha256(passwd.as_bytes())).unwrap(); + let nonce = match secretbox::Nonce::from_slice(&self.nonce) { + Some(n) => n, + None => return Err(io::Error::new(ErrorKind::InvalidData, "Wallet encryption data is corrupt")), + }; + if secretbox::open(&self.enc_seed, &nonce, &key).is_err() { + return Err(io::Error::new(ErrorKind::InvalidData, "Decryption failed. Is your password correct?")); + } + } + // Unlock the wallet if it's locked if !self.unlocked { self.unlock(passwd)?; diff --git a/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightwallet/tests.rs b/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightwallet/tests.rs index fedabdb..047be05 100644 --- a/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightwallet/tests.rs +++ b/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightwallet/tests.rs @@ -3502,3 +3502,197 @@ fn test_import_vk_ok_when_encrypted_unlocked() { wallet.unlock("pw".to_string()).unwrap(); assert_eq!(wallet.get_seed_phrase(), phrase); } + +// ---- R4: atomic lock and unlock, corrupt encryption data, seed export, remove_encryption (C252 Fix 3/3b/4, C253a) ---- + +fn r4_contains(hay: &[u8], needle: &[u8]) -> bool { + hay.windows(needle.len()).any(|w| w == needle) +} + +// Every private key of the wallet is present (nothing was wiped). +fn r4_all_keys_present(wallet: &LightWallet) -> bool { + wallet.tkeys.read().unwrap().iter().all(|tk| tk.tkey.is_some()) + && wallet.zkeys.read().unwrap().iter().all(|zk| zk.extsk.is_some()) +} + +#[test] +fn test_lock_is_atomic() { + // s1 test 2 (Fix 3). Before: lock() zeroed the seed and wiped the earlier keys, then failed on the key with + // no encrypted copy, leaving the wallet "unlocked" with the all-zero seed. + use super::{WalletTKey, WalletZKey}; + let config = get_test_config(); + let mut wallet = LightWallet::new(None, &config, 0, 0).unwrap(); + wallet.add_taddr(); + wallet.add_zaddr(); + let other = LightWallet::new(None, &config, 0, 0).unwrap(); + let phrase = wallet.get_seed_phrase(); + let seed = wallet.seed; + wallet.encrypt("pw".to_string()).unwrap(); + wallet.unlock("pw".to_string()).unwrap(); + + // A t-key with no encrypted copy, pushed past the import guard, then the same for an imported z spending key. + let tk = WalletTKey::import_hdkey(r3_key_one(), wallet.address_from_sk(&r3_key_one())); + let zk = WalletZKey::new_imported_sk(other.zkeys.read().unwrap()[0].extsk.clone().unwrap()); + for case in 0..2 { + if case == 0 { wallet.tkeys.write().unwrap().push(tk.clone()); } else { wallet.zkeys.write().unwrap().push(zk.clone()); } + let e = wallet.lock().unwrap_err(); + assert_eq!(wallet.seed, seed, "case {}: the seed changed", case); + assert!(wallet.is_unlocked_for_spending(), "case {}", case); + assert!(r4_all_keys_present(&wallet), "case {}: a key was wiped", case); + assert_eq!(wallet.get_seed_phrase(), phrase); + assert!(wallet.write(&mut vec![]).is_err(), "an encrypted, unlocked wallet must not serialize"); + assert_eq!(e.to_string(), "Can't lock: the wallet holds a private key that is not encrypted"); + if case == 0 { wallet.tkeys.write().unwrap().pop(); } else { wallet.zkeys.write().unwrap().pop(); } + } + + // With every key sealed again, lock and unlock work. + wallet.lock().unwrap(); + assert!(!wallet.is_unlocked_for_spending()); + assert_eq!(wallet.seed, [0u8; 32]); + wallet.unlock("pw".to_string()).unwrap(); + assert_eq!(wallet.get_seed_phrase(), phrase); + assert!(r4_all_keys_present(&wallet)); +} + +#[test] +fn test_unlock_failure_rewipes() { + // s1 test 5 (Fix 3b). Before: the t-keys and the z-keys before the failing one stayed decrypted while the + // wallet reported locked, and write() put them in plaintext into the encrypted file. + let config = get_test_config(); + let mut wallet = LightWallet::new(None, &config, 0, 0).unwrap(); + wallet.add_taddr(); + wallet.add_zaddr(); + wallet.add_zaddr(); + let phrase = wallet.get_seed_phrase(); + let tsecrets: Vec = wallet.tkeys.read().unwrap().iter().map(|tk| tk.tkey.unwrap()).collect(); + let zsecrets: Vec> = wallet.zkeys.read().unwrap().iter().map(|zk| { + let mut v = vec![]; + zk.extsk.as_ref().unwrap().write(&mut v).unwrap(); + v + }).collect(); + wallet.encrypt("pw".to_string()).unwrap(); + let num = wallet.zkeys.read().unwrap().last().unwrap().hdkey_num; + wallet.zkeys.write().unwrap().last_mut().unwrap().hdkey_num = Some(999); + + let e = wallet.unlock("pw".to_string()).unwrap_err(); + assert!(e.to_string().contains("mismatch"), "{}", e); + assert!(!wallet.is_unlocked_for_spending()); + assert!(wallet.tkeys.read().unwrap().iter().all(|tk| tk.tkey.is_none()), "a t-key stayed decrypted"); + assert!(wallet.zkeys.read().unwrap().iter().all(|zk| zk.extsk.is_none()), "a z-key stayed decrypted"); + assert_eq!(wallet.seed, [0u8; 32]); + assert_eq!(wallet.get_seed_phrase(), ""); + + let mut buf = vec![]; + wallet.write(&mut buf).unwrap(); + assert_eq!(buf[8], 1); + for sk in &tsecrets { + assert!(!r4_contains(&buf, &sk[..]), "a t-key secret reached the encrypted file"); + } + for z in &zsecrets { + assert!(!r4_contains(&buf, z), "a z spending key reached the encrypted file"); + } + + // Nothing was lost: with the right key number the same passphrase unlocks every key. + wallet.zkeys.write().unwrap().last_mut().unwrap().hdkey_num = num; + wallet.unlock("pw".to_string()).unwrap(); + assert_eq!(wallet.get_seed_phrase(), phrase); + assert!(r4_all_keys_present(&wallet)); +} + +#[test] +fn test_tkey_unlock_missing_enc_key_is_err() { + // s1 test 6 (Fix 3b). Before: WalletTKey::unlock, WalletZKey::unlock and LightWallet::unlock unwrapped the + // nonce and the sealed key, so a malformed or foreign file panicked under the wallet write lock. + use sodiumoxide::crypto::secretbox; + use super::{WalletTKey, WalletZKey}; + let config = get_test_config(); + let key = secretbox::Key::from_slice(&[7u8; 32]).unwrap(); + let corrupt = |e: std::io::Error| { + assert_eq!(e.kind(), std::io::ErrorKind::InvalidData); + assert_eq!(e.to_string(), "Wallet encryption data is corrupt"); + }; + + let mut tk = WalletTKey::import_hdkey(r3_key_one(), "t".to_string()); + assert!(!tk.can_lock()); + corrupt(tk.unlock(&key).unwrap_err()); + assert_eq!(tk.tkey, Some(r3_key_one())); + + let other = LightWallet::new(None, &config, 0, 0).unwrap(); + let mut zk = WalletZKey::new_imported_sk(other.zkeys.read().unwrap()[0].extsk.clone().unwrap()); + assert!(!zk.can_lock()); + corrupt(zk.unlock(&config, &[], &key).unwrap_err()); + + // A wallet nonce that is not 24 bytes: unlock and remove_encryption return Err and change nothing. + let mut wallet = LightWallet::new(None, &config, 0, 0).unwrap(); + let phrase = wallet.get_seed_phrase(); + wallet.encrypt("pw".to_string()).unwrap(); + let nonce = wallet.nonce.clone(); + for bad in vec![vec![], vec![1u8; 23], vec![1u8; 25]] { + wallet.nonce = bad; + corrupt(wallet.unlock("pw".to_string()).unwrap_err()); + corrupt(wallet.remove_encryption("pw".to_string()).unwrap_err()); + assert!(wallet.is_encrypted() && !wallet.is_unlocked_for_spending()); + assert!(wallet.tkeys.read().unwrap().iter().all(|tk| tk.tkey.is_none())); + } + wallet.nonce = nonce; + wallet.unlock("pw".to_string()).unwrap(); + assert_eq!(wallet.get_seed_phrase(), phrase); +} + +#[test] +fn test_remove_encryption_requires_passphrase() { + // s1 test 7 (C253a). Before: an unlocked wallet never checked the passphrase, so any string, even "", + // removed the encryption for good (b). + let config = get_test_config(); + let mut wallet = LightWallet::new(None, &config, 0, 0).unwrap(); + let phrase = wallet.get_seed_phrase(); + wallet.encrypt("right".to_string()).unwrap(); + let wrong_pw = "Decryption failed. Is your password correct?"; + + // (a) Locked: a wrong passphrase changes nothing. + assert_eq!(wallet.remove_encryption("wrong".to_string()).unwrap_err().to_string(), wrong_pw); + assert!(wallet.is_encrypted() && !wallet.is_unlocked_for_spending()); + + // (b) Unlocked: a wrong or empty passphrase is refused too; the wallet stays encrypted and still locks. + wallet.unlock("right".to_string()).unwrap(); + for wrong in &["wrong", ""] { + assert_eq!(wallet.remove_encryption(wrong.to_string()).unwrap_err().to_string(), wrong_pw); + assert!(wallet.is_encrypted() && wallet.is_unlocked_for_spending()); + assert!(wallet.write(&mut vec![]).is_err(), "an encrypted, unlocked wallet must not serialize"); + assert!(wallet.tkeys.read().unwrap().iter().all(|tk| tk.can_lock())); + assert!(wallet.zkeys.read().unwrap().iter().all(|zk| zk.can_lock())); + } + wallet.lock().unwrap(); + wallet.unlock("right".to_string()).unwrap(); + assert_eq!(wallet.get_seed_phrase(), phrase); + + // (c) The right passphrase removes it; the file round-trips unencrypted at version 9. + wallet.remove_encryption("right".to_string()).unwrap(); + assert!(!wallet.is_encrypted() && wallet.is_unlocked_for_spending()); + let mut buf = vec![]; + wallet.write(&mut buf).unwrap(); + assert_eq!(&buf[..8], &9u64.to_le_bytes()[..]); + assert_eq!(buf[8], 0); + let w2 = LightWallet::read(&buf[..], &config).unwrap(); + assert!(!w2.is_encrypted() && w2.is_unlocked_for_spending()); + assert_eq!(w2.get_seed_phrase(), phrase); + assert!(r4_all_keys_present(&w2)); + let mut buf2 = vec![]; + w2.write(&mut buf2).unwrap(); + assert_eq!(buf2, buf); +} + +#[test] +fn test_seed_phrase_empty_when_encrypted_seed_is_zero() { + // s1 Fix 4. An older build's half-done lock() left an encrypted wallet "unlocked" with a zeroed seed, and seed + // export then returned the all-zero-entropy phrase as the backup. + let config = get_test_config(); + let mut wallet = LightWallet::new(None, &config, 0, 0).unwrap(); + let phrase = wallet.get_seed_phrase(); + wallet.encrypt("pw".to_string()).unwrap(); + assert_eq!(wallet.get_seed_phrase(), ""); + wallet.unlock("pw".to_string()).unwrap(); + assert_eq!(wallet.get_seed_phrase(), phrase); + wallet.seed = [0u8; 32]; + assert_eq!(wallet.get_seed_phrase(), ""); +} diff --git a/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightwallet/walletzkey.rs b/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightwallet/walletzkey.rs index d8eb8e4..5ef42e4 100644 --- a/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightwallet/walletzkey.rs +++ b/third_party/silentdragonxlite/silentdragonxlite-cli/lib/src/lightwallet/walletzkey.rs @@ -35,6 +35,15 @@ pub struct WalletTKey { nonce: Option>, } +// The sealed key and its nonce. A key with no sealed copy or no nonce, or a nonce of the wrong length, means +// the wallet file is corrupt: unlock returns this error instead of panicking under the wallet write lock. +fn sealed_key<'a>(enc_key: &'a Option>, nonce: &Option>) -> io::Result<(&'a [u8], secretbox::Nonce)> { + match (enc_key, nonce.as_ref().and_then(|n| secretbox::Nonce::from_slice(n))) { + (Some(k), Some(n)) => Ok((&k[..], n)), + _ => Err(io::Error::new(ErrorKind::InvalidData, "Wallet encryption data is corrupt")), + } +} + impl WalletTKey { pub fn new_hdkey(key: secp256k1::SecretKey, address: String) -> Self { WalletTKey { @@ -120,6 +129,11 @@ impl WalletTKey { } + // lock() fails for a key with no encrypted copy; LightWallet::lock() checks every key with this first. + pub fn can_lock(&self) -> bool { + self.enc_key.is_some() + } + pub fn lock(&mut self) -> io::Result<()> { // For keys, encrypt the key into enckey // assert that we have the encrypted key. @@ -135,8 +149,8 @@ impl WalletTKey { pub fn unlock(&mut self, key: &secretbox::Key) -> io::Result<()> { // For imported keys, we need to decrypt from the encrypted key - let nonce = secretbox::Nonce::from_slice(&self.nonce.as_ref().unwrap()).unwrap(); - let sk_bytes = match secretbox::open(&self.enc_key.as_ref().unwrap(), &nonce, &key) { + let (enc_key, nonce) = sealed_key(&self.enc_key, &self.nonce)?; + let sk_bytes = match secretbox::open(enc_key, &nonce, &key) { Ok(s) => s, Err(_) => {return Err(io::Error::new(ErrorKind::InvalidData, "Decryption failed. Is your password correct?"));} }; @@ -328,6 +342,15 @@ impl WalletZKey { Vector::write(o, v, |o,n| o.write_u8(*n))) } + // lock() fails only for an imported spending key with no encrypted copy; LightWallet::lock() checks every + // key with this first. + pub fn can_lock(&self) -> bool { + match self.keytype { + WalletZKeyType::ImportedSpendingKey => self.enc_key.is_some(), + _ => true, + } + } + pub fn lock(&mut self) -> io::Result<()> { match self.keytype { WalletZKeyType::HdKey => { @@ -373,8 +396,8 @@ impl WalletZKey { }, WalletZKeyType::ImportedSpendingKey => { // For imported keys, we need to decrypt from the encrypted key - let nonce = secretbox::Nonce::from_slice(&self.nonce.as_ref().unwrap()).unwrap(); - let extsk_bytes = match secretbox::open(&self.enc_key.as_ref().unwrap(), &nonce, &key) { + let (enc_key, nonce) = sealed_key(&self.enc_key, &self.nonce)?; + let extsk_bytes = match secretbox::open(enc_key, &nonce, &key) { Ok(s) => s, Err(_) => {return Err(io::Error::new(ErrorKind::InvalidData, "Decryption failed. Is your password correct?"));} }; @@ -535,6 +558,50 @@ pub mod tests { } + #[test] + fn test_unlock_corrupt_encryption_data_is_err() { + // s1 Fix 3b (closes s2's X8 residual). Before: unlock unwrapped the nonce and the sealed key, so a sealed + // key with no nonce, a nonce of the wrong length or a missing sealed key panicked under the wallet lock. + let config = get_config(); + let key = secretbox::Key::from_slice(&[0; 32]).unwrap(); + let extsk = super::LightWallet::get_zaddr_from_bip39seed(&config, &[1u8; 64], 0).0; + + let mut wzk = WalletZKey::new_imported_sk(extsk.clone()); + wzk.encrypt(&key).unwrap(); + let mut wtk = super::WalletTKey::import_hdkey(secp256k1::SecretKey::from_slice(&[1u8; 32]).unwrap(), "t".to_string()); + wtk.encrypt(&key).unwrap(); + let (znonce, tnonce) = (wzk.nonce.clone(), wtk.nonce.clone()); + + for bad in vec![None, Some(vec![]), Some(vec![0u8; 23]), Some(vec![0u8; 25])] { + wzk.nonce = bad.clone(); + wtk.nonce = bad; + let ez = wzk.unlock(&config, &[], &key).unwrap_err(); + let et = wtk.unlock(&key).unwrap_err(); + for e in &[ez, et] { + assert_eq!(e.kind(), super::ErrorKind::InvalidData); + assert_eq!(e.to_string(), "Wallet encryption data is corrupt"); + } + assert!(wzk.extsk.is_none() && wtk.tkey.is_none()); + } + + // A good nonce with no sealed key is corrupt too. + wzk.nonce = znonce.clone(); + wtk.nonce = tnonce.clone(); + let (zenc, tenc) = (wzk.enc_key.take(), wtk.enc_key.take()); + assert_eq!(wzk.unlock(&config, &[], &key).unwrap_err().to_string(), "Wallet encryption data is corrupt"); + assert_eq!(wtk.unlock(&key).unwrap_err().to_string(), "Wallet encryption data is corrupt"); + assert!(!wzk.can_lock() && !wtk.can_lock()); + + // Restored, both unlock. + wzk.enc_key = zenc; + wtk.enc_key = tenc; + assert!(wzk.can_lock() && wtk.can_lock()); + wzk.unlock(&config, &[], &key).unwrap(); + wtk.unlock(&key).unwrap(); + assert_eq!(wzk.extsk, Some(extsk)); + assert_eq!(wtk.tkey, Some(secp256k1::SecretKey::from_slice(&[1u8; 32]).unwrap())); + } + #[test] fn test_encrypt_decrypt_vk() { let config = get_config();