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
