docker-library / postgres

Docker Official Image packaging for Postgres
http://www.postgresql.org
MIT License
2.16k stars 1.13k forks source link

Remove gosu #1262

Closed TommyTran732 closed 1 month ago

TommyTran732 commented 1 month ago

From the code, it seems like gosu is only used to drop privs:

https://github.com/docker-library/postgres/blob/66da3846b40396249936938ee17e9684e6968a57/16/alpine3.20/docker-ensure-initdb.sh#L30

https://github.com/docker-library/postgres/blob/66da3846b40396249936938ee17e9684e6968a57/16/alpine3.20/docker-ensure-initdb.sh#L30

The containers (at least 16-alpine) already have a postgres user with UID and GID 70. So I do not see a reason to have an unnecessary SUID binary in the container, when we can just tell the container to run as user postgres in the Dockerfile.

/var/lib/postgresql/data just needs to be owned by the postgres user and that's it.

LaurentGoderre commented 1 month ago

It isn't as simple as that. Gosu is used because the init script needs to change the user during initialization.

yosifkit commented 1 month ago

SUID binary

gosu is not setuid or setgid binary (and would refuse to run if it was), it can only go from root to a non-root user.

The image is designed such that a user can run it directly as the postgres (or any non-root user), but then the user is responsible for setting any volumes/mounts with correct permissions. gosu is there so that it will "just work" without a user knowing that they need to worry about permissions (i.e., they mount a local directory and the container fixes permissions and then uses gosu to drop down to the postgres user):

https://github.com/docker-library/postgres/blob/66da3846b40396249936938ee17e9684e6968a57/docker-entrypoint.sh#L36-L61

tianon commented 1 month ago

See also https://github.com/tianon/gosu/blob/052c5c2b186b84c4d9a41ed4f327490ef8d746fe/main.go#L49-L57 (for where the upstream gosu code will explicitly balk if it is installed with setuid or setgid bits).

(https://github.com/tianon/gosu/blob/master/SECURITY.md might also be interesting/relevant)