Date: Sat, 27 Dec 2014 03:09:26 -0500
From: Noah Misch <noah@leadboat.com>
To: Adam Brightwell <adam.brightwell@crunchydatasolutions.com>
Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Subject: Re: CATUPDATE confusion?
Message-ID: <20141227080926.GA2035611@tornado.leadboat.com>
References: 
 <CAKRt6CQOvT2KiyKg2gFf7h9k8+JVU1149zLb0EXTkKk7TAQGMQ@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: 
 <CAKRt6CQOvT2KiyKg2gFf7h9k8+JVU1149zLb0EXTkKk7TAQGMQ@mail.gmail.com>
User-Agent: Mutt/1.5.23 (2014-03-12)
Precedence: bulk
Sender: pgsql-hackers-owner@postgresql.org

On Mon, Nov 24, 2014 at 04:28:46PM -0500, Adam Brightwell wrote:
> So, where I find this confusing is that the documentation supports this
> functionality and the check keeps superuser separate from CATUPDATE...
> however... comments and implementation in user.c state/do the opposite and
> couple them together.
> 
> Documentation:
> http://www.postgresql.org/docs/9.4/static/catalog-pg-authid.html - "Role
> can update system catalogs directly. (Even a superuser cannot do this
> unless this column is true)"
> 
> src/backend/commands/user.c
> 
> /* superuser gets catupdate right by default */
> new_record[Anum_pg_authid_rolcatupdate - 1] = BoolGetDatum(issuper);
> 
> and...
> 
> /*
>  * issuper/createrole/catupdate/etc
>  *
>  * XXX It's rather unclear how to handle catupdate.  It's probably best to
>  * keep it equal to the superuser status, otherwise you could end up with
>  * a situation where no existing superuser can alter the catalogs,
>  * including pg_authid!
>  */
> if (issuper >= 0)
> {
>   new_record[Anum_pg_authid_rolsuper - 1] = BoolGetDatum(issuper > 0);
>   new_record_repl[Anum_pg_authid_rolsuper - 1] = true;
> 
>   new_record[Anum_pg_authid_rolcatupdate - 1] = BoolGetDatum(issuper > 0);
>   new_record_repl[Anum_pg_authid_rolcatupdate - 1] = true;
> }

That documentation is correct as far it goes.  It does neglect to mention
that, as you have discovered, any CREATE ROLE or ALTER ROLE [NO]SUPERUSER will
change rolcatupdate to match.

> Perhaps this is not an issue but it seemed odd to me, especially after
> giving Peter's comment more thought.  So, I suppose the question is whether
> or not a superuser check should be added to 'has_rolcatupdate' or not?  I

I would not.  PostgreSQL has had this feature since day one (original name
"usecatupd").  It has two use cases, (1) giving effect to non-superuser
catalog grants and (2) preventing a narrow class of superuser mistakes.  We
wouldn't add such a thing today, but one can safely ignore its existence.
Making has_rolcatupdate() approve superusers unconditionally would exclude use
case (2).  Neither use case is valuable, but if I had to pick, (2) is more
valuable than (1).

> believe I understand the reasoning for coupling the two at role
> creation/alter time, however, is there really a case where a superuser
> wouldn't be allowed to bypass this check?  Based on the comments, there
> seems like there is potentially enough concern to allow it.  And if it is
> allowed, couldn't CATUPDATE then be treated like every other attribute and
> the coupling with superuser removed?  Thoughts?

A superuser always has _some_ way to bypass the check, if nothing else by
loading a C-language function to update pg_authid.  The user.c code you quoted
ensures that, if the DBA manages to remove rolcatupdate from every user, a
simple CREATE ROLE or ALTER ROLE can fix things.


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers