Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

os: TestChown and friends fail in a user namespace #42525

Closed
prattmic opened this issue Nov 11, 2020 · 4 comments
Closed

os: TestChown and friends fail in a user namespace #42525

prattmic opened this issue Nov 11, 2020 · 4 comments
Assignees
Labels
NeedsFix The path to resolution is known, but the work has not been done. Testing An issue that has been verified to require only test changes, not just a test failure.
Milestone

Comments

@prattmic
Copy link
Member

What version of Go are you using (go version)?

$ go version
go version devel +b641f0dcf4 Wed Nov 11 20:26:44 2020 +0000 linux/amd64

Does this issue reproduce with the latest release?

Yes, this is not a new issue.

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/usr/local/google/home/mpratt/.cache/go-build"
GOENV="/usr/local/google/home/mpratt/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/usr/local/google/home/mpratt/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/usr/local/google/home/mpratt/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/google/home/mpratt/src/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/google/home/mpratt/src/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="devel +b641f0dcf4 Wed Nov 11 20:26:44 2020 +0000"
GCCGO="gccgo"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/usr/local/google/home/mpratt/src/go/src/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build140714091=/tmp/go-build -gno-record-gcc-switches"

What did you do?

$ unshare -U ../bin/go test os

What did you expect to see?

All tests pass.

What did you see instead?

$ unshare -U ../bin/go test os   
--- FAIL: TestChown (0.00s)
    os_unix_test.go:58: gid: 65534
    os_unix_test.go:60: chown /tmp/_Go_TestChown188607266 -1 65534: chown /tmp/_Go_TestChown188607266: invalid argument
--- FAIL: TestFileChown (0.00s)
    os_unix_test.go:101: gid: 65534
    os_unix_test.go:103: fchown /tmp/_Go_TestFileChown888841753 -1 65534: chown /tmp/_Go_TestFileChown888841753: invalid argument
--- FAIL: TestLchown (0.00s)
    os_unix_test.go:153: gid: 65534
    os_unix_test.go:158: lchown /tmp/_Go_TestLchown8979621482 -1 65534: lchown /tmp/_Go_TestLchown8979621482: invalid argument
FAIL
FAIL    os      18.447s
FAIL

These tests call Getgroups and then tries to chown files to all of those groups.

However, if the test is running in a user namespace (e.g., restricted CI environment), some of the groups may be OVERFLOWGID (65534) which is not a valid gid to use in chown, thus making the test fail.

@navytux
Copy link
Contributor

navytux commented Dec 6, 2021

For the reference TestSCMCredentials also fails under unshare -Um --map-current-user:

=== RUN   TestSCMCredentials
    creds_test.go:81: WriteMsgUnix failed with invalid argument, want EPERM
--- FAIL: TestSCMCredentials (0.00s)

Worked it around with the following patch:

From: Kirill Smelkov <kirr@nexedi.com>
Date: Mon, 6 Dec 2021 22:50:27 +0300
Subject: [PATCH] syscall: tests: Fix TestSCMCredentials for `unshare -Umc`

---
 src/syscall/creds_test.go | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/syscall/creds_test.go b/src/syscall/creds_test.go
