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

Runc 1.1.4 container fails with permission denied: unknown when relying on capabilities #3715

Closed
Kern-- opened this issue Jan 30, 2023 · 9 comments · Fixed by #3753
Closed

Comments

@Kern--
Copy link

Kern-- commented Jan 30, 2023

After upgrading from runc 1.1.3 to runc 1.1.4 we started to see a particular container fail with an error like:

FATA[0000] failed to create shim task: OCI runtime create failed: runc create failed: unable to start container process: exec /entrypoint.sh: permission denied: unknown

The same container runs successfully with runc 1.1.3.

Upon inspection of the container, we discoverd that the entrypoint's permissions were set to 744, but it was owned by root instead of the container's user. We were able to fix the issue by correctly chowning the entrypoint script, but the upgrade caused an outage, so I'm reporting it here as a bug.

Root Cause
I traced the cause down to this PR #3522

In that PR, a new check was introduced to exit early if the entrypoint isn't executable. It does so by

  1. Attempting to use faccessat2(2) to check the effective permissions if available (requires kernel 5.8+ and libseccomp 2.4.0+)
  2. Falling back to access(2) to check the real users permission if faccessat2(2) isn't available.

We were hitting case 2. The problem is that this check is more restrictive than execve(2) because access(2) removes capabilities when checking for permission.

Docker and containerd have a set of default capabilities that they add to the runtime spec. Both include CAP_DAC_OVERRIDE in this set. See containerd's list, docker's list.
The capabilities man page says this about CAP_DAC_OVERRIDE

Bypass file read, write, and execute permission checks.  (DAC is an abbreviation of "discretionary access control".)

So in runc 1.1.3, execve worked because runc has the CAP_DAC_OVERRIDE capability to ignore the fact that the user doesn't actually have execute permission. In runc 1.1.4, the addtional access(2) check striped capabilities when checking permissions and caused runc to exit with permission denied: unknown.

Conditions for this bug

  1. kernel < 5.8 or libseccomp < 2.4.0
  2. An entry point script where the owner has execute permission, but the container user has readonly permission
  3. CAP_DAC_OVERRIDE or similar capabilities set (which is the default for both containerd and docker)

Reproduction

entrypoint.sh

#!/bin/sh

echo "Hello, World!"

Dockerfile

FROM alpine:latest

COPY ./entrypoint.sh /entrypoint.sh
RUN  chown root:root /entrypoint.sh\
      && chmod 744 /entrypoint.sh

RUN adduser --disabled-password testuser
USER testuser
ENTRYPOINT ["/entrypoint.sh"]

When run with runc 1.1.3:

$ sudo nerdctl run --rm -it --net=none runc114error:latest
Hello, World!

When run with runc 1.1.4:

$ sudo nerdctl run --rm -it --net=none runc114error:latest
FATA[0000] failed to create shim task: OCI runtime create failed: runc create failed: unable to start container process: exec /entrypoint.sh: permission denied: unknown
@kolyshkin kolyshkin added this to the 1.1.5 milestone Feb 9, 2023
kolyshkin added a commit to kolyshkin/runc that referenced this issue Feb 14, 2023
This is a test case for issue reported as opencontainers#3715. In short, even if a
(non-root) user that the container is run as does not have execute
permission bit set for the executable, it should still work in case runc
has the CAP_DAC_OVERRIDE capability set.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
@kolyshkin
Copy link
Contributor

A quick heads up.

Looking into this for the past few days, the fix is to take into account whether CAP_DAC_OVERRIDE is set, which is sort of trivial, but for Go 1.20, we will see this very error from exec.LookPath thanks to https://go-review.googlesource.com/c/go/+/414824 (which made its way into Go 1.20).

@AkihiroSuda suggested reverting #3522 for 1.1.5, but it is not enough because in Go 1.20 this won't work.

So, this needs to be fixed in Go 1.20 as well, and I am working on it. Again, the fix is sort of trivial, but the test case is not.

@kolyshkin
Copy link
Contributor

Bug reported to golang: golang/go#58552

The fix (to golang) is here: https://go.dev/cl/468735

