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

On Fri, Apr 14, 2023 at 7:20 AM Daniel Gustafsson <daniel@yesql.se> wrote:
> And again with the attachment.

After some sleep... From inspection I think the final EOF branch could
be masked by the new branch, if verification has failed but was already
ignored.

To test that, I tried hanging up on the client partway through the
server handshake, and I got some strange results. With the patch, using
sslmode=require and OpenSSL 1.0.1, I see:

    connection to server at "127.0.0.1", port 50859 failed: SSL error:
certificate verify failed: self signed certificate

Which is wrong -- we shouldn't care about the self-signed failure if
we're not using verify-*. I was going to suggest a patch like the following:

>                     if (r == -1)
> -                       libpq_append_conn_error(conn, "SSL SYSCALL error: %s",
> -                                         SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
> +                   {
> +                       /*
> +                        * If we get an X509 error here without an error in the
> +                        * socket layer it means that verification failed without
> +                        * it being a protocol error. A common cause is trying to
> +                        * a default system CA which is missing or broken.
> +                        */
> +                       if (!save_errno && vcode != X509_V_OK)
> +                           libpq_append_conn_error(conn, "SSL error: certificate verify failed: %s",
> +                                                   X509_verify_cert_error_string(vcode));
> +                       else
> +                           libpq_append_conn_error(conn, "SSL SYSCALL error: %s",
> +                                             SOCK_STRERROR(save_errno, sebuf, sizeof(sebuf)));
> +                   }
>                     else
>                         libpq_append_conn_error(conn, "SSL SYSCALL error: EOF detected");

But then I tested my case against PG15, and I didn't get the EOF message
I expected:

    connection to server at "127.0.0.1", port 50283 failed: SSL SYSCALL
error: Success

So it appears that this (hanging up on the client during the handshake)
is _also_ a case where we could get a SYSCALL error with a zero errno,
and my patch doesn't actually fix the misleading error message.

That makes me worried, but I don't really have a concrete suggestion to
make it better, yet. I'm not opposed to pushing this as a fix for the
tests, but I suspect we'll have to iterate on this more.

Thanks,
--Jacob