On Tue, Mar 14, 2023 at 12:14:40PM -0700, Jacob Champion wrote: > On Mon, Mar 13, 2023 at 10:39 PM Michael Paquier <mich...@paquier.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
signature.asc
Description: PGP signature