Skip to content

Conversation

@TomSweeneyRedHat
Copy link
Member

Addresses the multiple "default" userns values found
in the podman-run(1) man page: http://docs.podman.io/en/latest/markdown/podman-run.1.html.

This in response to: https://bugzilla.redhat.com/show_bug.cgi?id=1860126
which this PR wil fix.

Signed-off-by: TomSweeneyRedHat tsweeney@redhat.com

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 23, 2020
@TomSweeneyRedHat
Copy link
Member Author

@giuseppe @mheon, please verify my changes are in line. That's what I could gronk from the code, but the default userns value setting is not easily parseable.

Also I did not test the change concerning the USER in the containerfile, but believe that is the case. Please holler if not, and perhaps we/I should add a test to verify? I'm not sure we have one atm.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe this should read "Without this argument the command will be run as root on the host unless a user was specified by the container image or the --user option"

Copy link
Member

Choose a reason for hiding this comment

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

Not root on the host, since it could be rootless or running in a user namespace. It will also run as the User of the image. So the wording of this should be

Without this argument the command will run as the user specified in the container image. Note that this user often defaults to root. The actual UID of the process running in the container is based on the User namespace the container is running in. By default if you are running a rootful Podman, the container will run as the hosts root. If you are running the container as a rootless user, the root inside of the container is actually the users UID.

Copy link
Member Author

@TomSweeneyRedHat TomSweeneyRedHat Jul 24, 2020

Choose a reason for hiding this comment

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

I think it's important to get the USER/--user option thought in. So I've tweaked the comment from @rhatdan to:

Without this argument, the command will run as the user specified in the container image. Unless 
overridden by a `USER` command in the Containerfile or by a value passed to this option, this 
user generally defaults to root. The actual UID of the process running in the container is based on
 the User namespace the container is running in. By default, if you are running a rootful Podman, 
the container will run using the UID of the host's root. If you are running the container as a 
rootless user, by default the UID used by root inside of the container is the rootless user's UID.

Copy link
Member

Choose a reason for hiding this comment

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

This is the default, not private

Choose a reason for hiding this comment

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

How about running as root vs non-root? Is this default the same for non-root too? I thought that running as non-root creates a new userns.

Copy link
Member

Choose a reason for hiding this comment

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

This is difficult to explain.

Root inside of the rootless container is your UID, so theoretically it has the same privs as a process run by your user.
Bottom line all containers will run within the same user namespace, and you will gain no separation advantage between containers launched by the same user. All of the processes in containers will run within the same range of UIDs that other containers you launch run with.

$ podman run --userns=host alpine cat /proc/self/uid_map
         0       3267          1
         1     100000      65536
$ podman run --userns=host alpine cat /proc/self/uid_map
         0       3267          1
         1     100000      65536

Copy link
Member Author

Choose a reason for hiding this comment

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

This is the default, not private

Thanks, I was looking at the wrong field with a default "Private" value (userns mode) when I change this. I've changed the default to host.

Copy link
Member

Choose a reason for hiding this comment

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

the last sentence is not correct, An empty value ("") means user namespaces are disabled unless an explicit mapping is set with --uidmapping and --gidmapping

Copy link
Member Author

@TomSweeneyRedHat TomSweeneyRedHat Jul 24, 2020

Choose a reason for hiding this comment

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

Thanks @giuseppe . I've added: unless an explicit mapping is set with --uidmapping and --gidmapping
to the end of that sentence as suggested.

@rhatdan
Copy link
Member

rhatdan commented Jul 25, 2020

LGTM

@akostadinov
Copy link

akostadinov commented Jul 25, 2020

btw I can't see any other option but host work on Fedora 32:

$ podman run --rm=true -ti --user=root --userns=private registry.access.redhat.com/ubi8/openjdk-11 /bin/bash -l
Error: --userns "private" is not valid
$ podman run --rm=true -ti --user=root --userns=auto registry.access.redhat.com/ubi8/openjdk-11 /bin/bash -l
Error: --userns "auto" is not valid
$ podman run --rm=true -ti --user=root --userns=auto:size=8192 registry.access.redhat.com/ubi8/openjdk-11 /bin/bash -l
Error: --userns "auto:size=8192" is not valid
$ podman run --rm=true -ti --user=root --userns=host registry.access.redhat.com/ubi8/openjdk-11 /bin/bash -l
[root@f5bca840ffa6 ~]# exit
logout
$ podman --version
podman version 1.7.0

@rhatdan
Copy link
Member

