mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From James Peach <jpe...@apache.org>
Subject Re: Review Request 59552: Add support for explicitly setting bounding capabilities.
Date Sun, 11 Jun 2017 18:55:38 GMT


> On June 11, 2017, 6:11 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/launch.cpp
> > Lines 642-643 (patched)
> > <https://reviews.apache.org/r/59552/diff/3/?file=1747441#file1747441line642>
> >
> >     Let's make sure inheritable is always a subset of bounding. This prevents a
container from getting permitted capabilities beyond its bounding set by crafting an executable
with proper `F(inheritable)` set.

Right below this, we force inheritable to be the same as bounding to address this issue. I
think this was a stale diff :-/


> On June 11, 2017, 6:11 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/linux/capabilities.cpp
> > Lines 91-92 (patched)
> > <https://reviews.apache.org/r/59552/diff/4/?file=1747634#file1747634line93>
> >
> >     I wouldn't do that here. I think the goal is to allow framework to specify bounding
as well in the future. I would set bounding below.
> >     
> >     Currently, framework cannot set bounding. We'll always use the operator specified
bounding if set. If not set, set bounding to effective if set.
> >     
> >     ```
> >     if (containerConfig.has_container_info() ...) {
> >       effective = ...;
> >     }
> >     
> >     if (effective.isNone()) {
> >       effective = flags.allowed_capabilities;
> >     }
> >     
> >     // NOTE: Currently, we do not allow framework to set
> >     // bounding capabilities separately. Therefore, it'll
> >     // always be what the operator has specified.
> >     bounding = flags.bounding_capabilities;
> >     
> >     // NOTE: This is for backwards compatibility.
> >     if (effective.isSome() && bounding.isNone()) {
> >       bounding = effective;
> >     }
> >     ```

This is done because the `effective` set it guaranteed to be equal or smaller than the `bounding`
set. If we use the operator `bounding_capabilities` here, then the task can gain more privilege
than the framework specifies. If the framework specifies a set of capabilities, it should
get exactly that and no more.


> On June 11, 2017, 6:11 p.m., Jie Yu wrote:
> > src/slave/containerizer/mesos/isolators/linux/capabilities.cpp
> > Line 93 (original), 112 (patched)
> > <https://reviews.apache.org/r/59552/diff/4/?file=1747634#file1747634line114>
> >
> >     I would use 'bounding' here instead of `flags.bounding_capabilites`. in the
future, we will allow framework to specify bounding set.

You can't use `bounding` here because `bounding` might be set from the framework's capabilities.
We need to ensure that whatever the framework specifies is within the limits set by the operator.
That will still be true when the framework gets to specify the bounding set.


- James


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/59552/#review177551
-----------------------------------------------------------


On June 10, 2017, 9:43 p.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/59552/
> -----------------------------------------------------------
> 
> (Updated June 10, 2017, 9:43 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jiang Yan Xu.
> 
> 
> Bugs: MESOS-7476
>     https://issues.apache.org/jira/browse/MESOS-7476
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The linux/capabilities isolator implements the `--allowed_capabilities`
> option by granting all the allowed capabilities. This change explicitly
> populates the only the bounding capabilities in the case where
> `--bounding_capabilities` has been set but the task itself has not been
> granted any effective capabilities. This improves the security of tasks
> since it is now possible to configure the bounding set without actually
> granting privilege to the task.
> 
> 
> Diffs
> -----
> 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.cpp 60d22aa877c1ab62a08222e5efe8800e337684da

>   src/slave/containerizer/mesos/launch.cpp f48d294a0a832dfe248c4a83849ee5a63cb76bce 
>   src/tests/containerizer/linux_capabilities_isolator_tests.cpp 40376a03fdb8f931f8d3f83b1c3fa6207e02c1d1

> 
> 
> Diff: https://reviews.apache.org/r/59552/diff/5/
> 
> 
> Testing
> -------
> 
> make check (Fedora 25)
> 
> 
> Thanks,
> 
> James Peach
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message