From: Jacob Champion <jchampion@timescale.com>
To: Drouvot, Bertrand <bdrouvot@amazon.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: Fri, 9 Sep 2022 16:21:32 -0700
Message-ID: <42ec83af-21e4-320a-a973-db4ac1ed053e@timescale.com> (raw)
In-Reply-To: <fff0d7c1-8ad4-76a1-9db3-0ab6ec338bf7@amazon.com>
References: <fff0d7c1-8ad4-76a1-9db3-0ab6ec338bf7@amazon.com>
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.
> +# 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.
Other than that, and Tom's note on potentially expanding this to other
areas, I think this is a pretty straightforward patch.
Thanks,
--Jacob
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: jchampion@timescale.com, bdrouvot@amazon.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: <42ec83af-21e4-320a-a973-db4ac1ed053e@timescale.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