Re: Proposal: template-ify (binary) extensions - Mailing list pgsql-hackers

From Markus Wanner
Subject Re: Proposal: template-ify (binary) extensions
Date
Msg-id 51ED8818.1000002@bluegap.ch
Whole thread Raw
In response to Re: Proposal: template-ify (binary) extensions  (Dimitri Fontaine <dimitri@2ndQuadrant.fr>)
List pgsql-hackers
Dimitri,

On 07/22/2013 08:44 PM, Dimitri Fontaine wrote:
> That's the trade-off we currently need to make to be able to ship with
> the current security protections we're talking about.

Anything wrong with shipping postgis-1.5.so and postgis-2.0.so, as I we
for Debian?

> Ok, here's the full worked out example:
> 
>   ~# select lo_import('/path/to/local/on/the/client/adminpack.so');
>    lo_import 
>   -----------
>        82153
>   (1 row)
>   
>   ~# select lo_export(82153, '/tmp/foo.so');
>    lo_export 
>   -----------
>            1
>   (1 row)
>   
>   ~# LOAD '/tmp/foo.so';
>   LOAD
>   
>   ~# CREATE FUNCTION pg_catalog.pg_file_write(text, text, bool)
>      RETURNS bigint AS '/tmp/foo.so', 'pg_file_write'
>      LANGUAGE C VOLATILE STRICT;
>   CREATE FUNCTION

Good point.

Note that these lo_import() and lo_export() methods - unlike the client
counterparts - read and write to the server's file system.

So, your example is incomplete in that SELECT lo_import() doesn't load
data from the client into the database. But there are many other ways to
do that.

None the less, your point holds: lo_export() is a way for a superuser to
write arbitrary files on the server's file-system via libpq.

>> Or to put it another way: Trusted PLs exist for a good reason. And some
>> people just value security a lot and want that separation of roles.
> 
> Yeah, security is important, but it needs to be addressing real threats
> to have any value whatsoever. We'not talking about adding new big holes
> as extensions containing modules are using LANGUAGE C in their script
> and thus are already restricted to superuser. That part would not
> change.

I agree, it's not a big hole. But with security, every additional layer
counts. I think the next step is to clarify whether or not we want to
provide that guarantee.

>> As someone mentioned previously, $PGDATA may well be mounted noexec, so
>> that seems to be a bad choice.
> 
> I don't understand. You want good default security policy in place, and
> you're saying that using PGDATA allows for a really easy policy to be
> implemented by people who don't want the feature. It seems to me to be
> exactly what you want, why would that be a bad choice then?

I'm just saying that mounting $PGDATA with noexec makes $PGDATA unusable
to load libraries from; i.e. pg_ldopen() just out right fails.

And noexec is another protective layer. Building a solution that
requires $PGDATA to be mounted with exec permissions means that very
solution won't work with that protective layer. Which is a bad thing.

>> Alternatively, we could solve that problem the other way around: Rather
>> than template-ify the DSO, we could instead turn the objects created by
>> the SQL scripts into something that's more linked to the script.
>> Something that would reload as soon as the file on disk changes.
> 
> I think that's the wrong way to do it, because you generally want more
> control over those two steps (preparing the template then instanciating
> it) and you're proposing to completely lose the control and have them be
> a single step instead.

Doesn't versioning take care of that? I.e. if you've version 1.0 created
in your databases and then install 1.1 system wide, that shouldn't
immediately "instantiate".

> Use case: shipping a new plproxy bunch of functions to 256 nodes. You
> want to push the templates everywhere then just do the extension update
> as fast as possible so that it appears to be about 'in sync'.
> 
> Pro-tip: you can use a plproxy function that runs the alter extension
> update step using RUN ON ALL facility.

Contrary use case: you actually *want* to *replace* the created version
installed, as it's a plain security fix that's backwards compatible. A
plain 'apt-get upgrade' would do the job.

Anyways, I think this is bike-shedding: The question of what mental
model fits best doesn't matter too much, here.

I'm concerned about being consistent with whatever mental model we
choose and about an implementation that would work with whatever
security standards we want to adhere to.

>> (Note how this would make out-of-line extensions a lot closer to the
>> in-line variant your recent patch adds? With the dependency between
>> "template" and "instantiation"?)
> 
> Those dependencies are almost gone now except for being able to deal
> with DROP TEMPLATE FOR EXTENSION and guaranteeing we can actually
> pg_restore our pg_dump script.

I appreciate the bug fixes. However, there still is at least one
dependency. And that might be a good thing. Depending on what mental
model you follow.

> I've been changing the implementation to be what you proposed because I
> think it's making more sense (thanks!), and regression tests are
> reflecting that. My github branch is up-to-date with the last changes
> and I'm soon to send an updated patch that will be really as ready for
> commit as possible without a commiter review.

Good, thanks.

>> An attacker having access to a libpq connection with superuser rights
>> cannot currently run arbitrary native code. He can try a DOS by
> 
> I think I showed a simple way to do that above in this very email.

Correct, thanks. I think Robert isn't aware of that, for example.

> As it's all about security, reaching to an agreement strikes me as the
> right thing to do.

Agreed.

Regards

Markus



pgsql-hackers by date:

Previous
From: Robert Haas
Date:
Subject: Re: proposal - psql - show longest tables
Next
From: Andrew Gierth
Date:
Subject: Re: Review: UNNEST (and other functions) WITH ORDINALITY