unshare: improve rootless detection #1312
Conversation
giuseppe
force-pushed
the
improve-rootless-detection
branch
from
6d413a4 to
966eeb8
Compare
| } | ||
| } | ||
| if !isRootless { | ||
| hasMappings, err := hasFullUsersMappings() |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
| // by looking up 4294967295 in /proc/self/uid_map. If the mappings would be | ||
| // copied as they are, the check in the OCI runtimes would fail. So just split | ||
| // it in two different ranges. | ||
| return bytes.Contains(content, []byte("4294967295")), nil |
There was a problem hiding this comment.
This comment doesn't seem to describe what's happening before or after it, so I don't understand what's happening here.
There was a problem hiding this comment.
I've fixed the comment and pushed a new version
in addition to check the UID of the user that launched the command, also check whether the process has CAP_SYS_ADMIN (in the same way as podman/pkg/rootless does) and also check that the current user namespace has all the IDs available. Closes: containers#1311 After this change, podman/pkg/rootless can use the function directly instead of defining another version with similar functionalities. Signed-off-by: Giuseppe Scrivano <[email protected]>
giuseppe
force-pushed
the
improve-rootless-detection
branch
from
966eeb8 to
975fab1
Compare
per containers#1312 (comment) . Signed-off-by: Miloslav Trmač <[email protected]>
in addition to check the UID of the user that launched the command,
also check whether the process has CAP_SYS_ADMIN (in the same way as
podman/pkg/rootless does) and also check that the current user
namespace has all the IDs available.
Closes: #1311
After this change, podman/pkg/rootless can use the function directly
instead of defining another version with similar functionalities.
Signed-off-by: Giuseppe Scrivano [email protected]