Hardening PostgreSQL via (optional) ban on local file system access

On Fri, Jun 24, 2022 at 3:08 PM Hannu Krosing <hannuk@google.com> wrote:

1) would it be enough to just disable WRITING to the filesystem (COPY
... TO ..., COPY TO ... PROGRAM ...) or are some reading functions
also potentially exploitable or at least making attackers life easier
?

I would protect read paths as well as write ones.

Though ISTM reading would need to be more fine-grained - raw filesystem
reads and system reads (i.e., something like get_raw_page(...))

2) should configuration be all-or-nothing or more fine-tunable (maybe

a comma-separated list of allowed features) ?

First pass, all-or-nothing, focus on architecture and identification.
Ideally we can then easily go in and figure out specific capabilities that
need to be enumerated should we desire. Or, as noted below, figure out how
to do a DBA administered whitelist.

3) should this be back-patched (we can provide batches for all

supported PgSQL versions)

I would love to in theory, but to do this right I suspect that the amount
of desirable refactoring would make doing so prohibitive.

4) We should likely start with this flag off, but any paranoid (read -
good and security conscious) DBA can turn it on.

In the end the vast majority of our users will have the decision as to the
default state of this decided for them by their distribution or service
provider. I'm fine with having build-from-source users get the more
permissive default.

5) Which file access functions should be in the unsafe list -

pg_current_logfile is likely safe as is pg_relation_filenode, but
pg_ls_dir likely is not. some subversions might be ok again, like
pg_ls_waldir ?
6) or should we control it via disabling the pg_*_server_* roles for
different take of configuration from 5) ?

I would suggest neither: we should funnel user-initiated access to the
filesystem, for read and write, through its own API that will simply
prevent all writes (and maybe reads) based upon this flag. If we can
somehow enforce that a C coded extension also use this API we should do so,
but IIUC that is not possible.
This basically puts things in a "default deny" mode. I do think that we
need something that permits a filesystem user to then say "except these"
(i.e., a whitelist). The thing added to the whitelist should be available
in the PostgreSQL log file when the API rejects the attempt to access the
filesystem. Unfortunately, at the moment I hit a brick wall when thinking
exactly how that could be accomplished. At least, in a minimal/zero trust
type of setup. Having the API access include the module, function, and
version making the request and having a semantic versioning based whitelist
(like, e.g., npm) would work sufficiently well in a "the only ones that get
installed on the server are trusted to play by the rules" setup.

Probably over-engineering it like I have a tendency to do, but some food
for thought nonetheless.

David J.

On Sat, Jun 25, 2022 at 12:08:13AM +0200, Hannu Krosing wrote:

As part of ongoing work on PostgreSQL security hardening we have
added a capability to disable all file system access (COPY TO/FROM
[PROGRAM] <filename>, pg_*file*() functions, lo_*() functions
accessing files, etc) in a way that can not be re-enabled without
already having access to the file system. That is via a flag which can
be set only in postgresql.conf or on the command line.

Currently the file system access is controlled via being a SUPREUSER
or having the pg_read_server_files, pg_write_server_files and
pg_execute_server_program roles. The problem with this approach is
that it will not stop an attacker who has managed to become the
PostgreSQL SUPERUSER from breaking out of the server to reading and
writing files and running programs in the surrounding container, VM or
OS.

There was some recent discussion in this area you might be interested in
[0]: /messages/by-id/20220520225619.GA876272@nathanxps13

[0]: /messages/by-id/20220520225619.GA876272@nathanxps13

--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

Hi,

On 2022-06-25 00:08:13 +0200, Hannu Krosing wrote:

Currently the file system access is controlled via being a SUPREUSER
or having the pg_read_server_files, pg_write_server_files and
pg_execute_server_program roles. The problem with this approach is
that it will not stop an attacker who has managed to become the
PostgreSQL SUPERUSER from breaking out of the server to reading and
writing files and running programs in the surrounding container, VM or
OS.

If a user has superuser rights, they automatically can execute arbitrary
code. It's that simple. Removing roles isn't going to change that. Our code
doesn't protect against C functions mismatching their SQL level
definitions. With that you can do a lot of things.

If we had had this then for example the infamous 2020 PgCrypto worm
[1] would have been much less likely to succeed.

Meh.

So here are a few questions to get discussion started.

1) would it be enough to just disable WRITING to the filesystem (COPY
... TO ..., COPY TO ... PROGRAM ...) or are some reading functions
also potentially exploitable or at least making attackers life easier
?
2) should configuration be all-or-nothing or more fine-tunable (maybe
a comma-separated list of allowed features) ?
3) should this be back-patched (we can provide batches for all
supported PgSQL versions)

Err, what?

