On Sat, 2021-01-30 at 16:18 -0500, Andrew Dunstan wrote:
> Here's a version of the patch that does it that way. For fun I have
> modified the certificate so it has two OU fields in the DN.
> diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml
> [...]
> + <literal>Common Name (CN)</literal>. If instead you specify
> + <literal>clientname=DN</literal> the username is matched against the
> + entire <literal>Distinguished Name (DN)</literal> of the certificate.
> + This option is probably best used in comjunction with a username map.
sp: comjunction -> conjunction
> diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c
> [...]
> +
> + bio = BIO_new(BIO_s_mem());
> + if (!bio)
> + {
> + pfree(port->peer_cn);
> + port->peer_cn = NULL;
> + return -1;
> + }
Should this case have a log entry, DEBUG or otherwise?
> + /* use commas instead of slashes */
> + X509_NAME_print_ex(bio, x509name, 0, XN_FLAG_RFC2253);
> + BIO_get_mem_ptr(bio, &bio_buf);
> + peer_dn = MemoryContextAlloc(TopMemoryContext, bio_buf->length + 1);
> + memcpy(peer_dn, bio_buf->data, bio_buf->length);
> + peer_dn[bio_buf->length] = '\0';
> + if (bio_buf->length != strlen(peer_dn))
> + {
> + ereport(COMMERROR,
> + (errcode(ERRCODE_PROTOCOL_VIOLATION),
> + errmsg("SSL certificate's distinguished name contains embedded null")));
> + BIO_free(bio);
> + pfree(peer_dn);
> + pfree(port->peer_cn);
> + port->peer_cn = NULL;
> + return -1;
> + }
Since the definition of XN_FLAG_RFC2253 includes ASN1_STRFLGS_ESC_CTRL,
this case shouldn't be possible. I think it's still worth it to double-
check, but maybe it should assert as well? Or at least have a comment
explaining that this is an internal error and not a client error.
> diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl
> +# correct client cert using whole DN
> +my $dn_connstr = $common_connstr;
> +$dn_connstr =~ s/certdb/certdb_dn/;
> +
> +test_connect_ok(
> + $dn_connstr,
> + "user=ssltestuser sslcert=ssl/client-dn.crt sslkey=ssl/client-dn_tmp.key",
> + "certificate authorization succeeds with DN mapping"
> +);
A negative case for the new DN code paths would be good to add.
--Jacob
