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

chmod 1777 has surprising behaviour #1556

Open
rlpowell opened this issue Dec 14, 2024 · 11 comments
Open

chmod 1777 has surprising behaviour #1556

rlpowell opened this issue Dec 14, 2024 · 11 comments

Comments

@rlpowell
Copy link

The following cost me hours of debugging. It's a side effect of #1383 plus https://sysctl-explorer.net/fs/protected_symlinks/ .

Dockerfile:

FROM php:8.1-apache

RUN mkdir /tmp/files
RUN ln -s /tmp/files myfiles

Commands:

$ podman build -t symtest -f Dockerfile .
$ podman run --userns=keep-id --rm --name symtest-container -it symtest bash

podman is basically the same as docker; --userns=keep-id just makes it so everything runs as the user running the command in the container, so in this case I was running with UID 1086.

In the container:

rlpowell@85b62d863dde:~$ ls -l myfiles
lrwxrwxrwx. 1 root root 10 Dec 13 20:16 myfiles -> /tmp/files
rlpowell@85b62d863dde:~$ ls -l /tmp/files/
total 0
rlpowell@85b62d863dde:~$ ls -l myfiles/
ls: cannot access 'myfiles/': Permission denied

I discovered this because https://github.com/wikimedia/mediawiki-docker has symlinks from /var/log/apache2/error.log to /dev/stderr (and a few other similar ones) that with this php docker image change, cause apache to fail to run if the running UID is not the same as whatever the default for www-data is (33 I think?) because /var/log/apache2 is 1777 and own by that ID.

I don't have any particular suggestions for how to solve this because I'm not sure what problem you were trying to solve by making this change in the first place.

@LaurentGoderre
Copy link
Member

In your Dockerfile, you don't explicitly set a user so the RUN mkdir /tmp/files runs as root. SInce in hyour use case you don't necessarily know the id or group the container is going to run as, setting your /tmp/files is required like in the image:

https://github.com/docker-library/php/blob/master/8.2/bookworm/fpm/Dockerfile#L49

@rlpowell
Copy link
Author

No, this has nothing to do with /tmp/files at all. You can get the same behaviour pointing the symlink at / or /etc/os-release or whatever you want. It's the symlink that can't be read. The symlink named myfiles. Which is created in the default current dir, which is /var/www/html, which is 1777, which is totally unexpected and leads to super weird behaviour.

Yes, creating the symlink with the same UID as the final user would fix this, but that's completely new and surprising and abnormal for these directories. You only need to care about the UID of symlinks in sticky-bit directories, and on a normal system that's only /tmp and /var/tmp

Again, I'd be happy to suggest a fix if I understood why the sticky bit was added in the first place, but I don't; the PR just said "security", but I have no idea what the threat model is for directory security inside a container.

@LaurentGoderre
Copy link
Member

Yeah, the symlink also needs its permission expanded. There seems to be something weird about the ls commandf though.

The following Dockerfile yields this result

FROM php:8.1-apache

RUN mkdir /tmp/files/ \
    && ln -s /tmp/files myfiles \
    && chmod +x /tmp/files/ \
    && chmod 1777 myfiles'
docker run --rm -it -u 1000:1000 test bash
I have no name!@b88631583fe8:/var/www/html$ echo "blah" > myfiles/test
I have no name!@b88631583fe8:/var/www/html$ ls -l myfiles/
ls: cannot access 'myfiles/': Permission denied
I have no name!@b88631583fe8:/var/www/html$ cat myfiles/test
blah

@rlpowell
Copy link
Author

You really need to go read https://sysctl-explorer.net/fs/protected_symlinks/ please; the ls behaviour you're seeing is exactly the problem I'm talking about, and exactly what's expected when you throw 1777 on random directories. Changing a symlink to 1777 is a bad idea and isn't relevant; it's about alignment of ownership.

Seriously, y'all need to take 1777 off these directories, and again if you told me what you were trying to accomplish I'd have suggestions for alternate solutions.

@LaurentGoderre
Copy link
Member

The modern function of the sticky bit refers to directories, and protects directories and their content from being hijacked by non-owners; this is found in most modern Unix-like systems. Files in a shared directory such as /tmp belong to individual owners, and non-owners may not delete, overwrite or rename them.

https://en.m.wikipedia.org/wiki/Sticky_bit

With that being said, you are able to change those permissions in your child image.

@rlpowell
Copy link
Author

That doesn't explain why you want it in a container. The sticky bit functionality is designed for shared directories on shared systems with lots of users, like /tmp. A container is, by definition, not normally shared between users. There's no reason to have sticky bit on a directory that different actual humans are not sharing access to.

@LaurentGoderre
Copy link
Member

The purpose is to prevent PHP, a general purpose language, from being able to perform destructive operations on the filesystem if an attacker is able to inject malicious code that gets executed.

@rlpowell
Copy link
Author

These directories exist in a container. A bind mount won't have these perms, so they exist only in the container. The user that apache is running as in the container already has perms on all these files. If someone injects malicious code, it'll be running as that user, which already has perms on all these files. The sticky bit won't have any impact in what is fundamentally a single-user context.

@LaurentGoderre
Copy link
Member

The use case that this support is running the container as another user that therefore doesn't have permission to modify the data.

@rlpowell
Copy link
Author

Yes, that's exactly the problem I hit, with extremely surprising results. A container should not have this behaviour, it is not a multi-user system.

I really think this behaviour is going to surprise and confuse people in the future.

It's clear, though, that you're not interested in what I'm saying, so I'm going to stop now.

@tianon
Copy link
Member

tianon commented Jan 7, 2025

The goal of using 777 permissions in the first place was to allow running the container as an arbitrary user -- we switched to 1777 (matching /tmp in a standard install/system) to tighten those permissions because 777 alone is uncomfortably wide.

The error.log symlinks you mention are actually created here in this image:

ln -sfT /dev/stderr "$APACHE_LOG_DIR/error.log"; \

That being said, I can't reproduce a failure related to error.log: 🤔

$ docker run -it --rm --user 1234:5678 --pull=always php:apache
apache: Pulling from library/php
Digest: sha256:2c9ae64a55950a3b44c5121cae9b1dc82601e9ff2a0ed0279d02c047019ca53d
Status: Image is up to date for php:apache
AH00558: apache2: Could not reliably determine the server's fully qualified domain name, using 172.27.0.9. Set the 'ServerName' directive globally to suppress this message
AH00558: apache2: Could not reliably determine the server's fully qualified domain name, using 172.27.0.9. Set the 'ServerName' directive globally to suppress this message
[Tue Jan 07 01:19:59.244877 2025] [mpm_prefork:notice] [pid 1:tid 1] AH00163: Apache/2.4.62 (Debian) PHP/8.4.2 configured -- resuming normal operations
[Tue Jan 07 01:19:59.244910 2025] [core:notice] [pid 1:tid 1] AH00094: Command line: 'apache2 -D FOREGROUND'

Can you provide more details about how I could reproduce?

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

No branches or pull requests

3 participants