pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Drouvot, Bertrand <bdrouvot@amazon.com>
To: Jacob Champion <jchampion@timescale.com>
To: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Patch proposal: make use of regular expressions for the username in pg_hba.conf
Date: Mon, 12 Sep 2022 09:55:25 +0200
Message-ID: <808019e5-5b28-4771-72fb-99e8013fc05e@amazon.com> (raw)
In-Reply-To: <42ec83af-21e4-320a-a973-db4ac1ed053e@timescale.com>
References: <fff0d7c1-8ad4-76a1-9db3-0ab6ec338bf7@amazon.com>
	<42ec83af-21e4-320a-a973-db4ac1ed053e@timescale.com>

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

view thread (37+ messages)  latest in thread

Message-ID: <808019e5-5b28-4771-72fb-99e8013fc05e@amazon.com>
Permalink:  ../808019e5-5b28-4771-72fb-99e8013fc05e@amazon.com/
Also on:    postgresql.org/message-id/808019e5-5b28-4771-72fb-99e8013fc05e@amazon.com

 · 

reply

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: bdrouvot@amazon.com, 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: <808019e5-5b28-4771-72fb-99e8013fc05e@amazon.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