4) We should likely start with this flag off, but any paranoid (read -
good and security conscious) DBA can turn it on.
5) Which file access functions should be in the unsafe list -
pg_current_logfile is likely safe as is pg_relation_filenode, but
pg_ls_dir likely is not. some subversions might be ok again, like
pg_ls_waldir ?
6) or should we control it via disabling the pg_*_server_* roles for
different take of configuration from 5) ?

As I said, we are happy to provide patches (and docs, etc) for all the
PostgreSQL versions we decide to support here.

I don't see anything here that provides a meaningful increase in security.

Greetings,

Andres Freund

On Sat, Jun 25, 2022 at 1:13 AM Andres Freund <andres@anarazel.de> wrote:

Hi,

On 2022-06-25 00:08:13 +0200, Hannu Krosing wrote:

Currently the file system access is controlled via being a SUPREUSER
or having the pg_read_server_files, pg_write_server_files and
pg_execute_server_program roles. The problem with this approach is
that it will not stop an attacker who has managed to become the
PostgreSQL SUPERUSER from breaking out of the server to reading and
writing files and running programs in the surrounding container, VM or
OS.

If a user has superuser rights, they automatically can execute arbitrary
code. It's that simple. Removing roles isn't going to change that. Our code
doesn't protect against C functions mismatching their SQL level
definitions. With that you can do a lot of things.

Are you claiming that one can manipulate PostgreSQL to do any file
writes directly by manipulating pg_proc to call the functions "in a
wrong way" ?

My impression was that this was largely fixed via disabling the old
direct file calling convention, but then again I did not pay much
attention at that time :)

So your suggestion would be to also include disabling access to at
least pg_proc for creating C and internal functions and possibly some
other system tables to remove this threat ?

Cheers
Hannu

On Sat, Jun 25, 2022 at 1:23 AM Hannu Krosing <hannuk@google.com> wrote:

My impression was that this was largely fixed via disabling the old
direct file calling convention, but then again I did not pay much
attention at that time :)

I meant of course direct FUNCTION calling convention (Version 0
Calling Conventions)

-- Hannu

On Fri, Jun 24, 2022 at 4:13 PM Andres Freund <andres@anarazel.de> wrote:

Hi,

On 2022-06-25 00:08:13 +0200, Hannu Krosing wrote:

Currently the file system access is controlled via being a SUPREUSER
or having the pg_read_server_files, pg_write_server_files and
pg_execute_server_program roles. The problem with this approach is
that it will not stop an attacker who has managed to become the
PostgreSQL SUPERUSER from breaking out of the server to reading and
writing files and running programs in the surrounding container, VM or
OS.

If a user has superuser rights, they automatically can execute arbitrary
code. It's that simple. Removing roles isn't going to change that. Our code
doesn't protect against C functions mismatching their SQL level
definitions. With that you can do a lot of things.

Using only psql connected by the postgres role, without touching the
filesystem to bootstrap your attack, how would this be done? If you
specify CREATE FUNCTION ... LANGUAGE c you have to supply filename
references, not a code body and you won't have been able to put that code
on the server.

We should be capable of having the core server be inescapable to the
filesystem for a superuser logged in remotely. All such access they can do
with the filesystem would be mediated by controlled code/APIs.

C-based extensions would be an issue without a solution that does provide
an inescapable sandbox aside from going through our API. Which I suspect
is basically impossible given our forked process driven execution model.

David J.

David J.

On Fri, Jun 24, 2022 at 4:13 PM Andres Freund <andres@anarazel.de> wrote:

On 2022-06-25 00:08:13 +0200, Hannu Krosing wrote:

3) should this be back-patched (we can provide batches for all
supported PgSQL versions)

Err, what?

Translation: Backpatching these changes to any stable versions will
not be acceptable (per the project versioning policy [1]https://www.postgresql.org/support/versioning/), since these
changes would be considered new feature. These changes can break
installations, if released in a minor version.

[1]: https://www.postgresql.org/support/versioning/

Best regards,
Gurjeet
http://Gurje.et

My understanding was that unless activated by admin these changes
would change nothing.

And they would be (borderline :) ) security fixes

And the versioning policy link actually does not say anything about
not adding features to older versions (I know this is the policy, just
pointing out the info in not on that page).

On Friday, June 24, 2022, Gurjeet Singh <gurjeet@singh.im> wrote:

On Fri, Jun 24, 2022 at 4:13 PM Andres Freund <andres@anarazel.de> wrote:

On 2022-06-25 00:08:13 +0200, Hannu Krosing wrote:

3) should this be back-patched (we can provide batches for all
supported PgSQL versions)

Err, what?

Translation: Backpatching these changes to any stable versions will
not be acceptable (per the project versioning policy [1]), since these
changes would be considered new feature. These changes can break
installations, if released in a minor version.

