Re: RFC: Additional Directory for Extensions

On Tue, 25 Jun 2024 at 12:12, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> wrote:
> They can mutilate the system catalogs: yes, they can TRUNCATE pg_type.
> So what? They've just destroyed their own ability to do anything else.
> The real issue here is that they can edit pg_proc to cause SQL function
> calls to call arbitrary code. But what if we limit functions so that
> the C code that they can call is located in specific places that are
> known to only contain secure code? This is easy: make sure the
> OS-installation only contains safe code in $libdir.

I wouldn't call it "easy" but I totally agree that changing pg_proc is
the main security issue that we have no easy way to tackle.

> I hear you say: ah, but they can modify dynamic_library_path, which is a
> GUC, to load code from anywhere -- especially /tmp, where the newest
> bitcoin-mining library was just written. This is true. I suggest, to
> solve this problem, that we should make dynamic_library_path no longer a
> GUC. It should be a setting that comes from a different origin, one
> that even superuser cannot write to. Only the OS-installation can
> modify that file; that way, superuser cannot load arbitrary code that
> way.

I don't think that needs a whole new file. Making this GUC be
PGC_SIGHUP/PGC_POSTMASTER + GUC_DISALLOW_IN_AUTO_FILE should be
enough. Just like was done for the new allow_alter_system GUC in PG17.

> This is where the new GUC setting being proposed in this thread rubs me
> the wrong way: it's adding yet another avenue for this to be exploited.
> I would like this new directory not to be a GUC either, just like
> dynamic_library_path.

We can make it PGC_SIGHUP/PGC_POSTMASTER + GUC_DISALLOW_IN_AUTO_FILE
too, either now or in the future.

> Now, I'm not saying that this is an easy journey. But if we don't
> start, we're not going to get there.

Sure, but it sounds like you're suggesting that you want to "start" by
not adding new features that have equivalent security holes as the
ones that we already have. I don't think that is a very helpful way to
get to a better place. It seems much more useful to tackle the current
problems that we have first, and almost certainly the same solutions
to those problems can be applied to any new features with security
issues.

It at least definitely seems the case for the proposal in this thread:
i.e. we already have a GUC that allows loading libraries from an
arbitrary location. This proposal adds another such GUC. If we solve
the security problem in that first GUC, either by
GUC_DISALLOW_IN_AUTO_FILE, or by creating a whole new mechanism for
the setting, then I see no reason why we cannot use that exact same
solution for the newly proposed GUC. So the required work to secure
postgres will not be meaningfully harder by adding this GUC.