audit: ensure unsafe blocks are really safe [WPB-10887]

Cargo.toml

@@ -6,14 +6,24 @@ members = [
     "keystore",
     "keystore-dump",
     "mls-provider",
-    "interop"
-, "decode"]
+    "interop",
+    "decode",
+]
 exclude = [
     "extras/webdriver-installation",
-    "extras/keystore-regression-versions"
+    "extras/keystore-regression-versions",
 ]
 resolver = "2"
 
+[workspace.lints.clippy]
+missing_safety_doc = "deny"
+undocumented_unsafe_blocks = "deny"
+
+[workspace.lints.rust]
+unexpected_cfgs = { level = "warn", check-cfg = [
+    'cfg(wasm_bindgen_unstable_test_coverage)',
+] }
+
 [workspace.dependencies]
 async-lock = "3.4"
 async-recursion = "1"
@@ -26,7 +36,12 @@ const_format = "0.2"
 core-crypto = { path = "crypto" }
 core-crypto-keystore = { path = "keystore" }
 core-crypto-macros = { path = "crypto-macros" }
-derive_more = { version = "0.99", features = ["from", "into", "deref", "deref_mut"] }
+derive_more = { version = "0.99", features = [
+    "from",
+    "into",
+    "deref",
+    "deref_mut",
+] }
 futures-util = "0.3"
 hex = "0.4"
 idb = "0.6"
@@ -37,7 +52,7 @@ log-reload = "0.1.0"
 mls-crypto-provider = { path = "mls-provider" }
 pem = "3.0"
 rand = { version = "0.8", features = ["getrandom"] }
-rexie = "0.6.1"  # TODO: remove once migration tests have been moved to rexie
+rexie = "0.6.1" # TODO: remove once migration tests have been moved to rexie
 schnellru = "0.2"
 serde = "1.0"
 serde_json = "1.0"

crypto-ffi/Cargo.toml

@@ -16,6 +16,9 @@ name = "uniffi-bindgen"
 path = "uniffi-bindgen.rs"
 required-features = ["uniffi/cli"]
 
