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