gopherbot pushed a commit to golang/go that referenced this issue Feb 17, 2023
CL 416115 added using faccessat2(2) from syscall.Faccessat on Linux
(which is the only true way to implement AT_EACCESS flag handing),
if available. If not available, it uses some heuristics to mimic the
kernel behavior, mostly taken from glibc (see CL 126415).

Next, CL 414824 added using the above call (via unix.Eaccess) to
exec.LookPath in order to check if the binary can really be executed.

As a result, in a very specific scenario, described below,
syscall.Faccessat (and thus exec.LookPath) mistakenly tells that the
binary can not be executed, while in reality it can be. This makes
this bug a regression in Go 1.20.

This scenario involves all these conditions:
 - no faccessat2 support available (i.e. either Linux kernel < 5.8,
   or a seccomp set up to disable faccessat2);
 - the current user is not root (i.e. geteuid() != 0);
 - CAP_DAC_OVERRIDE capability is set for the current process;
 - the file to be executed does not have executable permission
   bit set for either the current EUID or EGID;
 - the file to be executed have at least one executable bit set.

Unfortunately, this set of conditions was observed in the wild -- a
container run as a non-root user with the binary file owned by root with
executable permission set for a user only [1]. Essentially it means it
is not as rare as it may seem.

Now, CAP_DAC_OVERRIDE essentially makes the kernel bypass most of the
checks, so execve(2) and friends work the same was as for root user,
i.e. if at least one executable bit it set, the permission to execute
is granted (see generic_permission() function in the Linux kernel).

Modify the code to check for CAP_DAC_OVERRIDE and mimic the kernel
behavior for permission checks.

[1] opencontainers/runc#3715

Fixes #58552.

Change-Id: I82a7e757ab3fd3d0193690a65c3b48fee46ff067
Reviewed-on: https://go-review.googlesource.com/c/go/+/468735
Reviewed-by: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
gopherbot pushed a commit to golang/sys that referenced this issue Feb 17, 2023
CL 126516 added support for flags argument, implemented in the same way
as glibc does (it tries to guess what the kernel would do).

CL 246537 added using faccess2(2) Linux syscall which supports the flags
directly. For older kernels, though, the syscall is not available, and
the code uses glibc-like fallback.

There is one very specific scenario in which the fallback fails.
The scenario involves all these conditions:
 - no faccessat2 support available (i.e. either Linux kernel < 5.8,
   or a seccomp set up to disable faccessat2);
 - the current user is not root (i.e. geteuid() != 0);
 - CAP_DAC_OVERRIDE capability is set for the current process;
 - the file to be executed does not have executable permission
   bit set for either the current EUID or EGID;
 - the file to be executed have at least one executable bit set.

Unfortunately, this set of conditions was observed in the wild -- a
container run as a non-root user with the binary file owned by root with
executable permission set for a user only [1]. Essentially it means it
is not as rare as it may seem.

Now, CAP_DAC_OVERRIDE essentially makes the kernel bypass most of the
checks, so execve(2) and friends work the same was as for root user,
i.e. if at least one executable bit it set, the permission to execute
is granted (see generic_permission() function in the Linux kernel).

Modify the code to check for CAP_DAC_OVERRIDE and mimic the kernel
behavior for permission checks.

This is essentially the same fix as CL 468735 for Go syscall package.

Tested on CentOS 7 with the repro similar to the one from [2].

[1] opencontainers/runc#3715
[2] golang/go#58552 (comment)

Change-Id: I726b6acab6a6e6d0358ef98e6a582b405c347614
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Reviewed-on: https://go-review.googlesource.com/c/sys/+/468877
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
@kolyshkin
Copy link
Contributor

The fix to Golang has landed into the master branch; backport to Go 1.20.x is initiated: golang/go#58624

johanbrandhorst pushed a commit to Pryz/go that referenced this issue Feb 22, 2023
CL 416115 added using faccessat2(2) from syscall.Faccessat on Linux
(which is the only true way to implement AT_EACCESS flag handing),
if available. If not available, it uses some heuristics to mimic the
kernel behavior, mostly taken from glibc (see CL 126415).

Next, CL 414824 added using the above call (via unix.Eaccess) to
exec.LookPath in order to check if the binary can really be executed.

