Thread: why local_preload_libraries does require a separate directory ?

Hi,

why the libraries loaded using local_preload_libraries need to be placed
in a different subdirectory than libraries loaded using
shared_preload_libraries?

And why it does not use dynamic_library_path but a hardcoded path
'$libdir/plugins'?

I do understand that leaving the users to load whatever libraries they
want is a bad idea, but when the library is loaded from postgresql.conf
it should be safe.

Therefore I'd expect / propose this behaviour:

1) libs loaded from shared_preload_libraries/local_preload_libraries
  - any paths are allowed (relative and absolute)  - relative paths are resolved using dynamic_library_path if
specified,$libdir otherwise  - absolute paths are allowed, may load libraries from other locations
 

2) libs loaded using LOAD
  - check that the library is loaded from dynamic_library_path (if    specified), $libdir otherwise


AFAIK this prevents '..' type attacks and makes it easier to install
extensions (shared libs are installed to $libdir, so if you need to load
a library using local_preload_libraries, you have to copy it manually).

Tomas

Tomas Vondra <tv@fuzzy.cz> writes:
> why the libraries loaded using local_preload_libraries need to be placed
> in a different subdirectory than libraries loaded using
> shared_preload_libraries?

Security: it lets the DBA constrain which libraries are loadable this way.

> I do understand that leaving the users to load whatever libraries they
> want is a bad idea, but when the library is loaded from postgresql.conf
> it should be safe.

We don't have a mechanism that would allow different limitations to be
placed on a GUC variable depending on where the value came from.
To do what you're proposing would require restricting
local_preload_libraries to be superuser-only, which would be a net
decrease in functionality.
        regards, tom lane

On 3.12.2011 18:53, Tom Lane wrote:
> Tomas Vondra <tv@fuzzy.cz> writes:
>> why the libraries loaded using local_preload_libraries need to be placed
>> in a different subdirectory than libraries loaded using
>> shared_preload_libraries?
>
> Security: it lets the DBA constrain which libraries are loadable this way.

But local_preload_libraries can be set only in postgresql.conf, right?
As this file is maintainted by the DBA, so how does this improve the
security?

The problem I'm trying to solve right now is that I do have an extension
that needs to install two .so libraries - one loaded using
shared_preload_libraries, one loaded using local_preload_libraries.

The "make install" puts both files into $libdir, so I have to copy it to
the right directory.

>> I do understand that leaving the users to load whatever libraries they
>> want is a bad idea, but when the library is loaded from postgresql.conf
>> it should be safe.
>
> We don't have a mechanism that would allow different limitations to be
> placed on a GUC variable depending on where the value came from.
> To do what you're proposing would require restricting
> local_preload_libraries to be superuser-only, which would be a net
> decrease in functionality.

Now I'm really confused. AFAIK local_preload_libraries can be set only
from postgresql.conf, not by a user. So the check could be quite simple
- is the backend already started? No: don't check the path. Yes: check
that the path starts with '$libdir/plugin'.

Tomas

Tomas Vondra <tv@fuzzy.cz> writes:
> On 3.12.2011 18:53, Tom Lane wrote:
>> Security: it lets the DBA constrain which libraries are loadable this way.

> But local_preload_libraries can be set only in postgresql.conf, right?

No.  It's PGC_BACKEND, which means it can be set at connection start by
a client (eg, via PGOPTIONS).

> The problem I'm trying to solve right now is that I do have an extension
> that needs to install two .so libraries - one loaded using
> shared_preload_libraries, one loaded using local_preload_libraries.

Um ... why would you design it like that?
        regards, tom lane

On 4.12.2011 22:16, Tom Lane wrote:
> Tomas Vondra <tv@fuzzy.cz> writes:
>> On 3.12.2011 18:53, Tom Lane wrote:
>>> Security: it lets the DBA constrain which libraries are loadable this way.
> 
>> But local_preload_libraries can be set only in postgresql.conf, right?
> 
> No.  It's PGC_BACKEND, which means it can be set at connection start by
> a client (eg, via PGOPTIONS).
> 
>> The problem I'm trying to solve right now is that I do have an extension
>> that needs to install two .so libraries - one loaded using
>> shared_preload_libraries, one loaded using local_preload_libraries.
> 
> Um ... why would you design it like that?

