Just so we're on the same page: Sadrul, how do you expect that mus-gpu processes is going to be launched? Through the service manager itself? Or will it still go through some sort of process host?
Hmm. Not following. This would seem to create a service within the same process as the service manager? I didn't see how this flows into launching an external process.
One way of looking at this is to realize that //sandbox provides a mechanism but //content/common/sandbox provides the policy. Unfortunately, policies are complicated, platform-specific, and hard to ship around.
If we want to (re-)use the policies outside of content, we risk mixing the two, which would violate a design pattern in some sense. We could have a separate //sandbox_policy layer to wrap //sandbox, but that seems excessive.
Maybe just //sandbox/policy.
Note that the policies, apart from their names, are fairly independent of //content concepts, with a number of wrinkles like the mac code using resource bundles (a //src/ui/base concept as it turns out).
Continuing the discussion, I'd expect //sandbox/policy to have mac, win, and linux sub-directories.
//sandbox/policy/linux, for example would get these files from content/
bpf_cros_amd_gpu_policy_linux.cc
bpf_cros_amd_gpu_policy_linux.h
bpf_cros_arm_gpu_policy_linux.cc
bpf_cros_arm_gpu_policy_linux.h
bpf_gpu_policy_linux.cc
bpf_gpu_policy_linux.h
bpf_ppapi_policy_linux.cc
bpf_ppapi_policy_linux.h
bpf_renderer_policy_linux.cc
bpf_renderer_policy_linux.h
bpf_utility_policy_linux.cc
bpf_utility_policy_linux.h
bpf_widevine_policy_linux.cc
bpf_widevine_policy_linux.h
OWNERS
sandbox_bpf_base_policy_linux.cc
sandbox_bpf_base_policy_linux.h
sandbox_debug_handling_linux.cc
sandbox_debug_handling_linux.h
sandbox_init_linux.cc
sandbox_linux.cc
sandbox_linux.h
sandbox_seccomp_bpf_linux.c
//sandbox/policy/mac would get these files from content:
content/utility/utility.sb
content/renderer/renderer_v2.sb
content/renderer/renderer.sb
content/ppapi_plugin/ppapi.sb
content/browser/gpu.sb
content/common/common.sb
chrome/common/nacl_loader.sb
content/common/sandbox_init_mac.cc
content/common/sandbox_init_mac.h
content/common/sandbox_mac_diraccess_unittest.mm
content/common/sandbox_mac_fontloading_unittest.mm
content/common/sandbox_mac.h
content/common/sandbox_mac.mm
content/common/sandbox_mac_system_access_unittest.mm
content/common/sandbox_mac_unittest_helper.h
content/common/sandbox_mac_unittest_helper.mm
@jam asks:
Can you expand on why we would want src/sandbox to have knowledge of things like renderer, utility, network sandboxing? The philosophy so far has been that higher layers configure it depending on their knowledge of what their processes need.
My take is that such knowledge is arcane, verbose to express, platform-specific, unrelated to the actual tasks being accomplished by the layer, and impossible to re-use without duplicating the code outside of content/. For example, having code in content need to understand BPF filter assembly methods is a failure of abstraction, and I'd like to have it not creep higher up into the codebase. To the extent that we can avoid platform-specific splits in these higher directories makes things simpler, I think.
It is unfortunate that at the present the sandbox policies, eg. bpf_renderer_policy_linux.h are named after how they are used, as opposed to what they do.
Ideally, we'd like to have something like a bpf_level_0 sandbox, a bfp_level_1 sandbox, etc. but there is at best a partial order (network and gpu need different capabilities neither which is a subset of each other).
Here's a thought experiment: if I were to change these names to be something like bpf_red_sanbox, bpf_blue_sandbox, and then have content code say that the renderer needs a "blue" sandbox, the code we would find in src/sandbox in such a world would be completely independent of any content/ concepts, and wouldn't include any content/ headers (except for pre sandbox hook, which will remain in content). My hope was that these would cluster, so that we'd wind up with N re-usable canonical sandbox types, but things may be too idiosyncratic for that to happen.
There's a nice facility for extending SANDBOX_TYPE_AFTER_LAST_TYPE beyond content, but as far as I can tell, it is only ever used on Mac, and for one type, which seems like more generality than is needed. My temptation would be to fold that one usage into this directory as well, to insulate that code from having to implement the same things.
Regarding pre-sandbox hooks, I think we've made a mistake by conflating a policy to apply to the kernel with a set of layer-specific steps in the same object. Splitting these apart may allow for more re-use -- the same kernel policy, for example against different set of warm-up steps for different process types.
One final point that I think validates this design is that the number of per-file=security OWNERS lines is vastly reduced as all the security sensitive files wind up consolidated in the same directory.
I share the concern about //sandbox knowing about (former) //content-related concepts, and there was some discussion about that on the review here: https://chromium-review.googlesource.com/c/chromium/src/+/676191/18/chrome/browser/chrome_content_browser_client.cc#2801.
From the above discussion, it sounds like //sandbox/policy was a sort of compromise place to put this stuff, but it indeed may not be the best place. Could this instead move to be closer to the service manager, maybe somewhere in //services?
I think it would be reasonable to define specific policies somewhere like
//services/service_manager/sandbox_policies.
FWIW though, I do still believe a worthwhile long-term goal is to move away
from broadly defined, named configurations like "network" etc, and move
toward a reasonable set of more granular sandbox features which manifests
can configure. It would be unfortunate if terms like "renderer" and
"utility" ended up as permanent fixtures in our service APIs. Is that part
of the plan here?
WRT granular privileges, we've gone back and forth on this a few times. The issue here is that things are not going to be consistent across plaftorms: just because network needs fiilesystem access on one platform doesn't mean that its going to require it on another. We wind up with a large number of platform-specific "ifdefs" in the manifests in that case, esp since some kinds of privileges will exit only on one platform or another.
I've currenly added "sandbox_type" to the manifest, with the idea that the lower layers would deduce what that means. I was hoping for some more generic naming than, renderer, as you mention but I haven't found it yet.
Comment 1 by sadrul@chromium.org
, May 29 2017