Some more hackery around cryptohashes (some fixes + SHA1)

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Cc: Daniel Gustafsson <daniel(at)yesql(dot)se>
Subject: Some more hackery around cryptohashes (some fixes + SHA1)
Date: 2020-12-10 08:07:05
Message-ID: X9HXKTgrvJvYO7Oh@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi all,

The remnant work that I have on my agenda to replace the remaining
low-level cryptohash calls of OpenSSL (SHAXXInit and such) by EVP is
the stuff related to SHA1, that gets used in two places: pgcrypto and
uuid-ossp.

First, I got to wonder if it would be better to support SHA1 directly
in cryptohash{_openssl}.c, glue some code to pgcrypto to use EVP
discreetly or just do nothing. Contrary to SHA256 and MD5 that are
used for authentication or backup manifests, SHA1 has a limited use in
core, so I wanted first to just stick something in pgcrypto or just
let it go, hoping for the day where we'd remove those two modules but
that's not a call I think we can make now.

But then, my very-recent history with uuid-ossp has made me look at
what kind of tricks we use to pull in SHA1 from pgcrypto to
uuid-ossp, and I did not like much the shortcuts used in ./configure
or uuid-ossp's Makefile to get those files when needed, depending on
the version of libuuid used (grep for UUID_EXTRA_OBJS for example).
So, I got to look at the second option of moving SHA1 directly into
the new cryptohash stuff, and quite liked the cleanup this gives.

Please find attached a set of two patches:
- 0001 is a set of small adjustments for the existing code of
cryptohashes: some cleanup for MD5 in uuid-ossp, and more importantly
one fix to call explicit_bzero() on the context data for the fallback
implementations. With the existing code, we may leave behind some
context data. That could become a problem if somebody has access to
this area of the memory even when they should not be able to do so,
something that should not happen, but I see no reason to not play it
safe and eliminate any traces. If there are no objections, I'd like
to apply this part.
- 0002 is the addition of sha1 in the cryptohash infra, that includes
the cleanup between uuid-ossp and pgcrypto. This makes any caller of
cryptohash for SHA1 to use EVP when building with OpenSSL, or the
fallback implementation. I have adapted the fallback implementation
of SHA1 to have some symmetry with src/common/{md5.c,sha2.c}.

I am adding this patch set to the next commit fest. Thanks for
reading!
--
Michael

Attachment Content-Type Size
0001-Adjust-some-code-of-cryptohash.patch text/x-diff 3.2 KB
0002-Introduce-SHA1-in-cryptohash-infrastructure.patch text/x-diff 33.2 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message k.jamison@fujitsu.com 2020-12-10 08:09:55 RE: [Patch] Optimize dropping of relation buffers using dlist
Previous Message Pavel Stehule 2020-12-10 07:45:12 Re: On login trigger: take three