Re: Restrict ALTER FUNCTION CALLED ON NULL INPUT (was Re: Not quite a security hole: CREATE LANGUAGE for non-superusers)

Tom Lane <tgl@sss.pgh.pa.us> wrote:
> "Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes:
>> We have C replication trigger functions where this would be a bad
>> thing.  They can't work properly as SECURITY INVOKER, and I see
>> it as a big step backwards in security to make the only other
>> option SECURITY DEFINER with a superuser as the owner.
> 
> Could you provide more details about that?
We have a capture_replication_data() trigger function that we attach
to each table which is to be replicated as the first AFTER EACH ROW
trigger for INSERT OR UPDATE OR DELETE.  It records the data
involved in the primitive operation against the row for logical
replication at the row level.  We don't want users to be able to
modify or even view the captured data in the replication tables
except through this function.  (It's actually a bit more complicated
than that because of transaction metadata, but the overall concept
is the same.)
We currently use the database owner for the owner of these SECURITY
DEFINER functions, but it seems to me that there could be legitimate
reasons to create a special user with more limited rights than the
database owner in some cases -- just to ensure that a mistake in the
coding of a function doesn't open up an unnecessarily large security
hole.
> If nothing else, this could be handled with a non-C wrapper
> function, but I'm not clear on the generality of the use-case.
I'm not so sure that this would work for a generalized trigger
function that can be attached to any table like this.
>> It's not too hard to come up with other use cases where you want
>> to grant one class of users rights to do something only through a
>> certain function, not directly.
> 
> Generally I'd imagine that that has something to do with
> permission to call the function, not with who owns it.
Well, it's a matter of fail-safe techniques.  You grant execute
permission for the function to a the role(s) which should be allowed
to do it only through the function.  But do you then necessarily
want the function to execute with unlimited rights, or with the most
restricted set of rights which allows it to perform the intended
function?
> Basically, if we go down the road Noah is proposing, I foresee a
> steady stream of security bugs and ensuing random restrictions on
> what function owners can do.  I do not like that future.
I do see your point, but I don't like the solution you proposed.
As I understand it, the problem Noah is trying to address is that if
we created a "replication_manager" role for owning these functions,
instead of using the database owner, that role could alter a C
function which isn't coded to handle NULL input to allow it to be
called on NULL input anyway.  Is that right?
The first solution which comes to mind for me is to allow a C
function to be compiled with that limitation -- so that *nobody*
could set the wrong option for it.  Then I realize that you already
can test for this in a C function and return NULL if any inputs are
NULL if you want to.  Rather than trying to enforce this in the
ALTER FUNCTION implementation, maybe we should just advise that if
you're going to grant ownership of a C function to a role which
might accidentally or maliciously allow it to be called with NULL
input, the C function should return NULL in that case.
-Kevin