graphile / migrate

Opinionated SQL-powered productive roll-forward migration tool for PostgreSQL.
MIT License
751 stars 58 forks source link

Why are migrations not always run as the root user/why does graphile-migrate require multiple users? #215

Closed FelixZY closed 5 months ago

FelixZY commented 6 months ago

Summary

My initial experimentation with grahile-migrate has had me run into a few issues with permissions (for example, see the discussion in #214 ). More specifically:

So far, this has led to me creating a "special" graphile_migrate user with permissions to apply changes. However, given the constant permission issues, this user is essentially becoming another superuser to secure. Therefore, I'm thinking of just switching DATABASE_URL to use the same admin user as ROOT_DATABASE_URL instead.

My question is why DATABASE_URL is documented to use an appuser rather than root/postgres? Are there specific reasons graphile-migrate requires multiple users or security concerns with using the root/postgres user?

benjie commented 6 months ago

Postgres is an RDBMS, a relational database management system, designed to manage multiple databases. Each database will typically be used with a different application, and for security one application’s credentials should not be able to view, modify, destroy or corrupt another application’s database. To achieve this, each database has an “owner” which is like a mini-superuser; the owner can do most things inside the database: create/alter/drop tables, views, function, policies, permissions, etc, but cannot affect other databases (if permissions are carefully set up). We call this owner role the app user, since it’s the user used by this app to own the database.

You may use whatever role you want; Migrate doesn’t care. Use the superuser role if you want, though I’d advise against it from a security point of view: I wouldn’t want the app user credentials being able to alter the template database or install untrusted extensions like pythonu potentially turning an SQL injection attack into a remote code execution attack. But it’s your risk to analyse and to take.

FelixZY commented 6 months ago

Thank you for your in-depth response - let me see if I have this down correctly:

ROOT_DATABASE_URL

This variable is used to access a superuser, typically named postgres. This connection is only used during development in order to recreate the shadow database when necessary.

The postgres user is a highly privileged account (similar to root on linux or SYSTEM on windows) that should generally only be used with extreme caution. However, it is a necessary evil in order to install extensions ("software") and create databases.

Given the privileges of postgres, ROOT_DATABASE_URL is used for development only. This also means that the official recommendation of graphile-migrate maintainers is that commands such as CREATE DATABASE, CREATE EXTENSION etc. should be run manually outside the migrations managed by graphile-migrate. Correct?

DATABASE_URL

This variable is used to access the owner of the given database, named appuser in the docs. This connection is used both during development and production to apply migrations.

The owner is also a privileged account but should not be able to leave the context of its own database. This limits its ability to install extensions and cause other problems in case one or more migrations prove problematic (an example would be a bug inside a CREATE FUNCTION ... SECURITY DEFINER ... which could lead to privilege escalation within the db). By running migrations as the appuser account, damage is limited to the current database at the cost of forcing certain setup (such as CREATE EXTENSION etc.) to be perfomed manually, outside the migrations managed by graphile-migrate. From the perspective of a single-db postgres instance, the differing amounts of damage that can be caused by the postgres vs the appuser accounts may not be as obvious.


I think I was confused by the appuser naming and not thinking about the ability to have multiple databases under one postgres instance. To me, appuser sounds like the user my application would use rather than the database owner. However, I now understand this is not what you are promoting.

Perhaps this could be clarified in the docs in some way, e.g. by renaming appuser to dbowner or clarifying with something along the lines of "appuser most likely is not the user your application will be using to connect to the database, given its elevated privileges"?


Again, many thanks for your help as well as your work on graphile-migrate @benjie - I'm liking it really well so far!

benjie commented 5 months ago

With a slight clarification that cluster-level actions (create database, create user, etc) should never happen in migrations anyway, essentially you have the crux of it, yes. Also I wouldn’t necessarily forbid ROOT_DATABASE_URL being used in prod, it may be needed for a hook like beforeAllMigrations, perhaps to ensure all the extensions are installed and all the user accounts exist.

Feel free to submit a PR renaming appuser to dbowner, that seems reasonable.

Often the dbowner role will be the role your application uses to connect to the database; for example Rails would use the owner role. Some more secure setups, for example a PostGraphile instance, might be more cautious and use a lower privileged role instead, but often even then privileged code such as background workers and code related to account management will use the owner role as it bypasses things like row level security. So typically it is the role the app uses.

FelixZY commented 5 months ago

I'll get back to you with a PR for dbowner once I'm back at my PC 👍. Would an additional link back to this discussion be helpful as well as a guide for future users?


After doing some more thinking, what are your thoughts on GRANTs and RLS policies? If I'm too manage my roles outside the migrations I cannot ensure my current list of roles match with the roles required at a given point in time. I'd imagine this can cause problems when "migrating from zero" for example.

benjie commented 5 months ago

Would an additional link back to this discussion be helpful as well as a guide for future users?

Sure!

After doing some more thinking, what are your thoughts on GRANTs and RLS policies? If I'm too manage my roles outside the migrations I cannot ensure my current list of roles match with the roles required at a given point in time. I'd imagine this can cause problems when "migrating from zero" for example.

GRANT (when done on tables, columns, functions, views, etc) and CREATE POLICY belong to the database, not the cluster, so performing them in migrations is fine (and encouraged). It's your responsibility to ensure that the required roles exist cluster-wide before migrating; one way to do so is with a root beforeAllMigrations or afterReset hook. Note that migrations should not store passwords, so you may choose to use Graphile Migrate's placeholder replacement features for that. Similarly you would also ensure things like Graphile Worker are installed via beforeAllMigrations or afterReset.

FelixZY commented 5 months ago

Great, thank you very much! I'm currently looking at migrating the postgis extension to v5 (https://github.com/graphile/postgis/issues/58) but I'll make a PR for this issue in a few days as well.