Re: Key management with tests

Greetings,

* Masahiko Sawada (sawada(dot)mshk(at)gmail(dot)com) wrote:
> On Fri, Jan 29, 2021 at 5:22 AM Bruce Momjian <bruce(at)momjian(dot)us> wrote:
> > On Thu, Jan 28, 2021 at 02:41:09PM -0500, Tom Kincaid wrote:
> > > I would also like to add a "not wanted" entry for this feature on the
> > > TODO list, baaed on the feature's limited usefulness, but I already
> > > asked about that and no one seems to feel we don't want it.
> > >
> > >
> > > I want to avoid seeing this happen. As a result of a lot of customer and user
> > > discussions, around their criteria for choosing a database, I believe TDE is an
> > > important feature and having it appear with a "not-wanted" tag will keep the
> > > version of PostgreSQL released by the community out of certain (and possibly
> > > growing) number of deployment scenarios which I don't think anybody wants to
> > > see.
> >
> > With pg_upgrade, I could work on it out of the tree until it became
> > popular, with a small non-user-visible part in the backend. With the
> > Windows port, the port wasn't really visible to users until it we ready.
> >
> > For the key management part of TDE, it can't be done outside the tree,
> > and it is user-visible before it is useful, so that restricts how much
> > incremental work can be committed to the tree for TDE. I highlighted
> > that concern emails months ago, but never got any feedback --- now it
> > seems people are realizing the ramifications of that.
> >
> > > I think the current situation to be as follows (if I missed something please
> > > let me know):
> > >
> > > 1) We need to get the current patch for Key Management reviewed and tested
> > > further.
> > >
> > > I spoke to Bruce just now he will see if can get somebody to do this.
> >
> > Well, if we don't get anyone committed to working on the data encryption
> > part of TDE, the key management part is useless, so why review/test it
> > further?
> >
> > Although Sawada-san and Stephen Frost worked on the patch, they have not
> > commented much on my additions, and only a few others have commented on
> > the code, and there has been no discussion on who is working on the next
> > steps. This indicates to me that there is little interest in moving
> > this feature forward,
>
> TBH I’m confused a bit about the recent situation of this patch, but I
> can contribute to KMS work by discussing, writing, reviewing, and
> testing the patch. Also, I can work on the data encryption part of TDE
> (we need more discussion on that though). If the community concerns
> about the high-level design and thinks the design reviews by
> cryptography experts are still needed, we would need to do that first
> since the data encryption part of TDE depends on KMS. As far as I
> know, we have done that many times on pgsql-hackers, on offl-line and
> including the discussion on the past proposal, etc but given that the
> community still has a concern, it seems that we haven’t been able to
> share the details of the discussion enough that led to the design
> decision or the design is still not good. Honestly, I’m not sure how
> this feature can get consensus. But maybe we would need to have a
> break from refining the patch now and we need to marshal the
> discussions so far and the point behind the design so that everyone
> can understand why this feature is designed in that way. To do that,
> it might be a good start to sort the wiki page since it has data
> encryption part, KMS, and ToDo mixed.

I hope it's pretty clear that I'm also very much in support of both this
effort with the KMS and of TDE in general- TDE is specifically,
repeatedly, called out as a capability whose lack is blocking PG from
being able to be used for certain use-cases that it would otherwise be
well suited for, and that's really unfortunate.

I appreciate the recent discussion and reviews of the KMS in particular,
and of the patches which have been sent enabling TDE based on the KMS
patches. Having them be relatively independent seems to be an ongoing
concern and perhaps we should figure out a way to more clearly put them
together. That is- the KMS patches have been posted on one thread, and
TDE PoC patches which use the KMS patches have been on another thread,
leading some to not realize that there's already been TDE PoC work done
based on the KMS patches. Seems like it might make sense to get one
patch set which goes all the way from the KMS and includes the TDE PoC,
even if they don't all go in at once.

I'm happy to go look over the KMS patches again if that'd be helpful and
to comment on the TDE PoC. I can also spend some time trying to improve
on each, as I've already done. A few of the larger concerns that I have
revolve around how to store integrity information (I've tried to find a
way to make room for such information in our existing page layout and,
perhaps unsuprisingly, it's far from trivial to do so in a way that will
avoid breaking the existing page layout, or where the same set of
binaries could work on both unencrypted pages and encrypted pages with
integrity validation information, and that's a problem that we really
should consider trying to solve...), and how to automate key rotation
(one of the nice things about Bruce's approach to storing the keys is
that we're leveraging the filesystem as an index- it's easy to see how
we might extend the key-per-file approach to allow us to, say, have a
different key for every 32GB of LSN, but if we tried to put all of the
keys into a single file then we'd have to figure out an indexing
solution for it which would allow us to find the key we need to decrypt
a given page...). I tend to agree with Bruce that we need to take
these things in steps, getting each piece implemented as we go. Maybe
we can do that in a separate repo for a time and then bring it all
together, as a few on this thread have voiced, but there's no doubt that
this is a large project and it's hard to see how we could possibly
commit all of it at once.

Thanks!

Stephen