> On March 6, 2018, 6:34 p.m., Benjamin Bannier wrote:
> > I left two open questions:
> > 
> > * do we want to continue using `root` as the giant bag of priviledges or 
> > can we be more fine-grained in the future.
> > * can we build tooling so isolators do not just declare the widest possible 
> > requirements?

I think these are great questions! Unfortunately, if you have a lot of 
in-process isolators which each require privilege, the process itself will 
inevitably decend into the giant bag of privilege. I considered a few options 
other than this approach, but in the end I decided to just make a modest 
consolidation of the status quo.


> On March 6, 2018, 6:34 p.m., Benjamin Bannier wrote:
> > src/slave/containerizer/mesos/containerizer.cpp
> > Line 47 (original), 47 (patched)
> > <https://reviews.apache.org/r/65932/diff/1/?file=1971951#file1971951line47>
> >
> >     It is a little unfortunate that it is so hard for us to add enforcement 
> > making sure that isolators only make use of declared requirements. This 
> > carries the risk that isolator requirements become do not reflect what is 
> > actually needed, e.g., might be way too wide.

Here we are enforcing what isolators say they need, not what they don't say 
they need. I agree that this has the potential to bitrot (i.e. an isolator 
stops needing a namespace but fails to remove its declared requirement), but 
that doesn't seem any different to the status quo.


> On March 6, 2018, 6:34 p.m., Benjamin Bannier wrote:
> > src/slave/containerizer/mesos/isolator.hpp
> > Lines 98 (patched)
> > <https://reviews.apache.org/r/65932/diff/1/?file=1971952#file1971952line98>
> >
> >     Not sure this requires `root`, but rather the ability to perform 
> > certain priviledged actions. This set of actions seems can be large and 
> > likely unbounded, so while asking for `root` instead of certain privileges 
> > feels like tech debt to me, I am not sure how one could manage that 
> > complexity here. I still feel that in principle it would be great to not 
> > require blanket `root` going forward.

I did consider having the isolators declare their required capabilities 
bitmask, but it seemed overly complex (at least for now). Isolators aren't 
really written to consider fine-grained privileges and I didn't want to spend a 
long time figuring out which capabilities they actually need (and wouldn't be 
able to test the correctness of the claims anyway).


- James


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


