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

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 parsing.  The patch assigns an
>> optional regex_t to it, then extends the use of AuthToken for single
>> hostname entries in pg_hba.conf.  Things going first: shouldn't we
>> combine ident_user and "re" together in the same structure?  Even if
>> we finish by not using AuthToken to store the computed regex, it seems
>> to me that we'd better use the same base structure for pg_ident.conf
>> and pg_hba.conf.  While looking closely at the patch, we would expand
>> the use of AuthToken outside its original context.  I have also looked
>> at make_auth_token(), and wondered if it could be possible to have this
>> routine compile the regexes.  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.).  So with all that in mind, it feels right to
>> not use AuthToken at all when building each HbaLine and each
>> IdentLine, but a new, separate, structure.  We could call that an
>> AuthItem (string, its compiled regex) perhaps?  It could have its own
>> make() routine, taking in input an AuthToken and process
>> pg_regcomp().  Better ideas for this new structure would be welcome,
>> and the idea is that we'd store the post-parsing state of an
>> AuthToken to something that has a compiled regex.  We could finish by
>> using AuthToken at the end and expand its use, but it does not feel
>> completely right either to have a make() routine but not be able to
>> compile its regular expression when creating the AuthToken.
> 
> I have have sent this part too quickly.  As AuthTokens are used in
> check_db() and check_role() when matching entries, it is more
> intuitive to store the regex_t directly in it. 

Yeah, I also think this is the right place for it.

> Changing IdentLine to
> use a AuthToken makes the "quoted" part useless in this case, still it
> could be used in Assert()s to make sure that the data is shaped as
> expected at check-time, enforced at false when creating it in
> parse_ident_line()?

I agree, that makes sense. I'll work on that.

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com