Re: Recognizing superuser in pg_hba.conf

[Vik Fearing]

On 7/2/20 3:14 PM, Daniel Gustafsson wrote:
>> On 30 Mar 2020, at 20:28, Tom Lane  wrote:
>>
>> Tomas Vondra  writes:
>>> I see this patch is marked as RFC since 12/30, but there seems to be
>>> quite a lot of discussion about the syntax, keywords and how exactly to
>>> identify the superuser. So I'll switch it back to needs review, which I
>>> think is a better representation of the current state.
>>
>> Somebody switched it to RFC again, despite the facts that
>>
>> (a) there is absolutely no consensus about what syntax to use
>> (and some of the proposals imply very different patches),
>>
>> (b) there's been no discussion at all since the last CF, and
>>
>> (c) the patch is still failing in the cfbot (src/test/ssl fails).
>>
>> While resolving (c) would seem to be the author's problem, I don't
>> think it's worth putting effort into that detail until we have
>> some meeting of the minds about (a).  So I'll put this back to
>> "needs review".
> 
> Since there hasn't been any more progress on this since the last CF, and the
> fact that the outcome may result in a completely different patch, I'm inclined
> to mark this returned with feedback rather than have it linger.  The 
> discussion
> can continue and the entry be re-opened.
> 
> Thoughts?


No objection.
-- 
Vik Fearing

Re: Recognizing superuser in pg_hba.conf

[Daniel Gustafsson]

> On 30 Mar 2020, at 20:28, Tom Lane  wrote:
> 
> Tomas Vondra  writes:
>> I see this patch is marked as RFC since 12/30, but there seems to be
>> quite a lot of discussion about the syntax, keywords and how exactly to
>> identify the superuser. So I'll switch it back to needs review, which I
>> think is a better representation of the current state.
> 
> Somebody switched it to RFC again, despite the facts that
> 
> (a) there is absolutely no consensus about what syntax to use
> (and some of the proposals imply very different patches),
> 
> (b) there's been no discussion at all since the last CF, and
> 
> (c) the patch is still failing in the cfbot (src/test/ssl fails).
> 
> While resolving (c) would seem to be the author's problem, I don't
> think it's worth putting effort into that detail until we have
> some meeting of the minds about (a).  So I'll put this back to
> "needs review".

Since there hasn't been any more progress on this since the last CF, and the
fact that the outcome may result in a completely different patch, I'm inclined
to mark this returned with feedback rather than have it linger.  The discussion
can continue and the entry be re-opened.

Thoughts?

cheers ./daniel

Re: Recognizing superuser in pg_hba.conf

[Tom Lane]

Tomas Vondra  writes:
> I see this patch is marked as RFC since 12/30, but there seems to be
> quite a lot of discussion about the syntax, keywords and how exactly to
> identify the superuser. So I'll switch it back to needs review, which I
> think is a better representation of the current state.

Somebody switched it to RFC again, despite the facts that

(a) there is absolutely no consensus about what syntax to use
(and some of the proposals imply very different patches),

(b) there's been no discussion at all since the last CF, and

(c) the patch is still failing in the cfbot (src/test/ssl fails).

While resolving (c) would seem to be the author's problem, I don't
think it's worth putting effort into that detail until we have
some meeting of the minds about (a).  So I'll put this back to
"needs review".

regards, tom lane

Re: Recognizing superuser in pg_hba.conf

[Tomas Vondra]

Hi,

I see this patch is marked as RFC since 12/30, but there seems to be
quite a lot of discussion about the syntax, keywords and how exactly to
identify the superuser. So I'll switch it back to needs review, which I
think is a better representation of the current state.

regards

--
Tomas Vondra  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services 

Re: Recognizing superuser in pg_hba.conf

[Stephen Frost]

Greetings,

* Robert Haas (robertmh...@gmail.com) wrote:
> But, again, we already *have* a way of solving this problem: use
> quotes. As Simon pointed out, your proposed solution isn't really a
> solution at all, because & can appear in role names. It probably
> won't, but there probably also won't be a role name that matches
> either of these keywords, so it's just six of one, half a dozen of the
> other. The thing that really solves it is quoting.

I really just can't agree with the idea that:

"&superuser"

and

&superuser

in pg_hba.conf should mean materially different things and have far
reaching security differences.  Depending on quoting in pg_hba.conf for
this distinction is an altogether bad idea.

> Now I admit that if we decide pg_hba.conf keywords have to start with
> "pg_" and prevent names beginning with "pg_" from being used as object
> names, then we'd have TWO ways of distinguishing between a keyword and
> an object name. But I don't think TMTOWTDI is the right design
> principle here.

There is a *really* big difference here though which makes this not "two
ways to do the same thing"- you *can't* create a user starting with
"pg_".  You *can* create a user with an '&' in it.  If we prevented you
from being able to create users with '&' in it then I'd be more open to
the idea of using '&' to mean something special in pg_hba, and then it
really would be two different ways to do the same thing, but that's not
actually what's being proposed here.

Thanks,

Stephen


signature.asc
Description: PGP signature

Re: Recognizing superuser in pg_hba.conf

[Tom Lane]

Robert Haas  writes:
> On Thu, Jan 9, 2020 at 11:06 AM Tom Lane  wrote:
>> The problem is that we keep deciding that okay, it probably won't hurt
>> anybody if this particular thing-that-ought-to-be-a-reserved-word isn't
>> really reserved.

> But, again, we already *have* a way of solving this problem: use
> quotes. As Simon pointed out, your proposed solution isn't really a
> solution at all, because & can appear in role names.

I'm not sure that the pg_hba.conf parser allows that without quotes,
but in any case it's irrelevant to the proposal to use a pg_ prefix.
We don't allow non-built-in role names to be spelled that way,
quoted or not.

> Now I admit that if we decide pg_hba.conf keywords have to start with
> "pg_" and prevent names beginning with "pg_" from being used as object
> names, then we'd have TWO ways of distinguishing between a keyword and
> an object name. But I don't think TMTOWTDI is the right design
> principle here.

The principle I'm concerned about is not letting a configuration file
that was perfectly fine in version N silently become a security hazard
in version N+1.  The only way I will accept your proposal is if we
change the pg_hba.conf parser to *require* quotes around every
role and DB name that's not meant to be a keyword, so that people get
used to that requirement.  But I doubt that idea will fly.

regards, tom lane

Re: Recognizing superuser in pg_hba.conf

[Robert Haas]

On Thu, Jan 9, 2020 at 11:06 AM Tom Lane  wrote:
> The problem is that we keep deciding that okay, it probably won't hurt
> anybody if this particular thing-that-ought-to-be-a-reserved-word isn't
> really reserved.  Your exercise in justifying that for "superuser" is
> not unlike every other previous argument about this.  Sooner or later
> that's going to fail, and somebody's going to have a security problem
> because they didn't know that a particular name has magic properties
> in a particular context.  (Which, indeed, maybe it didn't have when
> they chose it.)  Claiming they should have known better isn't where
> I want to be when that happens.

But, again, we already *have* a way of solving this problem: use
quotes. As Simon pointed out, your proposed solution isn't really a
solution at all, because & can appear in role names. It probably
won't, but there probably also won't be a role name that matches
either of these keywords, so it's just six of one, half a dozen of the
other. The thing that really solves it is quoting.

Now I admit that if we decide pg_hba.conf keywords have to start with
"pg_" and prevent names beginning with "pg_" from being used as object
names, then we'd have TWO ways of distinguishing between a keyword and
an object name. But I don't think TMTOWTDI is the right design
principle here.

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

Re: Recognizing superuser in pg_hba.conf

[Tom Lane]

Robert Haas  writes:
> On Thu, Jan 9, 2020 at 10:06 AM Tom Lane  wrote:
>> What I'm basically objecting to is the pseudo-reservedness.  If we
>> don't want to dodge that with special syntax, we should dodge it
>> by making sure the keywords are actually reserved names.

> ...
> But I think I'm coming around to the view that we're making what ought
> to be a simple change complicated, and we should just not do that.

The problem is that we keep deciding that okay, it probably won't hurt
anybody if this particular thing-that-ought-to-be-a-reserved-word isn't
really reserved.  Your exercise in justifying that for "superuser" is
not unlike every other previous argument about this.  Sooner or later
that's going to fail, and somebody's going to have a security problem
because they didn't know that a particular name has magic properties
in a particular context.  (Which, indeed, maybe it didn't have when
they chose it.)  Claiming they should have known better isn't where
I want to be when that happens.

I don't want to keep going down that path.  These things are effectively
reserved words, and they need to act like reserved words, so that you
get an error if you misuse them.  Silently doing something else than
what (one reasonable reading of) a pg_hba.conf entry seems to imply
is *bad*.

regards, tom lane

Re: Recognizing superuser in pg_hba.conf

[Robert Haas]

On Thu, Jan 9, 2020 at 10:06 AM Tom Lane  wrote:
> What I'm basically objecting to is the pseudo-reservedness.  If we
> don't want to dodge that with special syntax, we should dodge it
> by making sure the keywords are actually reserved names.

You know, as I was reading this email, I got to thinking: aren't we
engineering a solution to a problem for which we already have a
solution?

The documentation says:

"Quoting one of the keywords in a database, user, or address field
(e.g., all or replication) makes the word lose its special character,
and just match a database, user, or host with that name."

So if you've writing a pg_hba.conf file that contains a specific role
name, and you want to make sure it doesn't get confused with a current
or future keyword, just quote it. If you don't quote it, make sure to
RTFM at the time and when upgrading.

If you want to argue that this isn't the cleanest possible solution to
the problem, I think I would agree. If we were doing this over again,
we could probably design a better syntax for pg_hba.conf, perhaps one
where all specific role names have to be quoted and anything that's
not quoted is expected to be a keyword. But, as it is, nothing's
really broken here, and practical confusion is unlikely. If someone
has a role named "superuser", then it's probably a superuser account,
and the worst that will happen is that we'll match all superuser
accounts rather than only that one. If someone has a non-superuser
account called "superuser", or if they have an account named
"nonsuperuser," then, uh, that's lame, and if this patch causes them
to improve their choice of role names, that's good. If it causes them
to use quotes, that's also good.

But I think I'm coming around to the view that we're making what ought
to be a simple change complicated, and we should just not do that.

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

Re: Recognizing superuser in pg_hba.conf

[Stephen Frost]

Greetings,

* Tom Lane (t...@sss.pgh.pa.us) wrote:
> What I'm basically objecting to is the pseudo-reservedness.  If we
> don't want to dodge that with special syntax, we should dodge it
> by making sure the keywords are actually reserved names.  In other
> words, add a "pg_" prefix, as somebody else suggested upthread.

Yes, that was my suggestion, and it was also my change a few major
versions ago that actually reserved the "pg_" prefix for roles.

> BTW, although that solution works for the immediate need of
> keywords that have to be distinguished from role names, it doesn't
> currently scale to keywords for the database column, because we
> don't treat "pg_" as a reserved prefix for database names:
> 
> regression=# create role pg_zit;
> ERROR:  role name "pg_zit" is reserved
> DETAIL:  Role names starting with "pg_" are reserved.
> regression=# create database pg_zit;
> CREATE DATABASE
> 
> Should we do so, or wait till there's an immediate need to?

I seem to recall (but it was years ago, so I might be wrong) advocating
that we should reserve the 'pg_' prefix for *all* object types.  All I
can recall is that there wasn't much backing for the idea (though I also
don't recall any specific objection, and it's also quite possible that
there was simply no response to the idea).

For my 2c, I'd *much* rather we reserve it across the board, and sooner
than later, since that would hopefully reduce the impact on people.  The
only justification for *not* reserving it is if we KNOW that we'll never
need a special one of those, but, well, we're well past that for
database names already- look at the fact that we've got a "replication"
one, for example.  Maybe we can't ever un-reserve that, but I like the
idea of reserving "pg_" for database names and then having
"pg_replication" be allowed to mean replication connections and then
encouraging users to use that instead.

Thanks,

Stephen


signature.asc
Description: PGP signature

Re: Recognizing superuser in pg_hba.conf

[Tom Lane]

Simon Riggs  writes:
> On Wed, 8 Jan 2020 at 23:55, Vik Fearing 
> wrote:
>> What is being proposed is what is in the Subject and the original
>> patch.  The other patch is because Tom didn't like "the continuing creep
>> of pseudo-reserved database and user names" so I wrote a patch to mark
>> such reserved names and rebased my original patch on top of it.  Only
>> the docs changed in the rebase.  The original patch (or its rebase) is
>> what I am interested in.

> Hopefully there will be no danger of me gaining access if I have a crafted
> rolename?
> postgres=# create role "&backdoor";
> CREATE ROLE

Well, the existence of such a role name wouldn't by itself cause any
change in the way that pg_hba.conf is parsed.  If you could then
persuade a superuser to insert a pg_hba.conf line that is trying
to match your username, the line might do something else than what the
superuser expected, which is bad.  But the *exact* same hazard applies
to proposals based on inventing pseudo-reserved keywords (by which
I mean things that look like names, but aren't reserved words, so that
somebody could create a role name matching them).  Either way, an
uninformed superuser could be tricked.

What I'm basically objecting to is the pseudo-reservedness.  If we
don't want to dodge that with special syntax, we should dodge it
by making sure the keywords are actually reserved names.  In other
words, add a "pg_" prefix, as somebody else suggested upthread.
I don't personally find that prettier than a punctuation prefix,
but I can live with it if other people do.

BTW, although that solution works for the immediate need of
keywords that have to be distinguished from role names, it doesn't
currently scale to keywords for the database column, because we
don't treat "pg_" as a reserved prefix for database names:

regression=# create role pg_zit;
ERROR:  role name "pg_zit" is reserved
DETAIL:  Role names starting with "pg_" are reserved.
regression=# create database pg_zit;
CREATE DATABASE

Should we do so, or wait till there's an immediate need to?

regards, tom lane

Re: Recognizing superuser in pg_hba.conf

[Simon Riggs]

On Wed, 8 Jan 2020 at 23:55, Vik Fearing 
wrote:

> On 08/01/2020 23:13, Peter Eisentraut wrote:
> > On 2020-01-06 17:03, Tom Lane wrote:
> >> So it's not clear to me whether we have any meeting of the minds
> >> on wanting this patch.
> >
> > This fairly far-ranging syntax reorganization of pg_hba.conf doesn't
> > appeal to me.  pg_hba.conf is complicated enough conceptually for
> > users, but AFAICT nobody ever complained about the syntax or the
> > lexical structure specifically.  Assigning meaning to randomly chosen
> > special characters, moreover in a security-relevant file, seems like
> > the wrong way to go.
> >
> > Moreover, this thread has morphed from what it says in the subject
> > line to changing the syntax of pg_hba.conf in a somewhat fundamental
> > way.  So at the very least someone should post a comprehensive summary
> > of what is being proposed, instead of just attaching patches that
> > implement whatever was discussed across the thread.
> >
>
> What is being proposed is what is in the Subject and the original
> patch.  The other patch is because Tom didn't like "the continuing creep
> of pseudo-reserved database and user names" so I wrote a patch to mark
> such reserved names and rebased my original patch on top of it.  Only
> the docs changed in the rebase.  The original patch (or its rebase) is
> what I am interested in.
>

Hopefully there will be no danger of me gaining access if I have a crafted
rolename?

postgres=# create role "&backdoor";
CREATE ROLE

-- 
Simon Riggshttp://www.2ndQuadrant.com/

PostgreSQL Solutions for the Enterprise

Re: Recognizing superuser in pg_hba.conf

[Vik Fearing]

On 08/01/2020 23:13, Peter Eisentraut wrote:
> On 2020-01-06 17:03, Tom Lane wrote:
>> So it's not clear to me whether we have any meeting of the minds
>> on wanting this patch.
>
> This fairly far-ranging syntax reorganization of pg_hba.conf doesn't
> appeal to me.  pg_hba.conf is complicated enough conceptually for
> users, but AFAICT nobody ever complained about the syntax or the
> lexical structure specifically.  Assigning meaning to randomly chosen
> special characters, moreover in a security-relevant file, seems like
> the wrong way to go.
>
> Moreover, this thread has morphed from what it says in the subject
> line to changing the syntax of pg_hba.conf in a somewhat fundamental
> way.  So at the very least someone should post a comprehensive summary
> of what is being proposed, instead of just attaching patches that
> implement whatever was discussed across the thread.
>