No longer having the public schema in the search_path was a feature that
got back-patched, with known bad consequences, without any way for the DBA
to voice their opinion on the matter. This proposal seems similar enough
to at least ask the question, with full DBA control and no known bad
consequences.

David J.

The old versions should definitely not have it turned on by default. I
probably was not as clear as I thought in bringing out that point..

For upcoming next ones the distributors may want to turn it on for
some more security-conscious ("enterprize") distributions.

-- Hannu

On Sat, Jun 25, 2022 at 2:08 AM David G. Johnston
<david.g.johnston@gmail.com> wrote:

(fixed your top-posting)

On Fri, Jun 24, 2022 at 4:59 PM Hannu Krosing <hannuk@google.com> wrote:

On Sat, Jun 25, 2022 at 1:46 AM Gurjeet Singh <gurjeet@singh.im> wrote:

On Fri, Jun 24, 2022 at 4:13 PM Andres Freund <andres@anarazel.de> wrote:

On 2022-06-25 00:08:13 +0200, Hannu Krosing wrote:

3) should this be back-patched (we can provide batches for all
supported PgSQL versions)

Err, what?

Translation: Backpatching these changes to any stable versions will
not be acceptable (per the project versioning policy [1]), since these
changes would be considered new feature. These changes can break
installations, if released in a minor version.

[1]: https://www.postgresql.org/support/versioning/

My understanding was that unless activated by admin these changes
would change nothing.

And they would be (borderline :) ) security fixes

And the versioning policy link actually does not say anything about
not adding features to older versions (I know this is the policy, just
pointing out the info in not on that page).

I wanted to be sure before I mentioned it, and also because I've been
away from the community for a few years [1]I'll milk the "I've been away from the community for a few years" excuse for as long as possible ;-), so I too searched the
page for any relevant mentions of the word "feature" on that page.
While you're correct that the policy does not address/prohibit
addition of new features in minor releases, but the following line
from the policy comes very close to saying it, without actually saying
it.

... PostgreSQL minor releases fix only frequently-encountered bugs, security issues, and data corruption problems to reduce the risk associated with upgrading ...

Like I recently heard a "wise one" recently say: "oh those [Postgres]
docs are totally unclear[,] but they're technically correct".

BTW, the "Translation" bit was for folks new to, or not familiar with,
community and its lingo; I'm sure you already knew what Andres meant
:-)

[1]: I'll milk the "I've been away from the community for a few years" excuse for as long as possible ;-)
excuse for as long as possible ;-)

Best regards,
Gurjeet
http://Gurje.et

Hi,

On 2022-06-25 01:23:36 +0200, Hannu Krosing wrote:

Are you claiming that one can manipulate PostgreSQL to do any file
writes directly by manipulating pg_proc to call the functions "in a
wrong way" ?

Yes.

My impression was that this was largely fixed via disabling the old
direct file calling convention, but then again I did not pay much
attention at that time :)

It got a tad harder, that's all.

So your suggestion would be to also include disabling access to at
least pg_proc for creating C and internal functions and possibly some
other system tables to remove this threat ?

No. I seriously doubt that pursuing this makes sense. Fundamentally, if you
found a way to escalate to superuser, you're superuser. Superuser can create
extensions etc. That's game over. Done.

You can of course make postgres drop a few privileges, to make it harder to
turn escalation-to-superuser into wider access to the whole system. That could
very well make sense - but of course there's quite a few things that postgres
needs to do to work, so there's significant limits to what you can do.

Greetings,

Andres Freund

(please don't top-post. Surely you've been around this community long
enough to know that)

On Sat, Jun 25, 2022 at 1:59 AM Hannu Krosing <hannuk@google.com> wrote:

My understanding was that unless activated by admin these changes
would change nothing.

That is assuming you can do this with changing just a couple of lines of
code. Which you will not be able to do. The risk of back patching something
like that even if off by default is *way* too large.

And they would be (borderline :) ) security fixes

No, they would not. Not anymore than adding a new authentication method for
example could be considered a security fix.

And the versioning policy link actually does not say anything about

not adding features to older versions (I know this is the policy, just
pointing out the info in not on that page).

Yes it does:

The PostgreSQL Global Development Group releases a new major version
containing new features about once a year. Each major version receives bug
fixes and, if need be, security fixes that are released at least once every
three months in what we call a "minor release."

And slightly further down:

While upgrading will always contain some level of risk, PostgreSQL minor
releases fix only frequently-encountered bugs, security issues, and data
corruption problems to reduce the risk associated with upgrading.

So unless you claim this is a frequently encountered bug (it's not -- it's
acting exactly has intentional), security issue (same) or data corruption
(unrelated), it should not go in a minor version. It's very clear.

--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/&gt;
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/&gt;

On Sat, Jun 25, 2022 at 1:13 AM Andres Freund <andres@anarazel.de> wrote:

On 2022-06-25 00:08:13 +0200, Hannu Krosing wrote:

Currently the file system access is controlled via being a SUPREUSER
or having the pg_read_server_files, pg_write_server_files and
pg_execute_server_program roles. The problem with this approach is
that it will not stop an attacker who has managed to become the
PostgreSQL SUPERUSER from breaking out of the server to reading and
writing files and running programs in the surrounding container, VM or
OS.

If a user has superuser rights, they automatically can execute arbitrary
code. It's that simple. Removing roles isn't going to change that. Our code
doesn't protect against C functions mismatching their SQL level
definitions. With that you can do a lot of things.

Yeah, trying to close this hole is a *very* large architectural change.

I think a much better use of time is to further reduce the *need* to use
superuser to the point that the vast majority of installations can run
without having it. For example the addition of the pg_monitor role has made
a huge difference to the number of things needing superuser access. As doe
the "grantable gucs" in 15, for example. Enumerating what remaining things
can be done safely without such access and working on turning that into
grantable permissions or roles will be a much safer way to work on the
underlying problem (which definitely is a real one), and as a bonus that
gives a more granular control over things even *beyond* just the file
system access.

--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/&gt;
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/&gt;

On 25 Jun 2022, at 03:08, Hannu Krosing <hannuk@google.com> wrote:

Currently the file system access is controlled via being a SUPREUSER

My 2 cents. Ongoing work on making superuser access unneeded seems much more relevant to me.
IMO superuser == full OS access available from postgres process. I think there's uncountable set of ways to affect OS from superuser.
E.g. you can create a TOAST value compressed by pglz that allows you to look few kilobytes before detoasted datum. Or make an archive_command = 'gcc my shell code'.
It's not even funny to invent things that you can hack as a superuser.

Best regards, Andrey Borodin.

What are your ideas of applying a change similar to above to actually
being a superuser ?

That is adding a check for "superuser being currently available" to
function superuser() in
./src/backend/utils/misc/superuser.c ?

It could be as simple as a flag that can be set only at startup for
maximum speed - though the places needing superuser check are never in
speed-critical path as far as I have seen.

Or it could be more complex and dynamix, like having a file similar to
pg_hba.conf that defines the ability to run code as superuser based on
a set of attributes each of which could be * meaning "any"

These could be
* database (to limit superuser to only certain databases)
* session user (to allow only some users to become superuser,
including via SECURITY DEFINER functions)
* backend pid (this would allow kind of 2-factor authentication -
connect, then use that session's pid to add a row to
pg_ok_to_be_sup.conf file, then continue with your superuser-y stuff)
* valid-until - this lets one allow superuser for a limited period
without fear of forgetting top disable it

This approach would have the the benefit of being very low-code while
delivering the extra protection of needing pre-existing access to
filesystem to enable/disable .

For easiest usability the pg_ok_to_be_sup.conf file should be outside
pg_reload_conf() - either just read each time the superuser() check is
run, or watched via inotify and reloaded each time it changes.

Cheers,
Hannu

P.S: - thanks Magnus for the "please don't top-post" notice - I also
need to remember to check if all the quoted mail history is left in
when I just write a replay without touching any of it. I hope it does
the right thing and leaves it out, but it just might unders some
conditions bluntly append it anyway just in case :)

Having a superuser.conf file could also be used to solve another issue
- currently, if you remove the SUPERUSER attribute from all users your
only option to get it back would be to run in single-user mode. With
conf file one could add an "always" option there which makes any
matching user superuser to fix whatever needs fixing as superuser.

Cheers
Hannu

On Sat, 2022-06-25 at 00:08 +0200, Hannu Krosing wrote:

Hi Pgsql-Hackers

As part of ongoing work on PostgreSQL security hardening we have
added a capability to disable all file system access (COPY TO/FROM
[PROGRAM] <filename>, pg_*file*() functions, lo_*() functions
accessing files, etc) in a way that can not be re-enabled without
already having access to the file system. That is via a flag which
can
be set only in postgresql.conf or on the command line.

How much of this can be done as a special extension already?

For instance, a ProcessUtility_hook can prevent superuser from
executing COPY TO/FROM PROGRAM.

As others point out, that would still leave a lot of surface area for
attacks, e.g. by manipulating the catalog. But it could be a starting
place to make attacks "harder", without core postgres needing to make
security promises that will be hard to keep.

Regards,
Jeff Davis

My current thinking is (based on more insights from Andres) that we
should also have a startup flag to disable superuser altogether to
avoid bypasses via direct manipulation of pg_proc.

Experience shows that 99% of the time one can run PostgreSQL just fine
without a superuser, so having a superuser available all the time is
kind of like leaving a loaded gun on the kitchen table because you
sometimes need to go hunting.

I am especially waiting for Andres' feedback on viability this approach.

Cheers
Hannu