mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gilbert Song <songzihao1...@gmail.com>
Subject Re: Review Request 68017: Added Seccomp-related protobuf messages.
Date Wed, 16 Jan 2019 08:55:10 GMT


> On Jan. 14, 2019, 6:39 p.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

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.


- Gilbert


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


On Nov. 8, 2018, 7:24 a.m., Andrei Budnik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68017/
> -----------------------------------------------------------
> 
> (Updated Nov. 8, 2018, 7:24 a.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/15/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>


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