+[lints]
+workspace = true
+
 [features]
 default = ["proteus"]
 proteus = [
@@ -81,11 +84,9 @@ wasm-opt = [
     "--detect-features",
 ]
 
-[lints.rust]
-unexpected_cfgs = { level = "warn", check-cfg = [
-    'cfg(wasm_bindgen_unstable_test_coverage)',
-] }
-
 [dev-dependencies]
 testing_logger = "0.1.1"
-tokio = { version = "1.43.0", default-features = false, features = ["macros", "rt"] }
+tokio = { version = "1.43.0", default-features = false, features = [
+    "macros",
+    "rt",
+] }

crypto-ffi/src/wasm/mod.rs

@@ -1183,6 +1183,13 @@ pub struct CoreCryptoWasmLogger {
     ctx: JsValue,
 }
 
+// SAFETY: WASM only ever runs in a single-threaded context, so this is intrinsically thread-safe.
+// If that invariant ever varies, we may need to rethink this (but more likely that would be addressed
+// upstream where the types are defined).
+unsafe impl Send for CoreCryptoWasmLogger {}
+// SAFETY: WASM only ever runs in a single-threaded context, so this is intrinsically thread-safe.
+unsafe impl Sync for CoreCryptoWasmLogger {}
+
 #[wasm_bindgen]
 #[derive(Debug, Clone, Copy)]
 #[repr(u8)]
@@ -1385,9 +1392,8 @@ Please make all callbacks `async` or manually return a `Promise` via `Promise.re
 
 // SAFETY: All callback instances are wrapped into Arc<RwLock> so this is safe to mark
 unsafe impl Send for MlsTransportProvider {}
+// SAFETY: All callback instances are wrapped into Arc<RwLock> so this is safe to mark
 unsafe impl Sync for MlsTransportProvider {}
-unsafe impl Send for CoreCryptoWasmLogger {}
-unsafe impl Sync for CoreCryptoWasmLogger {}
 
 #[async_trait::async_trait(?Send)]
 impl MlsTransport for MlsTransportProvider {

crypto-macros/Cargo.toml

@@ -9,6 +9,9 @@ license = "GPL-3.0-only"
 [lib]
 proc-macro = true
 
+[lints]
+workspace = true
+
 [dependencies]
 syn = { version = "2", features = ["full"] }
 quote = "1"

crypto-macros/src/entity_derive/derive_impl.rs

@@ -92,6 +92,7 @@ impl KeyStoreEntityFlattened {
                     conn: &mut Self::ConnectionType,
                     params: crate::entities::EntityFindParams,
                 ) -> crate::CryptoKeystoreResult<Vec<Self>> {
+                    let mut conn = conn.conn().await;
                     let transaction = conn.transaction()?;
                     let query = #find_all_query.to_string() + &params.to_sql();
 
@@ -120,6 +121,7 @@ impl KeyStoreEntityFlattened {
                     conn: &mut Self::ConnectionType,
                     id: &crate::entities::StringEntityId,
                 ) -> crate::CryptoKeystoreResult<Option<Self>> {
+                    let mut conn = conn.conn().await;
                     let transaction = conn.transaction()?;
                     use rusqlite::OptionalExtension as _;
 
@@ -152,7 +154,8 @@ impl KeyStoreEntityFlattened {
                 }
 
                 async fn count(conn: &mut Self::ConnectionType) -> crate::CryptoKeystoreResult<usize> {
-                    Ok(conn.query_row(&#count_query, [], |r| r.get(0))?)
+                    let conn = conn.conn().await;
+                    conn.query_row(&#count_query, [], |r| r.get(0)).map_err(Into::into)
                 }
             }
         }

crypto/Cargo.toml

@@ -11,10 +11,25 @@ publish = false
 name = "core_crypto"
 crate-type = ["lib", "cdylib"]
 
+[lints]
+workspace = true
+
 [features]
 default = ["proteus", "cryptobox-migrate"]
-proteus = ["dep:proteus-wasm", "dep:proteus-traits", "core-crypto-keystore/proteus-keystore"]
-cryptobox-migrate = ["proteus", "proteus-wasm?/cryptobox-identity", "dep:async-fs", "dep:futures-lite", "dep:rexie", "dep:idb", "dep:base64"]
+proteus = [
+    "dep:proteus-wasm",
+    "dep:proteus-traits",
+    "core-crypto-keystore/proteus-keystore",
+]
+cryptobox-migrate = [
+    "proteus",
+    "proteus-wasm?/cryptobox-identity",
+    "dep:async-fs",
+    "dep:futures-lite",
+    "dep:rexie",
+    "dep:idb",
+    "dep:base64",
+]
 # for test/bench all ciphersuites
 test-all-cipher = []
 # execute benches with also real db to better see overhead
@@ -58,7 +73,10 @@ proteus-traits = { workspace = true, optional = true }
 sha2 = "0.10.8"
 
 [target.'cfg(not(target_family = "wasm"))'.dependencies]
-sysinfo = { version = "0.32", default-features = false, features = ["apple-app-store", "system"] }
+sysinfo = { version = "0.32", default-features = false, features = [
+    "apple-app-store",
+    "system",
+] }
 async-fs = { version = "2.1", optional = true }
 futures-lite = { version = "2.6", optional = true }
 
@@ -105,7 +123,12 @@ version = "0.5"
 features = ["async_std", "html_reports"]
 
 [package.metadata.wasm-pack.profile.release]
-wasm-opt = ["-Os", "--enable-mutable-globals", "--enable-threads", "--detect-features"]
+wasm-opt = [
+    "-Os",
+    "--enable-mutable-globals",
+    "--enable-threads",
+    "--detect-features",
+]
 
 [[bench]]
 name = "key_package"
@@ -141,6 +164,3 @@ anyhow = "1.0.95"
 # is available on the command line. If not, we should replace this with
 # `vergen-gix` or `vergen-git2`.
 vergen-gitcl = { version = "1.0.2", features = ["build", "cargo"] }
-
-[lints.rust]
-unexpected_cfgs = { level = "warn", check-cfg = ['cfg(wasm_bindgen_unstable_test_coverage)'] }

decode/Cargo.toml

@@ -4,8 +4,11 @@ version = "0.1.0"
 edition = "2021"
 license = "GPL-3.0-only"
 
+[lints]
+workspace = true
+
 [dependencies]
 clap = { version = "4.5.27", features = ["derive"] }
-proteus-wasm = { workspace = true,  features = ["serde"]}
+proteus-wasm = { workspace = true, features = ["serde"] }
 base64 = { workspace = true }
 openmls = { workspace = true }

extras/keystore-regression-versions/Cargo.toml

@@ -13,4 +13,3 @@ rand = "0.8"
 clap = { version = "4", features = ["derive"] }
 cfg-if = "1"
 indicatif = { version = "0.17", features = ["futures"] }
-

extras/webdriver-installation/Cargo.toml

@@ -20,7 +20,12 @@ url = "2.3"
 reqwest = { version = "0.11", features = ["json", "stream"] }
 serde = { version = "1.0", features = ["derive"] }
 serde_json = "1.0"
-chrono = { version = "0.4", default-features = false, features = ["std", "clock", "wasmbind", "serde"] }
+chrono = { version = "0.4", default-features = false, features = [
+    "std",
+    "clock",
+    "wasmbind",
+    "serde",
+] }
 semver = { version = "1.0", features = ["serde"] }
 tempfile = "3.3"
 futures-util = "0.3"

interop/Cargo.toml

@@ -11,9 +11,12 @@ proteus = [
     "dep:proteus",
     "dep:proteus-wasm",
     "core-crypto/cryptobox-migrate",
-    "core-crypto/proteus"
+    "core-crypto/proteus",
 ]
 
+[lints]
+workspace = true
+
 [dependencies]
 color-eyre = "0.6"
 log = "0.4"
@@ -37,7 +40,9 @@ uuid = { workspace = true, features = ["v4"] }
 tempfile = { version = "3.15" }
 # Terminal support
 xshell = "0.2"
-spinoff = { version = "0.8", features = ["aesthetic"], default-features = false }
+spinoff = { version = "0.8", features = [
+    "aesthetic",
+], default-features = false }
 serde = { workspace = true, features = ["derive"] }
 thiserror = { workspace = true }
 
@@ -52,4 +57,3 @@ warp = { version = "0.3", default-features = false }
 webdriver-installation = { path = "../extras/webdriver-installation" }
 fantoccini = "0.21"
 proteus-wasm = { workspace = true, optional = true }
-

keystore-dump/Cargo.toml

@@ -5,11 +5,13 @@ version = "4.0.0"
 edition = "2021"
 license = "GPL-3.0-only"
 
+[lints]
+workspace = true
+
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 [dependencies]
 color-eyre = "0.6"
 
-
 [target.'cfg(not(target_family = "wasm"))'.dependencies]
 core-crypto-keystore = { workspace = true, features = ["serde"] }
 core-crypto = { path = "../crypto" }
@@ -28,7 +30,11 @@ openmls_basic_credential.workspace = true
 openmls_x509_credential.workspace = true
 tls_codec.workspace = true
 
-chrono = { version = "0.4", default-features = false, features = ["clock", "std", "serde"] }
+chrono = { version = "0.4", default-features = false, features = [
+    "clock",
+    "std",
+    "serde",
+] }
 
 [target.'cfg(not(target_family = "wasm"))'.dependencies.proteus-wasm]
 workspace = true

keystore/Cargo.toml

@@ -19,10 +19,17 @@ harness = false
 name = "write"
 harness = false
 
+[lints]
+workspace = true
+
 [features]
 default = ["proteus-keystore"]
 proteus-keystore = ["dep:proteus-traits"]
-ios-wal-compat = ["dep:security-framework", "dep:security-framework-sys", "dep:core-foundation"]
+ios-wal-compat = [
+    "dep:security-framework",
+    "dep:security-framework-sys",
+    "dep:core-foundation",
+]
 idb-regression-test = []
 log-queries = ["rusqlite/trace"]
 serde = ["dep:serde"]
@@ -66,7 +73,7 @@ features = [
     "limits",
     "unlock_notify",
     "uuid",
-    "functions"
+    "functions",
 ]
 
 [target.'cfg(not(target_family = "wasm"))'.dependencies.refinery]
@@ -105,7 +112,10 @@ rstest = "0.24"
 rstest_reuse = "0.7"
 async-std = { workspace = true, features = ["attributes"] }
 futures-lite = "2.6"
-core-crypto-keystore = { path = ".", features = ["idb-regression-test", "log-queries"] }
+core-crypto-keystore = { path = ".", features = [
+    "idb-regression-test",
+    "log-queries",
+] }
 pretty_env_logger = "0.5"
 proteus-wasm = { workspace = true }
 
@@ -114,7 +124,9 @@ version = "0.5"
 features = ["async_futures", "html_reports"]
 
 [package.metadata.wasm-pack.profile.release]
-wasm-opt = ["-Os", "--enable-mutable-globals", "--enable-threads", "--detect-features"]
-
-[lints.rust]
-unexpected_cfgs = { level = "warn", check-cfg = ['cfg(wasm_bindgen_unstable_test_coverage)'] }
+wasm-opt = [
+    "-Os",
+    "--enable-mutable-globals",
+    "--enable-threads",
+    "--detect-features",
+]

keystore/src/connection/mod.rs

@@ -58,7 +58,9 @@ pub trait DatabaseConnectionRequirements: Sized {}
 
 #[cfg_attr(target_family = "wasm", async_trait::async_trait(?Send))]
 #[cfg_attr(not(target_family = "wasm"), async_trait::async_trait)]
-pub trait DatabaseConnection: DatabaseConnectionRequirements {
+pub trait DatabaseConnection<'a>: DatabaseConnectionRequirements {
+    type Connection: 'a;
+
     async fn open(name: &str, key: &str) -> CryptoKeystoreResult<Self>;
 
     async fn open_in_memory(name: &str, key: &str) -> CryptoKeystoreResult<Self>;
@@ -82,13 +84,6 @@ pub trait DatabaseConnection: DatabaseConnectionRequirements {
 
         Ok(())
     }
-    #[cfg(not(target_family = "wasm"))]
-    async fn new_transaction(&mut self) -> CryptoKeystoreResult<TransactionWrapper<'_>>;
-    #[cfg(target_family = "wasm")]
-    async fn new_transaction<T: AsRef<str>>(
-        &mut self,
-        tables: &[T],
-    ) -> CryptoKeystoreResult<crate::connection::TransactionWrapper<'_>>;
 }
 
 #[derive(Debug, Clone)]
@@ -126,8 +121,9 @@ pub trait FetchFromDatabase: Send + Sync {
     async fn count<E: Entity<ConnectionType = KeystoreDatabaseConnection>>(&self) -> CryptoKeystoreResult<usize>;
 }
 
-// * SAFETY: this has mutexes and atomics protecting underlying data so this is safe to share between threads
+// SAFETY: this has mutexes and atomics protecting underlying data so this is safe to share between threads
 unsafe impl Send for Connection {}
+// SAFETY: this has mutexes and atomics protecting underlying data so this is safe to share between threads
 unsafe impl Sync for Connection {}
 
 impl Connection {