rhatdan commented Jul 25, 2020

You should update the version of podman you are using on fedora 32.

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'm going to change and re-push a slightly amended second sentence:

Suggested change
Set the user namespace mode for the container. It defaults to the **PODMAN_USERNS** environment variable. An empty value ("") means user namespaces are disabled unless an explicit mapping is set with --uidmapping and --gidmapping.
Set the user namespace mode for the container. It defaults to the **PODMAN_USERNS** environment variable. An empty value ("") means user namespaces are disabled unless an explicit mapping is set with the `--uidmapping` and `--gidmapping` options.

@TomSweeneyRedHat
Copy link
Member Author

All green and I think I've addressed all comments. @mheon, could you do a final review and push if it LGTY please?

Copy link
Member

Choose a reason for hiding this comment

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

I don't know if this paragraph makes much sense. We shouldn't be focusing on the user the container is run as, but that root in the container is or is not root on the host.

Copy link
Member Author

@TomSweeneyRedHat TomSweeneyRedHat Jul 29, 2020

Choose a reason for hiding this comment

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

This whole paragraph started from a question ni the BZ: "Namely should lack of --user option result in container running as specified by USER in Dockerfile? This is what I observe.'

I started the first bit of the USER, then Dan suggested the bit about the UID too and I tweaked it slightly. I'm happy to make it anything that folks see fit. Would it work if I just remove the last bit talking about the UID?

Without this argument, the command will run as the user specified in the 
container image. Unless overridden by a `USER` command in the 
Containerfile or by a value passed to this option, this user generally 
defaults to root. 

Copy link
Member

Choose a reason for hiding this comment

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

This sounds like two separate issues.

We need to beef up the documentation of --user to detail what happens there; and we need to improve the documentation here to specify what happens when user namespaces are involved. Intermingling the two here seems nonsensical from a reader's perspective, especially given that user namespaces do not just alter the mapping of the user the container is run as, but all users in the container.

Copy link
Member Author

Choose a reason for hiding this comment

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

That is what I've attempted to do here. Apparently I've missed the mark. Please give me suggested changes as I'm not sure what to change at this point.

Copy link
Member

Choose a reason for hiding this comment

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

I think we need to separate the bit about user/Dockerfile USER and put it in the description of the --user flag. This section should focus entirely on how a userns will result in users inside the container having different IDs from users outside the container, how this depends on the exact UID mappings of the user namespace, and how user namespaces are default for rootless and optional for root. Will write more tomorrow.

Copy link
Member Author

Choose a reason for hiding this comment

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

@mheon, this is currently under the --user option, not the --userns option. Did you get a chance to tickle the keyboard with your thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

Having this bit under --user and the rest under --userns does satisfy my request.

Still, the documentation here seems overly focused on root. Should probably be more generic - "When a user namespace is not in use, the UID and GID used within the container and on the host will match. When user namespaces are in use, however, the UID and GID in the container may correspond to another UID and GID on the host. In rootless containers, for example, a user namespace is always used, and root in the container will by default correspond to the UID and GID of the container invoking Podman."

Copy link
Member Author

Choose a reason for hiding this comment

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

Matt, I've removed this:

When a user namespace is not in use, the UID and GID used within the container and on the host will match. 
When user namespaces are in use, however, the UID and GID in the container may correspond to another 
UID and GID on the host. In rootless containers, for example, a user namespace is always used, and root in
the container will by default correspond to the UID and GID of the container invoking Podman.

From the original paragraph. I then took what you have above and made it a new second paragraph under user, BUT, I changed the second from last word from container to user

PTAL

Addresses the multiple "default" userns values found
in the podman-run(1) man page:  http://docs.podman.io/en/latest/markdown/podman-run.1.html.

This in response to: https://bugzilla.redhat.com/show_bug.cgi?id=1860126
which this PR wil fix.

Signed-off-by: TomSweeneyRedHat <tsweeney@redhat.com>
@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: TomSweeneyRedHat

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:

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

@mheon
Copy link
Member

mheon commented Aug 7, 2020

LGTM, thanks!

@rhatdan
Copy link
Member

rhatdan commented Aug 8, 2020

@TomSweeneyRedHat Great job.
/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Aug 8, 2020
@openshift-merge-robot openshift-merge-robot merged commit 1298161 into containers:master Aug 8, 2020
@TomSweeneyRedHat TomSweeneyRedHat deleted the dev/tsweeney/runman branch August 14, 2020 19:04
@github-actions 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 24, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants