More robust pg_hba.conf parsing/error logging

Started by Rafael Martinezover 16 years ago15 messages
#1Rafael Martinez
r.m.guerrero@usit.uio.no

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hello

The origin of this petition is an error produced today by a user on one
of our systems. Because of this error many users lost access to their
databases.

Problem:
- --------
If you define in pg_hba.conf a database or a user value with 'ALL'
instead of 'all', you will lose access to *all* databases involved. The
reload process will not report anything about 'ALL' been an invalid
value and the new pg_hba.conf will be reloaded.

This is the only thing in the log file:
"LOG: received SIGHUP, reloading configuration files"

Solution:
- ---------
Or change internally all uppercase to lowercase so users can define
values in pg_hba.conf with uppercase characters.

Or throw an error saying 'ALL' is not a valid value and *not* reload the
pg_hba.conf file. This is already done if you use uppercase when you
define connection type or authentication method.

regards,
- --
Rafael Martinez, <r.m.guerrero@usit.uio.no>
Center for Information Technology Services
University of Oslo, Norway

PGP Public Key: http://folk.uio.no/rafael/
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.7 (GNU/Linux)

iD8DBQFKp7GVBhuKQurGihQRAhCZAJ9y5BhdWbrpJeW12g/rJ6yRfgubgACglYC3
wkG1cHESexmSZ48/Fc63vU4=
=a46y
-----END PGP SIGNATURE-----

