From: Drouvot, Bertrand <bertranddrouvot.pg@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Jacob Champion <jchampion@timescale.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Patch proposal: make use of regular expressions for the username in pg_hba.conf
Date: Tue, 18 Oct 2022 09:14:21 +0200
Message-ID: <5000f886-e640-b284-fa64-5cef28b6d272@gmail.com> (raw)
In-Reply-To: <Y04+/KMwnQszTSK7@paquier.xyz>
References: <Yyk5rfYUTylD8XRg@paquier.xyz>
<572017.1663646973@sss.pgh.pa.us>
<YylB+vmF9dti1Alq@paquier.xyz>
<e3091081-dc98-7f2b-829f-c247b9337653@gmail.com>
<Yz0xO0emJ+mxtj2a@paquier.xyz>
<e346f82f-4a6b-22f7-d6c1-cef16d472de9@gmail.com>
<Y0UNP4FJyOBn6WX5@paquier.xyz>
<607af3c9-e45d-3941-41a2-d21065d6594b@gmail.com>
<Y0jz8QeyeEsnXKlm@paquier.xyz>
<b5e61748-a38e-60a0-a208-e813269661dc@gmail.com>
<Y04+/KMwnQszTSK7@paquier.xyz>
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 and after the base validation of a line
>>> (number of fields, etc.).
>>
>> I'm not sure to get the issue here with the proposed approach and
>> pg_ident.conf.
>
> My point is about parse_ident_line(), where we need to be careful in
> the order of the operations. The macros IDENT_MULTI_VALUE() and
> IDENT_FIELD_ABSENT() need to be applied on all the fields first, and
> the regex computation needs to be last. Your patch had a subtile
> issue here, as users may get errors on the computed regex before the
> ordering of the fields as the computation was used *before* the "Get
> the PG rolename token" part of the logic.
Gotcha, thanks! I was wondering if we shouldn't add a comment about that
and I see that you've added one in v2, thanks!
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 done).
> While putting my hands on that, I was also wondering whether we should
> have the error string generated after compilation within the internal
> regcomp() routine, but that would require more arguments to
> pg_regcomp() (as of file name, line number, **err_string), and that
> looks more invasive than necessary. Perhaps the follow-up steps will
> prove me wrong, though :)
I've had the same thought (and that was what the previous global patch
was doing). I'm tempted to think that the follow-steps will prove you
right ;-) (specially if at the end those will be the same error messages
for databases and roles).
>
> A last thing is the introduction of a free() routine for AuthTokens,
> to minimize the number of places where we haev pg_regfree(). The gain
> is minimal, but that looks more consistent with the execution and
> compilation paths.
Agree, that looks better.
I had a look at your v2, did a few tests and it looks good to me.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: bertranddrouvot.pg@gmail.com, michael@paquier.xyz, tgl@sss.pgh.pa.us, jchampion@timescale.com, pgsql-hackers@lists.postgresql.org
Subject: Re: Patch proposal: make use of regular expressions for the username in pg_hba.conf
In-Reply-To: <5000f886-e640-b284-fa64-5cef28b6d272@gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox