Add IsSandboxed() function to seatbelt wrapper.

Add IsSandboxed() function to seatbelt wrapper.

This adds a function that checks whether or not the process is currently
sandboxed. It uses the underlying sandbox_check() functionality,
which is not yet further exposed.

BUG=689306
Review-Url: https://codereview.chromium.org/2914693002
Cr-Commit-Position: refs/heads/master@{#476370}
Committed: https://chromium.googlesource.com/chromium/src/+/60afdae0e313e3af409e9c13776520525f34c0bc

[Greg K, 2017-05-30 21:30:19]

The CQ bit was checked by kerrnel@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-05-30 21:30:53]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-30 21:52:33]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-30 21:52:34]

Dry run: This issue passed the CQ dry run.

[Greg K, 2017-05-30 21:53:31]

kerrnel@chromium.org changed reviewers:
+ rsesek@chromium.org

[Greg K, 2017-05-30 21:53:31]

PTAL.

Thanks,

Greg

[Robert Sesek, 2017-05-31 17:31:08]

https://codereview.chromium.org/2914693002/diff/1/sandbox/mac/seatbelt.h
File sandbox/mac/seatbelt.h (right):

https://codereview.chromium.org/2914693002/diff/1/sandbox/mac/seatbelt.h#newc...
sandbox/mac/seatbelt.h:16: enum sandbox_filter_type {
Why is this extern C? I don't see this in /usr/include.

This could instead be:

enum class SandboxFilter {
  NONE,
  PATH,
  GLOBAL_NAME,
…
};

https://codereview.chromium.org/2914693002/diff/1/sandbox/mac/seatbelt.h#newc...
sandbox/mac/seatbelt.h:35: int sandbox_check(pid_t pid,
Rather than exposing this as a C function, let's wrap it via a Seatbelt static
method.

[Greg K, 2017-05-31 17:36:25]

https://codereview.chromium.org/2914693002/diff/1/sandbox/mac/seatbelt.h
File sandbox/mac/seatbelt.h (right):

https://codereview.chromium.org/2914693002/diff/1/sandbox/mac/seatbelt.h#newc...
sandbox/mac/seatbelt.h:35: int sandbox_check(pid_t pid,
On 2017/05/31 17:31:08, Robert Sesek wrote:
> Rather than exposing this as a C function, let's wrap it via a Seatbelt static
> method.

I wanted to do this, but I'm not sure how to pass along the "..." to the C
function. I can convert the `...` to a va_list, but I'm not aware of a version
of sandbox_check() that consumes a va_list. Are they the same thing?

[Robert Sesek, 2017-05-31 18:09:58]

https://codereview.chromium.org/2914693002/diff/1/sandbox/mac/seatbelt.h
File sandbox/mac/seatbelt.h (right):

https://codereview.chromium.org/2914693002/diff/1/sandbox/mac/seatbelt.h#newc...
sandbox/mac/seatbelt.h:35: int sandbox_check(pid_t pid,
On 2017/05/31 17:36:25, Greg K wrote:
> On 2017/05/31 17:31:08, Robert Sesek wrote:
> > Rather than exposing this as a C function, let's wrap it via a Seatbelt
static
> > method.
> 
> I wanted to do this, but I'm not sure how to pass along the "..." to the C
> function. I can convert the `...` to a va_list, but I'm not aware of a version
> of sandbox_check() that consumes a va_list. Are they the same thing?

Argh yeah the varargs is frustrating. Unless there were a variant that takes a
va_list (I don't see one...) you can't forward a varargs function. Do you intend
to use the varargs, because if not, then the Seatbelt method could simply not
take it. If you do, you could also overload it with non-varargs arguments, e.g.:

Seatbelt::Check(pid_t, const char* op, SandboxFilter)
Seatbelt::Check(pid_t, const char* op, SandboxFilter, const char* right)

[Greg K, 2017-05-31 18:14:37]

https://codereview.chromium.org/2914693002/diff/1/sandbox/mac/seatbelt.h
File sandbox/mac/seatbelt.h (right):

https://codereview.chromium.org/2914693002/diff/1/sandbox/mac/seatbelt.h#newc...
sandbox/mac/seatbelt.h:35: int sandbox_check(pid_t pid,
On 2017/05/31 18:09:58, Robert Sesek wrote:
> On 2017/05/31 17:36:25, Greg K wrote:
> > On 2017/05/31 17:31:08, Robert Sesek wrote:
> > > Rather than exposing this as a C function, let's wrap it via a Seatbelt
> static
> > > method.
> > 
> > I wanted to do this, but I'm not sure how to pass along the "..." to the C
> > function. I can convert the `...` to a va_list, but I'm not aware of a
version
> > of sandbox_check() that consumes a va_list. Are they the same thing?
> 
> Argh yeah the varargs is frustrating. Unless there were a variant that takes a
> va_list (I don't see one...) you can't forward a varargs function. Do you
intend
> to use the varargs, because if not, then the Seatbelt method could simply not
> take it. If you do, you could also overload it with non-varargs arguments,
e.g.:
> 
> Seatbelt::Check(pid_t, const char* op, SandboxFilter)
> Seatbelt::Check(pid_t, const char* op, SandboxFilter, const char* right)

Right now I only plan to use it to check if it's sandboxed. Maybe I should just
add Seatbelt::IsSandboxed() and expose the generic function later if
appropriate.

[Robert Sesek, 2017-05-31 18:19:08]

https://codereview.chromium.org/2914693002/diff/1/sandbox/mac/seatbelt.h
File sandbox/mac/seatbelt.h (right):

https://codereview.chromium.org/2914693002/diff/1/sandbox/mac/seatbelt.h#newc...
sandbox/mac/seatbelt.h:35: int sandbox_check(pid_t pid,
On 2017/05/31 18:14:37, Greg K wrote:
> On 2017/05/31 18:09:58, Robert Sesek wrote:
> > On 2017/05/31 17:36:25, Greg K wrote:
> > > On 2017/05/31 17:31:08, Robert Sesek wrote:
> > > > Rather than exposing this as a C function, let's wrap it via a Seatbelt
> > static
> > > > method.
> > > 
> > > I wanted to do this, but I'm not sure how to pass along the "..." to the C
> > > function. I can convert the `...` to a va_list, but I'm not aware of a
> version
> > > of sandbox_check() that consumes a va_list. Are they the same thing?
> > 
> > Argh yeah the varargs is frustrating. Unless there were a variant that takes
a
> > va_list (I don't see one...) you can't forward a varargs function. Do you
> intend
> > to use the varargs, because if not, then the Seatbelt method could simply
not
> > take it. If you do, you could also overload it with non-varargs arguments,
> e.g.:
> > 
> > Seatbelt::Check(pid_t, const char* op, SandboxFilter)
> > Seatbelt::Check(pid_t, const char* op, SandboxFilter, const char* right)
> 
> Right now I only plan to use it to check if it's sandboxed. Maybe I should
just
> add Seatbelt::IsSandboxed() and expose the generic function later if
> appropriate.

That sounds good. Keeping APIs minimal is always the right call.

[Greg K, 2017-06-01 00:17:42]

The CQ bit was checked by kerrnel@chromium.org to run a CQ dry run

[Greg K, 2017-06-01 00:18:21]

The CQ bit was checked by kerrnel@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-01 00:18:43]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-01 00:41:42]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-01 00:41:43]

Dry run: This issue passed the CQ dry run.

[Greg K, 2017-06-01 17:54:02]

Description was changed from

==========
Add sandbox_check function to seatbelt wrapper.

Adds the sandbox_check function to the seatbelt wrapper files.

BUG=689306
==========

to

==========
Add IsSandboxed() function to seatbelt wrapper.

This adds a function that checks whether or not the process is currently
sandboxed. It uses the underlying sandbox_check() functionality, which is not
yet further exposed.

BUG=689306
==========

[Greg K, 2017-06-01 18:13:26]

Description was changed from

==========
Add IsSandboxed() function to seatbelt wrapper.

This adds a function that checks whether or not the process is currently
sandboxed. It uses the underlying sandbox_check() functionality, which is not
yet further exposed.

BUG=689306
==========

to

==========
Add IsSandboxed() function to seatbelt wrapper.

This adds a function that checks whether or not the process is currently
sandboxed. It uses the underlying sandbox_check() functionality,
which is not yet further exposed.

BUG=689306
==========

[Greg K, 2017-06-01 18:16:46]

The CQ bit was checked by kerrnel@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-01 18:17:18]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Greg K, 2017-06-01 18:27:46]

On 2017/05/31 18:19:08, Robert Sesek wrote:
> https://codereview.chromium.org/2914693002/diff/1/sandbox/mac/seatbelt.h
> File sandbox/mac/seatbelt.h (right):
> 
>
https://codereview.chromium.org/2914693002/diff/1/sandbox/mac/seatbelt.h#newc...
> sandbox/mac/seatbelt.h:35: int sandbox_check(pid_t pid,
> On 2017/05/31 18:14:37, Greg K wrote:
> > On 2017/05/31 18:09:58, Robert Sesek wrote:
> > > On 2017/05/31 17:36:25, Greg K wrote:
> > > > On 2017/05/31 17:31:08, Robert Sesek wrote:
> > > > > Rather than exposing this as a C function, let's wrap it via a
Seatbelt
> > > static
> > > > > method.
> > > > 
> > > > I wanted to do this, but I'm not sure how to pass along the "..." to the
C
> > > > function. I can convert the `...` to a va_list, but I'm not aware of a
> > version
> > > > of sandbox_check() that consumes a va_list. Are they the same thing?
> > > 
> > > Argh yeah the varargs is frustrating. Unless there were a variant that
takes
> a
> > > va_list (I don't see one...) you can't forward a varargs function. Do you
> > intend
> > > to use the varargs, because if not, then the Seatbelt method could simply
> not
> > > take it. If you do, you could also overload it with non-varargs arguments,
> > e.g.:
> > > 
> > > Seatbelt::Check(pid_t, const char* op, SandboxFilter)
> > > Seatbelt::Check(pid_t, const char* op, SandboxFilter, const char* right)
> > 
> > Right now I only plan to use it to check if it's sandboxed. Maybe I should
> just
> > add Seatbelt::IsSandboxed() and expose the generic function later if
> > appropriate.
> 
> That sounds good. Keeping APIs minimal is always the right call.

PTAL. I switched to IsSandboxed() and noticed that there was no documentation
for the class methods.

[Robert Sesek, 2017-06-01 18:34:08]

LGTM

[commit-bot: I haz the power, 2017-06-01 18:38:11]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-06-01 18:38:13]

Dry run: This issue passed the CQ dry run.

[Greg K, 2017-06-01 18:39:54]

The CQ bit was checked by kerrnel@chromium.org

[commit-bot: I haz the power, 2017-06-01 18:40:29]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Greg K, 2017-06-01 18:42:09]

https://codereview.chromium.org/2914693002/diff/1/sandbox/mac/seatbelt.h
File sandbox/mac/seatbelt.h (right):

https://codereview.chromium.org/2914693002/diff/1/sandbox/mac/seatbelt.h#newc...
sandbox/mac/seatbelt.h:35: int sandbox_check(pid_t pid,
On 2017/05/31 18:19:08, Robert Sesek wrote:
> On 2017/05/31 18:14:37, Greg K wrote:
> > On 2017/05/31 18:09:58, Robert Sesek wrote:
> > > On 2017/05/31 17:36:25, Greg K wrote:
> > > > On 2017/05/31 17:31:08, Robert Sesek wrote:
> > > > > Rather than exposing this as a C function, let's wrap it via a
Seatbelt
> > > static
> > > > > method.
> > > > 
> > > > I wanted to do this, but I'm not sure how to pass along the "..." to the
C
> > > > function. I can convert the `...` to a va_list, but I'm not aware of a
> > version
> > > > of sandbox_check() that consumes a va_list. Are they the same thing?
> > > 
> > > Argh yeah the varargs is frustrating. Unless there were a variant that
takes
> a
> > > va_list (I don't see one...) you can't forward a varargs function. Do you
> > intend
> > > to use the varargs, because if not, then the Seatbelt method could simply
> not
> > > take it. If you do, you could also overload it with non-varargs arguments,
> > e.g.:
> > > 
> > > Seatbelt::Check(pid_t, const char* op, SandboxFilter)
> > > Seatbelt::Check(pid_t, const char* op, SandboxFilter, const char* right)
> > 
> > Right now I only plan to use it to check if it's sandboxed. Maybe I should
> just
> > add Seatbelt::IsSandboxed() and expose the generic function later if
> > appropriate.
> 
> That sounds good. Keeping APIs minimal is always the right call.

Done.

[commit-bot: I haz the power, 2017-06-01 18:46:33]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1496342394238920,
"parent_rev": "04b5424ac2e113a8ddbd18584b0e86a3a03299ae", "commit_rev":
"60afdae0e313e3af409e9c13776520525f34c0bc"}

[commit-bot: I haz the power, 2017-06-01 18:46:41]

Description was changed from

==========
Add IsSandboxed() function to seatbelt wrapper.

This adds a function that checks whether or not the process is currently
sandboxed. It uses the underlying sandbox_check() functionality,
which is not yet further exposed.

BUG=689306
==========

to

==========
Add IsSandboxed() function to seatbelt wrapper.

This adds a function that checks whether or not the process is currently
sandboxed. It uses the underlying sandbox_check() functionality,
which is not yet further exposed.

BUG=689306

Review-Url: https://codereview.chromium.org/2914693002
Cr-Commit-Position: refs/heads/master@{#476370}
Committed:
https://chromium.googlesource.com/chromium/src/+/60afdae0e313e3af409e9c137765...
==========

[commit-bot: I haz the power, 2017-06-01 18:46:43]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/60afdae0e313e3af409e9c137765...