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.

Sign up for GitHub

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

[WIP] Ps/sensitive test 2 #768

Closed
wants to merge 4 commits into from
Closed

[WIP] Ps/sensitive test 2 #768

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
913fd42
Improve memory testing
dani-garcia Apr 30, 2024
167e8ef
Missed some debug options
dani-garcia May 3, 2024
b335273
We only need to check the first dump for the test string
dani-garcia May 3, 2024
18be3b8
Hide Sensitive::test behind flag
Hinton May 3, 2024
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
1 change: 1 addition & 0 deletions Cargo.lock
View file
Open in desktop

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

1 change: 1 addition & 0 deletions crates/bitwarden-crypto/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ keywords.workspace = true
default = []

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

[dependencies]
aes = { version = ">=0.8.2, <0.9", features = ["zeroize"] }
Expand Down
4 changes: 1 addition & 3 deletions crates/bitwarden-crypto/src/sensitive/sensitive.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,9 +228,7 @@ impl<V: Zeroize + JsonSchema> JsonSchema for Sensitive<V> {
// We use a lot of `&str` and `&[u8]` in our tests, so we expose this helper
// to make it easier.
// IMPORTANT: This should not be used outside of test code
// Note that we can't just mark it with #[cfg(test)] because that only applies
// when testing this crate, not when testing other crates that depend on it.
// By at least limiting it to &'static reference we should be able to avoid accidental usages
#[cfg(any(test, feature = "test"))]
impl<V: Zeroize> Sensitive<V> {
pub fn test<T: ?Sized>(value: &'static T) -> Self
where
Expand Down
3 changes: 3 additions & 0 deletions crates/bitwarden-exporters/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,5 +29,8 @@ serde_json = ">=1.0.96, <2.0"
thiserror = ">=1.0.40, <2.0"
uuid = { version = ">=1.3.3, <2.0", features = ["serde", "v4"] }

[dev-dependencies]
bitwarden-crypto = { workspace = true, features = ["test"] }

[lints]
workspace = true
1 change: 1 addition & 0 deletions crates/bitwarden/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ reqwest = { version = ">=0.12, <0.13", features = [
], default-features = false }

[dev-dependencies]
bitwarden-crypto = { workspace = true, features = ["test"] }
rand_chacha = "0.3.1"
tokio = { version = "1.36.0", features = ["rt", "macros"] }
wiremock = "0.6.0"
Expand Down
2 changes: 1 addition & 1 deletion crates/memory-testing/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,4 +17,4 @@ comfy-table = "7.1.0"
hex = "0.4.3"
serde = "1.0.196"
serde_json = "1.0.113"
zeroize = "1.7.0"
zeroize = { version = "1.7.0", features = ["serde"] }
6 changes: 3 additions & 3 deletions crates/memory-testing/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ RUN mkdir -p /app/crates/bitwarden-crypto/src \
&& mkdir -p /app/crates/memory-testing/src \
&& touch /app/crates/bitwarden-crypto/src/lib.rs \
&& echo 'fn main(){}' > /app/crates/memory-testing/src/main.rs \
&& cargo build -p memory-testing
&& cargo build -p memory-testing --release

# Delete dummy files and copy the actual source code
RUN rm /app/crates/bitwarden-crypto/src/lib.rs /app/crates/memory-testing/src/main.rs
Expand All @@ -23,7 +23,7 @@ COPY crates/memory-testing/src /app/crates/memory-testing/src

# Build the project. We use touch to force a rebuild of the now real files
RUN touch /app/crates/bitwarden-crypto/src/lib.rs /app/crates/memory-testing/src/main.rs
RUN cargo build -p memory-testing
RUN cargo build -p memory-testing --release

###############################################
# App stage #
Expand All @@ -36,7 +36,7 @@ USER root
RUN apt-get update && apt-get install -y --no-install-recommends gdb=13.1-3 && apt-get clean && rm -rf /var/lib/apt/lists/*

# Copy built project from the build stage and the cases.json file
COPY --from=build /app/target/debug/memory-testing /app/target/debug/capture-dumps ./
COPY --from=build /app/target/release/memory-testing /app/target/release/capture-dumps ./
COPY crates/memory-testing/cases.json .

CMD [ "/capture-dumps", "./memory-testing", "/output" ]
91 changes: 64 additions & 27 deletions crates/memory-testing/cases.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,41 +1,78 @@
{
"symmetric_key": [
"cases": [
{
"key": "FfhVVP8fmFIZY1WmRszPmRmVCxXNWVcJffPrbkywTPtBNkgfhYGT+D9sVGizYXrPffuj2yoyWqMwF9iF5aMQhQ==",
"decrypted_key_hex": "15f85554ff1f9852196355a646cccf9919950b15cd5957097df3eb6e4cb04cfb",
"decrypted_mac_hex": "4136481f858193f83f6c5468b3617acf7dfba3db2a325aa33017d885e5a31085"
}
],
"master_key": [
"name": "Symmetric Key",
"symmetric_key": {
"key": "FfhVVP8fmFIZY1WmRszPmRmVCxXNWVcJffPrbkywTPtBNkgfhYGT+D9sVGizYXrPffuj2yoyWqMwF9iF5aMQhQ=="
},
"memory_lookups": [
{
"name": "Decrypted key",
"hex": "15f85554ff1f9852196355a646cccf9919950b15cd5957097df3eb6e4cb04cfb"
},
{
"name": "Decrypted MAC",
"hex": "4136481f858193f83f6c5468b3617acf7dfba3db2a325aa33017d885e5a31085"
}
]
},

{
"password": "123412341234",
"email": "[email protected]",
"name": "Master Key PBKDF2",
"master_key": {
"password": "123412341234",
"email": "[email protected]",

"kdf": {
"pBKDF2": {
"iterations": 100000
"kdf": {
"pBKDF2": {
"iterations": 100000
}
}
},

"key_hex": "2d55ac8e33bd14ee9eee26fa651163a41049a37b3b20c914a8b6abc9a38a89d5",
"hash": "gQ9O5ZhtQ23dL5r93e4BL04ATYOJVEvBAOwYsDDEJFA=",
"hash_hex": "810f4ee5986d436ddd2f9afdddee012f4e004d8389544bc100ec18b030c42450"
"memory_lookups": [
{
"name": "Key",
"hex": "2d55ac8e33bd14ee9eee26fa651163a41049a37b3b20c914a8b6abc9a38a89d5"
},
{
"name": "Hash B64",
"string": "gQ9O5ZhtQ23dL5r93e4BL04ATYOJVEvBAOwYsDDEJFA="
},
{
"name": "Hash bytes",
"hex": "810f4ee5986d436ddd2f9afdddee012f4e004d8389544bc100ec18b030c42450"
}
]
},
{
"password": "asdfasdfasdf",
"email": "[email protected]",
"name": "Master Key Argon2",
"master_key": {
"password": "asdfasdfasdf",
"email": "[email protected]",

"kdf": {
"argon2id": {
"iterations": 3,
"memory": 4,
"parallelism": 1
"kdf": {
"argon2id": {
"iterations": 3,
"memory": 4,
"parallelism": 1
}
}
},

"key_hex": "3bc0520a0abff0097d521ce0ee5e5b1cee301939a84742623c0c1697d7a4bd46",
"hash": "lHkprdORlICVJ4Umwi94Uz/nATK6Y7If7e+iFoabzh0=",
"hash_hex": "947929add391948095278526c22f78533fe70132ba63b21fedefa216869bce1d"
"memory_lookups": [
{
"name": "Key",
"hex": "3bc0520a0abff0097d521ce0ee5e5b1cee301939a84742623c0c1697d7a4bd46",
"allowed_count": 3
},
{
"name": "Hash B64",
"string": "lHkprdORlICVJ4Umwi94Uz/nATK6Y7If7e+iFoabzh0="
},
{
"name": "Hash bytes",
"hex": "947929add391948095278526c22f78533fe70132ba63b21fedefa216869bce1d"
}
]
}
]
}
6 changes: 3 additions & 3 deletions crates/memory-testing/run_test.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,14 @@ BASE_DIR="./crates/memory-testing"
mkdir -p $BASE_DIR/output
rm $BASE_DIR/output/* || true

cargo build -p memory-testing
cargo build -p memory-testing --release

if [ "$1" = "no-docker" ]; then
# This specifically needs to run as root to be able to capture core dumps
sudo ./target/debug/capture-dumps ./target/debug/memory-testing $BASE_DIR
sudo ./target/release/capture-dumps ./target/release/memory-testing $BASE_DIR
else
docker build -f crates/memory-testing/Dockerfile -t bitwarden/memory-testing .
docker run --rm -it -v $BASE_DIR:/output bitwarden/memory-testing
fi

./target/debug/analyze-dumps $BASE_DIR
./target/release/analyze-dumps $BASE_DIR
149 changes: 25 additions & 124 deletions crates/memory-testing/src/bin/analyze-dumps.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use std::{env, fmt::Display, io, path::Path, process};
use std::{env, io, path::Path, process};

use bitwarden_crypto::Kdf;
use memory_testing::*;

fn find_subarrays(needle: &[u8], haystack: &[u8]) -> Vec<usize> {
Expand Down Expand Up @@ -31,26 +30,6 @@ fn comma_sep(nums: &[usize]) -> String {
.join(", ")
}

fn add_row<N: Display>(
table: &mut comfy_table::Table,
name: N,
initial_pos: &[usize],
final_pos: &[usize],
ok_cond: bool,
) -> bool {
table.add_row(vec![
name.to_string(),
comma_sep(initial_pos),
comma_sep(final_pos),
if ok_cond {
OK.to_string()
} else {
FAIL.to_string()
},
]);
!ok_cond
}

fn main() -> io::Result<()> {
let args: Vec<String> = env::args().collect();
if args.len() < 2 {
Expand All @@ -72,114 +51,36 @@ fn main() -> io::Result<()> {

let test_string: Vec<u8> = TEST_STRING.as_bytes().to_vec();
let test_initial_pos = find_subarrays(&test_string, &initial_core);
let test_final_pos = find_subarrays(&test_string, &final_core);

error |= add_row(
&mut table,
"Test String",
&test_initial_pos,
&test_final_pos,
!test_final_pos.is_empty(),
);

if test_initial_pos.is_empty() {
println!("ERROR: Test string not found in initial core dump, is the dump valid?");
error = true;
}

for (idx, case) in cases.symmetric_key.iter().enumerate() {
let key_part: Vec<u8> = hex::decode(&case.decrypted_key_hex).unwrap();
let mac_part: Vec<u8> = hex::decode(&case.decrypted_mac_hex).unwrap();
let key_in_b64: Vec<u8> = case.key.as_bytes().to_vec();

let key_initial_pos = find_subarrays(&key_part, &initial_core);
let mac_initial_pos = find_subarrays(&mac_part, &initial_core);
let b64_initial_pos = find_subarrays(&key_in_b64, &initial_core);

let key_final_pos = find_subarrays(&key_part, &final_core);
let mac_final_pos = find_subarrays(&mac_part, &final_core);
let b64_final_pos = find_subarrays(&key_in_b64, &final_core);

error |= add_row(
&mut table,
format!("Symm. Key, case {}", idx),
&key_initial_pos,
&key_final_pos,
key_final_pos.is_empty(),
);

error |= add_row(
&mut table,
format!("Symm. MAC, case {}", idx),
&mac_initial_pos,
&mac_final_pos,
mac_final_pos.is_empty(),
);

error |= add_row(
&mut table,
format!("Symm. Key in Base64, case {}", idx),
&b64_initial_pos,
&b64_final_pos,
b64_final_pos.is_empty(),
);
}

for (idx, case) in cases.master_key.iter().enumerate() {
let password = case.password.as_bytes().to_vec();
let key = hex::decode(&case.key_hex).unwrap();
let hash_b64 = case.hash.as_bytes().to_vec();
let hash = hex::decode(&case.hash_hex).unwrap();

let pass_initial_pos = find_subarrays(&password, &initial_core);
let key_initial_pos = find_subarrays(&key, &initial_core);
let hashb64_initial_pos = find_subarrays(&hash_b64, &initial_core);
let hash_initial_pos = find_subarrays(&hash, &initial_core);

let pass_final_pos = find_subarrays(&password, &final_core);
let key_final_pos = find_subarrays(&key, &final_core);
let hashb64_final_pos = find_subarrays(&hash_b64, &final_core);
let hash_final_pos = find_subarrays(&hash, &final_core);

error |= add_row(
&mut table,
format!("Master Key password, case {}", idx),
&pass_initial_pos,
&pass_final_pos,
pass_final_pos.is_empty(),
);

// At the moment the argon library is producing some hard to pinpoint leaks
// Upgrading to the latest pre-release version solves at least one of them
let allowed_leaks = if matches!(case.kdf, Kdf::Argon2id { .. }) {
3
} else {
0
};

error |= add_row(
&mut table,
format!("Master Key, case {}", idx),
&key_initial_pos,
&key_final_pos,
key_final_pos.len() <= allowed_leaks,
);

error |= add_row(
&mut table,
format!("Master Key Hash B64, case {}", idx),
&hashb64_initial_pos,
&hashb64_final_pos,
hashb64_final_pos.is_empty(),
);

error |= add_row(
&mut table,
format!("Master Key Hash, case {}", idx),
&hash_initial_pos,
&hash_final_pos,
hash_final_pos.is_empty(),
);
for (idx, case) in cases.cases.iter().enumerate() {
for lookup in &case.memory_lookups {
let value = match &lookup.value {
MemoryLookupValue::String { string } => string.as_bytes().to_vec(),
MemoryLookupValue::Binary { hex } => hex::decode(hex).unwrap(),
};

let initial_pos = find_subarrays(&value, &initial_core);
let final_pos = find_subarrays(&value, &final_core);

let name = format!("{idx}: {} / {}", case.name, lookup.name);
let ok_cond = final_pos.len() <= lookup.allowed_count.unwrap_or(0);

table.add_row([
name.as_str(),
&comma_sep(&initial_pos),
&comma_sep(&final_pos),
if ok_cond { OK } else { FAIL },
]);

if !ok_cond {
error = true;
}
}
}

println!("{table}");
Expand Down
Loading
Loading