Re: Facility for detecting insecure object naming - Mailing list pgsql-hackers

On Wed, Dec 05, 2018 at 11:20:52PM -0800, Noah Misch wrote:
> 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.

Nobody else has stated an explicit preference, but here are the votes I would
expect based on other comments within this thread (feel free to correct):

Option-1: Misch
Option-4: Lane
Option-5: Haas

I would be okay with option-5, which presupposes a lexical search path for
functions.  Due to the magnitude of adding that feature, I don't expect to
attempt an implementation myself.  (Would anyone else like to?)

> 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$$;
> >