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

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Daniel Gustafsson <daniel(at)yesql(dot)se>
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-11-21 01:19:42
Message-ID: 20201121011942.GB6052@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Nov 20, 2020 at 11:17:44PM +0100, Daniel Gustafsson wrote:
> On 20 Nov 2020, at 01:33, Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>> knowing that we still need to deal with the OOM failure handling
>> and pass down the error to the callers playing with SHA2, it feels
>> like the most consistent API to me for the frontend and the backend.
>
> For the backend I'd prefer an API where the allocation worked like palloc, if
> not then it's a straight exit through the giftshop. But if we want an API for
> both the frontend and backend, I guess this is what we'll have to do.

The fallback implementations can somewhat achieve that as we know all
the internals of the structures used.

> Ok, then at least I think we should use PG_CRYPTOHASH_H to be more consistent
> with the tree, and since leading underscores in C are problematic spec-wise.

Makes sense. Will fix.

>> That was the least intrusive option I could figure out. Two other
>> options I have thought about:
>> - Compile those fallback implementations independently and publish the
>> internals in a separate header, but nobody is going to need that if we
>> have a generic entry point.
>> - Include the internals of each implementation in cryptohash.c, but
>> this bloats the file with more implementations added (HMAC and MD5
>> still need to be added on top of SHA2), and it messes up with the
>> existing copyright entries.
>> So splitting and just including them sounds like the cleanest option
>> of the set.
>
> Personally I think the first option of using an internal header seems cleaner,
> but MMV so I'll leave it to others to weigh in too.

What you meant and what I meant was slightly different here. I meant
publishing a header in src/include/common/ that would get installed,
and I'd rather avoid that. And you mean to have the header for local
consumption in src/common/. I would be fine with your third option as
well. Your suggestion is more consistent with what we do for the rest
of src/common/ and libpq actually. So I don't mind switching to
that.
--
Michael

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2020-11-21 01:30:03 Re: Online verification of checksums
Previous Message Michael Paquier 2020-11-21 01:12:09 Re: Removal of currtid()/currtid2() and some table AM cleanup