-
Notifications
You must be signed in to change notification settings - Fork 1.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Bug 1379761 moved and edited seccomp info #3020
Conversation
Seccomp support is achieved via two annotations in the pod configuration: | ||
|
||
* *seccomp.security.alpha.kubernetes.io/pod*: profile applies to all containers in the pod that do not override | ||
* *container.seccomp.security.alpha.kubernetes.io/<container name>*: container-specific profile override |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pweil- I'm not sure what the above are. Why are we configuring pods here? Is this something you have to configure before you can go through the tasks below?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
both of the annotations belong at the pod level.
$ cat /boot/config-`uname -r` | grep CONFIG_SECCOMP= | ||
CONFIG_SECCOMP=y | ||
---- | ||
==== |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be inside the topics below first? Or would this be more at home in something like a "Enabling Seccomp" section instead of a admonition?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is not related to OpenShift and is more about the operating system I called it out separately. I could see it going in the section below if you'd prefer it there.
`seccomp-profile-root` directory. | ||
+ | ||
If you are using the default *runtime/default* profile, you do not need to | ||
create one. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The above was taken over from the Examples section, but now I think I've confused it all. Is it that the file exists by default? Or is it that you have to take the file from the example inline above, so that you don't have to create it? Sorry if this is a confusing question...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you are using runtime/default
it is built into the container runtime (ie docker) and doesn't require a file on the node.
|
||
* *runtime/default*: the default profile for the container runtime (no profile on disk required) | ||
* *unconfined*: unconfined profile, ie, no seccomp sandboxing | ||
* *localhost/<profile-name>*: the profile installed to the node's local seccomp profile root |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure what the above is saying. Are they example profiles? can they be substituted into the step below this one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these are the allowable formats of the annotation values. runtime/default
is the container runtime's default, built-in profile and doesn't require a file. unconfined
is turning off seccomp and doesn't require a file. localhost/<name>
is pointing to a specific file on the node. The localhost
portion tells the system that it is, in fact, a file that needs read and the name corresponds to the actual file name that would be stored in the seccomp profile root that was configured on the kubelet.
---- | ||
|
||
[[seccomp-configuring-openshift-with-custom-seccomp]] | ||
== Configuring {product-title} for a Custom Seccomp Profile |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please let me know if anything below is incorrect.
+ | ||
---- | ||
seccompProfiles: | ||
- runtime/default |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this would be the localhost/<name>
option if you wanted to use a custom profile. If you're just using the runtime/default
you would not need to set the seccomp-profile-root.
- runtime/default | ||
---- | ||
|
||
. Restart the node host. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be done after the kubelet args are changed. They just need to restart the node service(s), not the whole host.
c0a51c3
to
c22602d
Compare
@pweil- Thanks for the prompt comments. I've made the changes I think need to happen and that you've suggested, but kept some of them because your comments helped them make sense to me. Could I ask for a final ack that all is well with the topic? Then I can move it to review. Thanks! |
- "/your/path" | ||
---- | ||
|
||
. Configure *seccomp-profile-root*: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a duplicate step of configuring the restricted SCC
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pweil- So should i delete this step in favour of the above one?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, reading this a bit closer I think the examples are just in the wrong spot. Here is what I see:
- create a profile in the seccomp-profile-root
- example of configuring the root
- configure the root
- example of configuring SCC
- restart node
- example restart
- configure the SCC
- example SCC
I think you just need to move lines 115-119 to replace 123-126 and then we're good.
LGTM |
Thanks, @pweil- @ahardin-rh @adellape @tpoitras Should be ok to peer review now if anyone wants. Just got one thing to sort out above. |
c22602d
to
3f021be
Compare
@pweil- I made change to what i think you suggested. Please let me know if I'm wrong. @ahardin-rh @tpoitras @adellape totally ready for peer review. Thanks, everybody. |
@bfallonf - here is what I would suggest. Everything looks good except for the example of creating a profile in the root which I think can be removed. It's a file on the host machines:
|
==== | ||
Seccomp is | ||
link:https://github.com/kubernetes/kubernetes/blob/release-1.4/docs/design/seccomp.md[currently | ||
in alpha support]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does alpha support mean? Is it the same thing as Technology Preview?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Paul made a comment below that tech preview works.
@bfallonf just one comment from me ⭐ |
@@ -496,6 +494,8 @@ Topics: | |||
- Name: Securing Builds by Strategy | |||
File: securing_builds | |||
Distros: openshift-origin,openshift-enterprise | |||
- Name: Seccomp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a sufficient title? Should you use the long form of "Secure Computing Mode"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm I'd changed to a better title already. Maybe I didn't push.
==== | ||
Seccomp is | ||
link:https://github.com/kubernetes/kubernetes/blob/release-1.4/docs/design/seccomp.md[currently | ||
in alpha support]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm assuming "alpha" here means "pre-beta", but should you quantify what the risks/ramifications are?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alpha should be defined as https://github.com/openshift/origin#alpha-and-unsupported-kubernetes-features
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pweil- What's the policy of RH? Does it mean technical preview?
The feature may have significant bugs and is suitable for testing and prototyping.
It looks like not production ready feature.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, tech preview
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. Thank you!
in alpha support]. | ||
==== | ||
|
||
Seccomp is used to restrict the set of system calls applications can make, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe the first instance you could say something like "Secure computing mode, or seccomp, is used to..."
in alpha support]. | ||
==== | ||
|
||
Seccomp is used to restrict the set of system calls applications can make, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/calls applications/calls that applications
|
||
[IMPORTANT] | ||
==== | ||
Containers are run with *unconfined* seccomp settings by default. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure you need the "are" here.
== Enabling Seccomp | ||
|
||
Seccomp is a feature of the Linux kernel. To ensure seccomp is enabled on your | ||
system run: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
comma after "system"?
Maybe not, I tend to over-use the comma.
|
||
. Create the seccomp profile. | ||
+ | ||
A seccomp profile is a json file providing syscalls and the appropriate action |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/json/JSON
or
s/json/.json
|
||
. Create the seccomp profile. | ||
+ | ||
A seccomp profile is a json file providing syscalls and the appropriate action |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is a "syscall"? Is this defined elsewhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From what I know, it's a linux thing...
A seccomp profile is a json file providing syscalls and the appropriate action | ||
to take when a syscall is invoked. The | ||
link:https://github.com/docker/docker/blob/master/profiles/seccomp/default.json[default | ||
profile] is sufficient in many cases, the cluster administrator must define the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
s/in many cases, the cluster/in many cases. The cluster...
^ This should be two separate sentences, no?
==== | ||
|
||
Seccomp is used to restrict the set of system calls applications can make, | ||
allowing administrators greater control over the security of workloads running |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
allowing cluster administrators
|
||
. Create the seccomp profile. | ||
+ | ||
A seccomp profile is a json file providing syscalls and the appropriate action |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This definition from "A seccomp profile" to the end of this step does not belong inside the procedure. It should be preamble under the section heading.
Sorry @adellape - I haven't had the chance to look at this today (10/31). Will look at this tomorrow in detail. |
Removed enterprise-3.3 branch because it's been picked/published there already. Added dedicated-3.3 + online + to_followup labels and moved to the Next Release milestone for further conditionalizing for Dedicated/Online. |
Followup to #2975
@pweil- I moved the seccomp info to the admin guide, and put the Examples section more into the Configuring section, but kept the Custom Profile stuff on the outside. WDYT?
I do have some questions on areas I think can be elaborated on though. I'll put them inline. Thanks!