Thread

Commits

Fix brain fade in e530be2c5ce77475d56ccf8f4e0c4872b666ad5f.
- 8bb3ad462f8d 16.0 landed
Do not allow removal of superuser privileges from bootstrap user.
- e530be2c5ce7 16.0 landed

let's disallow ALTER ROLE bootstrap_superuser NOSUPERUSER

Robert Haas <robertmhaas@gmail.com> — 2022-07-21T16:15:30Z

Hi,

Currently, it's possible to remove the rolissuper bit from the
bootstrap superuser, but this leaves that user - and the system in
general - in an odd state. The bootstrap user continues to own all of
the objects it owned before, e.g. all of the system catalogs. Direct
DML on system catalogs is blocked by pg_class_aclmask_ext(), but it's
possible to do things like rename a system function out of the way and
create a new function with the same signature. Therefore, creating a
new superuser and making the original one a non-superuser is probably
not viable from a security perspective, because anyone who gained
access to that role would likely have little difficulty mounting a
Trojan horse attack against the current superusers.

There are other problems, too. (1) pg_parameter_acl entries are
considered to be owned by the bootstrap superuser, so while the
bootstrap user loses the ability to directly ALTER SYSTEM SET
archive_command, they can still grant that ability to some other user
(possibly one they've just created, if they still have CREATEROLE)
which pretty much gives the whole show away. (2) When a trusted
extension is created, the extension objects are documented as ending
up owned by the bootstrap superuser, and the bootstrap user will end
up owning them even if they are no longer super. (3) Range
constructors end up getting owned by the bootstrap user, too. I
haven't really tried to verify whether ownership of trusted extension
objects or range constructors would allow the bootstrap
not-a-superuser to escalate back to superuser, but it seems fairly
likely. I believe these object ownership assignments were made with
the idea that the bootstrap user would always be a superuser.

pg_upgrade refers to the "install user" rather than the bootstrap
superuser, but it's talking about the same thing. If you've made the
bootstrap user non-super, pg_upgrade will fail. It is only able to
connect as the bootstrap user, and it must connect as superuser or it
can't do the things it needs to do.

All in all, it seems to me that various parts of the system are built
around the assumption that you will not try to execute ALTER ROLE
bootstrap_superuser NOSUPERUSER. I suggest that we formally prohibit
that, as per the attached patch. Otherwise, I suppose we need to
prevent privilege escalation attacks from a bootstrap ex-superuser,
which seems fairly impractical and a poor use of engineering
resources. Or I suppose we could continue with the present state of
affairs where our code and documentation assume you won't do that but
nothing actually stops you from doing it, but that doesn't seem to
have much to recommend it.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

Re: let's disallow ALTER ROLE bootstrap_superuser NOSUPERUSER

Tom Lane <tgl@sss.pgh.pa.us> — 2022-07-21T16:28:07Z

Robert Haas <robertmhaas@gmail.com> writes:
> Currently, it's possible to remove the rolissuper bit from the
> bootstrap superuser, but this leaves that user - and the system in
> general - in an odd state. The bootstrap user continues to own all of
> the objects it owned before, e.g. all of the system catalogs. Direct
> DML on system catalogs is blocked by pg_class_aclmask_ext(), but it's
> possible to do things like rename a system function out of the way and
> create a new function with the same signature. Therefore, creating a
> new superuser and making the original one a non-superuser is probably
> not viable from a security perspective, because anyone who gained
> access to that role would likely have little difficulty mounting a
> Trojan horse attack against the current superusers.

True, but what if the idea is to have *no* superusers?  I seem
to recall people being interested in setups like that.

On the whole I don't have any objection to your proposal, I just
worry that somebody else will.

Of course there's always "UPDATE pg_authid SET rolsuper = false",
which makes it absolutely clear that you're breaking the glass cover.

			regards, tom lane

Re: let's disallow ALTER ROLE bootstrap_superuser NOSUPERUSER

David G. Johnston <david.g.johnston@gmail.com> — 2022-07-21T16:41:04Z

On Thu, Jul 21, 2022 at 9:28 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

> Robert Haas <robertmhaas@gmail.com> writes:
> > Currently, it's possible to remove the rolissuper bit from the
> > bootstrap superuser, but this leaves that user - and the system in
> > general - in an odd state. The bootstrap user continues to own all of
> > the objects it owned before, e.g. all of the system catalogs. Direct
> > DML on system catalogs is blocked by pg_class_aclmask_ext(), but it's
> > possible to do things like rename a system function out of the way and
> > create a new function with the same signature. Therefore, creating a
> > new superuser and making the original one a non-superuser is probably
> > not viable from a security perspective, because anyone who gained
> > access to that role would likely have little difficulty mounting a
> > Trojan horse attack against the current superusers.
>
> True, but what if the idea is to have *no* superusers?  I seem
> to recall people being interested in setups like that.
>


> On the whole I don't have any objection to your proposal, I just
> worry that somebody else will.
>
> Of course there's always "UPDATE pg_authid SET rolsuper = false",
> which makes it absolutely clear that you're breaking the glass cover.
>
>
I would expect an initdb option (once this is possible) to specify this
desire and we just never set one up in the first place.  It seems
impractical to remove one after it already exists.  Though we could enable
the option (or a function) tied to the specific predefined role that, say,
permits catalog changes, when that day comes.

David J.

Re: let's disallow ALTER ROLE bootstrap_superuser NOSUPERUSER

Tom Lane <tgl@sss.pgh.pa.us> — 2022-07-21T16:46:57Z

"David G. Johnston" <david.g.johnston@gmail.com> writes:
> On Thu, Jul 21, 2022 at 9:28 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> True, but what if the idea is to have *no* superusers?  I seem
>> to recall people being interested in setups like that.

> I would expect an initdb option (once this is possible) to specify this
> desire and we just never set one up in the first place.  It seems
> impractical to remove one after it already exists.

There has to be a role that owns the built-in objects.  Robert's point
is that pretending that that role isn't high-privilege is silly.

			regards, tom lane

Re: let's disallow ALTER ROLE bootstrap_superuser NOSUPERUSER

Robert Haas <robertmhaas@gmail.com> — 2022-07-21T16:47:30Z

On Thu, Jul 21, 2022 at 12:28 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> True, but what if the idea is to have *no* superusers?  I seem
> to recall people being interested in setups like that.

Hmm, right. There's nothing that stops you from de-super-ing all of
your superusers today, and then if you ever need to do anything as
superuser again, you have to start up in single-user mode, which will
treat your session as super regardless. But considering how much power
the bootstrap user still has, I'm not sure that's really buying you
very much. In particular, the new GRANT ALTER SYSTEM stuff looks
sufficient to allow the bootstrap user to break out to the OS, so if
we want to regard no-superusers as a supported configuration, we
probably need to tighten that up. I think it's kind of hopeless,
though, because of the fact that you can also freely Trojan functions
and operators in pg_catalog. Maybe that's insufficient to break out to
the OS or assume superuser privileges, but you should be able to at
least Trojan every other user on the system.

> On the whole I don't have any objection to your proposal, I just
> worry that somebody else will.

OK, good to know. Thanks.

> Of course there's always "UPDATE pg_authid SET rolsuper = false",
> which makes it absolutely clear that you're breaking the glass cover.

Right.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

Re: let's disallow ALTER ROLE bootstrap_superuser NOSUPERUSER

Tom Lane <tgl@sss.pgh.pa.us> — 2022-07-21T17:02:50Z

Robert Haas <robertmhaas@gmail.com> writes:
> ... if
> we want to regard no-superusers as a supported configuration, we
> probably need to tighten that up. I think it's kind of hopeless,

Yeah, I agree.  At least, I'm uninterested in spending any of my
own time trying to make that usefully-more-secure than it is today.
If somebody else is interested enough to do the legwork, we can
look at what they come up with.

			regards, tom lane

Re: let's disallow ALTER ROLE bootstrap_superuser NOSUPERUSER

Nathan Bossart <nathandbossart@gmail.com> — 2022-07-21T17:27:26Z

On Thu, Jul 21, 2022 at 01:02:50PM -0400, Tom Lane wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> ... if
>> we want to regard no-superusers as a supported configuration, we
>> probably need to tighten that up. I think it's kind of hopeless,
> 
> Yeah, I agree.  At least, I'm uninterested in spending any of my
> own time trying to make that usefully-more-secure than it is today.
> If somebody else is interested enough to do the legwork, we can
> look at what they come up with.

Given the current assumptions the code makes about the bootstrap superuser,
I think it makes sense to disallow removing its superuser attribute (at
least via ALTER ROLE NOSUPERUSER).  It seems like there is much work to do
before a no-superuser configuration could be formally supported.  If/when
such support materializes, it might be possible to remove the restriction
that Robert is proposing.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

Re: let's disallow ALTER ROLE bootstrap_superuser NOSUPERUSER

Joe Conway <mail@joeconway.com> — 2022-07-22T17:21:54Z

On 7/21/22 12:46, Tom Lane wrote:
> "David G. Johnston" <david.g.johnston@gmail.com> writes:
>> On Thu, Jul 21, 2022 at 9:28 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> True, but what if the idea is to have *no* superusers?  I seem
>>> to recall people being interested in setups like that.
> 
>> I would expect an initdb option (once this is possible) to specify this
>> desire and we just never set one up in the first place.  It seems
>> impractical to remove one after it already exists.
> 
> There has to be a role that owns the built-in objects.  Robert's point
> is that pretending that that role isn't high-privilege is silly.

My strategy has been to ensure no other roles are members of the 
bootstrap superuser role, and then alter the bootstrap user to be 
NOLOGIN. E.g. in the example here:

https://github.com/pgaudit/set_user/blob/1335cd34ca91b6bd19d5e910cc93c831d1ed0db0/README.md?plain=1#L589

And checked here:

https://github.com/pgaudit/set_user/blob/1335cd34ca91b6bd19d5e910cc93c831d1ed0db0/README.md?plain=1#L612

https://github.com/pgaudit/set_user/blob/1335cd34ca91b6bd19d5e910cc93c831d1ed0db0/README.md?plain=1#L618

-- 
Joe Conway
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

Re: let's disallow ALTER ROLE bootstrap_superuser NOSUPERUSER

Robert Haas <robertmhaas@gmail.com> — 2022-07-22T20:40:48Z

On Fri, Jul 22, 2022 at 1:21 PM Joe Conway <mail@joeconway.com> wrote:
> My strategy has been to ensure no other roles are members of the
> bootstrap superuser role, and then alter the bootstrap user to be
> NOLOGIN. E.g. in the example here:

Yeah, making the bootstrap role NOLOGIN seems more reasonable than
making it NOSUPERUSER, at least to me.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

Re: let's disallow ALTER ROLE bootstrap_superuser NOSUPERUSER

Robert Haas <robertmhaas@gmail.com> — 2022-07-26T18:40:49Z

On Thu, Jul 21, 2022 at 1:27 PM Nathan Bossart <nathandbossart@gmail.com> wrote:
> Given the current assumptions the code makes about the bootstrap superuser,
> I think it makes sense to disallow removing its superuser attribute (at
> least via ALTER ROLE NOSUPERUSER).  It seems like there is much work to do
> before a no-superuser configuration could be formally supported.  If/when
> such support materializes, it might be possible to remove the restriction
> that Robert is proposing.

Reaction to this patch seems tentatively positive so far, so I have
committed it. Maybe someone will still show up to complain ... but I
think it's a good change, so I hope not.

-- 
Robert Haas
EDB: http://www.enterprisedb.com

Re: let's disallow ALTER ROLE bootstrap_superuser NOSUPERUSER

Tom Lane <tgl@sss.pgh.pa.us> — 2022-07-26T18:59:24Z

Robert Haas <robertmhaas@gmail.com> writes:
> Reaction to this patch seems tentatively positive so far, so I have
> committed it. Maybe someone will still show up to complain ... but I
> think it's a good change, so I hope not.

I had not actually read the patch, but now that I have, it's got
a basic typing error:

+       bool    should_be_super = BoolGetDatum(boolVal(dissuper->arg));
+
+       if (!should_be_super && roleid == BOOTSTRAP_SUPERUSERID)
+           ereport(ERROR,

The result of BoolGetDatum is not bool, it's Datum.  This is
probably harmless, but it's still a typing violation.
You want something like

	bool    should_be_super = boolVal(dissuper->arg);
	...
	new_record[Anum_pg_authid_rolsuper - 1] = BoolGetDatum(should_be_super);

			regards, tom lane

Re: let's disallow ALTER ROLE bootstrap_superuser NOSUPERUSER

Robert Haas <robertmhaas@gmail.com> — 2022-07-26T19:06:08Z

On Tue, Jul 26, 2022 at 2:59 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> I had not actually read the patch, but now that I have, it's got
> a basic typing error:
>
> +       bool    should_be_super = BoolGetDatum(boolVal(dissuper->arg));
> +
> +       if (!should_be_super && roleid == BOOTSTRAP_SUPERUSERID)
> +           ereport(ERROR,
>
> The result of BoolGetDatum is not bool, it's Datum.  This is
> probably harmless, but it's still a typing violation.
> You want something like
>
>         bool    should_be_super = boolVal(dissuper->arg);
>         ...
>         new_record[Anum_pg_authid_rolsuper - 1] = BoolGetDatum(should_be_super);

Oops. Will fix.

-- 
Robert Haas
EDB: http://www.enterprisedb.com