Add pg_authid view

[adriangb]

Add a view to replace pg_catalog.pg_authid

[CLAassistant]

All committers have signed the CLA.

[github-actions]

@jnidzwetzki, @mkindahl: please review this pull request.

Powered by pull-review

[adriangb]

Happy to make this something fancier, e.g. to exclude replication users, roles with expired passwords, etc.

[mkindahl]

Add a view to replace pg_catalog.pg_authid

Hi @adriangb and thank you for the contribution!

What I do not understand is what this new view would give that pg_roles do not give. Is the purpose to have have role-based access to the rows of pg_authid and be able to read passwords?

[adriangb]

The password in pg_role is masked out. So yes as you say this gives you access to the password! I'm not sure about role-based access, we should probably give this the same access as pg_authid itself has by default (I'm guessing only the tsdbadmin user gets access?).

My primary use case is to use it with pgbouncer for auth_query passthrough auth, but I see no reason not to expose this information in general.

[mkindahl]

The password in pg_role is masked out. So yes as you say this gives you access to the password! I'm not sure about role-based access, we should probably give this the same access as pg_authid itself has by default (I'm guessing only the tsdbadmin user gets access?).

Well... there is a risk that you expose passwords that should not be exposed, so the view need to make sure to filter out users that it should not be able to view.

Look at this as an example of why creating a view on pg_authid (without additional measures in place) is not a good idea.

mats=# create user nobody;
CREATE ROLE
mats=# create user big_boss password 'xyzzy';
CREATE ROLE
mats=# select rolname, rolpassword from pg_authid where rolname in ('nobody', 'big_boss');
 rolname  |                                                              rolpassword                                                              
----------+---------------------------------------------------------------------------------------------------------------------------------------
 nobody   | [NULL]
 big_boss | SCRAM-SHA-256$4096:lsy3j3SwAq1oek+jCDUtXQ==$1xwEu5WjIHODQPDziQtWKN2Ujwlt3BeHggTCQayLr3I=:puzhnWt7w1jYQgIr9fDzEmIGR3O0cqRyuGc/gEeD5aw=
(2 rows)

mats=# create view leaky as select * from pg_authid;
CREATE VIEW
mats=# grant all on leaky to public ;
GRANT
mats=# set role nobody;
SET
mats=> select rolname, rolpassword from leaky where rolname in ('nobody', 'big_boss');
 rolname  |                                                              rolpassword                                                              
----------+---------------------------------------------------------------------------------------------------------------------------------------
 nobody   | [NULL]
 big_boss | SCRAM-SHA-256$4096:lsy3j3SwAq1oek+jCDUtXQ==$1xwEu5WjIHODQPDziQtWKN2Ujwlt3BeHggTCQayLr3I=:puzhnWt7w1jYQgIr9fDzEmIGR3O0cqRyuGc/gEeD5aw=
(2 rows)

My primary use case is to use it with pgbouncer for auth_query passthrough auth, but I see no reason not to expose this information in general.

Correct, passwords should not be exposed unnecessarily, not even hashes of passwords.

[adriangb]

I understand the example but maybe I'm missing what it proves. What I want is for tsdbadmin (or a pooler account specifically granted those permissions via a security definer function created by tsdbadmin) to be able to read password hashes. My understanding is that, as far as timescale is concerned, the thing that needs to be prohibited is for tsdbadmin to view the superuser password. If a timescale user then creates a view on this view using tsdbadmin and does grant all on <view> to public that's on them.

Can't we make this view only accessible to tsdbadmin (by not doing grant all on leaky to public ; and instead doing revoke all... and grant execute on leaky to tsdbadmin or similar)?

[mkindahl]

I understand the example but maybe I'm missing what it proves. What I want is for tsdbadmin (or a pooler account specifically granted those permissions via a security definer function created by tsdbadmin) to be able to read password hashes.

Can't we make this view only accessible to tsdbadmin (by not doing grant all on leaky to public ; and instead doing revoke all... and grant execute on leaky to tsdbadmin or similar)?

Yes, that is possible, but it requires additional measures in place. In essence, you need to make sure that the view only shows passwords that the tsdbadmin should see (and tsdbadmin is not a superuser).

Note that only users that belong to the tsdbadmin role and to roles created by the tsdbadmin role should be shown (the same applies to any role, so it is not specific to tsdbadmin). You can look at timescaledb_information.log_errors if you want some idea of how it can be implemented.

Also note that there are still risks involved with this since you are sending the password over the network so it is necessary to make sure that you have an encrypted connection to the database.

[adriangb]

Yes, that is possible, but it requires additional measures in place. In essence, you need to make sure that the view only shows passwords that the tsdbadmin should see (and tsdbadmin is not a superuser).

Agreed. I'm not sure how you'd define that, I went with "any non-superuser".

Also note that there are still risks involved with this since you are sending the password over the network so it is necessary to make sure that you have an encrypted connection to the database.

