From f7bf34eeac8030888ac97f008d793e36e24d9a41 Mon Sep 17 00:00:00 2001 From: Vera Abramova Date: Tue, 19 Nov 2024 21:14:40 +0200 Subject: [PATCH 1/4] fix: zeroize secret stuff --- Cargo.toml | 6 +-- src/error.rs | 38 ++++++++++++---- src/lib.rs | 119 +++++++++++++++++++++++++++++++++++-------------- src/regular.rs | 10 ++--- src/tests.rs | 12 ++--- 5 files changed, 128 insertions(+), 57 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index b5815b9..6ec39e6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,17 +4,15 @@ version = "0.1.0" edition = "2021" [dependencies] -bitvec = { version = "1.0.1", default-features = false, features = ["alloc"] } sha2 = { version = "0.10.6", default-features = false } - -thiserror = { version = "1", optional = true } +zeroize = {version = "1.8.1", features = ["derive"]} [dev-dependencies] hex = "0.4.3" [features] default = ["std", "sufficient-memory"] -std = ["thiserror"] +std = [] sufficient-memory = [] [lib] diff --git a/src/error.rs b/src/error.rs index 0ec197d..5baab77 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,11 +1,17 @@ #[cfg(feature = "std")] -use std::fmt::{Debug, Display, Formatter, Result}; +use std::string::String; + +#[cfg(not(feature = "std"))] +use alloc::string::String; + #[cfg(feature = "std")] -use thiserror::Error; +use std::fmt::{Debug, Display, Formatter, Result as FmtResult}; + +#[cfg(not(feature = "std"))] +use core::fmt::{Debug, Display, Formatter, Result as FmtResult}; #[derive(Debug)] -#[cfg_attr(feature = "std", derive(Error))] -pub enum ErrorWordList { +pub enum ErrorMnemonic { DamagedWord, InvalidChecksum, InvalidEntropy, @@ -14,10 +20,24 @@ pub enum ErrorWordList { WordsNumber, } -// TODO: provide actual error descriptions. -#[cfg(feature = "std")] -impl Display for ErrorWordList { - fn fmt(&self, f: &mut Formatter<'_>) -> Result { - ::fmt(self, f) +impl ErrorMnemonic { + fn error_text(&self) -> String { + match &self { + ErrorMnemonic::DamagedWord => String::from("Unable to extract a word from the word list."), + ErrorMnemonic::InvalidChecksum => String::from("Invalid text mnemonic: the checksum does not match."), + ErrorMnemonic::InvalidEntropy => String::from("Unable to calculate the mnemonic from entropy. Invalid entropy length."), + ErrorMnemonic::InvalidWordNumber => String::from("Ordinal number for word requested is higher than total number of words in the word list."), + ErrorMnemonic::NoWord => String::from("Requested word in not in the word list."), + ErrorMnemonic::WordsNumber => String::from("Invalid text mnemonic: unexpected number of words."), + } } } + +impl Display for ErrorMnemonic { + fn fmt(&self, f: &mut Formatter<'_>) -> FmtResult { + write!(f, "{}", self.error_text()) + } +} + +#[cfg(feature = "std")] +impl std::error::Error for ErrorMnemonic {} diff --git a/src/lib.rs b/src/lib.rs index 32c6701..c9a5416 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -3,7 +3,9 @@ #[cfg(not(feature = "std"))] extern crate alloc; + #[cfg(feature = "std")] +#[macro_use] extern crate std; #[cfg(not(feature = "std"))] @@ -12,8 +14,8 @@ use alloc::{string::String, vec::Vec}; #[cfg(feature = "std")] use std::{string::String, vec::Vec}; -use bitvec::prelude::{BitSlice, BitVec, Msb0}; use sha2::{Digest, Sha256}; +use zeroize::{Zeroize, ZeroizeOnDrop}; pub mod error; @@ -26,7 +28,7 @@ mod tests; #[cfg(any(feature = "sufficient-memory", test))] pub mod wordlist; -use crate::error::ErrorWordList; +use crate::error::ErrorMnemonic; pub const TOTAL_WORDS: usize = 2048; pub const WORD_MAX_LEN: usize = 8; @@ -34,22 +36,23 @@ pub const SEPARATOR_LEN: usize = 1; pub const MAX_SEED_LEN: usize = 24; -#[derive(Clone, Copy, Debug)] +#[derive(Clone, Copy, Debug, Zeroize)] pub struct Bits11(u16); impl Bits11 { pub fn bits(self) -> u16 { self.0 } - pub fn from(i: u16) -> Result { + pub fn from(i: u16) -> Result { if (i as usize) < TOTAL_WORDS { Ok(Self(i)) } else { - Err(ErrorWordList::InvalidWordNumber) + Err(ErrorMnemonic::InvalidWordNumber) } } } +#[derive(Clone, Debug)] pub struct WordListElement { pub word: L::Word, pub bits11: Bits11, @@ -57,12 +60,12 @@ pub struct WordListElement { pub trait AsWordList { type Word: AsRef; - fn get_word(&self, bits: Bits11) -> Result; + fn get_word(&self, bits: Bits11) -> Result; fn get_words_by_prefix( &self, prefix: &str, - ) -> Result>, ErrorWordList>; - fn bits11_for_word(&self, word: &str) -> Result; + ) -> Result>, ErrorMnemonic>; + fn bits11_for_word(&self, word: &str) -> Result; } #[derive(Debug, Copy, Clone)] @@ -75,14 +78,14 @@ pub enum MnemonicType { } impl MnemonicType { - fn from(len: usize) -> Result { + fn from(len: usize) -> Result { match len { 12 => Ok(Self::Words12), 15 => Ok(Self::Words15), 18 => Ok(Self::Words18), 21 => Ok(Self::Words21), 24 => Ok(Self::Words24), - _ => Err(ErrorWordList::WordsNumber), + _ => Err(ErrorMnemonic::WordsNumber), } } fn checksum_bits(&self) -> u8 { @@ -108,27 +111,67 @@ impl MnemonicType { } } +#[derive(Clone, Debug, ZeroizeOnDrop)] +struct BitsHelper { + bits: Vec, +} + +impl BitsHelper { + fn with_capacity(cap: usize) -> Self { + Self { + bits: Vec::with_capacity(cap), + } + } + + fn extend_from_byte(&mut self, byte: u8) { + for i in 0..BITS_IN_BYTE { + let bit = (byte & (1 << (BITS_IN_BYTE - 1 - i))) != 0; + self.bits.push(bit); + } + } + + fn extend_from_bits11(&mut self, bits11: &Bits11) { + let two_bytes = bits11.0.to_be_bytes(); + + // last 3 bits of first byte - others are always zero + for i in (BITS_IN_BYTE - BITS_IN_U11 % BITS_IN_BYTE)..BITS_IN_BYTE { + let bit = (two_bytes[0] & (1 << (BITS_IN_BYTE - 1 - i))) != 0; + self.bits.push(bit); + } + + // all bits of second byte + self.extend_from_byte(two_bytes[1]) + } +} + +pub const BITS_IN_BYTE: usize = 8; +pub const BITS_IN_U11: usize = 11; + +#[derive(Clone, Debug, ZeroizeOnDrop)] pub struct WordSet { pub bits11_set: Vec, } impl WordSet { - pub fn from_entropy(entropy: &[u8]) -> Result { + pub fn from_entropy(entropy: &[u8]) -> Result { if entropy.len() < 16 || entropy.len() > 32 || entropy.len() % 4 != 0 { - return Err(ErrorWordList::InvalidEntropy); + return Err(ErrorMnemonic::InvalidEntropy); } let checksum_byte = sha256_first_byte(entropy); - let mut entropy_bits: BitVec = BitVec::with_capacity((entropy.len() + 1) * 8); - entropy_bits.extend_from_bitslice(&BitVec::::from_slice(entropy)); - entropy_bits.extend_from_bitslice(&BitVec::::from_element(checksum_byte)); - let mut bits11_set: Vec = Vec::new(); - for chunk in entropy_bits.chunks_exact(11usize) { + let mut entropy_bits = BitsHelper::with_capacity((entropy.len() + 1) * BITS_IN_BYTE); + for byte in entropy { + entropy_bits.extend_from_byte(*byte); + } + entropy_bits.extend_from_byte(checksum_byte); + + let mut bits11_set: Vec = Vec::with_capacity(MAX_SEED_LEN); + for chunk in entropy_bits.bits.chunks_exact(BITS_IN_U11) { let mut bits11: u16 = 0; - for (i, bit) in chunk.into_iter().enumerate() { + for (i, bit) in chunk.iter().enumerate() { if *bit { - bits11 |= 1 << (10 - i) + bits11 |= 1 << (BITS_IN_U11 - 1 - i) } } bits11_set.push(Bits11(bits11)); @@ -146,7 +189,7 @@ impl WordSet { &mut self, word: &str, wordlist: &L, - ) -> Result<(), ErrorWordList> { + ) -> Result<(), ErrorMnemonic> { if self.bits11_set.len() < MAX_SEED_LEN { let bits11 = wordlist.bits11_for_word(word)?; self.bits11_set.push(bits11); @@ -158,19 +201,28 @@ impl WordSet { MnemonicType::from(self.bits11_set.len()).is_ok() } - pub fn to_entropy(&self) -> Result, ErrorWordList> { + pub fn to_entropy(&self) -> Result, ErrorMnemonic> { let mnemonic_type = MnemonicType::from(self.bits11_set.len())?; - let mut entropy_bits: BitVec = BitVec::with_capacity(mnemonic_type.total_bits()); + let mut entropy_bits = BitsHelper::with_capacity(mnemonic_type.total_bits()); for bits11 in self.bits11_set.iter() { - entropy_bits.extend_from_bitslice( - &BitSlice::::from_slice(&bits11.bits().to_be_bytes())[5..16], - ) + entropy_bits.extend_from_bits11(bits11); + } + + let mut entropy: Vec = Vec::with_capacity(mnemonic_type.total_bits() / BITS_IN_BYTE); + + for chunk in entropy_bits.bits.chunks(BITS_IN_BYTE) { + let mut byte: u8 = 0; + for (i, bit) in chunk.iter().enumerate() { + if *bit { + byte |= 1 << (BITS_IN_BYTE - 1 - i) + } + } + entropy.push(byte); } - let mut entropy = entropy_bits.into_vec(); - let entropy_len = mnemonic_type.entropy_bits() / 8; + let entropy_len = mnemonic_type.entropy_bits() / BITS_IN_BYTE; let actual_checksum = checksum(entropy[entropy_len], mnemonic_type.checksum_bits()); @@ -181,15 +233,16 @@ impl WordSet { let expected_checksum = checksum(checksum_byte, mnemonic_type.checksum_bits()); if actual_checksum != expected_checksum { - Err(ErrorWordList::InvalidChecksum) + Err(ErrorMnemonic::InvalidChecksum) } else { Ok(entropy) } } - pub fn to_phrase(&self, wordlist: &L) -> Result { - let mut phrase = - String::with_capacity(self.bits11_set.len() * (WORD_MAX_LEN + SEPARATOR_LEN) - 1); + pub fn to_phrase(&self, wordlist: &L) -> Result { + let mut phrase = String::with_capacity( + self.bits11_set.len() * (WORD_MAX_LEN + SEPARATOR_LEN) - SEPARATOR_LEN, + ); for bits11 in self.bits11_set.iter() { if !phrase.is_empty() { phrase.push(' ') @@ -202,8 +255,8 @@ impl WordSet { } fn checksum(source: u8, bits: u8) -> u8 { - assert!(bits <= 8); - source >> (8 - bits) + assert!(bits <= BITS_IN_BYTE as u8); + source >> (BITS_IN_BYTE as u8 - bits) } fn sha256_first_byte(input: &[u8]) -> u8 { diff --git a/src/regular.rs b/src/regular.rs index 132ab43..5c89aac 100644 --- a/src/regular.rs +++ b/src/regular.rs @@ -4,7 +4,7 @@ use alloc::vec::Vec; #[cfg(feature = "std")] use std::vec::Vec; -use crate::error::ErrorWordList; +use crate::error::ErrorMnemonic; use crate::wordlist::WORDLIST_ENGLISH; use crate::{AsWordList, Bits11, WordListElement}; @@ -13,7 +13,7 @@ pub struct InternalWordList; impl AsWordList for InternalWordList { type Word = &'static str; - fn get_word(&self, bits: Bits11) -> Result { + fn get_word(&self, bits: Bits11) -> Result { let word_order = bits.bits() as usize; Ok(WORDLIST_ENGLISH[word_order]) } @@ -21,7 +21,7 @@ impl AsWordList for InternalWordList { fn get_words_by_prefix( &self, prefix: &str, - ) -> Result>, ErrorWordList> { + ) -> Result>, ErrorMnemonic> { let mut out: Vec> = Vec::new(); for (i, word) in WORDLIST_ENGLISH.iter().enumerate() { if word.starts_with(prefix) { @@ -34,12 +34,12 @@ impl AsWordList for InternalWordList { Ok(out) } - fn bits11_for_word(&self, word: &str) -> Result { + fn bits11_for_word(&self, word: &str) -> Result { for (i, element) in WORDLIST_ENGLISH.iter().enumerate() { if element == &word { return Bits11::from(i as u16); } } - Err(ErrorWordList::NoWord) + Err(ErrorMnemonic::NoWord) } } diff --git a/src/tests.rs b/src/tests.rs index a7bb01d..3bc4295 100644 --- a/src/tests.rs +++ b/src/tests.rs @@ -4,7 +4,7 @@ use alloc::{string::String, vec::Vec}; #[cfg(feature = "std")] use std::{string::String, vec::Vec}; -use crate::error::ErrorWordList; +use crate::error::ErrorMnemonic; #[cfg(feature = "sufficient-memory")] use crate::regular::InternalWordList; @@ -29,19 +29,19 @@ struct FlashMockWordList; impl AsWordList for FlashMockWordList { type Word = String; - fn get_word(&self, bits: Bits11) -> Result { + fn get_word(&self, bits: Bits11) -> Result { let word_order = bits.bits() as usize; let mut word_bytes = unsafe { FLASH_MOCK[word_order * WORD_MAX_LEN..(word_order + 1) * WORD_MAX_LEN].to_vec() }; word_bytes = word_bytes.into_iter().take_while(|x| *x != 255).collect(); - String::from_utf8(word_bytes).map_err(|_| ErrorWordList::DamagedWord) + String::from_utf8(word_bytes).map_err(|_| ErrorMnemonic::DamagedWord) } fn get_words_by_prefix( &self, prefix: &str, - ) -> Result>, ErrorWordList> { + ) -> Result>, ErrorMnemonic> { let mut words_by_prefix: Vec> = Vec::new(); for bits_u16 in 0..TOTAL_WORDS { let bits11 = Bits11::from(bits_u16 as u16)?; @@ -55,7 +55,7 @@ impl AsWordList for FlashMockWordList { Ok(words_by_prefix) } - fn bits11_for_word(&self, word: &str) -> Result { + fn bits11_for_word(&self, word: &str) -> Result { for bits_u16 in 0..TOTAL_WORDS { let bits11 = Bits11::from(bits_u16 as u16)?; let read_word = self.get_word(bits11)?; @@ -63,7 +63,7 @@ impl AsWordList for FlashMockWordList { return Ok(bits11); } } - Err(ErrorWordList::NoWord) + Err(ErrorMnemonic::NoWord) } } From 09d47cf4b29c4aed22a7a298285a40987143a021 Mon Sep 17 00:00:00 2001 From: Vera Abramova Date: Tue, 19 Nov 2024 22:25:05 +0200 Subject: [PATCH 2/4] chore: more clear bit operations --- src/lib.rs | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index c9a5416..1a366ba 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -124,8 +124,8 @@ impl BitsHelper { } fn extend_from_byte(&mut self, byte: u8) { - for i in 0..BITS_IN_BYTE { - let bit = (byte & (1 << (BITS_IN_BYTE - 1 - i))) != 0; + for i in (0..BITS_IN_BYTE).rev() { + let bit = (byte & (1 << i)) != 0; self.bits.push(bit); } } @@ -134,8 +134,8 @@ impl BitsHelper { let two_bytes = bits11.0.to_be_bytes(); // last 3 bits of first byte - others are always zero - for i in (BITS_IN_BYTE - BITS_IN_U11 % BITS_IN_BYTE)..BITS_IN_BYTE { - let bit = (two_bytes[0] & (1 << (BITS_IN_BYTE - 1 - i))) != 0; + for i in (0..BITS_IN_U11 % BITS_IN_BYTE).rev() { + let bit = (two_bytes[0] & (1 << i)) != 0; self.bits.push(bit); } @@ -169,9 +169,9 @@ impl WordSet { let mut bits11_set: Vec = Vec::with_capacity(MAX_SEED_LEN); for chunk in entropy_bits.bits.chunks_exact(BITS_IN_U11) { let mut bits11: u16 = 0; - for (i, bit) in chunk.iter().enumerate() { + for (i, bit) in chunk.iter().rev().enumerate() { if *bit { - bits11 |= 1 << (BITS_IN_U11 - 1 - i) + bits11 |= 1 << i } } bits11_set.push(Bits11(bits11)); @@ -212,16 +212,28 @@ impl WordSet { let mut entropy: Vec = Vec::with_capacity(mnemonic_type.total_bits() / BITS_IN_BYTE); - for chunk in entropy_bits.bits.chunks(BITS_IN_BYTE) { + let chunks_exact = entropy_bits.bits.chunks_exact(BITS_IN_BYTE); + let remainder = chunks_exact.remainder(); + + for chunk in chunks_exact { let mut byte: u8 = 0; - for (i, bit) in chunk.iter().enumerate() { + for (i, bit) in chunk.iter().rev().enumerate() { if *bit { - byte |= 1 << (BITS_IN_BYTE - 1 - i) + byte |= 1 << i } } entropy.push(byte); } + let mut last_byte: u8 = 0; + for (i, bit) in remainder.iter().rev().enumerate() { + if *bit { + last_byte |= 1 << BITS_IN_BYTE - remainder.len() + i + } + } + + entropy.push(last_byte); + let entropy_len = mnemonic_type.entropy_bits() / BITS_IN_BYTE; let actual_checksum = checksum(entropy[entropy_len], mnemonic_type.checksum_bits()); From be51fc1826dd011c7e66ed59d0d8ea4bd809ce5b Mon Sep 17 00:00:00 2001 From: Vera Abramova Date: Tue, 19 Nov 2024 22:27:37 +0200 Subject: [PATCH 3/4] chore: parenthesis --- src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 1a366ba..81ef528 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -228,7 +228,7 @@ impl WordSet { let mut last_byte: u8 = 0; for (i, bit) in remainder.iter().rev().enumerate() { if *bit { - last_byte |= 1 << BITS_IN_BYTE - remainder.len() + i + last_byte |= 1 << (BITS_IN_BYTE - remainder.len() + i) } } From ef5e44d245ec48ec738dae237d3d519aedb88728 Mon Sep 17 00:00:00 2001 From: varovainen <99664267+varovainen@users.noreply.github.com> Date: Tue, 19 Nov 2024 22:32:54 +0200 Subject: [PATCH 4/4] fix: correct division Co-authored-by: Slesarew <33295157+Slesarew@users.noreply.github.com> --- src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib.rs b/src/lib.rs index 81ef528..46cba91 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -234,7 +234,7 @@ impl WordSet { entropy.push(last_byte); - let entropy_len = mnemonic_type.entropy_bits() / BITS_IN_BYTE; + let entropy_len = mnemonic_type.entropy_bits().div_ceil(BITS_IN_BYTE); let actual_checksum = checksum(entropy[entropy_len], mnemonic_type.checksum_bits());