Linux sandbox: add basic Yama support

Linux sandbox: add basic Yama support

This CL adds basic detection of whether or not the Yama LSM module is
available, and allow to opt-in and opt-out of its protections.

BUG=349673
R=jorgelo@chromium.org, keescook@chromium.org

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=255475

[jln (very slow on Chromium), 2014-03-06 00:25:08]

Kees, could you please check if yama* makes sense?

I'm adding more meaningful unittests before committing, but a first look would
be useful.

[Kees Cook, 2014-03-06 00:49:30]

lgtm

https://codereview.chromium.org/188193002/diff/1/sandbox/linux/services/yama.cc
File sandbox/linux/services/yama.cc (right):

https://codereview.chromium.org/188193002/diff/1/sandbox/linux/services/yama....
sandbox/linux/services/yama.cc:29: set_ptracer_arg = PR_SET_PTRACER_ANY;
PR_SET_PTRACE_ANY is an extreme disabling state -- it turns off Yama entirely
for the process. This is fine to control, but I feel like the function name and
comments do not describe this state fully. Perhaps the comments should mention
the side-effect of calling with enabled=true (i.e. it will clear any previously
specified ptracer pid).

[Kees Cook, 2014-03-06 00:50:09]

On 2014/03/06 00:49:30, Kees Cook wrote:
> lgtm

Though, I wonder if the thread_helpers stuff should be a separate CL, since it
seems unrelated.

[mdempsky, 2014-03-06 01:56:27]

https://codereview.chromium.org/188193002/diff/1/sandbox/linux/services/yama.cc
File sandbox/linux/services/yama.cc (right):

https://codereview.chromium.org/188193002/diff/1/sandbox/linux/services/yama....
sandbox/linux/services/yama.cc:46: const char kProcfsKernelSysPath[] =
"/proc/sys/kernel/";
Can this be made static?  (kPtraceScopePath below too.)

https://codereview.chromium.org/188193002/diff/1/sandbox/linux/services/yama....
sandbox/linux/services/yama.cc:48: int ret = stat(kProcfsKernelSysPath, &buf);
Since you don't care about the 'stat' buf, would access(path, F_OK) work here?

[jln (very slow on Chromium), 2014-03-06 05:06:08]

Thanks! I've addressed the nits and added testing.

I created a new ScopedProcess class for toy processes which we can use in tests.
It takes a callback as a parameter, which makes it pretty handy.

Kees: could you please take another quick look for Yama stuff?

Jorge: could you please review this CL?

mdempsky: feel free to take a look as well if you want to.

https://chromiumcodereview.appspot.com/188193002/diff/1/sandbox/linux/service...
File sandbox/linux/services/yama.cc (right):

https://chromiumcodereview.appspot.com/188193002/diff/1/sandbox/linux/service...
sandbox/linux/services/yama.cc:29: set_ptracer_arg = PR_SET_PTRACER_ANY;
On 2014/03/06 00:49:30, Kees Cook wrote:
> PR_SET_PTRACE_ANY is an extreme disabling state -- it turns off Yama entirely
> for the process. This is fine to control, but I feel like the function name
and
> comments do not describe this state fully. Perhaps the comments should mention
> the side-effect of calling with enabled=true (i.e. it will clear any
previously
> specified ptracer pid).

This is mostly for testing.

I've added a comment to this effect in the header file. Let me know if this
works.

https://chromiumcodereview.appspot.com/188193002/diff/1/sandbox/linux/service...
sandbox/linux/services/yama.cc:46: const char kProcfsKernelSysPath[] =
"/proc/sys/kernel/";
On 2014/03/06 01:56:28, mdempsky wrote:
> Can this be made static?  (kPtraceScopePath below too.)

Done.

https://chromiumcodereview.appspot.com/188193002/diff/1/sandbox/linux/service...
sandbox/linux/services/yama.cc:48: int ret = stat(kProcfsKernelSysPath, &buf);
On 2014/03/06 01:56:28, mdempsky wrote:
> Since you don't care about the 'stat' buf, would access(path, F_OK) work here?

Done.

[Jorge Lucangeli Obes, 2014-03-06 15:42:09]

https://chromiumcodereview.appspot.com/188193002/diff/110001/sandbox/linux/se...
File sandbox/linux/services/yama.cc (right):

https://chromiumcodereview.appspot.com/188193002/diff/110001/sandbox/linux/se...
sandbox/linux/services/yama.cc:70: return SetYamaPtracersRestriction(false /*
enabled */);
/* disabled */

https://chromiumcodereview.appspot.com/188193002/diff/110001/sandbox/linux/se...
File sandbox/linux/services/yama.h (right):

