Re: scram-sha-256 broken with FIPS and OpenSSL 1.0.2
Michael Paquier <michael@paquier.xyz>
From: Michael Paquier <michael@paquier.xyz>
To: Daniel Gustafsson <daniel@yesql.se>
Cc: Heikki Linnakangas <hlinnaka@iki.fi>, Tom Lane <tgl@sss.pgh.pa.us>, Postgres hackers <pgsql-hackers@lists.postgresql.org>
Date: 2020-11-21T01:19:42Z
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@paquier.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
Commits
-
Change SHA2 implementation based on OpenSSL to use EVP digest routines
- 4f48a6fbe2b2 14.0 landed
- e21cbb4b893b 14.0 landed
-
Move SHA2 routines to a new generic API layer for crypto hashes
- 87ae9691d253 14.0 landed
-
Use OpenSSL EVP API for symmetric encryption in pgcrypto.
- 5ff4a67f63fd 10.0 cited