New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 889586 link

Starred by 1 user

Issue metadata

Status: Untriaged
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug

Blocking:
issue 649417



Sign in to add a comment

Add new tests to verify permissions needed by shill are properly granted

Project Member Reported by benchan@chromium.org, Sep 26

Issue description

With shill now running inside a sandbox, we need new automated tests to verify that permissions needed by shill are properly granted, which helps detect regressions if the sandbox or system is later updated. While we have existing tests to (indirectly) cover some of the permissions check, a more focused set of tests would be desired given the complexity of shill and its various different uses cases (WiFi, cellular, VPN, etc).

Here're a few areas we can check:
- D-Bus permissions - shill can access wpa_supplicant, ModemManager, etc over D-Bus
- File / device node permissions - /dev/ppp can be accessed by pppd spawned by shill, /var/cache/shill, etc can be accessed by shill
- Other capabilities like raw socket, rtnl, etc should be checked as well
 
Perhaps a more specific version of one of your bullets: previously-created stateful filesystems might have various /var/cache/shill/, etc., created and owned by root:root, and we need to make sure we continue to chown/chmod them appropriately for access by the shill user. This logic needs to stay intact indefinitely (or, as long as we continue to support upgrades from pre-M70).

That kind of test is probably even more important than basic "does a clean install work", because we're less likely to notice in typical manual tests, or even in M{X-1} to M{X} AU tests.
Naive question: for anyone attempting this, how we find existing utility/helper files that check for CAP_* privileges or access to nodes/files? 
I could only find: 

1. client/site_tests/security_Minijail0/src has some python -but that style doesn't all the other autotests I am used to (client-side autotests that require deploying a src/ directory on DUT and running make are also generally a pain to deal with and should be discouraged because they dont work well with test_that, but that's a separate thread). 

2. server/cros/servo/chrome_cr50.py has some checks. I don't understand capabilities enough to know if its different from what we want. 

Maybe its asking for a lot to want a for-dummies reference for security autotests, but trying in case it exists. 

@2: I wouldn't think it's too unreasonable to ask the security team to implement these security tests, so maybe you don't need a for-dummies reference ;)

I also wonder, since this isn't a suggestion for a full Wifi integration test (and so doesn't need to depend on most of our existing Wifi autotest frameworks), if maybe this should just be targeting tast. So the python / autotest questions aren't as relevant. See bug 871645.

But if we really need to check process CAP_*'s, it's not really that hard to check /proc/$pid/status in <insert your language> [1], rather than forking of shell scripts.

[1] Disclaimer: I've never written in Go.
Blocking: 649417
IMO, we should not re-enable sandboxing until we have some of these tests.
Labels: Enterprise-Triaged

Sign in to add a comment