Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-09-15 Thread Magnus Hagander
Tom Lane wrote: > Magnus Hagander <[EMAIL PROTECTED]> writes: Or perhaps the DETAIL portion would be more appropriate as a CONTEXT? >>> +1 > >> Is this the proper syntax for errcontext() messags? > > Hmm, I think I'd suggest following the wording already in use in > ts_locale.c: > >

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-09-15 Thread Tom Lane
Magnus Hagander <[EMAIL PROTECTED]> writes: >>> Or perhaps the DETAIL portion would be more appropriate as a CONTEXT? >> >> +1 > Is this the proper syntax for errcontext() messags? Hmm, I think I'd suggest following the wording already in use in ts_locale.c: errcontext("line %d

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-09-15 Thread Magnus Hagander
Tom Lane wrote: > "Brendan Jurd" <[EMAIL PROTECTED]> writes: >> Or perhaps the DETAIL portion would be more appropriate as a CONTEXT? > > +1 Is this the proper syntax for errcontext() messags? //Magnus Index: hba.c === RCS file: /c

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-09-15 Thread Tom Lane
"Brendan Jurd" <[EMAIL PROTECTED]> writes: > Or perhaps the DETAIL portion would be more appropriate as a CONTEXT? +1 regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/m

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-09-15 Thread Brendan Jurd
On Mon, Sep 15, 2008 at 11:46 PM, Magnus Hagander <[EMAIL PROTECTED]> wrote: > Tom Lane wrote: >> >> Hm, that doesn't seem like a great improvement --- in particular, it >> violates the style guideline that detail messages should be complete >> sentences. >> >> I think the error text is all right a

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-09-15 Thread Magnus Hagander
Tom Lane wrote: > "Brendan Jurd" <[EMAIL PROTECTED]> writes: >> Cool. I had a look at the commitdiff and I think you missed one of >> the error messages (it's still all on one line). It's at >> src/backend/libpq/hba.c line 841. I've included a patch to split it >> up below. > > Hm, that doesn't

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-09-15 Thread Tom Lane
"Brendan Jurd" <[EMAIL PROTECTED]> writes: > Cool. I had a look at the commitdiff and I think you missed one of > the error messages (it's still all on one line). It's at > src/backend/libpq/hba.c line 841. I've included a patch to split it > up below. Hm, that doesn't seem like a great improve

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-09-15 Thread Brendan Jurd
On Mon, Sep 15, 2008 at 10:53 PM, Magnus Hagander <[EMAIL PROTECTED]> wrote: > > Thanks for the reviews, guys. I've adjusted the patch per your comments > (I think), and applied it. > Cool. I had a look at the commitdiff and I think you missed one of the error messages (it's still all on one line

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-09-15 Thread Magnus Hagander
D'Arcy J.M. Cain wrote: > On Fri, 12 Sep 2008 06:53:55 +1000 > "Brendan Jurd" <[EMAIL PROTECTED]> wrote: >> Josh has assigned your patch to me for an initial review. > > And me. Thanks for the reviews, guys. I've adjusted the patch per your comments (I think), and applied it. //Magnus -- Sent

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-09-12 Thread D'Arcy J.M. Cain
On Fri, 12 Sep 2008 06:53:55 +1000 "Brendan Jurd" <[EMAIL PROTECTED]> wrote: > Josh has assigned your patch to me for an initial review. And me. > First up I'd like to say that this is a really nice upgrade. > Shielding a running server from reloading a bogus conf file makes a > whole lot of sens

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-09-12 Thread Magnus Hagander
Brendan Jurd wrote: > Hi Magnus, > > Josh has assigned your patch to me for an initial review. > > First up I'd like to say that this is a really nice upgrade. > Shielding a running server from reloading a bogus conf file makes a > whole lot of sense. Hi! Thanks for your review. > The patch do

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-09-11 Thread Brendan Jurd
Hi Magnus, Josh has assigned your patch to me for an initial review. First up I'd like to say that this is a really nice upgrade. Shielding a running server from reloading a bogus conf file makes a whole lot of sense. On Sun, Aug 17, 2008 at 1:47 AM, Magnus Hagander <[EMAIL PROTECTED]> wrote: >

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-08-16 Thread Magnus Hagander
Tom Lane wrote: > Magnus Hagander <[EMAIL PROTECTED]> writes: >> Other than that, it moves code around to do the parsing in the >> postmaster and the maching in the backend. > > How does that work in EXEC_BACKEND environments? (Not tested yet, still on my TODO, but still) It will parse the file

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-08-16 Thread Tom Lane
Magnus Hagander <[EMAIL PROTECTED]> writes: > Other than that, it moves code around to do the parsing in the > postmaster and the maching in the backend. How does that work in EXEC_BACKEND environments? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hack

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-08-16 Thread Magnus Hagander
Bruce Momjian wrote: > Magnus Hagander wrote: >>> To address Magnus' specific question, right now we store the pg_hba.conf >>> tokens as strings in the postmaster. I am fine with storing them in a >>> more native format and throwing errors for values that don't convert. >>> What would concern me

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-08-15 Thread Bruce Momjian
Magnus Hagander wrote: > > To address Magnus' specific question, right now we store the pg_hba.conf > > tokens as strings in the postmaster. I am fine with storing them in a > > more native format and throwing errors for values that don't convert. > > What would concern me is calling lots of 3rd

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-08-15 Thread Magnus Hagander
Bruce Momjian wrote: > Magnus Hagander wrote: >> Magnus Hagander wrote: >> >> [about the ability to use different maps for ident auth, gss and krb >> auth for example] >> >> It wouldn't be very easy/clean to do that w/o breaking the existing >> structure of pg_ident though, which makes me f

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-08-15 Thread Bruce Momjian
Magnus Hagander wrote: > Magnus Hagander wrote: > > [about the ability to use different maps for ident auth, gss and krb > auth for example] > > It wouldn't be very easy/clean to do that w/o breaking the existing > structure of pg_ident though, which makes me feel like using seperate >

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-08-14 Thread Andreas 'ads' Scherbaum
Hello, On Sat, 02 Aug 2008 18:37:25 +0200 Magnus Hagander wrote: > Tom Lane wrote: > > Magnus Hagander <[EMAIL PROTECTED]> writes: > > > We could catch some simple problems at file load time, perhaps, > > but those usually aren't the ones that cause trouble for people. > > It would catch thing

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-08-14 Thread Magnus Hagander
Magnus Hagander wrote: [about the ability to use different maps for ident auth, gss and krb auth for example] It wouldn't be very easy/clean to do that w/o breaking the existing structure of pg_ident though, which makes me feel like using seperate files is probably the way to go.

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-08-11 Thread Magnus Hagander
Stephen Frost wrote: > Magnus, > > * Magnus Hagander ([EMAIL PROTECTED]) wrote: >> Yeah. I think the question there is just - how likely is it that the >> same installation actually uses >1 authentication method. Personally, I >> think it's not very uncommon at all, but fact remains that as long a

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-08-08 Thread Stephen Frost
Magnus, * Magnus Hagander ([EMAIL PROTECTED]) wrote: > Yeah. I think the question there is just - how likely is it that the > same installation actually uses >1 authentication method. Personally, I > think it's not very uncommon at all, but fact remains that as long as > you only use one of them a

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-08-08 Thread Magnus Hagander
Stephen Frost wrote: > Magnus, > > * Magnus Hagander ([EMAIL PROTECTED]) wrote: >> I thought of another issue with this. My "grand plan" includes being >> able to do username mapping (per pg_ident.conf) for other authentication >> methods than ident. Specifically this would be interesting for all

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-08-07 Thread Stephen Frost
Magnus, * Magnus Hagander ([EMAIL PROTECTED]) wrote: > I thought of another issue with this. My "grand plan" includes being > able to do username mapping (per pg_ident.conf) for other authentication > methods than ident. Specifically this would be interesting for all > external methods, like gssap

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-08-07 Thread Magnus Hagander
Stephen Frost wrote: > * Magnus Hagander ([EMAIL PROTECTED]) wrote: >> Tom Lane wrote: >>> It isn't, and I seem to recall we've had that scenario play out a couple >>> times already for postgresql.conf changes. But pg_hba.conf is far more >>> complex than "variable = value" ... >> Ok, then I didn'

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-08-05 Thread Simon Riggs
On Sun, 2008-08-03 at 10:36 +0200, Magnus Hagander wrote: > Tom Lane wrote: > > Magnus Hagander <[EMAIL PROTECTED]> writes: > >>> The good way to solve this would be to have independant command line > >>> utilities which check pg_hba.conf, pg_ident.conf and postgresql.conf for > >>> errors. Then

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-08-05 Thread Stephen Frost
* Magnus Hagander ([EMAIL PROTECTED]) wrote: > Tom Lane wrote: > > It isn't, and I seem to recall we've had that scenario play out a couple > > times already for postgresql.conf changes. But pg_hba.conf is far more > > complex than "variable = value" ... > > Ok, then I didn't misunderstand that p

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-08-05 Thread Magnus Hagander
Tom Lane wrote: > Magnus Hagander <[EMAIL PROTECTED]> writes: >> Tom Lane wrote: >>> A point that I don't think has been made so far in the thread: the >>> only place the postmaster could complain in event of problems is the >>> postmaster log, which we know too well isn't watched by inexperienced

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-08-05 Thread Tom Lane
Magnus Hagander <[EMAIL PROTECTED]> writes: > Tom Lane wrote: >> A point that I don't think has been made so far in the thread: the >> only place the postmaster could complain in event of problems is the >> postmaster log, which we know too well isn't watched by inexperienced >> DBAs. I guarantee

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-08-05 Thread Magnus Hagander
Tom Lane wrote: > Magnus Hagander <[EMAIL PROTECTED]> writes: >> Should I read this as you warming up slightly to the idea of having the >> postmaster do that? ;-) > > No ;-). Bummer. Worth a shot though :-) > I still think that a "postgres --check-config" feature would be > far more complete

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-08-05 Thread Tom Lane
Magnus Hagander <[EMAIL PROTECTED]> writes: > Should I read this as you warming up slightly to the idea of having the > postmaster do that? ;-) No ;-). I still think that a "postgres --check-config" feature would be far more complete and useful, as well as less likely to cause bugs in critical co

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-08-05 Thread Magnus Hagander
Tom Lane wrote: > Magnus Hagander <[EMAIL PROTECTED]> writes: >> Tom Lane wrote: >>> Seems a lot better to me to just train people to run the check-config >>> code by hand before pulling the trigger to load the settings for real. > >> I think it'd be reasonable to refuse starting if the config is

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-08-04 Thread Tom Lane
Magnus Hagander <[EMAIL PROTECTED]> writes: > Tom Lane wrote: >> Seems a lot better to me to just train people to run the check-config >> code by hand before pulling the trigger to load the settings for real. > I think it'd be reasonable to refuse starting if the config is *known > broken* (such a

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-08-04 Thread Tom Lane
"Joshua D. Drake" <[EMAIL PROTECTED]> writes: > Alvaro Herrera wrote: >> (I think it's better to reuse the same postmaster executable, because >> that way it's easier to have the same parsing routines.) > Change that to pg_ctl and you have a deal :) Did you not understand Alvaro's point? Putting

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-08-03 Thread Stephen Frost
* Magnus Hagander ([EMAIL PROTECTED]) wrote: > Stephen Frost wrote: > > A little extra code in the backend is well worth fixing this foot-gun. > > The concerns raised so far have been "who will write it?" and "what if > > it has bugs?". Neither of these are particularly compelling arguments > > wh

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-08-03 Thread Stephen Frost
* Magnus Hagander ([EMAIL PROTECTED]) wrote: > I think it'd be reasonable to refuse starting if the config is *known > broken* (such as containing lines that are unparseable, or that contain > completely invalid tokens), whereas you'd start if they just contain > things that are "probably wrong". B

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-08-03 Thread Magnus Hagander
Stephen Frost wrote: > * Magnus Hagander ([EMAIL PROTECTED]) wrote: >> For pg_hba.conf, I don't see that as a very big problem, really. It >> doesn't (and shouldn't) modify any "external" variables, so it should be >> as simple as parsing the new file into a completely separate >> list-of-structs a

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-08-03 Thread Stephen Frost
* Magnus Hagander ([EMAIL PROTECTED]) wrote: > For pg_hba.conf, I don't see that as a very big problem, really. It > doesn't (and shouldn't) modify any "external" variables, so it should be > as simple as parsing the new file into a completely separate > list-of-structs and only if it's all correct

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-08-03 Thread Gregory Stark
"Tom Lane" <[EMAIL PROTECTED]> writes: > Robert Treat <[EMAIL PROTECTED]> writes: >> Certainly there isn't any reason to allow a reload of a file that is just >> going to break things when the first connection happens. For that matter, >> why would we ever not want to parse it at HUP time rathe

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-08-03 Thread Magnus Hagander
Tom Lane wrote: > Magnus Hagander <[EMAIL PROTECTED]> writes: >>> The good way to solve this would be to have independant command line >>> utilities which check pg_hba.conf, pg_ident.conf and postgresql.conf for >>> errors. Then DBAs could run a check *before* restarting the server. > >> While cl

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-08-03 Thread Magnus Hagander
Tom Lane wrote: > Robert Treat <[EMAIL PROTECTED]> writes: >> Certainly there isn't any reason to allow a reload of a file that is just >> going to break things when the first connection happens. For that matter, >> why would we ever not want to parse it at HUP time rather than connect time? >

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-08-02 Thread Tom Lane
Robert Treat <[EMAIL PROTECTED]> writes: > Certainly there isn't any reason to allow a reload of a file that is just > going to break things when the first connection happens. For that matter, > why would we ever not want to parse it at HUP time rather than connect time? Two or three reasons w

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-08-02 Thread Tom Lane
"Joshua D. Drake" <[EMAIL PROTECTED]> writes: > True enough but perhaps that is a problem in itself. IMO, we should be > encouraging people to never touch the postgres binary. I don't buy that at all. pg_ctl is useful for some people and not so useful for others; in particular, from the perspect

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-08-02 Thread Robert Treat
On Saturday 02 August 2008 13:36:40 Magnus Hagander wrote: > Josh Berkus wrote: > > Tom, > > > >> Seems a lot better to me to just train people to run the check-config > >> code by hand before pulling the trigger to load the settings for real. > > > > Or just have pg_ctl --check-first as an option.

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-08-02 Thread Alvaro Herrera
Joshua D. Drake wrote: > Alvaro Herrera wrote: >> The problem with pg_ctl is that it's indirectly calling postgres, and it >> doesn't have a lot of a way to know what happened after calling it; >> consider the mess we have with pg_ctl -w. > > True enough but perhaps that is a problem in itself. IM

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-08-02 Thread Joshua D. Drake
Alvaro Herrera wrote: Joshua D. Drake wrote: Tom Lane wrote: Doesn't it seem reasonable that it should be pg_ctl? You should never run postgres directly unless it is for DR. What on earth is DR? Disaster Recovery. The problem with pg_ctl is that it's indirectly calling postgres, and i

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-08-02 Thread Alvaro Herrera
Joshua D. Drake wrote: > Tom Lane wrote: >> I'd go for just >> >> postgres --check-config -D $PGDATA >> >> (In a reload scenario, you'd edit the files in-place and then do this >> before issuing SIGHUP.) Sounds good. > Doesn't it seem reasonable that it should be pg_ctl? You should never

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-08-02 Thread Magnus Hagander
Josh Berkus wrote: > Tom, > >> Seems a lot better to me to just train people to run the check-config >> code by hand before pulling the trigger to load the settings for real. > > Or just have pg_ctl --check-first as an option. Cautious DBAs would use > that. In 2-3 versions, we can make --check

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-08-02 Thread Josh Berkus
Tom, Seems a lot better to me to just train people to run the check-config code by hand before pulling the trigger to load the settings for real. Or just have pg_ctl --check-first as an option. Cautious DBAs would use that. In 2-3 versions, we can make --check-first the default, and requir

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-08-02 Thread Joshua D. Drake
Tom Lane wrote: Alvaro Herrera <[EMAIL PROTECTED]> writes: Tom Lane wrote: Idle thought: maybe what would really make sense here is a "lint" for PG config files, I like this idea. postgres --check-hba-file /path/to/hba.conf postgres --check-conf-file /path/to/postgresql.conf (I think i

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-08-02 Thread Joshua D. Drake
Alvaro Herrera wrote: Tom Lane wrote: Idle thought: maybe what would really make sense here is a "lint" for PG config files, which you'd run as a standalone program and which would look for not only clear errors but questionable things to warn about. For instance it might notice multiple pg_hb

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-08-02 Thread Tom Lane
Magnus Hagander <[EMAIL PROTECTED]> writes: >> The good way to solve this would be to have independant command line >> utilities which check pg_hba.conf, pg_ident.conf and postgresql.conf for >> errors. Then DBAs could run a check *before* restarting the server. > While clearly useful, it'd still

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-08-02 Thread Tom Lane
Alvaro Herrera <[EMAIL PROTECTED]> writes: > Tom Lane wrote: >> Idle thought: maybe what would really make sense here is a "lint" >> for PG config files, > I like this idea. > postgres --check-hba-file /path/to/hba.conf > postgres --check-conf-file /path/to/postgresql.conf > (I think it's better

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-08-02 Thread Magnus Hagander
Josh Berkus wrote: > Magnus, > >> However it would be nice to throw an error or at least a warning when >> parsing >> the file instead of pretending everything's ok. Perhaps authentication >> methods >> should have a function to check whether the method is supported which is >> called when the fil

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-08-02 Thread Magnus Hagander
Tom Lane wrote: > Magnus Hagander <[EMAIL PROTECTED]> writes: >> Is there any actual gain by not doing the parsing in the postmaster, > > Define "parsing". There's quite a lot of possible errors in pg_hba > that it would be totally unreasonable for the postmaster to detect. Parsing as in turning

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-08-02 Thread Josh Berkus
Magnus, However it would be nice to throw an error or at least a warning when parsing the file instead of pretending everything's ok. Perhaps authentication methods should have a function to check whether the method is supported which is called when the file is parsed. The good way to solve t

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-08-02 Thread Alvaro Herrera
Tom Lane wrote: > Idle thought: maybe what would really make sense here is a "lint" > for PG config files, which you'd run as a standalone program and > which would look for not only clear errors but questionable things > to warn about. For instance it might notice multiple pg_hba.conf > entries

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-08-02 Thread Tom Lane
Magnus Hagander <[EMAIL PROTECTED]> writes: > Is there any actual gain by not doing the parsing in the postmaster, Define "parsing". There's quite a lot of possible errors in pg_hba that it would be totally unreasonable for the postmaster to detect. We could catch some simple problems at file loa

Re: [HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-08-02 Thread Gregory Stark
"Magnus Hagander" <[EMAIL PROTECTED]> writes: > I've also noticed that authentication methods error out in different > ways when they are not supported. For example, if I try to use Kerberos > without having it compiled in, I get an error when a client tries to > connect (because we compile in stu

[HACKERS] Parsing of pg_hba.conf and authentication inconsistencies

2008-08-02 Thread Magnus Hagander
The way pg_hba.conf is set up to be loaded/parsed now, we have: postmaster: open and tokenize file (load_hba(), tokenize_file()). backend: parse lines (parse_hba) and check for matches (check_hba/hba_getauthmethod) This means that the code in the postmaster is very simple, and it's shared with t