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

> On 14 Apr 2023, at 01:27, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> Daniel Gustafsson <daniel(at)yesql(dot)se> writes:
>> Good points, it should of course be SOCK_ERRNO. The attached saves off errno
>> and reinstates it to avoid clobbering. Will test it on Windows in the morning
>> as well.
>
> I think instead of this:
>
> + SOCK_ERRNO_SET(save_errno);
>
> you could just do this:
>
> libpq_append_conn_error(conn, "SSL SYSCALL error: %s",
> - SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
> + SOCK_STRERROR(save_errno, sebuf, sizeof(sebuf)));
>
> Although ... we're already assuming that SSL_get_error and ERR_get_error
> don't clobber errno. Maybe SSL_get_verify_result doesn't either.
> Or we could make it look like this:
>
> + SOCK_ERRNO_SET(0);
> ERR_clear_error();
> r = SSL_connect(conn->ssl);
> if (r <= 0)
> + int save_errno = SOCK_ERRNO;
> int err = SSL_get_error(conn->ssl, r);
> unsigned long ecode;
>
> ...
>
> - SOCK_STRERROR(SOCK_ERRNO, sebuf, sizeof(sebuf)));
> + SOCK_STRERROR(save_errno, sebuf, sizeof(sebuf)));
>
> to remove all doubt.

I mainly put save_errno back into SOCK_ERRNO for greppability, I don't have any
strong opinions either way so I went with the latter suggestion. Attached v3
does the above change and passes the tests both with a broken and working
system CA pool. Unless objections from those with failing local envs I propose
this is pushed to close the open item.

--
Daniel Gustafsson