Re: [PoC] Let libpq reject unexpected authentication requests

On Tue, Mar 14, 2023 at 12:14:40PM -0700, Jacob Champion wrote:
> On Mon, Mar 13, 2023 at 10:39 PM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>> 0002 looks pretty simple as well, I think that's worth a look for this
>> CF.
>
> Cool. v17 just rebases the set over HEAD, then, for cfbot.

I have looked at 0002, and I am on board with using a separate
connection parameter for this case, orthogonal to require_auth, with
the three value "allow", "disable" and "require". So that's one thing
:)

- # 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?

+ <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.

+ 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 .."). The second
string where HAVE_SSL_CTX_SET_CERT_CB is not set could be refactored
the same way, I guess.

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

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? Even for "disable", we could
check check that ssl_client_cert_present() returns false? That would
make four tests if everything is covered:
- "allow" without a certificate sent.
- "allow" with a certificate sent.
- "disable".
- "require"

+ 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"?

freePGconn() is missing a free(sslcertmode).
--
Michael