Skip to content
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

Avoid using runc-dmz if capabilities are set as non-root #4129

Closed
wants to merge 1 commit into from

Conversation

dgl
Copy link

@dgl dgl commented Nov 28, 2023

...unless they are in both the bounding and ambient sets, in which case things work as expected.

Potential fix for #4125

...unless they are in both the bounding and ambient sets, in which case
things work as expected.

Signed-off-by: David Leadbeater <dgl@dgl.cx>
Copy link
Member

@cyphar cyphar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs a different approach. See this comment in the original issue. (Sorry, I only finished writing it after you'd already posted this PR.)

@@ -135,7 +135,7 @@ func (l *linuxSetnsInit) Init() error {
return &os.PathError{Op: "close log pipe", Path: "fd " + strconv.Itoa(l.logFd), Err: err}
}

if l.dmzExe != nil {
Copy link
Member

@cyphar cyphar Nov 28, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This approach re-introduces the CVEs that runc-dmz is protecting against. If we have chosen to use runc-dmz at this point, not using it allows the container to gain write access to the host runc binary (because /proc/self/exe at this point is the actual binary, not a copy).

In order to take this approach, you will need to modify isDmzBinarySafe to detect this problem before we do runc init. Effectively you need to make a check that mirrors the dmz.WorksWithSELinux check.

@cyphar
Copy link
Member

cyphar commented Dec 14, 2023

Closing in favour of #4137.

@cyphar cyphar closed this Dec 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants