Recognizing superuser in pg_hba.conf

Vik Fearing <vik.fearing@2ndquadrant.com> writes:

It can sometimes be useful to match against a superuser in pg_hba.conf.

Seems like a reasonable desire.

Adding another keyword can break backwards compatibility, of course. So
that is an issue that needs to be discussed, but I don't imagine too
many people are using role names "superuser" and "nonsuperuser". Those
who are will have to quote them.

I'm not very happy about the continuing creep of pseudo-reserved database
and user names in pg_hba.conf. I wish we'd adjust the notation so that
these keywords are syntactically distinct from ordinary names. Given
the precedent that "+" and "@" prefixes change what an identifier means,
maybe we could use "*" or some other punctuation character as a keyword
prefix? We'd have to give grandfather exceptions to the existing
keywords, at least for a while, but we could say that new ones won't be
recognized without the prefix.

regards, tom lane

On 28/12/2019 19:07, Tom Lane wrote:

Vik Fearing <vik.fearing@2ndquadrant.com> writes:

It can sometimes be useful to match against a superuser in pg_hba.conf.

Seems like a reasonable desire.

Adding another keyword can break backwards compatibility, of course. So
that is an issue that needs to be discussed, but I don't imagine too
many people are using role names "superuser" and "nonsuperuser". Those
who are will have to quote them.

I'm not very happy about the continuing creep of pseudo-reserved database
and user names in pg_hba.conf. I wish we'd adjust the notation so that
these keywords are syntactically distinct from ordinary names. Given
the precedent that "+" and "@" prefixes change what an identifier means,
maybe we could use "*" or some other punctuation character as a keyword
prefix? We'd have to give grandfather exceptions to the existing
keywords, at least for a while, but we could say that new ones won't be
recognized without the prefix.

I'm all for this (and even suggested it during the IRC conversation that
prompted this patch). It's rife with bikeshedding, though.  My original
proposal was to use '&' and Andrew Gierth would have used ':'.

I will submit two patches, one that recognizes the sigil for all the
other keywords, and then an update of this patch.

--

Vik

On Sat, Dec 28, 2019 at 2:02 PM Vik Fearing <vik.fearing@2ndquadrant.com> wrote:

these keywords are syntactically distinct from ordinary names. Given
the precedent that "+" and "@" prefixes change what an identifier means,
maybe we could use "*" or some other punctuation character as a keyword
prefix? We'd have to give grandfather exceptions to the existing
keywords, at least for a while, but we could say that new ones won't be
recognized without the prefix.

I'm all for this (and even suggested it during the IRC conversation that
prompted this patch). It's rife with bikeshedding, though. My original
proposal was to use '&' and Andrew Gierth would have used ':'.

I think this is a good proposal regardless of which character we
decide to use. My order of preference from highest-to-lowest would
probably be :*&, but maybe that's just because I'm reading this on
Sunday rather than on Tuesday.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Robert Haas <robertmhaas@gmail.com> writes:

On Sat, Dec 28, 2019 at 2:02 PM Vik Fearing <vik.fearing@2ndquadrant.com> wrote:

I'm all for this (and even suggested it during the IRC conversation that
prompted this patch). It's rife with bikeshedding, though. My original
proposal was to use '&' and Andrew Gierth would have used ':'.

I think this is a good proposal regardless of which character we
decide to use. My order of preference from highest-to-lowest would
probably be :*&, but maybe that's just because I'm reading this on
Sunday rather than on Tuesday.

I don't have any particular objection to '&' if people prefer that.
But ':' seems like it would introduce confusion with the
variable-substitution notation used in psql and some other places.

It's not that hard to imagine that somebody might want a
variable-substitution notation in pg_hba.conf someday, so we should
leave syntax room for one, and ':' seems like a likely choice
for it (although I suppose a case could be made for '$' too).

regards, tom lane

On Sun, Dec 29, 2019 at 11:31 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:

I don't have any particular objection to '&' if people prefer that.
But ':' seems like it would introduce confusion with the
variable-substitution notation used in psql and some other places.

It's not that hard to imagine that somebody might want a
variable-substitution notation in pg_hba.conf someday, so we should
leave syntax room for one, and ':' seems like a likely choice
for it (although I suppose a case could be made for '$' too).

Well, as I say, I don't care very much... I hope we can agree on
something and move forward.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 29/12/2019 17:31, Tom Lane wrote:

Robert Haas <robertmhaas@gmail.com> writes:

On Sat, Dec 28, 2019 at 2:02 PM Vik Fearing <vik.fearing@2ndquadrant.com> wrote:

I'm all for this (and even suggested it during the IRC conversation that
prompted this patch). It's rife with bikeshedding, though. My original
proposal was to use '&' and Andrew Gierth would have used ':'.

I think this is a good proposal regardless of which character we
decide to use. My order of preference from highest-to-lowest would
probably be :*&, but maybe that's just because I'm reading this on
Sunday rather than on Tuesday.

I don't have any particular objection to '&' if people prefer that.

I wrote the patch so I got to decide. :-)  I will also volunteer to do
the grunt work of changing the symbol if consensus wants that, though.

It turns out that my original patch didn't really change, all the meat
is in the keywords patch.  The superuser patch is to be applied on top
of the keywords patch.

--

Vik Fearing

On 29/12/2019 23:10, Vik Fearing wrote:

On 29/12/2019 17:31, Tom Lane wrote:

Robert Haas <robertmhaas@gmail.com> writes:

On Sat, Dec 28, 2019 at 2:02 PM Vik Fearing <vik.fearing@2ndquadrant.com> wrote:

I'm all for this (and even suggested it during the IRC conversation that
prompted this patch). It's rife with bikeshedding, though. My original
proposal was to use '&' and Andrew Gierth would have used ':'.

I think this is a good proposal regardless of which character we
decide to use. My order of preference from highest-to-lowest would
probably be :*&, but maybe that's just because I'm reading this on
Sunday rather than on Tuesday.

I don't have any particular objection to '&' if people prefer that.

I wrote the patch so I got to decide. :-)  I will also volunteer to do
the grunt work of changing the symbol if consensus wants that, though.

It turns out that my original patch didn't really change, all the meat
is in the keywords patch.  The superuser patch is to be applied on top
of the keywords patch.

I missed a few places in the tap tests.  New keywords patch attached,
superuser patch unchanged.

--

Vik Fearing

On Mon, Dec 30, 2019 at 11:56:17AM +0100, Vik Fearing wrote:

On 29/12/2019 23:10, Vik Fearing wrote:

On 29/12/2019 17:31, Tom Lane wrote:

Robert Haas <robertmhaas@gmail.com> writes:

On Sat, Dec 28, 2019 at 2:02 PM Vik Fearing <vik.fearing@2ndquadrant.com> wrote:

I'm all for this (and even suggested it during the IRC conversation that
prompted this patch). It's rife with bikeshedding, though. My original
proposal was to use '&' and Andrew Gierth would have used ':'.

I think this is a good proposal regardless of which character we
decide to use. My order of preference from highest-to-lowest would
probably be :*&, but maybe that's just because I'm reading this on
Sunday rather than on Tuesday.

I don't have any particular objection to '&' if people prefer that.

I wrote the patch so I got to decide. :-)� I will also volunteer to do
the grunt work of changing the symbol if consensus wants that, though.

It turns out that my original patch didn't really change, all the meat
is in the keywords patch.� The superuser patch is to be applied on top
of the keywords patch.

I missed a few places in the tap tests.� New keywords patch attached,
superuser patch unchanged.

--

Vik Fearing

Patches apply cleanly to 0ce38730ac72029f3f2c95ae80b44f5b9060cbcc, and
include documentation. They could use an example of the new
capability, possibly included in the sample pg_hba.conf, e.g.

host &all &superuser 0.0.0.0/0 reject

or similar.

The feature works as described, and is useful. I have thus far been
unable to make it crash.

I haven't used intentionally hostile strings to test it, as I didn't
see those as an important attack surface. This is because by the time
someone hostile can write to pg_hba.conf, they've got all the control
they need to manipulate the entire node, including root exploits.

I've marked this as Ready for Committer.

Best,
David.
--
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

Greetings,

* Vik Fearing (vik.fearing@2ndquadrant.com) wrote:

On 29/12/2019 23:10, Vik Fearing wrote:

On 29/12/2019 17:31, Tom Lane wrote:

Robert Haas <robertmhaas@gmail.com> writes:

On Sat, Dec 28, 2019 at 2:02 PM Vik Fearing <vik.fearing@2ndquadrant.com> wrote:

I'm all for this (and even suggested it during the IRC conversation that
prompted this patch). It's rife with bikeshedding, though. My original
proposal was to use '&' and Andrew Gierth would have used ':'.

I think this is a good proposal regardless of which character we
decide to use. My order of preference from highest-to-lowest would
probably be :*&, but maybe that's just because I'm reading this on
Sunday rather than on Tuesday.

I don't have any particular objection to '&' if people prefer that.

I wrote the patch so I got to decide. :-)  I will also volunteer to do
the grunt work of changing the symbol if consensus wants that, though.

It turns out that my original patch didn't really change, all the meat
is in the keywords patch.  The superuser patch is to be applied on top
of the keywords patch.

I missed a few places in the tap tests.  New keywords patch attached,
superuser patch unchanged.

We already have a reserved namespace when it comes to roles,
specifically "pg_".. why invent something new like this '&' prefix when
we could just declare that 'pg_superusers' is a role to which all
superusers are members? Or something along those lines?

Thanks,

Stephen

On 02/01/2020 20:52, Stephen Frost wrote:

Greetings,

* Vik Fearing (vik.fearing@2ndquadrant.com) wrote:

On 29/12/2019 23:10, Vik Fearing wrote:

On 29/12/2019 17:31, Tom Lane wrote:

Robert Haas <robertmhaas@gmail.com> writes:

On Sat, Dec 28, 2019 at 2:02 PM Vik Fearing <vik.fearing@2ndquadrant.com> wrote:

I'm all for this (and even suggested it during the IRC conversation that
prompted this patch). It's rife with bikeshedding, though. My original
proposal was to use '&' and Andrew Gierth would have used ':'.

I think this is a good proposal regardless of which character we
decide to use. My order of preference from highest-to-lowest would
probably be :*&, but maybe that's just because I'm reading this on
Sunday rather than on Tuesday.

I don't have any particular objection to '&' if people prefer that.

I wrote the patch so I got to decide. :-)� I will also volunteer to do
the grunt work of changing the symbol if consensus wants that, though.

It turns out that my original patch didn't really change, all the meat
is in the keywords patch.� The superuser patch is to be applied on top
of the keywords patch.

I missed a few places in the tap tests.� New keywords patch attached,
superuser patch unchanged.

We already have a reserved namespace when it comes to roles,
specifically "pg_".. why invent something new like this '&' prefix when
we could just declare that 'pg_superusers' is a role to which all
superusers are members? Or something along those lines?

This is an argument against the superusers patch, but surely you are not
suggesting we add a pg_all role that contains all users?� And what about
the keywords that aren't for users?

--

Vik Fearing

Stephen Frost <sfrost@snowman.net> writes:

We already have a reserved namespace when it comes to roles,
specifically "pg_".. why invent something new like this '&' prefix when
we could just declare that 'pg_superusers' is a role to which all
superusers are members? Or something along those lines?

Meh. If the things aren't actually roles, I think this'd just
add confusion. Or were you proposing to implement them as roles?
I'm not sure if that would be practical in every case.

regards, tom lane

"Tom" == Tom Lane <tgl@sss.pgh.pa.us> writes:

Stephen Frost <sfrost@snowman.net> writes:

We already have a reserved namespace when it comes to roles,
specifically "pg_".. why invent something new like this '&' prefix
when we could just declare that 'pg_superusers' is a role to which
all superusers are members? Or something along those lines?

Tom> Meh. If the things aren't actually roles, I think this'd just add
Tom> confusion. Or were you proposing to implement them as roles? I'm
Tom> not sure if that would be practical in every case.

In fact my original suggestion when this idea was discussed on IRC was
to remove the current superuser flag and turn it into a role; but the
issue then is that role membership is inherited and superuserness
currently isn't, so that's a more intrusive change.

--
Andrew (irc:RhodiumToad)

Greetings,

On Thu, Jan 2, 2020 at 15:04 Tom Lane <tgl@sss.pgh.pa.us> wrote:

Stephen Frost <sfrost@snowman.net> writes:

We already have a reserved namespace when it comes to roles,
specifically "pg_".. why invent something new like this '&' prefix when
we could just declare that 'pg_superusers' is a role to which all
superusers are members? Or something along those lines?

Meh. If the things aren't actually roles, I think this'd just
add confusion. Or were you proposing to implement them as roles?
I'm not sure if that would be practical in every case.

Having them as roles might be interesting though I don’t think it would be
required. As for your argument, surely we aren’t going to make
“&superusers” an actual role with this, so you have to accept that’s what
there isn’t a real role either way. I don’t really care for this idea of
making up new syntax that people have to learn, understand, train others
on, etc.

The pg_ prefix makes it clear that it’s a system role... literally by
definition.

As for Vik’s thought about “pg_all”- I hadn’t been thinking we would do
that (“all” is already accepted there anyway and trying to deprecate that
seems unlikely to result in ever actually removing it because that’s the
kind of thing we will argue about and never do..), but it seems like an
interesting idea. Using “public” is maybe another interesting thought there
since that’s the same thing and also reserved...

Thanks,

Stephen

## Stephen Frost (sfrost@snowman.net):

We already have a reserved namespace when it comes to roles,
specifically "pg_".. why invent something new like this '&' prefix when
we could just declare that 'pg_superusers' is a role to which all
superusers are members? Or something along those lines?

Taking this idea one step further (back?): with any non-trivial
number of (user-)roles in the database, DBAs would be well advised
to use group(-role)s for privilege management anyways. It's not
to unreasonable to grant SUPERUSER through a group, too. Although
I'm not sure we'd need a new pg_superuser role here, we're not
inventing a new set of object privileges as in e.g. pg_monitor;
the DBA can just create their own superuser group.
Is there really a need to add more features, or would it be sufficient
to make the applications of group roles more prominent in the docs?
(I've seen way too many cases in which people where granting privileges
to individual users when they should have used groups, so I might
be biased).

Regards,
Christoph

--
Spare Space

Andrew Gierth <andrew@tao11.riddles.org.uk> writes:

"Tom" == Tom Lane <tgl@sss.pgh.pa.us> writes:
Tom> Meh. If the things aren't actually roles, I think this'd just add
Tom> confusion. Or were you proposing to implement them as roles? I'm
Tom> not sure if that would be practical in every case.

In fact my original suggestion when this idea was discussed on IRC was
to remove the current superuser flag and turn it into a role; but the
issue then is that role membership is inherited and superuserness
currently isn't, so that's a more intrusive change.

To cover the proposed functionality, you'd still need some way to
select not-superuser. So I don't think this fully answers the need
even if we wanted to do it.

It's possible that role-ifying everything and then allowing "!role"
in the pg_hba.conf syntax would be enough. Not sure though.

More generally, allowing inheritance of superuser scares me a bit
from a security standpoint. I wouldn't mind turning all the other
legacy role properties into grantable roles, but I *like* the fact
that that one is special.

regards, tom lane

Greetings,

On Thu, Jan 2, 2020 at 15:50 Tom Lane <tgl@sss.pgh.pa.us> wrote:

Andrew Gierth <andrew@tao11.riddles.org.uk> writes:

"Tom" == Tom Lane <tgl@sss.pgh.pa.us> writes:
Tom> Meh. If the things aren't actually roles, I think this'd just add
Tom> confusion. Or were you proposing to implement them as roles? I'm
Tom> not sure if that would be practical in every case.

In fact my original suggestion when this idea was discussed on IRC was
to remove the current superuser flag and turn it into a role; but the
issue then is that role membership is inherited and superuserness
currently isn't, so that's a more intrusive change.

To cover the proposed functionality, you'd still need some way to
select not-superuser. So I don't think this fully answers the need
even if we wanted to do it.

Sorry- why do we need that..? The first match for a pg_hba line wins, so
you can define all the access methods that superuser accounts are allowed
to use first, then a “reject” line for superuser accounts, and then
whatever else you want after that.

More generally, allowing inheritance of superuser scares me a bit

from a security standpoint. I wouldn't mind turning all the other
legacy role properties into grantable roles, but I *like* the fact
that that one is special.

Requiring an extra “set role whatever;” is good to make sure the user
really understands they’re running as superuser, but it doesn’t really
improve actual security at all since there’s no way to require a password
or anything. That superuser-ness isn’t inherited but membership in the
“postgres” or other role-that-owns-everything role is actually strikes me
as less than ideal... the whole allow system table mods thing kinda helps
with that since you need that extra step to actually change most things but
it’s still not great imv. I can’t get too excited about trying to improve
this though since I’d expect material changes to improve security to be
beat back with backwards incompatibility concerns.

Thanks,

Stephen

Stephen Frost <sfrost@snowman.net> writes:

On Thu, Jan 2, 2020 at 15:50 Tom Lane <tgl@sss.pgh.pa.us> wrote:

To cover the proposed functionality, you'd still need some way to
select not-superuser. So I don't think this fully answers the need
even if we wanted to do it.

Sorry- why do we need that..? The first match for a pg_hba line wins, so
you can define all the access methods that superuser accounts are allowed
to use first, then a “reject” line for superuser accounts, and then
whatever else you want after that.

Seems kind of awkward. Or more to the point: you can already do whatever
you want in pg_hba.conf, as long as you're willing to be verbose enough
(and, perhaps, willing to maintain group memberships to fit your needs).
The discussion here, IMO, is about offering useful shorthands.
So a facility like "!role" seems potentially useful. Maybe it's not
really, but I don't think we should reject it just because there's
a verbose and non-obvious way to get the same result.

regards, tom lane

Greetings,

* Tom Lane (tgl@sss.pgh.pa.us) wrote:

Stephen Frost <sfrost@snowman.net> writes:

On Thu, Jan 2, 2020 at 15:50 Tom Lane <tgl@sss.pgh.pa.us> wrote:

To cover the proposed functionality, you'd still need some way to
select not-superuser. So I don't think this fully answers the need
even if we wanted to do it.

Sorry- why do we need that..? The first match for a pg_hba line wins, so
you can define all the access methods that superuser accounts are allowed
to use first, then a “reject” line for superuser accounts, and then
whatever else you want after that.

Seems kind of awkward. Or more to the point: you can already do whatever
you want in pg_hba.conf, as long as you're willing to be verbose enough
(and, perhaps, willing to maintain group memberships to fit your needs).

Sure it's awkward, but it's how people actually deal with these things
today. I'm not against improving on that situation but I also don't
hear tons of complaints about it either, so I do think we should be
thoughtful when it comes to making changes here.

The discussion here, IMO, is about offering useful shorthands.

In general, I'm alright with that idea, but I do want to make sure we're
really being thoughtful when it comes to inventing new syntax that will
only work in this one place and will have to be handled specially by any
tools or anything that wants to generate or look at this.

What are we going to have be displayed through pg_hba_file_rules() for
this '!role' or whatever else, in the 'user_name' column? (Also, ugh,
I find calling that column 'user_name' mildly offensive considering that
function was added well after roles, and it's not like it really meant
'user name' even before then..).

Yes, I'm sure we could just have it be the text '!role' and make
everyone who cares have to parse out that field, in SQL, to figure out
who it really applies to and basically just make everyone deal with it
but I remain skeptical about if it's really a particularly good
approach.

So a facility like "!role" seems potentially useful. Maybe it's not
really, but I don't think we should reject it just because there's
a verbose and non-obvious way to get the same result.

I don't agree that it's "non-obvious" that if you have a config file
where "first match wins" that things which don't match the first line
are, by definition, NOT whatever that first line was and then fall
through to the next, where you could use 'reject' if you want. In fact,
I've always kinda figured that's what 'reject' was for, though I'll
admit that it's been around for far longer than I've been involved in
the project (sadly, I hadn't discovered PG yet back in 1998).

Thanks,

Stephen

So it's not clear to me whether we have any meeting of the minds
on wanting this patch. In the meantime, though, the cfbot
reports that the patch breaks the ssl tests. Why is that?

regards, tom lane