Re: Patch proposal: make use of regular expressions for the username in pg_hba.conf

2022-10-24 Thread Drouvot, Bertrand
Hi, On 10/24/22 5:34 AM, Michael Paquier wrote: On Fri, Oct 21, 2022 at 02:10:37PM +0200, Drouvot, Bertrand wrote: On 10/21/22 2:58 AM, Michael Paquier wrote: I have spent a couple of hours doing a pass over v2, playing manually with regex patterns, reloads, the system views and item lists.

Re: Patch proposal: make use of regular expressions for the username in pg_hba.conf

2022-10-23 Thread Michael Paquier
On Fri, Oct 21, 2022 at 02:10:37PM +0200, Drouvot, Bertrand wrote: > On 10/21/22 2:58 AM, Michael Paquier wrote: >> The same approach with keywords first, regex, and exact match could be >> applied as well for the databases? Perhaps it is just mainly a matter >> of taste, > > Yeah, I think it

Re: Patch proposal: make use of regular expressions for the username in pg_hba.conf

2022-10-21 Thread Drouvot, Bertrand
Hi, On 10/21/22 2:58 AM, Michael Paquier wrote: On Wed, Oct 19, 2022 at 10:45:44AM +0200, Drouvot, Bertrand wrote: Please find attached v1-0001-regex-handling-for-db-and-roles-in-hba.patch to implement regexes for databases and roles in hba. It does also contain new regexes related TAP tests

Re: Patch proposal: make use of regular expressions for the username in pg_hba.conf

2022-10-20 Thread Michael Paquier
On Wed, Oct 19, 2022 at 10:45:44AM +0200, Drouvot, Bertrand wrote: > Please find attached v1-0001-regex-handling-for-db-and-roles-in-hba.patch to > implement regexes for databases and roles in hba. > > It does also contain new regexes related TAP tests and doc updates. Thanks for the updated

Re: Patch proposal: make use of regular expressions for the username in pg_hba.conf

2022-10-19 Thread Drouvot, Bertrand
Hi, On 10/14/22 7:30 AM, Michael Paquier wrote: On Wed, Oct 12, 2022 at 08:17:14AM +0200, Drouvot, Bertrand wrote: Indeed, ;-) The code could be split to tackle things step-by-step: - One refactoring patch to introduce token_regcomp() and token_regexec(), with the introduction of a new

Re: Patch proposal: make use of regular expressions for the username in pg_hba.conf

2022-10-19 Thread Drouvot, Bertrand
Hi, On 10/19/22 3:18 AM, Michael Paquier wrote: On Tue, Oct 18, 2022 at 09:14:21AM +0200, Drouvot, Bertrand wrote: BTW, what about adding a new TAP test (dedicated patch) to test the behavior in case of errors during the regexes compilation in pg_ident.conf and pg_hba.conf (not done yet)? (we

Re: Patch proposal: make use of regular expressions for the username in pg_hba.conf

2022-10-18 Thread Michael Paquier
On Tue, Oct 18, 2022 at 09:14:21AM +0200, Drouvot, Bertrand wrote: > BTW, what about adding a new TAP test (dedicated patch) to test the behavior > in case of errors during the regexes compilation in pg_ident.conf and > pg_hba.conf (not done yet)? (we could add it once this patch series is >

Re: Patch proposal: make use of regular expressions for the username in pg_hba.conf

2022-10-18 Thread Drouvot, Bertrand
Hi, On 10/18/22 7:51 AM, Michael Paquier wrote: On Mon, Oct 17, 2022 at 07:56:02PM +0200, Drouvot, Bertrand wrote: On 10/14/22 7:30 AM, Michael Paquier wrote: This approach would not stick with pg_ident.conf though, as we validate the fields in each line when we put our hands on ident_user

Re: Patch proposal: make use of regular expressions for the username in pg_hba.conf

2022-10-17 Thread Michael Paquier
On Mon, Oct 17, 2022 at 07:56:02PM +0200, Drouvot, Bertrand wrote: > On 10/14/22 7:30 AM, Michael Paquier wrote: >> This approach would not stick with >> pg_ident.conf though, as we validate the fields in each line when we >> put our hands on ident_user and after the base validation of a line >>

Re: Patch proposal: make use of regular expressions for the username in pg_hba.conf

2022-10-17 Thread Drouvot, Bertrand
Hi, On 10/14/22 7:30 AM, Michael Paquier wrote: On Wed, Oct 12, 2022 at 08:17:14AM +0200, Drouvot, Bertrand wrote: Indeed, ;-) I have also looked at make_auth_token(), and wondered if it could be possible to have this routine compile the regexes. I think that it makes sense. This

Re: Patch proposal: make use of regular expressions for the username in pg_hba.conf

2022-10-14 Thread Drouvot, Bertrand
Hi, On 10/14/22 7:30 AM, Michael Paquier wrote: On Wed, Oct 12, 2022 at 08:17:14AM +0200, Drouvot, Bertrand wrote: Indeed, ;-) So, I have spent the last two days looking at all that, studying the structure of the patch and the existing HEAD code, Thanks! The code could be split to tackle