Of course. If you're setting up a connection pooler it's either this or you store the passwords in plaintext on the connection pooler, which seems much more unsafe and inflexible.

[mkindahl]

@adriangb We're happy to accept the contribution, but in order for this to not introduce a security issue, we would need the following changes:

• Make sure that for any user X, only rows belonging to role X is shown.
• Add tests to demonstrate that it works this way.

[adriangb]

Make sure that for any user X, only rows belonging to role X is shown.

Do you mean that a user can only see their own password? The whole point is that you can have a user (lets say tsdbadmin by default) that can see all passwords (except superuser passwords)

[mkindahl]

Make sure that for any user X, only rows belonging to role X is shown.

Do you mean that a user can only see their own password? The whole point is that you can have a user (lets say tsdbadmin by default) that can see all passwords (except superuser passwords)

Well, no, of course not, but the user should not be allowed to see passwords for accounts that it is not explicitly granted permission to view. For example, if it is a non-superuser, it should not be allowed to see passwords of superusers (which you're handling). We need something similar for handling non-superuser admin users as well.

If you have tests set up and try to do something similar to how it works for timescaledb_information.job_errors we can work from there and make sure that there are no security issues.

I realize that the "requirement" is a little vague, but I think you get the gist. We do not want to expose passwords or hashes to users that are not allowed to view them.

[feikesteenbergen]

This needs a bit more design, referring to a blog post here

We could come up with a security definer functions that is limiting the allowed hashes. For example, something like:

CREATE FUNCTION public.get_password_hash (
   INOUT user     name,
   OUT   password text
) RETURNS record
   LANGUAGE sql SECURITY DEFINER SET search_path = pg_catalog AS
$$SELECT usename, passwd FROM pg_catalog.pg_shadow WHERE usename = user AND pg_catalog.pg_has_role(user, 'member') $$;

However, I'm not sure if we want to make this change to timescaledb. What problem are you solving exactly?

[adriangb]

What problem are you solving exactly?

As per some of the comments above I was trying to set up pgbouncer myself (for reasons I don't think it's worth getting into) and to do that you either need to put all of the passwords in a text file and keep them updated or use auth_query which needs to be able to read the password hashes.

This needs a bit more design, referring to a blog post here

Precisely, let's consider the use case in that blog post. pg_shadow is an alias for pg_authid and the latter should be used instead, so I'll ignore that difference (ref). The issue is that it's impossible to do what that blog posts suggest with Timescale (as far as I can tell) because you can't define a function like:

CREATE FUNCTION public.lookup (
   INOUT p_user     name,
   OUT   p_password text
) RETURNS record
   LANGUAGE sql SECURITY DEFINER SET search_path = pg_catalog AS
$$SELECT usename, passwd FROM pg_authid WHERE usename = p_user$$;

Because no user, not even tsdbadmin has access to pg_authid (presumably because it contains superuser passwords).

As per the article:

pg_authid is only accessible to superusers

Thus my goal here was to make a view that excludes superuser passwords but is still only accessible to the timescale cloud equivalent of a superuser which would be tsdbadmin. Hence my suggestion for permissions would be:

REVOKE ALL ON timescaledb_information.pg_authid FROM public;
GRANT SELECT ON timescaledb_information.pg_authid TO tsdbadmin;

Or something like that. The goal being that when logged in as tsdbadmin you can create a pgbouncer user and a security definer function.

[feikesteenbergen]

@adriangb a security definer function (as above) runs in the security context of the user that created it (postgres), as opposed to the one running it. That is also why we lock down the search_path, as one needs to be very strict with how to define such a function.

Therefore, access to pg_authid is possible within the function.

The usefulness of the function is that it clearly creates a boundary where we switch from a regular user to a superuser.

because you can't define a function like:

That's why we could create such a function as a superuser, and grant execute on that function to tsdbadmin for example.

check the topic The slightly more complicated method (querying the database) in the referred blog post.

[adriangb]

a security definer function (as above) runs in the security context of the user that created it (postgres), as opposed to the one running it

Yes I know that. But postgres is not available on Timescale cloud, only tsdbadmin. And tsdbadmin doesn't have access to pg_authid. So there is no user to define the function as! The goal of this feature request/PR is to give tsdbadmin the ability to read from pg_authid so that you can create a security definer function when logged in as tsdbadmin that then gets called by some other user (pgbouncer or whatever you want to call it).

check the topic The slightly more complicated method (querying the database) in the referred blog post.

Maybe I'm missing something but the code snippit in my previous comment was literally copied from that section.

[adriangb]

It's still not clear to me what the missing discussion or design is here. I just want to have tsdbadmin be a bit closer to the permissions of a superuser. This is a limitation of Timescale that doesn't seem to be necessary, at least according to this discussion and the fact that CrunchyBridge doesn't have this issue (they give you superuser access in their cloud offering).