[PM-6100] Test for memory leaks of secrets

.github/codecov.yml

@@ -1,3 +1,4 @@
 ignore:
   - "crates/sdk-schemas" # Tool
   - "crates/uniffi-bindgen" # Tool
+  - "crates/memory-testing" # Testing

.github/workflows/memory-testing.yml

@@ -0,0 +1,47 @@
+---
+name: Memory Testing
+
+on:
+  pull_request:
+    paths:
+      - "crates/bitwarden-crypto/**"
+      - "crates/memory-testing/**"
+  push:
+    paths:
+      - "crates/bitwarden-crypto/**"
+      - "crates/memory-testing/**"
+    branches:
+      - "main"
+      - "rc"
+      - "hotfix-rc"
+
+defaults:
+  run:
+    shell: bash
+
+jobs:
+  memory-test:
+    name: Memory Testing
+    runs-on: ubuntu-22.04
+
+    steps:
+      - name: Checkout
+        uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
+
+      - name: Set up gdb
+        run: |
+          sudo apt update
+          sudo apt -y install gdb
+
+      - name: Install rust
+        uses: dtolnay/rust-toolchain@be73d7920c329f220ce78e0234b8f96b7ae60248 # stable
+        with:
+          toolchain: stable
+
+      - name: Cache cargo registry
+        uses: Swatinem/rust-cache@23bce251a8cd2ffc3c1075eaa2367cf899916d84 # v2.7.3
+        with:
+          key: memtest-cargo
+
+      - name: Memory Testing
+        run: ./crates/memory-testing/run_test.sh no-docker

.gitignore

@@ -12,7 +12,6 @@ x64/
 x86/
 build/
 bld/
-[Bb]in/
 [Oo]bj/
 *.wasm
 

crates/memory-testing/.gitignore

@@ -0,0 +1 @@
+output

crates/memory-testing/Cargo.toml

@@ -0,0 +1,13 @@
+[package]
+name = "memory-testing"
+version = "0.1.0"
+edition = "2021"
+publish = false
+
+[dependencies]
+bitwarden-crypto = { path = "../bitwarden-crypto", version = "=0.1.0" }
+comfy-table = "7.1.0"
+hex = "0.4.3"
+serde = "1.0.196"
+serde_json = "1.0.113"
+zeroize = "1.7.0"

crates/memory-testing/Dockerfile

