Heikki Linnakangas <hlinnakan...@vmware.com> writes:
> I'm quite worried about the security ramifications of this patch. Today, if
> you're not sure if a system has e.g sslinfo installed, you can safely just
> run "CREATE EXTENSION sslinfo". With this patch, that's no longer true,
> because "foo" might not be the extension you're looking for. Mallory

With the attached patch, you can't create a template for an extension
that is already available, so to protect against your scenario you only
have to make "sslinfo" available.

Please also note that when actually installing the "sslinfo" extension,
the one from the system will get prefered over the one from the
templates, should you have both available.

Now, I can see why you would still think it's not enough. Baring better
ideas, the current patch restricts the feature to superusers.

> Documentation doesn't build, multiple errors. In addition to the reference
> pages, there should be a section in the main docs about these templates.

I'm now working on that, setting up the documentation tool set.

>> postgres=# create template for extension myextension version '1.0' with () 
>> as 'foobar';
>> CREATE TEMPLATE FOR EXTENSION
>> postgres=# create extension myextension;
>> ERROR:  syntax error at or near "foobar"
>> LINE 1: create extension myextension;
>>         ^
>
> Confusing error message.

Introducing a syntax error in hstore--1.1.sql leads to the same message.
Even if we agree that it must be changed, I think that's for another
patch.

    ~# create extension hstore;
    ERROR:  syntax error at or near "foobar" at character 115
    STATEMENT:  create extension hstore;
    ERROR:  syntax error at or near "foobar"
    
>> postgres=# create template for extension myextension version '1.0' with () 
>> as $$create table foobar(i int4) $$;
>> CREATE TEMPLATE FOR EXTENSION
>> postgres=# create extension myextension;
>> CREATE EXTENSION
>> postgres=# select * from foobar;
>> ERROR:  relation "foobar" does not exist
>> LINE 1: select * from foobar;
>>                       ^
>
> Where did that table go?

The control->schema was not properly initialized when not given by the
user. That's fixed, and I added a regression test.

> Ah, here... Where did that "    1.0" schema come from?

Fixed too, was the same bug.

>> postgres=> create template for extension myextension version '1.0' with
>> (schema public) as $$ create function evilfunc() returns int4 AS
>> evilfunc' language internal; $$;
>> CREATE TEMPLATE FOR EXTENSION
>> postgres=> create extension myextension version '1.0';ERROR:  permission 
>> denied for language internal
>> postgres=> drop template for extension myextension version '1.0';
>> ERROR:  extension with OID 16440 does not exist
>
> Something wrong with catalog caching.

Works for me, I couldn't reproduce the bug here, working from Álvaro's
version 4 of the patch. Maybe he did already fix it, and you tested my
version 3?

>> $ make -s  install
>> /usr/bin/install: cannot stat `./hstore--1.0.sql': No such file or directory
>> make: *** [install] Error 1
>
> Installing hstore fails.

Fixed in the attached. Seeing that makes me think that you actually used
Álvaro's version 4, though.

>> postgres=> create template for extension sslinfo version '1.0' with
>> (schema public) as $$ create function evilfunc() returns int4 AS
>> evilfunc' language internal; $$;
>> ERROR:  extension "sslinfo" is already available

Expected.

>> postgres=> create template for extension sslinfo2 version '1.0' with
>> (schema public) as $$ create function evilfunc() returns int4 AS
>> evilfunc' language internal; $$;
>> CREATE TEMPLATE FOR EXTENSION
>> postgres=> alter template for extension sslinfo2 rename to sslinfo;
>> ALTER TEMPLATE FOR EXTENSION
>
> If we check for an existing extension at CREATE, should also check for that
> in ALTER ... RENAME TO.

Indeed, good catch. Fixed in the attached version 5 of the patch.

I didn't add a regression test for that case, because we need to know
which extensions are available when we try to obtain this error, and I
don't know how to do that. We could create a template for the extension
foobar, then foobar2, then rename foobar2 to foobar, but that wouldn't
exercise the same code path.

Thanks again for your reviewing,
-- 
Dimitri Fontaine
http://2ndQuadrant.fr     PostgreSQL : Expertise, Formation et Support

Attachment: templates.v5.patch.gz
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to