diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a7f3d39..b67a8a6 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -11,10 +11,9 @@ on: - '**/rust-toolchain.toml' # Run on changed source files - 'src/**' - - 'src/**' - branches: main + branches: [main] pull_request: - branches: main + branches: [main] # Sometimes the rules above don't match even though they should. # This allows us to run the workflow manually anyways. workflow_dispatch: @@ -45,9 +44,6 @@ jobs: name: Rust format runs-on: ubuntu-latest - strategy: - fail-fast: true - steps: - uses: actions/checkout@v3 @@ -87,9 +83,6 @@ jobs: runs-on: ubuntu-latest needs: ["rust_check", "rust_lint"] - strategy: - fail-fast: true - steps: - uses: actions/checkout@v3 diff --git a/.vscode/settings.json b/.vscode/settings.json deleted file mode 100644 index dbb4894..0000000 --- a/.vscode/settings.json +++ /dev/null @@ -1,3 +0,0 @@ -{ - "rust-analyzer.cargo.features": ["serde"] -} \ No newline at end of file diff --git a/.woodpecker.yml b/.woodpecker.yml deleted file mode 100644 index dafbeb4..0000000 --- a/.woodpecker.yml +++ /dev/null @@ -1,12 +0,0 @@ -pipeline: - test: - image: rust:alpine - pull: true - commands: - - apk add musl-dev - - RUST_BACKTRACE=1 cargo test -- --nocapture - - RUST_BACKTRACE=1 cargo test -F serde -- --nocapture - - rustup target add i686-unknown-freebsd aarch64-pc-windows-msvc - - cargo check --target i686-unknown-freebsd - - cargo check --target aarch64-pc-windows-msvc - - cargo fmt --check diff --git a/Cargo.lock b/Cargo.lock index 3913792..4c47c53 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1,164 +1,68 @@ # This file is automatically @generated by Cargo. # It is not intended for manual editing. -version = 3 - -[[package]] -name = "cfg-if" -version = "0.1.10" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4785bdd1c96b2a846b2bd7cc02e86b6b3dbf14e7e53446c4f54c92a361040822" +version = 4 [[package]] name = "half" -version = "1.8.2" +version = "1.8.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "eabb4a44450da02c90444cf74558da904edde8fb4e9035a9a6a4e15445af0bd7" +checksum = "1b43ede17f21864e81be2fa654110bf1e793774238d86ef8555c37e6519c0403" [[package]] name = "itoa" -version = "1.0.9" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "af150ab688ff2122fcef229be89cb50dd66af9e01a4ff320cc137eecc9bacc38" - -[[package]] -name = "lazy_static" -version = "1.4.0" +version = "1.0.17" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e2abad23fbc42b3700f2f279844dc832adb2b2eb069b2df918f455c4e18cc646" +checksum = "92ecc6618181def0457392ccd0ee51198e065e016d1d527a7ac1b6dc7c1f09d2" [[package]] name = "libc" -version = "0.2.148" +version = "0.2.183" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9cdc71e17332e86d2e1d38c1f99edcb6288ee11b815fb1a4b049eaa2114d369b" +checksum = "b5b646652bf6661599e1da8901b3b9522896f01e736bad5f723fe7a3a27f899d" [[package]] -name = "pre" -version = "0.2.1" +name = "memchr" +version = "2.8.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f87e7dc12cd3c36e90697e072bd70d886def4580fb268292022f89014dc9ea4b" -dependencies = [ - "cfg-if", - "pre-proc-macro", - "rustc_version", -] - -[[package]] -name = "pre-proc-macro" -version = "0.2.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a40339af35253b6b41a9e74f4c747e0948a9cb9cc9ea01d117d8440524560de3" -dependencies = [ - "cfg-if", - "lazy_static", - "proc-macro-crate", - "proc-macro-error", - "proc-macro2", - "quote", - "rustc_version", - "syn 1.0.109", -] - -[[package]] -name = "proc-macro-crate" -version = "0.1.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1d6ea3c4595b96363c13943497db34af4460fb474a95c43f4446ad341b8c9785" -dependencies = [ - "toml", -] - -[[package]] -name = "proc-macro-error" -version = "1.0.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "da25490ff9892aab3fcf7c36f08cfb902dd3e71ca0f9f9517bea02a73a5ce38c" -dependencies = [ - "proc-macro-error-attr", - "proc-macro2", - "quote", - "syn 1.0.109", - "version_check", -] - -[[package]] -name = "proc-macro-error-attr" -version = "1.0.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a1be40180e52ecc98ad80b184934baf3d0d29f979574e439af5a55274b35f869" -dependencies = [ - "proc-macro2", - "quote", - "version_check", -] +checksum = "f8ca58f447f06ed17d5fc4043ce1b10dd205e060fb3ce5b979b8ed8e59ff3f79" [[package]] name = "proc-macro2" -version = "1.0.66" +version = "1.0.106" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "18fb31db3f9bddb2ea821cde30a9f70117e3f119938b5ee630b7403aa6e2ead9" +checksum = "8fd00f0bb2e90d81d1044c2b32617f68fcb9fa3bb7640c23e9c748e53fb30934" dependencies = [ "unicode-ident", ] [[package]] name = "quote" -version = "1.0.33" +version = "1.0.45" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5267fca4496028628a95160fc423a33e8b2e6af8a5302579e322e4b520293cae" +checksum = "41f2619966050689382d2b44f664f4bc593e129785a36d6ee376ddf37259b924" dependencies = [ "proc-macro2", ] -[[package]] -name = "rustc_version" -version = "0.2.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "138e3e0acb6c9fb258b19b67cb8abd63c00679d2851805ea151465464fe9030a" -dependencies = [ - "semver", -] - -[[package]] -name = "ryu" -version = "1.0.15" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1ad4cc8da4ef723ed60bced201181d83791ad433213d8c24efffda1eec85d741" - [[package]] name = "secure-string" version = "0.3.0" dependencies = [ "libc", - "pre", "serde", "serde_cbor", "serde_json", + "subtle", "zeroize", ] -[[package]] -name = "semver" -version = "0.9.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1d7eb9ef2c18661902cc47e535f9bc51b78acd254da71d375c2f6720d9a40403" -dependencies = [ - "semver-parser", -] - -[[package]] -name = "semver-parser" -version = "0.7.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "388a1df253eca08550bef6c72392cfe7c30914bf41df5269b68cbd6ff8f570a3" - [[package]] name = "serde" -version = "1.0.188" +version = "1.0.228" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "cf9e0fcba69a370eed61bcf2b728575f726b50b55cba78064753d708ddc7549e" +checksum = "9a8e94ea7f378bd32cbbd37198a4a91436180c5bb472411e48b5ec2e2124ae9e" dependencies = [ - "serde_derive", + "serde_core", ] [[package]] @@ -171,73 +75,70 @@ dependencies = [ "serde", ] +[[package]] +name = "serde_core" +version = "1.0.228" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "41d385c7d4ca58e59fc732af25c3983b67ac852c1a25000afe1175de458b67ad" +dependencies = [ + "serde_derive", +] + [[package]] name = "serde_derive" -version = "1.0.188" +version = "1.0.228" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4eca7ac642d82aa35b60049a6eccb4be6be75e599bd2e9adb5f875a737654af2" +checksum = "d540f220d3187173da220f885ab66608367b6574e925011a9353e4badda91d79" dependencies = [ "proc-macro2", "quote", - "syn 2.0.29", + "syn", ] [[package]] name = "serde_json" -version = "1.0.105" +version = "1.0.149" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "693151e1ac27563d6dbcec9dee9fbd5da8539b20fa14ad3752b2e6d363ace360" +checksum = "83fc039473c5595ace860d8c4fafa220ff474b3fc6bfdb4293327f1a37e94d86" dependencies = [ "itoa", - "ryu", + "memchr", "serde", + "serde_core", + "zmij", ] [[package]] -name = "syn" -version = "1.0.109" +name = "subtle" +version = "2.6.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "72b64191b275b66ffe2469e8af2c1cfe3bafa67b529ead792a6d0160888b4237" -dependencies = [ - "proc-macro2", - "quote", - "unicode-ident", -] +checksum = "13c2bddecc57b384dee18652358fb23172facb8a2c51ccc10d74c157bdea3292" [[package]] name = "syn" -version = "2.0.29" +version = "2.0.117" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c324c494eba9d92503e6f1ef2e6df781e78f6a7705a0202d9801b198807d518a" +checksum = "e665b8803e7b1d2a727f4023456bbbbe74da67099c585258af0ad9c5013b9b99" dependencies = [ "proc-macro2", "quote", "unicode-ident", ] -[[package]] -name = "toml" -version = "0.5.11" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f4f7f0dd8d50a853a531c426359045b1998f04219d88799810762cd4ad314234" -dependencies = [ - "serde", -] - [[package]] name = "unicode-ident" -version = "1.0.11" +version = "1.0.24" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "301abaae475aa91687eb82514b328ab47a211a533026cb25fc3e519b86adfc3c" +checksum = "e6e4313cd5fcd3dad5cafa179702e2b244f760991f45397d14d4ebf38247da75" [[package]] -name = "version_check" -version = "0.9.4" +name = "zeroize" +version = "1.8.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "49874b5167b65d7193b8aba1567f5c7d93d001cafc34600cee003eda787e483f" +checksum = "b97154e67e32c85465826e8bcc1c59429aaaf107c1e4a9e53c8d8ccd5eff88d0" [[package]] -name = "zeroize" -version = "1.6.0" +name = "zmij" +version = "1.0.21" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2a0956f1ba7c7909bfb66c2e9e4124ab6f6482560f6628b5aaeba39207c9aad9" +checksum = "b8848ee67ecc8aedbaf3e4122217aff892639231befc6a1b58d29fff4c2cabaa" diff --git a/Cargo.toml b/Cargo.toml index ff07063..fe921b2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,11 +11,11 @@ documentation = "https://docs.rs/secure-string/" edition = "2021" [dependencies] -libc = "0.2.148" -zeroize = { version = "1.6.0", features = ["std"] } -serde = { version = "1.0.188", optional = true } +libc = "0.2" +zeroize = { version = "1", features = ["std"] } +serde = { version = "1", optional = true } +subtle = "2" [dev-dependencies] -pre = "0.2.1" serde_cbor = "0.11" -serde_json = "1.0.105" +serde_json = "1" diff --git a/src/lib.rs b/src/lib.rs index 3d1a3bd..dec9d01 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -6,7 +6,7 @@ mod secure_utils; #[cfg(feature = "serde")] mod serde; -pub use secure_types::{array::SecureArray, boxed::SecureBox, string::SecureString, vec::SecureBytes, vec::SecureVec}; +pub use secure_types::{array::SecureArray, string::SecureString, vec::SecureBytes, vec::SecureVec}; #[doc = include_str!("../README.md")] #[cfg(doctest)] diff --git a/src/secure_types/array.rs b/src/secure_types/array.rs index 99e18a2..6971bd9 100644 --- a/src/secure_types/array.rs +++ b/src/secure_types/array.rs @@ -4,6 +4,7 @@ use std::{ str::FromStr, }; +use subtle::ConstantTimeEq; use zeroize::Zeroize; use crate::secure_utils::memlock; @@ -15,32 +16,27 @@ use crate::secure_utils::memlock; /// - Outputting `***SECRET***` to prevent leaking secrets into logs in `fmt::Debug` and `fmt::Display` /// - Automatic `mlock` to protect against leaking into swap (any unix) /// - Automatic `madvise(MADV_NOCORE/MADV_DONTDUMP)` to protect against leaking into core dumps (FreeBSD, DragonflyBSD, Linux) -/// -/// Comparisons using the `PartialEq` implementation are undefined behavior (and most likely wrong) if `T` has any padding bytes. -#[derive(Eq, PartialEq, PartialOrd, Ord, Hash)] -pub struct SecureArray +#[derive(Eq, PartialOrd, Ord, Hash)] +pub struct SecureArray where - T: Copy + Zeroize, + u8: Copy + Zeroize, { - pub(crate) content: [T; LENGTH], + pub(crate) content: [u8; LENGTH], } -impl SecureArray -where - T: Copy + Zeroize, -{ - pub fn new(mut content: [T; LENGTH]) -> Self { +impl SecureArray { + pub fn new(mut content: [u8; LENGTH]) -> Self { memlock::mlock(content.as_mut_ptr(), content.len()); Self { content } } /// Borrow the contents of the string. - pub fn unsecure(&self) -> &[T] { + pub fn unsecure(&self) -> &[u8] { self.borrow() } /// Mutably borrow the contents of the string. - pub fn unsecure_mut(&mut self) -> &mut [T] { + pub fn unsecure_mut(&mut self) -> &mut [u8] { self.borrow_mut() } @@ -50,36 +46,42 @@ where } } -impl Clone for SecureArray { +impl PartialEq for SecureArray { + fn eq(&self, other: &SecureArray) -> bool { + self.content.as_slice().ct_eq(other.content.as_slice()).into() + } +} + +impl Clone for SecureArray { fn clone(&self) -> Self { Self::new(self.content) } } // Creation -impl From<[T; LENGTH]> for SecureArray +impl From<[u8; LENGTH]> for SecureArray where - T: Copy + Zeroize, + u8: Copy + Zeroize, { - fn from(s: [T; LENGTH]) -> Self { + fn from(s: [u8; LENGTH]) -> Self { Self::new(s) } } -impl TryFrom> for SecureArray +impl TryFrom> for SecureArray where - T: Copy + Zeroize, + u8: Copy + Zeroize, { type Error = String; - fn try_from(s: Vec) -> Result { - Ok(Self::new(s.try_into().map_err(|error: Vec| { + fn try_from(s: Vec) -> Result { + Ok(Self::new(s.try_into().map_err(|error: Vec| { format!("length mismatch: expected {LENGTH}, but got {}", error.len()) })?)) } } -impl FromStr for SecureArray { +impl FromStr for SecureArray { type Err = std::array::TryFromSliceError; fn from_str(s: &str) -> Result { @@ -88,12 +90,11 @@ impl FromStr for SecureArray { } // Array item indexing -impl std::ops::Index for SecureArray +impl std::ops::Index for SecureArray where - T: Copy + Zeroize, - [T; LENGTH]: std::ops::Index, + [u8; LENGTH]: std::ops::Index, { - type Output = <[T; LENGTH] as std::ops::Index>::Output; + type Output = <[u8; LENGTH] as std::ops::Index>::Output; fn index(&self, index: U) -> &Self::Output { std::ops::Index::index(&self.content, index) @@ -101,29 +102,20 @@ where } // Borrowing -impl Borrow<[T]> for SecureArray -where - T: Copy + Zeroize, -{ - fn borrow(&self) -> &[T] { +impl Borrow<[u8]> for SecureArray { + fn borrow(&self) -> &[u8] { self.content.borrow() } } -impl BorrowMut<[T]> for SecureArray -where - T: Copy + Zeroize, -{ - fn borrow_mut(&mut self) -> &mut [T] { +impl BorrowMut<[u8]> for SecureArray { + fn borrow_mut(&mut self) -> &mut [u8] { self.content.borrow_mut() } } // Overwrite memory with zeros when we're done -impl Drop for SecureArray -where - T: Copy + Zeroize, -{ +impl Drop for SecureArray { fn drop(&mut self) { self.zero_out(); memlock::munlock(self.content.as_mut_ptr(), self.content.len()); @@ -131,19 +123,13 @@ where } // Make sure sensitive information is not logged accidentally -impl fmt::Debug for SecureArray -where - T: Copy + Zeroize, -{ +impl fmt::Debug for SecureArray { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { f.write_str("***SECRET***").map_err(|_| fmt::Error) } } -impl fmt::Display for SecureArray -where - T: Copy + Zeroize, -{ +impl fmt::Display for SecureArray { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { f.write_str("***SECRET***").map_err(|_| fmt::Error) } @@ -157,49 +143,48 @@ mod tests { #[test] fn test_basic() { - let my_sec: SecureArray<_, 5> = SecureArray::from_str("hello").unwrap(); + let my_sec: SecureArray<5> = SecureArray::from_str("hello").unwrap(); assert_eq!(my_sec, SecureArray::from_str("hello").unwrap()); assert_eq!(my_sec.unsecure(), b"hello"); } #[test] - #[cfg_attr(feature = "pre", pre::pre)] fn test_zero_out() { - let mut my_sec: SecureArray<_, 5> = SecureArray::from_str("hello").unwrap(); + let mut my_sec: SecureArray<5> = SecureArray::from_str("hello").unwrap(); my_sec.zero_out(); assert_eq!(my_sec.unsecure(), b"\x00\x00\x00\x00\x00"); } #[test] fn test_comparison() { - assert_eq!(SecureArray::<_, 5>::from_str("hello").unwrap(), SecureArray::from_str("hello").unwrap()); - assert_ne!(SecureArray::<_, 5>::from_str("hello").unwrap(), SecureArray::from_str("olleh").unwrap()); + assert_eq!(SecureArray::<5>::from_str("hello").unwrap(), SecureArray::from_str("hello").unwrap()); + assert_ne!(SecureArray::<5>::from_str("hello").unwrap(), SecureArray::from_str("olleh").unwrap()); } #[test] fn test_indexing() { - let string: SecureArray<_, 5> = SecureArray::from_str("hello").unwrap(); + let string: SecureArray<5> = SecureArray::from_str("hello").unwrap(); assert_eq!(string[0], b'h'); assert_eq!(&string[3..5], "lo".as_bytes()); } #[test] fn test_show() { - assert_eq!(format!("{:?}", SecureArray::<_, 5>::from_str("hello").unwrap()), "***SECRET***".to_string()); - assert_eq!(format!("{}", SecureArray::<_, 5>::from_str("hello").unwrap()), "***SECRET***".to_string()); + assert_eq!(format!("{:?}", SecureArray::<5>::from_str("hello").unwrap()), "***SECRET***".to_string()); + assert_eq!(format!("{}", SecureArray::<5>::from_str("hello").unwrap()), "***SECRET***".to_string()); } #[test] - #[cfg_attr(feature = "pre", pre::pre)] fn test_comparison_zero_out_mb() { - let mbstring1 = SecureArray::from(['H', 'a', 'l', 'l', 'o', ' ', '🦄', '!']); - let mbstring2 = SecureArray::from(['H', 'a', 'l', 'l', 'o', ' ', '🦄', '!']); - let mbstring3 = SecureArray::from(['!', '🦄', ' ', 'o', 'l', 'l', 'a', 'H']); + // "Hallo 🦄!" is 11 UTF-8 bytes: 6 ASCII + 4 (🦄) + 1 (!) + let mbstring1 = SecureArray::<11>::from(*b"Hallo \xf0\x9f\xa4\x84!"); + let mbstring2 = SecureArray::<11>::from(*b"Hallo \xf0\x9f\xa4\x84!"); + let mbstring3 = SecureArray::<11>::from(*b"!\xf0\x9f\xa4\x84 ollaH"); assert!(mbstring1 == mbstring2); assert!(mbstring1 != mbstring3); let mut mbstring = mbstring1.clone(); mbstring.zero_out(); - assert_eq!(mbstring.unsecure(), &['\0', '\0', '\0', '\0', '\0', '\0', '\0', '\0']); + assert_eq!(mbstring.unsecure(), &[0u8; 11]); } } diff --git a/src/secure_types/boxed.rs b/src/secure_types/boxed.rs deleted file mode 100644 index 7177c29..0000000 --- a/src/secure_types/boxed.rs +++ /dev/null @@ -1,197 +0,0 @@ -use core::fmt; -use std::{ - borrow::{Borrow, BorrowMut}, - mem::MaybeUninit, -}; - -use zeroize::Zeroize; - -use crate::secure_utils::memlock; - -/// A data type suitable for storing sensitive information such as passwords and private keys in memory, that implements: -/// -/// - Automatic zeroing in `Drop` -/// - Constant time comparison in `PartialEq` (does not short circuit on the first different character; but terminates instantly if strings have different length) -/// - Outputting `***SECRET***` to prevent leaking secrets into logs in `fmt::Debug` and `fmt::Display` -/// - Automatic `mlock` to protect against leaking into swap (any unix) -/// - Automatic `madvise(MADV_NOCORE/MADV_DONTDUMP)` to protect against leaking into core dumps (FreeBSD, DragonflyBSD, Linux) -/// -/// Comparisons using the `PartialEq` implementation are undefined behavior (and most likely wrong) if `T` has any padding bytes. -#[derive(Eq, PartialEq, PartialOrd, Ord, Hash)] -pub struct SecureBox -where - T: Copy, -{ - // This is an `Option` to avoid UB in the destructor, outside the destructor, it is always - // `Some(_)` - content: Option>, -} - -impl SecureBox -where - T: Copy, -{ - pub fn new(mut cont: Box) -> Self { - memlock::mlock(&mut cont, 1); - SecureBox { content: Some(cont) } - } - - /// Borrow the contents of the string. - pub fn unsecure(&self) -> &T { - self.content.as_ref().unwrap() - } - - /// Mutably borrow the contents of the string. - pub fn unsecure_mut(&mut self) -> &mut T { - self.content.as_mut().unwrap() - } -} - -impl Clone for SecureBox { - fn clone(&self) -> Self { - Self::new(self.content.clone().unwrap()) - } -} - -// Delegate indexing -impl std::ops::Index for SecureBox -where - T: std::ops::Index + Copy, -{ - type Output = >::Output; - - fn index(&self, index: U) -> &Self::Output { - std::ops::Index::index(self.content.as_ref().unwrap().as_ref(), index) - } -} - -// Borrowing -impl Borrow for SecureBox -where - T: Copy, -{ - fn borrow(&self) -> &T { - self.content.as_ref().unwrap() - } -} -impl BorrowMut for SecureBox -where - T: Copy, -{ - fn borrow_mut(&mut self) -> &mut T { - self.content.as_mut().unwrap() - } -} - -// Overwrite memory with zeros when we're done -impl Drop for SecureBox -where - T: Copy, -{ - #[cfg_attr(feature = "pre", pre::pre)] - fn drop(&mut self) { - // Make sure that the box does not need to be dropped after this function, because it may - // see an invalid type, if `T` does not support an all-zero byte-pattern - // Instead we manually destruct the box and only handle the potentially invalid values - // behind the pointer - let ptr = Box::into_raw(self.content.take().unwrap()); - - // There is no need to worry about dropping the contents, because `T: Copy` and `Copy` - // types cannot implement `Drop` - - unsafe { - std::slice::from_raw_parts_mut::>(ptr as *mut MaybeUninit, std::mem::size_of::()).zeroize(); - } - - memlock::munlock(ptr, 1); - - // Deallocate only non-zero-sized types, because otherwise it's UB - if std::mem::size_of::() != 0 { - // Safety: - // This way to manually deallocate is advertised in the documentation of `Box::into_raw`. - // The box was allocated with the global allocator and a layout of `T` and is thus - // deallocated using the same allocator and layout here. - unsafe { std::alloc::dealloc(ptr as *mut u8, std::alloc::Layout::new::()) }; - } - } -} - -// Make sure sensitive information is not logged accidentally -impl fmt::Debug for SecureBox -where - T: Copy, -{ - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - f.write_str("***SECRET***").map_err(|_| fmt::Error) - } -} - -impl fmt::Display for SecureBox -where - T: Copy, -{ - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - f.write_str("***SECRET***").map_err(|_| fmt::Error) - } -} - -#[cfg(test)] -mod tests { - use std::mem::MaybeUninit; - - use zeroize::Zeroize; - - use super::SecureBox; - - const PRIVATE_KEY_1: [u8; 32] = [ - 0xb0, 0x3b, 0x34, 0xc3, 0x3a, 0x1c, 0x44, 0xf2, 0x25, 0xb6, 0x62, 0xd2, 0xbf, 0x48, 0x59, 0xb8, 0x13, 0x54, 0x11, 0xfa, - 0x7b, 0x03, 0x86, 0xd4, 0x5f, 0xb7, 0x5d, 0xc5, 0xb9, 0x1b, 0x44, 0x66, - ]; - - const PRIVATE_KEY_2: [u8; 32] = [ - 0xc8, 0x06, 0x43, 0x9d, 0xc9, 0xd2, 0xc4, 0x76, 0xff, 0xed, 0x8f, 0x25, 0x80, 0xc0, 0x88, 0x8d, 0x58, 0xab, 0x40, 0x6b, - 0xf7, 0xae, 0x36, 0x98, 0x87, 0x90, 0x21, 0xb9, 0x6b, 0xb4, 0xbf, 0x59, - ]; - - /// Overwrite the contents with zeros. This is automatically done in the destructor. - /// - /// # Safety - /// An all-zero byte-pattern must be a valid value of `T` in order for this function call to not be - /// undefined behavior. - #[cfg_attr(feature = "pre", pre::pre("an all-zero byte-pattern is a valid value of `T`"))] - pub(crate) unsafe fn zero_out_secure_box(secure_box: &mut SecureBox) - where - T: Copy, - { - std::slice::from_raw_parts_mut::>( - &mut **secure_box.content.as_mut().unwrap() as *mut T as *mut MaybeUninit, - std::mem::size_of::(), - ) - .zeroize(); - } - - #[test] - #[cfg_attr(feature = "pre", pre::pre)] - fn test_secure_box() { - let key_1 = SecureBox::new(Box::new(PRIVATE_KEY_1)); - let key_2 = SecureBox::new(Box::new(PRIVATE_KEY_2)); - let key_3 = SecureBox::new(Box::new(PRIVATE_KEY_1)); - assert!(key_1 == key_1); - assert!(key_1 != key_2); - assert!(key_2 != key_3); - assert!(key_1 == key_3); - - let mut final_key = key_1.clone(); - #[cfg_attr( - feature = "pre", - assure( - "an all-zero byte-pattern is a valid value of `T`", - reason = "`T` is `i32`, for which an all-zero byte-pattern is valid" - ) - )] - unsafe { - zero_out_secure_box(&mut final_key) - }; - assert_eq!(final_key.unsecure(), &[0; 32]); - } -} diff --git a/src/secure_types/mod.rs b/src/secure_types/mod.rs index 8f18d01..1570d25 100644 --- a/src/secure_types/mod.rs +++ b/src/secure_types/mod.rs @@ -1,4 +1,3 @@ pub mod array; -pub mod boxed; pub mod string; pub mod vec; diff --git a/src/secure_types/string.rs b/src/secure_types/string.rs index 81a9e0e..dc6f6df 100644 --- a/src/secure_types/string.rs +++ b/src/secure_types/string.rs @@ -5,64 +5,35 @@ use crate::{secure_utils::memlock, SecureVec}; /// Wrapper for a vector that stores a valid UTF-8 string #[derive(Clone, Eq)] -pub struct SecureString(SecureVec); +pub struct SecureString(SecureVec); impl SecureString { /// Borrow the contents of the string. - #[cfg_attr(feature = "pre", pre::pre)] pub fn unsecure(&self) -> &str { - #[cfg_attr( - feature = "pre", - forward(pre), - assure( - "the content of `v` is valid UTF-8", - reason = "it is not possible to create a `SecureString` with invalid UTF-8 content - and it is also not possible to modify the content as non-UTF-8 directly, so - they must still be valid UTF-8 here" - ) - )] - unsafe { - std::str::from_utf8_unchecked(self.0.unsecure()) - } + // SAFETY: It is not possible to create a `SecureString` with invalid UTF-8 content, + // and it is also not possible to modify the content as non-UTF-8 directly, so + // the content must still be valid UTF-8 here. + unsafe { std::str::from_utf8_unchecked(self.0.unsecure()) } } /// Mutably borrow the contents of the string. - #[cfg_attr(feature = "pre", pre::pre)] pub fn unsecure_mut(&mut self) -> &mut str { - #[cfg_attr( - feature = "pre", - forward(pre), - assure( - "the content of `v` is valid UTF-8", - reason = "it is not possible to create a `SecureString` with invalid UTF-8 content - and it is also not possible to modify the content as non-UTF-8 directly, so - they must still be valid UTF-8 here" - ) - )] - unsafe { - std::str::from_utf8_unchecked_mut(self.0.unsecure_mut()) - } + // TODO: fix + // SAFETY: It is not possible to create a `SecureString` with invalid UTF-8 content, + // and it is also not possible to modify the content as non-UTF-8 directly, so + // the content must still be valid UTF-8 here. + unsafe { std::str::from_utf8_unchecked_mut(self.0.unsecure_mut()) } } /// Turn the string into a regular `String` again. - #[cfg_attr(feature = "pre", pre::pre)] pub fn into_unsecure(mut self) -> String { memlock::munlock(self.0.content.as_mut_ptr(), self.0.content.capacity()); let content = std::mem::take(&mut self.0.content); std::mem::forget(self); - #[cfg_attr( - feature = "pre", - forward(impl pre::std::string::String), - assure( - "the content of `bytes` is valid UTF-8", - reason = "it is not possible to create a `SecureString` with invalid UTF-8 content - and it is also not possible to modify the content as non-UTF-8 directly, so - they must still be valid UTF-8 here" - ) - )] - unsafe { - String::from_utf8_unchecked(content) - } + // SAFETY: It is not possible to create a `SecureString` with invalid UTF-8 content, + // and it is also not possible to modify the content as non-UTF-8 directly, so + // the content must still be valid UTF-8 here. + unsafe { String::from_utf8_unchecked(content) } } /// Overwrite the string with zeros. This is automatically called in the destructor. diff --git a/src/secure_types/vec.rs b/src/secure_types/vec.rs index 8598025..615d8a3 100644 --- a/src/secure_types/vec.rs +++ b/src/secure_types/vec.rs @@ -4,6 +4,7 @@ use std::{ str::FromStr, }; +use subtle::ConstantTimeEq; use zeroize::Zeroize; use crate::secure_utils::memlock; @@ -16,37 +17,29 @@ use crate::secure_utils::memlock; /// - Automatic `mlock` to protect against leaking into swap (any unix) /// - Automatic `madvise(MADV_NOCORE/MADV_DONTDUMP)` to protect against leaking into core dumps (FreeBSD, DragonflyBSD, Linux) /// -/// Comparisons using the `PartialEq` implementation are undefined behavior (and most likely wrong) if `T` has any padding bytes. -/// -/// Be careful with `SecureBytes::from`: if you have a borrowed string, it will be copied. -/// Use `SecureBytes::new` if you have a `Vec`. -#[derive(PartialEq, Eq, PartialOrd, Ord, Hash)] -pub struct SecureVec -where - T: Copy + Zeroize, -{ - pub(crate) content: Vec, +/// Be careful with `SecureVec::from`: if you have a borrowed string, it will be copied. +/// Use `SecureVec::new` if you have a `Vec`. +#[derive(Eq, PartialOrd, Ord, Hash)] +pub struct SecureVec { + pub(crate) content: Vec, } /// Type alias for a vector that stores just bytes -pub type SecureBytes = SecureVec; +pub type SecureBytes = SecureVec; -impl SecureVec -where - T: Copy + Zeroize, -{ - pub fn new(mut cont: Vec) -> Self { +impl SecureVec { + pub fn new(mut cont: Vec) -> Self { memlock::mlock(cont.as_mut_ptr(), cont.capacity()); SecureVec { content: cont } } /// Borrow the contents of the string. - pub fn unsecure(&self) -> &[T] { + pub fn unsecure(&self) -> &[u8] { self.borrow() } /// Mutably borrow the contents of the string. - pub fn unsecure_mut(&mut self) -> &mut [T] { + pub fn unsecure_mut(&mut self) -> &mut [u8] { self.borrow_mut() } @@ -58,8 +51,8 @@ where /// This ensures that the new memory region is secured if reallocation occurs. /// /// Similar to [`Vec::resize`](https://doc.rust-lang.org/std/vec/struct.Vec.html#method.resize) - pub fn resize(&mut self, new_len: usize, value: T) { - // Trucnate if shorter or same length + pub fn resize(&mut self, new_len: usize, value: u8) { + // Truncate if shorter or same length if new_len <= self.content.len() { self.content.truncate(new_len); return; @@ -84,24 +77,29 @@ where } } -impl Clone for SecureVec { +impl PartialEq for SecureVec { + fn eq(&self, other: &SecureVec) -> bool { + self.content.as_slice().ct_eq(other.content.as_slice()).into() + } +} + +impl Clone for SecureVec { fn clone(&self) -> Self { Self::new(self.content.clone()) } } // Creation -impl From for SecureVec +impl From for SecureVec where - U: Into>, - T: Copy + Zeroize, + U: Into>, { - fn from(s: U) -> SecureVec { + fn from(s: U) -> SecureVec { SecureVec::new(s.into()) } } -impl FromStr for SecureVec { +impl FromStr for SecureVec { type Err = std::convert::Infallible; fn from_str(s: &str) -> Result { @@ -110,12 +108,11 @@ impl FromStr for SecureVec { } // Vec item indexing -impl std::ops::Index for SecureVec +impl std::ops::Index for SecureVec where - T: Copy + Zeroize, - Vec: std::ops::Index, + Vec: std::ops::Index, { - type Output = as std::ops::Index>::Output; + type Output = as std::ops::Index>::Output; fn index(&self, index: U) -> &Self::Output { std::ops::Index::index(&self.content, index) @@ -123,29 +120,20 @@ where } // Borrowing -impl Borrow<[T]> for SecureVec -where - T: Copy + Zeroize, -{ - fn borrow(&self) -> &[T] { +impl Borrow<[u8]> for SecureVec { + fn borrow(&self) -> &[u8] { self.content.borrow() } } -impl BorrowMut<[T]> for SecureVec -where - T: Copy + Zeroize, -{ - fn borrow_mut(&mut self) -> &mut [T] { +impl BorrowMut<[u8]> for SecureVec { + fn borrow_mut(&mut self) -> &mut [u8] { self.content.borrow_mut() } } // Overwrite memory with zeros when we're done -impl Drop for SecureVec -where - T: Copy + Zeroize, -{ +impl Drop for SecureVec { fn drop(&mut self) { self.zero_out(); memlock::munlock(self.content.as_mut_ptr(), self.content.capacity()); @@ -153,19 +141,13 @@ where } // Make sure sensitive information is not logged accidentally -impl fmt::Debug for SecureVec -where - T: Copy + Zeroize, -{ +impl fmt::Debug for SecureVec { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { f.write_str("***SECRET***").map_err(|_| fmt::Error) } } -impl fmt::Display for SecureVec -where - T: Copy + Zeroize, -{ +impl fmt::Display for SecureVec { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { f.write_str("***SECRET***").map_err(|_| fmt::Error) } @@ -183,33 +165,17 @@ mod tests { } #[test] - #[cfg_attr(feature = "pre", pre::pre)] fn test_zero_out() { let mut my_sec = SecureBytes::from("hello"); my_sec.zero_out(); // `zero_out` sets the `len` to 0, here we reset it to check that the bytes were zeroed - #[cfg_attr( - feature = "pre", - forward(impl pre::std::vec::Vec), - assure( - new_len <= self.capacity(), - reason = "the call to `zero_out` did not reduce the capacity and the length was `5` before, - so the capacity must be greater or equal to `5`" - ), - assure( - "the elements at `old_len..new_len` are initialized", - reason = "they were initialized to `0` by the call to `zero_out`" - ) - )] - unsafe { - my_sec.content.set_len(5) - } + unsafe { my_sec.content.set_len(5) } assert_eq!(my_sec.unsecure(), b"\x00\x00\x00\x00\x00"); } #[test] fn test_resize() { - let mut my_sec = SecureVec::from([0, 1]); + let mut my_sec = SecureVec::from([0u8, 1u8]); assert_eq!(my_sec.unsecure().len(), 2); my_sec.resize(1, 0); assert_eq!(my_sec.unsecure().len(), 1); @@ -240,33 +206,18 @@ mod tests { } #[test] - #[cfg_attr(feature = "pre", pre::pre)] fn test_comparison_zero_out_mb() { - let mbstring1 = SecureVec::from(vec!['H', 'a', 'l', 'l', 'o', ' ', '🦄', '!']); - let mbstring2 = SecureVec::from(vec!['H', 'a', 'l', 'l', 'o', ' ', '🦄', '!']); - let mbstring3 = SecureVec::from(vec!['!', '🦄', ' ', 'o', 'l', 'l', 'a', 'H']); + let mbstring1 = SecureVec::from("Hallo 🦄!"); + let mbstring2 = SecureVec::from("Hallo 🦄!"); + let mbstring3 = SecureVec::from("!🦄 ollaH"); assert!(mbstring1 == mbstring2); assert!(mbstring1 != mbstring3); + let len = mbstring1.unsecure().len(); let mut mbstring = mbstring1.clone(); mbstring.zero_out(); // `zero_out` sets the `len` to 0, here we reset it to check that the bytes were zeroed - #[cfg_attr( - feature = "pre", - forward(impl pre::std::vec::Vec), - assure( - new_len <= self.capacity(), - reason = "the call to `zero_out` did not reduce the capacity and the length was `8` before, - so the capacity must be greater or equal to `8`" - ), - assure( - "the elements at `old_len..new_len` are initialized", - reason = "they were initialized to `0` by the call to `zero_out`" - ) - )] - unsafe { - mbstring.content.set_len(8) - } - assert_eq!(mbstring.unsecure(), &['\0', '\0', '\0', '\0', '\0', '\0', '\0', '\0']); + unsafe { mbstring.content.set_len(len) } + assert_eq!(mbstring.unsecure(), vec![0u8; len]); } } diff --git a/src/serde.rs b/src/serde.rs index 15a3355..fcc9b9b 100644 --- a/src/serde.rs +++ b/src/serde.rs @@ -59,8 +59,8 @@ where } } -impl<'de> Deserialize<'de> for SecureVec { - fn deserialize(deserializer: D) -> Result, D::Error> +impl<'de> Deserialize<'de> for SecureVec { + fn deserialize(deserializer: D) -> Result where D: Deserializer<'de>, { @@ -68,7 +68,7 @@ impl<'de> Deserialize<'de> for SecureVec { } } -impl Serialize for SecureVec { +impl Serialize for SecureVec { fn serialize(&self, serializer: S) -> Result where S: Serializer, @@ -77,7 +77,7 @@ impl Serialize for SecureVec { } } -impl<'de, const LENGTH: usize> Deserialize<'de> for SecureArray { +impl<'de, const LENGTH: usize> Deserialize<'de> for SecureArray { fn deserialize(deserializer: D) -> Result where D: Deserializer<'de>, @@ -86,7 +86,7 @@ impl<'de, const LENGTH: usize> Deserialize<'de> for SecureArray { } } -impl Serialize for SecureArray { +impl Serialize for SecureArray { fn serialize(&self, serializer: S) -> Result where S: Serializer, @@ -112,7 +112,7 @@ mod tests { #[test] fn test_cbor_array() { - let data: SecureArray<_, 5> = SecureArray::from_str("hello").unwrap(); + let data: SecureArray<5> = SecureArray::from_str("hello").unwrap(); let cbor = serde_cbor::to_vec(&data).unwrap(); assert_eq!(cbor, b"\x45hello"); let deserialised = serde_cbor::from_slice(&cbor).unwrap(); @@ -126,7 +126,7 @@ mod tests { let json = serde_json::to_string_pretty(secure_bytes.unsecure()).unwrap(); println!("json = {json}"); - let secure_bytes_serde: SecureVec = serde_json::from_str(&json).unwrap(); + let secure_bytes_serde: SecureVec = serde_json::from_str(&json).unwrap(); assert_eq!(secure_bytes, secure_bytes_serde); }