#2Andrew Dunstan
andrew@dunslane.net
In reply to: Rafael Martinez (#1)
Re: More robust pg_hba.conf parsing/error logging

Rafael Martinez wrote:

Or throw an error saying 'ALL' is not a valid value and *not* reload the
pg_hba.conf file.

But it's not invalid. It would designate a database or user named "ALL".
That might be a silly thing to do, but that's another question.

cheers

andrew

#3Alvaro Herrera
alvherre@commandprompt.com
In reply to: Rafael Martinez (#1)
Re: More robust pg_hba.conf parsing/error logging

Rafael Martinez wrote:

Problem:
- --------
If you define in pg_hba.conf a database or a user value with 'ALL'
instead of 'all', you will lose access to *all* databases involved. The
reload process will not report anything about 'ALL' been an invalid
value and the new pg_hba.conf will be reloaded.

This is the only thing in the log file:
"LOG: received SIGHUP, reloading configuration files"

Aye, that's surprising. I think the correct fix here is to change the
strcmp comparisons to pg_strcasecmp() in several places in hba.c.

(BTW the business about appending newlines to special tokens in
next_token() seems ugly and underdocumented.)

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

#4Alvaro Herrera
alvherre@commandprompt.com
In reply to: Andrew Dunstan (#2)
Re: More robust pg_hba.conf parsing/error logging

Andrew Dunstan wrote:

Rafael Martinez wrote:

Or throw an error saying 'ALL' is not a valid value and *not* reload the
pg_hba.conf file.

But it's not invalid. It would designate a database or user named
"ALL". That might be a silly thing to do, but that's another
question.

Surely if you want to designate a database named ALL you should use
quotes, same as if you wanted to designate a database named all (see my
other followup).

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

#5Andrew Dunstan
andrew@dunslane.net
In reply to: Alvaro Herrera (#4)
Re: More robust pg_hba.conf parsing/error logging

Alvaro Herrera wrote:

Andrew Dunstan wrote:

Rafael Martinez wrote:

Or throw an error saying 'ALL' is not a valid value and *not* reload the
pg_hba.conf file.

But it's not invalid. It would designate a database or user named
"ALL". That might be a silly thing to do, but that's another
question.

Surely if you want to designate a database named ALL you should use
quotes, same as if you wanted to designate a database named all (see my
other followup).

OK, but if we move to using pg_strcasecmp() that would be a behaviour
change, so I think we couldn't do it before 8.5, in case someone is
relying on it. It will affect any dbname or username in mixed or upper
case, not just ALL, won't it?

cheers

andrew

#6Rafael Martinez
r.m.guerrero@usit.uio.no
In reply to: Andrew Dunstan (#2)
Re: More robust pg_hba.conf parsing/error logging

Andrew Dunstan wrote:

Rafael Martinez wrote:

Or throw an error saying 'ALL' is not a valid value and *not* reload the
pg_hba.conf file.

But it's not invalid. It would designate a database or user named "ALL".
That might be a silly thing to do, but that's another question.

Shouldn't 'all' be a reserved word?, it has a special meaning when used
in pg_hba.conf.

regards
--
Rafael Martinez, <r.m.guerrero@usit.uio.no>
Center for Information Technology Services
University of Oslo, Norway

PGP Public Key: http://folk.uio.no/rafael/

#7Alvaro Herrera
alvherre@commandprompt.com
In reply to: Andrew Dunstan (#5)
Re: More robust pg_hba.conf parsing/error logging

Andrew Dunstan wrote:

Alvaro Herrera wrote:

Surely if you want to designate a database named ALL you should use
quotes, same as if you wanted to designate a database named all (see my
other followup).

OK, but if we move to using pg_strcasecmp() that would be a
behaviour change, so I think we couldn't do it before 8.5, in case
someone is relying on it.

Yeah, I think so. It doesn't seem like this is backpatchable (I lean
towards doubting that anyone is using a database named ALL, but still).

It will affect any dbname or username in mixed or upper case, not just
ALL, won't it?

No, I am suggesting to change only the comparisons to the literals
"all", "sameuser", "samegroup" and "samerole".

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

#8Alvaro Herrera
alvherre@commandprompt.com
In reply to: Rafael Martinez (#6)
Re: More robust pg_hba.conf parsing/error logging

Rafael Martinez wrote:

Shouldn't 'all' be a reserved word?, it has a special meaning when used
in pg_hba.conf.

No, it works fine with a line like this:

local "all" all md5

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

#9Rafael Martinez
r.m.guerrero@usit.uio.no
In reply to: Alvaro Herrera (#8)
Re: More robust pg_hba.conf parsing/error logging

Alvaro Herrera wrote:

Rafael Martinez wrote:

Shouldn't 'all' be a reserved word?, it has a special meaning when used
in pg_hba.conf.

No, it works fine with a line like this:

local "all" all md5

Ok, then "all" and "ALL" should be valid values but not all and ALL
(without "")

regards
--
Rafael Martinez, <r.m.guerrero@usit.uio.no>
Center for Information Technology Services
University of Oslo, Norway

PGP Public Key: http://folk.uio.no/rafael/

#10Tom Lane
tgl@sss.pgh.pa.us
In reply to: Alvaro Herrera (#7)
Re: More robust pg_hba.conf parsing/error logging

Alvaro Herrera <alvherre@commandprompt.com> writes:

Andrew Dunstan wrote:

It will affect any dbname or username in mixed or upper case, not just
ALL, won't it?

No, I am suggesting to change only the comparisons to the literals
"all", "sameuser", "samegroup" and "samerole".

Hmm. These words are effectively keywords, so +1 for treating them
case-insensitively, as we do in SQL. But I wonder whether there isn't
an argument for making the comparisons of role and database names
behave more like SQL, too --- that is FOO matches foo but not "FOO".

regards, tom lane

#11Stephen Frost
sfrost@snowman.net
In reply to: Tom Lane (#10)
Re: More robust pg_hba.conf parsing/error logging

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

Alvaro Herrera <alvherre@commandprompt.com> writes:

Andrew Dunstan wrote:

It will affect any dbname or username in mixed or upper case, not just
ALL, won't it?

No, I am suggesting to change only the comparisons to the literals
"all", "sameuser", "samegroup" and "samerole".

Hmm. These words are effectively keywords, so +1 for treating them
case-insensitively, as we do in SQL. But I wonder whether there isn't
an argument for making the comparisons of role and database names
behave more like SQL, too --- that is FOO matches foo but not "FOO".

In general, I think that sounds like a good idea. At the same time, I
wouldn't be against changing the specific 'ALL' special-case comparison
in 8.4.2, using the argument that not many people have moved to it yet
and it's pretty far out there for an 'ALL' database to exist anyway..

Might be too much for a point-release. :/

Just my 2c.

Thanks,

Stephen

#12Tom Lane
tgl@sss.pgh.pa.us
In reply to: Stephen Frost (#11)
Re: More robust pg_hba.conf parsing/error logging

Stephen Frost <sfrost@snowman.net> writes:

In general, I think that sounds like a good idea. At the same time, I
wouldn't be against changing the specific 'ALL' special-case comparison
in 8.4.2, using the argument that not many people have moved to it yet
and it's pretty far out there for an 'ALL' database to exist anyway..

I don't think this is back-patch material. We've had what, one
complaint in twelve years? The odds of causing a problem seem
higher than the odds of preventing one.

regards, tom lane

#13Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#10)
Re: More robust pg_hba.conf parsing/error logging

Tom Lane wrote:

Alvaro Herrera <alvherre@commandprompt.com> writes:

Andrew Dunstan wrote:

It will affect any dbname or username in mixed or upper case, not just
ALL, won't it?

No, I am suggesting to change only the comparisons to the literals
"all", "sameuser", "samegroup" and "samerole".

What happened to this idea?

Hmm. These words are effectively keywords, so +1 for treating them
case-insensitively, as we do in SQL. But I wonder whether there isn't
an argument for making the comparisons of role and database names
behave more like SQL, too --- that is FOO matches foo but not "FOO".

And this one?

-- 
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com
  PG East:  http://www.enterprisedb.com/community/nav-pg-east-2010.do
  + If your life is a hard drive, Christ can be your backup. +
#14Robert Haas
robertmhaas@gmail.com
In reply to: Bruce Momjian (#13)
Re: More robust pg_hba.conf parsing/error logging

On Tue, Feb 23, 2010 at 12:22 AM, Bruce Momjian <bruce@momjian.us> wrote:

Tom Lane wrote:

Alvaro Herrera <alvherre@commandprompt.com> writes:

Andrew Dunstan wrote:

It will affect any dbname or username in mixed or upper case, not just
ALL, won't it?

No, I am suggesting to change only the comparisons to the literals
"all", "sameuser", "samegroup" and "samerole".

What happened to this idea?

Hmm.  These words are effectively keywords, so +1 for treating them
case-insensitively, as we do in SQL.  But I wonder whether there isn't
an argument for making the comparisons of role and database names
behave more like SQL, too --- that is FOO matches foo but not "FOO".

And this one?

Nobody's implemented them? I'd suggest adding these to the TODO.

...Robert

#15Bruce Momjian
bruce@momjian.us
In reply to: Robert Haas (#14)
Re: More robust pg_hba.conf parsing/error logging

Robert Haas wrote:

On Tue, Feb 23, 2010 at 12:22 AM, Bruce Momjian <bruce@momjian.us> wrote:

Tom Lane wrote:

Alvaro Herrera <alvherre@commandprompt.com> writes:

Andrew Dunstan wrote:

It will affect any dbname or username in mixed or upper case, not just
ALL, won't it?

No, I am suggesting to change only the comparisons to the literals
"all", "sameuser", "samegroup" and "samerole".

What happened to this idea?

Hmm. ?These words are effectively keywords, so +1 for treating them
case-insensitively, as we do in SQL. ?But I wonder whether there isn't
an argument for making the comparisons of role and database names
behave more like SQL, too --- that is FOO matches foo but not "FOO".

And this one?

Nobody's implemented them? I'd suggest adding these to the TODO.

Added:

|Process pg_hba.conf keywords as case-insensitive

-- 
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com
  PG East:  http://www.enterprisedb.com/community/nav-pg-east-2010.do
  + If your life is a hard drive, Christ can be your backup. +