> 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 > >
