diff --git a/packages/rs-platform-wallet-storage/SECRETS.md b/packages/rs-platform-wallet-storage/SECRETS.md index 3f5c23c5cc1..0cad64d2e54 100644 --- a/packages/rs-platform-wallet-storage/SECRETS.md +++ b/packages/rs-platform-wallet-storage/SECRETS.md @@ -400,13 +400,24 @@ unwrapped copy is allocated. symlink cannot hide an unsafe ancestor behind a safe-looking one. An ancestor is refused at `open` with `SecretStoreError::InsecureParentDir` when it is group/other writable (`mode & 0o022`) WITHOUT the sticky bit, - or when it is owned by neither the effective user nor a root identity: + or when it is owned by neither the effective user nor a root identity + (both rules make exceptions for the Android identities trusted below): directory write access governs rename/replace of the vault, and an untrusted owner can grant itself that access at will (the A1 guarantee depends on both). A sticky writable directory such as `/tmp` (`0o1777`) is accepted — the sticky bit is what stops one user replacing another's entries. A read-only group-accessible ancestor (`0o750`) is accepted too - — it only leaks filenames, never the 0600-protected vault contents. The + — it only leaks filenames, never the 0600-protected vault contents. On + Android, `system` (`AID_SYSTEM`, 1000) is trusted as well, both as an + owner and as the group of a group-writable ancestor: Android creates every + app's private storage beneath `system:system` `0771` directories (`/data`, + `/data/user/0`, `/data/data`), and no app can run as or join `system`. + The app's own per-app group (gid equal to its uid) is trusted as the group + of a group-writable ancestor too, because Android creates the app's + `files` and `databases` directories `0771` in that group. Other-writable + ancestors and ancestors writable by any other group are still refused + there. The trusted sets are `PLATFORM_IDS` and `trusted_groups` in + `src/parent_permissions.rs`, empty on every other target. The walk is Unix-only; Windows ACLs are not inspected (issue #3754). Each secret is capped at `MAX_SECRET_LEN` (8176 B) at the write boundary — still ~30× any mnemonic/seed/xpriv — so a single oversized diff --git a/packages/rs-platform-wallet-storage/src/parent_permissions.rs b/packages/rs-platform-wallet-storage/src/parent_permissions.rs index 9359a21d6cb..7d8dc9f60c2 100644 --- a/packages/rs-platform-wallet-storage/src/parent_permissions.rs +++ b/packages/rs-platform-wallet-storage/src/parent_permissions.rs @@ -63,9 +63,60 @@ pub(crate) fn insecure_ancestor_message( } } +/// Platform identities trusted as ancestor owners and as the group of a +/// group-writable ancestor, beyond the current user and root. +/// +/// Android creates every app's private storage beneath `/data`, `/data/user`, +/// `/data/user/0` and `/data/data`, all owned `system:system` (`AID_SYSTEM`, +/// 1000) and mostly `0771`. No app-private path can pass the walk without +/// trusting that identity. `AID_SYSTEM` is the platform itself — it already +/// administers every app's data — and no third-party app can run as it or join +/// its group, so a `system`-group-writable ancestor is not replaceable by +/// anything the check defends against. +#[cfg(target_os = "android")] +const PLATFORM_IDS: &[u32] = &[1000]; +#[cfg(all(unix, not(target_os = "android")))] +const PLATFORM_IDS: &[u32] = &[]; + #[cfg(unix)] -fn trusted_owner(owner: u32, current_uid: u32, root_uid: u32) -> bool { - owner == current_uid || owner == 0 || owner == root_uid +fn trusted_owner(owner: u32, current_uid: u32, root_uid: u32, platform_ids: &[u32]) -> bool { + owner == current_uid || owner == 0 || owner == root_uid || platform_ids.contains(&owner) +} + +/// Groups whose write access to an ancestor is not a replacement risk. +/// +/// On Android this is `PLATFORM_IDS` plus the app's own per-app group. Android +/// gives every app a gid equal to its uid, and `Context.getFilesDir()`, +/// `getDatabasesDir()` and friends create the app's own directories `0771` +/// in that group, so `/files` is group-writable by design. No other +/// app can be a member of that group. Everywhere else the set is empty: a +/// desktop user's primary group (macOS `staff`, for one) is commonly shared. +/// +/// The group is keyed on the uid, not `getegid()`. The two are equal in every +/// app process, but only the uid carries the guarantee: Android reserves the +/// gid numbered like an app's uid for that app alone, while the effective gid +/// is whatever group the process was started with, which outside an app +/// process can be one that other processes share. +#[cfg(target_os = "android")] +fn trusted_groups(current_uid: u32) -> Vec { + let mut groups = PLATFORM_IDS.to_vec(); + groups.push(current_uid); + groups +} +#[cfg(all(unix, not(target_os = "android")))] +fn trusted_groups(_current_uid: u32) -> Vec { + PLATFORM_IDS.to_vec() +} + +/// Whether an identity outside the trusted set can replace entries in a +/// directory with this mode and group: other-writable, or group-writable for a +/// group outside `trusted_groups`, in both cases without the sticky bit. +#[cfg(unix)] +fn writable_without_sticky(mode: u32, gid: u32, trusted_groups: &[u32]) -> bool { + if mode & 0o1000 != 0 { + return false; + } + mode & 0o002 != 0 || (mode & 0o020 != 0 && !trusted_groups.contains(&gid)) } #[cfg(unix)] @@ -98,7 +149,7 @@ fn reject_ancestor( ) -> Result<(), ParentPermissionsError> { let reason = if writable_without_sticky { InsecureAncestor::WritableWithoutSticky { mode } - } else if !trusted_owner(uid, current_uid, root_uid) { + } else if !trusted_owner(uid, current_uid, root_uid, PLATFORM_IDS) { InsecureAncestor::UntrustedOwner { uid, current_uid } } else { return Ok(()); @@ -114,6 +165,7 @@ pub(crate) fn check_parent_perms(parent: &Path) -> Result<(), ParentPermissionsE use std::os::unix::fs::MetadataExt; let current_uid = effective_uid(); + let trusted_groups = trusted_groups(current_uid); let root_uid = std::fs::metadata("/") .map_err(ParentPermissionsError::Io)? .uid(); @@ -129,8 +181,8 @@ pub(crate) fn check_parent_perms(parent: &Path) -> Result<(), ParentPermissionsE for ancestor in absolute.ancestors() { let meta = std::fs::symlink_metadata(ancestor).map_err(ParentPermissionsError::Io)?; let mode = meta.mode() & 0o7777; - let writable_without_sticky = - !meta.file_type().is_symlink() && mode & 0o022 != 0 && mode & 0o1000 == 0; + let writable_without_sticky = !meta.file_type().is_symlink() + && writable_without_sticky(mode, meta.gid(), &trusted_groups); reject_ancestor( ancestor, meta.uid(), @@ -143,7 +195,7 @@ pub(crate) fn check_parent_perms(parent: &Path) -> Result<(), ParentPermissionsE for ancestor in canonical.ancestors() { let meta = std::fs::metadata(ancestor).map_err(ParentPermissionsError::Io)?; let mode = meta.mode() & 0o7777; - let writable_without_sticky = mode & 0o022 != 0 && mode & 0o1000 == 0; + let writable_without_sticky = writable_without_sticky(mode, meta.gid(), &trusted_groups); reject_ancestor( ancestor, meta.uid(), @@ -165,13 +217,73 @@ pub(crate) fn check_parent_perms(_parent: &Path) -> Result<(), ParentPermissions #[cfg(all(test, unix))] mod tests { - use super::trusted_owner; + use super::{trusted_owner, writable_without_sticky}; + + const ANDROID_SYSTEM: &[u32] = &[1000]; #[test] fn trusted_owner_accepts_current_and_root_identities_only() { - assert!(trusted_owner(1000, 1000, 65_534)); - assert!(trusted_owner(0, 1000, 65_534)); - assert!(trusted_owner(65_534, 1000, 65_534)); - assert!(!trusted_owner(2000, 1000, 65_534)); + assert!(trusted_owner(1000, 1000, 65_534, &[])); + assert!(trusted_owner(0, 1000, 65_534, &[])); + assert!(trusted_owner(65_534, 1000, 65_534, &[])); + assert!(!trusted_owner(2000, 1000, 65_534, &[])); + } + + #[test] + fn should_trust_android_system_owner_only_when_it_is_a_platform_identity() { + // App uid 10123 under /data/user/0 (owner system, 1000). + assert!(!trusted_owner(1000, 10_123, 0, &[])); + assert!(trusted_owner(1000, 10_123, 0, ANDROID_SYSTEM)); + // Another app's uid stays untrusted. + assert!(!trusted_owner(10_456, 10_123, 0, ANDROID_SYSTEM)); + } + + #[test] + fn should_reject_group_write_unless_group_is_a_platform_identity() { + // `/data` on Android: system:system 0771. + assert!(writable_without_sticky(0o771, 1000, &[])); + assert!(!writable_without_sticky(0o771, 1000, ANDROID_SYSTEM)); + // Group-writable for any other group is still replaceable. + assert!(writable_without_sticky(0o775, 1015, ANDROID_SYSTEM)); + assert!(writable_without_sticky(0o770, 0, ANDROID_SYSTEM)); + } + + #[test] + fn should_accept_app_files_dir_group_writable_by_the_apps_own_group() { + // `/files` on Android: app uid 10150, group 10150, 0771. + // `trusted_groups(10_150)` on Android is `[1000, 10_150]`. + let android_app: &[u32] = &[1000, 10_150]; + assert!(!writable_without_sticky(0o771, 10_150, android_app)); + // Another app's group, and the cache gid (20000 + app id), stay refused. + assert!(writable_without_sticky(0o771, 10_456, android_app)); + assert!(writable_without_sticky(0o2771, 20_150, android_app)); + // Other-write is refused even in the app's own group. + assert!(writable_without_sticky(0o777, 10_150, android_app)); + } + + #[cfg(not(target_os = "android"))] + #[test] + fn should_trust_no_extra_groups_off_android() { + assert!(super::trusted_groups(501).is_empty()); + // A user-owned, group-writable folder (macOS `staff` is gid 20) is + // still refused on desktop. + assert!(writable_without_sticky( + 0o775, + 20, + &super::trusted_groups(501) + )); + } + + #[test] + fn should_reject_other_write_regardless_of_group() { + assert!(writable_without_sticky(0o757, 1000, ANDROID_SYSTEM)); + assert!(writable_without_sticky(0o777, 1000, ANDROID_SYSTEM)); + } + + #[test] + fn should_accept_sticky_and_unwritable_directories() { + assert!(!writable_without_sticky(0o1777, 2000, &[])); + assert!(!writable_without_sticky(0o755, 2000, &[])); + assert!(!writable_without_sticky(0o700, 2000, &[])); } }