> On Jan. 15, 2019, 2:39 a.m., Gilbert Song wrote: > > include/mesos/mesos.proto > > Lines 3158-3159 (patched) > > <https://reviews.apache.org/r/68017/diff/14/?file=2118633#file2118633line3158> > > > > Seems like this was added recently. > > > > Is this field only used when there is a default agent wide seccomp > > profile provided from the agent flag? and we reply on it to give an > > opportunity for user/framework to get rid of seccomp? > > > > (probably more comment needed) > > > > If it is the case, do we have other options for naming? e.g., > > `no_seccomp` (maybe more explicit) > > Gilbert Song wrote: > after second thought, I would suggest to remove this field for now. since > there is not use case yet (I understand your motivation: you want users could > get rid of seccomp if there is a default one). We could add this field later > if necessary. For now: > > two options: > 1. remove it > 2. do it implicitly by using the optional profile_name: if seccompinfo > isSome but profile_name is None. do not set seccomp filtering for container > > Gilbert Song wrote: > I would prefer option #2. > > The reason we want to avoid introducing `unconfined` now is that > framework could set both field at the same time and ideally we may need an > `enum type`. given the use case is not clear yet (people may not necessary to > use it yet). lets make it implicit for now. > > Qian Zhang wrote: > > do it implicitly by using the optional profile_name: if seccompinfo > isSome but profile_name is None. do not set seccomp filtering for container > > So the semantic will be: > 1. seccompinfo is None: The default seccomp profile (if any) will be > applied for the container. > 2. seccompinfo is Some but profile_name is None: No seccomp filtering for > the container. > > This is kind of unintuitive to me because none-seccompinfo and > none-profile_name will have totally different result. > > Qian Zhang wrote: > Second thought, do we really need to support framework to disable seccomp > filtering? I think that may not be what the operator wants. If the operator > specifies a default seccomp profile, that means the operator cares about the > security of the agent host, and he/she would expect the default seccomp > profile to be applied for each container unless framework overrides it with > another seccomp profile. Override does not mean disable because any seccomp > profiles that framework can use are also set up by operator under > `--seccomp_config_dir` (i.e., still under the operator's control), but if > framework can disable seccomp filtering for its containers, that means the > security of the agent host is out of the operator's control which seems kind > of security hole. > > However, disabling seccomp filtering can still be supported (if some > operators want it) by not specifying the default seccomp profile, and in this > case if framework does not specify seccompinfo or profile_name, the seccomp > filtering will be disabled. But if operator specifies a default seccomp > profile, I think we should not support disabling seccomp filtering. And > another way to support disabling seccomp filtering is, operator specifies the > default seccomp profile and also put some other profiles under > `--seccomp_config_dir` and names them like `strict`, `loose` and `none`, and > there is no syscall filtering in the `none` profile which is equivalent to > disabling seccomp filtering. > > Andrei Budnik wrote: > I think we should adhere semantics for Seccomp configuration similar to > Docker and Kubernetes: > https://docs.docker.com/engine/security/seccomp/ (especially, `You can > pass unconfined to run a container without the default seccomp profile.`) > https://kubernetes.io/docs/concepts/policy/pod-security-policy/#seccomp > > Gilbert Song wrote: > Probably we should think deeper on this API. docker/k8s API may not > always make 100% sense. > > 1. From the perspective of cluster management, as an operator, if Dan > sets a default seccomp profile for the cluster, Dan would expect the profile > could be overwritten by framework for one container, but could not be > disabled. If any framework want to support seccomp `unconfined`, it is the > same effect with no-default profile, totally up to framework to set the > profile name. > > 2. From user's perspective, there may not be sufficient communication > between operators and users. when users realize that an app could not be > launched due to the default seccomp profile, he/she would expect `unconfined`. > > for concern #2, we need to design a better API than existing one (at > least introducing an enum since mesos does not expect both field set at the > same time). Also, it is not clear to me that how many frameworks will support > the seccomp API in near term. probably most people would just consume the > default profile in near term. > > Either way, I think it does not hurt to remove `unconfined` for now and > introduce it later when people ask.
Removed `unconfined` flag. Updated the following patches in the patch chain. - Andrei ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68017/#review211986 ----------------------------------------------------------- On Jan. 16, 2019, 1:41 p.m., Andrei Budnik wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/68017/ > ----------------------------------------------------------- > > (Updated Jan. 16, 2019, 1:41 p.m.) > > > Review request for mesos, Gilbert Song, Jie Yu, James Peach, and Qian Zhang. > > > Bugs: MESOS-9033 > https://issues.apache.org/jira/browse/MESOS-9033 > > > Repository: mesos > > > Description > ------- > > See summary. > > > Diffs > ----- > > include/mesos/mesos.proto 2ef6ba3aef67cf34227569948fd3ddc8dfd5879d > include/mesos/seccomp/seccomp.hpp PRE-CREATION > include/mesos/seccomp/seccomp.proto PRE-CREATION > include/mesos/slave/containerizer.proto > 5b4dcdda0f55ea3355c78d1447c7be9ca54d9dc9 > include/mesos/v1/mesos.proto 1a701da65f653fe4191f92ff1fb1436809b50acb > src/CMakeLists.txt a574d449dc26b820cbef7ff0b5e94b42b6fe86cf > src/Makefile.am cd785255fcdf1302a8f9fa358039e5d1f200e132 > > > Diff: https://reviews.apache.org/r/68017/diff/16/ > > > Testing > ------- > > > Thanks, > > Andrei Budnik > >
