Skip to content

Commit 7ea82c7

Browse files
fix(platform-wallet-storage): open the wallet store on Android (#5352)
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 78d494f commit 7ea82c7

2 files changed

Lines changed: 136 additions & 13 deletions

File tree

‎packages/rs-platform-wallet-storage/SECRETS.md‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -400,13 +400,24 @@ unwrapped copy is allocated.
400400
symlink cannot hide an unsafe ancestor behind a safe-looking one. An
401401
ancestor is refused at `open` with `SecretStoreError::InsecureParentDir`
402402
when it is group/other writable (`mode & 0o022`) WITHOUT the sticky bit,
403-
or when it is owned by neither the effective user nor a root identity:
403+
or when it is owned by neither the effective user nor a root identity
404+
(both rules make exceptions for the Android identities trusted below):
404405
directory write access governs rename/replace of the vault, and an
405406
untrusted owner can grant itself that access at will (the A1 guarantee
406407
depends on both). A sticky writable directory such as `/tmp` (`0o1777`)
407408
is accepted — the sticky bit is what stops one user replacing another's
408409
entries. A read-only group-accessible ancestor (`0o750`) is accepted too
409-
— it only leaks filenames, never the 0600-protected vault contents. The
410+
— it only leaks filenames, never the 0600-protected vault contents. On
411+
Android, `system` (`AID_SYSTEM`, 1000) is trusted as well, both as an
412+
owner and as the group of a group-writable ancestor: Android creates every
413+
app's private storage beneath `system:system` `0771` directories (`/data`,
414+
`/data/user/0`, `/data/data`), and no app can run as or join `system`.
415+
The app's own per-app group (gid equal to its uid) is trusted as the group
416+
of a group-writable ancestor too, because Android creates the app's
417+
`files` and `databases` directories `0771` in that group. Other-writable
418+
ancestors and ancestors writable by any other group are still refused
419+
there. The trusted sets are `PLATFORM_IDS` and `trusted_groups` in
420+
`src/parent_permissions.rs`, empty on every other target. The
410421
walk is Unix-only; Windows ACLs are not inspected (issue #3754).
411422
Each secret is capped at `MAX_SECRET_LEN` (8176 B) at the write
412423
boundary — still ~30× any mnemonic/seed/xpriv — so a single oversized

‎packages/rs-platform-wallet-storage/src/parent_permissions.rs‎

Lines changed: 123 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,60 @@ pub(crate) fn insecure_ancestor_message(
6363
}
6464
}
6565

66+
/// Platform identities trusted as ancestor owners and as the group of a
67+
/// group-writable ancestor, beyond the current user and root.
68+
///
69+
/// Android creates every app's private storage beneath `/data`, `/data/user`,
70+
/// `/data/user/0` and `/data/data`, all owned `system:system` (`AID_SYSTEM`,
71+
/// 1000) and mostly `0771`. No app-private path can pass the walk without
72+
/// trusting that identity. `AID_SYSTEM` is the platform itself — it already
73+
/// administers every app's data — and no third-party app can run as it or join
74+
/// its group, so a `system`-group-writable ancestor is not replaceable by
75+
/// anything the check defends against.
76+
#[cfg(target_os = "android")]
77+
const PLATFORM_IDS: &[u32] = &[1000];
78+
#[cfg(all(unix, not(target_os = "android")))]
79+
const PLATFORM_IDS: &[u32] = &[];
80+
6681
#[cfg(unix)]
67-
fn trusted_owner(owner: u32, current_uid: u32, root_uid: u32) -> bool {
68-
owner == current_uid || owner == 0 || owner == root_uid
82+
fn trusted_owner(owner: u32, current_uid: u32, root_uid: u32, platform_ids: &[u32]) -> bool {
83+
owner == current_uid || owner == 0 || owner == root_uid || platform_ids.contains(&owner)
84+
}
85+
86+
/// Groups whose write access to an ancestor is not a replacement risk.
87+
///
88+
/// On Android this is `PLATFORM_IDS` plus the app's own per-app group. Android
89+
/// gives every app a gid equal to its uid, and `Context.getFilesDir()`,
90+
/// `getDatabasesDir()` and friends create the app's own directories `0771`
91+
/// in that group, so `<dataDir>/files` is group-writable by design. No other
92+
/// app can be a member of that group. Everywhere else the set is empty: a
93+
/// desktop user's primary group (macOS `staff`, for one) is commonly shared.
94+
///
95+
/// The group is keyed on the uid, not `getegid()`. The two are equal in every
96+
/// app process, but only the uid carries the guarantee: Android reserves the
97+
/// gid numbered like an app's uid for that app alone, while the effective gid
98+
/// is whatever group the process was started with, which outside an app
99+
/// process can be one that other processes share.
100+
#[cfg(target_os = "android")]
101+
fn trusted_groups(current_uid: u32) -> Vec<u32> {
102+
let mut groups = PLATFORM_IDS.to_vec();
103+
groups.push(current_uid);
104+
groups
105+
}
106+
#[cfg(all(unix, not(target_os = "android")))]
107+
fn trusted_groups(_current_uid: u32) -> Vec<u32> {
108+
PLATFORM_IDS.to_vec()
109+
}
110+
111+
/// Whether an identity outside the trusted set can replace entries in a
112+
/// directory with this mode and group: other-writable, or group-writable for a
113+
/// group outside `trusted_groups`, in both cases without the sticky bit.
114+
#[cfg(unix)]
115+
fn writable_without_sticky(mode: u32, gid: u32, trusted_groups: &[u32]) -> bool {
116+
if mode & 0o1000 != 0 {
117+
return false;
118+
}
119+
mode & 0o002 != 0 || (mode & 0o020 != 0 && !trusted_groups.contains(&gid))
69120
}
70121

71122
#[cfg(unix)]
@@ -98,7 +149,7 @@ fn reject_ancestor(
98149
) -> Result<(), ParentPermissionsError> {
99150
let reason = if writable_without_sticky {
100151
InsecureAncestor::WritableWithoutSticky { mode }
101-
} else if !trusted_owner(uid, current_uid, root_uid) {
152+
} else if !trusted_owner(uid, current_uid, root_uid, PLATFORM_IDS) {
102153
InsecureAncestor::UntrustedOwner { uid, current_uid }
103154
} else {
104155
return Ok(());
@@ -114,6 +165,7 @@ pub(crate) fn check_parent_perms(parent: &Path) -> Result<(), ParentPermissionsE
114165
use std::os::unix::fs::MetadataExt;
115166

116167
let current_uid = effective_uid();
168+
let trusted_groups = trusted_groups(current_uid);
117169
let root_uid = std::fs::metadata("/")
118170
.map_err(ParentPermissionsError::Io)?
119171
.uid();
@@ -129,8 +181,8 @@ pub(crate) fn check_parent_perms(parent: &Path) -> Result<(), ParentPermissionsE
129181
for ancestor in absolute.ancestors() {
130182
let meta = std::fs::symlink_metadata(ancestor).map_err(ParentPermissionsError::Io)?;
131183
let mode = meta.mode() & 0o7777;
132-
let writable_without_sticky =
133-
!meta.file_type().is_symlink() && mode & 0o022 != 0 && mode & 0o1000 == 0;
184+
let writable_without_sticky = !meta.file_type().is_symlink()
185+
&& writable_without_sticky(mode, meta.gid(), &trusted_groups);
134186
reject_ancestor(
135187
ancestor,
136188
meta.uid(),
@@ -143,7 +195,7 @@ pub(crate) fn check_parent_perms(parent: &Path) -> Result<(), ParentPermissionsE
143195
for ancestor in canonical.ancestors() {
144196
let meta = std::fs::metadata(ancestor).map_err(ParentPermissionsError::Io)?;
145197
let mode = meta.mode() & 0o7777;
146-
let writable_without_sticky = mode & 0o022 != 0 && mode & 0o1000 == 0;
198+
let writable_without_sticky = writable_without_sticky(mode, meta.gid(), &trusted_groups);
147199
reject_ancestor(
148200
ancestor,
149201
meta.uid(),
@@ -165,13 +217,73 @@ pub(crate) fn check_parent_perms(_parent: &Path) -> Result<(), ParentPermissions
165217

166218
#[cfg(all(test, unix))]
167219
mod tests {
168-
use super::trusted_owner;
220+
use super::{trusted_owner, writable_without_sticky};
221+
222+
const ANDROID_SYSTEM: &[u32] = &[1000];
169223

170224
#[test]
171225
fn trusted_owner_accepts_current_and_root_identities_only() {
172-
assert!(trusted_owner(1000, 1000, 65_534));
173-
assert!(trusted_owner(0, 1000, 65_534));
174-
assert!(trusted_owner(65_534, 1000, 65_534));
175-
assert!(!trusted_owner(2000, 1000, 65_534));
226+
assert!(trusted_owner(1000, 1000, 65_534, &[]));
227+
assert!(trusted_owner(0, 1000, 65_534, &[]));
228+
assert!(trusted_owner(65_534, 1000, 65_534, &[]));
229+
assert!(!trusted_owner(2000, 1000, 65_534, &[]));
230+
}
231+
232+
#[test]
233+
fn should_trust_android_system_owner_only_when_it_is_a_platform_identity() {
234+
// App uid 10123 under /data/user/0 (owner system, 1000).
235+
assert!(!trusted_owner(1000, 10_123, 0, &[]));
236+
assert!(trusted_owner(1000, 10_123, 0, ANDROID_SYSTEM));
237+
// Another app's uid stays untrusted.
238+
assert!(!trusted_owner(10_456, 10_123, 0, ANDROID_SYSTEM));
239+
}
240+
241+
#[test]
242+
fn should_reject_group_write_unless_group_is_a_platform_identity() {
243+
// `/data` on Android: system:system 0771.
244+
assert!(writable_without_sticky(0o771, 1000, &[]));
245+
assert!(!writable_without_sticky(0o771, 1000, ANDROID_SYSTEM));
246+
// Group-writable for any other group is still replaceable.
247+
assert!(writable_without_sticky(0o775, 1015, ANDROID_SYSTEM));
248+
assert!(writable_without_sticky(0o770, 0, ANDROID_SYSTEM));
249+
}
250+
251+
#[test]
252+
fn should_accept_app_files_dir_group_writable_by_the_apps_own_group() {
253+
// `<dataDir>/files` on Android: app uid 10150, group 10150, 0771.
254+
// `trusted_groups(10_150)` on Android is `[1000, 10_150]`.
255+
let android_app: &[u32] = &[1000, 10_150];
256+
assert!(!writable_without_sticky(0o771, 10_150, android_app));
257+
// Another app's group, and the cache gid (20000 + app id), stay refused.
258+
assert!(writable_without_sticky(0o771, 10_456, android_app));
259+
assert!(writable_without_sticky(0o2771, 20_150, android_app));
260+
// Other-write is refused even in the app's own group.
261+
assert!(writable_without_sticky(0o777, 10_150, android_app));
262+
}
263+
264+
#[cfg(not(target_os = "android"))]
265+
#[test]
266+
fn should_trust_no_extra_groups_off_android() {
267+
assert!(super::trusted_groups(501).is_empty());
268+
// A user-owned, group-writable folder (macOS `staff` is gid 20) is
269+
// still refused on desktop.
270+
assert!(writable_without_sticky(
271+
0o775,
272+
20,
273+
&super::trusted_groups(501)
274+
));
275+
}
276+
277+
#[test]
278+
fn should_reject_other_write_regardless_of_group() {
279+
assert!(writable_without_sticky(0o757, 1000, ANDROID_SYSTEM));
280+
assert!(writable_without_sticky(0o777, 1000, ANDROID_SYSTEM));
281+
}
282+
283+
#[test]
284+
fn should_accept_sticky_and_unwritable_directories() {
285+
assert!(!writable_without_sticky(0o1777, 2000, &[]));
286+
assert!(!writable_without_sticky(0o755, 2000, &[]));
287+
assert!(!writable_without_sticky(0o700, 2000, &[]));
176288
}
177289
}

0 commit comments

Comments
 (0)