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

access() reports file on bind mount as executable when it is not #6367

Closed
Sjord opened this issue Jun 17, 2022 · 6 comments
Closed

access() reports file on bind mount as executable when it is not #6367

Sjord opened this issue Jun 17, 2022 · 6 comments

Comments

@Sjord
Copy link

Sjord commented Jun 17, 2022

The access() syscall reports files on bind mounts as executable, when they are not:

$ touch hello.txt
$ docker run --rm -v "$PWD:/foo" -w "/foo"  python python3 -c 'import os; print(os.access("hello.txt", os.X_OK))'
True

The file hello.txt has default 644 permissions, so no executable bit, so I would expect access() to return false.

This is not specific to Python, I also tried it with a C program. It seems to be specific to MacOS:

I could reproduce this on:

  • MacOS Docker server 20.10.7
  • MacOS Docker server 20.10.16

But not on:

  • Linux Docker server 20.10.12
  • Linux Docker server 18.09.1

Output of docker version:

Client:
 Cloud integration: v1.0.25
 Version:           20.10.16
 API version:       1.41
 Go version:        go1.17.10
 Git commit:        aa7e414
 Built:             Thu May 12 09:20:34 2022
 OS/Arch:           darwin/amd64
 Context:           default
 Experimental:      true

Server: Docker Desktop 4.9.1 (81317)
 Engine:
  Version:          20.10.16
  API version:      1.41 (minimum version 1.12)
  Go version:       go1.17.10
  Git commit:       f756502
  Built:            Thu May 12 09:15:42 2022
  OS/Arch:          linux/amd64
  Experimental:     false
 containerd:
  Version:          1.6.4
  GitCommit:        212e8b6fa2f44b9c21b2798135fc6fb7c53efc16
 runc:
  Version:          1.1.1
  GitCommit:        v1.1.1-0-g52de29d
 docker-init:
  Version:          0.19.0
  GitCommit:        de40ad0

Output of docker info:

Client:
 Context:    default
 Debug Mode: false
 Plugins:
  buildx: Docker Buildx (Docker Inc., v0.8.2)
  compose: Docker Compose (Docker Inc., v2.6.0)
  sbom: View the packaged-based Software Bill Of Materials (SBOM) for an image (Anchore Inc., 0.6.0)
  scan: Docker Scan (Docker Inc., v0.17.0)

Server:
 Containers: 23
  Running: 0
  Paused: 0
  Stopped: 23
 Images: 10
 Server Version: 20.10.16
 Storage Driver: overlay2
  Backing Filesystem: extfs
  Supports d_type: true
  Native Overlay Diff: true
  userxattr: false
 Logging Driver: json-file
 Cgroup Driver: cgroupfs
 Cgroup Version: 2
 Plugins:
  Volume: local
  Network: bridge host ipvlan macvlan null overlay
  Log: awslogs fluentd gcplogs gelf journald json-file local logentries splunk syslog
 Swarm: inactive
 Runtimes: io.containerd.runtime.v1.linux runc io.containerd.runc.v2
 Default Runtime: runc
 Init Binary: docker-init
 containerd version: 212e8b6fa2f44b9c21b2798135fc6fb7c53efc16
 runc version: v1.1.1-0-g52de29d
 init version: de40ad0
 Security Options:
  seccomp
   Profile: default
  cgroupns
 Kernel Version: 5.10.104-linuxkit
 Operating System: Docker Desktop
 OSType: linux
 Architecture: x86_64
 CPUs: 4
 Total Memory: 7.774GiB
 Name: docker-desktop
 ID: JOCP:IUGQ:4KYM:ORYA:E44M:CGVK:CT34:ZXI6:XASD:CB3D:WMSZ:HNKV
 Docker Root Dir: /var/lib/docker
 Debug Mode: false
 HTTP Proxy: http.docker.internal:3128
 HTTPS Proxy: http.docker.internal:3128
 No Proxy: hubproxy.docker.internal
 Registry: https://index.docker.io/v1/
 Labels:
 Experimental: false
 Insecure Registries:
  hubproxy.docker.internal:5000
  127.0.0.0/8
 Live Restore Enabled: false

Diagnostics ID: E1B25F71-3310-4701-8061-DD025CDCAE2E/20220617111756

@thaJeztah
Copy link
Member

/cc @fredericdalleau

@fredericdalleau
Copy link

fredericdalleau commented Jun 22, 2022

Interesting finding, it's confirmed by strace.

faccessat(AT_FDCWD, "hello.txt", X_OK)  = 0

I think it's because Docker Desktop uses specific mount options for bind mounts.

Now, from the man page : https://man7.org/linux/man-pages/man2/access.2.html :

If the calling process has appropriate privileges (i.e., is superuser), POSIX.1-2001 permits an implementation to indicate success for an X_OK check even if none of the execute file permission bits are set. Linux does not do this.

Hard to say who is right or wrong in this case.
I also note that ls -l /foo/hello.txt displays the right attributes. I think it's because it uses getattr rather than access. Maybe your implementation could use that.

@Sjord
Copy link
Author

Sjord commented Jun 23, 2022

Thanks, @fredericdalleau. I agree that access() is not the correct syscall to check executable bits. But it also seems the behavior of Docker is undesirable.

My manual page and POSIX.1-2001 says:

If the process has appropriate privileges, an implementation may indicate success for X_OK even if none of the execute file permission bits are set.

This means all bets are off for X_OK. However, another sentence specifies that this only applies to superusers:

The sentence concerning appropriate privileges and execute permission bits reflects the two possibilities implemented by historical implementations when checking superuser access for X_OK.

The "two possibilities" are described elsewhere as cases where either the effective ID or the real ID is the superuser.

It suggests that this is undesired behaviour:

New implementations are discouraged from returning X_OK unless at least one execution permission bit is set.

The behavior of access() in Docker for mac stays the same when running as a normal user in the docker. I.e. with a Dockerfile like this:

FROM python:3
RUN adduser test
USER test
$ docker build -t test-user-python .
$ docker run --rm -v "$PWD:/foo" -w "/foo" test-user-python python3 -c 'import os; print(os.access("hello.txt", os.X_OK))'
True

This does "indicate success for X_OK even if none of the execute file permission bits are set", as the manual says, so this is not strictly wrong. But that sentence is there to support historical implementations that didn't handle superuser situations correctly, and Docker continous this bug, even outside of superuser situations.

So Docker's behavior is "discouraged", if not strictly incorrect.

@docker-robott
Copy link
Collaborator

Issues go stale after 90 days of inactivity.
Mark the issue as fresh with /remove-lifecycle stale comment.
Stale issues will be closed after an additional 30 days of inactivity.

Prevent issues from auto-closing with an /lifecycle frozen comment.

If this issue is safe to close now please do so.

Send feedback to Docker Community Slack channels #docker-for-mac or #docker-for-windows.
/lifecycle stale

@nicks
Copy link

nicks commented Oct 11, 2022

I'm pretty sure this is the same issue as #5509, so I'm going to mark as a duplicate to centralize communication.

@nicks nicks closed this as completed Oct 11, 2022
@docker-robott
Copy link
Collaborator

Closed issues are locked after 30 days of inactivity.
This helps our team focus on active issues.

If you have found a problem that seems similar to this, please open a new issue.

/lifecycle locked

@docker docker locked and limited conversation to collaborators Nov 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants