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

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

Closed
FelixZY opened this issue May 25, 2024 · 6 comments · Fixed by #216

Comments

@FelixZY
Copy link
Contributor

FelixZY commented May 25, 2024

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:

  • I'm having trouble running CREATE EXTENSION statements because they sometimes require superuser access.
  • graphile-migrate uses the DATABASE_URL credentials to apply migrations. However, the documentation explicitly uses another user than ROOT_DATABASE_URL for this connection called appuser. However, I do not want my application user to run CREATE TABLE/DROP TABLE etc..

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
Copy link
Member

benjie commented May 25, 2024

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
Copy link
Contributor Author

FelixZY commented May 25, 2024

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
Copy link
Member

benjie commented May 26, 2024

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
Copy link
Contributor Author

FelixZY commented May 26, 2024

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
Copy link
Member

benjie commented Jun 4, 2024

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
Copy link
Contributor Author

FelixZY commented Jun 4, 2024

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

FelixZY added a commit to FelixZY/graphile-migrate that referenced this issue Jun 10, 2024
As noted in graphile#215, the naming `appuser` may confuse users into
thinking the account to use for DATABASE_URL is the same as the
account that should be used by the application to connect to the
database. While this may be true in some setups, it is not a hard
requirement (see the discussion in graphile#215 for further details).

resolves graphile#215
FelixZY added a commit to FelixZY/graphile-migrate that referenced this issue Jun 10, 2024
FelixZY added a commit to FelixZY/graphile-migrate that referenced this issue Jun 10, 2024
As noted in graphile#215, the naming `appuser` may confuse users into
thinking the account to use for DATABASE_URL is the same as the
account that should be used by the application to connect to the
database. While this may be true in some setups, it is not a hard
requirement (see the discussion in graphile#215 for further details).

resolves graphile#215
FelixZY added a commit to FelixZY/graphile-migrate that referenced this issue Jun 10, 2024
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

Successfully merging a pull request may close this issue.

2 participants