feat: comprehensive audit fixes, new tests, and production improvements

Cargo.toml

@@ -29,6 +29,7 @@ hkdf = "0.12"
 zeroize = "1"
 secrecy = "0.8"
 serde = { version = "1", features = ["derive"] }
+serde_bytes = "0.11"
 serde_json = "1"
 thiserror = "1"
 cfg-if = "1"

THREAT_MODEL.md

@@ -327,6 +327,82 @@ Signature = Ed25519.Sign(ed25519_secret, H)
 
 ---
 
+### 5. FFI Boundary (Mobile Applications)
+**Entry Points**:
+- UniFFI-generated bindings for iOS (Swift) and Android (Kotlin/Java)
+- Cross-language function calls
+- Memory ownership transfers
+
+**Threats**:
+- **Memory Safety**: Incorrect memory management across language boundaries
+- **Type Confusion**: Mismatched types between Rust and target language
+- **Resource Leaks**: Unclosed handles or sessions
+- **Panic Propagation**: Rust panics crossing FFI boundary
+
+**Mitigations**:
+- ✅ UniFFI handles memory management automatically (Arc refcounting)
+- ✅ Structured error codes (`NoiseErrorCode` as `i32`) prevent type confusion
+- ✅ No raw pointers exposed to generated bindings
+- ✅ Thread-safe wrappers (`ThreadSafeSessionManager`) for concurrent access
+- ✅ Error codes mapped to platform-specific error types
+
+**Risk**: LOW (UniFFI provides safe abstractions)
+
+**Mobile-Specific Considerations**:
+- **App Suspension**: Sessions must be persisted before app backgrounding
+- **Memory Dumps**: Keys in memory vulnerable if device is compromised
+- **Jailbreak/Root**: Elevated privileges can access process memory
+- **Debugging**: Debuggers can inspect memory (mitigated by release builds)
+
+**Recommendation**: Applications SHOULD:
+- Persist session state before app suspension (use `NoiseManager::save_state()`)
+- Use secure storage for master seeds (iOS Keychain, Android Keystore)
+- Enable `secure-mem` feature on servers (page locking)
+- Clear sensitive data on app termination
+
+---
+
+### 6. Mobile-Specific Threats
+
+#### 6.1 App Lifecycle Attacks
+**Threat**: App suspension/resume can cause session state loss or corruption
+
+**Mitigations**:
+- ✅ `NoiseManager` provides `save_state()` and `restore_state()` methods
+- ✅ Session state is serializable for persistence
+- ✅ Connection status tracking for reconnection logic
+
+**Risk**: LOW (if state is properly persisted)
+
+#### 6.2 Memory Dump Attacks
+**Threat**: Compromised device can dump process memory containing keys
+
+**Mitigations**:
+- ✅ `Zeroizing` reduces key lifetime in memory
+- ✅ Closure-based key access (keys don't escape function scope)
+- ✅ Optional `secure-mem` feature (page locking on supported OSes)
+- ❌ Cannot fully protect against root/admin access
+
+**Risk**: MEDIUM (requires device compromise)
+
+**Recommendation**: Use secure hardware storage (HSM, TEE) for master seeds in high-security deployments
+
+#### 6.3 Platform-Specific Threats
+
+**iOS**:
+- **Jailbreak Detection**: Jailbroken devices have elevated attack surface
+- **Keychain Access**: Secure storage via iOS Keychain (recommended)
+- **App Sandbox**: Provides isolation but keys still in process memory
+
+**Android**:
+- **Root Detection**: Rooted devices can access all app memory
+- **Keystore**: Hardware-backed key storage available on modern devices
+- **Debugging**: Release builds obfuscate but don't fully protect
+
+**Risk**: MEDIUM (platform-dependent)
+
+---
+
 ## Cryptographic Assumptions
 
 ### Standard Assumptions (Accepted)
@@ -347,6 +423,39 @@ Signature = Ed25519.Sign(ed25519_secret, H)
 
 ---
 
+## FFI Boundary Security
+
+### Trust Model
+The FFI layer (UniFFI) is considered **TRUSTED** for memory safety but **UNTRUSTED** for application logic:
+- ✅ UniFFI-generated code is safe (no manual memory management)
+- ⚠️ Application code calling FFI must handle errors correctly
+- ⚠️ Platform-specific code (Swift/Kotlin) must validate inputs
+
+### Error Handling
+FFI errors are mapped to structured error codes:
+- `NoiseErrorCode` enum provides platform-agnostic error types
+- Errors are serialized as `i32` for cross-language compatibility
+- Platform bindings should map these to native error types
+
+### Memory Safety
+- **Ownership**: UniFFI uses `Arc` for shared ownership (automatic refcounting)
+- **Lifetimes**: No manual lifetime management required
+- **Leaks**: Automatic cleanup when objects are dropped
+
+### Thread Safety
+- `ThreadSafeSessionManager` uses `Arc<Mutex<>>` for concurrent access
+- FFI layer is thread-safe if internal types are thread-safe
+- Mobile apps should use thread-safe wrappers for background workers
+
+### Security Best Practices for FFI
+1. **Input Validation**: Validate all inputs from platform code
+2. **Error Handling**: Never ignore FFI errors
+3. **Resource Management**: Ensure sessions are properly closed
+4. **State Persistence**: Save state before app suspension
+5. **Secure Storage**: Use platform secure storage for master seeds
+
+---
+
 ## Known Limitations
 
 ### 1. No Post-Quantum Cryptography

build.rs

@@ -1,3 +1,4 @@
 fn main() {
     uniffi_build::generate_scaffolding("src/pubky_noise.udl").unwrap();
+    println!("cargo::rustc-check-cfg=cfg(loom)");
 }

fuzz/Cargo.toml

@@ -10,6 +10,8 @@ cargo-fuzz = true
 [dependencies]
 libfuzzer-sys = "0.4"
 arbitrary = { version = "1", features = ["derive"] }
+sha2 = "0.10"
+zeroize = "1"
 
 [dependencies.pubky-noise]
 path = ".."

fuzz/fuzz_targets/fuzz_handshake.rs

@@ -93,17 +93,24 @@ fuzz_target!(|input: HandshakeInput| {
     );
 
     // Should succeed with valid inputs
-    if let Ok((hs_state, first_msg)) = handshake_result {
-        // Test server processing valid message
-        let server_result = pubky_noise::datalink_adapter::server_accept_ik(&server, &first_msg);
+    if let Ok((_hs_state, first_msg)) = handshake_result {
+        // Test server processing valid message using 3-step handshake
+        let server_result = server.build_responder_read_ik(&first_msg);
         // Server should be able to process a properly formed message
         // (though verification may fail due to dummy signatures)
-        let _ = server_result;
+        if let Ok((mut hs, _identity)) = server_result {
+            // Generate response
+            let mut response = vec![0u8; 128];
+            if let Ok(n) = hs.write_message(&[], &mut response) {
+                response.truncate(n);
+                let _ = pubky_noise::datalink_adapter::server_complete_ik(hs);
+            }
+        }
     }
 
     // Test 2: Server handling malformed/arbitrary message
     // This should NOT panic, just return an error
-    let malformed_result = pubky_noise::datalink_adapter::server_accept_ik(&server, &input.malformed_message);
+    let malformed_result = server.build_responder_read_ik(&input.malformed_message);
 
     // We expect an error for malformed messages, but no panic
     match malformed_result {

fuzz/fuzz_targets/fuzz_noise_link.rs

@@ -4,7 +4,7 @@ use arbitrary::Arbitrary;
 use libfuzzer_sys::fuzz_target;
 use pubky_noise::{NoiseClient, NoiseServer, RingKeyProvider, NoiseError};
 use pubky_noise::datalink_adapter::{
-    client_start_ik_direct, client_complete_ik, server_accept_ik, server_complete_ik,
+    client_start_ik_direct, client_complete_ik, server_complete_ik,
 };
 use std::sync::Arc;
 
@@ -74,10 +74,18 @@ fuzz_target!(|input: NoiseLinkInput| {
         Err(_) => return,
     };
 
-    let (s_hs, _identity, response) = match server_accept_ik(&server, &first_msg) {
+    let (mut s_hs, _identity) = match server.build_responder_read_ik(&first_msg) {
         Ok(result) => result,
         Err(_) => return,
     };
+
+    // Generate response message
+    let mut response = vec![0u8; 128];
+    let n = match s_hs.write_message(&[], &mut response) {
+        Ok(n) => n,
+        Err(_) => return,
+    };
+    response.truncate(n);
 
     let mut client_link = match client_complete_ik(c_hs, &response) {
         Ok(link) => link,

src/ffi/types.rs

@@ -33,7 +33,7 @@ impl From<FfiConnectionStatus> for ConnectionStatus {
 }
 
 /// FFI-safe mobile configuration wrapper
-#[derive(uniffi::Record)]
+#[derive(uniffi::Record, Clone)]
 pub struct FfiMobileConfig {
     pub auto_reconnect: bool,
     pub max_reconnect_attempts: u32,

src/identity_payload.rs

@@ -101,7 +101,7 @@ pub fn make_binding_message(params: &BindingMessageParams<'_>) -> [u8; 32] {
     if let Some(r) = params.remote_noise_pub {
         h.update(r);
     }
-    h.update(&INTERNAL_EPOCH.to_le_bytes());
+    h.update(INTERNAL_EPOCH.to_le_bytes());
     h.update(match params.role {
         Role::Client => b"client",
         Role::Server => b"server",

src/lib.rs

@@ -19,6 +19,8 @@
 //! let server = NoiseServer::<_, ()>::new_direct("server_kid", b"server_device", server_ring);
 //! ```
 
+#![allow(unpredictable_function_pointer_comparisons)]
+
 pub mod client;
 pub mod datalink_adapter;
 pub mod errors;
@@ -60,5 +62,4 @@ pub use transport::NoiseTransport;
 
 // UniFFI setup - must be at crate root for proc macros to work
 #[cfg(feature = "uniffi_macros")]
-#[allow(clippy::fn_address_comparisons)]
 uniffi::setup_scaffolding!();

src/mobile_manager.rs

@@ -27,9 +27,6 @@ use std::sync::Arc;
 #[cfg(feature = "storage-queue")]
 use crate::storage_queue::{RetryConfig, StorageBackedMessaging};
 
-/// Internal epoch value - always 0 (epoch is not a user-facing concept).
-const INTERNAL_EPOCH: u32 = 0;
-
 /// Connection status for a Noise session
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
 #[cfg_attr(
@@ -442,7 +439,7 @@ impl<R: RingKeyProvider> NoiseManager<R> {
         &mut self,
         session_id: &SessionId,
         session: pubky::PubkySession,
-        public_client: pubky::Pubky,
+        public_client: pubky::PublicStorage,
         write_path: String,
         read_path: String,
     ) -> Result<StorageBackedMessaging, NoiseError> {

src/pkarr.rs

@@ -15,7 +15,7 @@ pub struct PkarrNoiseRecord {
 }
 
 mod serde_big_array {
-    use serde::{Deserialize, Deserializer, Serialize, Serializer};
+    use serde::{Deserializer, Serializer};
 
     pub fn serialize<S>(arr: &[u8; 64], serializer: S) -> Result<S::Ok, S::Error>
     where

src/session_manager.rs

@@ -140,32 +140,48 @@ impl<R: RingKeyProvider> ThreadSafeSessionManager<R> {
 
     /// Add a session to the manager
     pub fn add_session(&self, session_id: SessionId, link: NoiseLink) -> Option<NoiseLink> {
-        self.inner.lock().unwrap().add_session(session_id, link)
+        self.inner
+            .lock()
+            .expect("Mutex poisoned - thread panicked while holding lock")
+            .add_session(session_id, link)
     }
 
     /// Get a session by ID (returns a copy of the session for thread safety)
     ///
     /// Note: For encryption/decryption, use `with_session` or `with_session_mut` instead
     pub fn has_session(&self, session_id: &SessionId) -> bool {
-        self.inner.lock().unwrap().get_session(session_id).is_some()
+        self.inner
+            .lock()
+            .expect("Mutex poisoned - thread panicked while holding lock")
+            .get_session(session_id)
+            .is_some()
     }
 
     /// Remove a session
     pub fn remove_session(&self, session_id: &SessionId) -> Option<NoiseLink> {
-        self.inner.lock().unwrap().remove_session(session_id)
+        self.inner
+            .lock()
+            .expect("Mutex poisoned - thread panicked while holding lock")
+            .remove_session(session_id)
     }
 
     /// List all sessions
     pub fn list_sessions(&self) -> Vec<SessionId> {
-        self.inner.lock().unwrap().list_sessions()
+        self.inner
+            .lock()
+            .expect("Mutex poisoned - thread panicked while holding lock")
+            .list_sessions()
     }
 
     /// Execute a closure with read access to a session
     pub fn with_session<F, T>(&self, session_id: &SessionId, f: F) -> Option<T>
     where
         F: FnOnce(&NoiseLink) -> T,
     {
-        let manager = self.inner.lock().unwrap();
+        let manager = self
+            .inner
+            .lock()
+            .expect("Mutex poisoned - thread panicked while holding lock");
         manager.get_session(session_id).map(f)
     }
 
@@ -174,7 +190,10 @@ impl<R: RingKeyProvider> ThreadSafeSessionManager<R> {
     where
         F: FnOnce(&mut NoiseLink) -> T,
     {
-        let mut manager = self.inner.lock().unwrap();
+        let mut manager = self
+            .inner
+            .lock()
+            .expect("Mutex poisoned - thread panicked while holding lock");
         manager.get_session_mut(session_id).map(f)
     }
 
@@ -184,7 +203,10 @@ impl<R: RingKeyProvider> ThreadSafeSessionManager<R> {
         session_id: &SessionId,
         plaintext: &[u8],
     ) -> Result<Vec<u8>, crate::errors::NoiseError> {
-        let mut manager = self.inner.lock().unwrap();
+        let mut manager = self
+            .inner
+            .lock()
+            .expect("Mutex poisoned - thread panicked while holding lock");
         manager
             .get_session_mut(session_id)
             .ok_or_else(|| crate::errors::NoiseError::Other("Session not found".to_string()))?
@@ -197,7 +219,10 @@ impl<R: RingKeyProvider> ThreadSafeSessionManager<R> {
         session_id: &SessionId,
         ciphertext: &[u8],
     ) -> Result<Vec<u8>, crate::errors::NoiseError> {
-        let mut manager = self.inner.lock().unwrap();
+        let mut manager = self
+            .inner
+            .lock()
+            .expect("Mutex poisoned - thread panicked while holding lock");
         manager
             .get_session_mut(session_id)
             .ok_or_else(|| crate::errors::NoiseError::Other("Session not found".to_string()))?

src/storage_queue.rs

@@ -2,7 +2,7 @@
 
 use crate::datalink_adapter::NoiseLink;
 use crate::errors::NoiseError;
-use pubky::{Pubky, PubkySession};
+use pubky::{PubkySession, PublicStorage};
 use std::time::Duration;
 
 /// Configuration for retry behavior
@@ -40,7 +40,7 @@ impl Default for RetryConfig {
 pub struct StorageBackedMessaging {
     noise_link: NoiseLink,
     session: PubkySession,
-    public_client: Pubky,
+    public_client: PublicStorage,
     write_path: String,
     read_path: String,
     write_counter: u64,
@@ -59,7 +59,7 @@ impl StorageBackedMessaging {
     pub fn new(
         link: NoiseLink,
         session: PubkySession,
-        public_client: Pubky,
+        public_client: PublicStorage,
         write_path: String,
         read_path: String,
     ) -> Self {
@@ -198,7 +198,7 @@ impl StorageBackedMessaging {
                             )));
                         }
                     }
-                    Err(e) if retry_attempt < self.retry_config.max_retries => {
+                    Err(_e) if retry_attempt < self.retry_config.max_retries => {
                         // Network error - retry
                         retry_attempt += 1;