Re: Proposal: SET ROLE hook

From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: Joe Conway <mail(at)joeconway(dot)com>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Proposal: SET ROLE hook
Date: 2016-01-04 11:05:49
Message-ID: CAFj8pRA14jp_eSMXhXyqQ+T5B=tw4_8sJGbZxcaLCgMSuobyaw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi

2015-10-16 18:20 GMT+02:00 Joe Conway <mail(at)joeconway(dot)com>:

> In many environments there is a policy requiring users to login using
> unprivileged accounts, and then escalate their privileges if and when
> they require it. In PostgreSQL this could be done by granting the
> superuser role to an unprivileged user with noinherit, and then making
> the superuser role nologin. Something like:
>
> 8<-------------
> psql -U postgres
> create user joe noinherit;
> grant postgres to joe;
> alter user postgres nologin;
> \q
> psql -U joe
> -- do stuff *not requiring* escalated privs
> set role postgres;
> -- do stuff *requiring* escalated privs
> reset role;
> 8<-------------
>
> One of the problems with this is we would ideally like to know whenever
> joe escalates himself to postgres. Right now that is not really possible
> without doing untenable things such as logging every statement.
>
> In order to address this issue, I am proposing a new SET ROLE hook. The
> attached patch (role-esc-hook.diff) is I believe all we need. Then
> extension authors could implement logging of privilege escalation.
>
> A proof of concept extension patch is also attached. That one is not
> meant to be applied, just illustrates one potential use of the hook. I
> just smashed it on top of passwordcheck for the sake of convenience.
>
> With both patches applied, the following scenario:
> 8<------------------------
> psql -U joe postgres
> psql (9.6devel)
> Type "help" for help.
>
> postgres=> set role postgres;
> SET
> postgres=# select rolname, rolpassword from pg_authid;
> rolname | rolpassword
> ----------+-------------
> joe |
> postgres |
> (2 rows)
>
> postgres=# set log_statement = none;
> SET
> postgres=# reset role;
> RESET
> 8<------------------------
>
> Generates the following in the log:
>
> 8<------------------------
> LOG: Role joe transitioning to Superuser Role postgres
> STATEMENT: set role postgres;
> LOG: statement: select rolname, rolpassword from pg_authid;
> LOG: statement: set log_statement = none;
> LOG: Superuser Role postgres transitioning to Role joe
> STATEMENT: reset role;
> 8<------------------------
>
> Note that we cannot prevent joe from executing
> set log_statement = none;
> but we at least get the evidence in the log and can ask joe why he felt
> the need to do that. We could also set up alerts based on the logged
> events, etc.
>
> This particular hook will not capture role changes due to SECURITY
> DEFINER functions, but I think that is not only ok but preferred.
> Escalation during a SECURITY DEFINER function is a preplanned sanctioned
> event, unlike an ad hoc unconstrained role change to superuser. And
> given the demo patch, we could see any SECURITY DEFINER function created
> by the superuser when it gets created in the log (which again is subject
> to auditing, alerts, etc.)
>
> Ultimately I would also like to see a general hook available which would
> fire for all changes to GUC settings, but I think this one is still very
> useful as it is highly targeted.
>
> Comments?
>

I read discussion in this thread and I am thinking so creating hook is the
most simple and workable solution.

Personally, I am not sure if missing support SECURE DEFINER function is ok.
When you do some secure audit, probably any role change can be interesting.
This situation can be distinguished by another hook parameter.

Support of event triggers can be other topic, nice to have it but not
necessary in first moment.

Regards

Pavel

>
> Thanks,
>
> Joe
>
> --
> Crunchy Data - http://crunchydata.com
> PostgreSQL Support for Secure Enterprises
> Consulting, Training, & Open Source Development
>
>

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message amul sul 2016-01-04 11:11:28 Bug in MergeAttributesIntoExisting() function.
Previous Message Shulgin, Oleksandr 2016-01-04 10:38:01 Re: \x auto and EXPLAIN