Re: Allow GRANT TRIGGER privilege to DROP TRIGGER (Re: Bug ##7716)

Keith Fiske <keith(at)omniti(dot)com> writes:
> I found RemoveTriggerById() in backend/commands/trigger.c which seems to
> be the code that actually drops the trigger. And I see in CreateTrigger()
> above it how to use pg_class_aclcheck() to check for valid permissions, so
> I was going to see about adding that code to the Remove section. However, I
> see no code in Remove for checking for object ownership, so I'm not sure
> how that is being enforced currently which would also have to be adjusted.

An easy way to find the code involved in any error report is to set a
breakpoint at errfinish() and then provoke the error. In this case
I get

#0 errfinish (dummy=0) at elog.c:410
#1 0x00000000004f407f in aclcheck_error (aclerr=<value optimized out>,
objectkind=ACL_KIND_CLASS, objectname=0x7f39db129d68 "bobstable")
at aclchk.c:3374
#2 0x00000000004fc8e4 in check_object_ownership (roleid=207490,
objtype=OBJECT_TRIGGER, address=..., objname=0x1e301e0,
objargs=<value optimized out>, relation=0x7f39db129b50)
at objectaddress.c:1160
#3 0x000000000055ba5d in RemoveObjects (stmt=0x1e30218) at dropcmds.c:123
#4 0x00000000006a2540 in ExecDropStmt (stmt=0x1e30218,
isTopLevel=<value optimized out>) at utility.c:1370
#5 0x00000000006a2d93 in ProcessUtilitySlow (parsetree=0x1e30218,
queryString=0x1e2f728 "drop trigger insert_oid on bobstable;",
context=PROCESS_UTILITY_TOPLEVEL, params=0x0, dest=<value optimized out>,
completionTag=0x7fff3ad9ed90 "") at utility.c:1301
#6 0x00000000006a370a in standard_ProcessUtility (parsetree=0x1e30218,
queryString=0x1e2f728 "drop trigger insert_oid on bobstable;",
context=PROCESS_UTILITY_TOPLEVEL, params=0x0, dest=0x1e30670,
completionTag=0x7fff3ad9ed90 "") at utility.c:793
#7 0x00000000006a3837 in ProcessUtility (parsetree=<value optimized out>,
queryString=<value optimized out>, context=<value optimized out>,
params=<value optimized out>, dest=<value optimized out>,
completionTag=<value optimized out>) at utility.c:311
...

A look at check_object_ownership suggests that you could take the TRIGGER
case out of the generic relation path and make it a special case that
allows either ownership or TRIGGER permission.

TBH, though, I'm not sure this is something to pursue. We discussed all
this back in 2006. As I pointed out at the time, giving somebody TRIGGER
permission is tantamount to giving them full control of your account:
http://www.postgresql.org/message-id/21827.1166115978@sss.pgh.pa.us
because they can install a trigger that will execute arbitrary code with
*your* privileges the next time you modify that table.

I think we should get rid of the separate TRIGGER privilege altogether,
not make it an even bigger security hole.

regards, tom lane