Re: Key management with tests

From: Stephen Frost <sfrost(at)snowman(dot)net>
To: Bruce Momjian <bruce(at)momjian(dot)us>
Cc: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Tom Kincaid <tomjohnkincaid(at)gmail(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Andres Freund <andres(at)anarazel(dot)de>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Thomas Munro <thomas(dot)munro(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Masahiko Sawada <masahiko(dot)sawada(at)2ndquadrant(dot)com>
Subject: Re: Key management with tests
Date: 2021-02-03 15:33:57
Message-ID: 20210203153357.GF27507@tamriel.snowman.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Greetings,

* Bruce Momjian (bruce(at)momjian(dot)us) wrote:
> On Mon, Feb 1, 2021 at 07:47:57PM -0500, Bruce Momjian wrote:
> > On Mon, Feb 1, 2021 at 06:31:32PM -0500, Stephen Frost wrote:
> > > * Bruce Momjian (bruce(at)momjian(dot)us) wrote:
> > > > The purpose of cluster file encryption is to prevent users with read
> > > > access to the directories used to store database files and write-ahead
> > > > log files from being able to access the data stored in those files.
> > > > For example, when using cluster file encryption, users who have read
> > > > access to the cluster directories for backup purposes will not be able
> > > > to decrypt the data stored in these files. It also protects against
> > > > decrypted data access after media theft.
> > >
> > > That's one valid use-case and it particularly makes sense to consider,
> > > now that we support group read-access to the data cluster. The last
> >
> > Do enough people use group read-access to be useful?
>
> I am thinking group read-access might be a requirement for cluster file
> encryption to be effective.

People certainly do use group read-access, but I don't see that as being
a requirement for cluster file encryption to be effective, it's just one
thing TDE can address, among others, as discussed.

> > > line seems a bit unclear- I would update it to say:
> > > Cluster file encryption also provides data-at-rest security, protecting
> > > users from data loss should the physical media on which the cluster is
> > > stored be stolen, improperly deprovisioned (not wiped or destroyed), or
> > > otherwise ends up in the hands of an attacker.
> >
> > I have split the section into three paragraphs, trimmed down some of the
> > suggested text, and added it. Full version below.
>
> Here is an updated doc description of memory reading:
>
> This also does not protect against users who have read access to
> database process memory &mdash; all in-memory data pages and data
> encryption keys are stored unencrypted in memory, so an attacker who
> --> is able to read memory can decrypt the entire cluster. The Postgres
> --> operating system user and the operating system administrator, e.g.,
> --> the <literal>root</literal> user, have such access.

That's helpful, +1.

> > > > File system write access can allow for unauthorized file system data
> > > > decryption if the writes can be used to weaken the system's security
> > > > and this weakened system is later supplied with externally-stored keys.
> > >
> > > This isn't very clear as to exactly what the concern is or how an
> > > attacker would be able to thwart the system if they had write access to
> > > it. An attacker with write access could possibly attempt to replace the
> > > existing keys, but with the key wrapping that we're using, that should
> > > result in just a decryption failure (unless, of course, the attacker has
> > > the actual KEK that was used, but that's not terribly interesting to
> > > worry about since then they could just go access the files directly).
> >
> > Uh, well, they could modify postgresql.conf to change the script to save
> > the secret returned by the script before returning it to the PG server.
> > We could require postgresql.conf to be somewhere secure, but then how do
> > we know that is secure? I just don't see a clean solution here, but the
> > idea that you write and then wait for the key to show up seems like a
> > very valid way of attack, and it took me a while to be able to
> > articulate it.

postgresql.conf isn't always writable by the postgres user, though
postgresql.auto.conf is likely to always be. I'm not sure how much of a
concern that is, but it we wanted to take steps to explicitly address
this issue, we could have some kind of 'secure' postgresql.conf file
which we would encourage users to make owned by root and whose values
wouldn't be allowed to be overridden once set.

> Let's suppose you lock down your cluster --- the non-PGDATA files are
> owned by root, postgresql.conf and pg_hba.conf are moved out of PGDATA
> and are not writable by the database OS user, or we have the PGDATA
> directory on another server, so the adversary can only write to the
> remote PGDATA directory.
>
> What can they do? Well, they can't modify pg_proc to add a shared
> library since pg_proc is encrypted, so we have to focus on files needed
> before encryption starts or files that can't be easily encrypted.

This isn't accurate- just because it's encrypted doesn't mean they can't
modify it. That's exactly why integrity is important, because an
attacker absolutely could modify the files directly and potentially
exploit the system through those modifications.

> They could create postgresql.conf.auto in PGDATA, and modify
> cluster_key_command to capture the key, or they could modify preload
> libraries or archive command to call a command to read memory as the PG
> OS user and write the key out somewhere, or use the key to rewrite the
> database files --- those wouldn't even need a database restart, just a
> reload.

They would need to actually be able to effect that reload though. This
is where the question comes up as to just what attack vector we're
trying to address. It's certainly possible that an attacker has only
access to the stored data in an off-line fashion (eg: a hard drive that
was mistakenly thrown away without being properly wiped) and that's one
of the cases which is addressed by cluster encryption. An attacker
might have access to the LUN that PG is running on but not to the
running server itself, which it seems like is what you're contemplating
here. That's a much harder attack vector to fully protect against and
we might need to do more than we're currently contemplating to address
it- but I don't think we necessarily must solve for all cases in the
first pass at this.

> They could also modify pg_xact files so that, even though the heap/index
> files are encrypted, how the contents of those files are interpreted
> would change.

Yes, ideally, we'd encrypt/integrity check just about every part of the
running system and that's one area the patch doesn't address- things
like temporary files and other parts.

> In summary, to detect malicious user writes, you would need to protect
> the files used before encryption starts (root owned or owned by another
> user?), and encrypt all files after encryption starts --- any other
> approach would probably leave open attack vectors, and I don't think
> there is sufficient community desire to add such boundaries.

There's going to be some attack vectors that TDE doesn't address. We
should identify and document those where we're able to. We could offer
up some mitigations (eg: strongly suggest monitoring of key utilization
such that if the KEK is used without a reboot of the system or similar
happening that it is reported and someone goes to look into it). While
such mitigations aren't perfect, they can be enough to allow approval of
a system to go operational (ultimately it comes down to what the
relevant security officer is willing to accept).

> How do other database systems guarantee to detect malicious writes?

I doubt anyone would actually stipulate that they *guarantee* detection
of malicious writes, and I don't think we should either, but certainly
the other systems which provide TDE do so in a manner that provides both
confidentiality and integrity. The big O, at least, documents that they
use SHA-1 for their integrity checking, though they also provide an
option which disables it. If we used an additional fork to provide the
integrity then we could also give users the option of either having
integrity included or not.

Thanks,

Stephen

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Heikki Linnakangas 2021-02-03 15:43:47 Removing support for COPY FROM STDIN in protocol version 2
Previous Message Tom Lane 2021-02-03 15:31:38 Re: Dumping/restoring fails on inherited generated column