Re: [PATCH] Add `verify-system` sslmode to use system CA pool for server cert

> On 12 Apr 2023, at 23:46, Daniel Gustafsson <daniel@yesql.se> wrote:
>> On 12 Apr 2023, at 23:40, Tom Lane <tgl@sss.pgh.pa.us> wrote:

>> So this smells to me like a new OpenSSL bug: they should tolerate
>> a missing certs dir like they used to.  Who wants to file it?
> 
> They are specifying that: "A missing default location is still treated as a
> success".  That leaves out the interesting bit of what a success means here,
> and how it should work when verifications are requested.  That being said, the
> same is written in the 1.1.1 manpage.

After a little bit of digging I have a vague idea.

OpenSSL will treat a missing default location as a success simply due to the
fact that it mainly just stores the path, loading of the certs is deferred
until use (which maps well to the error we are seeing).  Patching OpenSSL to
report all errors makes no difference, a missing default is indeed not an error
even with errors turned on.

The change in OpenSSL 3 is the addition of certificate stores via ossl_store
API.  When SSL_CTX_set_default_verify_paths() is called it will in 1.1.1 set
the default (hardcoded) filename and path; in 3 it also sets the default store.
Stores are initialized with a URL, and the default store falls back to using the
default certs dir as the URI as no store is set.

If I patch OpenSSL 3 to skip setting the default store, the tests pass even
with a missing cert directory. This is effectively the 1.1.1 behavior.

The verification error we are hitting is given to us in the verify_cb which
we've short circuited.  The issue we have is that we cannot get PGconn in
verify_cb so logging an error is tricky.

I need to sleep on this before I do some more digging to figure out if OpenSSL
considers this to be the intended behavior, a regression in 3, or if we have a
bug in how we catch verification errors which is exposed by a non-existing
store.  I'll add an open item for this in the morning to track how we'd like to
proceed.

--
Daniel Gustafsson