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.

Sign up for GitHub

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

unshare: improve rootless detection #1312

Merged
merged 1 commit into from
Aug 26, 2022
Merged

unshare: improve rootless detection #1312

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions pkg/unshare/unshare_linux.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"errors"
"fmt"
"io"
"io/ioutil"
"os"
"os/exec"
"os/signal"
Expand Down Expand Up @@ -387,10 +388,39 @@ const (
UsernsEnvName = "_CONTAINERS_USERNS_CONFIGURED"
)

// hasFullUsersMappings checks whether the current user namespace has all the IDs mapped.
func hasFullUsersMappings() (bool, error) {
content, err := ioutil.ReadFile("/proc/self/uid_map")
if err != nil {
return false, err
}
// if the uid_map contains 4294967295, the entire IDs space is available in the
// user namespace, so it is likely the initial user namespace.
return bytes.Contains(content, []byte("4294967295")), nil
Copy link
Member

Choose a reason for hiding this comment

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

This comment doesn't seem to describe what's happening before or after it, so I don't understand what's happening here.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've fixed the comment and pushed a new version

mtrmac marked this conversation as resolved.
Show resolved Hide resolved
}

// IsRootless tells us if we are running in rootless mode
func IsRootless() bool {
isRootlessOnce.Do(func() {
isRootless = getRootlessUID() != 0 || getenv(UsernsEnvName) != ""
if !isRootless {
hasCapSysAdmin, err := HasCapSysAdmin()
if err != nil {
logrus.Warnf("Failed to read CAP_SYS_ADMIN presence for the current process")
}
if err == nil && !hasCapSysAdmin {
isRootless = true
}
}
if !isRootless {
hasMappings, err := hasFullUsersMappings()
Copy link
Member

Choose a reason for hiding this comment

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

Why would this indicate isRootless.

If I have a range of 0 0 1000000 won't the isRootless return true? Aren't I running as root in this case?

Copy link
Member Author

Choose a reason for hiding this comment

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

But we are using isRootless to mean "not in the initial user namespace". That is what the kernel does when checking what is allowed and what not. For example, we need to use user. xattrs for overlay whiteouts even if we are root in a user namespace

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think by now, we desperately need documentation. “IsRootless tells us if we are running in rootless mode” says nothing.

Describe in which situations that function returns true (possibly how can one get into such situations), and what does that means for application behavior. What should, or shouldn’t, they be doing differently?

It might well be the case that we need three or more different functions, IsNotInTheInitialUserNamespace, IsInsideAPodmanCreatedContainerNamespace, and so on.

Just vaguely saying “rootless” is apparently causing misunderstandings and unanticipated breakage.

if err != nil {
logrus.Warnf("Failed to read current user namespace mappings")
}
if err == nil && !hasMappings {
isRootless = true
}
}
})
return isRootless
}
Expand Down