Re: Proposed patch for key managment

Greetings,

* Bruce Momjian (bruce(at)momjian(dot)us) wrote:
> On Fri, Dec 18, 2020 at 10:01:22AM +0900, Michael Paquier wrote:
> > On Thu, Dec 17, 2020 at 12:10:22PM -0500, Bruce Momjian wrote:
> > > On Thu, Dec 17, 2020 at 11:39:55AM -0500, Stephen Frost wrote:
> > >> I don't think there's any need for us to implement a fallback
> > >> implementation of AES. I'm not entirely sure we need it for hashes
> > >> but since we've already got it...
> >
> > We need fallback implementations for cryptohashes (MD5, SHA1/2) and
> > HMAC because we have SCRAM authentication, pgcrypto and SQL functions
> > that should be able to work even when not building with any SSL
> > libraries. So that's here to stay to ensure compatibility with what
> > we do. All this stuff is already in the tree for ages, it was just
> > not shaped for a more centralized reuse.
>
> One question is whether we want to support cluster file encryption
> without OpenSSL --- right now we can't. I am wondering if we really
> need the hardware acceleration of OpenSSL for AES so doing our own AES
> implementation might not even make sense, performance-wise.

No, I don't think we need to support file encryption independently of
any library being available. Maybe some day we will, but that should
really just be another option to build with.

Guess it wasn't clear, but I was being a bit tounge-in-cheek regarding
the idea of dropping SCRAM support if we aren't built with OpenSSL.

> > > Agreed. I think there is serious risk we would do AES in a different
> > > way than OpenSSL, especially if I did it. ;-) We can add a native AES
> > > one day if we want, but as stated by Michael Paquier, it has to be
> > > tested so we are sure it returns exactly the same values as OpenSSL.
> >
> > I think that it would be good to put some generalization here, and
> > look at options that are offered by other SSL libraries, like libnss
> > so as we don't finish with a design that restricts the use of a given
> > feature only to OpenSSL.
>
> Uh, you mean the C API or the user API? I don't think we have any
> OpenSSL requirement at the user level, except that my examples use
> command-line openssl to generate the passphrase.

What I would be thinking about with this are really three pieces-

- C API for libpq (not relevant for this, but we have had issues in the
past with exposing OpenSSL-specific things there)

- C API in backend - we should try to at least set up the structure to
allow multiple encryption implementations, either via different
libraries or if someone feels it'd be useful to write a built-in
implementation, but as I mentioned just a moment ago, we aren't going
to get this perfect and we should accept that.

- User API - we should avoid having OpenSSL-specific bits exposed to
users, and I don't think we do today, so I don't think this is an
issue at the moment.

> I split apart my patch to create cipher{_openssl}.c and hmac{_openssl}.c
> so the single HMAC function is not in the cipher file anymore, attached.

Will try to look at this soon, but in general the idea seems right.

Thanks,

Stephen