New issue
Advanced search Search tips

Issue 652969 link

Starred by 0 users

Issue metadata

Status: Fixed
Owner:
Closed: May 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Feature



Sign in to add a comment

security_SandboxedServices: rework to not require minijail command line

Project Member Reported by vapier@chromium.org, Oct 5 2016

Issue description

there are cases where we run minijail, it sets up all the sandboxing, and then exits leaving behind its child process/server.  when security_SandboxedServices runs, it can only verify sandbox behavior by looking at minijail's command line, so when minijail exits, the test starts failing by claiming services aren't sandboxed.

let's rework the test to look directly at the service's active settings (user name, namespace settings, etc...) to see whether it actually has sandbox settings active.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Oct 7 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/e6cd0d000fe3e28e42b3f72d4162ef6ad82b7c68

commit e6cd0d000fe3e28e42b3f72d4162ef6ad82b7c68
Author: Mike Frysinger <vapier@chromium.org>
Date: Wed Oct 05 05:19:42 2016

security_SandboxedServices: simplify logging calls

We have local wrappers that don't really add a whole lot, so replace
them with direct logging calls to make them easier to understand.
This also fixes a pylint error about missing docstrings.

BUG= chromium:652969 
TEST=security_SandboxedServices still passes

Change-Id: Iea1c3ea4cc9ceed4ba2abcef99429194ec027a2e
Reviewed-on: https://chromium-review.googlesource.com/393586
Commit-Ready: Mike Frysinger <vapier@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org>

[modify] https://crrev.com/e6cd0d000fe3e28e42b3f72d4162ef6ad82b7c68/client/site_tests/security_SandboxedServices/security_SandboxedServices.py

Project Member

Comment 2 by bugdroid1@chromium.org, Oct 7 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/f533269554d7182018e4da62a6f18a6f094dba3e

commit f533269554d7182018e4da62a6f18a6f094dba3e
Author: Mike Frysinger <vapier@chromium.org>
Date: Wed Oct 05 03:00:27 2016

security_SandboxedServices: rework PS_FIELDS

This makes it easier to add fields and to see what's in here.
We also wanted it more often as a list than a string.

BUG= chromium:652969 
TEST=security_SandboxedServices still passes

Change-Id: Iadb0d313bb4f8a5563cc8a8225d4474b3f91c8b9
Reviewed-on: https://chromium-review.googlesource.com/393587
Commit-Ready: Mike Frysinger <vapier@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org>

[modify] https://crrev.com/f533269554d7182018e4da62a6f18a6f094dba3e/client/site_tests/security_SandboxedServices/security_SandboxedServices.py

Project Member

Comment 3 by bugdroid1@chromium.org, Oct 7 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/69b313618f85d837abe5f962dd5f1d5781317bf0

commit 69b313618f85d837abe5f962dd5f1d5781317bf0
Author: Mike Frysinger <vapier@chromium.org>
Date: Wed Oct 05 04:46:28 2016

security_SandboxedServices: increase & document user max setting

Since uid's can be 32-bits, make sure we have enough space for the
largest 32-bit number.  When working with user namespaces, we might
not have a name for the group to look up.

BUG= chromium:652969 
TEST=security_SandboxedServices still passes

Change-Id: I069c1c8f156df46e92da24f87c138b751b1ba909
Reviewed-on: https://chromium-review.googlesource.com/393588
Commit-Ready: Mike Frysinger <vapier@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org>

[modify] https://crrev.com/69b313618f85d837abe5f962dd5f1d5781317bf0/client/site_tests/security_SandboxedServices/security_SandboxedServices.py

Project Member

Comment 4 by bugdroid1@chromium.org, Oct 9 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/170f3fb4f93d4a2f123e0acd64143b302e2dd4cc

commit 170f3fb4f93d4a2f123e0acd64143b302e2dd4cc
Author: Mike Frysinger <vapier@chromium.org>
Date: Wed Oct 05 04:40:06 2016

security_SandboxedServices: start collecting active group details

We had an option for checking the minijail group option was used,
but not whether the group was actually active.

BUG= chromium:652969 
TEST=security_SandboxedServices still passes

Change-Id: I5bf24ab459672d4bb49c94bf2e1985b061d74963
Reviewed-on: https://chromium-review.googlesource.com/393589
Commit-Ready: Mike Frysinger <vapier@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org>

[modify] https://crrev.com/170f3fb4f93d4a2f123e0acd64143b302e2dd4cc/client/site_tests/security_SandboxedServices/baseline
[modify] https://crrev.com/170f3fb4f93d4a2f123e0acd64143b302e2dd4cc/client/site_tests/security_SandboxedServices/security_SandboxedServices.py

Project Member

Comment 5 by bugdroid1@chromium.org, Oct 10 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/a6a7bf040cee8a88dc535d0932a4d39201db4a45

commit a6a7bf040cee8a88dc535d0932a4d39201db4a45
Author: Mike Frysinger <vapier@chromium.org>
Date: Wed Oct 05 05:11:29 2016

security_SandboxedServices: extract settings from /proc not minijail

Some minijail invocations will exit once they've sandboxed the process,
so there's no minijail command line left to check against.  Plus, it's
no guarantee that the child is still sandboxed (it could have changed
after launching).  Extract all the relevant settings directly from the
/proc files so we don't have to rely on minijail at all.

BUG= chromium:652969 
TEST=security_SandboxedServices still passes

