Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PM-8275] Implement a zeroizing allocator #788

Merged
merged 21 commits into from
May 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
104 changes: 104 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion crates/bitwarden-c/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ bench = false

[target.'cfg(not(target_arch="wasm32"))'.dependencies]
tokio = { version = ">=1.28.2, <2.0", features = ["rt-multi-thread", "macros"] }

bitwarden-json = { path = "../bitwarden-json", features = ["secrets"] }

[dependencies]
Expand Down
16 changes: 14 additions & 2 deletions crates/bitwarden-crypto/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,9 @@ keywords.workspace = true
[features]
default = []

mobile = ["dep:uniffi"] # Mobile-specific features
test = [] # Test methods
mobile = ["dep:uniffi"] # Mobile-specific features
no-memory-hardening = [] # Disable memory hardening features
test = [] # Test methods

[dependencies]
aes = { version = ">=0.8.2, <0.9", features = ["zeroize"] }
Expand Down Expand Up @@ -47,8 +48,19 @@ uuid = { version = ">=1.3.3, <2.0", features = ["serde"] }
zeroize = { version = ">=1.7.0, <2.0", features = ["derive", "aarch64"] }

[dev-dependencies]
criterion = "0.5.1"
rand_chacha = "0.3.1"
serde_json = ">=1.0.96, <2.0"

[[bench]]
name = "default_allocator"
harness = false
required-features = ["no-memory-hardening"]

[[bench]]
name = "zeroizing_allocator"
harness = false
required-features = ["no-memory-hardening"]

[lints]
workspace = true
14 changes: 14 additions & 0 deletions crates/bitwarden-crypto/benches/default_allocator.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
use criterion::{black_box, criterion_group, criterion_main, Criterion};

fn allocate_string(s: &str) -> String {
s.to_owned()
}

pub fn criterion_benchmark(c: &mut Criterion) {
c.bench_function("string abc", |b| {
b.iter(|| allocate_string(black_box("abc")))
});
}

criterion_group!(benches, criterion_benchmark);
criterion_main!(benches);
11 changes: 11 additions & 0 deletions crates/bitwarden-crypto/benches/zeroizing_allocator.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
use bitwarden_crypto::ZeroizingAllocator;
use criterion::{criterion_group, criterion_main};
use default_allocator::criterion_benchmark;

#[global_allocator]
static ALLOC: ZeroizingAllocator<std::alloc::System> = ZeroizingAllocator(std::alloc::System);

mod default_allocator;

criterion_group!(benches, criterion_benchmark);
criterion_main!(benches);
167 changes: 167 additions & 0 deletions crates/bitwarden-crypto/src/allocator.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,167 @@
use core::slice;
use std::alloc::{GlobalAlloc, Layout};

use zeroize::Zeroize;

/// Custom allocator that zeroizes memory before deallocating it
///
/// This is highly recommended to be enabled when using the Bitwarden crates to avoid sensitive data
/// persisting in memory after it has been deallocated.
///
/// This allocator is a decorator around another allocator.
///
/// # Example
///
/// This example shows how to use the `ZeroizingAllocator` with the system allocator.
///
/// ```rust,ignore
/// #[global_allocator]
/// static ALLOC: bitwarden_crypto::ZeroizingAllocator<std::alloc::System> =
/// bitwarden_crypto::ZeroizingAllocator(std::alloc::System);
/// ```
pub struct ZeroizingAllocator<Alloc: GlobalAlloc>(pub Alloc);

unsafe impl<T: GlobalAlloc> GlobalAlloc for ZeroizingAllocator<T> {
unsafe fn alloc(&self, layout: Layout) -> *mut u8 {
self.0.alloc(layout)
}

unsafe fn dealloc(&self, ptr: *mut u8, layout: Layout) {
slice::from_raw_parts_mut(ptr, layout.size()).zeroize();

self.0.dealloc(ptr, layout);
}

unsafe fn alloc_zeroed(&self, layout: Layout) -> *mut u8 {
self.0.alloc_zeroed(layout)
}
}

#[cfg(test)]
mod tests {
Hinton marked this conversation as resolved.
Show resolved Hide resolved
#[test]
#[ignore = "It produces inconsistent results on some platforms"]
fn string() {
let s = String::from("hello");

let p1 = s.as_str().as_ptr();
let c1 = s.capacity();

assert_eq!(
unsafe { std::slice::from_raw_parts(p1, c1) },
b"hello",
"String is not at the expected memory location"
);

drop(s);

assert_eq!(
unsafe { std::slice::from_raw_parts(p1, c1) },
[0, 0, 0, 0, 0],
"memory was not zeroized after dropping the string"
);
}

#[test]
#[ignore = "It produces inconsistent results on some platforms"]
fn string_expand() {
let mut s = String::from("hello");

let p1 = s.as_str().as_ptr();
let c1 = s.capacity();

assert_eq!(unsafe { std::slice::from_raw_parts(p1, c1) }, b"hello");

s.push_str(" world");

let p2 = s.as_str().as_ptr();
let c2 = s.capacity();

// We allocated a new string
assert_ne!(p1, p2);
assert_eq!(
unsafe { std::slice::from_raw_parts(p1, c1) },
[0, 0, 0, 0, 0],
"old string was not zeroized"
);

assert_eq!(
unsafe { std::slice::from_raw_parts(p2, c2) },
b"hello world"
);

// Drop the expanded string
drop(s);

assert_eq!(
unsafe { std::slice::from_raw_parts(p2, c2) },
[0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
"expanded string was not zeroized"
);
}

#[test]
#[ignore = "It produces inconsistent results on some platforms"]
fn vec() {
let v = vec![1, 2, 3, 4, 5];

let p1 = v.as_slice().as_ptr();
let c1 = v.capacity();

assert_eq!(
unsafe { std::slice::from_raw_parts(p1, c1) },
[1, 2, 3, 4, 5],
"vec is not at the expected memory location"
);

drop(v);

assert_eq!(
unsafe { std::slice::from_raw_parts(p1, c1) },
[0, 0, 0, 0, 0],
"vec was not zeroized after dropping"
);
}

#[test]
#[ignore = "It produces inconsistent results on some platforms"]
fn vec_expand() {
let mut v = vec![1, 2, 3, 4, 5];

let p1 = v.as_slice().as_ptr();
let c1 = v.capacity();

assert_eq!(
unsafe { std::slice::from_raw_parts(p1, c1) },
[1, 2, 3, 4, 5],
"vec is not at the expected memory location"
);

v.extend_from_slice(&[6, 7, 8, 9, 10]);

let p2 = v.as_slice().as_ptr();
let c2 = v.capacity();

// We allocated a new vector
assert_ne!(p1, p2);
assert_eq!(
unsafe { std::slice::from_raw_parts(p1, c1) },
[0, 0, 0, 0, 0],
"old vec was not zeroized"
);

assert_eq!(
unsafe { std::slice::from_raw_parts(p2, c2) },
[1, 2, 3, 4, 5, 6, 7, 8, 9, 10]
);

// Drop the expanded vector
drop(v);

assert_eq!(
unsafe { std::slice::from_raw_parts(p2, c2) },
[0, 0, 0, 0, 0, 0, 0, 0, 0, 0],
"expanded vec was not zeroized"
);
}
}
Loading
Loading