Skip to content

Runsc exec wipes capabilities if they are provided #11108

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

Closed
pawalt opened this issue Nov 2, 2024 · 3 comments · Fixed by #11137
Closed

Runsc exec wipes capabilities if they are provided #11108

pawalt opened this issue Nov 2, 2024 · 3 comments · Fixed by #11137
Labels
good first issue Good for newcomers type: bug Something isn't working

Comments

@pawalt
Copy link
Contributor

pawalt commented Nov 2, 2024

Description

In runc, the -cap flag always appends to the set of capabilities. In GVisor, the root process's capabilities are used by default, but if -cap is provided, they are reset to only the provided capabilities.

I'm wondering if this a bug or intended, and if it's intended, how can I emulate the runc capability appending behavior.

I think this functionality is here:

if e.Capabilities == nil {

Steps to reproduce

I've got docker installed with the runsc runtime. First I'll run a container with runc and see the caps:

# in one terminal
host $ docker run -it --name exectest --rm ubuntu bash -c "apt update && apt install libcap2-bin -y && bash"

# in another terminal
host $ sudo runc --root /run/docker/runtime-runc/moby/ exec $(docker inspect -f '{{.Id}}' exectest)  /bin/bash
container $ getpcaps $$
250: cap_chown,cap_dac_override,cap_fowner,cap_fsetid,cap_kill,cap_setgid,cap_setuid,cap_setpcap,cap_net_bind_service,cap_net_raw,cap_sys_chroot,cap_mknod,cap_audit_write,cap_setfcap=ep

host $ sudo runc --root /run/docker/runtime-runc/moby/ exec -cap CAP_SYS_PTRACE $(docker inspect -f '{{.Id}}' exectest)  /bin/bash
container $ getpcaps $$
257: cap_chown,cap_dac_override,cap_fowner,cap_fsetid,cap_kill,cap_setgid,cap_setuid,cap_setpcap,cap_net_bind_service,cap_net_raw,cap_sys_chroot,cap_sys_ptrace,cap_mknod,cap_audit_write,cap_setfcap=ep

Now with GVisor:

# in one terminal
host $ docker run -it --name exectest --runtime runsc --rm ubuntu bash -c "apt update && apt install libcap2-bin -y && bash"

# in another terminal
host $ sudo runsc --root /run/docker/runtime-runc/moby/ exec $(docker inspect -f '{{.Id}}' exectest)  /bin/bash 
container $ getpcaps $$
180: cap_chown,cap_dac_override,cap_fowner,cap_fsetid,cap_kill,cap_setgid,cap_setuid,cap_setpcap,cap_net_bind_service,cap_sys_chroot,cap_mknod,cap_audit_write,cap_setfcap=ep

host $ sudo runsc --root /run/docker/runtime-runc/moby/ exec -cap CAP_SYS_PTRACE $(docker inspect -f '{{.Id}}' exectest)  /bin/bash
container $ getpcaps $$
184: cap_sys_ptrace=eip

runsc version

$ runsc --version
runsc version 35d3c6e
spec: 1.1.0-rc.1

docker version (if using docker)

$ docker version
Client: Docker Engine - Community
Version: 26.1.4
API version: 1.45
Go version: go1.21.11
Git commit: 5650f9b
Built: Wed Jun 5 11:30:53 2024
OS/Arch: linux/amd64
Context: default

Server: Docker Engine - Community
Engine:
Version: 26.1.4
API version: 1.45 (minimum version 1.24)
Go version: go1.21.11
Git commit: de5c9cf
Built: Wed Jun 5 11:29:11 2024
OS/Arch: linux/amd64
Experimental: false
containerd:
Version: 1.6.33
GitCommit: d2d58213f83a351ca8f528a95fbd145f5654e957
runc:
Version: 1.1.12
GitCommit: v1.1.12-0-g51d5e94
docker-init:
Version: 0.19.0
GitCommit: de40ad0

uname

$ uname -a Linux ip-10-1-4-158.ec2.internal 5.15.0-207.156.6.el9uek.x86_64 #2 SMP Thu Jun 6 02:32:40 PDT 2024 x86_64 x86_64 x86_64 GNU/Linux

kubectl (if using Kubernetes)

N/A

repo state (if built from source)

No response

runsc debug logs (if available)

No response

@pawalt pawalt added the type: bug Something isn't working label Nov 2, 2024
@EtiennePerot EtiennePerot added the good first issue Good for newcomers label Nov 4, 2024
@ayushr2
Copy link
Collaborator

ayushr2 commented Nov 4, 2024

Thanks for the report. I think this bug extends beyond just "wiping capabilities".

When --process flag is not specified (see runsc/cmd/exec.go:argsFromCLI()), gVisor should initialize control.ExecArgs{} using the container's OCI spec and override certain fields based on the flags passed, rather initialize all fields based on the flags passed (if not passed, then the default flag value is being used).

copybara-service bot pushed a commit that referenced this issue Nov 7, 2024
…ided.

This is consistent with runc. This fixes several bugs with runsc exec:
- When --process flag is specified, the process spec should be validated. The
  process spec should not inherit values from the OCI spec. Earlier, we were
  setting WorkingDirectory, Envv and Capabilities from the spec if these were
  not set.
- When --process flag is not specified, we should use the Process defined in
  the container spec as the base and append the following flags onto that
  process spec. Earlier if these flags were specified, we were not using the
  container spec values and just setting to these passed flags, hence making it
  look like runsc is "clearing" these fields when their flags are passed.
    - additional-gids
    - cap
    - env
- When --process flag is not specified, we should use the following values
  defined in the container spec's Process. Those values should be selectively
  overridden when the corresponding flag is set. Earlier, we were always using
  the flag values, even when the flag was not set. One implication was that we
  were always running with UID=GID=0 when --process and --user are not set.
    - user
    - cwd
- When --cap is set, it should not append to the Inheritable capabilities
  defined in the spec. And it should only be appended to Ambient if Inheritable
  in the original spec is non-empty.

Fixes #11108

PiperOrigin-RevId: 694260484
@ayushr2
Copy link
Collaborator

ayushr2 commented Nov 8, 2024

@pawalt I think #11137 should fix this issue.

copybara-service bot pushed a commit that referenced this issue Nov 8, 2024
…ided.

This is consistent with runc. This fixes several bugs with runsc exec:
- When --process flag is specified, the process spec should be validated. The
  process spec should not inherit values from the OCI spec. Earlier, we were
  setting WorkingDirectory, Envv and Capabilities from the spec if these were
  not set.
- When --process flag is not specified, we should use the Process defined in
  the container spec as the base and append the following flags onto that
  process spec. Earlier if these flags were specified, we were not using the
  container spec values and just setting to these passed flags, hence making it
  look like runsc is "clearing" these fields when their flags are passed.
    - additional-gids
    - cap
    - env
- When --process flag is not specified, we should use the following values
  defined in the container spec's Process. Those values should be selectively
  overridden when the corresponding flag is set. Earlier, we were always using
  the flag values, even when the flag was not set. One implication was that we
  were always running with UID=GID=0 when --process and --user are not set.
    - user
    - cwd
- When --cap is set, it should not append to the Inheritable capabilities
  defined in the spec. And it should only be appended to Ambient if Inheritable
  in the original spec is non-empty.

Fixes #11108

PiperOrigin-RevId: 694260484
copybara-service bot pushed a commit that referenced this issue Nov 8, 2024
…ided.

This is consistent with runc. This fixes several bugs with runsc exec:
- When --process flag is specified, the process spec should be validated. The
  process spec should not inherit values from the OCI spec except capabilities.
  Earlier, we were setting WorkingDirectory and Envv from the spec if these
  were not set in the process file.
- When --process flag is not specified, we should use the Process defined in
  the container spec as the base and append the following flags onto that
  process spec. Earlier if these flags were specified, we were not using the
  container spec values and just setting to these passed flags, hence making it
  look like runsc is "clearing" these fields when their flags are passed.
    - additional-gids
    - cap
    - env
- When --process flag is not specified, we should use the following values
  defined in the container spec's Process. Those values should be selectively
  overridden when the corresponding flag is set. Earlier, we were always using
  the flag values, even when the flag was not set. One implication was that we
  were always running with UID=GID=0 when --process and --user are not set.
    - user
    - cwd
- When --cap is set, it should not append to the Inheritable capabilities
  defined in the spec. And it should only be appended to Ambient if Inheritable
  in the original spec is non-empty.

Fixes #11108

PiperOrigin-RevId: 694260484
@pawalt
Copy link
Contributor Author

pawalt commented Nov 8, 2024

@ayushr2 thanks!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers type: bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants