Refactoring HMAC in the core code

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Cc: Bruce Momjian <bruce(at)momjian(dot)us>, Daniel Gustafsson <daniel(at)yesql(dot)se>
Subject: Refactoring HMAC in the core code
Date: 2020-12-16 07:17:50
Message-ID: X9m0nkEJEzIPXjeZ@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi all,
(Added Bruce and Daniel in CC:)

$subject has been mentioned a couple of times lately, mainly by me for
the recent cryptohash refactoring that has been done. We use in the
core code HMAC with SHA256 for SCRAM, but this logic should go through
SSL libraries able to support it (OpenSSL and libnss allow that) so as
requirements like FIPS can be pushed down to any lower-level library
we are building with and not Postgres.

FWIW, I have also bumped into this stuff as being a requirement for
the recent thread about file-level encryption in [1] where the code
makes use of HMAC with SHA512.

So, please find attached a patch set to rework all that. This
provides a split similar to what I have done recently for cryptohash
functions, with a fallback implementation located as of
src/common/hmac.c, that depends itself on the fallback implementations
of cryptohash functions. The OpenSSL part is done hmac_openssl.c.

There are five APIs to be able to plug in HMAC implementations to
create, initialize, update, finalize and free a HMAC context, in a
fashion similar to cryptohashes.

Regarding OpenSSL, upstream has changed lately the way it is possible
to control HMACs. 3.0.0 has introduced a new set of APIs, with
compatibility macros for older versions, as mentioned here:
https://www.openssl.org/docs/manmaster/man3/EVP_MAC_CTX_new.html
The new APIs are named EVP_MAC_CTX_new() and such.

I think that this is a bit too new to use though, as we need to
support OpenSSL down to 1.0.1 on HEAD and because there are
compatibility macros. So instead I have decided to rely on the older
interface based on HMAC_Init_ex() and co:
https://www.openssl.org/docs/manmaster/man3/HMAC.html

After that there is another point to note. In 1.1.0, any consumers of
HMAC *have* to let OpenSSL allocate the HMAC context, like
cryptohashes because the internals of the HMAC context are only known
to OpenSSL. In 1.0.2 and older versions, it is possible to have
access to HMAC_CTX. This requires an extra tweak in hmac_openssl.c
where we need to {m,p}alloc by ourselves instead of calling
HMAC_CTX_new() for 1.1.0 and 1.1.1 but some extra configure switches
are able to do the trick. That means that we could use resowners only
when building with OpenSSL >= 1.1.0, and not for older versions but
note that the patch uses resowners anyway, as a matter of simplicity.
As the changes are local to a single file, that's easy enough to
follow and update. It would be easy enough to rip off this code once
support for older OpenSSL versions is removed.

Please note that I have added code that should be enough for the
compilation on Windows, but I have not taken the time to check that.
I have checked that things compiled and that check-world passes
with and without OpenSSL 1.1.1 on Linux though, so I guess that I have
not messed up too badly. This stuff requires much more tests, like
making sure that we are able to connect to PG correctly with SCRAM
when using combinations like libpq based on OpenSSL and the backend
using the fallback, or just check the consistency of the results of
computations with SQL functions or such. An extra thing that can be
done is to clean up pgcrypto's px-hmac.c but this also requires SHA1
in cryptohash.c, something that I have submitted separately in [2].
So this could just be done later. This patch has updated the code of
SCRAM so as we don't use anymore all the SCRAM/HMAC business but the
generic HMAC routines instead for this work.

Thoughts are welcome. I am adding that to the next CF.

[1]: https://www.postgresql.org/message-id/X9lhi1ht04I+v/rV(at)paquier(dot)xyz
[2]: https://commitfest.postgresql.org/31/2868/

Thanks,
--
Michael

Attachment Content-Type Size
0001-Refactor-HMAC-implementations.patch text/x-diff 38.1 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Kyotaro Horiguchi 2020-12-16 07:24:59 Re: Use standard SIGHUP and SIGTERM handlers in autoprewarm module
Previous Message Bharath Rupireddy 2020-12-16 06:35:47 Re: Parallel Inserts in CREATE TABLE AS