As a result, in a very specific scenario, described below,
syscall.Faccessat (and thus exec.LookPath) mistakenly tells that the
binary can not be executed, while in reality it can be. This makes
this bug a regression in Go 1.20.

This scenario involves all these conditions:
 - no faccessat2 support available (i.e. either Linux kernel < 5.8,
   or a seccomp set up to disable faccessat2);
 - the current user is not root (i.e. geteuid() != 0);
 - CAP_DAC_OVERRIDE capability is set for the current process;
 - the file to be executed does not have executable permission
   bit set for either the current EUID or EGID;
 - the file to be executed have at least one executable bit set.

Unfortunately, this set of conditions was observed in the wild -- a
container run as a non-root user with the binary file owned by root with
executable permission set for a user only [1]. Essentially it means it
is not as rare as it may seem.

Now, CAP_DAC_OVERRIDE essentially makes the kernel bypass most of the
checks, so execve(2) and friends work the same was as for root user,
i.e. if at least one executable bit it set, the permission to execute
is granted (see generic_permission() function in the Linux kernel).

Modify the code to check for CAP_DAC_OVERRIDE and mimic the kernel
behavior for permission checks.

[1] opencontainers/runc#3715

Fixes golang#58552.

Change-Id: I82a7e757ab3fd3d0193690a65c3b48fee46ff067
Reviewed-on: https://go-review.googlesource.com/c/go/+/468735
Reviewed-by: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
kolyshkin added a commit to kolyshkin/runc that referenced this issue Feb 28, 2023
This is a test case for issue reported as opencontainers#3715. In short, even if a
(non-root) user that the container is run as does not have execute
permission bit set for the executable, it should still work in case runc
has the CAP_DAC_OVERRIDE capability set.

