-
Notifications
You must be signed in to change notification settings - Fork 223
seccomp: add add_key and request_key #2487
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
Conversation
Signed-off-by: Clinton Bowen <[email protected]>
Reviewer's GuideThis PR updates the default seccomp profile to include the add_key and request_key syscalls, reflecting recent kernel improvements that enforce user‐namespace isolation for keyring operations. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: afbase, sourcery-ai[bot] The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Luap99
left a comment
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 json file is generated from the code so you must add them in the go code and the generate the file.
@giuseppe PTAL
Even if they respect the user namesapce boundary root containers by default do not run in a new user namespace so this would enable access the main system keyring of the host for them?
|
same concern as @Luap99, wouldn't this allow containers that run without a new user namespace to get access to these syscalls? |
@Luap99 and @giuseppe this is a great point for which I didn't consider. To enable this for any configuration, we would have to require some additional UID/GID measure - especially in the root container case. 🤦🏾 I feel a little embarrassed but am very glad you both raised this point. Notably so that @giuseppe wrote about the k8s user namespace isolation blog post that could be a good measure to manage this issue and potentially allow for the use of these syscalls in the k8s scenario. I am going to go ahead and close this. Thank you! |
relates to moby/profiles#2
Historical Context
In Late December 2015, it was decided to exclude the
add_key,keyctl, andrequest_keysyscalls in the new default seccomp profile in the moby project. The rationale was that these syscalls for the Kernel Key Retention and Kernel Key Requesting were not user namespaced.In January 2016, @jessfraz wrote about the newly released default seccomp profile. Jess also contributed to incorporating seccomp profiles into kubernetes later that year.
In 2019, @dhowells and @thejh worked on these syscalls to respect user namespace boundaries.
For containters/common, the PR #421 added keyctl syscall but not the related
add_keyorrequest_keysyscalls.Why These Syscalls Are Now Safe
The original 2015 security concerns have been addressed through specific kernel code changes that ensure proper user namespace isolation. Here are some examples:
1. Namespace-Aware Keyring Access
The
add_keysyscall now operates within the caller's user namespace context throughlookup_user_key():This function ensures containers can only access keyrings that exist within their namespace boundary, preventing cross-namespace key access.
2. Namespace Validation for Key Operations
Key ownership changes are validated within the caller's namespace via
keyctl_chown_key:3. Namespace-Aware Request Key Operations
The
request_keysyscall properly handles namespace translation during userspace callouts inrequest_key.c:Summary by Sourcery
Enable add_key and request_key syscalls in the default seccomp profile now that kernel keyring operations respect user namespace boundaries.
Enhancements: