Re: Facility for detecting insecure object naming

On Thu, Aug 30, 2018 at 12:06:09AM -0700, Noah Misch wrote:
> On Sat, Aug 11, 2018 at 12:47:05PM -0700, Noah Misch wrote:
> > On Wed, Aug 08, 2018 at 09:58:38AM -0400, Tom Lane wrote:
> > > When the security team was discussing this issue before, we speculated
> > > about ideas like inventing a function trust mechanism, so that attacks
> > > based on search path manipulations would fail even if they managed to
> > > capture an operator reference.  I'd rather go down that path than
> > > encourage people to do more schema qualification.
> > 
> > Interesting.  If we got a function trust mechanism, how much qualification
> > would you then like?  Here are the levels I know about, along with their
> > implications:
> 
> Any preferences among these options and the fifth option I gave in
> https://postgr.es/m/20180815024429.GA3535710@rfd.leadboat.com?  I don't want
> to leave earthdistance broken.  So far, though, it seems no two people accept
> any one fix.

Ping.  I prefer (1) for most functions.  The loss of readability doesn't weigh
on me much, because contrib and pg_proc.dat don't have much SQL code.  I would
use (3) for ts_debug(), because that function is long, not
performance-critical, and uses only pg_catalog objects; there may be a few
other functions like that.  Implementing function trust would raise the
absolute attractiveness of (2), but I think I would continue to prefer (1).

If we can't agree on something here, (4) stands, and earthdistance functions
will continue to fail unless both the cube extension's schema and the
earthdistance extension's schema are in search_path.  That's bad, and I don't
want to prolong it.  I don't think implementing function trust or a lexical
search_path makes (4) cease to be bad.  Implementing both, however, would make
(4) non-bad.

> > -- (1) Use qualified references and exact match for all objects.
> > --
> > -- Always secure, even if schema usage does not conform to ddl-schemas-patterns
> > -- and function trust is disabled.
> > --
> > -- Subject to denial of service from anyone able to CREATE in cube schema or
> > -- earthdistance schema.
> > CREATE FUNCTION latitude(earth)
> > RETURNS float8
> > LANGUAGE SQL
> > IMMUTABLE STRICT
> > PARALLEL SAFE
> > AS $$SELECT CASE
> >   WHEN @cube_schema@.cube_ll_coord($1::@cube_schema@.cube, 3)
> >     OPERATOR(pg_catalog./)
> >     @extschema@.earth() OPERATOR(pg_catalog.<) -1 THEN -90::pg_catalog.float8
> >   WHEN @cube_schema@.cube_ll_coord($1::@cube_schema@.cube, 3)
> >     OPERATOR(pg_catalog./)
> >     @extschema@.earth() OPERATOR(pg_catalog.>)  1 THEN  90::pg_catalog.float8
> >   ELSE pg_catalog.degrees(pg_catalog.asin(@cube_schema@.cube_ll_coord(
> >     $1::@cube_schema@.cube, 3) OPERATOR(pg_catalog./) @extschema@.earth()))
> > END$$;
> > 
> > 
> > -- (2) Use qualified references for objects outside pg_catalog.
> > --
> > -- With function trust disabled, this would be subject to privilege escalation
> > -- from anyone able to CREATE in cube schema.
> > --
> > -- Subject to denial of service from anyone able to CREATE in cube schema or
> > -- earthdistance schema.
> > CREATE FUNCTION latitude(earth)
> > RETURNS float8
> > LANGUAGE SQL
> > IMMUTABLE STRICT
> > PARALLEL SAFE
> > AS $$SELECT CASE
> >   WHEN @cube_schema@.cube_ll_coord($1, 3)
> >     /
> >     @extschema@.earth() < -1 THEN -90::float8
> >   WHEN @cube_schema@.cube_ll_coord($1, 3)
> >     /
> >     @extschema@.earth() >  1 THEN  90::float8
> >   ELSE degrees(asin(@cube_schema@.cube_ll_coord($1, 3) / @extschema@.earth()))
> > END$$;
> > 
> > 
> > -- (3) "SET search_path" with today's code.
> > --
> > -- Security and reliability considerations are the same as (2).  Today, this
> > -- reduces performance by suppressing optimizations like inlining.
> > CREATE FUNCTION latitude(earth)
> > RETURNS float8
> > LANGUAGE SQL
> > IMMUTABLE STRICT
> > PARALLEL SAFE
> > SET search_path FROM CURRENT
> > AS $$SELECT CASE
> >   WHEN cube_ll_coord($1, 3)
> >     /
> >     earth() < -1 THEN -90::float8
> >   WHEN cube_ll_coord($1, 3)
> >     /
> >     earth() >  1 THEN  90::float8
> >   ELSE degrees(asin(cube_ll_coord($1, 3) / earth()))
> > END$$;
> > 
> > 
> > -- (4) Today's code (reformatted).
> > --
> > -- Always secure if schema usage conforms to ddl-schemas-patterns, even if
> > -- function trust is disabled.  If cube schema or earthdistance schema is not in
> > -- search_path, function doesn't work.
> > CREATE FUNCTION latitude(earth)
> > RETURNS float8
> > LANGUAGE SQL
> > IMMUTABLE STRICT
> > PARALLEL SAFE
> > AS $$SELECT CASE
> >   WHEN cube_ll_coord($1, 3)
> >     /
> >     earth() < -1 THEN -90::float8
> >   WHEN cube_ll_coord($1, 3)
> >     /
> >     earth() >  1 THEN  90::float8
> >   ELSE degrees(asin(cube_ll_coord($1, 3) / earth()))
> > END$$;
>