Since the upstream golang is also broken (see [1]), let's skip this test
for Go 1.20 and 1.20.1 (and hope it'll be fixed in Go 1.20.2 as per [2].

[1] https://go.dev/issue/58552
[2] https://go-review.googlesource.com/c/go/+/469956

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
kolyshkin added a commit to kolyshkin/runc that referenced this issue Feb 28, 2023
Since this commit was made, a few things happened:

- a similar functionality appeared in go 1.20 [1]
- a bug in runc was found [2], which also affects go [3]
- the bug was fixed in go 1.21 [4] and 1.20.2 [5]
- a similar fix was made to x/sys/unix.Faccessat [6]

Revert commit 957d97b
so we can fix the bug [2] when go > 1.21.1 is used.
Note that this will reintroduce the older bug [7]
when the older go version is used, but since this
is a minor bug which will be fixed once everyone
switches to a recent go version, let's keep things
simple and not introduce any complex code here.

[1] https://go-review.googlesource.com/c/go/+/414824
[2] opencontainers#3715
[3] https://go.dev/issue/58552
[4] https://go-review.googlesource.com/c/go/+/468735
[5] https://go-review.googlesource.com/c/go/+/469956
[6] https://go-review.googlesource.com/c/sys/+/468877
[7] opencontainers#3520

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
@kolyshkin
Copy link
Contributor

OK this is how we're fixing this: #3753

kolyshkin added a commit to kolyshkin/runc that referenced this issue Feb 28, 2023
Since this commit was made, a few things happened:

- a similar functionality appeared in go 1.20 [1], so the issue
  mentioned in the comment (being removed) is no longer true;
- a bug in runc was found [2], which also affects go [3];
- the bug was fixed in go 1.21 [4] and 1.20.2 [5];
- a similar fix was made to x/sys/unix.Faccessat [6].

Revert commit 957d97b
so we can fix the bug [2] when go > 1.21.1 is used.
Note that this will reintroduce the older bug [7]
when the older go version is used, but since this
is a minor bug which will be fixed once everyone
switches to a recent go version, let's keep things
simple and not introduce any complex code here.

[1] https://go-review.googlesource.com/c/go/+/414824
[2] opencontainers#3715
[3] https://go.dev/issue/58552
[4] https://go-review.googlesource.com/c/go/+/468735
[5] https://go-review.googlesource.com/c/go/+/469956
[6] https://go-review.googlesource.com/c/sys/+/468877
[7] opencontainers#3520

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
kolyshkin added a commit to kolyshkin/runc that referenced this issue Feb 28, 2023
This is a test case for issue reported as opencontainers#3715. In short, even if a
(non-root) user that the container is run as does not have execute
permission bit set for the executable, it should still work in case runc
has the CAP_DAC_OVERRIDE capability set.

Since the upstream golang is also broken (see [1]), let's skip this test
for Go 1.20 and 1.20.1 (and hope it'll be fixed in Go 1.20.2 as per [2].

[1] https://go.dev/issue/58552
[2] https://go-review.googlesource.com/c/go/+/469956

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
@kolyshkin
Copy link
Contributor

OK, what I guess we'll do is

kolyshkin added a commit to kolyshkin/runc that referenced this issue Feb 28, 2023
This is a test case for issue reported as opencontainers#3715. In short, even if a
(non-root) user that the container is run as does not have execute
permission bit set for the executable, it should still work in case runc
has the CAP_DAC_OVERRIDE capability set.

Since the upstream golang is also broken (see [1]), let's skip this test
for Go 1.20 and 1.20.1 (and hope it'll be fixed in Go 1.20.2 as per [2].

[1] https://go.dev/issue/58552
[2] https://go-review.googlesource.com/c/go/+/469956

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
gopherbot pushed a commit to golang/go that referenced this issue Feb 28, 2023
…E on Linux

CL 416115 added using faccessat2(2) from syscall.Faccessat on Linux
(which is the only true way to implement AT_EACCESS flag handing),
if available. If not available, it uses some heuristics to mimic the
kernel behavior, mostly taken from glibc (see CL 126415).

Next, CL 414824 added using the above call (via unix.Eaccess) to
exec.LookPath in order to check if the binary can really be executed.

As a result, in a very specific scenario, described below,
syscall.Faccessat (and thus exec.LookPath) mistakenly tells that the
binary can not be executed, while in reality it can be. This makes
this bug a regression in Go 1.20.

This scenario involves all these conditions:
 - no faccessat2 support available (i.e. either Linux kernel < 5.8,
   or a seccomp set up to disable faccessat2);
 - the current user is not root (i.e. geteuid() != 0);
 - CAP_DAC_OVERRIDE capability is set for the current process;
 - the file to be executed does not have executable permission
   bit set for either the current EUID or EGID;
 - the file to be executed have at least one executable bit set.

Unfortunately, this set of conditions was observed in the wild -- a
container run as a non-root user with the binary file owned by root with
executable permission set for a user only [1]. Essentially it means it
is not as rare as it may seem.

Now, CAP_DAC_OVERRIDE essentially makes the kernel bypass most of the
checks, so execve(2) and friends work the same was as for root user,
i.e. if at least one executable bit it set, the permission to execute
is granted (see generic_permission() function in the Linux kernel).

Modify the code to check for CAP_DAC_OVERRIDE and mimic the kernel
behavior for permission checks.

[1] opencontainers/runc#3715

For #58552.
Fixes #58624.

Change-Id: I82a7e757ab3fd3d0193690a65c3b48fee46ff067
Reviewed-on: https://go-review.googlesource.com/c/go/+/468735
Reviewed-by: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
(cherry picked from commit 031401a)
Reviewed-on: https://go-review.googlesource.com/c/go/+/469956
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
romaindoumenc pushed a commit to TroutSoftware/go that referenced this issue Mar 3, 2023
…E on Linux

CL 416115 added using faccessat2(2) from syscall.Faccessat on Linux
(which is the only true way to implement AT_EACCESS flag handing),
if available. If not available, it uses some heuristics to mimic the
kernel behavior, mostly taken from glibc (see CL 126415).

Next, CL 414824 added using the above call (via unix.Eaccess) to
exec.LookPath in order to check if the binary can really be executed.

As a result, in a very specific scenario, described below,
syscall.Faccessat (and thus exec.LookPath) mistakenly tells that the
binary can not be executed, while in reality it can be. This makes
this bug a regression in Go 1.20.

This scenario involves all these conditions:
 - no faccessat2 support available (i.e. either Linux kernel < 5.8,
   or a seccomp set up to disable faccessat2);
 - the current user is not root (i.e. geteuid() != 0);
 - CAP_DAC_OVERRIDE capability is set for the current process;
 - the file to be executed does not have executable permission
   bit set for either the current EUID or EGID;
 - the file to be executed have at least one executable bit set.

Unfortunately, this set of conditions was observed in the wild -- a
container run as a non-root user with the binary file owned by root with
executable permission set for a user only [1]. Essentially it means it
is not as rare as it may seem.

Now, CAP_DAC_OVERRIDE essentially makes the kernel bypass most of the
checks, so execve(2) and friends work the same was as for root user,
i.e. if at least one executable bit it set, the permission to execute
is granted (see generic_permission() function in the Linux kernel).

Modify the code to check for CAP_DAC_OVERRIDE and mimic the kernel
behavior for permission checks.

[1] opencontainers/runc#3715

For golang#58552.
Fixes golang#58624.

Change-Id: I82a7e757ab3fd3d0193690a65c3b48fee46ff067
Reviewed-on: https://go-review.googlesource.com/c/go/+/468735
Reviewed-by: Damien Neil <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
(cherry picked from commit 031401a)
Reviewed-on: https://go-review.googlesource.com/c/go/+/469956
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
Run-TryBot: Than McIntosh <thanm@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
kolyshkin added a commit to kolyshkin/runc that referenced this issue Mar 9, 2023
Since commit 957d97b was made to fix issue [7],
a few things happened:

- a similar functionality appeared in go 1.20 [1], so the issue
  mentioned in the comment (being removed) is no longer true;
- a bug in runc was found [2], which also affects go [3];
- the bug was fixed in go 1.21 [4] and 1.20.2 [5];
- a similar fix was made to x/sys/unix.Faccessat [6].

The essense of [2] is, even if a (non-root) user that the container is
run as does not have execute permission bit set for the executable, it
should still work in case runc has the CAP_DAC_OVERRIDE capability set.

To fix this [2] without reintroducing the older bug [7]:
- drop own Eaccess implementation;
- use the one from x/sys/unix for Go 1.19 (depends on [6]);
- do not use anything when Go 1.20+ is used.

NOTE it is virtually impossible to fix the bug [2] when Go 1.20 or Go
1.20.1 is used because of [3].

A test case is added by a separate commit.

Fixes: opencontainers#3715.

[1] https://go-review.googlesource.com/c/go/+/414824
[2] opencontainers#3715
[3] https://go.dev/issue/58552
[4] https://go-review.googlesource.com/c/go/+/468735
[5] https://go-review.googlesource.com/c/go/+/469956
[6] https://go-review.googlesource.com/c/sys/+/468877
[7] opencontainers#3520

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
kolyshkin added a commit to kolyshkin/runc that referenced this issue Mar 9, 2023
This is a test case for issue reported as opencontainers#3715. In short, even if a
(non-root) user that the container is run as does not have execute
permission bit set for the executable, it should still work in case runc
has the CAP_DAC_OVERRIDE capability set.

Since the upstream golang is also broken (see [1]), let's skip this test
for Go 1.20 and 1.20.1 (as it's fixed in Go 1.20.2 as per [2]).

[1] https://go.dev/issue/58552
[2] https://go-review.googlesource.com/c/go/+/469956

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
kolyshkin added a commit to kolyshkin/runc that referenced this issue Mar 9, 2023
Since commit 957d97b was made to fix issue [7],
a few things happened:

- a similar functionality appeared in go 1.20 [1], so the issue
  mentioned in the comment (being removed) is no longer true;
- a bug in runc was found [2], which also affects go [3];
- the bug was fixed in go 1.21 [4] and 1.20.2 [5];
- a similar fix was made to x/sys/unix.Faccessat [6].

The essense of [2] is, even if a (non-root) user that the container is
run as does not have execute permission bit set for the executable, it
should still work in case runc has the CAP_DAC_OVERRIDE capability set.

To fix this [2] without reintroducing the older bug [7]:
- drop own Eaccess implementation;
- use the one from x/sys/unix for Go 1.19 (depends on [6]);
- do not use anything when Go 1.20+ is used.

NOTE it is virtually impossible to fix the bug [2] when Go 1.20 or Go
1.20.1 is used because of [3].

A test case is added by a separate commit.

Fixes: opencontainers#3715.

[1] https://go-review.googlesource.com/c/go/+/414824
[2] opencontainers#3715
[3] https://go.dev/issue/58552
[4] https://go-review.googlesource.com/c/go/+/468735
[5] https://go-review.googlesource.com/c/go/+/469956
[6] https://go-review.googlesource.com/c/sys/+/468877
[7] opencontainers#3520

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
kolyshkin added a commit to kolyshkin/runc that referenced this issue Mar 9, 2023
This is a test case for issue reported as opencontainers#3715. In short, even if a
(non-root) user that the container is run as does not have execute
permission bit set for the executable, it should still work in case runc
has the CAP_DAC_OVERRIDE capability set.

Since the upstream golang is also broken (see [1]), let's skip this test
for Go 1.20 and 1.20.1 (as it's fixed in Go 1.20.2 as per [2]).

[1] https://go.dev/issue/58552
[2] https://go-review.googlesource.com/c/go/+/469956

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
kolyshkin added a commit to kolyshkin/runc that referenced this issue Mar 16, 2023
Since commit 957d97b was made to fix issue [7],
a few things happened:

- a similar functionality appeared in go 1.20 [1], so the issue
  mentioned in the comment (being removed) is no longer true;
- a bug in runc was found [2], which also affects go [3];
- the bug was fixed in go 1.21 [4] and 1.20.2 [5];
- a similar fix was made to x/sys/unix.Faccessat [6].

The essense of [2] is, even if a (non-root) user that the container is
run as does not have execute permission bit set for the executable, it
should still work in case runc has the CAP_DAC_OVERRIDE capability set.

To fix this [2] without reintroducing the older bug [7]:
- drop own Eaccess implementation;
- use the one from x/sys/unix for Go 1.19 (depends on [6]);
- do not use anything when Go 1.20+ is used.

NOTE it is virtually impossible to fix the bug [2] when Go 1.20 or Go
1.20.1 is used because of [3].

A test case is added by a separate commit.

Fixes: opencontainers#3715.

[1] https://go-review.googlesource.com/c/go/+/414824
[2] opencontainers#3715
[3] https://go.dev/issue/58552
[4] https://go-review.googlesource.com/c/go/+/468735
[5] https://go-review.googlesource.com/c/go/+/469956
[6] https://go-review.googlesource.com/c/sys/+/468877
[7] opencontainers#3520

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
kolyshkin added a commit to kolyshkin/runc that referenced this issue Mar 16, 2023
This is a test case for issue reported as opencontainers#3715. In short, even if a
(non-root) user that the container is run as does not have execute
permission bit set for the executable, it should still work in case runc
has the CAP_DAC_OVERRIDE capability set.

Since the upstream golang is also broken (see [1]), let's skip this test
for Go 1.20 and 1.20.1 (as it's fixed in Go 1.20.2 as per [2]).

[1] https://go.dev/issue/58552
[2] https://go-review.googlesource.com/c/go/+/469956

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
@kolyshkin
Copy link
Contributor

Fixed by #3753 (which needs to be merged and backported to 1.1)

kolyshkin added a commit to kolyshkin/runc that referenced this issue Mar 27, 2023
Since commit 957d97b was made to fix issue [7],
a few things happened:

- a similar functionality appeared in go 1.20 [1], so the issue
  mentioned in the comment (being removed) is no longer true;
- a bug in runc was found [2], which also affects go [3];
- the bug was fixed in go 1.21 [4] and 1.20.2 [5];
- a similar fix was made to x/sys/unix.Faccessat [6].

The essense of [2] is, even if a (non-root) user that the container is
run as does not have execute permission bit set for the executable, it
should still work in case runc has the CAP_DAC_OVERRIDE capability set.

To fix this [2] without reintroducing the older bug [7]:
- drop own Eaccess implementation;
- use the one from x/sys/unix for Go 1.19 (depends on [6]);
- do not use anything when Go 1.20+ is used.

NOTE it is virtually impossible to fix the bug [2] when Go 1.20 or Go
1.20.1 is used because of [3].

A test case is added by a separate commit.

Fixes: opencontainers#3715.

[1] https://go-review.googlesource.com/c/go/+/414824
[2] opencontainers#3715
[3] https://go.dev/issue/58552
[4] https://go-review.googlesource.com/c/go/+/468735
[5] https://go-review.googlesource.com/c/go/+/469956
[6] https://go-review.googlesource.com/c/sys/+/468877
[7] opencontainers#3520

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
kolyshkin added a commit to kolyshkin/runc that referenced this issue Mar 27, 2023
This is a test case for issue reported as opencontainers#3715. In short, even if a
(non-root) user that the container is run as does not have execute
permission bit set for the executable, it should still work in case runc
has the CAP_DAC_OVERRIDE capability set.

Note that since the upstream golang is also broken (see [1]), this test
will fail for Go 1.20 and 1.20.1 (fix is in Go 1.20.2 as per [2]).

[1] https://go.dev/issue/58552
[2] https://go-review.googlesource.com/c/go/+/469956

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
@kolyshkin
Copy link
Contributor

Not fixed until we do a backport to release-1.1 (in progress).

kolyshkin added a commit to kolyshkin/runc that referenced this issue Mar 28, 2023
1. bump golang.org/x/sys to v0.6.0;
2. bump golang.org/x/net to v0.8.0
3. require Go 1.17.

Newer x/sys is needed to fix [1].

Go 1.17 is needed because x/sys/unix is using unsafe.Slice which
requires go1.17 or later.

[1] opencontainers#3715

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
@kolyshkin kolyshkin modified the milestones: 1.1.5, 1.1.6 Mar 28, 2023
@kolyshkin
Copy link
Contributor

As this is a rather rare bug, and we want to release 1.1.5 ASAP due to a couple of CVEs (see #3789), let's fix this issue after 1.1.5 is out.

kolyshkin added a commit to kolyshkin/runc that referenced this issue Mar 31, 2023
1. bump golang.org/x/sys to v0.6.0;
2. bump golang.org/x/net to v0.8.0
3. require Go 1.17.

Newer x/sys is needed to fix [1].

Go 1.17 is needed because x/sys/unix is using unsafe.Slice which
requires go1.17 or later.

[1] opencontainers#3715

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
kolyshkin added a commit to kolyshkin/runc that referenced this issue Apr 3, 2023
1. bump golang.org/x/sys to v0.6.0;
2. bump golang.org/x/net to v0.8.0
3. require Go 1.17.

Newer x/sys is needed to fix [1].

Go 1.17 is needed because x/sys/unix is using unsafe.Slice which
requires go1.17 or later.

This reuses parts of main commit a0f8847.

[1] opencontainers#3715

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
kolyshkin added a commit to kolyshkin/runc that referenced this issue Apr 4, 2023
1. bump golang.org/x/sys to v0.6.0;
2. bump golang.org/x/net to v0.8.0
3. require Go 1.17.

Newer x/sys is needed to fix [1].

Go 1.17 is needed because x/sys/unix is using unsafe.Slice which
requires go1.17 or later.

This reuses parts of main commit a0f8847.

[1] opencontainers#3715

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
kolyshkin added a commit to kolyshkin/runc that referenced this issue Apr 5, 2023
1. bump golang.org/x/sys to v0.6.0;
2. bump golang.org/x/net to v0.8.0
3. require Go 1.17.

Newer x/sys is needed to fix [1].

Go 1.17 is needed because x/sys/unix is using unsafe.Slice which
requires go1.17 or later.

This reuses parts of main commit a0f8847.

[1] opencontainers#3715

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
kolyshkin added a commit to kolyshkin/runc that referenced this issue Apr 5, 2023
1. bump golang.org/x/sys to v0.6.0;
2. bump golang.org/x/net to v0.8.0
3. require Go 1.17.

Newer x/sys is needed to fix [1].

Go 1.17 is needed because x/sys/unix is using unsafe.Slice which
requires go1.17 or later.

This reuses parts of main commit a0f8847.

[1] opencontainers#3715

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
kolyshkin added a commit to kolyshkin/runc that referenced this issue Apr 5, 2023
1. bump golang.org/x/sys to v0.6.0;
2. bump golang.org/x/net to v0.8.0
3. require Go 1.17.

Newer x/sys is needed to fix [1].

Go 1.17 is needed because x/sys/unix is using unsafe.Slice which
requires go1.17 or later.

This reuses parts of main commit a0f8847.

[1] opencontainers#3715

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
kolyshkin added a commit to kolyshkin/runc that referenced this issue Apr 6, 2023
Since commit 957d97b was made to fix issue [7],
a few things happened:

- a similar functionality appeared in go 1.20 [1], so the issue
  mentioned in the comment (being removed) is no longer true;
- a bug in runc was found [2], which also affects go [3];
- the bug was fixed in go 1.21 [4] and 1.20.2 [5];
- a similar fix was made to x/sys/unix.Faccessat [6].

The essense of [2] is, even if a (non-root) user that the container is
run as does not have execute permission bit set for the executable, it
should still work in case runc has the CAP_DAC_OVERRIDE capability set.

To fix this [2] without reintroducing the older bug [7]:
- drop own Eaccess implementation;
- use the one from x/sys/unix for Go 1.19 (depends on [6]);
- do not use anything when Go 1.20+ is used.

NOTE it is virtually impossible to fix the bug [2] when Go 1.20 or Go
1.20.1 is used because of [3].

A test case is added by a separate commit.

Fixes: opencontainers#3715.

[1] https://go-review.googlesource.com/c/go/+/414824
[2] opencontainers#3715
[3] https://go.dev/issue/58552
[4] https://go-review.googlesource.com/c/go/+/468735
[5] https://go-review.googlesource.com/c/go/+/469956
[6] https://go-review.googlesource.com/c/sys/+/468877
[7] opencontainers#3520

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 8491d33)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
kolyshkin added a commit to kolyshkin/runc that referenced this issue Apr 6, 2023
This is a test case for issue reported as opencontainers#3715. In short, even if a
(non-root) user that the container is run as does not have execute
permission bit set for the executable, it should still work in case runc
has the CAP_DAC_OVERRIDE capability set.

Note that since the upstream golang is also broken (see [1]), this test
will fail for Go 1.20 and 1.20.1 (fix is in Go 1.20.2 as per [2]).

[1] https://go.dev/issue/58552
[2] https://go-review.googlesource.com/c/go/+/469956

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 8293ef2)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
kolyshkin added a commit to kolyshkin/runc that referenced this issue Apr 7, 2023
Since commit 957d97b was made to fix issue [7],
a few things happened:

- a similar functionality appeared in go 1.20 [1], so the issue
  mentioned in the comment (being removed) is no longer true;
- a bug in runc was found [2], which also affects go [3];
- the bug was fixed in go 1.21 [4] and 1.20.2 [5];
- a similar fix was made to x/sys/unix.Faccessat [6].

The essense of [2] is, even if a (non-root) user that the container is
run as does not have execute permission bit set for the executable, it
should still work in case runc has the CAP_DAC_OVERRIDE capability set.

To fix this [2] without reintroducing the older bug [7]:
- drop own Eaccess implementation;
- use the one from x/sys/unix for Go 1.19 (depends on [6]);
- do not use anything when Go 1.20+ is used.

NOTE it is virtually impossible to fix the bug [2] when Go 1.20 or Go
1.20.1 is used because of [3].

A test case is added by a separate commit.

Fixes: opencontainers#3715.

[1] https://go-review.googlesource.com/c/go/+/414824
[2] opencontainers#3715
[3] https://go.dev/issue/58552
[4] https://go-review.googlesource.com/c/go/+/468735
[5] https://go-review.googlesource.com/c/go/+/469956
[6] https://go-review.googlesource.com/c/sys/+/468877
[7] opencontainers#3520

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 8491d33)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
kolyshkin added a commit to kolyshkin/runc that referenced this issue Apr 7, 2023
This is a test case for issue reported as opencontainers#3715. In short, even if a
(non-root) user that the container is run as does not have execute
permission bit set for the executable, it should still work in case runc
has the CAP_DAC_OVERRIDE capability set.

Note that since the upstream golang is also broken (see [1]), this test
will fail for Go 1.20 and 1.20.1 (fix is in Go 1.20.2 as per [2]).

[1] https://go.dev/issue/58552
[2] https://go-review.googlesource.com/c/go/+/469956

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
(cherry picked from commit 8293ef2)
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
@kolyshkin
Copy link
Contributor

Fixed in 1.1 by #3817

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants