Re: scram-sha-256 broken with FIPS and OpenSSL 1.0.2

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Daniel Gustafsson <daniel(at)yesql(dot)se>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: scram-sha-256 broken with FIPS and OpenSSL 1.0.2
Date: 2020-10-14 03:29:35
Message-ID: 20201014032935.GD3349@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Sep 28, 2020 at 12:55:06PM +0900, Michael Paquier wrote:
> Thanks. I have done more tests with the range of OpenSSL versions we
> support on HEAD, and applied this one. I have noticed that the
> previous patch forgot two fail-and-abort code paths as of
> EVP_DigestInit_ex() and EVP_DigestUpdate().

As this got reverted with fe0a1dc because of the lack of correct error
reporting in libpq, I have restarted this work from scratch, and
finished with the set of two patches attached.

0001 is a redesign of the APIs we use for the SHA2 implementations.
The origin of the problem is that we cannot have a control of the
memory context used by OpenSSL to allocate any of the EVP-related
data, so we need to add some routines to be able to allocate and free
the SHA2 contexts, basically. We have too many routines to do the
work now, so I reduced the whole to 5, instead of 12 originally (this
number would become 20 if we'd add the free/alloc routines for each
SHA2 part), giving the following structure:
/* Context Structures for SHA224/256/384/512 */
typedef enum
{
PG_SHA224 = 0,
PG_SHA256,
PG_SHA384,
PG_SHA512
} pg_sha2_type;
typedef struct pg_sha2_ctx
{
pg_sha2_type type;
/* private area used by each SHA2 implementation */
void *data;
} pg_sha2_ctx;
extern pg_sha2_ctx *pg_sha2_create(pg_sha2_type type);
extern int pg_sha2_init(pg_sha2_ctx *ctx);
extern int pg_sha2_update(pg_sha2_ctx *ctx, const uint8 *data, size_t len);
extern int pg_sha2_final(pg_sha2_ctx *ctx, uint8 *dest);
extern void pg_sha2_free(pg_sha2_ctx *ctx);

A huge advantage of this approach is that the keep all the details of
the SHA2 implementations within each file, so we have nothing related
to OpenSSL in sha2.h, which is rather clean. All the internal
structures part of the fallback implementations are also moved into
their own file sha2.c. I have made the choice to limit the number of
ifdef FRONTEND in the files of src/common/ for clarity, meaning that
the callers of those routines can handle errors as they want, in the
frontend and the backend. The areas making use of the SHA2
implementations are SCRAM (libpq and backend) and the checksum
manifests, so this has required some rework of the existing interfaces
to pass down errors correctly, but at the end the changes needed in
libpq and pg_verifybackup are straight-forward.

With 0001 in place, switching the SHA2 implementation of OpenSSL to
use EVP is straight-forward, as the only thing that's actually needed
here is to put in place a callback to clean up the EVP contexts
allocated by OpenSSL. This is rather similar to what we do in
pgcrypto in some ways, but that's actually simpler and I made things
so as we only track down the EVP_MD_CTX members to free on abort.

I'll add that to the next CF for review.
--
Michael

Attachment Content-Type Size
0001-Rework-SHA2-APIs.patch text/x-diff 60.5 KB
0002-Switch-sha2_openssl.c-to-use-EVP.patch text/x-diff 7.4 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2020-10-14 03:40:08 Track statistics for streaming of in-progress transactions
Previous Message Tom Lane 2020-10-14 03:26:45 Re: Assertion failure with LEFT JOINs among >500 relations