Re: [HACKERS] [COMMITTERS] pgsql: Add a hook in ExecCheckRTPerms().
(2010/07/10 2:12), Simon Riggs wrote: On Fri, 2010-07-09 at 11:09 -0400, Stephen Frost wrote: Strangely, I was looking into removing the ExecCheckRTPerms check altogether by forcing plan invalidation when permissions are updated. That would be a performance tweak that would render this change useless. I don't see how you could remove ExecCheckRTPerms..? It's what handles all permissions checking for DML (like, making sure you have SELECT rights on the table you're trying to query). I could see forcing plan invalidation when permissions are updated, sure, but that doesn't mean you can stop doing them altogether anywhere. Where would you move the permissions checking to? I apologise, when I said removing the check altogether, I meant removing from the executor path. The Linux kernel has a facility that notify userspace applications when the security policy is changed. This message is delivered using netlink socket from the kernel. Once we received it, then SE-PG's access control decision cache will be invalidated. Even if it was invalidated in mid-query, please note that the older policy was valid when we make access control decision. If and when we have cached plans being already permission checked, I'd like the core PG to ask external securities whether it is still valid from the perspective of external security policy. If already invalid, we can check permissions again on the cached plan. Thanks, -- KaiGai Kohei kai...@ak.jp.nec.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [COMMITTERS] pgsql: Add a hook in ExecCheckRTPerms().
On Fri, 2010-07-09 at 17:21 -0400, Tom Lane wrote: Simon Riggs si...@2ndquadrant.com writes: On Fri, 2010-07-09 at 14:01 -0400, Tom Lane wrote: Consider PREPARE followed only later by EXECUTE. Your proposal would make the PREPARE fail outright, when it currently does not. Just to avoid wasted investigation: are you saying that is important behaviour that is essential we retain in PostgreSQL, or will you hear evidence that supporting that leads to a performance decrease elsewhere? Well, I think that that problem makes moving the checks into the planner a nonstarter. But as somebody pointed out upthread, you could still get what you want by keeping a flag saying permission checks have been done so the executor could skip the checks on executions after the first. Seems like best idea. I'd still want to see some evidence showing that it's worth troubling over though. Premature optimization being the root of all evil, and all that. (In this case, the hazard we expose ourselves to seems to be security holes due to missed resets of the flag.) If we did this it would be to add one line to the code if (!perms_ok) That doesn't seem to fall into the category of evil optimization to me. I've seen you recode other parts of the executor stating reasons like shave another few cycles off the main path and that seems the case here. We shouldn't need to debate the consequences of Amhdahls law each time. Attached is a script to allow pgbench to be used to measure difference between superuser and a typical privilege model for the pgbench tables. -- Simon Riggs www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Training and Services drop user if exists ref; create user ref; drop user if exists cust; create user cust; drop user if exists report; create user report; drop user if exists xact; create user xact; /* Reference data */ GRANT INSERT, UPDATE, DELETE ON pgbench_tellers TO ref; GRANT INSERT, UPDATE, DELETE ON pgbench_branches TO ref; /* Customer maintenance */ GRANT INSERT, DELETE ON pgbench_accounts TO cust; GRANT UPDATE (bid, filler) ON pgbench_accounts TO cust; /* Reporting */ GRANT SELECT ON pgbench_accounts TO report; GRANT SELECT ON pgbench_branches TO report; GRANT SELECT ON pgbench_tellers TO report; GRANT SELECT ON pgbench_history TO report; /* Transactions */ GRANT UPDATE (abalance) ON pgbench_accounts TO xact; GRANT UPDATE (tbalance) ON pgbench_tellers TO xact; GRANT UPDATE (bbalance) ON pgbench_branches TO xact; GRANT INSERT ON pgbench_history TO xact; GRANT SELECT ON pgbench_accounts TO xact; GRANT SELECT ON pgbench_branches TO xact; GRANT SELECT ON pgbench_tellers TO xact; GRANT SELECT ON pgbench_history TO xact; -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [COMMITTERS] pgsql: Add a hook in ExecCheckRTPerms().
Simon Riggs si...@2ndquadrant.com writes: On Fri, 2010-07-09 at 17:21 -0400, Tom Lane wrote: I'd still want to see some evidence showing that it's worth troubling over though. Premature optimization being the root of all evil, and all that. (In this case, the hazard we expose ourselves to seems to be security holes due to missed resets of the flag.) If we did this it would be to add one line to the code if (!perms_ok) That doesn't seem to fall into the category of evil optimization to me. The problem I foresee is not in the testing of the flag, it's in the setting/resetting of it. It's a reliability penalty not a performance penalty --- and any mistakes would count as security issues. Now it may be that you can offer a convincing argument that no such mistake/oversight is likely. But you haven't even tried to make that case. Even if you can show that the risk is small, it's not going to be zero, so we have to trade it off against a demonstrated performance improvement. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [COMMITTERS] pgsql: Add a hook in ExecCheckRTPerms().
On Jul 11, 2010, at 10:44 AM, Tom Lane t...@sss.pgh.pa.us wrote: Simon Riggs si...@2ndquadrant.com writes: On Fri, 2010-07-09 at 17:21 -0400, Tom Lane wrote: I'd still want to see some evidence showing that it's worth troubling over though. Premature optimization being the root of all evil, and all that. (In this case, the hazard we expose ourselves to seems to be security holes due to missed resets of the flag.) If we did this it would be to add one line to the code if (!perms_ok) That doesn't seem to fall into the category of evil optimization to me. The problem I foresee is not in the testing of the flag, it's in the setting/resetting of it. It's a reliability penalty not a performance penalty --- and any mistakes would count as security issues. Now it may be that you can offer a convincing argument that no such mistake/oversight is likely. But you haven't even tried to make that case. Even if you can show that the risk is small, it's not going to be zero, so we have to trade it off against a demonstrated performance improvement. There's no point in going back and forth here until we have a patch and the results of some performance testing using said patch. If Simon writes one and submits it with some results, we'll consider it on the merits. I think that's all Simon is asking for, and I don't think anyone is seriously arguing anything to the contrary. Like Tom, I'm skeptical that there is much performance to be found here, but if I'm wrong, I'm happy to have someone demonstrate it. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [COMMITTERS] pgsql: Add a hook in ExecCheckRTPerms().
On Fri, 2010-07-09 at 14:06 +, Robert Haas wrote: Log Message: --- Add a hook in ExecCheckRTPerms(). This hook allows a loadable module to gain control when table permissions are checked. It is expected to be used by an eventual SE-PostgreSQL implementation, but there are other possible applications as well. A sample contrib module can be found in the archives at: http://archives.postgresql.org/pgsql-hackers/2010-05/msg01095.php The loadable module doesn't gain control here it simplify kicks-in after, and in addition to, normal checking. That just means you have the option of failing for additional reasons. We're not passing in any form of context other than the rangetable so what additional reasons could there be? This is of no use to anything that uses object labelling. We're not even at the part of the executor where we would be able to identify objects yet, so I can't see what value this brings. Though I am certainly in favour in general terms of simple changes to enhance security configuration features. Strangely, I was looking into removing the ExecCheckRTPerms check altogether by forcing plan invalidation when permissions are updated. That would be a performance tweak that would render this change useless. -- Simon Riggs www.2ndQuadrant.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [COMMITTERS] pgsql: Add a hook in ExecCheckRTPerms().
On Fri, Jul 9, 2010 at 10:51 AM, Simon Riggs si...@2ndquadrant.com wrote: The loadable module doesn't gain control here it simplify kicks-in after, and in addition to, normal checking. That just means you have the option of failing for additional reasons. True. We could change it so that the normal checking is bypassed if the hook is installed, and leave it up to the hook whether to call the standard checks as well, but I don't think there's much of a use case for that. We're not passing in any form of context other than the rangetable so what additional reasons could there be? This is of no use to anything that uses object labelling. We're not even at the part of the executor where we would be able to identify objects yet, so I can't see what value this brings. Though I am certainly in favour in general terms of simple changes to enhance security configuration features. Well, KaiGai Kohei already posted a proof-of-concept patch showing how this could be used by a simple SE-PostgreSQL implementation. Since we don't have a security labelling facility yet, he used the comment on the relation to store the security label (there are other ways it could be done too, of course). Strangely, I was looking into removing the ExecCheckRTPerms check altogether by forcing plan invalidation when permissions are updated. That would be a performance tweak that would render this change useless. Huh. Obviously, I would have refrained from committing the patch had I known that it was going to conflict with work someone else was doing in this area, at least until we reached consensus on which way to go with it, but since you didn't post about it on -hackers, I had no idea that was the case. Sounds like you should probably post your proposal and we can discuss what to do in general and also with respect to this hook. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [COMMITTERS] pgsql: Add a hook in ExecCheckRTPerms().
* Simon Riggs (si...@2ndquadrant.com) wrote: The loadable module doesn't gain control here it simplify kicks-in after, and in addition to, normal checking. That just means you have the option of failing for additional reasons. Right, additional checks (such as the label) can be done. We're not passing in any form of context other than the rangetable so what additional reasons could there be? This is of no use to anything that uses object labelling. We're not even at the part of the executor where we would be able to identify objects yet, so I can't see what value this brings. I'm a bit confused by this. By this point, we've fully planned out the query, looked up info about all the objects involved, and the module can now go look up any other information about them that it needs. It can also use info like what the current user is and information about the connection. There was actually a proof-of-concept module created by KaiGai to do all of this with SELinux using the existing COMMENT tables, I'm pretty sure we would have heard a bit earlier if it was useless. Strangely, I was looking into removing the ExecCheckRTPerms check altogether by forcing plan invalidation when permissions are updated. That would be a performance tweak that would render this change useless. I don't see how you could remove ExecCheckRTPerms..? It's what handles all permissions checking for DML (like, making sure you have SELECT rights on the table you're trying to query). I could see forcing plan invalidation when permissions are updated, sure, but that doesn't mean you can stop doing them altogether anywhere. Where would you move the permissions checking to? Wherever it is, this hook would just need to follow. I don't know that you could (or that I'd be comfortable with) move the permissions checking to the planner and then rely on plan invalidation on permission changes, but if you did, just make sure the hook is included in the decision about allowing the query. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] [COMMITTERS] pgsql: Add a hook in ExecCheckRTPerms().
Simon Riggs si...@2ndquadrant.com writes: Strangely, I was looking into removing the ExecCheckRTPerms check altogether by forcing plan invalidation when permissions are updated. That would be a performance tweak that would render this change useless. That seems both pointless and wrong. Permissions checks should happen at execution time not plan time. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [COMMITTERS] pgsql: Add a hook in ExecCheckRTPerms().
On Fri, 2010-07-09 at 11:07 -0400, Robert Haas wrote: Strangely, I was looking into removing the ExecCheckRTPerms check altogether by forcing plan invalidation when permissions are updated. That would be a performance tweak that would render this change useless. Huh. Obviously, I would have refrained from committing the patch had I known that it was going to conflict with work someone else was doing in this area, at least until we reached consensus on which way to go with it, but since you didn't post about it on -hackers, I had no idea that was the case. Sounds like you should probably post your proposal and we can discuss what to do in general and also with respect to this hook. Sorry, yes, you couldn't possibly know I was looking at that. I just meant it was strange those things overlapped. -- Simon Riggs www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Training and Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [COMMITTERS] pgsql: Add a hook in ExecCheckRTPerms().
On Fri, 2010-07-09 at 11:09 -0400, Tom Lane wrote: Simon Riggs si...@2ndquadrant.com writes: Strangely, I was looking into removing the ExecCheckRTPerms check altogether by forcing plan invalidation when permissions are updated. That would be a performance tweak that would render this change useless. That seems both pointless and wrong. Permissions checks should happen at execution time not plan time. Agreed that permission checks should logically be applied at execution time. I am proposing a performance optimisation, not a change in behaviour. Permissions are set much less frequently than plans and connections, so when the only permission check is at table level it makes more sense to optimistically assume that permission checks will never change for a plan and cache the result of the permission check. That way we need only check permissions once at plan time rather than checking them every single execution. The only extra code to do this would be to invalidate plans when permissions change for a table. That doesn't seem hard or invasive. The proposed performance enhancement would be very useful since we have to check permissions of functions, views, tables and every other aspect. We could spend a while quantifying that overhead, though non-zero is all we need to know. -- Simon Riggs www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Training and Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [COMMITTERS] pgsql: Add a hook in ExecCheckRTPerms().
On Fri, 2010-07-09 at 11:07 -0400, Robert Haas wrote: On Fri, Jul 9, 2010 at 10:51 AM, Simon Riggs si...@2ndquadrant.com wrote: The loadable module doesn't gain control here it simplify kicks-in after, and in addition to, normal checking. That just means you have the option of failing for additional reasons. True. We could change it so that the normal checking is bypassed if the hook is installed, and leave it up to the hook whether to call the standard checks as well, but I don't think there's much of a use case for that. With respect, there doesn't seem to be much use case anyway. I'm sorry to be expressing that opinion now; been away for a while. I am somewhat amazed that Tom isn't dancing on your head for proposing it though. We're not passing in any form of context other than the rangetable so what additional reasons could there be? This is of no use to anything that uses object labelling. We're not even at the part of the executor where we would be able to identify objects yet, so I can't see what value this brings. Though I am certainly in favour in general terms of simple changes to enhance security configuration features. Well, KaiGai Kohei already posted a proof-of-concept patch showing how this could be used by a simple SE-PostgreSQL implementation. Since we don't have a security labelling facility yet, he used the comment on the relation to store the security label (there are other ways it could be done too, of course). What's the difference between that and a GRANT command? GRANT is designed to allow privileges to be defined at table level. I don't see how a plugin whose only API input is a rangetable and which executes before any tuples have been touched can possibly add value here. KaiGai's had an uphill task here and I don't wish to be part of slowing him down. I'm not seeing how this moves label security forwards in any measurable way. Tom's test of a useful plugin has been one where a useful contrib module gets added at the same time. I don't think a useful plugin has been demonstrated or produced, as yet. -- Simon Riggs www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Training and Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [COMMITTERS] pgsql: Add a hook in ExecCheckRTPerms().
On Fri, 2010-07-09 at 11:09 -0400, Stephen Frost wrote: Strangely, I was looking into removing the ExecCheckRTPerms check altogether by forcing plan invalidation when permissions are updated. That would be a performance tweak that would render this change useless. I don't see how you could remove ExecCheckRTPerms..? It's what handles all permissions checking for DML (like, making sure you have SELECT rights on the table you're trying to query). I could see forcing plan invalidation when permissions are updated, sure, but that doesn't mean you can stop doing them altogether anywhere. Where would you move the permissions checking to? I apologise, when I said removing the check altogether, I meant removing from the executor path. -- Simon Riggs www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Training and Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [COMMITTERS] pgsql: Add a hook in ExecCheckRTPerms().
Simon, * Simon Riggs (si...@2ndquadrant.com) wrote: With respect, there doesn't seem to be much use case anyway. I'm sorry to be expressing that opinion now; been away for a while. I am somewhat amazed that Tom isn't dancing on your head for proposing it though. I believe it's because we've been through it with him already and explained how and why it's useful. Well, KaiGai Kohei already posted a proof-of-concept patch showing how this could be used by a simple SE-PostgreSQL implementation. Since we don't have a security labelling facility yet, he used the comment on the relation to store the security label (there are other ways it could be done too, of course). What's the difference between that and a GRANT command? GRANT is designed to allow privileges to be defined at table level. I don't see how a plugin whose only API input is a rangetable and which executes before any tuples have been touched can possibly add value here. The *hook*'s API is just a range-table- the plugin has access to a huge amount of additional information. I don't think it makes sense to try to limit that in some fashion by dictating what other information the hook is allowed to gather. Even if we did, it wouldn't be everything the hook needs since we're not going to collect the SE-Linux label from the kernel in the main backend code. The difference between this and a GRANT command would be the whole DAC vs MAC discussion and overall label-based security (this is *not* the same as *row-level* security). KaiGai's had an uphill task here and I don't wish to be part of slowing him down. I'm not seeing how this moves label security forwards in any measurable way. I'm afraid we're talking about different things here. Tom's test of a useful plugin has been one where a useful contrib module gets added at the same time. I don't think a useful plugin has been demonstrated or produced, as yet. We have a plugin which *could* be used to allow SE-Linux in the backend today- but it uses the COMMENT system, which isn't exactly ideal. Robert's already working on a patch which will add the ability to track actual labels in the catalog (using some new catalog tables which are similar to the comment tables, which is why it's not done yet, it's waiting on the get_xxx_oid() infrastructure which will greatly simplify both), which could then be queried by the module. There will be a hook there as well, which will get called whenever someone tries to modify or add a label to an object. We've also discussed new syntax for supporting those catalogs (ALTER SECURITY LABEL ON TABLE x TO y, or something like that, it's in the archives). So, no, it's not done today, but I'm certainly hopeful this will all get into 9.1 and will allow label-based security for DML with SELinux (and maybe Smack too) and this is a necessary step along the way. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] [COMMITTERS] pgsql: Add a hook in ExecCheckRTPerms().
Simon Riggs si...@2ndquadrant.com writes: On Fri, 2010-07-09 at 11:09 -0400, Tom Lane wrote: Simon Riggs si...@2ndquadrant.com writes: Strangely, I was looking into removing the ExecCheckRTPerms check altogether by forcing plan invalidation when permissions are updated. That would be a performance tweak that would render this change useless. That seems both pointless and wrong. Permissions checks should happen at execution time not plan time. Agreed that permission checks should logically be applied at execution time. I am proposing a performance optimisation, not a change in behaviour. Except that it *is* a change in behavior: the first check will occur too soon. The fact that we're interested in adding plugin permissions checking pretty much destroys the idea anyway. You cannot assume that a plan cache invalidation will happen for any change in external state that a plugin might be consulting. The proposed performance enhancement would be very useful since we have to check permissions of functions, views, tables and every other aspect. We could spend a while quantifying that overhead, though non-zero is all we need to know. No, it's not all we need to know. If you can't prove the overhead involved here is significant, we should not be expending effort and creating subtle behavioral changes in pursuit of a minor optimization. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [COMMITTERS] pgsql: Add a hook in ExecCheckRTPerms().
* Tom Lane (t...@sss.pgh.pa.us) wrote: Simon Riggs si...@2ndquadrant.com writes: Agreed that permission checks should logically be applied at execution time. I am proposing a performance optimisation, not a change in behaviour. Except that it *is* a change in behavior: the first check will occur too soon. Yeah, I have to say that I don't see any way you could avoid the behaviour change from doing this. Specifically, you can prepare plans today that you don't have access to run and then run them later after you've been granted the permission. I'm not saying that's a huge use-case or anything, but moving the checks to planner time would definitely change the behavior. No clue if any of this is covered in the SQL spec. The fact that we're interested in adding plugin permissions checking pretty much destroys the idea anyway. You cannot assume that a plan cache invalidation will happen for any change in external state that a plugin might be consulting. Yeah, this would certainly be a problem too, unless we kept the plugin hook in the executor and only used the optimization for the stock PG checks. The proposed performance enhancement would be very useful since we have to check permissions of functions, views, tables and every other aspect. We could spend a while quantifying that overhead, though non-zero is all we need to know. No, it's not all we need to know. If you can't prove the overhead involved here is significant, we should not be expending effort and creating subtle behavioral changes in pursuit of a minor optimization. Agreed. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] [COMMITTERS] pgsql: Add a hook in ExecCheckRTPerms().
On Fri, Jul 9, 2010 at 1:21 PM, Tom Lane t...@sss.pgh.pa.us wrote: Simon Riggs si...@2ndquadrant.com writes: On Fri, 2010-07-09 at 11:09 -0400, Tom Lane wrote: Simon Riggs si...@2ndquadrant.com writes: Strangely, I was looking into removing the ExecCheckRTPerms check altogether by forcing plan invalidation when permissions are updated. That would be a performance tweak that would render this change useless. That seems both pointless and wrong. Permissions checks should happen at execution time not plan time. Agreed that permission checks should logically be applied at execution time. I am proposing a performance optimisation, not a change in behaviour. Except that it *is* a change in behavior: the first check will occur too soon. You might be able to get around this by doing the first check on first use of the plan and then going and marking all the plans as needing a recheck whenever a permissions change happens. Whether the performance savings are sufficient to justify such a thing is another matter. The fact that we're interested in adding plugin permissions checking pretty much destroys the idea anyway. You cannot assume that a plan cache invalidation will happen for any change in external state that a plugin might be consulting. This is certainly true, but I also wonder what SE-PostgreSQL plans to do about this. Taking this to its logical exteme, the system security policy could change in mid-query - and while you'd like to think that the system would stop emitting tuples on a dime, that's probably not too feasible in practice. I am assuming that SE-PostgreSQL will want to do some kind of caching, but I wonder how one decides what to cache and for how long, and whether there's any mechanism for propagating cache invalidations. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise Postgres Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [COMMITTERS] pgsql: Add a hook in ExecCheckRTPerms().
On Fri, 2010-07-09 at 13:21 -0400, Tom Lane wrote: Agreed that permission checks should logically be applied at execution time. I am proposing a performance optimisation, not a change in behaviour. Except that it *is* a change in behavior: the first check will occur too soon. Sooner matters why? We already have a lock on the table at plan time so there cannot be a concurrent GRANT against a plan-then-execute transaction. Later transactions would invalidate and replan. The fact that we're interested in adding plugin permissions checking pretty much destroys the idea anyway. You cannot assume that a plan cache invalidation will happen for any change in external state that a plugin might be consulting. Plugin can still be executed at appropriate time, its mostly absent and so cheap. I guess we can keep plugin whatever else I attempt. The proposed performance enhancement would be very useful since we have to check permissions of functions, views, tables and every other aspect. We could spend a while quantifying that overhead, though non-zero is all we need to know. No, it's not all we need to know. If you can't prove the overhead involved here is significant, we should not be expending effort and creating subtle behavioral changes in pursuit of a minor optimization. OK, will gather evidence. -- Simon Riggs www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Training and Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [COMMITTERS] pgsql: Add a hook in ExecCheckRTPerms().
Robert, * Robert Haas (robertmh...@gmail.com) wrote: This is certainly true, but I also wonder what SE-PostgreSQL plans to do about this. Taking this to its logical exteme, the system security policy could change in mid-query - and while you'd like to think that the system would stop emitting tuples on a dime, that's probably not too feasible in practice. I am assuming that SE-PostgreSQL will want to do some kind of caching, but I wonder how one decides what to cache and for how long, and whether there's any mechanism for propagating cache invalidations. Yes, SE-PG will be doing cacheing and this exact problem has already been addressed (KaiGai's original SE-PG patches included cacheing, actually). It's also not something that's unique to PG in any way. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] [COMMITTERS] pgsql: Add a hook in ExecCheckRTPerms().
Simon Riggs si...@2ndquadrant.com writes: On Fri, 2010-07-09 at 13:21 -0400, Tom Lane wrote: Except that it *is* a change in behavior: the first check will occur too soon. Sooner matters why? Consider PREPARE followed only later by EXECUTE. Your proposal would make the PREPARE fail outright, when it currently does not. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [COMMITTERS] pgsql: Add a hook in ExecCheckRTPerms().
On Fri, 2010-07-09 at 14:01 -0400, Tom Lane wrote: Simon Riggs si...@2ndquadrant.com writes: On Fri, 2010-07-09 at 13:21 -0400, Tom Lane wrote: Except that it *is* a change in behavior: the first check will occur too soon. Sooner matters why? Consider PREPARE followed only later by EXECUTE. Your proposal would make the PREPARE fail outright, when it currently does not. Just to avoid wasted investigation: are you saying that is important behaviour that is essential we retain in PostgreSQL, or will you hear evidence that supporting that leads to a performance decrease elsewhere? -- Simon Riggs www.2ndQuadrant.com PostgreSQL Development, 24x7 Support, Training and Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [COMMITTERS] pgsql: Add a hook in ExecCheckRTPerms().
Simon Riggs si...@2ndquadrant.com writes: On Fri, 2010-07-09 at 14:01 -0400, Tom Lane wrote: Consider PREPARE followed only later by EXECUTE. Your proposal would make the PREPARE fail outright, when it currently does not. Just to avoid wasted investigation: are you saying that is important behaviour that is essential we retain in PostgreSQL, or will you hear evidence that supporting that leads to a performance decrease elsewhere? Well, I think that that problem makes moving the checks into the planner a nonstarter. But as somebody pointed out upthread, you could still get what you want by keeping a flag saying permission checks have been done so the executor could skip the checks on executions after the first. I'd still want to see some evidence showing that it's worth troubling over though. Premature optimization being the root of all evil, and all that. (In this case, the hazard we expose ourselves to seems to be security holes due to missed resets of the flag.) regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