Re: Patch proposal: make use of regular expressions for the username in pg_hba.conf

2022-10-14 Thread Drouvot, Bertrand
Hi, On 10/14/22 8:18 AM, Michael Paquier wrote: On Fri, Oct 14, 2022 at 02:30:25PM +0900, Michael Paquier wrote: First, as of HEAD, AuthToken is only used for elements in a list of role and database names in hba.conf before filling in each HbaLine, hence we limit its usage to the initial

Re: Patch proposal: make use of regular expressions for the username in pg_hba.conf

2022-10-14 Thread Michael Paquier
On Fri, Oct 14, 2022 at 02:30:25PM +0900, Michael Paquier wrote: > First, as of HEAD, AuthToken is only used for elements in a list of > role and database names in hba.conf before filling in each HbaLine, > hence we limit its usage to the initial parsing. The patch assigns an > optional regex_t

Re: Patch proposal: make use of regular expressions for the username in pg_hba.conf

2022-10-13 Thread Michael Paquier
On Wed, Oct 12, 2022 at 08:17:14AM +0200, Drouvot, Bertrand wrote: > Indeed, ;-) So, I have spent the last two days looking at all that, studying the structure of the patch and the existing HEAD code, and it looks like that a few things could be consolidated. First, as of HEAD, AuthToken is only

Re: Patch proposal: make use of regular expressions for the username in pg_hba.conf

