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
- postpone-ssl-write-failures-too-1.patch (text/x-diff) patch
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
-
Move libpq's write_failed mechanism down to pqsecure_raw_write().
- faa189c932d5 15.0 landed
-
Fix thinko in PQisBusy().
- c9d35dc7f8a9 12.11 landed
- ae27b1acc43d 14.3 landed
- 51ee561f5671 13.7 landed
- 335fa5a26029 15.0 landed
-
Set SNI ClientHello extension to localhost in tests
- 6d503d2a4732 15.0 landed
- 5f00ef065ec8 14.3 landed
-
Improve worst-case performance of text_position_get_match_pos()
- b31e3f561365 15.0 cited