Thread: Recognizing superuser in pg_hba.conf
It can sometimes be useful to match against a superuser in pg_hba.conf. For example, one could imagine wanting to reject nonsuperuser from a particular database. This used to be possible by creating an empty role and matching against that, but that functionality was removed (a long time ago) by commit 94cd0f1ad8a. 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. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Attachment
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
Attachment
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
Attachment
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
Attachment
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
Attachment
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
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
Vik Fearing <vik.fearing@2ndquadrant.com> 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
codemay 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
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
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
On Wed, 8 Jan 2020 at 23:55, Vik Fearing <vik.fearing@2ndquadrant.com> 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 Riggs <simon@2ndquadrant.com> writes:
> On Wed, 8 Jan 2020 at 23:55, Vik Fearing <vik.fearing@2ndquadrant.com>
> 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
Greetings, * Tom Lane (tgl@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
Attachment
On Thu, Jan 9, 2020 at 10:06 AM Tom Lane <tgl@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. 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
Robert Haas <robertmhaas@gmail.com> writes:
> On Thu, Jan 9, 2020 at 10:06 AM Tom Lane <tgl@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.
> ...
> 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
On Thu, Jan 9, 2020 at 11:06 AM Tom Lane <tgl@sss.pgh.pa.us> 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
Robert Haas <robertmhaas@gmail.com> writes:
> On Thu, Jan 9, 2020 at 11:06 AM Tom Lane <tgl@sss.pgh.pa.us> 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
Greetings, * Robert Haas (robertmhaas@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
Attachment
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
Tomas Vondra <tomas.vondra@2ndquadrant.com> 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
> On 30 Mar 2020, at 20:28, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > Tomas Vondra <tomas.vondra@2ndquadrant.com> 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
On 7/2/20 3:14 PM, Daniel Gustafsson wrote: >> On 30 Mar 2020, at 20:28, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> >> Tomas Vondra <tomas.vondra@2ndquadrant.com> 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