2022-10-12 Thread Drouvot, Bertrand
Hi, On 10/11/22 8:29 AM, Michael Paquier wrote: On Mon, Oct 10, 2022 at 09:00:06AM +0200, Drouvot, Bertrand wrote: foreach(cell, tokens) { [...] + tokreg = lfirst(cell); + if (!token_is_regexp(tokreg)) { - if

Re: Patch proposal: make use of regular expressions for the username in pg_hba.conf

2022-10-11 Thread Michael Paquier
On Mon, Oct 10, 2022 at 09:00:06AM +0200, Drouvot, Bertrand wrote: > foreach(cell, tokens) > { > [...] > + tokreg = lfirst(cell); > + if (!token_is_regexp(tokreg)) > { > - if (strcmp(dbname, role) == 0) > +

Re: Patch proposal: make use of regular expressions for the username in pg_hba.conf

2022-10-10 Thread Drouvot, Bertrand
Hi, On 10/6/22 2:53 AM, Michael Paquier wrote: On Wed, Oct 05, 2022 at 03:32:20PM +0200, Drouvot, Bertrand wrote: On 10/5/22 9:24 AM, Michael Paquier wrote: - test_role() -> test_conn() to be able to pass down a database name. - reset_pg_hba() to control the host, db and user parts. The host

Re: Patch proposal: make use of regular expressions for the username in pg_hba.conf

2022-10-10 Thread Drouvot, Bertrand
Hi, On 10/5/22 9:24 AM, Michael Paquier wrote: On Tue, Sep 20, 2022 at 01:33:09PM +0200, Drouvot, Bertrand wrote: Anyway, I have looked at the patch. + List *roles_re; + List *databases_re; + regex_thostname_re; I am surprised by the approach of using separate lists for

Re: Patch proposal: make use of regular expressions for the username in pg_hba.conf

2022-10-05 Thread Michael Paquier
On Wed, Oct 05, 2022 at 03:32:20PM +0200, Drouvot, Bertrand wrote: > On 10/5/22 9:24 AM, Michael Paquier wrote: >> - test_role() -> test_conn() to be able to pass down a database name. >> - reset_pg_hba() to control the host, db and user parts. The host >> part does not really apply after moving

Re: Patch proposal: make use of regular expressions for the username in pg_hba.conf

2022-10-05 Thread Drouvot, Bertrand
Hi, On 10/5/22 9:24 AM, Michael Paquier wrote: Something that stood out on a first review is the refactoring of 001_password.pl that can be done independently of the main patch: Good idea, thanks for the proposal. - test_role() -> test_conn() to be able to pass down a database name. -

Re: Patch proposal: make use of regular expressions for the username in pg_hba.conf

2022-10-05 Thread Michael Paquier
On Tue, Sep 20, 2022 at 01:33:09PM +0200, Drouvot, Bertrand wrote: > I assume (maybe i should not) that if objects starting with / already exist > there is very good reason(s) behind. Then I don't think that preventing > their creation in the DDL would help (quite the contrary for the ones that >

Re: Patch proposal: make use of regular expressions for the username in pg_hba.conf

2022-09-20 Thread Jacob Champion
On Mon, Sep 19, 2022 at 9:09 PM Tom Lane wrote: > You have to assume that somebody (a) has a role or DB name starting > with slash, (b) has an explicit reference to that name in their > pg_hba.conf, (c) doesn't read the release notes, and (d) doesn't > notice that things are misbehaving until

Re: Patch proposal: make use of regular expressions for the username in pg_hba.conf

2022-09-20 Thread Drouvot, Bertrand
Hi, On 9/20/22 6:30 AM, Michael Paquier wrote: On Tue, Sep 20, 2022 at 12:09:33AM -0400, Tom Lane wrote: You have to assume that somebody (a) has a role or DB name starting with slash, (b) has an explicit reference to that name in their pg_hba.conf, (c) doesn't read the release notes, and (d)

Re: Patch proposal: make use of regular expressions for the username in pg_hba.conf

2022-09-19 Thread Michael Paquier
On Tue, Sep 20, 2022 at 12:09:33AM -0400, Tom Lane wrote: > You have to assume that somebody (a) has a role or DB name starting > with slash, (b) has an explicit reference to that name in their > pg_hba.conf, (c) doesn't read the release notes, and (d) doesn't > notice that things are misbehaving

Re: Patch proposal: make use of regular expressions for the username in pg_hba.conf

2022-09-19 Thread Tom Lane
Michael Paquier writes: >> On Thu, Sep 8, 2022 at 5:46 PM Tom Lane wrote: >>> Meh ... that concern seems overblown to me. I guess it's possible >>> that somebody has an HBA entry that looks like that, but it doesn't >>> seem very plausible. Note that we made this exact same change in >>>

Re: Patch proposal: make use of regular expressions for the username in pg_hba.conf

2022-09-19 Thread Michael Paquier
On Fri, Sep 09, 2022 at 03:05:18PM -0700, Jacob Champion wrote: > On Thu, Sep 8, 2022 at 5:46 PM Tom Lane wrote: >> Jacob Champion writes: >> > I think you're going to have to address backwards compatibility >> > concerns. Today, I can create a role named "/a", and I can put that >> > into the

Re: Patch proposal: make use of regular expressions for the username in pg_hba.conf

2022-09-19 Thread Drouvot, Bertrand
Hi, On 9/17/22 8:53 AM, Michael Paquier wrote: On Fri, Sep 16, 2022 at 06:24:07PM +0200, Drouvot, Bertrand wrote: The CF bot is failing for Windows (all other tests are green) and only for the new tap test related to the regular expression on the host name (the ones on database and role are

Re: Patch proposal: make use of regular expressions for the username in pg_hba.conf

2022-09-17 Thread Michael Paquier
On Fri, Sep 16, 2022 at 06:24:07PM +0200, Drouvot, Bertrand wrote: > The CF bot is failing for Windows (all other tests are green) and only for > the new tap test related to the regular expression on the host name (the > ones on database and role are fine). > > The issue is not related to the

Re: Patch proposal: make use of regular expressions for the username in pg_hba.conf

2022-09-16 Thread Drouvot, Bertrand
Hi, On 9/12/22 9:55 AM, Drouvot, Bertrand wrote: Hi, On 9/10/22 1:21 AM, Jacob Champion wrote: On 8/19/22 01:12, Drouvot, Bertrand wrote: + wstr = palloc((strlen(tok->string + 1) + 1) * sizeof(pg_wchar)); + wlen = pg_mb2wchar_with_len(tok->string + 1, +

Re: Patch proposal: make use of regular expressions for the username in pg_hba.conf

2022-09-09 Thread Jacob Champion
On 8/19/22 01:12, Drouvot, Bertrand wrote: > + wstr = palloc((strlen(tok->string + 1) + 1) * sizeof(pg_wchar)); > > + wlen = pg_mb2wchar_with_len(tok->string + 1, > > + wstr, strlen(tok->string + 1)); The

Re: Patch proposal: make use of regular expressions for the username in pg_hba.conf

2022-09-09 Thread Jacob Champion
On Thu, Sep 8, 2022 at 5:46 PM Tom Lane wrote: > Jacob Champion writes: > > I think you're going to have to address backwards compatibility > > concerns. Today, I can create a role named "/a", and I can put that > > into the HBA without quoting it. I'd be unamused if, after an upgrade, > > my

Re: Patch proposal: make use of regular expressions for the username in pg_hba.conf

2022-09-09 Thread Tom Lane
"Drouvot, Bertrand" writes: > Agree that it seems unlikely but maybe we could add a new GUC to turn > the regex usage on the hba file on/off (and use off as the default)? I think that will just add useless complication. regards, tom lane

Re: Patch proposal: make use of regular expressions for the username in pg_hba.conf

2022-09-08 Thread Tom Lane
Jacob Champion writes: > On Fri, Aug 19, 2022 at 1:13 AM Drouvot, Bertrand wrote: >> This is why I think username filtering with regular expressions would >> provide its own advantages. > I think your motivation for the feature is solid. Yeah. I'm not sure that I buy the argument that this is

Re: Patch proposal: make use of regular expressions for the username in pg_hba.conf

2022-09-08 Thread Jacob Champion
On Fri, Aug 19, 2022 at 1:13 AM Drouvot, Bertrand wrote: > This is why I think username filtering with regular expressions would > provide its own advantages. > > Thoughts? Looking forward to your feedback, I think your motivation for the feature is solid. It is killing me a bit that this is