docker-library / postgres

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

POSTGRES_USER should not be superuser #175

Open teohhanhui opened 8 years ago

teohhanhui commented 8 years ago

Please make the user / database setup in line with the mysql image. Default database owned by superuser is most likely not what most would expect from a service.

avongluck-r1soft commented 7 years ago

this is a bad idea. Please leave it as a superuser. A lot of applications need to be able to enable extensions such as uuid support. There is no clear way to do something like that without the user being a superuser.

teohhanhui commented 7 years ago

An example /docker-entrypoint-initdb.d/pg_buffercache.sh, where POSTGRES_SUPERUSER=postgres. Not hard at all...

#!/bin/bash
set -e

psql -v ON_ERROR_STOP=1 --username="$POSTGRES_SUPERUSER" --dbname="$POSTGRES_DB" <<-'EOSQL'
    CREATE EXTENSION IF NOT EXISTS pg_buffercache;
EOSQL
teohhanhui commented 7 years ago

Common sense tells us that using the superuser by default is a security risk.

tianon commented 7 years ago

I think we're too late to change the default here (and tend to agree that by default, SUPERUSER is probably expected), but I'm definitely +1 to adding an easy way to opt out of that! (something like POSTGRES_USER_NOT_SUPER -- definitely want to hear alternate name suggestions for that)

teohhanhui commented 7 years ago

I'd love to hear from Postgres people on whether they think a default of superuser is "to be expected". (Is it a safe default? Because otherwise it should be treated as a security issue and fixed.)

I mean, it'd be great to hear it from the horse's mouth...

On 26 Jan 2017 07:31, "Tianon Gravi" notifications@github.com wrote:

I think we're too late to change the default here (and tend to agree that by default, SUPERUSER is probably expected), but I'm definitely +1 to adding an easy way to opt out of that! (something like POSTGRES_USER_NOT_SUPER -- definitely want to hear alternate name suggestions for that)

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/docker-library/postgres/issues/175#issuecomment-275266166, or mute the thread https://github.com/notifications/unsubscribe-auth/AAhf6z1A-bLrL2Ms6NH79XkyRGRbgUq_ks5rV9tggaJpZM4JLTjI .

Globegitter commented 7 years ago

Any news on this, would be great to have an option to create a user without superuser rights.

znmeb commented 7 years ago

The security model of PostgreSQL is complex but in operation

  1. The DBA needs to become a superuser at times but all other humans / processes can be forced to have the weakest permissions that enable them to fulfill the tasks dictated by their role.

  2. PostgreSQL servers (before Docker, anyhow) usually aren't sitting out on the open internet with an IP address known or discoverable by an attacker. They'd be connected to a Django or Rails or similar web application framework that did all the communication with end users and/or provided an API to them and connect to PostgreSQL via some object-relation mapping.

So I as a system integrator deploying PostgreSQL servers in containers using an "official" image would expect that I could log into the command line of a running container with docker exec as a PostgreSQL super-user if I had to fix something, at least in development and test. That might not be necessary in production. But I wouldn't expect port 5432 to be accessible outside of the local Docker network to any account in the PostgreSQL server.

teohhanhui commented 7 years ago

I don't understand that argument, because you can easily do (where POSTGRES_SUPERUSER=postgres):

psql --username="$POSTGRES_SUPERUSER"

Things should definitely be secure by default.

teohhanhui commented 7 years ago

PostgreSQL servers (before Docker, anyhow) usually aren't sitting out on the open internet with an IP address known or discoverable by an attacker.

Security by obscurity?

znmeb commented 7 years ago

@teohhanhui More like only allowing intra-server connections via Unix sockets - PostgreSQL service isn't even listening on localhost or the internet. With Docker, that corresponds to only listening on the internal Docker network.

I can't see a reason to host a PostgreSQL server listening on the internet unless you're a cloud provider and you're selling the service. If you're using PostgreSQL you should be the only one capable of connecting to it.

Globegitter commented 7 years ago