@@ -0,0 +1,28 @@
+###############################################
+#                 Build stage                 #
+###############################################
+FROM rust:1.76 AS build
+
+# Copy required project files
+COPY . /app
+
+# Build project
+WORKDIR /app
+RUN cargo build -p memory-testing
+
+###############################################
+#                  App stage                  #
+###############################################
+FROM debian:bookworm-slim
+
+# This specifically needs to run as root to be able to capture core dumps
+USER root
+
+RUN apt-get update && apt-get install -y --no-install-recommends gdb && apt-get clean && rm -rf /var/lib/apt/lists/*
+
+# Copy built project from the build stage
+COPY --from=build /app/target/debug/memory-testing .
+COPY --from=build /app/target/debug/capture-dumps .
+COPY --from=build /app/crates/memory-testing/cases.json .
+
+CMD [ "/capture-dumps", "./memory-testing", "/output" ]

crates/memory-testing/Dockerfile.dockerignore

@@ -0,0 +1,4 @@
+*
+!crates/*
+!Cargo.toml
+!Cargo.lock

crates/memory-testing/cases.json

@@ -0,0 +1,9 @@
+{
+  "symmetric_key": [
+    {
+      "key": "FfhVVP8fmFIZY1WmRszPmRmVCxXNWVcJffPrbkywTPtBNkgfhYGT+D9sVGizYXrPffuj2yoyWqMwF9iF5aMQhQ==",
+      "decrypted_key_hex": "15f85554ff1f9852196355a646cccf9919950b15cd5957097df3eb6e4cb04cfb",
+      "decrypted_mac_hex": "4136481f858193f83f6c5468b3617acf7dfba3db2a325aa33017d885e5a31085"
+    }
+  ]
+}

crates/memory-testing/run_test.sh

@@ -0,0 +1,20 @@
+# Move to the root of the repository
+cd "$(dirname "$0")"
+cd ../../
+
+BASE_DIR="./crates/memory-testing"
+
+mkdir -p $BASE_DIR/output
+rm $BASE_DIR/output/*
+
+cargo build -p memory-testing
+
+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
+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

crates/memory-testing/src/bin/analyze-dumps.rs

@@ -0,0 +1,132 @@
+use std::{env, fmt::Display, io, path::Path, process};
+
+use memory_testing::*;
+
+fn find_subarrays(needle: &[u8], haystack: &[u8]) -> Vec<usize> {
+    let needle_len = needle.len();
+    let haystack_len = haystack.len();
+    let mut subarrays = vec![];
+
+    if needle_len == 0 || haystack_len == 0 || needle_len > haystack_len {
+        return vec![];
+    }
+
+    for i in 0..=(haystack_len - needle_len) {
+        if &haystack[i..i + needle_len] == needle {
+            subarrays.push(i);
+        }
+    }
+
+    subarrays
+}
+
+const OK: &str = "✅";
+const FAIL: &str = "❌";
+
+fn comma_sep(nums: &[usize]) -> String {
+    nums.iter()
+        .map(ToString::to_string)
+        .collect::<Vec<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 {
+        println!("Usage: ./analyze-dumps <base_dir>");
+        process::exit(1);
+    }
+    let base_dir: &Path = args[1].as_ref();
+
+    println!("Memory testing script started");
+
+    let initial_core = std::fs::read(base_dir.join("output/initial_dump.bin"))?;
+    let final_core = std::fs::read(base_dir.join("output/final_dump.bin"))?;
+
+    let mut error = false;
+    let mut table = comfy_table::Table::new();
+    table.set_header(vec!["Name", "Initial", "Final", "OK"]);
+
+    let cases = memory_testing::load_cases(base_dir);
+
+    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(),
+        );
+
+        // TODO: At the moment we are not zeroizing the base64 key in from_str, so this test is
+        // ignored
+        add_row(
+            &mut table,
+            format!("Symm. Key in Base64, case {}", idx),
+            &b64_initial_pos,
+            &b64_final_pos,
+            b64_final_pos.is_empty(),
+        );
+    }
+
+    println!("{table}");
+
+    process::exit(if error { 1 } else { 0 });
+}

crates/memory-testing/src/bin/capture-dumps.rs

@@ -0,0 +1,70 @@
+use std::{
+    fs,
+    io::{self, prelude::*},
+    path::Path,
+    process::{Command, Stdio},
+    thread::sleep,
+    time::Duration,
+};
+
+fn dump_process_to_bytearray(pid: u32, output_dir: &Path, output_name: &Path) -> io::Result<u64> {
+    Command::new("gcore")
+        .args(["-a", &pid.to_string()])
+        .output()?;
+
+    let core_path = format!("core.{}", pid);
+    let output_path = output_dir.join(output_name);
+    let len = fs::copy(&core_path, output_path)?;
+    fs::remove_file(&core_path)?;
+    Ok(len)
+}
+
+fn main() -> io::Result<()> {
+    let args: Vec<String> = std::env::args().collect();
+    if args.len() < 3 {
+        println!("Usage: ./capture_dumps <binary_path> <base_dir>");
+        std::process::exit(1);
+    }
+
+    let binary_path = &args[1];
+    let base_dir: &Path = args[2].as_ref();
+
+    println!("Memory dump capture script started");
+
+    let mut proc = Command::new(binary_path)
+        .arg(base_dir)
+        .stdout(Stdio::inherit())
+        .stdin(Stdio::piped())
+        .spawn()?;
+    let id = proc.id();
+    println!("Started memory testing process with PID: {}", id);
+    let stdin = proc.stdin.as_mut().expect("Valid stdin");
+
+    // Wait a bit for it to process
+    sleep(Duration::from_secs(3));
+
+    // Dump the process before the variables are freed
+    let initial_core =
+        dump_process_to_bytearray(id, &base_dir.join("output"), "initial_dump.bin".as_ref())?;
+    println!("Initial core dump file size: {}", initial_core);
+
+    stdin.write_all(b".")?;
+    stdin.flush()?;
+
+    // Wait a bit for it to process
+    sleep(Duration::from_secs(1));
+
+    // Dump the process after the variables are freed
+    let final_core =
+        dump_process_to_bytearray(id, &base_dir.join("output"), "final_dump.bin".as_ref())?;
+    println!("Final core dump file size: {}", final_core);
+
+    stdin.write_all(b".")?;
+    stdin.flush()?;
+
+    // Wait for the process to finish and print the output
+    let output = proc.wait()?;
+    println!("Return code: {}", output);
+
+    std::process::exit(output.code().unwrap_or(1));
+}