Re: Proposed patch for key managment

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Bruce Momjian <bruce(at)momjian(dot)us>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Masahiko Sawada <masahiko(dot)sawada(at)2ndquadrant(dot)com>
Subject: Re: Proposed patch for key managment
Date: 2020-12-16 23:26:48
Message-ID: X9qXuLekePLRYrB2@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Dec 16, 2020 at 05:04:12PM -0500, Bruce Momjian wrote:
> On Wed, Dec 16, 2020 at 10:23:23AM +0900, Michael Paquier wrote:
>> Hmm. I don't think that this is right. First, this still mixes
>> ciphers and HMAC.
>
> I looked at the code. The HMAC function body is just one function call
> to OpenSSL. Do you want it moved to cryptohash.c and
> cryptohash_openssl.c? To a new C file?
>
>> Second, it is only possible here to use HMAC with
>> SHA512 but we want to be able to use SHA256 as well with SCRAM (per se
>> the scram_HMAC_* business in scram-common.c). Third, this lacks a
>
> I looked at the SCRAM HMAC code. It looks complex, but I assume ideally
> that would be moved into a separate C file and used by cluster file
> encryption and SCRAM, right? I need help to do that because I am
> unclear how much of the SCRAM hash code is specific to SCRAM.

I have gone though the same exercise for HMAC, with more success it
seems:
https://www.postgresql.org/message-id/X9m0nkEJEzIPXjeZ@paquier.xyz

This includes a fallback implementation that returns the same results
as OpenSSL, and the same results as samples in wikipedia or such
sources. The set APIs in this refactoring could be reused here and
plugged into any SSL libraries, and my patch has changed SCRAM to
reuse that. With this, it is also possible to remove all the HMAC
code from pgcrypto but this cannot happen without SHA1 support in
cryptohash.c, another patch I have submitted -hackers recently. So I
think that it is a win as a whole.

>> fallback implementation. Finally, pgcrypto is not touched, but we
>
> I have a fallback implemention --- it fails? ;-) Did you want me to
> include an AES implementation?

No idea about this one yet. There are no direct users of AES except
pgcrypto in core. One thing that would be good IMO is to properly
split the patch of this thread into individual parts that could be
reviewed separately using for example "git format-patch" to generate
patch series. What's presented is a mixed bag, so that's harder to
look at it and consider how this stuff should work, and if there are
pieces that should be designed better or not.
--
Michael

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alexander Korotkov 2020-12-16 23:34:39 Re: range_agg
Previous Message Bruce Momjian 2020-12-16 22:58:54 Re: Added missing copy related data structures to typedefs.list