Re: Fetch channel binding digest explicitly with OpenSSL 3.0 and later

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Mark Atwood <mark(at)reviewcommit(dot)com>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Fetch channel binding digest explicitly with OpenSSL 3.0 and later
Date: 2026-08-12 01:20:33
Message-ID: anvKYXoc3weXF5cf@paquier.xyz
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Aug 11, 2026 at 01:33:31PM -0700, Mark Atwood wrote:
> Implicit digests DO reach providers. evp_md_init_internal() re-fetches an MD
> with type->prov == NULL by name, so EVP_sha256() and EVP_get_digestbynid()
> end up provider-backed. The probe I cited measured ctx->reqdigest, the MD
> passed in, not the one used. Details and a corrected probe are on the
> cryptohash thread [1].

This does not change the fact that removing the dependency to the
deprecated EVP_sha256() is still something that we need to do in the
long-term in the frontend and the backend code, because OpenSSL wants
code to switch to EVP.

> Michael, you named channel binding as your interest, so you should have that
> before spending time on it. The patch behavior is unchanged, and the question
> about freeing the EVP_MD on every error path still stands.

Using EVP_MD_free() where adapted is an implementation artifact, there
is no big deal in them except making sure that nothing leaks, and
the channel binding paths are, contrary to the crypthash and hmac
bits, purely local, meaning that there is no need for post-cleanup
actions based on resowners and the kind. The commit message just has
to reflect what's been done, let's make it simpler without the
provider part then.

Now I think that this patch should be reworked in a style closer to
what has been done in 1f3b9bb109b8 and b91f79cd08ab:
- Keep the variable declarations at the top of each function. For
example algo_type is shared between the pre-3.0 block and the post-3.0
block. No need for two declarations.
- The #if parts work as the way as the curly brackets, let's remove
one level of indentation. Your patch makes the whole diff harder to
parse and the pre-3.0 code is still the same.
- Minimization of the diffs by planting more #if blocks. Here I am
looking at the EVP_MD_free() calls. Let's minimize the duplicated
libpq_append_conn_error() and elog(ERROR) calls in the final result.

I was wondering a bit about OBJ_nid2sn(), to retrieve the EVP_MD from
an algorithm name, and it looks like it's safe choice at the end.

Could you rework the patch among these lines, please?
--
Michael

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Chao Li 2026-08-12 02:31:45 Re: [HACKERS] comment/security label for publication/subscription
Previous Message Michael Paquier 2026-08-12 00:59:26 Re: Fix archive restore race that could unlink WAL before rename