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

Data Administrators security group doesn't provide database access (or any access to the SRE) #1438

Closed
5 tasks done
edwardchalstrey1 opened this issue Apr 5, 2023 · 12 comments
Assignees
Milestone

Comments

@edwardchalstrey1
Copy link
Contributor

edwardchalstrey1 commented Apr 5, 2023

✅ Checklist

  • I have searched open and closed issues for duplicates.
  • This is a problem observed when managing a Data Safe Haven.
  • I can reproduce this with the latest version.
  • I have read through the documentation.
  • This isn't an open-ended question (open a discussion if it is).

💻 System information

  • Data Safe Haven version: 4.0.3
  • Operating system details: mac

🚫 Describe the problem

A user of an SRE should have the ability to create schemas in postgres via being in the SG <SRE ID> Data Administrators group, however there seems to be a problem with this group. At the moment, being a member of each of these groups results in the following problems:

  1. SG <SRE ID> Research Users: can log into the TRE and connect to postgres with psql -h PSTGRS-<SRE ID>.<SHM ID>.turingsafehaven.ac.uk -p 5432 -d postgres, but I try to create a schema with create schema test; we get ERROR: permission denied for database postgres which is what we expect. User was in both this group and the data admin group below when he got this.
  2. SG <SRE ID> Data Administrators or SG <SRE ID> System Administrators If a user is one of these, then they can log into the SRE first login, but not the Ubuntu login, we just get the No recent connections screen. I think ideally both of these access levels should allow login.

When I log in as the research user and connect to the psql db and run \du to check the user privileges, users in the SG <SRE ID> Data Administrators don't show up.

I have tested this on multiple SREs so this is not specific to one particular instance.

Linked to #1392

🚂 Workarounds or solutions

Connected as a superuser, and edited the permissions manually
- On the PSTGRS VM (logging in via Serial Console in Azure) do sudo -u postgres psql
- Then GRANT CREATE ON DATABASE postgres TO "user.name";

@edwardchalstrey1
Copy link
Contributor Author

edwardchalstrey1 commented Apr 6, 2023

I think I know the problem here: "dataAdminIpAddresses" needs to be set in the config when "databases" is set, but the DSH docs don't make this clear - I will test

Note: re-running just the database creation script of sbox2 with "dataAdminIpAddresses" in the config set to the same IP I am using seems to have not worked

@jemrobinson
Copy link
Member

Data Administrators should have permissions to create databases in existing schemas but not to create new schemas. Can we check this?

@craddm
Copy link
Contributor

craddm commented Jan 23, 2024

Some further notes on this one.

The intended behaviour seems to be as follows.

There is a public schema where anyone can create/read new tables.
There should be a data schema where Data Administrators can create new tables and Research Users can only read from those tables.

The behaviour in Ed's point 1 is thus the intended behaviour. However, there is a bug (corrected in #1708) where no data schema is created and the relevant script for setting permissions etc isn't copied to the PostgreSQL server. This is what @jemrobinson refers to above.

Ed's point 2 seems to be about a related point, but one that can't be addressed through modifying the database server itself. Instead, it seems like it would involve modifying how access to the relevant VMs is given through Guacamole, and I'm not sure whether it's best to address it or simply always give sys admins/data admins research user roles, so they can log in to the VM themselves.

See also comments on #1392

@jemrobinson
Copy link
Member

FYI for the Pulumi rewrite we have dropped a lot of this complication by having a local admin for the databases and (potentially) giving these admin credentials to the project PI as needed. This means that data ingress would need to be done on e.g. dumps from a database that can be imported, but means that the database servers don't have special permissions that need TRE admins to carry out any actions.

@craddm
Copy link
Contributor

craddm commented Jan 25, 2024

So it's great that lots of this complexity is gone with the new codebase, but still wondering how to handle this codebase.

Couple more things I've come across:

  1. In the MSSQL database it seems like anyone can do anything, or at least anyone with the Data Administrator or SysAdmin roles can create or delete arbitrary schemas etc.
  2. For the PostgreSQL database, people in the SRE System Administrator groups are supposed to be granted superuser privileges, but aren't. Looking at the code that is supposed to do this, I think the problem is the code to grant superuser privileges needs to be run by a superuser:

IF EXISTS (SELECT usename FROM pg_user WHERE ((usename = CURRENT_USER) AND (usesuper='t'))) THEN
https://github.com/alan-turing-institute/data-safe-haven/blob/fc4dc129c92f028f5bc02585e6b7208f9061349c/deployment/secure_research_environment/cloud_init/postgres_create_postgres_triggers.mustache.sql#L11C1-L11C97

This would make sense in that only superusers can modify roles, but if I'm reading this correctly, this code is by triggered by users who aren't superusers but should be. So they have to be superusers to make themselves superusers.

The docs need updated to explain how all this works, too. I'm working on a PR to do this but not sure whether the intended behaviour is as above (at least whether MSSQL users should be able to do whatever they want or not).

@jemrobinson
Copy link
Member

Excellent point that we need to focus on the existing codebase! It's certainly supposed to be the case that the MSSQL and PostgreSQL database servers behave in the same way.

Are we missing a line like sudo -i -u postgres psql -f /opt/configuration/create-postgres-triggers.sql somewhere here (

- sudo -i -u postgres psql -f /opt/configuration/install-postgres-extensions.sql
) ?

@craddm
Copy link
Contributor

craddm commented Jan 25, 2024

Excellent point that we need to focus on the existing codebase! It's certainly supposed to be the case that the MSSQL and PostgreSQL database servers behave in the same way.

Are we missing a line like sudo -i -u postgres psql -f /opt/configuration/create-postgres-triggers.sql somewhere here (

- sudo -i -u postgres psql -f /opt/configuration/install-postgres-extensions.sql

) ?

On develop, create-postgres-triggers.sql is in the wrong place, so was never copied to the server and run. I fixed that in #1708. It gets copied to runonce.d so runs on reboot, so the triggers are getting set up. But they don't trigger until anyone tries to do anything, so only get triggered by users who aren't superusers (yet).

I can't see anywhere where anything similar gets set up on the MSSQL server

@jemrobinson
Copy link
Member

$scriptPath = Join-Path $PSScriptRoot ".." "remote" "create_databases" "scripts" "Lockdown_Sql_Server.ps1"

runs Lockdown_Sql_Server.ps1 from

https://github.com/alan-turing-institute/data-safe-haven/blob/fc4dc129c92f028f5bc02585e6b7208f9061349c/deployment/secure_research_environment/remote/create_databases/scripts/Lockdown_Sql_Server.ps1

which sets permissions here

@jemrobinson
Copy link
Member

For PostgreSQL, would running a command in runonce as the psql superuser, trigger the commands appropriately?

@craddm
Copy link
Contributor

craddm commented Jan 25, 2024

$scriptPath = Join-Path $PSScriptRoot ".." "remote" "create_databases" "scripts" "Lockdown_Sql_Server.ps1"

runs Lockdown_Sql_Server.ps1 from

https://github.com/alan-turing-institute/data-safe-haven/blob/fc4dc129c92f028f5bc02585e6b7208f9061349c/deployment/secure_research_environment/remote/create_databases/scripts/Lockdown_Sql_Server.ps1

which sets permissions here

I did see that before but forgot 🤦 I'll try to figure out where it goes wrong!

@craddm
Copy link
Contributor

craddm commented Jan 31, 2024

While trying to debug some of this from the command line, it seems that, currently, making a user a superuser on the postgres server also adds them locally to the system admins group, even if they should not be in that group:

image image

This means that the current trigger setup cannot remove superuser privileges from these users, as it requires them no to be in the system admins groups.

This causes an issue if system admin status is removed at the SRE level (i.e. they are removed from the security group on the DC), as the remote sys admin status is ignored in favour of the database's own record of who is in the group.

edit:
An answer on stack exchange suggests that pg_has_role returns all roles for superusers:
https://dba.stackexchange.com/questions/56096/how-to-get-all-roles-that-a-user-is-a-member-of-including-inherited-roles

@craddm
Copy link
Contributor

craddm commented Feb 5, 2024

Closed by #1708

@craddm craddm closed this as completed Feb 5, 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

No branches or pull requests

3 participants