https://chromiumcodereview.appspot.com/188193002/diff/110001/sandbox/linux/se...
sandbox/linux/services/yama.h:41: // Checks if Yama is currently in enforcing
for the machine (not the current
"in enforcing mode"

https://chromiumcodereview.appspot.com/188193002/diff/110001/sandbox/linux/se...
File sandbox/linux/services/yama_unittests.cc (right):

https://chromiumcodereview.appspot.com/188193002/diff/110001/sandbox/linux/se...
sandbox/linux/services/yama_unittests.cc:24: void WaitForever() {
test_utils.{h|cc} for these functions eventually?

[jln (very slow on Chromium), 2014-03-06 21:43:46]

Thanks, PTAL!

I've also made some changes to ScopedProcess to support explicit synchronization
to wait for the closure to finish running, which the Yama unittests needed.

https://chromiumcodereview.appspot.com/188193002/diff/110001/sandbox/linux/se...
File sandbox/linux/services/yama.cc (right):

https://chromiumcodereview.appspot.com/188193002/diff/110001/sandbox/linux/se...
sandbox/linux/services/yama.cc:70: return SetYamaPtracersRestriction(false /*
enabled */);
On 2014/03/06 15:42:09, Jorge Lucangeli Obes wrote:
> /* disabled */

I meant to document the variable name. I've made it more explicit.

https://chromiumcodereview.appspot.com/188193002/diff/110001/sandbox/linux/se...
File sandbox/linux/services/yama.h (right):

https://chromiumcodereview.appspot.com/188193002/diff/110001/sandbox/linux/se...
sandbox/linux/services/yama.h:41: // Checks if Yama is currently in enforcing
for the machine (not the current
On 2014/03/06 15:42:09, Jorge Lucangeli Obes wrote:
> "in enforcing mode"

Done.

https://chromiumcodereview.appspot.com/188193002/diff/110001/sandbox/linux/se...
File sandbox/linux/services/yama_unittests.cc (right):

https://chromiumcodereview.appspot.com/188193002/diff/110001/sandbox/linux/se...
sandbox/linux/services/yama_unittests.cc:24: void WaitForever() {
On 2014/03/06 15:42:09, Jorge Lucangeli Obes wrote:
> test_utils.{h|cc} for these functions eventually?

I've removed this anyways now.

[Jorge Lucangeli Obes, 2014-03-06 21:49:40]

On 2014/03/06 21:43:46, jln wrote:
> Thanks, PTAL!
> 
> I've also made some changes to ScopedProcess to support explicit
synchronization
> to wait for the closure to finish running, which the Yama unittests needed.
> 
>
https://chromiumcodereview.appspot.com/188193002/diff/110001/sandbox/linux/se...
> File sandbox/linux/services/yama.cc (right):
> 
>
https://chromiumcodereview.appspot.com/188193002/diff/110001/sandbox/linux/se...
> sandbox/linux/services/yama.cc:70: return SetYamaPtracersRestriction(false /*
> enabled */);
> On 2014/03/06 15:42:09, Jorge Lucangeli Obes wrote:
> > /* disabled */
> 
> I meant to document the variable name. I've made it more explicit.
> 
>
https://chromiumcodereview.appspot.com/188193002/diff/110001/sandbox/linux/se...
> File sandbox/linux/services/yama.h (right):
> 
>
https://chromiumcodereview.appspot.com/188193002/diff/110001/sandbox/linux/se...
> sandbox/linux/services/yama.h:41: // Checks if Yama is currently in enforcing
> for the machine (not the current
> On 2014/03/06 15:42:09, Jorge Lucangeli Obes wrote:
> > "in enforcing mode"
> 
> Done.
> 
>
https://chromiumcodereview.appspot.com/188193002/diff/110001/sandbox/linux/se...
> File sandbox/linux/services/yama_unittests.cc (right):
> 
>
https://chromiumcodereview.appspot.com/188193002/diff/110001/sandbox/linux/se...
> sandbox/linux/services/yama_unittests.cc:24: void WaitForever() {
> On 2014/03/06 15:42:09, Jorge Lucangeli Obes wrote:
> > test_utils.{h|cc} for these functions eventually?
> 
> I've removed this anyways now.

lgtm

[jln (very slow on Chromium), 2014-03-06 22:36:32]

The CQ bit was checked by jln@chromium.org

[commit-bot: I haz the power, 2014-03-06 22:50:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/jln@chromium.org/188193002/220001

[jln (very slow on Chromium), 2014-03-07 00:24:55]

Committed patchset #7 manually as r255475 (presubmit successful).

[commit-bot: I haz the power, 2014-03-07 00:34:29]

Try jobs failed on following builders: mac_chromium_compile_dbg