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

Ensure that per-service postgres users are not superusers #2498

Closed
djmitche opened this issue Mar 12, 2020 · 5 comments · Fixed by #2506
Closed

Ensure that per-service postgres users are not superusers #2498

djmitche opened this issue Mar 12, 2020 · 5 comments · Fixed by #2506
Assignees

Comments

@djmitche
Copy link
Collaborator

In db:ugprade, we check that per-table permissions correspond to those in db/access.yml, but we don't look at whether a user is a superuser. We should.

@djmitche djmitche self-assigned this Mar 12, 2020
@edunham
Copy link
Contributor

edunham commented Mar 12, 2020

This is because when users are created with gcp's command line tooling, they automatically get the superuser, createrole, and createdb roles assigned. I believe that alter role taskcluster_notify set nosuperuser nocreaterole nocreatedb (or whatever the correct perms are) will no-op if the service user's roles are correct, and fix them if they aren't.

I have verified that the credentials with which db:upgrade is called are able to make the above changes to users.

@sciurus
Copy link
Contributor

sciurus commented Mar 12, 2020

If this can be done in such a way that

  1. Any unexpected attributes are removed from each services role
  2. Any unexpected role memberships are removed from each services role

then it sounds okay.

What will happen by default if we continue setting up the non-admin users the way edunham is doing it is

taskcluster-> \du
                                                List of roles
         Role name         |                         Attributes                         |      Member of      
---------------------------+------------------------------------------------------------+---------------------
 taskcluster               | Create role, Create DB                                     | {cloudsqlsuperuser}
 taskcluster_notify        | Create role, Create DB                                     | {cloudsqlsuperuser}

We can verify by testing after this command is run and seeing

                                                List of roles
         Role name         |                         Attributes                         |      Member of      
---------------------------+------------------------------------------------------------+---------------------
 taskcluster               | Create role, Create DB                                     | {cloudsqlsuperuser}
 taskcluster_notify        |                                                            | {}

@djmitche
Copy link
Collaborator Author

So far we've been checking but not enforcing permissions in db:upgrade. The idea is that any discrepancy is potentially an IOC and we don't want to sweep it under the rug. So I'll continue to do that here.

@djmitche
Copy link
Collaborator Author

postgres=# select
  r.rolname as role,
  u.rolname
from
  pg_catalog.pg_roles as r,
  pg_catalog.pg_roles as u,
  pg_catalog.pg_auth_members
where
 r.oid = roleid and
 u.oid = member and
 u.rolname like 'test\_%';                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                     
     role      |   rolname                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                    
---------------+-------------
 somefunkyrole | test_github

@djmitche
Copy link
Collaborator Author

postgres=# select
  rolname as user,
  rolsuper,
  rolcreaterole,
  rolcreatedb,
  rolreplication
from
  pg_catalog.pg_roles
where
  (rolsuper or rolcreaterole or rolcreatedb or rolreplication ) and
  rolname like 'test\_%';                                                                                                                                                                                                                                                                                                                                                                                                                                                                                              
   user    | rolsuper | rolcreaterole | rolcreatedb | rolreplication                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                     
-----------+----------+---------------+-------------+----------------
 test_auth | t        | f             | t           | f

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.

3 participants