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

From: Daniel Gustafsson <daniel(at)yesql(dot)se>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, 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-12-01 09:10:45
Message-ID: C0FDC26B-E29A-41BF-882F-55C00F491BE6@yesql.se
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On 1 Dec 2020, at 06:38, Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>
> On Mon, Nov 30, 2020 at 02:29:29PM +0100, Daniel Gustafsson wrote:
>> Yeah, that's along the lines of what I was thinking of.
>
> Hmm. I have looked at that, and thought first about having directly a
> reference to the resowner directly in pg_cryptohash_ctx, but that's
> not a good plan for two reasons:
> - common/cryptohash.h would get knowledge of that, requiring bundling
> in a bunch of dependencies.
> - There is no need for that in the non-OpenSSL case.
> So, instead, I have been thinking about using an extra context layer
> only for cryptohash_openssl.c with a structure saved as
> pg_cryptohash_context->data that stores the information about
> EVP_MD_CTX* and the resource owner. Then, I was thinking about
> storing directly pg_cryptohash_ctx in the resowner EVP array and just
> call pg_cryptohash_free() from resowner.c without the need of an
> extra routine. I have not tested this idea but that should work.
> What's your take?

That sounds like it would work. Since the cryptohash implementation owns and
controls the void *data contents it can store whatever it needs to properly
free it.

> In parallel, I have spent more time today polishing and reviewing 0001
> (indented, adjusted a couple of areas and added also brackets and
> extra comments as you suggested) and tested it on Linux and Windows,
> with and without OpenSSL down to 1.0.1, the oldest version supported
> on HEAD. So I'd like to apply the attached first and sort out the
> resowner stuff in a next step.

+1 on separating the API, EVP migration, resowner patches.

Reading through the v6 patch I see nothing sticking out and all review comments
addressed, +1 on applying that one and then we'll take if from there with the
remaining ones in the patchset.

cheers ./daniel

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Drouvot, Bertrand 2020-12-01 09:32:17 Re: [BUG] orphaned function
Previous Message Georgios Kokolatos 2020-12-01 09:10:38 Re: Display individual query in pg_stat_activity