Re: Simplify ACL handling for large objects and removal of superuser() checks

Tom, Michael,

* Tom Lane (tgl(at)sss(dot)pgh(dot)pa(dot)us) wrote:
> Michael Paquier <michael(dot)paquier(at)gmail(dot)com> writes:
> > On Thu, Nov 9, 2017 at 6:05 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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