]> Cypherpunks.ru repositories - gostls13.git/commit
syscall: avoid making assumptions about syscall permissions
authorBryan C. Mills <bcmills@google.com>
Thu, 8 Dec 2022 20:23:32 +0000 (15:23 -0500)
committerGopher Robot <gobot@golang.org>
Fri, 27 Jan 2023 17:37:09 +0000 (17:37 +0000)
commit092671423cd95eaa6df93eb29442fef41504d097
tree71a7a72b99ecabaa4568cc26cbc94bfcea949b80
parentc8c646d31bec3cbe56ecf5a26fbbd235c97cfb21
syscall: avoid making assumptions about syscall permissions

We currently check for at least three different permission bits before
running tests that require root permissions: we look for UID 0, lack
of an LXC container, and lack of a Docker container, and probe a
number of distro-specific files in /proc and /sys.

The sheer number of these checks suggests that we have probably missed
at least one. Per Alan J. Perlis, “If you have a procedure with ten
parameters, you probably missed some.” (And, indeed, we definitely
have: a Debian patch¹ adds one more environment check!)

CL 58170 added some of these container checks, but “decided to go this
way instead of just skipping os.IsPermission errors because many of
those tests were specifically written to check false positive
permission errors.” However, we can't in general distinguish between a
false-positive error and a real one caused by a container: if one is
making a change to the syscall package, they should run the tests with
-v and check for unexpected skips.

Notably:

- TestUnshare already skips itself if the command fails with an error
  ending in the string "operation not permitted", which could be caused
  by a variety of possible bugs.

- The Unshare tests added in CL 38471 will fail with a permission
  error if CLONE_NEWNS is not supported, but it seems to me that if
  CLONE_NEWNS is supported — sufficient to start the process! — then
  Unmount must also be supported, and the test can at least check that
  the two are consistent.

- The AmbientCaps tests should fail to start the subprocess with
  EINVAL or similar (not produce bogus output) if the kernel does not
  support ambient caps for any reason, which we can then detect.
  (If the subprocess fails in the way the test is concerned about, it
  will exit with status 2, not fail to start in the first place.)

By executing the system calls and checking for permission errors,
this change exposed an existing bug for AmbientCaps (filed as #57208),
which was detected by the linux-arm-aws builder.

For #57208.
Updates #21379.
Updates #14693.

¹https://sources.debian.org/patches/golang-1.19/1.19.3-1/0006-skip-userns-test-in-schroot-as-well.patch/

Change-Id: I9b167661fa1bb823168c8b50d8bbbf9643e49f76
Reviewed-on: https://go-review.googlesource.com/c/go/+/456375
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Alexander Morozov <lk4d4math@gmail.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
src/syscall/exec_linux_test.go