Thread: Proposal: template-ify (binary) extensions

Hi,

let me elaborate on an idea I had to streamline extension templates. As
I wrote in my recent review [1], I didn't have the mental model of a
template in mind for extensions, so far.

However, writing the review, I came to think of it. I certainly agree
that extensions which do not carry a binary executable can be considered
templates. The SQL scripts can be deleted after creation of the
extension in the database, because the objects created with them are an
instantiation in the database itself. They do not depend on the template.

That's not the case for compiled, executable code that usually lives in
a shared library on the filesystem. There, the "instantiation" of the
"template" merely consists of a link to the library on the file-system.
If you remove that file, you break the extension.

One way to resolve that - and that seems to be the direction Dimitri's
patch is going - is to make the extension depend on its template. (And
thereby breaking the mental model of a template, IMO. In the spirit of
that mental model, one could argue that code for stored procedures
shouldn't be duplicated, but instead just reference its ancestor.)

The other possible resolution is to make all extensions fit the template
model, i.e. make extensions independent of their templates.

This obviously requires Postgres to internalize the compiled binary code
of the library upon instantiation. (Storing the binary blobs by hash in
a shared catalog could easily prevent unwanted duplication between
databases.)

Advantages:- Consistent mental model: template- Closes the gap between how SQL scripts and binary parts of  an
extensionare handled. (Or between binary and non-binary  extensions.)- Compatibility and co-existence between
file-systemand  system catalog based extension templates is simplified.- Independence of extensions from library files
shippedby  3rd party extensions (those would only ship a template, not  the extension per se).- Paves the way to
uploadingextensions that carry a binary,  executable payload via libpq.
 

Challenges:- CPU Architecture must be taken into account for backups. Restoring  a backup on a different architecture
wouldstill require the  (external) binary code for that specific architecture, because we  cannot possibly store
binariesfor all architectures.- A bit of a mind shift for how binary extensions work.
 


Regards

Markus Wanner

On Fri, Jul 5, 2013 at 4:27 PM, Markus Wanner <markus@bluegap.ch> wrote:
> One way to resolve that - and that seems to be the direction Dimitri's
> patch is going - is to make the extension depend on its template. (And
> thereby breaking the mental model of a template, IMO. In the spirit of
> that mental model, one could argue that code for stored procedures
> shouldn't be duplicated, but instead just reference its ancestor.)

The security problems with this model have been discussed in every
email thread about the extension template feature.  There is a lot of
(well-founded, IMHO) resistance to the idea of letting users install C
code via an SQL connection.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 2013-07-14 23:49:47 -0400, Robert Haas wrote:
> On Fri, Jul 5, 2013 at 4:27 PM, Markus Wanner <markus@bluegap.ch> wrote:
> > One way to resolve that - and that seems to be the direction Dimitri's
> > patch is going - is to make the extension depend on its template. (And
> > thereby breaking the mental model of a template, IMO. In the spirit of
> > that mental model, one could argue that code for stored procedures
> > shouldn't be duplicated, but instead just reference its ancestor.)
> 
> The security problems with this model have been discussed in every
> email thread about the extension template feature.  There is a lot of
> (well-founded, IMHO) resistance to the idea of letting users install C
> code via an SQL connection.

I still fail how to see that that argument has much merits:
CREATE EXTENSION adminpack;
CREATE OR REPLACE FUNCTION pg_catalog.pg_binary_file_write(text, bytea, boolean)RETURNS bigintLANGUAGE cSTRICT
AS '$libdir/adminpack', $function$pg_file_write$function$

SELECT pg_binary_file_write('some-path.so', 'some-so-as-bytea'::bytea, 1);
LOAD '/var/lib/postgresql/9.2/main/some-path.so';

All done with the default contribs, without even a PL. It's obviously
even more trivial if you have access to plpython or plperlu. You can do
similar nastiness without even contrib installed misusing binary COPY or
possibly even using tuplesort tapes.

A superuser can execute native code as postges user. That's it.


Now, I don't think Markus proposal is a good idea on *other* grounds
though... but that's for another email.

Greetings,

Andres Freund

-- Andres Freund                       http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services

Robert,

thanks for answering. I think you responded to the (or some) idea in
general, as I didn't see any relation to the quoted part. (Otherwise
this is a hint that I've not been clear enough.)

On 07/15/2013 05:49 AM, Robert Haas wrote (in a slightly different order):
> There is a lot of
> (well-founded, IMHO) resistance to the idea of letting users install C
> code via an SQL connection.

Nowhere did I propose anything close to that. I'm in full support of the
resistance. Pitchforks and torches ready to rumble. :-)

> The security problems with this model have been discussed in every
> email thread about the extension template feature.

I read through a lot of these discussions, recently, but mostly found
misunderstanding and failure to distinguish between available extension
(template) and created extension (instantiation). I think this is a new
proposal, which I didn't ever see discussed. It does not have much, if
anything, to do with Dimitri's patch, except for it having made the
point obvious to me, as it continues to mix the two mental models.

I don't see much of a difference security wise between loading the DSO
at extension creation time vs. loading it at every backend start. More
details below.

My major gripe with the current way extensions are handled is that SQL
scripts are treated as a template, where as the DSO is only "pointed
to". Changing the SQL scripts in your extension directory has no effect
to the installed extension. Modifying the DSO file certainly has. That's
the inconsistency that I'd like to get rid of.


A security analysis of the proposal follows.

Granted, the "internalization" of the DSO is somewhat critical to
implement. Running as a non-root user, Postgres has less power than that
and can certainly not protect the internalized DSO from modification by
root. It can, however, store the DSO well protected from other users
(e.g. in a directory within $PGDATA).

Relying on the external DSO only exactly once can provide additional
safety. Consider an extensions directory that's accidentally
world-writable. As it stands, an attacker can modify the DSO and gain
control as soon as a new connection loads it. With my proposal, the
attacker would have to wait until CREATE EXTENSION. A point in time
where the newly created extension is more likely to be tested and
cross-checked.

I'm arguing both of these issues are very minor and negligible, security
wise. Baring other issues, I conclude there's no security impact by this
proposal.

Arguably, internalizing the DSO (together with the SQL) protects way
better from accidental modifications (i.e. removing the DSO by
un-installing the extension template via the distribution's packaging
system while some database still has the extension instantiated). That
clearly is a usability and not a security issue, though.

If nothing else (and beneficial in either case): Postgres should
probably check the permissions on the extension directory and at least
emit a warning, if it's world-writable. Or refuse to create (or even
load) an extension, right away.

Regards

Markus Wanner

On 2013-07-15 12:12:36 +0200, Markus Wanner wrote:
> Granted, the "internalization" of the DSO is somewhat critical to
> implement. Running as a non-root user, Postgres has less power than that
> and can certainly not protect the internalized DSO from modification by
> root. It can, however, store the DSO well protected from other users
> (e.g. in a directory within $PGDATA).

There's heaps of problems with that though. Think noexec mounts or selinux.

> Relying on the external DSO only exactly once can provide additional
> safety.

Not necessarily. Think of a distribution provided upgrade with a
security fix in an extension. On a machine with dozens of clusters. Now
you need to make sure each and every one of them has the new .so.

> Consider an extensions directory that's accidentally
> world-writable. As it stands, an attacker can modify the DSO and gain
> control as soon as a new connection loads it. With my proposal, the
> attacker would have to wait until CREATE EXTENSION. A point in time
> where the newly created extension is more likely to be tested and
> cross-checked.

I think protecting against the case where such directories are writable
is a rather bad argument. If anything screwed up those the postgres
binaries directory itself quite likely has also been changed and such.

Greetings,

Andres Freund

-- Andres Freund                       http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services

On 07/15/2013 12:05 PM, Andres Freund wrote:
> A superuser can execute native code as postges user. That's it.

Oh, I though Robert meant postgres users, i.e. non-superusers.

I hereby withdraw my pitchforks: I'm certainly not opposing to simplify
the life of superusers, who already have the power.

> Now, I don't think Markus proposal is a good idea on *other* grounds
> though... but that's for another email.

Separation of concerns, huh? Good thing, thanks :-)

