policy for seccomp-profile selection #4806
Conversation
openshift-ci-robot
added
the
do-not-merge/work-in-progress
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: vrothberg The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci-robot
added
approved
|
Note that tests are still missing. I will push an image to Quay that we can use for testing. |
Rename `data` to `imageData` to make it more obvious which kind of data the variable refers to. Signed-off-by: Valentin Rothberg <[email protected]>
vrothberg
force-pushed
the
seccomp
branch
2 times, most recently
from
ae7fc1b
to
3e3ccb7
Compare
vrothberg
changed the title
openshift-ci-robot
removed
the
do-not-merge/work-in-progress
|
Still need to add tests. @baude gave me access to the libpod project on Quay where I'll push an image with a seccomp profile. |
vrothberg
added
the
do-not-merge/work-in-progress
openshift-ci-robot
removed
the
do-not-merge/work-in-progress
|
Now with tests |
|
@vbatts WDYT? |
| if policy, err := cc.LookupSeccompPolicy(c.String("seccomp-policy")); err != nil { | ||
| return nil, err | ||
| } else { | ||
| secConfig.SeccompPolicy = policy |
There was a problem hiding this comment.
I don't see where this is actually consumed - where are we trying to decode/use the image seccomp annotation
There was a problem hiding this comment.
We're doing that in pkg/spec.getSeccompConfig
vrothberg
force-pushed
the
seccomp
branch
2 times, most recently
from
5c4f795
to
155a0f3
Compare
|
Dang. We can't use Quay as docker v1 doesn't support annotations -.- |
Implement a policy for selecting a seccomp profile. In addition to the default behaviour (default profile unless --security-opt seccomp is set) add a second policy doing a lookup in the image annotation. If the image has the "io.containers.seccomp.profile" set its value will be interpreted as a seccomp profile. The policy can be selected via the new --seccomp-policy CLI flag. Once the containers.conf support is merged into libpod, we can add an option there as well. Note that this feature is marked as experimental and may change in the future. Signed-off-by: Valentin Rothberg <[email protected]>
|
The images are now at |
you can request your repo on quay to be switched to v2.s2 |
| } | ||
| if policy, err := cc.LookupSeccompPolicy(c.String("seccomp-policy")); err != nil { | ||
| return nil, err | ||
| } else { |
There was a problem hiding this comment.
As there is a return inside the if clause, you could happy path this.
|
I believe we could do this by default, iff we could determine the seccomp file specified in the image has less access then the default policy. Then we don't need the user to be involved in the decision. In my opinion, if the user has to be involved, then no one will use it. I could see the developer of the image going and specifying a locked down seccomp filter. The problem is whether or not the image should be trusted to specify its own security constraints. If the image can specify limited seccomp rules, that are provable more secure, then we could use by default. For that matter we could also add annotations to the image to specify the list of capabiltiies required by the container. Which would be even easier for the developer to access, then the container engine compares the list of capabilities to its default list and runs the container with less or fails to run if the image requires more caps, then the caller specified.... |
Doing that for capabilities is much easier as it boils down to diff slices of strings. Doing that for seccomp files is much harder where we'll soon run into difficult choices:
Such choices might be answered differently depending on the person or use-case etc. Hence, I feel rather objective to implement comparing the seccomp profiles.
I don't think we can do this reliably (see comments above). I believe that a policy such as suggested in this PR is the way forward. It somehow boils down to the questions whether we can trust an image. If we are concerned that an image could open the fence via a less secure seccomp profile, then we should check and inspect the entire image in any case. For instance, an image might specify volumes and send them to a remote server etc. To give a different perspective/use case: Currently, seccomp profiles in OpenShift/Kubernetes must reside on the host. With this PR (assuming we'll lift it to the K8s world) we can attach profiles to the image and make seccomp portable. Certainly, we should only do this when we trust the image. If we don't then there's a whole load of other things we must disable. One use case might be to further tighten the security, another might be to explicitly allow certain syscalls which might be needed for a certain image. So we have opt-in AND opt-out use cases. In the opt-out case, we could have a much tighter default profile and use the image to opt-out. I think this is another example for not comparing seccomp profiles: How do we measure more or less secure? |
|
Doing that for capabilities is much easier as it boils down to diff slices of strings. Doing that for seccomp files is much harder where we'll soon run into difficult choices: Not sure I agree. If we run the hosts seccomp files and the images seccomp files through the same algorithm to fire off the filters and get the final list of syscalls available to the container. If image lists contains a syscall not listed in the host list, then container is blocked. |
We don't have a list of syscalls. We have a list of rules and those are conditional. An image might specify to only allow/block a syscall when an argument has a certain value. And that's where it's getting hairy as some might argue that a conditional blocking of an argument is safer or more dangerous or just needed for the sake of portability. In the end, this PR allows to add more policies in the future. I would still vote the "default" policy to mean "default chosen by the podman". We can still add a "fallback" policy in the future. |
|
What about calls that block individual arguments? Whitelist vs blacklist? I
remember looking into something similar (but composing the two profiles
together instead) and abandoning it because it did not seem possible.
…On Mon, Jan 13, 2020, 13:04 Daniel J Walsh ***@***.***> wrote:
Doing that for capabilities is much easier as it boils down to diff slices
of strings. Doing that for seccomp files is much harder where we'll soon
run into difficult choices:
Not sure I agree. If we run the hosts seccomp files and the images seccomp
files through the same algorithm to fire off the filters and get the final
list of syscalls available to the container. If image lists contains a
syscall not listed in the host list, then container is blocked.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#4806>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AB3AOCGAYRHEEZUTRDU64S3Q5SULDANCNFSM4KD3KNGA>
.
|
That's exactly my concern as well. We could drive some heuristics when comparing but I don't think that's user friendly. I feel objective to semantics that exceed an if-then-else in the docs. |
|
I think there is a list of syscalls that end up being loaded into the kernel. This list is done by prepossessing all of the capabilities to get the final list. I think we could get pretty close to the correct list. Then we could look at syscalls that have specific fields, and force the user to override then, since it gets difficult. In the general case, I believe we could tell the difference. |
|
Alright. @rhatdan and I agreed to merge this PR as is but to also look into the idea to create a policy that only uses the image's seccomp profile if it doesn't allow syscalls that are blocked by the default profile. |
|
/lgtm |
github-actions
bot
added
the
locked - please file new issue/PR
Implement a policy for selecting a seccomp profile. In addition to the
default behaviour (default profile unless --security-opt seccomp is set)
add a second policy doing a lookup in the image annotation.
If the image has the "io.containers.seccomp.profile" set its value will be
interpreted as a seccomp profile. The policy can be selected via the
new --seccomp-policy CLI flag.
Once the containers.conf support is merged into libpod, we can add an
option there as well.
Note that this feature is marked as experimental and may change in the
future.
Signed-off-by: Valentin Rothberg [email protected]