Skip to main content

max / makenotwork

synckit: cap Argon2 memory, scrub key windows, guard change-password (risk Phase 3) Crypto/Security hardening (crate-internal; no API/consumer break). - Argon2 memory ceiling 1 GiB -> 256 MiB: a hostile envelope can no longer force a 1 GiB alloc on setup/change-password/rotate (mobile OOM/jetsam lever). Wrap cost is 64 MiB so real envelopes still validate. - Wrap fresh/recovered keys in ZeroizeOnDrop from generation in setup_encryption_new/existing and rotation_key_material, so a failed wrap/put/store scrubs the key instead of dropping a bare [u8;32]. - Add `key` to the instrument skip on authenticate/authenticate_with_code so the per-user billing key never lands in tracing spans. - change_password: if the master key is cached, the server envelope must wrap that same key or we refuse — blocks a hostile server substituting a different valid envelope to lock the user out of their own data. - blob_chunk_count saturates instead of truncating the attacker-controlled decrypt-header total_len (a debug_assert there would be a crash DoS). Gate: 308 lib + 101 integration + 1 doc tests, clippy --all-targets clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-07-07 19:03 UTC
Commit: 6ffcc528405d071cbc22c0f892a334c0236c5da9
Parent: ae1d01b
4 files changed, +50 insertions, -15 deletions
M shared/synckit-client/src/client/auth.rs +2 -2
@@ -26,7 +26,7 @@ impl SyncKitClient {
26 26 /// # Errors
27 27 ///
28 28 /// Returns `Server { status: 401, .. }` for wrong credentials.
29 - #[instrument(skip(self, email, password))]
29 + #[instrument(skip(self, email, password, key))]
30 30 pub async fn authenticate(
31 31 &self,
32 32 email: &str,
@@ -146,7 +146,7 @@ impl SyncKitClient {
146 146 ///
147 147 /// Call this after receiving the code from the localhost callback server.
148 148 /// On success, stores the session internally (same as `authenticate()`).
149 - #[instrument(skip(self, code, code_verifier))]
149 + #[instrument(skip(self, code, code_verifier, key))]
150 150 pub async fn authenticate_with_code(
151 151 &self,
152 152 code: &str,
M shared/synckit-client/src/client/encryption.rs +26 -8
@@ -42,17 +42,20 @@ impl SyncKitClient {
42 42 pub async fn setup_encryption_new(&self, password: &str) -> Result<()> {
43 43 let (app_id, user_id) = self.require_session_ids()?;
44 44
45 - let master_key = crypto::generate_master_key();
46 - let envelope = crypto::wrap_master_key(&master_key, password)?;
45 + // Wrap the fresh key immediately so it is scrubbed on every early-return
46 + // path below (wrap/put/store can each fail), not left as a bare array on
47 + // the stack until the final move into the lock.
48 + let master_key = crypto::ZeroizeOnDrop(crypto::generate_master_key());
49 + let envelope = crypto::wrap_master_key(&master_key.0, password)?;
47 50
48 51 // Push to server (expected_version 0 = first key)
49 52 self.put_server_key(&envelope, 0).await?;
50 53
51 54 // Cache in OS keychain
52 - keystore::store_key(app_id, user_id, &master_key)?;
55 + keystore::store_key(app_id, user_id, &master_key.0)?;
53 56
54 57 // Store in memory
55 - *self.master_key.write() = Some(crypto::ZeroizeOnDrop(master_key));
58 + *self.master_key.write() = Some(master_key);
56 59
57 60 tracing::info!("New master key generated and stored");
58 61 Ok(())
@@ -64,13 +67,15 @@ impl SyncKitClient {
64 67 let (app_id, user_id) = self.require_session_ids()?;
65 68
66 69 let (envelope_json, _key_version) = self.get_server_key().await?;
67 - let master_key = crypto::unwrap_master_key(&envelope_json, password)?;
70 + // Wrap immediately so a failed keychain store scrubs the key rather than
71 + // dropping a bare array.
72 + let master_key = crypto::ZeroizeOnDrop(crypto::unwrap_master_key(&envelope_json, password)?);
68 73
69 74 // Cache in OS keychain
70 - keystore::store_key(app_id, user_id, &master_key)?;
75 + keystore::store_key(app_id, user_id, &master_key.0)?;
71 76
72 77 // Store in memory
73 - *self.master_key.write() = Some(crypto::ZeroizeOnDrop(master_key));
78 + *self.master_key.write() = Some(master_key);
74 79
75 80 tracing::info!("Master key recovered from server");
76 81 Ok(())
@@ -113,8 +118,21 @@ impl SyncKitClient {
113 118
114 119 let master_key = crypto::ZeroizeOnDrop(verified_key);
115 120
121 + // If we hold the master key in memory, the envelope the server just handed
122 + // us MUST wrap that same key. A hostile server could otherwise substitute a
123 + // *different* envelope that also unwraps under `old_password` (e.g. one it
124 + // captured), making us re-wrap the wrong key under the new password and
125 + // lock the user out of their own (original-key-encrypted) data. Refuse.
126 + if let Some(cached) = self.master_key.read().as_ref()
127 + && cached.0 != master_key.0
128 + {
129 + return Err(crate::error::SyncKitError::Crypto(
130 + "server key envelope does not match the in-memory master key; refusing to change password".into(),
131 + ));
132 + }
133 +
116 134 // Re-wrap with new password (generates fresh random salt)
117 - let new_envelope = crypto::wrap_master_key(&master_key, new_password)?;
135 + let new_envelope = crypto::wrap_master_key(&master_key.0, new_password)?;
118 136
119 137 // Optimistic lock: reject if another device changed the password
120 138 // between our GET and this PUT.
M shared/synckit-client/src/client/rotation.rs +5 -3
@@ -127,9 +127,11 @@ impl SyncKitClient {
127 127 Ok((key, pending.encrypted_key))
128 128 }
129 129 None => {
130 - let key = crypto::generate_master_key();
131 - let envelope = crypto::wrap_master_key(&key, password)?;
132 - Ok((key, envelope))
130 + // Wrap before the fallible `wrap_master_key` so a wrap error
131 + // scrubs the fresh key instead of dropping a bare array.
132 + let key = crypto::ZeroizeOnDrop(crypto::generate_master_key());
133 + let envelope = crypto::wrap_master_key(&key.0, password)?;
134 + Ok((key.0, envelope))
133 135 }
134 136 }
135 137 }
M shared/synckit-client/src/crypto.rs +17 -2
@@ -46,7 +46,12 @@ const ARGON2_PARALLELISM: u32 = 1;
46 46 // ceiling rejects an inflated one whose allocation would OOM every syncing device.
47 47 // The pinned wrap constants above sit inside this range, so real envelopes unwrap.
48 48 const ARGON2_MEM_MIN_KB: u32 = 8 * 1024; // 8 MiB
49 - const ARGON2_MEM_MAX_KB: u32 = 1024 * 1024; // 1 GiB
49 + // 256 MiB. The wrap cost is 64 MiB, so real envelopes sit comfortably inside the
50 + // range. A 1 GiB ceiling let a hostile server demand a 1 GiB Argon2 allocation on
51 + // every setup/change-password/rotate — an OOM (jetsam) kill lever on a 2-4 GiB
52 + // mobile device. 256 MiB still permits a stronger-than-default envelope while
53 + // staying well under the mobile memory budget.
54 + const ARGON2_MEM_MAX_KB: u32 = 256 * 1024; // 256 MiB
50 55 const ARGON2_TIME_MAX: u32 = 16;
51 56 const ARGON2_PARALLELISM_MAX: u32 = 16;
52 57
@@ -496,7 +501,14 @@ fn blob_chunk_count(total_len: usize, chunk_size: usize) -> u32 {
496 501 if total_len == 0 {
497 502 1
498 503 } else {
499 - total_len.div_ceil(chunk_size) as u32
504 + // Saturate rather than truncate. `total_len` is trusted (= `data.len()`,
505 + // capped upstream) on the encrypt path but ATTACKER-CONTROLLED on the
506 + // decrypt path (it comes from the v3 header), where a bare `as u32` would
507 + // silently wrap an astronomical claim down to a small count. Saturating to
508 + // u32::MAX keeps the value obviously-too-large so the caller's
509 + // `total_len`-vs-input-length bound rejects the header instead — and a
510 + // panic (debug_assert) here would itself be a crash DoS on that path.
511 + total_len.div_ceil(chunk_size).min(u32::MAX as usize) as u32
500 512 }
501 513 }
502 514
@@ -1432,6 +1444,9 @@ mod tests {
1432 1444 let salt = [7u8; 32];
1433 1445 // Inflated memory (OOM DoS from a hostile envelope) rejected before allocation.
1434 1446 assert!(derive_wrapping_key_with_params("pw", &salt, 4_000_000, 3, 1).is_err());
1447 + // Mobile-OOM fix: 512 MiB — valid under the old 1 GiB ceiling — is now
1448 + // rejected (before any allocation) by the 256 MiB ceiling.
1449 + assert!(derive_wrapping_key_with_params("pw", &salt, 512 * 1024, 3, 1).is_err());
1435 1450 // Weakened memory (KDF downgrade) rejected.
1436 1451 assert!(derive_wrapping_key_with_params("pw", &salt, 8, 3, 1).is_err());
1437 1452 // Zero time / parallelism rejected.