On March 6, 2018, 6:18 p.m., James Peach wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/65932/
> -----------------------------------------------------------
> 
> (Updated March 6, 2018, 6:18 p.m.)
> 
> 
> Review request for mesos, Andrew Schwartzmeyer and Jie Yu.
> 
> 
> Bugs: MESOS-6555
>     https://issues.apache.org/jira/browse/MESOS-6555
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Many isolators require root privileges and support for specific
> Linux namespaces. By adding some additional virtual functions
> to the `Isolator` interface, we can let isolators declare
> their needs and perform the checks in a containerizer single
> pass when we create the isolators.
> 
> 
> Diffs
> -----
> 
>   include/mesos/slave/isolator.hpp f682e3037523b1c210b5ba36246556518f261e1b 
>   src/linux/ns.cpp 64722c785cecce5b861efbc3f3f2027a61f666d4 
>   src/slave/containerizer/mesos/containerizer.cpp 
> a61ed5bb4f60ec63aebd6bdabbc9a4b01235623e 
>   src/slave/containerizer/mesos/isolator.hpp 
> b7af3946155e28d80013a650973ae230de7d01c5 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.hpp 
> 5763c9880728f02e44116fd50e62b592a8ef69b6 
>   src/slave/containerizer/mesos/isolators/cgroups/cgroups.cpp 
> 4431ce13d28035b0c5c037b2848ae03aeaf65562 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.hpp 
> 0e1761d44f7dce51bef138c5eda2ac3d8eb6ea21 
>   src/slave/containerizer/mesos/isolators/docker/volume/isolator.cpp 
> a6f05a8cf7eab2b2cc4c2142efdf3125462ec68e 
>   src/slave/containerizer/mesos/isolators/filesystem/linux.hpp 
> d933af4e5a605f63d3c4bc584e82de7f3f80da55 
>   src/slave/containerizer/mesos/isolators/filesystem/linux.cpp 
> f1d377eb911fb6a0ca3a4749a0e00d5166f35129 
>   src/slave/containerizer/mesos/isolators/filesystem/posix.hpp 
> 794b6e5990db5f8eb21a6535872f284ca02e0553 
>   src/slave/containerizer/mesos/isolators/filesystem/shared.hpp 
> 414682ba8380e8960ff93666799e2f0f674022c1 
>   src/slave/containerizer/mesos/isolators/filesystem/shared.cpp 
> 118f39871fd76ca00b92e4fa80c698a2a8f02f19 
>   src/slave/containerizer/mesos/isolators/gpu/isolator.hpp 
> 5d1bc7bc637fb266018ba89e668910ef7f89d5e8 
>   src/slave/containerizer/mesos/isolators/gpu/isolator.cpp 
> f5e8860cc3ce3dcd6d4d15ccfd1cc690484b1b57 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.hpp 
> 73e26a0c78ccfa43f533d6811f0a58ebe1bc4403 
>   src/slave/containerizer/mesos/isolators/linux/capabilities.cpp 
> 2e8ad930df489777845928fb24239365c043f618 
>   src/slave/containerizer/mesos/isolators/namespaces/ipc.hpp 
> b3815ae21dd3d31c2c0b94305e262f167fb95030 
>   src/slave/containerizer/mesos/isolators/namespaces/ipc.cpp 
> 6c8e8eed69ed355094f7d9b54bde25a8a9bc63f7 
>   src/slave/containerizer/mesos/isolators/namespaces/pid.hpp 
> 5ed4a6eceb0a7bd340a85482eab013a157c56cff 
>   src/slave/containerizer/mesos/isolators/namespaces/pid.cpp 
> 73e09636808081ce411c022bf2bcb2a157a3ad25 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.hpp 
> 1d01915c2db66e54ed68a3dbaa12ea061ca5f6b2 
>   src/slave/containerizer/mesos/isolators/network/cni/cni.cpp 
> c60c23f74f2abf6bef8dd32cc2e47e33bf666169 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.hpp 
> 6ff3820de6af544c6ec92b0e76fd934715b87a96 
>   src/slave/containerizer/mesos/isolators/network/port_mapping.cpp 
> 96e1a2b4133171e7a204e5d234c28ed81913261c 
>   src/slave/containerizer/mesos/isolators/posix/rlimits.hpp 
> 2d95fb2af0283ed5201cb33b653e6dfb9c35d532 
>   src/slave/containerizer/mesos/isolators/posix/rlimits.cpp 
> 9aa886fc4b8a1eddaf96a93b4385dd99681a8588 
>   src/slave/containerizer/mesos/isolators/volume/sandbox_path.hpp 
> 20d5b32fb7ada1a17a40bf1a2438db4d85cf1063 
>   src/slave/containerizer/mesos/isolators/volume/sandbox_path.cpp 
> 5801977e93bcb8f463a2635f73e763098f2aa97d 
>   src/slave/containerizer/mesos/isolators/volume/secret.hpp 
> 2680345c974c5c20bef1f2715677b37f6bead27f 
>   src/slave/containerizer/mesos/isolators/volume/secret.cpp 
> 8071e4ee808bc825b13a6291767778d6ce3c2746 
>   src/slave/containerizer/mesos/isolators/xfs/disk.hpp 
> 07e68a777aefba4dd35066f2eb207bba7f199d83 
>   src/slave/containerizer/mesos/isolators/xfs/disk.cpp 
> 8d9f8f846866f9de377c59cb7fb311041283ba70 
>   src/tests/containerizer/ns_tests.cpp 
> fa4349e29b975550e05b00a1f848a24cd8e4f0de 
> 
> 
> Diff: https://reviews.apache.org/r/65932/diff/1/
> 
> 
> Testing
> -------
> 
> make check (Fedora 27)
> 
> 
> Thanks,
> 
> James Peach
> 
>

Reply via email to