Well, the purpose of the extension is to limit number of connections by
db/user/IP. It's using pg_stat_activity and auth hook, and the rules are
loaded from a file into a shared memory segment.

So I need to:

1) allocate memory to keep the rules (so it has to be loaded using  shared_preload_libraries)

2) check the rules (this is done in auth hook) using pg_stat_activity

The backends are added to pg_stat_activity after the auth hook finishes,
which means possible race conditions (backends executed at about the
same time don't see each other in pg_stat_activity). So I use an
exclusive lock that's acquired before reading pg_stat_activity and
released after the pg_stat_activity is updated.

That's the only thing the library loaded using local_preload_libraries
does - it releases the lock.

Tomas

Tomas Vondra <tv@fuzzy.cz> writes:
> On 4.12.2011 22:16, Tom Lane wrote:
>> Um ... why would you design it like that?

> The backends are added to pg_stat_activity after the auth hook finishes,
> which means possible race conditions (backends executed at about the
> same time don't see each other in pg_stat_activity). So I use an
> exclusive lock that's acquired before reading pg_stat_activity and
> released after the pg_stat_activity is updated.
> That's the only thing the library loaded using local_preload_libraries
> does - it releases the lock.

That's an unbelievably ugly, and dangerous, kluge.  All you need is one
backend not loading the second library (and remember,
local_preload_libraries is user-settable) and you've just locked up the
system.

Why are you using pg_stat_activity for this anyway?  Searching the
ProcArray seems much safer ... see CountDBBackends for an example.
        regards, tom lane

On 5.12.2011 00:06, Tom Lane wrote:
> Tomas Vondra <tv@fuzzy.cz> writes:
>> On 4.12.2011 22:16, Tom Lane wrote:
>>> Um ... why would you design it like that?
> 
>> The backends are added to pg_stat_activity after the auth hook finishes,
>> which means possible race conditions (backends executed at about the
>> same time don't see each other in pg_stat_activity). So I use an
>> exclusive lock that's acquired before reading pg_stat_activity and
>> released after the pg_stat_activity is updated.
>> That's the only thing the library loaded using local_preload_libraries
>> does - it releases the lock.
> 
> That's an unbelievably ugly, and dangerous, kluge.  All you need is one
> backend not loading the second library (and remember,
> local_preload_libraries is user-settable) and you've just locked up the
> system.

Yes, but I wasn't aware of that - I thought local_preload_libraries is
defined in postgresql.conf so it seemed fine (yes, it was ugly but it
did not seem that dangerous).

> Why are you using pg_stat_activity for this anyway?  Searching the
> ProcArray seems much safer ... see CountDBBackends for an example.

Because I've never user ProcArray before, but I use pg_stat_activity
quite frequently, so it seemed very natural. Thanks for pointing to
ProcArray/CountDBBackends, I'll see how to use that.

Tomas

On 5.12.2011 00:06, Tom Lane wrote:
> Tomas Vondra <tv@fuzzy.cz> writes:
>> On 4.12.2011 22:16, Tom Lane wrote:
>>> Um ... why would you design it like that?
> 
>> The backends are added to pg_stat_activity after the auth hook finishes,
>> which means possible race conditions (backends executed at about the
>> same time don't see each other in pg_stat_activity). So I use an
>> exclusive lock that's acquired before reading pg_stat_activity and
>> released after the pg_stat_activity is updated.
>> That's the only thing the library loaded using local_preload_libraries
>> does - it releases the lock.
> 
> That's an unbelievably ugly, and dangerous, kluge.  All you need is one
> backend not loading the second library (and remember,
> local_preload_libraries is user-settable) and you've just locked up the
> system.
> 
> Why are you using pg_stat_activity for this anyway?  Searching the
> ProcArray seems much safer ... see CountDBBackends for an example.

Thanks, reading ProcArray works fine, although the ProcArrayStruct is
private to procarray.c so I had to create a local copy. That sounds a
bit too fragile I guess - whenever it changes, the extension will break
without a warning.

Tomas