> On Sept. 6, 2016, 8:35 nachm., Jie Yu wrote: > > src/slave/containerizer/mesos/launch.cpp, line 378 > > <https://reviews.apache.org/r/50270/diff/6/?file=1491898#file1491898line378> > > > > I am wondering if we should rename this to: > > ``` > > capabilities->setKeepCaps() > > capabilities->clearKeepCaps() > > ```
Done, also submitted https://reviews.apache.org/r/51697/. Could you commit that as part of this chain? I did not add a `Capabilities::clearKeepCaps` as we currently have no use for it. I am also not sure how useful clearing keep caps again might be midterm as we'd likely never execute `setKeepCaps` in anything not already executing in a separate process we could just let die (like we do here). > On Sept. 6, 2016, 8:35 nachm., Jie Yu wrote: > > src/slave/containerizer/mesos/launch.cpp, lines 382-383 > > <https://reviews.apache.org/r/50270/diff/6/?file=1491898#file1491898line382> > > > > `uid` might be None, right? I reworded the log output slightly to not require some `uid` anymore. > On Sept. 6, 2016, 8:35 nachm., Jie Yu wrote: > > src/slave/containerizer/mesos/launch.cpp, line 417 > > <https://reviews.apache.org/r/50270/diff/6/?file=1491898#file1491898line417> > > > > Why this is not guarded by ifdef linux? I was trying to minimize code behind `ifdef` guards. While it should not be possible to set the capabilities flag on non-Linux systems the `Flag` member is defined for all platforms as it has no dependencies on Linux-specific features. > On Sept. 6, 2016, 8:35 nachm., Jie Yu wrote: > > src/slave/flags.cpp, line 466 > > <https://reviews.apache.org/r/50270/diff/6/?file=1491900#file1491900line466> > > > > What's this? I thought we use PR_SET_KEEPCAPS and agent is running > > under root. I think there's actually no need to make a hard requirement on the agent to be run as root (at least to make use of capabilities) -- we do only require `SETPCAP` and that the capabilities requested for a container are contained in the agent's capabilities. I believe it makes sense to anticipate that users of this capabilities feature are aware of fine-grained permissions. I added an additional note to the help text to make clear that `SETPCAP` is required for the agent. > On Sept. 6, 2016, 8:35 nachm., Jie Yu wrote: > > src/slave/flags.cpp, line 94 > > <https://reviews.apache.org/r/50270/diff/6/?file=1491900#file1491900line94> > > > > This list is not complete. I'd suggest we don't make this change in > > this patch. It should point to a document online with a complete list. > > Let's create a ticket to track. I filed https://issues.apache.org/jira/browse/MESOS-6133. - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50270/#review147872 ----------------------------------------------------------- On Sept. 7, 2016, 6:43 nachm., Benjamin Bannier wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50270/ > ----------------------------------------------------------- > > (Updated Sept. 7, 2016, 6:43 nachm.) > > > Review request for mesos, Jay Guo and Jie Yu. > > > Bugs: MESOS-5303 > https://issues.apache.org/jira/browse/MESOS-5303 > > > Repository: mesos > > > Description > ------- > > This change introduces linux capability based security for unified > containerizer. A new agent flag \`allowed_capabilities\` has been > introduced to override the default capabilities of the user or the > capabilities requested by the user. > > This feature is only available on linux. > > > Diffs > ----- > > src/slave/containerizer/mesos/launch.hpp > 0e86da9c7bd9c7fbedd7102d66b902d1c10e5e0b > src/slave/containerizer/mesos/launch.cpp > 13b65d82e029650e150eb2bc3647d95af167bd72 > src/slave/flags.hpp 1a006663e7cc58ee548b3dda686cfbac0c240baa > src/slave/flags.cpp 0f2be1700f41b74da4ea1ce699a81ec33cf92a9a > > Diff: https://reviews.apache.org/r/50270/diff/ > > > Testing > ------- > > `make check` and `sudo make check` (Debian jessie, gcc-4.9.2, w/o > optimizations) > > > Thanks, > > Benjamin Bannier > >
