diff --git a/crates/vnidrop/src/secure_secret/android.rs b/crates/vnidrop/src/secure_secret/android.rs index 1f827a6..72bf491 100644 --- a/crates/vnidrop/src/secure_secret/android.rs +++ b/crates/vnidrop/src/secure_secret/android.rs @@ -70,7 +70,11 @@ impl AndroidSecureSecretStore { } fn record_path(&self, handle: &SecretHandle) -> PathBuf { - let encoded = HEXLOWER.encode(handle.as_str().as_bytes()); + // Hash the handle for the on-disk name. Scoped handles are longer than + // Linux/Android NAME_MAX when hex-encoded, and the handle is already + // authenticated inside the record body. + let digest = blake3::hash(handle.as_str().as_bytes()); + let encoded = HEXLOWER.encode(digest.as_bytes()); self.records_dir .join(format!("{encoded}.{RECORD_EXTENSION}")) } @@ -192,16 +196,12 @@ impl SecureSecretStore for AndroidSecureSecretStore { if path.extension().and_then(|value| value.to_str()) != Some(RECORD_EXTENSION) { continue; } - let stem = path - .file_stem() - .and_then(|value| value.to_str()) - .ok_or(SecureSecretStoreError::Corrupted)?; - let decoded = HEXLOWER - .decode(stem.as_bytes()) - .map_err(|_| SecureSecretStoreError::Corrupted)?; - let handle = - String::from_utf8(decoded).map_err(|_| SecureSecretStoreError::Corrupted)?; - handles.push(SecretHandle(handle)); + let mut bytes = Vec::new(); + File::open(&path) + .map_err(map_io_error)? + .read_to_end(&mut bytes) + .map_err(map_io_error)?; + handles.push(decode_handle_from_record(&bytes)?); } handles.sort_by(|left, right| left.as_str().cmp(right.as_str())); Ok(handles) @@ -242,11 +242,32 @@ fn decode_record( bytes: &[u8], expected_handle: &SecretHandle, ) -> Result { + let (handle, state, nonce, ciphertext) = parse_record(bytes)?; + if handle.as_str() != expected_handle.as_str() || state != RECORD_SEALED { + return Err(SecureSecretStoreError::Corrupted); + } + if nonce.is_empty() || ciphertext.is_empty() { + return Err(SecureSecretStoreError::Corrupted); + } + Ok(AndroidSealedValue { nonce, ciphertext }) +} + +fn decode_handle_from_record(bytes: &[u8]) -> Result { + let (handle, _state, _nonce, _ciphertext) = parse_record(bytes)?; + Ok(handle) +} + +fn parse_record( + bytes: &[u8], +) -> Result<(SecretHandle, u8, Vec, Vec), SecureSecretStoreError> { const HEADER_LEN: usize = 8 + 1 + 2 + 2 + 4; if bytes.len() < HEADER_LEN || &bytes[..8] != RECORD_MAGIC { return Err(SecureSecretStoreError::Corrupted); } let state = bytes[8]; + if state != RECORD_STAGED && state != RECORD_SEALED { + return Err(SecureSecretStoreError::Corrupted); + } let handle_len = usize::from(u16::from_be_bytes([bytes[9], bytes[10]])); let nonce_len = usize::from(u16::from_be_bytes([bytes[11], bytes[12]])); let ciphertext_len = usize::try_from(u32::from_be_bytes([ @@ -258,23 +279,19 @@ fn decode_record( .and_then(|value| value.checked_add(nonce_len)) .and_then(|value| value.checked_add(ciphertext_len)) .ok_or(SecureSecretStoreError::Corrupted)?; - if bytes.len() != expected_len || state != RECORD_SEALED { + if bytes.len() != expected_len { return Err(SecureSecretStoreError::Corrupted); } let handle_end = HEADER_LEN + handle_len; let handle = std::str::from_utf8(&bytes[HEADER_LEN..handle_end]) .map_err(|_| SecureSecretStoreError::Corrupted)?; - if handle != expected_handle.as_str() { - return Err(SecureSecretStoreError::Corrupted); - } let nonce_end = handle_end + nonce_len; - if nonce_len == 0 || ciphertext_len == 0 { - return Err(SecureSecretStoreError::Corrupted); - } - Ok(AndroidSealedValue { - nonce: bytes[handle_end..nonce_end].to_vec(), - ciphertext: bytes[nonce_end..].to_vec(), - }) + Ok(( + SecretHandle(handle.to_string()), + state, + bytes[handle_end..nonce_end].to_vec(), + bytes[nonce_end..].to_vec(), + )) } fn map_io_error(error: std::io::Error) -> SecureSecretStoreError { diff --git a/crates/vnidrop/src/secure_secret/platform.rs b/crates/vnidrop/src/secure_secret/platform.rs index e7a01d4..8c104c0 100644 --- a/crates/vnidrop/src/secure_secret/platform.rs +++ b/crates/vnidrop/src/secure_secret/platform.rs @@ -1,5 +1,6 @@ use std::{ fs::{File, OpenOptions}, + io, path::Path, sync::Arc, }; @@ -29,13 +30,51 @@ pub(crate) fn lock_profile(app_data_dir: &Path) -> Result Result<(), VnidropError> { + #[cfg(unix)] + { + use std::os::fd::AsRawFd; + let rc = unsafe { libc::flock(file.as_raw_fd(), libc::LOCK_EX | libc::LOCK_NB) }; + if rc == 0 { + return Ok(()); + } + let err = io::Error::last_os_error(); + return Err(match err.kind() { + io::ErrorKind::WouldBlock => VnidropError::SecureStorageUnavailable { + reason: "another protected core is already using this profile".to_string(), + }, + _ => VnidropError::filesystem(err), + }); + } + #[cfg(windows)] + { + match file.try_lock() { + Ok(()) => Ok(()), + Err(err) if err.kind() == io::ErrorKind::WouldBlock => { + Err(VnidropError::SecureStorageUnavailable { + reason: "another protected core is already using this profile".to_string(), + }) + } + Err(err) => Err(VnidropError::filesystem(err)), + } + } + #[cfg(not(any(unix, windows)))] + { + let _ = file; + Err(VnidropError::SecureStorageUnavailable { + reason: "profile locking is unsupported on this platform".to_string(), + }) + } +} + /// Opens the profile marker without locking so in-process restart tests can /// reopen the same directory after dropping the previous core. #[cfg(test)] diff --git a/crates/vnidrop/src/tests/secure_secret.rs b/crates/vnidrop/src/tests/secure_secret.rs index b3fe813..e15422b 100644 --- a/crates/vnidrop/src/tests/secure_secret.rs +++ b/crates/vnidrop/src/tests/secure_secret.rs @@ -38,13 +38,33 @@ fn a_profile_allows_only_one_protected_core_mutator() { assert!(matches!( lock_profile(temp.path()), - Err(VnidropError::SecureStorageUnavailable { .. }) + Err(VnidropError::SecureStorageUnavailable { reason }) + if reason.contains("already using this profile") )); drop(first); assert!(lock_profile(temp.path()).is_ok()); } +#[test] +fn profile_lock_maps_contention_not_generic_io_failures() { + // Regression: Android's std File::try_lock returns Unsupported; lock_profile + // must use flock and only treat WouldBlock as "already using this profile". + let temp = tempfile::tempdir().unwrap(); + let held = lock_profile(temp.path()).unwrap(); + match lock_profile(temp.path()) { + Err(VnidropError::SecureStorageUnavailable { reason }) => { + assert!( + reason.contains("already using this profile"), + "unexpected reason: {reason}" + ); + } + Ok(_) => panic!("expected contended lock to fail"), + Err(other) => panic!("expected SecureStorageUnavailable, got {other:?}"), + } + drop(held); +} + #[tokio::test] async fn reconciliation_is_scoped_to_one_application_profile() { let root = tempfile::tempdir().unwrap(); diff --git a/crates/vnidrop/src/tests/secure_secret_android.rs b/crates/vnidrop/src/tests/secure_secret_android.rs index 32d03a3..ada9db0 100644 --- a/crates/vnidrop/src/tests/secure_secret_android.rs +++ b/crates/vnidrop/src/tests/secure_secret_android.rs @@ -178,3 +178,27 @@ fn failed_replacement_keeps_the_previous_secret_readable() { )); assert_eq!(store.get(&handle).unwrap(), original); } + +#[test] +fn scoped_handle_record_names_fit_linux_name_max() { + let (_directory, store, _keystore) = fixture(); + // Mirrors ScopedSecretStore physical handles used on Android profiles. + let handle = secret_handle_for_test(&format!( + "vnidrop/v1/scope-{}/endpoint-identity/{}", + "a".repeat(64), + "b".repeat(36), + )); + assert!(handle.as_str().len() > 100); + let path = store.record_path_for_test(&handle); + let file_name = path.file_name().and_then(|value| value.to_str()).unwrap(); + assert!( + file_name.len() <= 255, + "record file name exceeds NAME_MAX: {} bytes ({file_name})", + file_name.len() + ); + + let material = SecretMaterial::new(vec![0x5a; TEST_SECRET_BYTES]).unwrap(); + store.put(&handle, material.clone()).unwrap(); + assert_eq!(store.list_handles().unwrap(), vec![handle.clone()]); + assert_eq!(store.get(&handle).unwrap(), material); +} diff --git a/shared/src/commonMain/kotlin/com/vnidrop/app/App.kt b/shared/src/commonMain/kotlin/com/vnidrop/app/App.kt index aae57bd..803990b 100644 --- a/shared/src/commonMain/kotlin/com/vnidrop/app/App.kt +++ b/shared/src/commonMain/kotlin/com/vnidrop/app/App.kt @@ -255,7 +255,7 @@ fun App( windowChrome?.invoke() val startingLabel = stringResource(Res.string.app_starting) AnimatedVisibility( - visible = !sendCoreState.isInitialized, + visible = !sendCoreState.isInitialized && !appState.startupSettled, enter = fadeIn(), exit = fadeOut(), ) { diff --git a/shared/src/commonMain/kotlin/com/vnidrop/app/feature/app/AppViewModel.kt b/shared/src/commonMain/kotlin/com/vnidrop/app/feature/app/AppViewModel.kt index a34a8f3..574caae 100644 --- a/shared/src/commonMain/kotlin/com/vnidrop/app/feature/app/AppViewModel.kt +++ b/shared/src/commonMain/kotlin/com/vnidrop/app/feature/app/AppViewModel.kt @@ -21,6 +21,8 @@ import kotlinx.coroutines.launch data class AppState( val destination: AppDestination = AppDestination.Send, val themeMode: ThemeMode = ThemeMode.System, + /** True after the first core initialize attempt finishes (success or failure). */ + val startupSettled: Boolean = false, ) class AppGraphViewModel(dependencies: AppDependencies) : ViewModel() { @@ -46,6 +48,7 @@ class AppViewModel( viewModelScope.launch { val relaySettings = preferencesRepository.preferences.first().relaySettings repository.initialize(environment.defaultCoreDataDir, relaySettings).onFailure(messages::error) + _state.update { it.copy(startupSettled = true) } } viewModelScope.launch { preferencesRepository.preferences.collect { preferences -> diff --git a/shared/src/commonTest/kotlin/com/vnidrop/app/feature/ViewModelsTest.kt b/shared/src/commonTest/kotlin/com/vnidrop/app/feature/ViewModelsTest.kt index f95b342..2de9f47 100644 --- a/shared/src/commonTest/kotlin/com/vnidrop/app/feature/ViewModelsTest.kt +++ b/shared/src/commonTest/kotlin/com/vnidrop/app/feature/ViewModelsTest.kt @@ -80,6 +80,7 @@ class ViewModelsTest { val viewModel = AppViewModel(environment(), core, preferences(), UiMessageController()) advanceUntilIdle() assertTrue(core.state.value.isInitialized) + assertTrue(viewModel.state.value.startupSettled) assertEquals(listOf(RelaySettings()), core.initializedRelaySettings) viewModel.selectDestination(AppDestination.Settings) assertEquals(AppDestination.Settings, viewModel.state.value.destination)