index c1a8b516e8..ed6e80c0c3 100644
--- a/src/syscall/creds_test.go
+++ b/src/syscall/creds_test.go
@@ -78,8 +78,10 @@ func TestSCMCredentials(t *testing.T) {
                        if sys, ok := err.(*os.SyscallError); ok {
                                err = sys.Err
                        }
-                       if err != syscall.EPERM {
-                               t.Fatalf("WriteMsgUnix failed with %v, want EPERM", err)
+                       // can get EINVAL instead of EPERM under `unshare -Umc` because uid0 is not mapped and maps to -1
+                       // see also https://github.com/golang/go/issues/42525
+                       if !(err == syscall.EPERM || err == syscall.EINVAL) {
+                               t.Fatalf("WriteMsgUnix failed with %v, want EPERM/EINVAL", err)
                        }
                }

-- 
2.30.2

NexediGitlab pushed a commit to SlapOS/slapos that referenced this issue Dec 10, 2021
If we enter user namespace via regular unshare without help from SUID
newuidmap/newgidmap, all supplementary groups are mapped to -1. As the result
when Go test tries to chown to a supplementary group, it gets EINVAL:

golang/go#42525

-> work it around with patch to skip this chown tests.

A more proper, longer-term fix would be to fix Linux kernel to allow writes to
/proc/self/gid_map to setup mapping not only to original gid, but to all
original supplementary groups as well here:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/kernel/user_namespace.c?id=v5.16-rc4-0-g0fcfb00b28c0#n1143

this fix, even if accepted by upstream, would be long to be waited for to
propagate to distribution kernels that we currently use. So we go with this
workaround for now.

--------

Another patch is to fix the following TestSCMCredentials failure:

    === RUN   TestSCMCredentials
        creds_test.go:81: WriteMsgUnix failed with invalid argument, want EPERM
    --- FAIL: TestSCMCredentials (0.00s)

There the code tries to send uid0/gid0 credentials from non-zero uid and
expects EPERM reject from kernel. However under `unshare -Umc` uid0/gid0 are
not mapped to anywhere and so implicitly map to -1 and are rejected with EINVAL
by the kernel.

/reviewed-by @jerome
/reviewed-on https://lab.nexedi.com/nexedi/slapos/merge_requests/1095
zowoq pushed a commit to c00w/nixpkgs that referenced this issue Feb 3, 2022
zowoq pushed a commit to NixOS/nixpkgs that referenced this issue Feb 4, 2022
zowoq pushed a commit to NixOS/nixpkgs that referenced this issue Feb 4, 2022
kalbasit pushed a commit to NixOS/nixpkgs that referenced this issue May 28, 2022
Workaround for <golang/go#42525>

(Also related to <NixOS/nix#3245>)

(cherry picked from commit af3cd7c)
kalbasit pushed a commit to NixOS/nixpkgs that referenced this issue May 28, 2022
Workaround for <golang/go#42525>

(Also related to <NixOS/nix#3245>)

(cherry picked from commit a66d9c8)
@seankhliao seankhliao added this to the Unplanned milestone Aug 27, 2022
rkaippully pushed a commit to awakesecurity/nixpkgs that referenced this issue Jul 3, 2023
jsoo1 pushed a commit to awakesecurity/nixpkgs that referenced this issue Jul 3, 2023
@levinzimmermannn
Copy link

For the reference TestSCMCredentials also fails under unshare -Um --map-current-user

Also for the reference: this has been applied/fixed upstream with f839aaa (included in go >= 1.19).

@bcmills bcmills added the Testing An issue that has been verified to require only test changes, not just a test failure. label Aug 16, 2023
@bcmills bcmills self-assigned this Aug 16, 2023
@bcmills bcmills removed this from the Unplanned milestone Aug 16, 2023
@bcmills bcmills added this to the Go1.22 milestone Aug 16, 2023
@gopherbot
Copy link

Change https://go.dev/cl/520055 mentions this issue: os: skip Chown tests for auxiliary groups that fail due to permission errors

@bcmills
Copy link
Contributor

bcmills commented Aug 21, 2023

I believe this is fixed by https://go.dev/cl/520055.

@bcmills bcmills closed this as completed Aug 21, 2023
NexediGitlab pushed a commit to SlapOS/slapos that referenced this issue Dec 28, 2023
Go1.21 already has the fifth minor revision (released 2023-12-05) and
should therefore already be sufficiently stable. Furthermore we need it
to fix a bug in a NEO/go dependency [1].

Please find all details in the official release note:

    https://go.dev/doc/go1.21

It was released on 2023-08-08 [2]. Due to the go promise of compatibility
most software should still compile without any problems.

In golang < 1.21 we needed to patch golang to fix
golang/go#42525. In golang 1.21 we still need
to apply a fix, but can't apply the old patch because the code changed.
In golang > 1.21 this problem is already fixed with https://go-review.googlesource.com/c/go/+/520055.

Because of golang/go@0926714
'TestUnshareMountNameSpace' fails on golang 1.21 [3]. In golang 1.20 this test
was skipped [4]. To fix this failure the additional patch
'skip-unshare-mount-test.patch' has been added.

---

[1] https://lab.nexedi.com/nexedi/wendelin.core/merge_requests/22#note_195769]
[2] https://go.dev/doc/devel/release
[3] --- FAIL: TestUnshareMountNameSpace (0.18s)
    exec_linux_test.go:243: unshare failed: exit status 2
        unshare: mount /tmp/TestUnshareMountNameSpace2210137852/001 failed: 0x1
[4] === RUN TestUnshareMountNameSpace exec_linux_test.go:333: kernel prohibits unshare in unprivileged process, unless using user namespace — SKIP: TestUnshareMountNameSpace (0.00s)

/reviewed-by @kirr and @jerome
/reviewed-on https://lab.nexedi.com/nexedi/slapos/merge_requests/1494
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsFix The path to resolution is known, but the work has not been done. Testing An issue that has been verified to require only test changes, not just a test failure.
Projects
None yet
Development

No branches or pull requests

6 participants