Re: use has_privs_of_role() for pg_hba.conf

2022-10-16 Thread Stephen Frost
Greetings, * Nathan Bossart (nathandboss...@gmail.com) wrote: > On Sat, Oct 08, 2022 at 11:46:50AM -0400, Robert Haas wrote: > > Now there may be some other scenario in which the patch is going in > > exactly the right direction, and if I knew what it was, maybe I'd > > agree that the patch was a

Re: use has_privs_of_role() for pg_hba.conf

2022-10-11 Thread Nathan Bossart
On Tue, Oct 11, 2022 at 02:01:07PM +0900, Michael Paquier wrote: > Thanks, applied. It took me a few minutes to note that > regress_regression_* is required in the object names because we need > to use the same name for the parent role and the database, with > "regress_" being required for the

Re: use has_privs_of_role() for pg_hba.conf

2022-10-10 Thread Michael Paquier
On Sun, Oct 09, 2022 at 02:13:48PM -0700, Nathan Bossart wrote: > Here you go. Thanks, applied. It took me a few minutes to note that regress_regression_* is required in the object names because we need to use the same name for the parent role and the database, with "regress_" being required for

Re: use has_privs_of_role() for pg_hba.conf

2022-10-09 Thread Nathan Bossart
On Sun, Oct 09, 2022 at 10:19:51AM +0900, Michael Paquier wrote: > Even if the patch is at the end rejected, I think that the test is > still useful once you switch its logic to use membership and not > inherited privileges for the roles created, and there is zero coverage > for "samplegroup" and

Re: use has_privs_of_role() for pg_hba.conf

2022-10-08 Thread Michael Paquier
On Sat, Oct 08, 2022 at 10:12:22AM -0700, Nathan Bossart wrote: > Okay, I think there are sufficient votes against this change to simply mark > it Rejected. Thanks for the discussion! Even if the patch is at the end rejected, I think that the test is still useful once you switch its logic to use

Re: use has_privs_of_role() for pg_hba.conf

2022-10-08 Thread Nathan Bossart
On Sat, Oct 08, 2022 at 09:57:02AM -0700, David G. Johnston wrote: > I would tend to agree that even membership probably shouldn't be involved > here, and that this entire feature would be implemented in an orthogonal > manner. I don't see any specific need to try and move to a more isolated >

Re: use has_privs_of_role() for pg_hba.conf

2022-10-08 Thread Nathan Bossart
On Sat, Oct 08, 2022 at 11:46:50AM -0400, Robert Haas wrote: > Now there may be some other scenario in which the patch is going in > exactly the right direction, and if I knew what it was, maybe I'd > agree that the patch was a great idea. But I haven't seen anything > like that on the thread.

Re: use has_privs_of_role() for pg_hba.conf

2022-10-08 Thread David G. Johnston
On Sat, Oct 8, 2022 at 8:47 AM Robert Haas wrote: > On Sat, Oct 8, 2022 at 11:14 AM Tom Lane wrote: > > Joe Conway writes: > > > Thanks -- looks good to me. If there are no other comments or concerns, > > > I will commit/push by the end of the weekend. > > > > Robert seems to think that this

Re: use has_privs_of_role() for pg_hba.conf

2022-10-08 Thread Robert Haas
On Sat, Oct 8, 2022 at 11:14 AM Tom Lane wrote: > Joe Conway writes: > > Thanks -- looks good to me. If there are no other comments or concerns, > > I will commit/push by the end of the weekend. > > Robert seems to think that this patch might be completely misguided, > so I'm not sure we have

Re: use has_privs_of_role() for pg_hba.conf

2022-10-08 Thread Tom Lane
Joe Conway writes: > Thanks -- looks good to me. If there are no other comments or concerns, > I will commit/push by the end of the weekend. Robert seems to think that this patch might be completely misguided, so I'm not sure we have real consensus. I think he may have a point. An angle that

Re: use has_privs_of_role() for pg_hba.conf

2022-10-08 Thread Joe Conway
On 10/7/22 17:58, Nathan Bossart wrote: On Fri, Oct 07, 2022 at 04:18:59PM -0400, Tom Lane wrote: There's another problem there, which is that buildfarm animals using -DENFORCE_REGRESSION_TEST_NAME_RESTRICTIONS will complain about role names that don't start with "regress_". Huh, I hadn't

Re: use has_privs_of_role() for pg_hba.conf

2022-10-07 Thread Michael Paquier
On Fri, Oct 07, 2022 at 07:59:08AM -0400, Robert Haas wrote: > I hadn't noticed this thread before. > > I'm not sure whether this is properly considered a privilege check. It > could even be an anti-privilege, if the pg_hba.conf line in question > is maked "reject". > > I'm not taking the

Re: use has_privs_of_role() for pg_hba.conf

2022-10-07 Thread Nathan Bossart
On Fri, Oct 07, 2022 at 04:18:59PM -0400, Tom Lane wrote: > There's another problem there, which is that buildfarm animals > using -DENFORCE_REGRESSION_TEST_NAME_RESTRICTIONS will complain > about role names that don't start with "regress_". Huh, I hadn't noticed that one before. It looks like

Re: use has_privs_of_role() for pg_hba.conf

2022-10-07 Thread Tom Lane
Nathan Bossart writes: > On Fri, Oct 07, 2022 at 03:34:51PM +0900, Michael Paquier wrote: >> Thanks. I would perhaps use names less generic than role{1,2,3} for >> the roles or "role1" for the database name, but the logic looks >> sound. > Here is a new version with more descriptive role names.

Re: use has_privs_of_role() for pg_hba.conf

2022-10-07 Thread Nathan Bossart
On Fri, Oct 07, 2022 at 03:34:51PM +0900, Michael Paquier wrote: > Thanks. I would perhaps use names less generic than role{1,2,3} for > the roles or "role1" for the database name, but the logic looks > sound. Here is a new version with more descriptive role names. -- Nathan Bossart Amazon Web

Re: use has_privs_of_role() for pg_hba.conf

2022-10-07 Thread Robert Haas
On Fri, Apr 1, 2022 at 6:07 PM Nathan Bossart wrote: > > 6198420 ensured that has_privs_of_role() is used for predefined roles, > which means that the role inheritance hierarchy is checked instead of mere > role membership. However, inheritance is still not respected for > pg_hba.conf.

Re: use has_privs_of_role() for pg_hba.conf

2022-10-07 Thread Michael Paquier
On Thu, Oct 06, 2022 at 08:27:11PM -0700, Nathan Bossart wrote: > On Fri, Oct 07, 2022 at 11:06:47AM +0900, Michael Paquier wrote: >> Rather than putting that in a separate script, which means >> initializing a new node, etc. could it be better to put that in >> 001_password.pl instead? It would

Re: use has_privs_of_role() for pg_hba.conf

2022-10-06 Thread Nathan Bossart
On Fri, Oct 07, 2022 at 11:06:47AM +0900, Michael Paquier wrote: > Rather than putting that in a separate script, which means > initializing a new node, etc. could it be better to put that in > 001_password.pl instead? It would be cheaper. Works for me. -- Nathan Bossart Amazon Web Services:

Re: use has_privs_of_role() for pg_hba.conf

2022-10-06 Thread Michael Paquier
On Thu, Oct 06, 2022 at 10:43:43AM -0700, Nathan Bossart wrote: > Here is a new version of the patch with a test. Thanks, that helps a lot. Now I grab the difference even if your previous patch was already switching the documentation to tell exactly that. On the ground of 6198420, it looks

Re: use has_privs_of_role() for pg_hba.conf

2022-10-06 Thread Nathan Bossart
On Thu, Oct 06, 2022 at 07:33:46AM -0400, Joe Conway wrote: > On 10/6/22 04:09, Michael Paquier wrote: >> This patch looks simple, but it is a very sensitive area so I think >> that we should be really careful. pg_hba.conf does not have a lot of >> test coverage, so I'd really prefer if we add

Re: use has_privs_of_role() for pg_hba.conf

2022-10-06 Thread Joe Conway
On 10/6/22 04:09, Michael Paquier wrote: On Mon, Apr 04, 2022 at 07:25:51AM -0700, Nathan Bossart wrote: On Mon, Apr 04, 2022 at 09:36:13AM -0400, Joshua Brindle wrote: Good catch, I think this is a logical followup to the previous has_privs_of_role patch. Reviewed and +1 Thanks! I created

Re: use has_privs_of_role() for pg_hba.conf

2022-10-06 Thread Michael Paquier
On Mon, Apr 04, 2022 at 07:25:51AM -0700, Nathan Bossart wrote: > On Mon, Apr 04, 2022 at 09:36:13AM -0400, Joshua Brindle wrote: >> Good catch, I think this is a logical followup to the previous >> has_privs_of_role patch. >> >> Reviewed and +1 > > Thanks! I created a commitfest entry for

Re: use has_privs_of_role() for pg_hba.conf

2022-04-04 Thread Nathan Bossart
On Mon, Apr 04, 2022 at 09:36:13AM -0400, Joshua Brindle wrote: > Good catch, I think this is a logical followup to the previous > has_privs_of_role patch. > > Reviewed and +1 Thanks! I created a commitfest entry for this: https://commitfest.postgresql.org/38/3609/ -- Nathan Bossart

Re: use has_privs_of_role() for pg_hba.conf

2022-04-04 Thread Joshua Brindle
On Fri, Apr 1, 2022 at 6:06 PM Nathan Bossart wrote: > > Hi hackers, > > 6198420 ensured that has_privs_of_role() is used for predefined roles, > which means that the role inheritance hierarchy is checked instead of mere > role membership. However, inheritance is still not respected for >

use has_privs_of_role() for pg_hba.conf

2022-04-01 Thread Nathan Bossart
Hi hackers, 6198420 ensured that has_privs_of_role() is used for predefined roles, which means that the role inheritance hierarchy is checked instead of mere role membership. However, inheritance is still not respected for pg_hba.conf. Specifically, "samerole", "samegroup", and "+" still use