What is being proposed is what is in the Subject and the original
patch.  The other patch is because Tom didn't like "the continuing creep
of pseudo-reserved database and user names" so I wrote a patch to mark
such reserved names and rebased my original patch on top of it.  Only
the docs changed in the rebase.  The original patch (or its rebase) is
what I am interested in.

-- 

Vik Fearing

Re: Recognizing superuser in pg_hba.conf

[Peter Eisentraut]

On 2020-01-06 17:03, Tom Lane wrote:

So it's not clear to me whether we have any meeting of the minds
on wanting this patch.


This fairly far-ranging syntax reorganization of pg_hba.conf doesn't 
appeal to me.  pg_hba.conf is complicated enough conceptually for users, 
but AFAICT nobody ever complained about the syntax or the lexical 
structure specifically.  Assigning meaning to randomly chosen special 
characters, moreover in a security-relevant file, seems like the wrong 
way to go.


Moreover, this thread has morphed from what it says in the subject line 
to changing the syntax of pg_hba.conf in a somewhat fundamental way.  So 
at the very least someone should post a comprehensive summary of what is 
being proposed, instead of just attaching patches that implement 
whatever was discussed across the thread.


--
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: Recognizing superuser in pg_hba.conf

[Tom Lane]

Vik Fearing  writes:
> On 06/01/2020 17:03, Tom Lane wrote:
>> 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?

> I have no idea.  I cannot reproduce the failure locally.

Hm, it blows up pretty thoroughly for me too, on a RHEL6 box.
Are you sure you're running that test -- check-world doesn't do it?

At least in the 001_ssltests test, the failures seem to all look
like this in the TAP test's log file:

psql: error: could not connect to server: could not initiate GSSAPI security 
context: Unspecified GSS failure.  Minor code may provide more information
could not initiate GSSAPI security context: Credentials cache file 
'/tmp/krb5cc_502' not found

There are no matching entries in the postmaster log file, so this
seems to be strictly a client-side failure.

(Are we *really* putting security credentials in /tmp ???)

regards, tom lane

Re: Recognizing superuser in pg_hba.conf

[Vik Fearing]

On 06/01/2020 17:03, Tom Lane wrote:
> 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?


I have no idea.  I cannot reproduce the failure locally.

-- 

Vik Fearing

Re: Recognizing superuser in pg_hba.conf

[Tom Lane]

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

Re: Recognizing superuser in pg_hba.conf

[Stephen Frost]

Greetings,

* Tom Lane (t...@sss.pgh.pa.us) wrote:
> Stephen Frost  writes:
> > On Thu, Jan 2, 2020 at 15:50 Tom Lane  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


signature.asc
Description: PGP signature

Re: Recognizing superuser in pg_hba.conf

[Tom Lane]

Stephen Frost  writes:
> On Thu, Jan 2, 2020 at 15:50 Tom Lane  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

Re: Recognizing superuser in pg_hba.conf

[Stephen Frost]

Greetings,

On Thu, Jan 2, 2020 at 15:50 Tom Lane  wrote:

> Andrew Gierth  writes:
> > "Tom" == Tom Lane  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

>

Re: Recognizing superuser in pg_hba.conf

[Tom Lane]

Andrew Gierth  writes:
> "Tom" == Tom Lane  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

Re: Recognizing superuser in pg_hba.conf

[Christoph Moench-Tegeder]

## Stephen Frost (sfr...@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

Re: Recognizing superuser in pg_hba.conf

[Stephen Frost]

Greetings,

On Thu, Jan 2, 2020 at 15:04 Tom Lane  wrote:

> Stephen Frost  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

>

Re: Recognizing superuser in pg_hba.conf

[Andrew Gierth]

> "Tom" == Tom Lane  writes:

 > Stephen Frost  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)

Re: Recognizing superuser in pg_hba.conf

[Vik Fearing]

On 02/01/2020 20:52, Stephen Frost wrote:
> Greetings,
>
> * Vik Fearing (vik.fear...@2ndquadrant.com) wrote:
>> On 29/12/2019 23:10, Vik Fearing wrote:
>>> On 29/12/2019 17:31, Tom Lane wrote:
 Robert Haas  writes:
> On Sat, Dec 28, 2019 at 2:02 PM Vik Fearing  
> 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

Re: Recognizing superuser in pg_hba.conf

[Tom Lane]

Stephen Frost  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

Re: Recognizing superuser in pg_hba.conf

[Stephen Frost]

Greetings,

* Vik Fearing (vik.fear...@2ndquadrant.com) wrote:
> On 29/12/2019 23:10, Vik Fearing wrote:
> > On 29/12/2019 17:31, Tom Lane wrote:
> >> Robert Haas  writes:
> >>> On Sat, Dec 28, 2019 at 2:02 PM Vik Fearing  
> >>> 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


signature.asc
Description: PGP signature

Re: Recognizing superuser in pg_hba.conf

[David Fetter]

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  writes:
> >>> On Sat, Dec 28, 2019 at 2:02 PM Vik Fearing  
> >>> 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  http://fetter.org/
Phone: +1 415 235 3778

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

Re: Recognizing superuser in pg_hba.conf

[Vik Fearing]

On 29/12/2019 23:10, Vik Fearing wrote:
> On 29/12/2019 17:31, Tom Lane wrote:
>> Robert Haas  writes:
>>> On Sat, Dec 28, 2019 at 2:02 PM Vik Fearing  
>>> 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

diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 55694c4368..add25b2b43 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -8953,8 +8953,8 @@ SCRAM-SHA-256$:&l
  text
  
   Host name or IP address, or one
-  of all, samehost,
-  or samenet, or null for local connections
+  of &all, &samehost,
+  or &samenet, or null for local connections
  
 
 
diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml
index 5f1eec78fb..96fc4b01e9 100644
--- a/doc/src/sgml/client-auth.sgml
+++ b/doc/src/sgml/client-auth.sgml
@@ -81,11 +81,26 @@
A record is made
up of a number of fields which are separated by spaces and/or tabs.
Fields can contain white space if the field value is double-quoted.
-   Quoting one of the keywords in a database, user, or address field (e.g.,
-   all or replication) makes the word lose its special
-   meaning, and just match a database, user, or host with that name.
   
 
+  
+   
+As of version 13, keywords are preceded by the character "&".
+For compatibility, the following legacy keywords are still accepted
+on their own:
+all, replication,
+samegroup, samehost,
+samenet, samerole,
+sameuser.
+   
+
+   
+Quoting one of these legacy keywords in a database, user, or address
+field makes the word lose its special meaning, and just match a
+database, user, or host with that name.
+   
+  
+
   
Each record specifies a connection type, a client IP address range
(if relevant for the connection type), a database name, a user name,
@@ -221,18 +236,18 @@ hostnogssenc database  user
   
Specifies which database name(s) this record matches.  The value
-   all specifies that it matches all databases.
-   The value sameuser specifies that the record
+   &all specifies that it matches all databases.
+   The value &sameuser specifies that the record
matches if the requested database has the same name as the
-   requested user.  The value samerole specifies that
+   requested user.  The value &samerole specifies that
the requested user must be a member of the role with the same
-   name as the requested database.  (samegroup is an
-   obsolete but still accepted spelling of samerole.)
+   name as the requested database.  (&samegroup is an
+   obsolete but still accepted spelling of &samerole.)
Superusers are not considered to be members of a role for the
-   purposes of samerole unless they are explicitly
+   purposes of &samerole unless they are explicitly
members of the role, directly or indirectly, and not just by
virtue of being a superuser.
-   The value replication specifies that the record
+   The value &replication specifies that the record
matches if a physical replication connection is requested (note that
replication connections do not specify any particular database).
Otherwise, this is the name of
@@ -249,7 +264,7 @@ hostnogssenc database  user
   
