libct: fix user ns join order for rootful container

[lifubang]

Fix: #4390

If we are rootless and there are userns-owned namespaces, we need to be in the
userns in order to have the necessary permissions to do setns. This is what
2cd9c31 fixed.
But for rootful container, if we join the userns in first step, we may can't
join some namespaces which have no permissions for the userns we have joined.

Signed-off-by: lifubang [email protected]

[cyphar]

I don't think this is the nicest solution for #4390. As I suggested, we should do what nsenter does. I had a draft of this but only just got around to sending it -- see #4492.

You don't need to move the joining to after unshare(CLONE_NEWUSER) -- each namespace has an owner user namespace, and so joining a newly created one will not give you extra permissions for any other namespaces. This patchset works because you are accidentally doing a two-stage join like #4390 but the second stage is after unshare(CLONE_NEWUSER).

I'm biased, but I think #4492 is cleaner.

[lifubang]

This patchset works because you are accidentally doing a two-stage join like #4390 but the second stage is after unshare(CLONE_NEWUSER).

Really? I think there is no unshare(CLONE_NEWUSER) call here. Because we are using setns to join a existing user ns here for the original issue and my testcase.

[cyphar]

We don't do unshare(CLONE_NEWUSER) but you've moved the second call to join_namespaces to be after the unshare(CLONE_NEWUSER) block and that isn't necessary. #4492 is the way I'd do it.

[lifubang]

but you've moved the second call to join_namespaces to be after the unshare(CLONE_NEWUSER) block and that isn't necessary.

Ah, yes, it is not really necessary to do that.

[lifubang]

@cyphar As you described in #4492 (comment), we can just only need to adjust the ns type order for rootful container? I think it's more simple to do it in go code. Do you think this is enough to fix the issue?

[lifubang]

I test your test case in #4492 with this PR, it can work.

bats ./tests/integration/userns.bats 
userns.bats
 ✓ userns with simple mount
 ✓ userns with 2 inaccessible mounts
 ✓ userns with inaccessible mount + exec
 - userns with bind mount before a cgroupfs mount (skipped: test requires cgroups_v1)
 ✓ userns join other container userns
 - userns join other container userns[selinux enabled] (skipped: requires SELinux enabled and in enforcing mode)
 ✓ userns join other container userns [bind-mounted nsfd]
 ✓ userns join external namespaces [wrong userns owner]

[rata]

Is there agreement if we should pursue this PR or #4492?