Re: [PoC] Let libpq reject unexpected authentication requests

On Tue, Mar 21, 2023 at 11:01 PM Michael Paquier <michael@paquier.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