Specifies which database user name(s) this record
-   matches. The value all specifies that it
+   matches. The value &all specifies that it
matches all users.  Otherwise, this is either the name of a specific
database user, or a group name preceded by +.
(Recall that there is no real distinction between users and groups
@@ -312,9 +327,9 @@ hostnogssenc database  user
 
   
-   You can also write all to match any IP address,
-   samehost to match any of the server's own IP
-   addresses, or samenet to match any address in any
+   You can also write &all to match any IP address,
+   &samehost to match any of the server's own IP
+   addresses, or &

Re: Recognizing superuser in pg_hba.conf

[Vik Fearing]

On 29/12/2019 17:31, Tom Lane wrote:
> Robert Haas  writes:
>> On Sat, Dec 28, 2019 at 2:02 PM Vik Fearing  
>> 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

diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 55694c4368..add25b2b43 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -8953,8 +8953,8 @@ SCRAM-SHA-256$:&l
  text
  
   Host name or IP address, or one
-  of all, samehost,
-  or samenet, or null for local connections
+  of &all, &samehost,
+  or &samenet, or null for local connections
  
 
 
diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml
index 5f1eec78fb..96fc4b01e9 100644
--- a/doc/src/sgml/client-auth.sgml
+++ b/doc/src/sgml/client-auth.sgml
@@ -81,11 +81,26 @@
A record is made
up of a number of fields which are separated by spaces and/or tabs.
Fields can contain white space if the field value is double-quoted.
-   Quoting one of the keywords in a database, user, or address field (e.g.,
-   all or replication) makes the word lose its special
-   meaning, and just match a database, user, or host with that name.
   
 
+  
+   
+As of version 13, keywords are preceded by the character "&".
+For compatibility, the following legacy keywords are still accepted
+on their own:
+all, replication,
+samegroup, samehost,
+samenet, samerole,
+sameuser.
+   
+
+   
+Quoting one of these legacy keywords in a database, user, or address
+field makes the word lose its special meaning, and just match a
+database, user, or host with that name.
+   
+  
+
   
Each record specifies a connection type, a client IP address range
(if relevant for the connection type), a database name, a user name,
@@ -221,18 +236,18 @@ hostnogssenc database  user
   
Specifies which database name(s) this record matches.  The value
-   all specifies that it matches all databases.
-   The value sameuser specifies that the record
+   &all specifies that it matches all databases.
+   The value &sameuser specifies that the record
matches if the requested database has the same name as the
-   requested user.  The value samerole specifies that
+   requested user.  The value &samerole specifies that
the requested user must be a member of the role with the same
-   name as the requested database.  (samegroup is an
-   obsolete but still accepted spelling of samerole.)
+   name as the requested database.  (&samegroup is an
+   obsolete but still accepted spelling of &samerole.)
Superusers are not considered to be members of a role for the
-   purposes of samerole unless they are explicitly
+   purposes of &samerole unless they are explicitly
members of the role, directly or indirectly, and not just by
virtue of being a superuser.
-   The value replication specifies that the record
+   The value &replication specifies that the record
matches if a physical replication connection is requested (note that
replication connections do not specify any particular database).
Otherwise, this is the name of
@@ -249,7 +264,7 @@ hostnogssenc database  user
   
Specifies which database user name(s) this record
-   matches. The value all specifies that it
+   matches. The value &all specifies that it
matches all users.  Otherwise, this is either the name of a specific
database user, or a group name preceded by +.
(Recall that there is no real distinction between users and groups
@@ -312,9 +327,9 @@ hostnogssenc database  user
 
   
-   You can also write all to match any IP address,
-   samehost to match any of the server's own IP
-   addresses, or samenet to match any address in any
+   You can also write &all to match any IP address,
+   &samehost to match any of the server's own IP
+   addresses, or &samenet to match any address in any
subnet that the server is directly connected to.
   
 
@@ -699,40 +714,40 @@ hostnogssenc database  user

  
diff

Re: Recognizing superuser in pg_hba.conf

[Robert Haas]

On Sun, Dec 29, 2019 at 11:31 AM Tom Lane  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

Re: Recognizing superuser in pg_hba.conf

[Tom Lane]

Robert Haas  writes:
> On Sat, Dec 28, 2019 at 2:02 PM Vik Fearing  
> 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

Re: Recognizing superuser in pg_hba.conf

[Robert Haas]

On Sat, Dec 28, 2019 at 2:02 PM Vik Fearing  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

Re: Recognizing superuser in pg_hba.conf

[Vik Fearing]

On 28/12/2019 19:07, Tom Lane wrote:
> Vik Fearing  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

Re: Recognizing superuser in pg_hba.conf

[Tom Lane]

Vik Fearing  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