Regards

Markus Wanner

On 2013-07-15 12:20:15 +0200, Markus Wanner wrote:
> On 07/15/2013 12:05 PM, Andres Freund wrote:
> > A superuser can execute native code as postges user. That's it.
> 
> Oh, I though Robert meant postgres users, i.e. non-superusers.

Oh, I am talking about *postgres* superusers ;). The example provided
upthread doesn't require 'root' permissions but only database level
superuser permissions.


> I hereby withdraw my pitchforks: I'm certainly not opposing to simplify
> the life of superusers, who already have the power.

I think what dimitri has in mind could easily be delegating the
"dangerous" work to a suid binary which does policy checking and the
real install...

Greetings,

Andres Freund

-- Andres Freund                       http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services

On 07/15/2013 12:19 PM, Andres Freund wrote:
> On 2013-07-15 12:12:36 +0200, Markus Wanner wrote:
>> Granted, the "internalization" of the DSO is somewhat critical to
>> implement. Running as a non-root user, Postgres has less power than that
>> and can certainly not protect the internalized DSO from modification by
>> root. It can, however, store the DSO well protected from other users
>> (e.g. in a directory within $PGDATA).
> 
> There's heaps of problems with that though. Think noexec mounts or selinux.

Good point.

Note, however, that "internalize" doesn't necessarily mean exactly that
approach. It could be yet another directory, even system wide, which any
distribution should well be able to prepare.

I intentionally left the "internalizing" somewhat undefined. It could
even be more equivalent to what is done with the SQL stuff, i.e. some
system catalog. But that just poses other implementation issues.
(Loading a DSO from memory doesn't sound very portable to me.)

>> Relying on the external DSO only exactly once can provide additional
>> safety.
> 
> Not necessarily. Think of a distribution provided upgrade with a
> security fix in an extension.

Ugh.. only to figure out the patched DSO is incompatible to the old
version of the SQL scripts? And therefore having to re-create the
extension, anyways? That only highlights why this difference in
treatment of SQL and DSO is troublesome.

> On a machine with dozens of clusters. Now
> you need to make sure each and every one of them has the new .so.

Agreed.

So this sounds like the other approach to unification may be more
useful: Linking the SQL scripts better and make them (re-)load from the
extensions directory, just like the DSOs.

> I think protecting against the case where such directories are writable
> is a rather bad argument.

I agree. That's why I'm neglecting he security implications and stated
there are none.

Regards

Markus Wanner

Robert,

On 07/15/2013 12:12 PM, Markus Wanner wrote:
> On 07/15/2013 05:49 AM, Robert Haas wrote (in a slightly different order):
>> There is a lot of
>> (well-founded, IMHO) resistance to the idea of letting users install C
>> code via an SQL connection.
> 
> Nowhere did I propose anything close to that. I'm in full support of the
> resistance. Pitchforks and torches ready to rumble. :-)

To clarify: In the above agreement, I had the Postgres level ordinary
users in mind, who certainly shouldn't ever be able to upload and run
arbitrary native code.

I'm not quite as enthusiastic if you meant the system level user that
happens to have superuser rights on Postgres. As Andres pointed out,
there are some more holes to plug, if you want to lock down a superuser
account. [1]

I'm not quite clear what kind of "user" you meant in your statement above.

Regards

Markus Wanner


[1]: I see the theoretical benefits of providing yet another layer of
security. Closing the existing holes would require restricting LOAD, but
that seems to suffice (given the sysadmin makes sure he doesn't
accidentally provide any of the harmful extensions).

How about the (optional?) ability to restrict LOAD to directories and
files that are only root writable? Is that enough for a root to disallow
the untrusted Postgres superuser to run arbitrary native code? Or does
that still have other holes? How many real use cases does it break? How
many does it fix?

On Mon, Jul 15, 2013 at 9:27 AM, Markus Wanner <markus@bluegap.ch> wrote:
> To clarify: In the above agreement, I had the Postgres level ordinary
> users in mind, who certainly shouldn't ever be able to upload and run
> arbitrary native code.
>
> I'm not quite as enthusiastic if you meant the system level user that
> happens to have superuser rights on Postgres. As Andres pointed out,
> there are some more holes to plug, if you want to lock down a superuser
> account. [1]
>
> I'm not quite clear what kind of "user" you meant in your statement above.

I'm talking about people who are accessing the database via SQL (with
or without superuser privileges) rather than people who are accessing
the database via the file system (regardless of which OS user they
authenticate as).  As things stand today, the only way to get machine
code to run inside the Postgres binary is to have local filesystem
access.  Andres points out that you can install adminpack to obtain
local filesystem access, and that is true.  But the system
administrator can also refuse to allow adminpack, and/or use selinux
or other mechanisms to prevent the postgres binary from writing a file
with execute permissions.

If, however, we change things so that Postgres can grab bits out of a
system catalog and map them, with execute permissions, into its own
memory space, and then jump to that address, we've effectively
installed a backdoor around those types of OS-level security measures.And we've made it quite a lot easier for a bad
guyto install a
 
rootkit within postgres.

Things aren't quite so bad if we write the bits to a file first and
then dynamically load the file.  That way at least noexec or similar
can provide protection.  But it still seems like a pretty dangerous
direction.  I'm inclined to think any such thing would have to be an
extension that administrators could enable at their own risk, rather
than something that we enabled by default.  Insufficient privilege
separation is one of the things that has made security on Microsoft
Windows such a huge problem over the last several decades; I don't
want us to emulate that.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

On 07/16/2013 01:27 AM, Robert Haas wrote:
> Andres points out that you can install adminpack to obtain
> local filesystem access, and that is true.  But the system
> administrator can also refuse to allow adminpack, and/or use selinux
> or other mechanisms to prevent the postgres binary from writing a file
> with execute permissions.

I think execute permissions (on the FS) are irrelevant. It's about
loading a shared library. The noexec mount option can prevent that, though.

But okay, you're saying we *have* and *want* a guarantee that even a
superuser cannot execute arbitrary native code via libpq (at least in
default installs w/o extensions).

Andres made two contrib-free suggestions: with COPY TO BINARY, you get a
header prepended, which I think is sufficient to prevent a dlopen() or
LoadLibrary(). Text and CSV formats of COPY escape their output, so it's
hard to write \000 or other control bytes. ESCAPE and DELIMITER also
have pretty restrictive requirements. So COPY doesn't seem quite "good"
enough to write a valid DSO.

His second suggestion was tuplesort tapes. tuplesort.c says: "We require
the first "unsigned int" of a stored tuple to be the total size on-tape
of the tuple...". That's kind of a header as well. Writing a proper DSO
certainly does not sound trivial, either.

From a security perspective, I wouldn't want to rely on that guarantee.
Postgres writes too many files to be sure none of those can be abused to
write a loadable DSO, IMO.

Mounting $PGDATA 'noexec' and allowing the postgres user to write only
to such noexec mounts sounds like a good layer. It's independent, though
- it can be used whether or not the above guarantee holds.

> Things aren't quite so bad if we write the bits to a file first and
> then dynamically load the file.  That way at least noexec or similar
> can provide protection.  But it still seems like a pretty dangerous
> direction.

I agree now. Thanks for elaborating.

Regards

Markus Wanner

On Tue, Jul 16, 2013 at 3:14 PM, Markus Wanner <markus@bluegap.ch> wrote:
> But okay, you're saying we *have* and *want* a guarantee that even a
> superuser cannot execute arbitrary native code via libpq (at least in
> default installs w/o extensions).

Yes, that's a good way of summarizing my position.  I think I'd
support having an extension that allows that, although I don't think
I'd want such an extension installed on any machine I administer.  But
I oppose having it be something the server allows by default.  YMMV.
:-)

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Markus Wanner <markus@bluegap.ch> writes:
> But okay, you're saying we *have* and *want* a guarantee that even a
> superuser cannot execute arbitrary native code via libpq (at least in
> default installs w/o extensions).

There are several problems confused into that sentence already. I think
the next step of this discussion should be about talking about the
problems we have and figuring out *if* we want to solve them, now that
we managed to explicitely say what we want as a project.
 - per-installation (not even per-cluster) DSO availability
   If you install PostGIS 1.5 on a system, then it's just impossible to   bring another cluster (of the same PostgreSQL
majorversion), let   alone database, with PostGIS 2.x, even for migration assessment   purposes. The "By Design™" part
isreally hard to explain even to   security concious users. 
 - hot standby and modules (aka DSO)
   As soon as you use some functions in 'language C' you need to   carefully watch your external dependencies ahead of
time.If you do   CREATE EXTENSION hstore;, create an hstore column and a GiST index   on it, then query the table on
thestandby… no luck. You would tell   me that it's easy enough to do and that it's part of the job, so see   next
point.
 - sysadmin vs dba, or PostgreSQL meets the Cloud
   The current model of operations is intended for places where you   have separate roles: the sysadmin cares about the
OSsetup and will   provide with system packages (compiled extensions and the like), and   DBA are never root on the OS.
Theycan CREATE EXTENSION and maybe   use the 'postgres' system account, but that's about it. 
   Given the current raise of the Cloud environements and the devops   teams, my understanding is that this model is no
longerthe only   one. Depending on who you talk to, in some circles it's not even a   relevant model anymore: user
actionsshould not require the   intervention of a "sysadmin" before hand. 
   While I appreciate that many companies still want the old behavior   that used to be the only relevant model of
operations,I think we   should also provide for the new one as it's pervasive enough now for   us to stop ignoring it
withour "I know better" smiling face. 

Now it should be possible to solve at least some of those items while
still keeping the restriction above, or with an opt-in mechanism to
enable the "works by default, but you have to solve the security
implications yourself" behaviour. A new GUC should do it, boolean,
defaults false:
 runs_in_the_cloud_with_no_security_concerns = false

I don't think the relaxed behaviour we're talking about is currently
possible to develop as an extension, by the way.

> Andres made two contrib-free suggestions: with COPY TO BINARY, you get a

Well, what about using lo_import()?

>> Things aren't quite so bad if we write the bits to a file first and
>> then dynamically load the file.  That way at least noexec or similar
>> can provide protection.  But it still seems like a pretty dangerous
>> direction.
>
> I agree now. Thanks for elaborating.

Yes it's dangerous. It's also solving real world problems that I see no
other way to solve apart from bypassing the need to ever load a DSO
file, that is embedding a retargetable C compiler in the backend.

Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr     PostgreSQL : Expertise, Formation et Support

On 07/17/2013 08:52 PM, Dimitri Fontaine wrote:
> the next step of this discussion should be about talking about the
> problems we have and figuring out *if* we want to solve them, now that
> we managed to explicitely say what we want as a project.
> 
>   - per-installation (not even per-cluster) DSO availability
> 
>     If you install PostGIS 1.5 on a system, then it's just impossible to
>     bring another cluster (of the same PostgreSQL major version), let
>     alone database, with PostGIS 2.x, even for migration assessment
>     purposes. The "By Design™" part is really hard to explain even to
>     security concious users.

On Debian, that should be well possible. Certainly installing Postgres
9.1 w/ postgis-1.5 in parallel to Postgres 9.2 w/ postgis-2.0 is. I
designed it to be.

On distributions that do not allow parallel installation of multiple
Postges major versions, it's certainly not the extension's fault.

I think I'm misunderstanding the problem statement, here.

>   - hot standby and modules (aka DSO)
> 
>     As soon as you use some functions in 'language C' you need to
>     carefully watch your external dependencies ahead of time. If you do
>     CREATE EXTENSION hstore;, create an hstore column and a GiST index
>     on it, then query the table on the standby… no luck. You would tell
>     me that it's easy enough to do and that it's part of the job, so see
>     next point.

Agreed, that's an area where Postgres could do better. I'd argue this
should be possible without relaxing the security guarantees provided,
though. Because there likely are people wanting both.

Can CREATE EXTENSION check if the standbys have the extension installed?
And refuse creation, if they don't?

>   - sysadmin vs dba, or PostgreSQL meets the Cloud
> 
>     The current model of operations is intended for places where you
>     have separate roles: the sysadmin cares about the OS setup and will
>     provide with system packages (compiled extensions and the like), and
>     DBA are never root on the OS. They can CREATE EXTENSION and maybe
>     use the 'postgres' system account, but that's about it.

I'm sure you are aware that even without this clear separation of roles,
the guarantee means we provide an additional level of security against
attackers.

>     Given the current raise of the Cloud environements and the devops
>     teams, my understanding is that this model is no longer the only
>     one. Depending on who you talk to, in some circles it's not even a
>     relevant model anymore: user actions should not require the
>     intervention of a "sysadmin" before hand.
> 
>     While I appreciate that many companies still want the old behavior
>     that used to be the only relevant model of operations, I think we
>     should also provide for the new one as it's pervasive enough now for
>     us to stop ignoring it with our "I know better" smiling face.

I'd even think it's a minority who actually uses the guarantee we're
talking about. Mostly because of the many and wide spread untrusted PLs
(which undermine the guarantee). And thus even before the rise of the cloud.

None the less, the "safe by default" has served us well, I think.

> Now it should be possible to solve at least some of those items while
> still keeping the restriction above, or with an opt-in mechanism to
> enable the "works by default, but you have to solve the security
> implications yourself" behaviour. A new GUC should do it, boolean,
> defaults false:
> 
>   runs_in_the_cloud_with_no_security_concerns = false

[ I usually associate "cloud" with (increased) "security concerns", but that's an entirely different story. ]

> I don't think the relaxed behaviour we're talking about is currently
> possible to develop as an extension, by the way.

It's extensions that undermine the guarantee, at the moment. But yeah,
it depends a lot on what kind of "relaxed behavior" you have in mind.

>> Andres made two contrib-free suggestions: with COPY TO BINARY, you get a
> 
> Well, what about using lo_import()?

That only reads from the file-system. You probably meant lo_export(),
which is writing. Although not on the server's, but only on the (libpq)
client's file-system. No threat to the server.

> Yes it's dangerous. It's also solving real world problems that I see no
> other way to solve apart from bypassing the need to ever load a DSO
> file, that is embedding a retargetable C compiler in the backend.

If the sysadmin wants to disallow arbitrary execution of native code to
postgres (the process), any kind of embedded compiler likely is equally
unwelcome.

Regards

Markus Wanner

Markus Wanner <markus@bluegap.ch> writes:
>>   - per-installation (not even per-cluster) DSO availability
>>
>>     If you install PostGIS 1.5 on a system, then it's just impossible to
>>     bring another cluster (of the same PostgreSQL major version), let
> On Debian, that should be well possible. Certainly installing Postgres
> 9.1 w/ postgis-1.5 in parallel to Postgres 9.2 w/ postgis-2.0 is. I
> designed it to be.
>
> I think I'm misunderstanding the problem statement, here.

(of the same PostgreSQL major version)

> Can CREATE EXTENSION check if the standbys have the extension installed?
> And refuse creation, if they don't?

No, because we don't register standbies so we don't know who they are,
and also because some things that we see connected and using the
replication protocol could well be pg_basebackup or pg_receivexlog.

Also, it's possible that the standby is only there for High Availability
purposes and runs no user query.

> I'm sure you are aware that even without this clear separation of roles,
> the guarantee means we provide an additional level of security against
> attackers.

Given lo_import() to upload a file from the client to the server then
LOAD with the absolute path where the file ended up imported (or any
untrusted PL really), this argument carries no sensible weight in my
opinion.

> None the less, the "safe by default" has served us well, I think.

That's true. We need to consider carefully the proposal at hand though.

It's all about allowing the backend to automatically load a file that it
finds within its own $PGDATA so that we can have per-cluster and
per-database modules (DSO files).

The only difference with before is the location where the file is read
from, and the main security danger comes from the fact that we used to
only consider root-writable places and now propose to consider postgres
bootstrap user writable places.

Having the modules in different places in the system when it's a
template and when it's instanciated allows us to solve a problem I
forgot to list:
 - upgrading an extension at the OS level
   Once you've done that, any new backend will load the newer module   (DSO file), so you have to be real quick if
installingan hot fix in   production and the SQL definition must be changed to match the new   module version… 

With the ability to "instanciate" the module in a per-cluster
per-database directory within $PGDATA the new version of the DSO module
would only put in place and loaded at ALTER EXTENSION UPDATE time.

I'm still ok with allowing to fix those problems only when a security
option that defaults to 'false' has been switched to 'true', by the way,
so that it's an opt-in, but I will admit having a hard time swallowing
the threat model we're talking about…

>> I don't think the relaxed behaviour we're talking about is currently
>> possible to develop as an extension, by the way.
>
> It's extensions that undermine the guarantee, at the moment. But yeah,
> it depends a lot on what kind of "relaxed behavior" you have in mind.

The ability to load a module from another place than the current
Dynamic_library_path is what we're talking about here, and IIUC Robert
mentioned that maybe it could be down to an extension to allow for
changing the behavior. I didn't look at that in details but I would be
surprised to be able to tweak that without having to patch the backend.

> If the sysadmin wants to disallow arbitrary execution of native code to
> postgres (the process), any kind of embedded compiler likely is equally
> unwelcome.

You already mentioned untrusted PL languages, and I don't see any
difference in between offering PL/pythonu and PL/C on security grounds,
really. I don't think we would consider changing the current model of
the "LANGUAGE C" PL, we would add a new "LANGUAGE PL/C" option, either
untrusted or with a nice sandbox.

The problem of sandboxing PL/C is that it makes it impossible to address
such use cases as uuid or PostGIS extensions even if it still allows for
hstore or other datatypes styles of extensions.

Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr     PostgreSQL : Expertise, Formation et Support

Salut Dimitri,

On 07/20/2013 01:23 AM, Dimitri Fontaine wrote:
> Markus Wanner <markus@bluegap.ch> writes:
>>>   - per-installation (not even per-cluster) DSO availability
>>>
>>>     If you install PostGIS 1.5 on a system, then it's just impossible to
>>>     bring another cluster (of the same PostgreSQL major version), let
>> On Debian, that should be well possible. Certainly installing Postgres
>> 9.1 w/ postgis-1.5 in parallel to Postgres 9.2 w/ postgis-2.0 is. I
>> designed it to be.
>>
>> I think I'm misunderstanding the problem statement, here.
> 
> (of the same PostgreSQL major version)

Not sure what the issue is, here, but I agree that should be possible.

>> Can CREATE EXTENSION check if the standbys have the extension installed?
>> And refuse creation, if they don't?
> 
> No, because we don't register standbies so we don't know who they are,
> and also because some things that we see connected and using the
> replication protocol could well be pg_basebackup or pg_receivexlog.

Can the standby check? In any case, these seem to be problems we can
solve without affecting security.

> Also, it's possible that the standby is only there for High Availability
> purposes and runs no user query.

Requiring the sysadmin to install the extensions there, too, seems
justified to me. Sounds like good advice, anyways.

>> I'm sure you are aware that even without this clear separation of roles,
>> the guarantee means we provide an additional level of security against
>> attackers.
> 
> Given lo_import() to upload a file from the client to the server then
> LOAD with the absolute path where the file ended up imported (or any
> untrusted PL really), this argument carries no sensible weight in my
> opinion.

lo_import() won't write a file for LOAD to load. An untrusted PL (or any
other extension allowing the superuser to do that) is currently required
to do that.

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.

>> None the less, the "safe by default" has served us well, I think.
> 
> That's true. We need to consider carefully the proposal at hand though.
> 
> It's all about allowing the backend to automatically load a file that it
> finds within its own $PGDATA so that we can have per-cluster and
> per-database modules (DSO files).

As someone mentioned previously, $PGDATA may well be mounted noexec, so
that seems to be a bad choice.

> The only difference with before is the location where the file is read
> from, and the main security danger comes from the fact that we used to
> only consider root-writable places and now propose to consider postgres
> bootstrap user writable places.

FWIW, I only proposed to let postgres check write permissions on
libraries it loads. IIUC we don't currently do that, yet. And Postgres
happily loads a world-writable library, ATM.

> Having the modules in different places in the system when it's a
> template and when it's instanciated allows us to solve a problem I
> forgot to list:
> 
>   - upgrading an extension at the OS level
> 
>     Once you've done that, any new backend will load the newer module
>     (DSO file), so you have to be real quick if installing an hot fix in
>     production and the SQL definition must be changed to match the new
>     module version…

I agree, that's a problem.

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.

(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"?)

> With the ability to "instanciate" the module in a per-cluster
> per-database directory within $PGDATA the new version of the DSO module
> would only put in place and loaded at ALTER EXTENSION UPDATE time.
> 
> I'm still ok with allowing to fix those problems only when a security
> option that defaults to 'false' has been switched to 'true', by the way,
> so that it's an opt-in,

Okay, good.

For the issues you raised, I'd clearly prefer fixes that maintain
current security standards, though.

> but I will admit having a hard time swallowing
> the threat model we're talking about…

An attacker having access to a libpq connection with superuser rights
cannot currently run arbitrary native code. He can try a DOS by
exhausting system resources, but that's pretty far from being
invisible. Or he can delete valuable data. Maybe other nasty things. But
he should not be able to gain root access and remove its traces.

Dropping this barrier by installing an untrusted PL (or equally insecure
extensions), an attacker with superuser rights can trivially gain
root.

Of course, an attacker shouldn't gain superuser rights in the first
place. But if he did, you better stop him right there with yet another
fence.

>> It's extensions that undermine the guarantee, at the moment. But yeah,
>> it depends a lot on what kind of "relaxed behavior" you have in mind.
> 
> The ability to load a module from another place than the current
> Dynamic_library_path is what we're talking about here, and IIUC Robert
> mentioned that maybe it could be down to an extension to allow for
> changing the behavior. I didn't look at that in details but I would be
> surprised to be able to tweak that without having to patch the backend.

Well, an extension can certainly perform something akin to pg_dlopen().
And thus load pretty much any code it wants. However, I don't think an
extension can hook into CREATE EXTENSION, yet.

>> If the sysadmin wants to disallow arbitrary execution of native code to
>> postgres (the process), any kind of embedded compiler likely is equally
>> unwelcome.
> 
> You already mentioned untrusted PL languages, and I don't see any
> difference in between offering PL/pythonu and PL/C on security grounds,
> really.

I agree. However, this also means that any kind of solution it offers is
not a good one for the security conscious sysadmin.

Regards

Markus Wanner

On 07/21/2013 10:30 PM, Markus Wanner wrote:
>> but I will admit having a hard time swallowing
>> the threat model we're talking about…
> An attacker having access to a libpq connection with superuser rights
> cannot currently run arbitrary native code. He can try a DOS by
> exhausting system resources, but that's pretty far from being
> invisible. Or he can delete valuable data. Maybe other nasty things. But
> he should not be able to gain root access and remove its traces.
>
> Dropping this barrier by installing an untrusted PL (or equally insecure
> extensions), an attacker with superuser rights can trivially gain
> root.
Could you elaborate ?

This is equivalent to claiming that any linux user can trivially gain root.

>>> If the sysadmin wants to disallow arbitrary execution of native code to
>>> postgres (the process), any kind of embedded compiler likely is equally
>>> unwelcome.
>> You already mentioned untrusted PL languages, and I don't see any
>> difference in between offering PL/pythonu and PL/C on security grounds,
>> really.
> I agree. However, this also means that any kind of solution it offers is
> not a good one for the security conscious sysadmin.
This is usually the case with a "security conscious sysadmin" - they very
seldom want to install anything.

A "cloud style"  solution to this problem is installing the whole
PostgreSQL
host in its own VM and deklegate all security to developers ;)
>
> Regards
>
> Markus Wanner
>
>


-- 
Hannu Krosing
PostgreSQL Consultant
Performance, Scalability and High Availability
2ndQuadrant Nordic OÜ

On 07/22/2013 12:11 AM, Hannu Krosing wrote:
>> Dropping this barrier by installing an untrusted PL (or equally insecure
>> extensions), an attacker with superuser rights can trivially gain
>> root.
> Could you elaborate ?
> 
> This is equivalent to claiming that any linux user can trivially gain root.

Uh. Sorry, you're of course right, the attacker can only gain postgres
rights in that case. Thanks for correcting.

The point still holds. It's another layer that an attacker would have to
overcome.

>>> You already mentioned untrusted PL languages, and I don't see any
>>> difference in between offering PL/pythonu and PL/C on security grounds,
>>> really.
>> I agree. However, this also means that any kind of solution it offers is
>> not a good one for the security conscious sysadmin.
> This is usually the case with a "security conscious sysadmin" - they very
> seldom want to install anything.

Exactly. That's why I'm favoring solutions that don't require any
extension and keep the guarantee of preventing arbitrary native code.

Regards

Markus Wanner

Markus Wanner <markus@bluegap.ch> writes:
>>>>   - per-installation (not even per-cluster) DSO availability
>
> Not sure what the issue is, here, but I agree that should be possible.

For any extension where the new package version is shipping the same .so
file name, you can only have one module on the whole system for a given
PostgreSQL major version. The extensions are per-database, the DSO
modules are per major-version (not even per-cluster).

That's the trade-off we currently need to make to be able to ship with
the current security protections we're talking about.

> Requiring the sysadmin to install the extensions there, too, seems
> justified to me. Sounds like good advice, anyways.

Yes, installing the same extensions on any standby as the ones installed
on the primary should be *required*. My idea to enforce that would be
with using Event Triggers on the standby, but how to be able to run them
when when the standby replay currently isn't opening a session is quite
a problem.

The other way to address that is of course PL/C.

> lo_import() won't write a file for LOAD to load. An untrusted PL (or any
> other extension allowing the superuser to do that) is currently required
> to do that.

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

> 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?

>>   - upgrading an extension at the OS level
>>
>>     Once you've done that, any new backend will load the newer module
>>     (DSO file), so you have to be real quick if installing an hot fix in
>>     production and the SQL definition must be changed to match the new
>>     module version…
>
> I agree, that's a problem.
>
> 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.

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.

> (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'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.

> 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.

> Well, an extension can certainly perform something akin to pg_dlopen().
> And thus load pretty much any code it wants. However, I don't think an
> extension can hook into CREATE EXTENSION, yet.

When you call a function, its MODULE_PATHNAME is getting loaded by the
backend. You would need to ship a dumb DSO module file so that this
works and arrange for the real DSO module file to be loaded via a hook
or an event trigger or something else (e.g. local_preload_libraries).

I don't see a way to be able to do that without changing the extension's
binary package, either replacing MODULE_PATHNAME in the sql script or in
the control file, or changing the DSO file itself and shipping the
genuine one in a new place.

In short, doing something different than the backend is currently doing
with respect to DSO modules looks like a core patch to me, and requires
a community agreement on the way to address the stated problems.

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

> I agree. However, this also means that any kind of solution it offers is
> not a good one for the security conscious sysadmin.

Maybe we'll be able to find a way to offer PL/C in a "trusted" way.

Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr     PostgreSQL : Expertise, Formation et Support

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

On 07/16/2013 09:14 PM, I wrote:
> But okay, you're saying we *have* and *want* a guarantee that even a
> superuser cannot execute arbitrary native code via libpq (at least in
> default installs w/o extensions).

I stand corrected and have to change my position, again. For the record:

We do not have such a guarantee. Nor does it seem reasonable to want
one. On a default install, it's well possible for the superuser to run
arbitrary code via just libpq.

There are various ways to do it, but the simplest one I was shown is:- upload a DSO from the client into a large
object-SELECT lo_export() that LO to a file on the server- LOAD it
 

There are a couple other options, so even if we let LOAD perform
permission checks (as I proposed before in this thread), the superuser
can still fiddle with function definitions. To the point that it doesn't
seem reasonable to try to protect against that.

Thus, the argument against the original proposal based on security
grounds is moot. Put another way: There already are a couple of
"backdoors" a superuser can use. By default. Or with plpgsql removed.

Thanks to Dimitri and Andres for patiently explaining and providing
examples.

Regards

Markus Wanner