Skip to content

[merged] Only --unshare-user automatically if we're not root#123

Closed
cgwalters wants to merge 2 commits intocontainers:masterfrom
cgwalters:unshare-user-reg
Closed

[merged] Only --unshare-user automatically if we're not root#123
cgwalters wants to merge 2 commits intocontainers:masterfrom
cgwalters:unshare-user-reg

Conversation

@cgwalters
Copy link
Collaborator

#122 introduced a
regression for the case of rpm-ostree running bubblewrap on CentOS 7.

Previously the is_privileged variable captured whether or not
our uid was 0, now it captures whether we're setuid.

This bit of code enabled --unshare-user automatically if we're not
privileged, but we suddenly started doing that for running as real uid
0 (CAP_SYS_ADMIN), which we don't want, since on CentOS/RHEL 7 today
userns isn't even available to root without a module parameter and
reboot.

So, let's just do this only if not setuid and we're not uid 0
(really we should check "have CAP_SYS_ADMIN" but eh).

containers#122 introduced a
regression for the case of rpm-ostree running bubblewrap on CentOS 7.

Previously the `is_privileged` variable captured whether or not
our uid was 0, now it captures whether we're setuid.

This bit of code enabled `--unshare-user` automatically if we're not
privileged, but we suddenly started doing that for running as real uid
0 (CAP_SYS_ADMIN), which we don't want, since on CentOS/RHEL 7 today
userns isn't even available to root without a module parameter and
reboot.

So, let's just do this only if not setuid *and* we're not uid 0
(really we should check "have CAP_SYS_ADMIN" but eh).
@alexlarsson
Copy link
Collaborator

@rh-atomic-bot r+ 8f30d69

@rh-atomic-bot
Copy link

⌛ Testing commit 8f30d69 with merge 2a408e8...

@rh-atomic-bot
Copy link

☀️ Test successful - status-redhatci
Approved by: alexlarsson
Pushing 2a408e8 to master...

@rh-atomic-bot rh-atomic-bot changed the title Only --unshare-user automatically if we're not root [merged] Only --unshare-user automatically if we're not root Dec 5, 2016
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.

3 participants