1382246 - The Linux file broker no longer needs to support running with different real uid/gid from its client

Status: RESOLVED FIXED

(Core :: Security: Process Sandboxing, enhancement)

Description

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

B2G used threads in the parent process to run the file broker, rather than separate processes, to save memory (and code complexity to some extent).  But the parent process ran as root.  To prevent confused deputy problems[1], the broker threads set their effective uid/gid[2] to those of the client process[3].  Which brings us to this comment in the broker[4]:

          // This can't use access() itself because that uses the ruid
          // and not the euid.  In theory faccessat() with AT_EACCESS
          // would work, but Linux doesn't actually implement the
          // flags != 0 case; glibc has a hack which doesn't even work
          // in this case so it'll ignore the flag, and Bionic just
          // passes through the syscall and always ignores the flags.
          //
          // Instead, because we've already checked the requested
          // r/w/x bits against the policy, just return success if the
          // file exists and hope that's close enough.

The code that sets euid/egid is being removed in bug 1382099, so this could also go away and just implement access() with access().

[1] https://en.wikipedia.org/wiki/Confused_deputy_problem 
[2] On Linux, uid/gid/etc. can be thread-local; see also bug 970676.
[3] On Android/B2G, each running app / content process gets a separate uid.
[4] https://searchfox.org/mozilla-central/rev/a83a4b68974aecaaacdf25145420e0fe97b7aa22/security/sandbox/linux/broker/SandboxBroker.cpp#607

Comment 1

[Jed Davis [:jld] ⟨⏰|UTC-7⟩ ⟦he/him⟧]

Fixed in https://hg.mozilla.org/mozilla-central/rev/c0838ed41842 (bug 1386279).