Thread: More robust pg_hba.conf parsing/error logging
-----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 ServicesUniversity 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-----
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
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
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.
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
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 ServicesUniversity of Oslo, Norway PGP Public Key: http://folk.uio.no/rafael/
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.
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
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 ServicesUniversity of Oslo, Norway PGP Public Key: http://folk.uio.no/rafael/
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
* 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
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
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.comPG East: http://www.enterprisedb.com/community/nav-pg-east-2010.do + If your life is a hard drive,Christ can be your backup. +
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
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.comPG East: http://www.enterprisedb.com/community/nav-pg-east-2010.do + If your life is a hard drive,Christ can be your backup. +