runtime: create userns when CAP_SYS_ADMIN is not present #10079

giuseppe · 2021-04-19T11:18:48Z

when deciding to create a user namespace, check for CAP_SYS_ADMIN instead of looking at the euid.

This is useful for running podman inside podman.

Signed-off-by: Giuseppe Scrivano [email protected]

giuseppe · 2021-04-19T11:30:23Z

Luap99 · 2021-04-19T11:39:12Z

cmd/podman/root.go

+		return err
+	}
+
+	if !registry.IsRemote() && !hasCapSysAdmin && !found {


The same change must be made here

podman/cmd/podman/common/completion.go

Line 46 in 6908557

if !registry.IsRemote() && rootless.IsRootless() {

rhatdan · 2021-04-19T11:51:27Z

pkg/rootless/rootless.go

+)
+
+// HasCapSysAdmin returns whether the current process has CAP_SYS_ADMIN.
+func HasCapSysAdmin() (bool, error) {


I think we need this in storage/pkg/unshare so that buildah could use it.

buildah has already the equivalent check, I'll refactor podman and buildah to use the common code

containers/storage#877

rhatdan · 2021-04-19T11:53:06Z

pkg/rootless/rootless_linux.go

+	// 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.
+	if bytes.Contains(content, []byte("4294967295")) {


mheon · 2021-04-19T13:28:53Z

Concern: a lot of our code is wired to use different paths for rootless, but I think we really only want to do that for non-EUID 0 rootless - root + non SYS_ADMIN can (and IMO should) still use standard Podman paths.

rhatdan · 2021-04-19T20:04:48Z

This would only cause this in rare conditions like podman inside of a container. We will have to study this, and see if it is an issue with using /root/.config/containers rather then /etc/containers ...

giuseppe · 2021-04-20T14:18:27Z

reworked to use the code from containers/storage

Signed-off-by: Giuseppe Scrivano <[email protected]>

giuseppe · 2021-04-22T14:17:59Z

tests are green now

pkg/rootless/rootless_linux.go

rhatdan

One small change and then LGTM

openshift-ci-robot · 2021-04-22T14:44:12Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: giuseppe, rhatdan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [giuseppe,rhatdan]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

rhatdan · 2021-04-22T17:03:41Z

@containers/podman-maintainers PTAL

mheon · 2021-04-22T17:30:57Z

Would we consider this a breaking change for Podman in a unprivileged container when run as root, given that the paths in use potentially changed? I know this isn't really a supported configuration, but I imagine someone may have figured out how to get it working?

I'm not opposed to merging if it is, but we should have release notes and troubleshooting guide ready if so.

rhatdan · 2021-04-22T17:45:22Z

No, because rootfull podman within a container and with overlay is currently broken. and would not work. I see this as more of a bugfix. Basically you can not mount any thing without SYS_ADMIN or a user namespace. So only way to get this to work would be if the users had created a user namepspace for podman to use, and this change would not break that.

rhatdan · 2021-04-24T10:22:50Z

@giuseppe fix my Error->Warn and we can get this merged.

when creating a user namespace, attempt to create it first by copying the current mappings and then fallback to the other methods: 1) use newidmap tools and ... 2) create a user namespace with a single user mapped. Signed-off-by: Giuseppe Scrivano <[email protected]>

when deciding to create a user namespace, check for CAP_SYS_ADMIN instead of looking at the euid. [NO TESTS NEEDED] Needs nested Podman Signed-off-by: Giuseppe Scrivano <[email protected]>

giuseppe · 2021-04-26T06:59:16Z

@giuseppe fix my Error->Warn and we can get this merged.

fixed and pushed a new version

rhatdan · 2021-04-26T10:55:17Z

LGTM

rhatdan · 2021-04-26T10:55:42Z

@containers/podman-maintainers PTAL

umohnani8 · 2021-04-26T13:18:28Z

/lgtm

openshift-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Apr 19, 2021

Luap99 reviewed Apr 19, 2021

View reviewed changes

rhatdan reviewed Apr 19, 2021

View reviewed changes

giuseppe force-pushed the create-userns-for-root-without-CAP_SYS_ADMIN branch from f9028e3 to 8f0b02b Compare April 19, 2021 12:17

giuseppe force-pushed the create-userns-for-root-without-CAP_SYS_ADMIN branch from 8f0b02b to 3ec3916 Compare April 20, 2021 14:18

giuseppe changed the title ~~[WIP] runtime: create userns when CAP_SYS_ADMIN is not present~~ runtime: create userns when CAP_SYS_ADMIN is not present Apr 20, 2021

giuseppe marked this pull request as ready for review April 20, 2021 14:18

openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 20, 2021

giuseppe force-pushed the create-userns-for-root-without-CAP_SYS_ADMIN branch from 3ec3916 to 64ccec9 Compare April 20, 2021 14:44

giuseppe marked this pull request as draft April 20, 2021 14:47

openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 20, 2021

giuseppe force-pushed the create-userns-for-root-without-CAP_SYS_ADMIN branch from 64ccec9 to a1e737d Compare April 20, 2021 14:49

giuseppe marked this pull request as ready for review April 20, 2021 15:09

openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 20, 2021

giuseppe force-pushed the create-userns-for-root-without-CAP_SYS_ADMIN branch 4 times, most recently from 7b6d113 to 7806c1b Compare April 22, 2021 11:00

giuseppe marked this pull request as draft April 22, 2021 12:52

openshift-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 22, 2021

giuseppe force-pushed the create-userns-for-root-without-CAP_SYS_ADMIN branch from 7806c1b to 498164e Compare April 22, 2021 13:03

rootless: if root is not sub?id raise a debug message

24f00e4

Signed-off-by: Giuseppe Scrivano <[email protected]>

giuseppe marked this pull request as ready for review April 22, 2021 14:14

openshift-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 22, 2021

rhatdan reviewed Apr 22, 2021

View reviewed changes

pkg/rootless/rootless_linux.go Outdated Show resolved Hide resolved

rhatdan approved these changes Apr 22, 2021

View reviewed changes

giuseppe added 2 commits April 26, 2021 08:58

runtime: create userns when CAP_SYS_ADMIN is not present

722ea2f

when deciding to create a user namespace, check for CAP_SYS_ADMIN instead of looking at the euid. [NO TESTS NEEDED] Needs nested Podman Signed-off-by: Giuseppe Scrivano <[email protected]>

giuseppe force-pushed the create-userns-for-root-without-CAP_SYS_ADMIN branch from 498164e to 722ea2f Compare April 26, 2021 06:59

openshift-ci-robot assigned umohnani8 Apr 26, 2021

openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Apr 26, 2021

openshift-merge-robot merged commit 333817a into containers:master Apr 26, 2021

nalind mentioned this pull request Jun 10, 2021

podman build error creating new mount namespace #9222

Closed

github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 23, 2023

github-actions bot locked as resolved and limited conversation to collaborators Sep 23, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

runtime: create userns when CAP_SYS_ADMIN is not present #10079

runtime: create userns when CAP_SYS_ADMIN is not present #10079

giuseppe commented Apr 19, 2021

giuseppe commented Apr 19, 2021

Luap99 Apr 19, 2021

rhatdan Apr 19, 2021

giuseppe Apr 19, 2021

giuseppe Apr 19, 2021

rhatdan Apr 19, 2021

mheon commented Apr 19, 2021

rhatdan commented Apr 19, 2021

giuseppe commented Apr 20, 2021

giuseppe commented Apr 22, 2021

rhatdan left a comment

openshift-ci-robot commented Apr 22, 2021

rhatdan commented Apr 22, 2021

mheon commented Apr 22, 2021

rhatdan commented Apr 22, 2021

rhatdan commented Apr 24, 2021

giuseppe commented Apr 26, 2021

rhatdan commented Apr 26, 2021

rhatdan commented Apr 26, 2021

umohnani8 commented Apr 26, 2021

runtime: create userns when CAP_SYS_ADMIN is not present #10079

runtime: create userns when CAP_SYS_ADMIN is not present #10079

Conversation

giuseppe commented Apr 19, 2021

giuseppe commented Apr 19, 2021

Luap99 Apr 19, 2021

Choose a reason for hiding this comment

rhatdan Apr 19, 2021

Choose a reason for hiding this comment

giuseppe Apr 19, 2021

Choose a reason for hiding this comment

giuseppe Apr 19, 2021

Choose a reason for hiding this comment

rhatdan Apr 19, 2021

Choose a reason for hiding this comment

mheon commented Apr 19, 2021

rhatdan commented Apr 19, 2021

giuseppe commented Apr 20, 2021

giuseppe commented Apr 22, 2021

rhatdan left a comment

Choose a reason for hiding this comment

openshift-ci-robot commented Apr 22, 2021

rhatdan commented Apr 22, 2021

mheon commented Apr 22, 2021

rhatdan commented Apr 22, 2021

rhatdan commented Apr 24, 2021

giuseppe commented Apr 26, 2021

rhatdan commented Apr 26, 2021

rhatdan commented Apr 26, 2021

umohnani8 commented Apr 26, 2021