Re: BUG #17391: While using --with-ssl=openssl and PG_TEST_EXTRA='ssl' options, SSL tests fail on OpenBSD 7.0

Tom Lane <tgl@sss.pgh.pa.us>

From: Tom Lane <tgl@sss.pgh.pa.us>
To: Daniel Gustafsson <daniel@yesql.se>
Cc: Thomas Munro <thomas.munro@gmail.com>, Andres Freund <andres@anarazel.de>, byavuz81@gmail.com, PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>, Heikki Linnakangas <heikki.linnakangas@iki.fi>, Michael Paquier <michael@paquier.xyz>
Date: 2022-02-11T00:44:57Z
Lists: pgsql-bugs

Attachments

I wrote:
> Just to note that I have a plan for fixing this part.  I've concluded
> that it was a design error to implement the write_failed error
> postponement mechanism in pqSendSome, and instead we should shove it
> down a couple of abstraction layers into pqsecure_raw_write.  This'd
> visibly have no effect on non-encrypted connections, because
> pqSendSome is the only caller of pqsecure_write.  But in encrypted
> connections, we'd be additionally allowing write-failure postponement
> during OpenSSL's internal machinations, which seems to be exactly
> what's wanted now that we have seen this failure mode.

Here's a draft patch for that.  It seems to fix the OpenBSD problem
reliably.

I'd originally thought that everything under pqsecure_write needed to be
converted to use the write_failed mechanism, which would have required
rather invasive changes in fe-secure-openssl.c and fe-secure-gssapi.c.
But now I think we can leave those modules alone, as long as the
bottom-level physical I/O uses write_failed.  If we get some other
error out of the SSL/GSS layer, there's no harm in reporting it
right away instead of postponing it.  This reflects the fact that
it's not that easy to tell whether an OpenSSL error ought to be
classified as "read" or "write", since both I/O directions might be
going on under the hood.

BTW, this also changes what is looking to me like a thinko in
my_sock_write(): if we get a zero result from pqsecure_raw_write(),
we should not be consulting errno, because it wouldn't have
been set.  Am I missing something there?

Thoughts?

			regards, tom lane

Commits

  1. Move libpq's write_failed mechanism down to pqsecure_raw_write().

  2. Fix thinko in PQisBusy().

  3. Set SNI ClientHello extension to localhost in tests

  4. Improve worst-case performance of text_position_get_match_pos()