Thread

  1. Not quite a security hole: CREATE LANGUAGE for non-superusers

    Tom Lane <tgl@sss.pgh.pa.us> — 2012-05-30T16:02:06Z

    We allow non-superuser database owners to execute CREATE LANGUAGE for a
    trusted language (one marked as tmpldbacreate in pg_pltemplate).
    Currently, the C-language support functions for the language end up owned
    by that non-superuser.  This is on the hairy edge of being a security
    hole, since generally it's supposed that a function owner can redefine the
    function.  Could the non-superuser alter the function into a state where
    it can be used unsafely?  A non-superuser cannot directly execute CREATE
    OR REPLACE FUNCTION with LANGUAGE set to C, but what can he do if he owns
    a function that already has that setting?
    
    One possible attack path is to use ALTER FUNCTION RENAME, which with a
    C language function might be thought to change the target entry point in
    the language's shared library, thus leading at least to a server crash and
    possibly to undesirable execution of C-level code.  But actually that
    won't happen, because the target is identified by pg_proc.prosrc which is
    set up from the function name at CREATE time and isn't changed by RENAME.
    Besides, the functions at issue here are created in the pg_catalog schema,
    and non-superusers do not have permission to rename anything in
    pg_catalog.
    
    One thing the owner *can* do is use ALTER FUNCTION to change secondary
    properties of the function, such as strictness, volatility, SECURITY
    DEFINER, etc.  So far as I can see, none of these properties are examined
    for a PL support function when it is used to call or validate a function
    in the language, so this doesn't constitute a security hole either.
    Still, it's not very hard to envision innocent-looking extensions to ALTER
    FUNCTION that might result in live security holes here.
    
    A different line of attack is to replace the function altogether with
    CREATE OR REPLACE FUNCTION, using a non-C-language definition.  It would
    no longer be a gateway to executing non-SQL code ... but it would still be
    referenced by the PL.  In this way the function owner could insert
    trojan-horse code that would be executed whenever somebody else tried to
    define or use a function written in the PL.  It turns out that the
    placement of the support functions in pg_catalog saves us from this too,
    but that seems rather accidental to me; it's not immediately obvious that
    a REPLACE operation on an existing object should require CREATE rights on
    the containing schema.
    
    In short, neither I nor anybody else on the PG security list have been
    able to think of an exploitable security issue here, but we're not
    entirely convinced that there isn't one.  Can anyone think of an attack
    vector we missed?
    
    As of 9.2, there is a new hazard of the same ilk, namely the constructor
    functions for range types, which are language INTERNAL and are being
    created as owned by the type's creator.  This seems potentially a worse
    hole than the CREATE LANGUAGE case, in that any random SQL user can create
    a range type, not only the database owner (who might be assumed to be at
    least somewhat trustworthy).  Furthermore, because these functions aren't
    created in pg_catalog but in the type's creation schema, the protections
    afforded to functions in pg_catalog don't help us.  I still don't see
    any exploitable security hole from ALTER FUNCTION, but it is definitely
    possible for a range type's creator to replace a constructor function with
    a trojan horse.  The significance of that is debatable though, since you
    more or less have to trust a type's creator anyway if you use any of its
    functions.  (An example is that a domain's creator can trivially insert
    trojan horse functions into the domain's CHECK constraints.)
    
    Whether or not there is a live security hole in existing releases,
    it seems clear that we could easily create one by accident in future.
    To forestall that, I suggest that we should modify CREATE LANGUAGE and
    CREATE RANGE TYPE to mark the support functions as owned by the bootstrap
    superuser, not the caller of the CREATE operation.  This would ensure that
    non-superusers couldn't muck around with the function definitions.
    (Dropping the language or type still works, since cascade deletions don't
    pay attention to who owns an object that the delete cascades to.)
    At present it seems sufficient to patch this in HEAD, along the lines of
    the attached proposed patch.
    
    If anyone can think of a workable attack against existing releases, we
    will need to back-patch some form of this change, and also advise DBAs
    to manually alter the ownership of existing language support functions.
    That would be enough of a pain in the rear that I don't want to counsel
    DBAs to do it unless there's a demonstrable need.
    
    Another point here is that if we replace the current pg_pltemplate-based
    method of creating trusted languages, we will need to be sure that the
    created C functions always end up owned by a superuser, else the problem
    comes back again.  That will be a matter to consider when we think about
    how CREATE EXTENSION works for that case.
    
    Comments?
    
    			regards, tom lane
    
    
  2. Re: Not quite a security hole: CREATE LANGUAGE for non-superusers

    Noah Misch <noah@leadboat.com> — 2012-05-30T23:34:16Z

    On Wed, May 30, 2012 at 12:02:06PM -0400, Tom Lane wrote:
    > One thing the owner *can* do is use ALTER FUNCTION to change secondary
    > properties of the function, such as strictness, volatility, SECURITY
    > DEFINER, etc.  So far as I can see, none of these properties are examined
    > for a PL support function when it is used to call or validate a function
    > in the language, so this doesn't constitute a security hole either.
    > Still, it's not very hard to envision innocent-looking extensions to ALTER
    > FUNCTION that might result in live security holes here.
    
    I wondered about ALTER FUNCTION SET guc = '...' and tried to test it:
    
    	CREATE FUNCTION f(out ret text) RETURNS text LANGUAGE plpgsql AS
    		'BEGIN ret := current_setting(''work_mem''); END';
    	ALTER FUNCTION plpgsql_call_handler() SET work_mem = '2MB';
    	SELECT f();
    
    However, that test hit a SIGSEGV with stack corruption:
    
    #0  DirectFunctionCall1Coll (func=0x477004 <hashoid>, collation=0, arg1=65551) at fmgr.c:1018
    #1  0x00000000007246c5 in CatalogCacheComputeHashValue (cache=0xc61398, nkeys=<value optimized out>, cur_skey=0x7fff026ed390) at catcache.c:210
    #2  0x00000000007257c6 in SearchCatCache (cache=0xc61398, v1=65551, v2=0, v3=0, v4=0) at catcache.c:1091
    #3  0x00000000007304e2 in SearchSysCache (cacheId=0, key1=65551, key2=0, key3=0, key4=0) at syscache.c:859
    #4  0x000000000073d382 in fmgr_info_cxt_security (functionId=0, finfo=0x7fff026ed650, mcxt=<value optimized out>, ignore_security=0 '\000') at fmgr.c:214
    #5  0x000000000073d939 in fmgr_info_cxt (functionId=4681732, finfo=0x0, mcxt=0x1000f) at fmgr.c:171
    #6  0x000000000073d94e in fmgr_info (functionId=4681732, finfo=0x0) at fmgr.c:161
    #7  0x000000000073d890 in fmgr_info_other_lang (functionId=65555, finfo=0x7fcc68bd54d8, mcxt=<value optimized out>, ignore_security=<value optimized out>) at fmgr.c:408
    #8  fmgr_info_cxt_security (functionId=65555, finfo=0x7fcc68bd54d8, mcxt=<value optimized out>, ignore_security=<value optimized out>) at fmgr.c:290
    #9  0x000000000073e8dd in fmgr_security_definer (fcinfo=0xccae20) at fmgr.c:901
    #10 0x000000000073eb24 in fmgr_security_definer (fcinfo=0x477004) at fmgr.c:968
    #11 0x000000000073eb24 in fmgr_security_definer (fcinfo=0x477004) at fmgr.c:968
    ...
    #2526 0x000000000073eb24 in fmgr_security_definer (fcinfo=0x477004) at fmgr.c:968
    ...
    
    > In short, neither I nor anybody else on the PG security list have been
    > able to think of an exploitable security issue here, but we're not
    > entirely convinced that there isn't one.  Can anyone think of an attack
    > vector we missed?
    
    Stepping outside the special case of language support functions, it follows
    that ALTER FUNCTION OWNER TO on a C-language function conveys more trust than
    meets the eye:
    
    	BEGIN;
    	CREATE ROLE alice;
    	CREATE FUNCTION mylen(text) RETURNS integer LANGUAGE internal IMMUTABLE STRICT AS 'textlen';
    	ALTER FUNCTION mylen(text) OWNER TO alice;
    	COMMIT;
    
    	SET SESSION AUTHORIZATION alice;
    	ALTER FUNCTION mylen(text) CALLED ON NULL INPUT;
    	SELECT mylen(NULL); -- SIGSEGV
    
    CREATE FUNCTION + ALTER FUNCTION OWNER TO is useful for creating another
    user's untrusted-language SECURITY DEFINER function.  ALTER FUNCTION CALLED ON
    NULL INPUT ought to require that the user be eligible to redefine the function
    completely.  (The argument types for procedural language support functions
    keep this from affecting them directly.)
    
    > Whether or not there is a live security hole in existing releases,
    > it seems clear that we could easily create one by accident in future.
    > To forestall that, I suggest that we should modify CREATE LANGUAGE and
    > CREATE RANGE TYPE to mark the support functions as owned by the bootstrap
    > superuser, not the caller of the CREATE operation.
    
    Sounds like a clear improvement for CREATE LANGUAGE, seeing those functions go
    into pg_catalog.  I'm less sure about CREATE RANGE TYPE, but I cannot think of
    a concrete reason not to do so.
    
    Thanks,
    nm
    
    
  3. Re: Not quite a security hole: CREATE LANGUAGE for non-superusers

    Tom Lane <tgl@sss.pgh.pa.us> — 2012-05-31T00:56:08Z

    Noah Misch <noah@leadboat.com> writes:
    > I wondered about ALTER FUNCTION SET guc = '...' and tried to test it:
    
    > 	CREATE FUNCTION f(out ret text) RETURNS text LANGUAGE plpgsql AS
    > 		'BEGIN ret := current_setting(''work_mem''); END';
    > 	ALTER FUNCTION plpgsql_call_handler() SET work_mem = '2MB';
    > 	SELECT f();
    
    Huh, interesting.  I coulda sworn I tried exactly that case a couple
    days ago, but I must have done something different.
    
    > However, that test hit a SIGSEGV with stack corruption:
    
    It's not so much that the stack is corrupted as that it recurses until
    the stack overflows.  fmgr_info_cxt_security sees that f() is of a
    non-builtin language, so it calls fmgr_info_other_lang, which calls
    fmgr_info for the call handler, which goes back to
    fmgr_info_cxt_security, which sees that the call handler has SET
    options, so it opts to use fmgr_security_definer for invoking the
    call handler.  But back at fmgr_info_other_lang, we expect fn_addr
    to be pointing directly at the call handler.  So instead of running
    the call handler, we run fmgr_security_definer.  It thinks that
    the function it's supposed to call is identified by
    fcinfo->flinfo->fn_oid, ie the original function f() not the call
    handler, so it now tries to call that function, and starts the
    whole lookup process over again.  Lather, rinse, repeat.
    
    So this seems to be a shortcoming in the fmgr.c stuff: it's not really
    prepared for the possibility that a PL handler function has got any
    of the attributes that would trigger use of fmgr_security_definer.
    
    I think the most expedient fix for this would be to just ignore those
    attributes, by having fmgr_info_other_lang invoke fmgr_info_cxt_security
    with ignore_security = true while looking up the PL handler.  We could
    alternatively add a test to see if we got back a pointer to
    fmgr_security_definer, but that would require doing a function pointer
    comparison which I believe to not be very reliable.
    
    So this is a security issue after all, to the extent that you can crash
    the server this way --- there's definitely no possibility of doing
    something else, since the recursion-to-overflow is certain.
    
    			regards, tom lane
    
    
  4. Restrict ALTER FUNCTION CALLED ON NULL INPUT (was Re: Not quite a security hole: CREATE LANGUAGE for non-superusers)

    Noah Misch <noah@leadboat.com> — 2012-06-11T17:19:20Z

    On Wed, May 30, 2012 at 07:34:16PM -0400, Noah Misch wrote:
    > ALTER FUNCTION OWNER TO on a C-language function conveys more trust than
    > meets the eye:
    > 
    > 	BEGIN;
    > 	CREATE ROLE alice;
    > 	CREATE FUNCTION mylen(text) RETURNS integer LANGUAGE internal IMMUTABLE STRICT AS 'textlen';
    > 	ALTER FUNCTION mylen(text) OWNER TO alice;
    > 	COMMIT;
    > 
    > 	SET SESSION AUTHORIZATION alice;
    > 	ALTER FUNCTION mylen(text) CALLED ON NULL INPUT;
    > 	SELECT mylen(NULL); -- SIGSEGV
    > 
    > CREATE FUNCTION + ALTER FUNCTION OWNER TO is useful for creating another
    > user's untrusted-language SECURITY DEFINER function.  ALTER FUNCTION CALLED ON
    > NULL INPUT ought to require that the user be eligible to redefine the function
    > completely.
    
    Here's a patch implementing that restriction.  To clarify, I see no need to
    repeat *all* the CREATE-time checks; for example, there's no need to recheck
    permission to use the return type.  The language usage check is enough.
    
    I didn't feel the need to memorialize a test like the above in an actual
    regression test, but that's the one I used to verify the change.
    
    Considering the crash potential, I'd recommend backpatching this.
    
    Thanks,
    nm
    
  5. 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> — 2012-06-12T03:03:10Z

    Noah Misch <noah@leadboat.com> writes:
    >> CREATE FUNCTION + ALTER FUNCTION OWNER TO is useful for creating another
    >> user's untrusted-language SECURITY DEFINER function.  ALTER FUNCTION CALLED ON
    >> NULL INPUT ought to require that the user be eligible to redefine the function
    >> completely.
    
    > Here's a patch implementing that restriction.  To clarify, I see no need to
    > repeat *all* the CREATE-time checks; for example, there's no need to recheck
    > permission to use the return type.  The language usage check is enough.
    
    This seems bizarre and largely unnecessary.  As you stated to begin
    with, granting ownership of a function implies some degree of trust.
    I do not want to get into the business of parsing exactly which variants
    of ALTER FUNCTION ought to be considered safe.  And I definitely don't
    want to add a check that enforces restrictions against cases that have
    got nothing whatever to do with C-language functions, as this patch
    does.
    
    			regards, tom lane
    
    
  6. Re: Restrict ALTER FUNCTION CALLED ON NULL INPUT (was Re: Not quite a security hole: CREATE LANGUAGE for non-superusers)

    Noah Misch <noah@leadboat.com> — 2012-06-12T15:31:03Z

    On Mon, Jun 11, 2012 at 11:03:10PM -0400, Tom Lane wrote:
    > Noah Misch <noah@leadboat.com> writes:
    > >> CREATE FUNCTION + ALTER FUNCTION OWNER TO is useful for creating another
    > >> user's untrusted-language SECURITY DEFINER function.  ALTER FUNCTION CALLED ON
    > >> NULL INPUT ought to require that the user be eligible to redefine the function
    > >> completely.
    > 
    > > Here's a patch implementing that restriction.  To clarify, I see no need to
    > > repeat *all* the CREATE-time checks; for example, there's no need to recheck
    > > permission to use the return type.  The language usage check is enough.
    > 
    > This seems bizarre and largely unnecessary.  As you stated to begin
    > with, granting ownership of a function implies some degree of trust.
    
    Yes, but I would never expect that level of trust to include access to crash
    the server as a consequence of the function's reliance on STRICT.
    
    > I do not want to get into the business of parsing exactly which variants
    > of ALTER FUNCTION ought to be considered safe.
    
    Fair concern.
    
    > And I definitely don't
    > want to add a check that enforces restrictions against cases that have
    > got nothing whatever to do with C-language functions, as this patch
    > does.
    
    We don't have a principled basis for assuming that this hazard cannot apply to
    third-party untrusted languages.  We could add another pg_language flag to
    make the distinction for languages like plperlu.  They're untrusted by virtue
    of granted access beyond the database, but no mismatch between the function
    definition and the function implementation can crash the server or similar.
    Adding such a thing at this point seems excessive to me.
    
    
  7. Re: Restrict ALTER FUNCTION CALLED ON NULL INPUT (was Re: Not quite a security hole: CREATE LANGUAGE for non-superusers)

    Robert Haas <robertmhaas@gmail.com> — 2012-06-12T18:50:44Z

    On Tue, Jun 12, 2012 at 11:31 AM, Noah Misch <noah@leadboat.com> wrote:
    >> > Here's a patch implementing that restriction.  To clarify, I see no need to
    >> > repeat *all* the CREATE-time checks; for example, there's no need to recheck
    >> > permission to use the return type.  The language usage check is enough.
    >>
    >> This seems bizarre and largely unnecessary.  As you stated to begin
    >> with, granting ownership of a function implies some degree of trust.
    >
    > Yes, but I would never expect that level of trust to include access to crash
    > the server as a consequence of the function's reliance on STRICT.
    
    +1.  Crashes are bad.
    
    -- 
    Robert Haas
    EnterpriseDB: http://www.enterprisedb.com
    The Enterprise PostgreSQL Company
    
    
  8. 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> — 2012-06-12T19:13:26Z

    Robert Haas <robertmhaas@gmail.com> writes:
    > On Tue, Jun 12, 2012 at 11:31 AM, Noah Misch <noah@leadboat.com> wrote:
    >>> This seems bizarre and largely unnecessary. As you stated to begin
    >>> with, granting ownership of a function implies some degree of trust.
    
    >> Yes, but I would never expect that level of trust to include access to crash
    >> the server as a consequence of the function's reliance on STRICT.
    
    > +1.  Crashes are bad.
    
    C functions, by definition, carry a risk of crashing the server.
    I cannot fathom the reasoning why we should consider that granting
    ownership of one to an untrustworthy user is ever a good idea, let alone
    something we promise to protect you from any bad consequences of.
    
    Even if I accepted that premise, this patch is a pretty bad
    implementation of it, because it restricts cases that there is no
    reason to think are unsafe.
    
    A less bizarre and considerably more future-proof restriction, IMO,
    would simply refuse any attempt to give ownership of a C function
    to a non-superuser.
    
    			regards, tom lane
    
    
  9. Re: Restrict ALTER FUNCTION CALLED ON NULL INPUT (was Re: Not quite a security hole: CREATE LANGUAGE for non-superusers)

    Kevin Grittner <kevin.grittner@wicourts.gov> — 2012-06-12T20:00:45Z

    Tom Lane <tgl@sss.pgh.pa.us> wrote:
     
    > A less bizarre and considerably more future-proof restriction,
    > IMO, would simply refuse any attempt to give ownership of a C
    > function to a non-superuser.
     
    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.  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.
     
    So there is clearly a need to support ownership of functions,
    including C functions, by users who are effectively at an
    "intermediate" level of trust.  We could conceivably use the
    database owner for that role, but that seem unnecessarily limiting.
     
    -Kevin
    
    
  10. 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> — 2012-06-12T20:14:48Z

    "Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes:
    > Tom Lane <tgl@sss.pgh.pa.us> wrote:
    >> A less bizarre and considerably more future-proof restriction,
    >> IMO, would simply refuse any attempt to give ownership of a C
    >> function to a non-superuser.
     
    > 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?  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.
    
    > 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.
    
    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.
    
    			regards, tom lane
    
    
  11. Re: Restrict ALTER FUNCTION CALLED ON NULL INPUT (was Re: Not quite a security hole: CREATE LANGUAGE for non-superusers)

    Kevin Grittner <kevin.grittner@wicourts.gov> — 2012-06-12T20:58:19Z

    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
    
    
  12. Re: Restrict ALTER FUNCTION CALLED ON NULL INPUT (was Re: Not quite a security hole: CREATE LANGUAGE for non-superusers)

    Stephen Frost <sfrost@snowman.net> — 2012-06-12T21:01:11Z

    * Tom Lane (tgl@sss.pgh.pa.us) wrote:
    > "Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes:
    > > 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.
    
    What I believe Kevin is getting at here is this:
    
    There's no way to say "run this function as user X" except by making it
    SECURITY DEFINER and owned by the user you want the function to run as.
    
    I believe everyone agrees that only a superuser should be allowed
    CHANGE a C-language function.  Unfortunately, being the 'OWNER' conveys
    more than just the ability to change the function.
    
    If we had an independent way to have the function run as a specific
    user, where that user DIDN'T own the function, I think Kevin's use case
    would be satisfied.
    
    I'm not sure if it's be reasonable for a C-language function to just go
    ahead and decide to change the user it's running as in the database..
    I have to admit that I don't think I've ever tried to do that.
    
    > 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 agree that we don't want to have to police what a function owner can
    do to a function, and therefore untrusted language functions should only
    be owned by superusers, but I feel that means we need to look at what
    function ownership currently implies and allows and consider if those
    operations should be broken out and made independently grantable.  When
    it comes to 'SECURITY DEFINER' and it's relationship to 'OWNER', I think
    we have to provide some kind of solution that doesn't require those
    functions to be run as superuser simply because the function has to be
    owned by a superuser.
    
    	Thanks,
    
    		Stephen
    
  13. 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> — 2012-06-12T21:06:55Z

    Stephen Frost <sfrost@snowman.net> writes:
    > What I believe Kevin is getting at here is this:
    
    > There's no way to say "run this function as user X" except by making it
    > SECURITY DEFINER and owned by the user you want the function to run as.
    
    > If we had an independent way to have the function run as a specific
    > user, where that user DIDN'T own the function, I think Kevin's use case
    > would be satisfied.
    
    Interesting thought.  I'm not exactly sure who should be allowed to
    apply the "RUN AS other-user" option to a function, but I can see the
    possible value of separating the right to modify the function's
    definition from the user the function runs as.  Kevin, does this seem
    like it would address your concern?
    
    			regards, tom lane
    
    
  14. Re: Restrict ALTER FUNCTION CALLED ON NULL INPUT (was Re: Not quite a security hole: CREATE LANGUAGE for non-superusers)

    Kevin Grittner <kevin.grittner@wicourts.gov> — 2012-06-12T21:08:09Z

    >Stephen Frost <sfrost@snowman.net> wrote: 
     
    > If we had an independent way to have the function run as a
    > specific user, where that user DIDN'T own the function, I think
    > Kevin's use case would be satisfied.
     
    I agree.  I'm not sure quite what that would look like, but maybe
    SECURITY ROLE <rolename> or some such could be an alternative to
    SECURITY INVOKER and SECURITY DEFINER.  (I haven't looked to see
    what the standard has here.)
     
    -Kevin
    
    
  15. 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> — 2012-06-12T21:12:23Z

    "Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes:
    > Tom Lane <tgl@sss.pgh.pa.us> wrote:
    >> 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.
    
    I'm not entirely following here.  If the function is coded in C, then
    it can pretty much do what it pleases independently of what user ID
    is thought to be executing it at the SQL level.  That would really only
    matter if you were doing some SQL stuff via the SPI interface --- and
    if that's the case, couldn't the C function set the appropriate role
    to use for itself, anyway?  (In other words, it's not that hard to build
    a "RUN AS other-user" feature into a C function, even without any support
    from the rest of the system.)
    
    > 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.
    
    Yeah, the just-code-defensively option is worth considering too.
    
    			regards, tom lane
    
    
  16. Re: Restrict ALTER FUNCTION CALLED ON NULL INPUT (was Re: Not quite a security hole: CREATE LANGUAGE for non-superusers)

    Alvaro Herrera <alvherre@commandprompt.com> — 2012-06-12T21:13:13Z

    Excerpts from Kevin Grittner's message of mar jun 12 17:08:09 -0400 2012:
    > >Stephen Frost <sfrost@snowman.net> wrote: 
    >  
    > > If we had an independent way to have the function run as a
    > > specific user, where that user DIDN'T own the function, I think
    > > Kevin's use case would be satisfied.
    >  
    > I agree.  I'm not sure quite what that would look like, but maybe
    > SECURITY ROLE <rolename> or some such could be an alternative to
    > SECURITY INVOKER and SECURITY DEFINER.  (I haven't looked to see
    > what the standard has here.)
    
    We talked briefly about this a year ago:
    http://wiki.postgresql.org/wiki/PgCon_2011_Developer_Meeting#Authorization_Issues
    (Not quite the same thing, but it's closely related).
    
    -- 
    Álvaro Herrera <alvherre@commandprompt.com>
    The PostgreSQL Company - Command Prompt, Inc.
    PostgreSQL Replication, Consulting, Custom Development, 24x7 support
    
    
  17. Re: Restrict ALTER FUNCTION CALLED ON NULL INPUT (was Re: Not quite a security hole: CREATE LANGUAGE for non-superusers)

    Stephen Frost <sfrost@snowman.net> — 2012-06-12T21:14:34Z

    Tom,
    
    * Tom Lane (tgl@sss.pgh.pa.us) wrote:
    > I'm not exactly sure who should be allowed to
    > apply the "RUN AS other-user" option to a function, but I can see the
    > possible value of separating the right to modify the function's
    > definition from the user the function runs as.
    
    When it comes to 'who can set it'- my first reaction is "the owner".
    The next question is- what rights does the owner have to have on the
    "other-user" role, and I would suggest "membership".  This could be
    extremely useful for non-C functions as well, consider this:
    
    I'm Bob.  I have an 'audit' role which is granted to me.  I'd like to
    create a function that runs as 'audit' (which has various rights granted
    to it which are less than the rights of 'Bob'), but which only I can
    modify.  If I've been granted the 'audit' role, then I can create a
    function which is owned by 'audit' (set role audit; create function
    ...), and I could make it security definer, therefore I should be able
    to create a function which is owned by me and runs as 'audit'.
    
    Writing this a bit off-the-cuff, so apologies if there are obvious flaws
    in this logic. :)
    
    	Thanks,
    
    		Stephen
    
  18. Re: Restrict ALTER FUNCTION CALLED ON NULL INPUT (was Re: Not quite a security hole: CREATE LANGUAGE for non-superusers)

    Stephen Frost <sfrost@snowman.net> — 2012-06-12T21:19:36Z

    * Tom Lane (tgl@sss.pgh.pa.us) wrote:
    > (In other words, it's not that hard to build
    > a "RUN AS other-user" feature into a C function, even without any support
    > from the rest of the system.)
    
    I was considering this and a bit concerned about what would happen if
    the C function actually did this and if we'd clean things up properly at
    the end or if the function would be required to handle that clean-up
    (if it was written as SECUURITY INVOKER, which is what's being suggested
    here)...
    
    In general, I'd certainly rather have the database handle that cleanly
    and consistently than expect my function to clean up after itself.
    
    Alvaro's point about the discussion of a stack of roles is certainly
    something else to consider, though I feel that the 'run-as' option is
    pretty straight-forward and could be done more-or-less identically to
    how we do secuirty definer now, it's just changing where we get the role
    to change to before running the function.
    
    	Thanks,
    
    		Stephen
    
  19. Re: Restrict ALTER FUNCTION CALLED ON NULL INPUT (was Re: Not quite a security hole: CREATE LANGUAGE for non-superusers)

    Kevin Grittner <kevin.grittner@wicourts.gov> — 2012-06-12T21:26:40Z

    Tom Lane <tgl@sss.pgh.pa.us> wrote:
     
    > I'm not entirely following here.  If the function is coded in C,
    > then it can pretty much do what it pleases independently of what
    > user ID is thought to be executing it at the SQL level.  That
    > would really only matter if you were doing some SQL stuff via the
    > SPI interface
     
    Yeah, there is a lot of SPI through cached prepared statements.
     
    > if that's the case, couldn't the C function set the appropriate
    > role to use for itself, anyway?
     
    I suppose it could, though I never really thought about that aspect
    of the issue before.
     
    > (In other words, it's not that hard to build a "RUN AS other-user"
    > feature into a C function, even without any support from the rest
    > of the system.)
     
    Similar to what Stephen says in his post that came through while I
    was typing this, I would be less comfortable with this being a
    hand-rolled feature of individual C functions than having some more
    systematic way to handle it.  Even if there is the possibility that
    someone could subvert the more systematic way of doing things with
    clever C code, I think the systematic approach reduces risk from
    error and would tend to make hostile code in C functions stand out
    more.  And having the information at the catalog level would make
    the intended usages easier to manage.
     
    -Kevin
    
    
  20. 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> — 2012-06-12T21:38:33Z

    Stephen Frost <sfrost@snowman.net> writes:
    > * Tom Lane (tgl@sss.pgh.pa.us) wrote:
    >> (In other words, it's not that hard to build
    >> a "RUN AS other-user" feature into a C function, even without any support
    >> from the rest of the system.)
    
    > I was considering this and a bit concerned about what would happen if
    > the C function actually did this and if we'd clean things up properly at
    > the end or if the function would be required to handle that clean-up
    > (if it was written as SECUURITY INVOKER, which is what's being suggested
    > here)...
    
    It would have to remember to restore the previous role on normal exit,
    but I believe that the system would take care of cleanup if an error is
    thrown.  Looking at fmgr_security_definer, there are just a couple of
    lines involved with changing the active role.  (There's a boatload of
    *other* crap that's been shoved into that function over time, but the
    part of it that actually does what it's named for is pretty darn small.)
    
    > Alvaro's point about the discussion of a stack of roles is certainly
    > something else to consider, though I feel that the 'run-as' option is
    > pretty straight-forward and could be done more-or-less identically to
    > how we do secuirty definer now, it's just changing where we get the role
    > to change to before running the function.
    
    Yes, it would surely not be much more code, just a bit added here:
    
            if (procedureStruct->prosecdef)
                fcache->userid = procedureStruct->proowner;
    
    			regards, tom lane
    
    
  21. Re: Restrict ALTER FUNCTION CALLED ON NULL INPUT (was Re: Not quite a security hole: CREATE LANGUAGE for non-superusers)

    Noah Misch <noah@leadboat.com> — 2012-06-14T13:50:25Z

    On Tue, Jun 12, 2012 at 03:13:26PM -0400, Tom Lane wrote:
    > Even if I accepted that premise, this patch is a pretty bad
    > implementation of it, because it restricts cases that there is no
    > reason to think are unsafe.
    > 
    > A less bizarre and considerably more future-proof restriction, IMO,
    > would simply refuse any attempt to give ownership of a C function
    > to a non-superuser.
    
    That just moves the collateral damage to a different set of victims.
    
    Hardcoding the list of vulnerable languages isn't so "future-proof".  I'd
    otherwise agree in principle if we were designing a system from scratch, but
    it doesn't fit with the need to harmoniously protect existing systems.  Adding
    this restriction now would make some existing databases fail to restore from
    dumps.  That is grave enough at any juncture, let alone for a backpatched fix.
    
    On Tue, Jun 12, 2012 at 04:14:48PM -0400, Tom Lane wrote:
    > 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.
    
    Then let's have every ALTER FUNCTION require the same language usage check as
    CREATE FUNCTION.  Or, if you insist, do so only for the hardcoded cases of
    INTERNALlanguageId and ClanguageId, and document that no third-party PL may
    rely on STRICT to the extent they do.  This of course forbids more than
    necessary but still strictly less than your proposal of blocking the original
    ownership change.
    
    nm
    
    
  22. Re: Restrict ALTER FUNCTION CALLED ON NULL INPUT (was Re: Not quite a security hole: CREATE LANGUAGE for non-superusers)

    Robert Haas <robertmhaas@gmail.com> — 2012-07-20T17:12:32Z

    On Tue, Jun 12, 2012 at 5:12 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
    > "Kevin Grittner" <Kevin.Grittner@wicourts.gov> writes:
    >> 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.
    >
    > Yeah, the just-code-defensively option is worth considering too.
    
    After rereading this thread, I think I agree with Kevin as well.  In
    order to have a problem, you have to (a) be superuser (i.e. be a
    person who already has many ways of compromising the security and
    integrity of the system), (b) decide to grant ownership of a C
    function to a non-superuser, and (c) fail to verify that the function
    is coded in a sufficiently defensive fashion to survive whatever that
    user might do with it.  So it seems plausible to simply define this
    problem as administrator error rather than a failure of security.
    
    Having said that, I do believe that answer is to some extent a
    cop-out.  It's practical to define things that way here because the
    cost of checking for NULL inputs is pretty darn small.  But suppose
    ALTER FUNCTION had a facility to change the type of a function
    argument.  Would we then insist that any C function intended to be
    used in this way must check that the type of each argument matches the
    function's expectation?  Fortunately, we don't have to decide that
    right now because ALTER FUNCTION doesn't allow that and probably never
    will.  But it would certainly be awkward if we did someday allow that,
    because now any C function that is intended to be used in this way has
    to include this extra check or be labelled insecure.  Short of
    allowing the C code to advertise the function's expectations so that
    CREATE/ALTER FUNCTION can cross-check them, I don't see a way out of
    that problem.... because on the flip side, the C code could - not to
    put too fine a point on it - be relying on just about anything.  It
    could assert that work_mem is less than 1GB (and the user could crash
    it by increasing work_mem).  It could crash on any day except Tuesday
    (we always do payroll here on Tuesdays, so why would you call it on
    any other day?).  It could erase the entire database unless it's
    marked immutable.  We would surely blame any of those failure modes on
    the person who wrote the C code, and if we make the opposite decision
    here, then I think we put ourselves in the awkward position of trying
    to adjudicate what is and is not reasonably for third parties to do in
    their C code.  I don't really want to go there, but I do think this
    points to the need for extreme caution if we ever create any more
    function properties that C coders might be tempted to rely on, or
    allow any properties that C code may already be relying on to be
    changed in new ways.
    
    I'm going to mark this patch as rejected in the CF app, which is not
    intended to foreclose debate on what to do about this, but just to
    state that there seems to be no consensus to solve this problem in the
    manner proposed.
    
    -- 
    Robert Haas
    EnterpriseDB: http://www.enterprisedb.com
    The Enterprise PostgreSQL Company
    
    
  23. 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> — 2012-07-20T17:52:20Z

    Robert Haas <robertmhaas@gmail.com> writes:
    > On Tue, Jun 12, 2012 at 5:12 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
    >> Yeah, the just-code-defensively option is worth considering too.
    
    > After rereading this thread, I think I agree with Kevin as well. ...
    > Having said that, I do believe that answer is to some extent a
    > cop-out.
    
    I agree with that --- doing nothing at all doesn't seem like the best
    option here.
    
    > ... on the flip side, the C code could - not to
    > put too fine a point on it - be relying on just about anything.
    
    And with that too.  The STRICT option is a fairly obvious security
    hazard, but who's to say there are not others?  I think it'd be easier
    to make a case for forbidding a non-superuser from altering *any*
    property of a C function.  I'd rather start from the point of allowing
    only what is clearly safe than disallowing only what is clearly unsafe.
    
    Taking a step or two back, I think that the real use-case we should
    be considering here is allowing non-superusers to own (or at least
    install) extensions that contain C functions.  We would probably want
    the non-superuser to be able to drop the extension again, maybe
    ALTER EXTENSION SET SCHEMA, maybe ALTER EXTENSION OWNER ... and likely
    not too darn much else.  Fooling with any of the contained objects
    doesn't seem like something we want to permit, since it's likely that
    something like a datatype is going to have dependencies on not just
    specific objects' properties but their interrelationships.
    
    One possible approach to that is to say that the nominal owner of such
    an extension only owns the extension itself, and ownership of the
    contained objects is held by, say, the bootstrap superuser.  There are
    other ways too of course, but this way would bypass the problem of
    figuring out how to restrict what an object's nominal owner can do
    to it.
    
    			regards, tom lane
    
    
  24. Re: Restrict ALTER FUNCTION CALLED ON NULL INPUT (was Re: Not quite a security hole: CREATE LANGUAGE for non-superusers)

    Robert Haas <robertmhaas@gmail.com> — 2012-07-20T19:39:33Z

    On Fri, Jul 20, 2012 at 1:52 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
    > Robert Haas <robertmhaas@gmail.com> writes:
    >> On Tue, Jun 12, 2012 at 5:12 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
    >>> Yeah, the just-code-defensively option is worth considering too.
    >
    >> After rereading this thread, I think I agree with Kevin as well. ...
    >> Having said that, I do believe that answer is to some extent a
    >> cop-out.
    >
    > I agree with that --- doing nothing at all doesn't seem like the best
    > option here.
    >
    >> ... on the flip side, the C code could - not to
    >> put too fine a point on it - be relying on just about anything.
    >
    > And with that too.  The STRICT option is a fairly obvious security
    > hazard, but who's to say there are not others?  I think it'd be easier
    > to make a case for forbidding a non-superuser from altering *any*
    > property of a C function.  I'd rather start from the point of allowing
    > only what is clearly safe than disallowing only what is clearly unsafe.
    
    That seems like a fairly drastic overreaction.  Are you going to ban
    renaming it or changing the owner, which are in completely different
    code paths?  Yuck.  Even if you only ban it for the main ALTER
    FUNCTION code path, it seems pretty draconian, because it looks to me
    like nearly everything else that's there is perfectly safe.  I mean,
    assuming the guy who wrote the C code didn't do anything completely
    insane or malicious, setting GUCs or whatever should be perfectly OK.
    Honestly, if you want to change something in the code, I'm not too
    convinced that there's anything better than what Noah proposed
    originally.
    
    > Taking a step or two back, I think that the real use-case we should
    > be considering here is allowing non-superusers to own (or at least
    > install) extensions that contain C functions.  We would probably want
    > the non-superuser to be able to drop the extension again, maybe
    > ALTER EXTENSION SET SCHEMA, maybe ALTER EXTENSION OWNER ... and likely
    > not too darn much else.  Fooling with any of the contained objects
    > doesn't seem like something we want to permit, since it's likely that
    > something like a datatype is going to have dependencies on not just
    > specific objects' properties but their interrelationships.
    
    Moreover, it breaks dump-and-restore.
    
    > One possible approach to that is to say that the nominal owner of such
    > an extension only owns the extension itself, and ownership of the
    > contained objects is held by, say, the bootstrap superuser.  There are
    > other ways too of course, but this way would bypass the problem of
    > figuring out how to restrict what an object's nominal owner can do
    > to it.
    
    I don't particularly care for that solution; it seems like a kludge.
    I've kind of wondered whether we ought to have checks in all the ALTER
    routines that spit up if you try to ALTER an extension member from any
    place other than an extension upgrade script...  but that still
    wouldn't prevent the extension owner from dropping the members out of
    the extension and then modifying them afterwards.  I'm not sure we
    want to prevent that in general, but maybe there could be some
    locked-down mode that has that effect.
    
    -- 
    Robert Haas
    EnterpriseDB: http://www.enterprisedb.com
    The Enterprise PostgreSQL Company
    
    
  25. 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> — 2012-07-20T19:45:15Z

    Robert Haas <robertmhaas@gmail.com> writes:
    > I don't particularly care for that solution; it seems like a kludge.
    > I've kind of wondered whether we ought to have checks in all the ALTER
    > routines that spit up if you try to ALTER an extension member from any
    > place other than an extension upgrade script...  but that still
    > wouldn't prevent the extension owner from dropping the members out of
    > the extension and then modifying them afterwards.  I'm not sure we
    > want to prevent that in general, but maybe there could be some
    > locked-down mode that has that effect.
    
    Right, I wasn't too clear about that, but I meant that we'd have some
    sort of locked-down state for an extension that would forbid fooling
    with its contents.  For development purposes, or for anybody that "knows
    what they're doing", adding/subtracting/modifying member objects is
    mighty handy.  But a non-superuser who's loaded an extension that
    contains C functions ought not have those privileges for it.
    
    			regards, tom lane
    
    
  26. Re: Restrict ALTER FUNCTION CALLED ON NULL INPUT (was Re: Not quite a security hole: CREATE LANGUAGE for non-superusers)

    Robert Haas <robertmhaas@gmail.com> — 2012-07-20T20:28:11Z

    On Fri, Jul 20, 2012 at 3:45 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
    > Robert Haas <robertmhaas@gmail.com> writes:
    >> I don't particularly care for that solution; it seems like a kludge.
    >> I've kind of wondered whether we ought to have checks in all the ALTER
    >> routines that spit up if you try to ALTER an extension member from any
    >> place other than an extension upgrade script...  but that still
    >> wouldn't prevent the extension owner from dropping the members out of
    >> the extension and then modifying them afterwards.  I'm not sure we
    >> want to prevent that in general, but maybe there could be some
    >> locked-down mode that has that effect.
    >
    > Right, I wasn't too clear about that, but I meant that we'd have some
    > sort of locked-down state for an extension that would forbid fooling
    > with its contents.  For development purposes, or for anybody that "knows
    > what they're doing", adding/subtracting/modifying member objects is
    > mighty handy.  But a non-superuser who's loaded an extension that
    > contains C functions ought not have those privileges for it.
    
    I could see having such a mode.  I'm not sure that it would eliminate
    people's desire to manually give away functions, though.  In fact,
    thinking about a couple of our customers, I'm pretty sure it wouldn't.
     Now whether it's a good idea is another question, but...
    
    -- 
    Robert Haas
    EnterpriseDB: http://www.enterprisedb.com
    The Enterprise PostgreSQL Company
    
    
  27. Re: Restrict ALTER FUNCTION CALLED ON NULL INPUT (was Re: Not quite a security hole: CREATE LANGUAGE for non-superusers)

    Noah Misch <noah@leadboat.com> — 2012-07-22T23:50:03Z

    On Fri, Jul 20, 2012 at 03:39:33PM -0400, Robert Haas wrote:
    > On Fri, Jul 20, 2012 at 1:52 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
    > > And with that too.  The STRICT option is a fairly obvious security
    > > hazard, but who's to say there are not others?  I think it'd be easier
    > > to make a case for forbidding a non-superuser from altering *any*
    > > property of a C function.  I'd rather start from the point of allowing
    > > only what is clearly safe than disallowing only what is clearly unsafe.
    > 
    > That seems like a fairly drastic overreaction.  Are you going to ban
    > renaming it or changing the owner, which are in completely different
    > code paths?  Yuck.  Even if you only ban it for the main ALTER
    > FUNCTION code path, it seems pretty draconian, because it looks to me
    > like nearly everything else that's there is perfectly safe.  I mean,
    > assuming the guy who wrote the C code didn't do anything completely
    > insane or malicious, setting GUCs or whatever should be perfectly OK.
    > Honestly, if you want to change something in the code, I'm not too
    > convinced that there's anything better than what Noah proposed
    > originally.
    
    How about a compromise of blocking GUC and STRICT changes while allowing
    everything else?  We add PGC_USERSET GUCs in most releases.  As long as
    non-superuser owners of trusted-language functions can change attached GUC
    settings, review for each new GUC really ought to consider that scenario.
    That will be easy to forget.  I'm already wary about allowing changes to GUCs
    like sql_inheritance and search_path.  By contrast, the list of ALTER FUNCTION
    alterations has grown slowly; the last addition before PostgreSQL 9.2 came in
    PostgreSQL 8.3.  Anyone implementing a new alteration will be modifying
    AlterFunction() and have ample opportunity to notice from surrounding code the
    need to identify a suitable permissions check.  Also, like you say, the other
    existing alterations are clearly safe.
    
    Thanks,
    nm