@znmeb You are right about your arguments, but you are still increasing the attack surface - if my app/service does not need super-user rights at runtime (and it doesn't) it should not have them.

znmeb commented 7 years ago

@Globegitter Absolutely! A lot of this is a carry-over from the Linux-Apache-RDBMS-PHP model where everything lived on a single server and only Apache talked to the user and only PHP talked to Apache and the RDBMS. In the Docker model, you have a lot more flexibility.

avongluck-r1soft commented 7 years ago

How about a way to pass in a list of extensions to create / enable via something more generic of what @teohhanhui recommended?

I agree superuser is likely a security issue... but anything that requires a superuser needs to have a way to have those actions performed without users forking and editing the postgres base container. (aka Bob's old-postgresql-with-uuid-suppport)

avongluck-r1soft commented 7 years ago
#!/bin/bash
set -e

for EXT in $POSTGRES_EXTENSIONS; do
    psql -v ON_ERROR_STOP=1 --username="$POSTGRES_SUPERUSER" --dbname="$POSTGRES_DB" <<-'EOSQL'
        CREATE EXTENSION IF NOT EXISTS $EXT;
    EOSQL
done

That's a pretty minimal example that needs more work though. Maybe validate aginst pg_available_extensions and display a friendly warning?

Then in the environment: POSTGRES_EXTENSIONS="uuid pg_buffercache etc"

teohhanhui commented 7 years ago

@avongluck-r1soft I don't see how that's better than https://github.com/docker-library/postgres/issues/175#issuecomment-274051394

We shouldn't create more shortcuts to do the same thing. But maybe it'd be great to have helper scripts like docker-php-ext-install, perhaps docker-postgres-ext-create.

alexanderkjeldaas commented 7 years ago

@teohhanhui your version would not work in CI setups for example - the behavior needs to be fully controlled through environment variables. The gitlab runner is one example.

embray commented 7 years ago

Security aside it's just safer from an application perspective. For example, this protects if someone "accidentally" adds code to the app, say, to drop the entire database.

yosifkit commented 7 years ago

In hindsight the POSTGRES_USER env was a poor choice of name. At this point, I think that we would break too many users by changing what access the POSTGRES_USER has by default, but as @tianon mentioned in January we are open to adding a NON_SUPER user creation similar to the MySQL, MariaDB, and PerconaDB images.

I see a two options that are mostly backwards compatible (new env names are up for debate):

  1. add a POSTGRES_USER_NOT_SUPER env to create a regular database user with all permissions to the database specified by POSTGRES_DB (GRANT ALL ON DATABASE ... TO ...) and leave POSTGRES_USER unchanged
  2. add a POSTGRES_SUPERUSER env and if it is provided along with POSTGRES_USER then POSTGRES_USER will not be a super user (but will have full permissions to POSTGRES_DB). If the POSTGRES_SUPERUSER is not provided then POSTGRES_USER will still be a superuser as it is currently

Even though is more complicated I am partial to number two. Basically if you only supply a single user (via POSTGRES_SUPERUSER or POSTGRES_USER) it will be a superuser, but if you supply both, then the logical one will be super and the other not. Plus we don't have to come up with a name for the non-super user variable. (the only debate I see is POSTGRES_SUPERUSER vs POSTGRES_SUPER_USER)


Any discussion about extensions is orthogonal to this issue and would be better suited to its own issue.

masterBrog commented 6 years ago

@yosifkit just putting it out there, could there be another option:

  1. Add two new users POSTGRES_DB_USER and POSTGRES_DB_SUPERUSER and then deprecate POSTGRES_USER. Gives backwards compatibility, encourages a move away from default superuser using deprecation, and has transparent/clear behaviour.
embray commented 6 years ago

@masterBrog's comment sounds good.

dakotahawkins commented 6 years ago

I'm having good luck with this, added to /docker-entrypoint-initdb.d/

I'm using the latest alpine image, so YMMV:

https://gist.github.com/dakotahawkins/d67054072d3ea2e507b1327ca544855f

WARNING Indent with tabs or the EOSQL bash heredoc won't work!

#!/usr/bin/env bash

####################################################################################################
# Initializes a new non-superuser user and password for use with the database
#
# This may be officially supported soon: https://github.com/docker-library/postgres/issues/175

main() {
    file_env 'POSTGRES_DB_USER'
    file_env 'POSTGRES_DB_PASSWORD'

    if [ "$POSTGRES_DB_USER" ] && [ "$POSTGRES_DB_PASSWORD" ] && [ "$POSTGRES_DB" ]; then
        "${psql[@]}" <<-EOSQL
            CREATE ROLE "$POSTGRES_DB_USER" WITH NOSUPERUSER LOGIN PASSWORD '$POSTGRES_DB_PASSWORD' ;
            GRANT ALL PRIVILEGES ON DATABASE "$POSTGRES_DB" TO "$POSTGRES_DB_USER" ;
        EOSQL
    else
        error-exit "POSTGRES_DB_USER, POSTGRES_DB_PASSWORD, POSTGRES_DB must all be set."
    fi
}

error-exit() {
    echo "$1" >&2
    echo
    exit 1
}

main "$@"

$POSTGRES_USER and $POSTGRES_PASSWORD still create a non-default superuser for the non-default $POSTGRES_DB, while this adds $POSTGRES_DB_USER with $POSTGRES_DB_PASSWORD as a non-superuser with access to $POSTGRES_DB.

Note: I'm using _FILEs instead of raw env. vars. Since docker-entrypoint.sh sourcees bash scripts in docker-entrypoint-initdb.d, we can "inherit" the file_env function it uses as well as the current psql command ("${psql[@]}").

Hopefully this helps somebody else!

petrus-v commented 6 years ago

Hi,

Thanks for all this greate works.

@tianon does this commit close this request or something slightly different ?

regards,

teohhanhui commented 6 years ago

No, it does not.

tianon commented 6 years ago

@petrus-v no, that commit does something slightly different, but it does IMO clarify the reason for our choice -- POSTGRES_USER is passed directly to initdb as-is. We don't create any users in our script now, and all users are directly created by PostgreSQL tooling. I think the solution in https://github.com/docker-library/postgres/issues/175#issuecomment-374038271 is the best one (using /docker-entrypoint-initdb.d/... to get exactly the desired behavior rather than adding even more custom behavior to the default image).

dakotahawkins commented 6 years ago

In case it helps anybody else, I've modified what I use for #175 (comment) mostly because I use a windows host and a semi-recent change stopped sourcing the file all the time if it's executable. Nothing major, just copied the file_env implementation and rebuilt ${psql[@]} locally. It's annoying, and I filed #490 about it, but probably just doing everything you need yourself is the most stable solution...

teohhanhui commented 6 years ago

@tianon

(using /docker-entrypoint-initdb.d/... to get exactly the desired behavior rather than adding even more custom behavior to the default image)

I'd still argue that it's a security risk to run as a db superuser out of the box.

tianon commented 6 years ago

Sure, but no matter what we do, postgres will create a super user.

quantumkoen commented 5 years ago

I can't see a reason to host a PostgreSQL server listening on the internet unless you're a cloud provider and you're selling the service. If you're using PostgreSQL you should be the only one capable of connecting to it.

Sorry for ressurecting an old thread, but since no-one pointed out the obvious I thought i'd add this: even though you are not exposing your database connection to the world, you don't want your web app to have superuser access. If your web app has a sql injection vulnerability, it might be possible to indirectly execute nasty commands on your postgresql server from the web. And if your web app has superuser access, it is even possible to mess with the containerized filesystem or possibly even get code executed (as the user that postgres is running as or root even).

It would not be the first time this is how sensitive data ends up in the public. Security is an in-depth matter: every chain in the link needs to be strong, just one weak link can be exploited for fun & profit.

We've created our own postgres container based on this one, that has additional variables to initialize a non-superuser account for use by the application. I sleep better at night knowing that my web app has no superuser access :)

embray commented 5 years ago

@quantumkoen 👍 and also do you have a link to your version of the Dockerfile?

quantumkoen commented 5 years ago

@embray sadly it's in a private repo, and open-sourcing anything here involves a lot of red-tape. But it's just two files and rather trivial:

Dockerfile:

FROM postgres:latest

ADD docker-entrypoint-initdb.d /docker-entrypoint-initdb.d

USER postgres

docker-entrypoint-initdb.d/init-user-db.sh

#!/bin/bash
set -e

# Get the postgres user or set it to a default value
if [ -n $POSTGRES_USER ]; then pg_user=$POSTGRES_USER; else pg_user="postgres"; fi
# Get the postgres db or set it to a default value
if [ -n $POSTGRES_DB ]; then pg_db=$POSTGRES_DB; else pg_db=$POSTGRES_USER; fi

if [ -n "$POSTGRES_NON_ROOT_USER" ]; then
psql -v ON_ERROR_STOP=1 --username "$pg_user" --dbname "$pg_db" <<-EOSQL
    CREATE USER $POSTGRES_NON_ROOT_USER with encrypted password '$POSTGRES_NON_ROOT_USER_PASSWORD';
    GRANT CREATE, CONNECT ON DATABASE $pg_db TO $POSTGRES_NON_ROOT_USER;
    ALTER DEFAULT PRIVILEGES IN SCHEMA public GRANT SELECT, UPDATE, INSERT, DELETE, REFERENCES ON TABLES TO $POSTGRES_NON_ROOT_USER;
EOSQL
fi

It adds two variables: POSTGRES_NON_ROOT_USER and POSTGRES_NON_ROOT_USER_PASSWORD which if present adds the non-superuser and gives it access to database POSTGRES_DB (or POSTGRES_USER if POSTGRES_DB is not set).

Oh, and it also runs the postgresql daemon as the postgres user instead of as root.

We may tighten the permissions a bit later on, they're still quite broad for a web app.

embray commented 5 years ago

@quantumkoen Thanks for the hints; that's certainly enough to go on.

quantumkoen commented 5 years ago

@embray actually, according to @oschusler (who created the dockerfile I quoted) the script was heavily inspired by the script on the dockerhub readme.

tuttle commented 3 years ago

Cpt. Obvious here who just wants to note that USER postgres used and explained in https://github.com/docker-library/postgres/issues/175#issuecomment-482105107 above seems obsoleted by the current pg 13+ images as the daemon already runs as non-root with UID 999, which is a postgres user in the container.

srstsavage commented 3 years ago

using /docker-entrypoint-initdb.d/... to get exactly the desired behavior rather than adding even more custom behavior to the default image

I totally get this, and I'm sure there are requests to add env var configs for all sorts of things. However, this specific use case (wanting to create a non-superuser in addition to the superuser specified by POSTGRES_USER) happens pretty much 100% of the time in my shop and I can only assume/hope that pattern is universal. Absolutely this can be done using docker-entrypoint-initdb.d scripts (that's what we do), but since it's such a common use case/best practice it would be really nice to be able to do this with an env var like masterBrog or quantumkoen suggested (POSTGRES_DB_USER/POSTGRES_NON_ROOT_USER).

virtualdxs commented 3 years ago

Exactly what @shane-axiom said - this is (and very much should be) a common enough use case to justify including environment variables to let us do this in a couple lines in a docker-compose file (for example) vs having to write an entrypoint script every time.

tianon commented 2 years ago

What if we added something like DOCKER_INITDB_EXTRA_SQL or something for a block of explicit SQL code to invoke in addition to (/ after) /docker-entrypoint-initdb.d?

srstsavage commented 2 years ago

What if we added something like DOCKER_INITDB_EXTRA_SQL or something for a block of explicit SQL code to invoke in addition to (/ after) /docker-entrypoint-initdb.d?

Makes sense to me and would cover a wide range of use cases.

virtualdxs commented 2 years ago

What if we added something like DOCKER_INITDB_EXTRA_SQL or something for a block of explicit SQL code to invoke in addition to (/ after) /docker-entrypoint-initdb.d?

That could be useful, but I still think there should be an environment variable explicitly for this use case. Ideally almost every time this image is used this environment variable would be used, as it's a significant security benefit and very few apps need superuser access to the database. Therefore, I feel it's certainly worth its own environment variable.

I particularly like https://github.com/docker-library/postgres/issues/175#issuecomment-337387487 's suggestion we use POSTGRES_SUPERUSER together with POSTGRES_USER to create the nonprivileged user. Doesn't break anybody's existing setups and allows a superuser and an unprivileged user to be created together.

srstsavage commented 2 years ago

I'm adding notes to our deployment tools about how using POSTGRES_USER to create the users used by apps is bad practice due to security concerns etc and thought I'd ping here. I agree that ideally we'd have a dedicated env var for non-superuser creation since it's an almost universal use case.

In the meantime, is there any reason not to put together a PR for DOCKER_INITDB_EXTRA_SQL so that we have some non-file based approach to address this?

aristofun commented 2 years ago

This is so weird to see the lack of one of the most commmon sense and not advanced feature in a docker image of the "most advanced" DB. Considering how well PG configuration is designed in general, it was a huge surprise for me.

srstsavage commented 10 months ago

For lack of a DOCKERDB_INITDB_EXTRA_SQL env var, the suggested approach to override the entrypoint and put the creation of initdb scripts into the Docker command is pretty clever:

docker run --rm -p 5432:5432 \
  -e POSTGRES_PASSWORD=asdf \
  --entrypoint "" \
  --name stupid-pg-tricks \
  postgres:16.1 \
  /bin/bash -c "echo CREATE ROLE nonsuper WITH LOGIN PASSWORD \'test\' >> /docker-entrypoint-initdb.d/50-create-nonsuper-user.sql && exec docker-entrypoint.sh postgres"

Quote management gets nasty etc, but it's possible if you really can't/don't want to template separate initdb files.

(Code for demo only, use a hashed password in CREATE ROLE or else your plain text password will be visible to anyone who can inspect the container, etc)

tianon commented 10 months ago

You can also easily adjust that to make it use an environment variable for slightly simpler quoting (and significantly simpler quoting if you're using YAML like in Compose or k8s); docker run --env FOO="CREATE ... 'test'" ... postgres bash -c 'echo "$FOO" > /docker-entrypoint-initdb.d/50-foo-env.sql && exec "$@"' -- docker-entrypoint.sh postgres

(Also using exec "$@" + -- to make arguments to postgres easier to quote correctly)