Re: [PoC] Let libpq reject unexpected authentication requests

On Tue, Mar 21, 2023 at 11:01 PM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
> - # Function introduced in OpenSSL 1.0.2.
> + # Functions introduced in OpenSSL 1.0.2. LibreSSL doesn't have all of these.
> ['X509_get_signature_nid'],
> + ['SSL_CTX_set_cert_cb'],
>
> From what I can see, X509_get_signature_nid() is in LibreSSL, but not
> SSL_CTX_set_cert_cb(). Perhaps that's worth having two different
> comments?

I took a stab at that in v18. I diverged a bit between Meson and
Autoconf, which you may not care for.

> + <para>
> + a certificate may be sent, if the server requests one and it has
> + been provided via <literal>sslcert</literal>
> + </para>
>
> It seems to me that this description is not completely exact. The
> default is to look at ~/.postgresql/postgresql.crt, so sslcert is not
> mandatory. There could be a certificate even without sslcert set.

Reworded.

> + libpq_append_conn_error(conn, "sslcertmode value \"%s\" invalid when SSL support is not compiled in",
> + conn->sslcertmode);
>
> This string could be combined with the same one used for sslmode,
> saving a bit in translation effortm by making the connection parameter
> name a value of the string ("%s value \"%s\" invalid ..").

Done.

> + * figure out if a certficate was actually requested, so "require" is
> s/certficate/certificate/.

Heh, fixed. I need new glasses, clearly.

> contrib/sslinfo/ has ssl_client_cert_present(), that we could use in
> the tests to make sure that the client has actually sent a
> certificate? How about adding some of these tests to 003_sslinfo.pl
> for the "allow" and "require" cases?

Added; see what you think.

> + if (!conn->ssl_cert_requested)
> + {
> + libpq_append_conn_error(conn, "server did not request a certificate");
> + return false;
> + }
> + else if (!conn->ssl_cert_sent)
> + {
> + libpq_append_conn_error(conn, "server accepted connection without a valid certificate");
> + return false;
> + }
> Perhaps useless question: should this say "SSL certificate"?

I have no objection, so done that way.

> freePGconn() is missing a free(sslcertmode).

Argh, I keep forgetting that. Fixed, thanks!

--Jacob