Repository navigation
exec spawning: restore secondary zygote supplementary groups - #483
Open
inthewaves wants to merge 6 commits into
Open
inthewaves wants to merge 6 commits into
inthewaves wants to merge 6 commits into
Conversation
Extend the native signal probe to report supplementary groups. Verify that direct native service
launches and native app zygote children do not inherit the primary zygote readproc and reserved_disk
groups, with exec spawning both enabled and disabled.
NativeZygoteProcess does not pass the shared app GID that AppZygote requests when starting a native
app zygote, so conventional native app zygote children have no supplementary groups, unlike the
children of a Java app zygote. Require the same for exec-spawned replacements.
The native app zygote service bind also catches attempts to call setgroups() after the conventional
child has inherited the app zygote seccomp filter.
Test: atest
GosCompatSecureSpawnTests:SecureSpawn{En,Dis}abledHostTest#{nativeServiceHasNoSupplementaryGroups,nativeServiceUsingAppZygoteHasNoSupplementaryGroups}
Conventional Java app and WebView zygote children inherit the defining package's shared app GID from
their parent. Exec-spawned replacements start from the primary zygote instead and were retaining its
readproc and reserved_disk groups.
Mark app zygote replacements and request the defining package's shared app GID when app or WebView
zygote semantics are required and the caller did not provide an explicit group list. Preserve
nonempty group lists unchanged.
NativeZygoteProcess does not pass groups to native spawn requests, including the shared app GID that
AppZygote requests for a native app zygote. Native app zygote replacements therefore keep matching
conventional native app zygote children, which have no supplementary groups.
Test: atest
GosCompatSecureSpawnTests:SecureSpawnEnabledHostTest#{serviceUsingManagedAppZygoteHasExpectedSupplementaryGroups,webViewRendererHasExpectedSupplementaryGroups}
Add a non-exported activity and shared device helper that loads a local WebView page, identifies the single renderer created for the current provider, and passes its activity, provider metadata and PID to a focused assertion. Ignore new processes whose parent is a renderer, since a process forked by a renderer keeps the renderer's name until it execs and would otherwise look like a second renderer. Include the PID, parent PID, UID and name of each renderer in the failure message. Keep provider discovery, bounded page loading, process lookup and activity teardown in one place so WebView security and lifecycle tests contain only policy-specific behavior.
Check the supplementary groups of a Java service using a managed app zygote and of a WebView
renderer. Each must have only the defining package's shared app GID, which is the WebView provider's
for the renderer, with secure app spawning both enabled and disabled.
The disabled case uses the conventional app and WebView zygote paths as the control. The enabled
case catches exec-spawned replacements retaining the primary zygote readproc and reserved_disk
groups instead.
Test: atest
GosCompatSecureSpawnTests:SecureSpawn{En,Dis}abledHostTest#{serviceUsingManagedAppZygoteHasExpectedSupplementaryGroups,webViewRendererHasExpectedSupplementaryGroups}
Check the supplementary groups of the test app process and of an isolated Java service without
useAppZygote, with secure app spawning both enabled and disabled.
The app process must have only the groups that ProcessList computes for a package without permission
GIDs or external storage access: its shared app GID, cache app GID and user GID. The isolated
service receives no group list, so it must keep the primary zygote readproc and reserved_disk groups
in both modes, as it does with conventional zygote spawning.
The exec spawning group fixes do not change these processes. The checks catch a fix that does.
Test: atest
GosCompatSecureSpawnTests:SecureSpawn{En,Dis}abledHostTest#{appProcessHasExpectedSupplementaryGroups,isolatedServiceWithoutAppZygoteHasPrimaryZygoteSupplementaryGroups}
Check the parent process of each process that conventionally uses a secondary zygote: a service
using a managed app zygote, a native service with and without useAppZygote, and a WebView renderer.
With secure app spawning disabled, the parent must be the app zygote, native app zygote, standalone
native zygote or WebView zygote, and must have the supplementary groups that its children inherit.
This covers the zygote processes themselves, including the native app zygote having no groups
because NativeZygoteProcess does not pass the shared app GID that AppZygote requests.
With secure app spawning enabled, the parent must be a primary zygote with its readproc and
reserved_disk groups. This shows that the supplementary group tests ran against exec-spawned
replacements rather than conventional zygote children, which would pass them with the same groups.
Add getPid() to the native signal probe to locate native service processes.
Test: atest
GosCompatSecureSpawnTests:SecureSpawnDisabledHostTest#{serviceUsingManagedAppZygoteHasAppZygoteParent,nativeServiceUsingAppZygoteHasNativeAppZygoteParent,nativeServiceHasNativeZygoteParent,webViewRendererHasWebViewZygoteParent}
GosCompatSecureSpawnTests:SecureSpawnEnabledHostTest#{serviceUsingManagedAppZygoteHasPrimaryZygoteParent,nativeServiceUsingAppZygoteHasPrimaryZygoteParent,nativeServiceHasPrimaryZygoteParent,webViewRendererHasPrimaryZygoteParent}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Requires GrapheneOS/platform_system_zygote#6 for
nativeServiceHasNoSupplementaryGroups,nativeServiceUsingAppZygoteHasNoSupplementaryGroupsto passreadproc,reserved_disk(kept from the primary zygote)readproc,reserved_diskreadproc,reserved_diskreadproc,reserved_diskzygote_nexthas none)readproc,reserved_diskgids)3readproc,reserved_diskreadproc,reserved_diskWith the fix, every row matches the exec spawning off column.
Footnotes
From this PR.
ActiveServicesmarks app zygote replacements, andProcessListrequests the defining package's shared app GID for app zygote and WebView replacements when no group list was given. ↩ ↩2From system/zygote PR.
child_process::re_initializecallssetgroupswith exec spawning even for an empty list, clearing the groups inherited from the primary zygote. ↩ ↩2Upstream behavior:
NativeZygoteProcessnever passes the shared app GID thatAppZygoterequests when starting a native app zygote (jintArray gids = nullptrinandroid_os_NativeZygoteProcess.cpp).ProcessListalso requests the GID for native replacements, it is dropped the same way, so both modes match stock. If upstream starts passing it, both modes gain it together. ↩ ↩2 ↩3