Tom, Michael, * Tom Lane (t...@sss.pgh.pa.us) wrote: > Michael Paquier <michael.paqu...@gmail.com> writes: > > On Thu, Nov 9, 2017 at 6:05 AM, Tom Lane <t...@sss.pgh.pa.us> wrote: > >> Another idea would be to invent a new external flag bit "INV_WRITE_ONLY", > >> so that people who wanted true write-only could get it, without breaking > >> backwards-compatible behavior. But I'm inclined to wait for some field > >> demand to show up before adding even that little bit of complication. > > > Demand that may never show up, and the current behavior on HEAD does > > not receive any complains either. I am keeping the patch simple for > > now. That's less aspirin needed for everybody. > > Looks good to me, pushed.
While we have been working to reduce the number of superuser() checks in the backend in favor of having the ability to GRANT explicit rights, one of the guideing principles has always been that capabilities which can be used to gain superuser rights with little effort should remain restricted to the superuser, which is why the lo_import/lo_export hadn't been under consideration for superuser-check removal in the analysis I provided previously. As such, I'm not particularly thrilled to see those checks simply just removed. If we are going to say that it's acceptable to allow non-superusers arbitrary access to files on the filesystem as the OS user then we would also be removing similar checks from adminpack, something I've also been against quite clearly in past discussions. This also didn't update the documentation regarding these functions which clearly states that superuser is required. If we are going to allow non-superusers access to these functions, we certainly need to remove the claim stating that we don't do that and further we must make it abundantly clear that these functions are extremely dangerous and could be trivially used to make someone who has access to them into a superuser. I continue to feel that this is something worthy of serious consideration and discussion, and not something to be done because we happen to be modifying the code in this area. I'm tempted to suggest we should have another role attribute or some other additional check when it comes to functions like these. The right way to allow users to be able to pull in data off the filesystem, imv, would be by providing a way to define specific locations on the filesystem which users can be granted access to import data from, as I once wrote a patch to do. That's certainly quite tricky to get correct, but that's much better than allowing non-superusers arbitrary access, which is what this does and what users may start using if we continue to remove these restrictions without providing a better option. Thanks! Stephen
signature.asc
Description: Digital signature
