Thread: [HACKERS] logical replication access control patches
Here is a patch set to refine various access control settings in logical replication. Currently, you need to be replication or superuser for most things, and the goal of these patches is to allow ordinary users equipped with explicit privileges to do most things. (Btw., current documentation is here: https://www.postgresql.org/docs/devel/static/logical-replication-security.html) 0001 Refine rules for altering publication owner No conceptual changes here, just some fixes to allow altering publication owner in more cases. 0002 Add PUBLICATION privilege Add a new privilege kind to tables to determine whether they can be added to a publication. 0003 Add USAGE privilege for publications This controls whether a subscription can use the publication. There is an open issue with this patch: Since the walsender reads system catalogs according to what it is currently streaming, you can't grant this privilege after a subscription has already tried to connect and failed, because the grant will only appear in the "future" of the stream. (You can drop and recreate the subscription, as the test shows.) This might need some snapshot trickery around the aclcheck call. 0004 Add CREATE SUBSCRIPTION privilege on databases New privilege to allow creating a subscription, currently restricted to superuser. (We could also add a CREATE PUBLICATION privilege for symmetry. Currently, publications use the CREATE privilege that schemas also use.) 0005 Add subscription apply worker privilege checks Makes apply workers check privileges on tables before writing to them. Currently, all subscription owners are superuser, but 0004 proposes to change that. 0006 Change logical replication pg_hba.conf use No longer use the "replication" keyword in pg_hba.conf for logical replication. Use the normal database entries instead. Relates to https://www.postgresql.org/message-id/flat/CAB7nPqRf8eOv15SPQJbC1npJoDWTNPMTNp6AvMN-XWwB53h2Cg%40mail.gmail.com -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Attachment
- 0001-Refine-rules-for-altering-publication-owner.patch
- 0002-Add-PUBLICATION-privilege.patch
- 0003-Add-USAGE-privilege-for-publications.patch
- 0004-Add-CREATE-SUBSCRIPTION-privilege-on-databases.patch
- 0005-Add-subscription-apply-worker-privilege-checks.patch
- 0006-Change-logical-replication-pg_hba.conf-use.patch
Peter, * Peter Eisentraut (peter.eisentraut@2ndquadrant.com) wrote: > 0002 Add PUBLICATION privilege > > Add a new privilege kind to tables to determine whether they can be > added to a publication. I'm not convinced that it really makes sense to have PUBLICATION of a table be independent from the rights an owner of a table has. We don't allow other ALTER commands on objects based on GRANT'able rights, in general, so I'm not really sure that it makes sense to do so here. The downside of adding these privileges is that we're burning through the last few bits in the ACLMASK for a privilege that doesn't really seem like it's something that would be GRANT'd in general usage. I have similar reservations regarding the proposed SUBSCRIPTION privilege. I'm certainly all for removing the need for users to be the superuser for such commands, just not sure that they should be GRANT'able privileges instead of privileges which the owner of the relation or database has. Thanks! Stephen
On 2/18/17 18:06, Stephen Frost wrote: > I'm not convinced that it really makes sense to have PUBLICATION of a > table be independent from the rights an owner of a table has. We don't > allow other ALTER commands on objects based on GRANT'able rights, in > general, so I'm not really sure that it makes sense to do so here. The REFERENCES and TRIGGER privileges are very similar in principle. > The downside of adding these privileges is that we're burning through > the last few bits in the ACLMASK for a privilege that doesn't really > seem like it's something that would be GRANT'd in general usage. I don't see any reason why we couldn't increase the size of AclMode if it becomes necessary. > I'm certainly all for removing the need for users to be the superuser > for such commands, just not sure that they should be GRANT'able > privileges instead of privileges which the owner of the relation or > database has. Then you couldn't set up a replication structure involving tables owned by different users without resorting to blunt instruments like having everything owned by the same user or using superusers. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Peter, * Peter Eisentraut (peter.eisentraut@2ndquadrant.com) wrote: > On 2/18/17 18:06, Stephen Frost wrote: > > I'm not convinced that it really makes sense to have PUBLICATION of a > > table be independent from the rights an owner of a table has. We don't > > allow other ALTER commands on objects based on GRANT'able rights, in > > general, so I'm not really sure that it makes sense to do so here. > > The REFERENCES and TRIGGER privileges are very similar in principle. TRIGGER is a great example of an, ultimately, poorly chosen privilege to GRANT away, with a good history of discussion about it: https://www.postgresql.org/message-id/21827.1166115978%40sss.pgh.pa.us https://www.postgresql.org/message-id/7389.1405554356%40sss.pgh.pa.us Further, how would RLS be handled with publication? I've been assuming that it's simply ignored, but that's clearly wrong if a non-owner can publish a table that they just have SELECT,PUBLISH on, isn't it? > > The downside of adding these privileges is that we're burning through > > the last few bits in the ACLMASK for a privilege that doesn't really > > seem like it's something that would be GRANT'd in general usage. > > I don't see any reason why we couldn't increase the size of AclMode if > it becomes necessary. I've fought exactly that argument before and there is a good deal of entirely reasonable push-back on increasing our catalogs by so much. > > I'm certainly all for removing the need for users to be the superuser > > for such commands, just not sure that they should be GRANT'able > > privileges instead of privileges which the owner of the relation or > > database has. > > Then you couldn't set up a replication structure involving tables owned > by different users without resorting to blunt instruments like having > everything owned by the same user or using superusers. That's not correct- you would simply need a user who is considered an owner for all of the objects which you want to replicate, that doesn't have to be a *direct* owner or a superuser. The tables can have individual roles, with some admin role who is a member of those other roles, or another mechanism (as Simon has proposed not too long ago...) to have a given role be considered equivilant to an owner for all of the relations in a particular database. Thanks! Stephen
On 28/02/17 04:10, Stephen Frost wrote: > Peter, > > * Peter Eisentraut (peter.eisentraut@2ndquadrant.com) wrote: >> On 2/18/17 18:06, Stephen Frost wrote: >>> I'm not convinced that it really makes sense to have PUBLICATION of a >>> table be independent from the rights an owner of a table has. We don't >>> allow other ALTER commands on objects based on GRANT'able rights, in >>> general, so I'm not really sure that it makes sense to do so here. >> >> The REFERENCES and TRIGGER privileges are very similar in principle. > > TRIGGER is a great example of an, ultimately, poorly chosen privilege to > GRANT away, with a good history of discussion about it: > > https://www.postgresql.org/message-id/21827.1166115978%40sss.pgh.pa.us > https://www.postgresql.org/message-id/7389.1405554356%40sss.pgh.pa.us > > Further, how would RLS be handled with publication? I've been assuming > that it's simply ignored, but that's clearly wrong if a non-owner can > publish a table that they just have SELECT,PUBLISH on, isn't it? > I don't see why would PUBLISH care about RLS. >>> The downside of adding these privileges is that we're burning through >>> the last few bits in the ACLMASK for a privilege that doesn't really >>> seem like it's something that would be GRANT'd in general usage. >> >> I don't see any reason why we couldn't increase the size of AclMode if >> it becomes necessary. > > I've fought exactly that argument before and there is a good deal of > entirely reasonable push-back on increasing our catalogs by so much. > Hmm to be honest I don't see what's the issue there. >>> I'm certainly all for removing the need for users to be the superuser >>> for such commands, just not sure that they should be GRANT'able >>> privileges instead of privileges which the owner of the relation or >>> database has. >> >> Then you couldn't set up a replication structure involving tables owned >> by different users without resorting to blunt instruments like having >> everything owned by the same user or using superusers. > > That's not correct- you would simply need a user who is considered an > owner for all of the objects which you want to replicate, that doesn't > have to be a *direct* owner or a superuser. > > The tables can have individual roles, with some admin role who is a > member of those other roles, or another mechanism (as Simon has proposed > not too long ago...) to have a given role be considered equivilant to an > owner for all of the relations in a particular database. > I do agree with this though. And I also agree with the overall sentiment that we don't need special PUBLICATION privilege on tables. I do like the rest of the patch. -- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
* Petr Jelinek (petr.jelinek@2ndquadrant.com) wrote: > On 28/02/17 04:10, Stephen Frost wrote: > > Peter, > > > > * Peter Eisentraut (peter.eisentraut@2ndquadrant.com) wrote: > >> On 2/18/17 18:06, Stephen Frost wrote: > >>> I'm not convinced that it really makes sense to have PUBLICATION of a > >>> table be independent from the rights an owner of a table has. We don't > >>> allow other ALTER commands on objects based on GRANT'able rights, in > >>> general, so I'm not really sure that it makes sense to do so here. > >> > >> The REFERENCES and TRIGGER privileges are very similar in principle. > > > > TRIGGER is a great example of an, ultimately, poorly chosen privilege to > > GRANT away, with a good history of discussion about it: > > > > https://www.postgresql.org/message-id/21827.1166115978%40sss.pgh.pa.us > > https://www.postgresql.org/message-id/7389.1405554356%40sss.pgh.pa.us > > > > Further, how would RLS be handled with publication? I've been assuming > > that it's simply ignored, but that's clearly wrong if a non-owner can > > publish a table that they just have SELECT,PUBLISH on, isn't it? > > I don't see why would PUBLISH care about RLS. Perhaps I'm missing something here, in which case I would certainly welcome some clarification, but in curreng PG I can GRANT you access to a table and then limit what you can see with it using RLS and policies. If I then GRANT you the PUBLISH right- shouldn't that also respect the RLS setting and the policies set on the table? Otherwise, PUBLISH is allowing you to gain access to the data in the table that you wouldn't normally be able to see with a simple SELECT. We don't really even need to get to the RLS level to consider this situation- what about column-level privileges? Will users really understand that the PUBLISH right actually allows complete access to the entire relation, rather than just the ability for a user to PUBLISH what they are currently about to SELECT? It certainly doesn't seem intuitive to me, which is why I am concerned that it's going to lead to confusion and bug/security reports down the road, or, worse, poorly configured systems. > >>> The downside of adding these privileges is that we're burning through > >>> the last few bits in the ACLMASK for a privilege that doesn't really > >>> seem like it's something that would be GRANT'd in general usage. > >> > >> I don't see any reason why we couldn't increase the size of AclMode if > >> it becomes necessary. > > > > I've fought exactly that argument before and there is a good deal of > > entirely reasonable push-back on increasing our catalogs by so much. > > Hmm to be honest I don't see what's the issue there. It's a not-insignificant increase in our system catalog data, plus a bunch of work for whomever ends up being the one to want to push us over. The other approach would be to make the ACL system understand that not every privilege applies to every object, but that's would also be a bunch of work (most likely more...). Thanks! Stephen
On 2/27/17 22:10, Stephen Frost wrote: > Peter, > > * Peter Eisentraut (peter.eisentraut@2ndquadrant.com) wrote: >> On 2/18/17 18:06, Stephen Frost wrote: >>> I'm not convinced that it really makes sense to have PUBLICATION of a >>> table be independent from the rights an owner of a table has. We don't >>> allow other ALTER commands on objects based on GRANT'able rights, in >>> general, so I'm not really sure that it makes sense to do so here. >> >> The REFERENCES and TRIGGER privileges are very similar in principle. > > TRIGGER is a great example of an, ultimately, poorly chosen privilege to > GRANT away, with a good history of discussion about it: > > https://www.postgresql.org/message-id/21827.1166115978%40sss.pgh.pa.us > https://www.postgresql.org/message-id/7389.1405554356%40sss.pgh.pa.us Those discussions about the trigger privileges are valid, but the actual reason why this is a problem is because our trigger implementation is broken by default. In the SQL standard, triggers are executed as the table owner, so the trigger procedure does not have full account access to the role that is causing the trigger to fire. This is an independent problem that deserves consideration, but it does not invalidate the kind of privilege that can be granted. >> Then you couldn't set up a replication structure involving tables owned >> by different users without resorting to blunt instruments like having >> everything owned by the same user or using superusers. > > That's not correct- you would simply need a user who is considered an > owner for all of the objects which you want to replicate, that doesn't > have to be a *direct* owner or a superuser. > > The tables can have individual roles, with some admin role who is a > member of those other roles, or another mechanism (as Simon has proposed > not too long ago...) to have a given role be considered equivilant to an > owner for all of the relations in a particular database. I'm not really following what you are saying here. It seems to me that you are describing a new kind of facility that gives a role a given capability with respect to a table. If so, we already have that, namely privileges. If not, please elaborate. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 3/3/17 10:07, Stephen Frost wrote: > Will users really understand that the PUBLISH right actually allows > complete access to the entire relation, rather than just the ability for > a user to PUBLISH what they are currently about to SELECT? It certainly > doesn't seem intuitive to me, which is why I am concerned that it's > going to lead to confusion and bug/security reports down the road, or, > worse, poorly configured systems. Well, this is a new system with its own properties, which is why I'm proposing a new way to manage access. If it were just the same as the existing stuff, we wouldn't need this conversation. I'm interested in other ideas. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 10/03/17 20:02, Peter Eisentraut wrote: > On 2/27/17 22:10, Stephen Frost wrote: >> Peter, >> >> * Peter Eisentraut (peter.eisentraut@2ndquadrant.com) wrote: >>> On 2/18/17 18:06, Stephen Frost wrote: >>>> I'm not convinced that it really makes sense to have PUBLICATION of a >>>> table be independent from the rights an owner of a table has. We don't >>>> allow other ALTER commands on objects based on GRANT'able rights, in >>>> general, so I'm not really sure that it makes sense to do so here. >>> >>> The REFERENCES and TRIGGER privileges are very similar in principle. >> >> TRIGGER is a great example of an, ultimately, poorly chosen privilege to >> GRANT away, with a good history of discussion about it: >> >> https://www.postgresql.org/message-id/21827.1166115978%40sss.pgh.pa.us >> https://www.postgresql.org/message-id/7389.1405554356%40sss.pgh.pa.us > > Those discussions about the trigger privileges are valid, but the actual > reason why this is a problem is because our trigger implementation is > broken by default. In the SQL standard, triggers are executed as the > table owner, so the trigger procedure does not have full account access > to the role that is causing the trigger to fire. This is an independent > problem that deserves consideration, but it does not invalidate the kind > of privilege that can be granted. > >>> Then you couldn't set up a replication structure involving tables owned >>> by different users without resorting to blunt instruments like having >>> everything owned by the same user or using superusers. >> >> That's not correct- you would simply need a user who is considered an >> owner for all of the objects which you want to replicate, that doesn't >> have to be a *direct* owner or a superuser. >> >> The tables can have individual roles, with some admin role who is a >> member of those other roles, or another mechanism (as Simon has proposed >> not too long ago...) to have a given role be considered equivilant to an >> owner for all of the relations in a particular database. > > I'm not really following what you are saying here. It seems to me that > you are describing a new kind of facility that gives a role a given > capability with respect to a table. > > If so, we already have that, namely privileges. If not, please elaborate. > My understanding of what Shephen is proposing is, you have "ownerA" of tableA and "ownerB" of tableB, then you want role "publishe"r to be able to publish those, so you simply grant it the "ownerA" and "ownerB" roles. Obviously that might is many situations mean that the "publisher" role potentially also gets sweeping privileges to other tables which may not be desirable. -- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
On Tue, Mar 14, 2017 at 2:41 PM, Petr Jelinek <petr.jelinek@2ndquadrant.com> wrote: > My understanding of what Shephen is proposing is, you have "ownerA" of > tableA and "ownerB" of tableB, then you want role "publishe"r to be able > to publish those, so you simply grant it the "ownerA" and "ownerB" > roles. Obviously that might is many situations mean that the "publisher" > role potentially also gets sweeping privileges to other tables which may > not be desirable. I didn't hear Stephen propose that "publish" should be a role-attribute, and I don't understand why that would be a good idea. Presumably, we don't want unprivileged users to be able to fire up logical replication because that involves making connections to other systems from the PostgreSQL operating system user's account, and that should be a privileged operation. But that's the subscriber side, not the publisher side. I don't otherwise follow Stephen's argument. It seems like he's complaining that PUBLISH might give more access to the relation than SELECT, but, uh, that's what granting additional privileges does in general, by definition. Mostly we consider that a feature, not a bug. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 14/03/17 19:47, Robert Haas wrote: > On Tue, Mar 14, 2017 at 2:41 PM, Petr Jelinek > <petr.jelinek@2ndquadrant.com> wrote: >> My understanding of what Shephen is proposing is, you have "ownerA" of >> tableA and "ownerB" of tableB, then you want role "publishe"r to be able >> to publish those, so you simply grant it the "ownerA" and "ownerB" >> roles. Obviously that might is many situations mean that the "publisher" >> role potentially also gets sweeping privileges to other tables which may >> not be desirable. > > I didn't hear Stephen propose that "publish" should be a > role-attribute, and I don't understand why that would be a good idea. > Presumably, we don't want unprivileged users to be able to fire up > logical replication because that involves making connections to other > systems from the PostgreSQL operating system user's account, and that > should be a privileged operation. But that's the subscriber side, not > the publisher side. > > I don't otherwise follow Stephen's argument. It seems like he's > complaining that PUBLISH might give more access to the relation than > SELECT, but, uh, that's what granting additional privileges does in > general, by definition. Mostly we consider that a feature, not a bug. > Not what I mean - owner should be able to publish table. If you are granted role of the owner you can do what owner can no? That's how I understand Stephen's proposal. -- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
On 14/03/17 19:49, Petr Jelinek wrote: > On 14/03/17 19:47, Robert Haas wrote: >> On Tue, Mar 14, 2017 at 2:41 PM, Petr Jelinek >> <petr.jelinek@2ndquadrant.com> wrote: >>> My understanding of what Shephen is proposing is, you have "ownerA" of >>> tableA and "ownerB" of tableB, then you want role "publishe"r to be able >>> to publish those, so you simply grant it the "ownerA" and "ownerB" >>> roles. Obviously that might is many situations mean that the "publisher" >>> role potentially also gets sweeping privileges to other tables which may >>> not be desirable. >> >> I didn't hear Stephen propose that "publish" should be a >> role-attribute, and I don't understand why that would be a good idea. >> Presumably, we don't want unprivileged users to be able to fire up >> logical replication because that involves making connections to other >> systems from the PostgreSQL operating system user's account, and that >> should be a privileged operation. But that's the subscriber side, not >> the publisher side. >> >> I don't otherwise follow Stephen's argument. It seems like he's >> complaining that PUBLISH might give more access to the relation than >> SELECT, but, uh, that's what granting additional privileges does in >> general, by definition. Mostly we consider that a feature, not a bug. >> > > Not what I mean - owner should be able to publish table. If you are > granted role of the owner you can do what owner can no? That's how I > understand Stephen's proposal. > Note that I am not necessarily saying it's better though, just trying to explain. It definitely has drawbacks, as in order to grant publish on one table you might be granting lots of privileges on various objects by granting the role. So for granularity purposes Peter's PUBLISH privilege for tables sounds better to me. -- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
Greetings, * Petr Jelinek (petr.jelinek@2ndquadrant.com) wrote: > On 14/03/17 19:47, Robert Haas wrote: > > On Tue, Mar 14, 2017 at 2:41 PM, Petr Jelinek > > <petr.jelinek@2ndquadrant.com> wrote: > >> My understanding of what Shephen is proposing is, you have "ownerA" of > >> tableA and "ownerB" of tableB, then you want role "publishe"r to be able > >> to publish those, so you simply grant it the "ownerA" and "ownerB" > >> roles. Obviously that might is many situations mean that the "publisher" > >> role potentially also gets sweeping privileges to other tables which may > >> not be desirable. > > > > I didn't hear Stephen propose that "publish" should be a > > role-attribute, and I don't understand why that would be a good idea. > > Presumably, we don't want unprivileged users to be able to fire up > > logical replication because that involves making connections to other > > systems from the PostgreSQL operating system user's account, and that > > should be a privileged operation. But that's the subscriber side, not > > the publisher side. > > > > I don't otherwise follow Stephen's argument. It seems like he's > > complaining that PUBLISH might give more access to the relation than > > SELECT, but, uh, that's what granting additional privileges does in > > general, by definition. Mostly we consider that a feature, not a bug. > > Not what I mean - owner should be able to publish table. If you are > granted role of the owner you can do what owner can no? That's how I > understand Stephen's proposal. Exactly correct, yes. I was not suggesting it be a role attribute. If we move forward with making PUBLISH a GRANT'able option then I do worry that people will be surprised that PUBLISH gives you more access to the table than a straight SELECT does. We have a good deal of granularity in what a user can GRANT'd to see and PUBLISH completely ignores all of that. The way I see PUBLISH, it's another command which is able to read from a table, not unlike the way COPY works, but we don't have an independent COPY privilege and the COPY command does actually respect the SELECT privileges on the table. Another approach to solving my concern would be to only allow the publishing of tables by non-owner users who have table-level SELECT rights (meaning they can see all columns of the table) and which don't have RLS enabled. Frankly, though, I really don't buy the argument that there's a serious use-case for non-owners to be GRANT'd the PUBLISH capability, which means we would end up burning one of the few remaining privilege bits for something that isn't going to be used and I don't care for that either. Thanks! Stephen
On Tue, Mar 14, 2017 at 2:56 PM, Petr Jelinek <petr.jelinek@2ndquadrant.com> wrote: > Note that I am not necessarily saying it's better though, just trying to > explain. It definitely has drawbacks, as in order to grant publish on > one table you might be granting lots of privileges on various objects by > granting the role. So for granularity purposes Peter's PUBLISH privilege > for tables sounds better to me. I get that. If, without the patch, letting user X do operation Y will require either giving user X membership in a role that has many privileges, and with the patch, will require only granting a specific privilege on a specific object, then the latter is obviously far better from a security point of view. However, what I'm not clear about is whether this is a situation that's likely to come up much in practice. I would have thought that publications and subscriptions would typically be configured by roles with quite high levels of privilege anyway, in which case the separate PUBLISH privilege would rarely be used in practice, and might therefore fail to be worth using up a bit. I might be missing a plausible scenario in which that's not the case, though. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Greetings, * Robert Haas (robertmhaas@gmail.com) wrote: > However, what I'm not clear about is whether this is a situation > that's likely to come up much in practice. I would have thought that > publications and subscriptions would typically be configured by roles > with quite high levels of privilege anyway, in which case the separate > PUBLISH privilege would rarely be used in practice, and might > therefore fail to be worth using up a bit. I might be missing a > plausible scenario in which that's not the case, though. Right, this is part of my concern also. Further, PUBLISH, as I understand it, is something of a one-time or at least reasonably rarely done operation. This is quite different from a SELECT privilege which is used on every query against the table and which may be GRANT'd to user X today and user Y tomorrow and perhaps REVOKE'd from user X the next day. What happens when the PUBLISH right is REVOKE'd from the user who did the PUBLISH in the first place, for example..? Thanks! Stephen
On 14/03/17 20:09, Robert Haas wrote: > On Tue, Mar 14, 2017 at 2:56 PM, Petr Jelinek > <petr.jelinek@2ndquadrant.com> wrote: >> Note that I am not necessarily saying it's better though, just trying to >> explain. It definitely has drawbacks, as in order to grant publish on >> one table you might be granting lots of privileges on various objects by >> granting the role. So for granularity purposes Peter's PUBLISH privilege >> for tables sounds better to me. > > I get that. If, without the patch, letting user X do operation Y will > require either giving user X membership in a role that has many > privileges, and with the patch, will require only granting a specific > privilege on a specific object, then the latter is obviously far > better from a security point of view. > > However, what I'm not clear about is whether this is a situation > that's likely to come up much in practice. I would have thought that > publications and subscriptions would typically be configured by roles > with quite high levels of privilege anyway, in which case the separate > PUBLISH privilege would rarely be used in practice, and might > therefore fail to be worth using up a bit. I might be missing a > plausible scenario in which that's not the case, though. > Yeah that's rather hard to say in front. Maybe safest action would be to give the permission to owners in 10 and revisit special privilege in 11 based on feedback? -- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
On Tue, Mar 14, 2017 at 3:37 PM, Petr Jelinek <petr.jelinek@2ndquadrant.com> wrote: > On 14/03/17 20:09, Robert Haas wrote: >> On Tue, Mar 14, 2017 at 2:56 PM, Petr Jelinek >> <petr.jelinek@2ndquadrant.com> wrote: >>> Note that I am not necessarily saying it's better though, just trying to >>> explain. It definitely has drawbacks, as in order to grant publish on >>> one table you might be granting lots of privileges on various objects by >>> granting the role. So for granularity purposes Peter's PUBLISH privilege >>> for tables sounds better to me. >> >> I get that. If, without the patch, letting user X do operation Y will >> require either giving user X membership in a role that has many >> privileges, and with the patch, will require only granting a specific >> privilege on a specific object, then the latter is obviously far >> better from a security point of view. >> >> However, what I'm not clear about is whether this is a situation >> that's likely to come up much in practice. I would have thought that >> publications and subscriptions would typically be configured by roles >> with quite high levels of privilege anyway, in which case the separate >> PUBLISH privilege would rarely be used in practice, and might >> therefore fail to be worth using up a bit. I might be missing a >> plausible scenario in which that's not the case, though. > > Yeah that's rather hard to say in front. Maybe safest action would be to > give the permission to owners in 10 and revisit special privilege in 11 > based on feedback? I think that would be entirely sensible. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 3/14/17 15:05, Stephen Frost wrote: > Another approach to solving my concern would be to only allow the > publishing of tables by non-owner users who have table-level SELECT > rights An early version of the logical replication patch set did that. But the problem is that this way someone with SELECT privilege could include a table without replica identity index into a publication and thereby prevent updates to the table. There might be ways to tweak things to make that work better, but right now it works that way. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 3/14/17 15:37, Petr Jelinek wrote: > Yeah that's rather hard to say in front. Maybe safest action would be to > give the permission to owners in 10 and revisit special privilege in 11 > based on feedback? I'm fine with that. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 3/14/17 14:49, Petr Jelinek wrote: > Not what I mean - owner should be able to publish table. If you are > granted role of the owner you can do what owner can no? I didn't actually know that ownership worked that way. You can grant the role of an owner to someone, and then that someone has ownership rights. So that should satisfy a pretty good range of use cases for who can publish what tables. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
New patch set based on the discussions. I have dropped the PUBLICATION privilege patch. The patches are also reordered a bit in approximate decreasing priority order. 0001 Refine rules for altering publication owner kind of a bug fix 0002 Change logical replication pg_hba.conf use This was touched upon in the discussion at <https://www.postgresql.org/message-id/flat/CAB7nPqRf8eOv15SPQJbC1npJoDWTNPMTNp6AvMN-XWwB53h2Cg%40mail.gmail.com> and seems to have been viewed favorably there. 0003 Add USAGE privilege for publications a way to control who can subscribe to a publication 0004 Add subscription apply worker privilege checks This is a prerequisite for the next one (or one like it). 0005 Add CREATE SUBSCRIPTION privilege on databases Need a way to determine which user can create subscriptions. The presented approach made sense to me, but maybe there are other ideas. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Attachment
Hi, I went over this patch set, don't really have all that much to say except it looks good for the most part (details inline). On 16/03/17 02:54, Peter Eisentraut wrote: > New patch set based on the discussions. I have dropped the PUBLICATION > privilege patch. The patches are also reordered a bit in approximate > decreasing priority order. > > 0001 Refine rules for altering publication owner > > kind of a bug fix Agreed, this can be committed as is. > > 0002 Change logical replication pg_hba.conf use > > This was touched upon in the discussion at > <https://www.postgresql.org/message-id/flat/CAB7nPqRf8eOv15SPQJbC1npJoDWTNPMTNp6AvMN-XWwB53h2Cg%40mail.gmail.com> > and seems to have been viewed favorably there. Seems like a good idea and I think can be committed as well. > > 0003 Add USAGE privilege for publications > > a way to control who can subscribe to a publication > Hmm IIUC this removes ability of REPLICATION role to subscribe to publications. I am not quite sure I like that. > 0004 Add subscription apply worker privilege checks > > This is a prerequisite for the next one (or one like it). > > 0005 Add CREATE SUBSCRIPTION privilege on databases > > Need a way to determine which user can create subscriptions. The > presented approach made sense to me, but maybe there are other ideas. > The CREATE SUBSCRIPTION as name of privilege is bit weird but something like SUBSCRIBE would be more fitting for publish side (to which you subscriber) so don't really have a better name. I like that the patches cache the acl result so performance impact should be negligible. -- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
On 3/18/17 09:31, Petr Jelinek wrote: >> 0003 Add USAGE privilege for publications >> >> a way to control who can subscribe to a publication >> > Hmm IIUC this removes ability of REPLICATION role to subscribe to > publications. I am not quite sure I like that. Well, this is kind of the way with all privileges. They take away abilities by default so you can assign them in a more fine-grained manner. You can still connect as superuser and do anything you want, if you want a "quick start" setup. Right now, any replication user connecting can use any publication. There is no way to distinguish different table groupings or different use cases, such as partial replication of some tables that should go over here, or archiving of some other tables that should go over there. That's not optimal. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 20/03/17 13:32, Peter Eisentraut wrote: > On 3/18/17 09:31, Petr Jelinek wrote: >>> 0003 Add USAGE privilege for publications >>> >>> a way to control who can subscribe to a publication >>> >> Hmm IIUC this removes ability of REPLICATION role to subscribe to >> publications. I am not quite sure I like that. > > Well, this is kind of the way with all privileges. They take away > abilities by default so you can assign them in a more fine-grained manner. > > You can still connect as superuser and do anything you want, if you want > a "quick start" setup. > > Right now, any replication user connecting can use any publication. > There is no way to distinguish different table groupings or different > use cases, such as partial replication of some tables that should go > over here, or archiving of some other tables that should go over there. > That's not optimal. > Hmm but REPLICATION role can do basebackup/consume wal, so how does giving it limited publication access help? Wouldn't we need some SUBSCRIPTION role/grant used instead for logical replication connections instead of REPLICATION for this to make sense? -- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
On 3/20/17 15:10, Petr Jelinek wrote: > Hmm but REPLICATION role can do basebackup/consume wal, so how does > giving it limited publication access help? Wouldn't we need some > SUBSCRIPTION role/grant used instead for logical replication connections > instead of REPLICATION for this to make sense? Since we're splitting up the pg_hba.conf setup for logical and physical connections, it would probably not matter. But just to think it through, how could we split this up sensibly? Here is the complete list of things that rolreplication allows: - create/drop replication slot - pg_logical_slot_get_changes() and friends - connect to walsender For logical replication, we could slice it up this way: - new user attribute allowing the creating of logical replication slots - store owner of slot, allow drop and get based on ownership - allow anyone to connect as walsender Another problem is that the walsender command to create a replication slot allows you to load an arbitrary plugin. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 22/03/17 03:38, Peter Eisentraut wrote: > On 3/20/17 15:10, Petr Jelinek wrote: >> Hmm but REPLICATION role can do basebackup/consume wal, so how does >> giving it limited publication access help? Wouldn't we need some >> SUBSCRIPTION role/grant used instead for logical replication connections >> instead of REPLICATION for this to make sense? > > Since we're splitting up the pg_hba.conf setup for logical and physical > connections, it would probably not matter. Hmm yeah I know about that, I am not quite clear on how that change affects this. > > But just to think it through, how could we split this up sensibly? > > Here is the complete list of things that rolreplication allows: > > - create/drop replication slot > - pg_logical_slot_get_changes() and friends > - connect to walsender > > For logical replication, we could slice it up this way: > > - new user attribute allowing the creating of logical replication slots > - store owner of slot, allow drop and get based on ownership > - allow anyone to connect as walsender > I am not quite sure we can do the owner part. Slots are not usual catalog and there is this idea that it should be possible to create them on standby (at least it was reason why our last year proposal to propagate slot creation/updates via WAL was shot down). So we can't do any of the dependency stuff for them. > Another problem is that the walsender command to create a replication > slot allows you to load an arbitrary plugin. > Yeah I am also not sure what to do with the SQL interface tbh as that has same problem. -- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
On 3/18/17 09:31, Petr Jelinek wrote: >> 0001 Refine rules for altering publication owner >> >> kind of a bug fix > > Agreed, this can be committed as is. > >> >> 0002 Change logical replication pg_hba.conf use >> >> This was touched upon in the discussion at >> <https://www.postgresql.org/message-id/flat/CAB7nPqRf8eOv15SPQJbC1npJoDWTNPMTNp6AvMN-XWwB53h2Cg%40mail.gmail.com> >> and seems to have been viewed favorably there. > > Seems like a good idea and I think can be committed as well. Committed these two. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 3/22/17 08:12, Petr Jelinek wrote: > On 22/03/17 03:38, Peter Eisentraut wrote: >> On 3/20/17 15:10, Petr Jelinek wrote: >>> Hmm but REPLICATION role can do basebackup/consume wal, so how does >>> giving it limited publication access help? Wouldn't we need some >>> SUBSCRIPTION role/grant used instead for logical replication connections >>> instead of REPLICATION for this to make sense? >> >> Since we're splitting up the pg_hba.conf setup for logical and physical >> connections, it would probably not matter. > > Hmm yeah I know about that, I am not quite clear on how that change > affects this. Well, the explanation for what I had in mind is too complicated to even put into words, so it's probably not a good idea. ;-) -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 3/15/17 21:54, Peter Eisentraut wrote: > 0001 Refine rules for altering publication owner > 0002 Change logical replication pg_hba.conf use These two were committed. > 0003 Add USAGE privilege for publications I'm withdrawing this one for now, because of some issues that were discussed in the thread. > 0004 Add subscription apply worker privilege checks > 0005 Add CREATE SUBSCRIPTION privilege on databases It would be nice to reach a conclusion on these (the second one particularly), because otherwise we'll be stuck with only superusers being allowed to create subscriptions. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Peter Eisentraut wrote: > On 3/15/17 21:54, Peter Eisentraut wrote: > > 0004 Add subscription apply worker privilege checks > > 0005 Add CREATE SUBSCRIPTION privilege on databases > > It would be nice to reach a conclusion on these (the second one > particularly), because otherwise we'll be stuck with only superusers > being allowed to create subscriptions. I note that the CREATE privilege on databases, which previously only enabled schema creation, now also allows to create publications. I wonder what is different about subscriptions that we need a separate CREATE SUBSCRIPTION privilege; could we allow the three things under the same privilege type? (I suspect not; why give logical replication controls to users who in previous releases were only able to create schemas?) If not, does it make sense to have one privilege for both new things, perhaps something like GRANT LOGICAL REPLICATION THINGIES? If not, maybe we should have three separate priv bits: GRANT CREATE for schemas, GRANT CREATE PUBLICATION and GRANT CREATE SUBSCRIPTION? So this CREATE SUBSCRIPTION priv actually gives you the power to cause the system to open network connections to the outside world. It's not something you give freely to random strangers -- should be guarded moderately tight, because it could be used as covert channel for data leaking. However, it's 1000x better than requiring superuser for subscription creation, so +1 for the current approach. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 3/29/17 19:01, Petr Jelinek wrote: >> So this CREATE SUBSCRIPTION priv actually gives you the power to cause >> the system to open network connections to the outside world. It's not >> something you give freely to random strangers -- should be guarded >> moderately tight, because it could be used as covert channel for data >> leaking. However, it's 1000x better than requiring superuser for >> subscription creation, so +1 for the current approach. >> > Plus on the other hand you might want to allow somebody to stream data > from another server but not necessarily allow said person to create new > objects in the database which standard CREATE privilege would allow. So > I think it makes sense to push this approach. One new concern I just thought about is that having GRANT whatever ON DATABASE would allow a database owner to assign these privileges, but a database owner is not necessarily someone highly privileged to make this decision. So I'm prepared to set this patch to returned with feedback for now. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services