Change-Id: Iebcea2da239f6aedeb4b01ebb37f047b5fd48340
Reviewed-on: https://chromium-review.googlesource.com/393590
Commit-Ready: Mike Frysinger <vapier@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/a6a7bf040cee8a88dc535d0932a4d39201db4a45/client/site_tests/security_SandboxedServices/baseline
[modify] https://crrev.com/a6a7bf040cee8a88dc535d0932a4d39201db4a45/client/site_tests/security_SandboxedServices/security_SandboxedServices.py

Project Member

Comment 6 by bugdroid1@chromium.org, Oct 17 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/7deb318ada5c6dcf9b1d8ab3c2c10124df78c67a

commit 7deb318ada5c6dcf9b1d8ab3c2c10124df78c67a
Author: Mike Frysinger <vapier@chromium.org>
Date: Mon Oct 10 05:03:51 2016

security_SandboxedServices: reject new unknown root services

We currently warn about all new services which means people can add them
running as root and no one will notice.  Let's reject those assuming that
they aren't sandboxed at all.  If they really need to be run as root, they
can explicitly whitelist the service in the baseline.

We also update the baseline to match the few new programs that have been
added (at least on samus), especially the ones running as root.

BUG= chromium:652969 
TEST=security_SandboxedServices still passes

Change-Id: I2498aa0a78142899c87d85475dae36f1c1532ba1
Reviewed-on: https://chromium-review.googlesource.com/395730
Commit-Ready: Mike Frysinger <vapier@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org>

[modify] https://crrev.com/7deb318ada5c6dcf9b1d8ab3c2c10124df78c67a/client/site_tests/security_SandboxedServices/security_SandboxedServices.py
[modify] https://crrev.com/7deb318ada5c6dcf9b1d8ab3c2c10124df78c67a/client/site_tests/security_SandboxedServices/baseline
[add] https://crrev.com/7deb318ada5c6dcf9b1d8ab3c2c10124df78c67a/client/site_tests/security_SandboxedServices/baseline.x86-zgb
[add] https://crrev.com/7deb318ada5c6dcf9b1d8ab3c2c10124df78c67a/client/site_tests/security_SandboxedServices/baseline.lakitu

Project Member

Comment 7 by bugdroid1@chromium.org, Oct 20 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/d84eafb620de54d79fd39a01d46014800609447a

commit d84eafb620de54d79fd39a01d46014800609447a
Author: Mike Frysinger <vapier@chromium.org>
Date: Thu Oct 20 01:51:01 2016

security_SandboxedServices: ignore supplemental awk exit status

The time between the shell glob expansion (/proc/*/status) and awk
actually reading the file might see a process exit and the status
file disappear:
awk: cannot open /proc/31530/status (No such file or directory)

Since we only track/care about long running processes, ignore a
fail exit code.  It means we might ignore valid failures, but
there's no clear way to differentiate between legit failures.

BUG= chromium:652969 
TEST=precq still passes

Change-Id: Id55f92273728104fdb724ee6343d159a7ff043aa
Reviewed-on: https://chromium-review.googlesource.com/401178
Reviewed-by: Brian Norris <briannorris@chromium.org>
Reviewed-by: Luigi Semenzato <semenzato@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>
Commit-Queue: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/d84eafb620de54d79fd39a01d46014800609447a/client/site_tests/security_SandboxedServices/security_SandboxedServices.py

Project Member

Comment 8 by bugdroid1@chromium.org, Oct 21 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/195ea20f33cb0090845761206ea7cc463eb45a9d

commit 195ea20f33cb0090845761206ea7cc463eb45a9d
Author: Mike Frysinger <vapier@chromium.org>
Date: Wed Oct 19 16:42:26 2016

security_SandboxedServices: filter out comments from input files

The csv module doesn't handle comments, so filter out the entries manually.

BUG= chromium:652969 
TEST=precq still passes

Change-Id: Ie92167977dfb6d8e08eb1ed1e4075843424f2444
Reviewed-on: https://chromium-review.googlesource.com/400539
Commit-Ready: Mike Frysinger <vapier@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Cheng-Yi Chiang <cychiang@chromium.org>
Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org>

[modify] https://crrev.com/195ea20f33cb0090845761206ea7cc463eb45a9d/client/site_tests/security_SandboxedServices/security_SandboxedServices.py

Project Member

Comment 9 by bugdroid1@chromium.org, Oct 22 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/0113b5b30b88a280feb0f26173dbf5f0def5a2e3

commit 0113b5b30b88a280feb0f26173dbf5f0def5a2e3
Author: Mike Frysinger <vapier@chromium.org>
Date: Wed Oct 12 21:53:56 2016

security_SandboxedServices: rework status file parsing again

In order to support newer fields (like CapAmb), rework the parsing logic
to handle missing ones.  This requires passing back keyed values from the
device, and then parsing them as a dict in the test.

The advantage here is that we no longer rely on the order.

BUG= chromium:652969 ,  chromium:654531 
TEST=security_SandboxedServices still passes

Change-Id: If29d94fb6a9aaa63ba6ee4beff4165cc910cf86f
Reviewed-on: https://chromium-review.googlesource.com/400038
Commit-Ready: Mike Frysinger <vapier@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>
Reviewed-by: Jorge Lucangeli Obes <jorgelo@chromium.org>

[modify] https://crrev.com/0113b5b30b88a280feb0f26173dbf5f0def5a2e3/client/site_tests/security_SandboxedServices/security_SandboxedServices.py

Status: Fixed (was: Started)
Labels: VerifyIn-61

Comment 12 Deleted

Status: Fixed (was: Archived)

Sign in to add a comment