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

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,
>> + wstr, strlen(tok->string + 1));
> The (tok->string + 1) construction comes up often enough that I think it
> should be put in a `regex` variable or similar. That would help my eyes
> with the (strlen(tok->string + 1) + 1) construction, especially.
>
> I noticed that for pg_ident, we precompile the regexes per-line and
> reuse those in child processes. Whereas here we're compiling, using, and
> then discarding the regex for each check. I think the example set by the
> pg_ident code is probably the one to follow, unless you have a good
> reason not to.

Thanks for the feedback.

Yeah fully agree. I'll provide a new version that follow the same logic
as the pg_ident code.

>> +# Testing with regular expression for username
>> +reset_pg_hba($node, '/^.*md.*$', 'password');
>> +test_role($node, 'md5_role', 'password from pgpass and regular expression for username', 0);
>> +
> IMO the coverage for this patch needs to be filled out. Negative test
> cases are more important than positive ones for security-related code.

Agree, will do.

>
> Other than that, and Tom's note on potentially expanding this to other
> areas,

I'll add regexp usage for the database column and also the for the
address one when non CIDR is provided (so host name(s)) (I think it also
makes sense specially as we don't allow multiple values for this column).

Regards,

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