On 3/24/21 12:54 AM, Michael Paquier wrote:
[numerous useful comments]
OK, here's a new patch. I hope to commit this within a few days.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
diff --git a/doc/src/sgml/client-auth.sgml b/doc/src/sgml/client-auth.sgml
index b420486a0a..951af49e9a 100644
--- a/doc/src/sgml/client-auth.sgml
+++ b/doc/src/sgml/client-auth.sgml
@@ -598,7 +598,7 @@ hostnogssenc <replaceable>database</replaceable> <replaceable>user</replaceabl
</para>
<para>
- In addition to the method-specific options listed below, there is one
+ In addition to the method-specific options listed below, there is a
method-independent authentication option <literal>clientcert</literal>, which
can be specified in any <literal>hostssl</literal> record.
This option can be set to <literal>verify-ca</literal> or
@@ -612,6 +612,28 @@ hostnogssenc <replaceable>database</replaceable> <replaceable>user</replaceabl
the verification of client certificates with any authentication
method that supports <literal>hostssl</literal> entries.
</para>
+ <para>
+ On any record using client certificate authentication (i.e. one
+ using the <literal>cert</literal> authentication method or one
+ using the <literal>clientcert</literal> option), you can specify
+ which part of the client certificate credentials to match using
+ the <literal>clientname</literal> option. This option can have one
+ of two values. If you specify <literal>clientname=CN</literal>, which
+ is the default, the username is matched against the certificate's
+ <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 conjunction with a username map.
+ The comparison is done with the <literal>DN</literal> in
+ <ulink url="https://tools.ietf.org/html/rfc2253";>RFC 2253</ulink>
+ format. To see the <literal>DN</literal> of a client certificate
+ in this format, do
+<programlisting>
+openssl x509 -in myclient.crt -noout --subject -nameopt RFC2253 | sed "s/^subject=//"
+</programlisting>
+ Care needs to be taken when using this option, especially when using
+ regular expression matching against the <literal>DN</literal>.
+ </para>
</listitem>
</varlistentry>
</variablelist>
diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c
index 994251e7d9..32ef05783a 100644
--- a/src/backend/libpq/auth.c
+++ b/src/backend/libpq/auth.c
@@ -2800,12 +2800,23 @@ static int
CheckCertAuth(Port *port)
{
int status_check_usermap = STATUS_ERROR;
+ char *peer_username;
Assert(port->ssl);
+ /* select the correct field to compare */
+ switch (port->hba->clientcertname)
+ {
+ case clientCertDN:
+ peer_username = port->peer_dn;
+ break;
+ default:
+ peer_username = port->peer_cn;
+ }
+
/* Make sure we have received a username in the certificate */
- if (port->peer_cn == NULL ||
- strlen(port->peer_cn) <= 0)
+ if (peer_username == NULL ||
+ strlen(peer_username) <= 0)
{
ereport(LOG,
(errmsg("certificate authentication failed for user \"%s\": client certificate contains no user name",
@@ -2813,8 +2824,8 @@ CheckCertAuth(Port *port)
return STATUS_ERROR;
}
- /* Just pass the certificate cn to the usermap check */
- status_check_usermap = check_usermap(port->hba->usermap, port->user_name, port->peer_cn, false);
+ /* Just pass the certificate cn/dn to the usermap check */
+ status_check_usermap = check_usermap(port->hba->usermap, port->user_name, peer_username, false);
if (status_check_usermap != STATUS_OK)
{
/*
@@ -2824,9 +2835,18 @@ CheckCertAuth(Port *port)
*/
if (port->hba->clientcert == clientCertFull && port->hba->auth_method != uaCert)
{
- ereport(LOG,
- (errmsg("certificate validation (clientcert=verify-full) failed for user \"%s\": CN mismatch",
- port->user_name)));
+ if (port->hba->clientcertname == clientCertDN)
+ {
+ ereport(LOG,
+ (errmsg("certificate validation (clientcert=verify-full) failed for user \"%s\": DN mismatch",
+ port->user_name)));
+ }
+ else
+ {
+ ereport(LOG,
+ (errmsg("certificate validation (clientcert=verify-full) failed for user \"%s\": CN mismatch",
+ port->user_name)));
+ }
}
}
return status_check_usermap;
diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c
index 5ce3f27855..65b07c7d3f 100644
--- a/src/backend/libpq/be-secure-openssl.c
+++ b/src/backend/libpq/be-secure-openssl.c
@@ -551,22 +551,26 @@ aloop:
/* Get client certificate, if available. */
port->peer = SSL_get_peer_certificate(port->ssl);
- /* and extract the Common Name from it. */
+ /* and extract the Common Name and Distinguished Name from it. */
port->peer_cn = NULL;
+ port->peer_dn = NULL;
port->peer_cert_valid = false;
if (port->peer != NULL)
{
int len;
+ X509_NAME *x509name = X509_get_subject_name(port->peer);
+ char *peer_dn;
+ BIO *bio = NULL;
+ BUF_MEM *bio_buf = NULL;
- len = X509_NAME_get_text_by_NID(X509_get_subject_name(port->peer),
- NID_commonName, NULL, 0);
+ len = X509_NAME_get_text_by_NID(x509name, NID_commonName, NULL, 0);
if (len != -1)
{
char *peer_cn;
peer_cn = MemoryContextAlloc(TopMemoryContext, len + 1);
- r = X509_NAME_get_text_by_NID(X509_get_subject_name(port->peer),
- NID_commonName, peer_cn, len + 1);
+ r = X509_NAME_get_text_by_NID(x509name, NID_commonName, peer_cn,
+ len + 1);
peer_cn[len] = '\0';
if (r != len)
{
@@ -590,6 +594,41 @@ aloop:
port->peer_cn = peer_cn;
}
+
+ bio = BIO_new(BIO_s_mem());
+ if (!bio)
+ {
+ pfree(port->peer_cn);
+ port->peer_cn = NULL;
+ return -1;
+ }
+ /*
+ * RFC2253 is the closest thing to an accepted standard format for
+ * DNs. We have documented how to produce this format from a
+ * certificate. It uses commas instead of slashes for delimiters,
+ * which make regular expression matching a bit easier. Also note that
+ * it prints the Subject fields in reverse order.
+ */
+ 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);
+ len = bio_buf->length;
+ BIO_free(bio);
+ peer_dn[len] = '\0';
+ if (len != strlen(peer_dn))
+ {
+ ereport(COMMERROR,
+ (errcode(ERRCODE_PROTOCOL_VIOLATION),
+ errmsg("SSL certificate's distinguished name contains embedded null")));
+ pfree(peer_dn);
+ pfree(port->peer_cn);
+ port->peer_cn = NULL;
+ return -1;
+ }
+
+ port->peer_dn = peer_dn;
+
port->peer_cert_valid = true;
}
@@ -618,6 +657,12 @@ be_tls_close(Port *port)
pfree(port->peer_cn);
port->peer_cn = NULL;
}
+
+ if (port->peer_dn)
+ {
+ pfree(port->peer_dn);
+ port->peer_dn = NULL;
+ }
}
ssize_t
diff --git a/src/backend/libpq/be-secure.c b/src/backend/libpq/be-secure.c
index bb603ad209..8ef083200a 100644
--- a/src/backend/libpq/be-secure.c
+++ b/src/backend/libpq/be-secure.c
@@ -120,8 +120,9 @@ secure_open_server(Port *port)
r = be_tls_open_server(port);
ereport(DEBUG2,
- (errmsg_internal("SSL connection from \"%s\"",
- port->peer_cn ? port->peer_cn : "(anonymous)")));
+ (errmsg_internal("SSL connection from DN:\"%s\" CN:\"%s\"",
+ port->peer_dn ? port->peer_dn : "(anonymous)",
+ port->peer_cn ? port->peer_cn : "(anonymous)")));
#endif
return r;
diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c
index 9a04c093d5..feb711a6ef 100644
--- a/src/backend/libpq/hba.c
+++ b/src/backend/libpq/hba.c
@@ -1753,6 +1753,37 @@ parse_hba_auth_opt(char *name, char *val, HbaLine *hbaline,
return false;
}
}
+ else if (strcmp(name, "clientname") == 0)
+ {
+ if (hbaline->conntype != ctHostSSL)
+ {
+ ereport(elevel,
+ (errcode(ERRCODE_CONFIG_FILE_ERROR),
+ errmsg("clientname can only be configured for \"hostssl\" rows"),
+ errcontext("line %d of configuration file \"%s\"",
+ line_num, HbaFileName)));
+ *err_msg = "clientname can only be configured for \"hostssl\" rows";
+ return false;
+ }
+
+ if (strcmp(val, "CN") == 0)
+ {
+ hbaline->clientcertname = clientCertCN;
+ }
+ else if (strcmp(val, "DN") == 0)
+ {
+ hbaline->clientcertname = clientCertDN;
+ }
+ else
+ {
+ ereport(elevel,
+ (errcode(ERRCODE_CONFIG_FILE_ERROR),
+ errmsg("invalid value for clientname: \"%s\"", val),
+ errcontext("line %d of configuration file \"%s\"",
+ line_num, HbaFileName)));
+ return false;
+ }
+ }
else if (strcmp(name, "pamservice") == 0)
{
REQUIRE_AUTH_OPTION(uaPAM, "pamservice", "pam");
diff --git a/src/include/libpq/hba.h b/src/include/libpq/hba.h
index 8f09b5638f..1ec8603da7 100644
--- a/src/include/libpq/hba.h
+++ b/src/include/libpq/hba.h
@@ -71,6 +71,12 @@ typedef enum ClientCertMode
clientCertFull
} ClientCertMode;
+typedef enum ClientCertName
+{
+ clientCertCN,
+ clientCertDN
+} ClientCertName;
+
typedef struct HbaLine
{
int linenumber;
@@ -101,6 +107,7 @@ typedef struct HbaLine
char *ldapprefix;
char *ldapsuffix;
ClientCertMode clientcert;
+ ClientCertName clientcertname;
char *krb_realm;
bool include_realm;
bool compat_realm;
diff --git a/src/include/libpq/libpq-be.h b/src/include/libpq/libpq-be.h
index 891394b0c3..713c34fedd 100644
--- a/src/include/libpq/libpq-be.h
+++ b/src/include/libpq/libpq-be.h
@@ -195,6 +195,7 @@ typedef struct Port
*/
bool ssl_in_use;
char *peer_cn;
+ char *peer_dn;
bool peer_cert_valid;
/*
diff --git a/src/test/ssl/Makefile b/src/test/ssl/Makefile
index 4b53fdf6c0..a9eca9e049 100644
--- a/src/test/ssl/Makefile
+++ b/src/test/ssl/Makefile
@@ -18,7 +18,7 @@ export with_ssl
CERTIFICATES := server_ca server-cn-and-alt-names \
server-cn-only server-single-alt-name server-multiple-alt-names \
server-no-names server-revoked server-ss \
- client_ca client client-revoked \
+ client_ca client client-dn client-revoked \
root_ca
SSLFILES := $(CERTIFICATES:%=ssl/%.key) $(CERTIFICATES:%=ssl/%.crt) \
@@ -91,6 +91,13 @@ ssl/client.crt: ssl/client.key ssl/client_ca.crt
openssl x509 -in ssl/temp.crt -out ssl/client.crt # to keep just the PEM cert
rm ssl/client.csr ssl/temp.crt
+# Client certificate with multi-parth DN, signed by the client CA:
+ssl/client-dn.crt: ssl/client-dn.key ssl/client_ca.crt
+ openssl req -new -key ssl/client-dn.key -out ssl/client-dn.csr -config client-dn.config
+ openssl ca -name client_ca -batch -out ssl/temp.crt -config cas.config -infiles ssl/client-dn.csr
+ openssl x509 -in ssl/temp.crt -out ssl/client-dn.crt # to keep just the PEM cert
+ rm ssl/client-dn.csr ssl/temp.crt
+
# Another client certificate, signed by the client CA. This one is revoked.
ssl/client-revoked.crt: ssl/client-revoked.key ssl/client_ca.crt client.config
openssl req -new -key ssl/client-revoked.key -out ssl/client-revoked.csr -config client.config
diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl
index bfada03d3e..adaa1b4e9b 100644
--- a/src/test/ssl/t/001_ssltests.pl
+++ b/src/test/ssl/t/001_ssltests.pl
@@ -17,7 +17,7 @@ if ($ENV{with_ssl} ne 'openssl')
}
else
{
- plan tests => 100;
+ plan tests => 103;
}
#### Some configuration
@@ -40,7 +40,7 @@ my $common_connstr;
my @keys = (
"client", "client-revoked",
"client-der", "client-encrypted-pem",
- "client-encrypted-der");
+ "client-encrypted-der", "client-dn");
foreach my $key (@keys)
{
copy("ssl/${key}.key", "ssl/${key}_tmp.key")
@@ -453,6 +453,36 @@ test_connect_fails(
"certificate authorization fails with correct client cert and wrong password in encrypted PEM format"
);
+
+# correct client cert using whole DN
+my $dn_connstr = "$common_connstr dbname=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"
+);
+
+# same thing but with a regex
+$dn_connstr = "$common_connstr dbname=certdb_dn_re";
+
+test_connect_ok(
+ $dn_connstr,
+ "user=ssltestuser sslcert=ssl/client-dn.crt sslkey=ssl/client-dn_tmp.key",
+ "certificate authorization succeeds with DN regex mapping"
+);
+
+# same thing but using explicit CN
+$dn_connstr = "$common_connstr dbname=certdb_cn";
+
+test_connect_ok(
+ $dn_connstr,
+ "user=ssltestuser sslcert=ssl/client-dn.crt sslkey=ssl/client-dn_tmp.key",
+ "certificate authorization succeeds with CN mapping"
+);
+
+
+
TODO:
{
# these tests are left here waiting on us to get better pty support
diff --git a/src/test/ssl/t/SSLServer.pm b/src/test/ssl/t/SSLServer.pm
index 5ec5e0dac8..c494c1ad1c 100644
--- a/src/test/ssl/t/SSLServer.pm
+++ b/src/test/ssl/t/SSLServer.pm
@@ -109,6 +109,9 @@ sub configure_test_server_for_ssl
$node->psql('postgres', "CREATE USER yetanotheruser");
$node->psql('postgres', "CREATE DATABASE trustdb");
$node->psql('postgres', "CREATE DATABASE certdb");
+ $node->psql('postgres', "CREATE DATABASE certdb_dn");
+ $node->psql('postgres', "CREATE DATABASE certdb_dn_re");
+ $node->psql('postgres', "CREATE DATABASE certdb_cn");
$node->psql('postgres', "CREATE DATABASE verifydb");
# Update password of each user as needed.
@@ -217,7 +220,20 @@ sub configure_hba_for_ssl
"hostssl verifydb yetanotheruser $servercidr $authmethod clientcert=verify-ca\n";
print $hba
"hostssl certdb all $servercidr cert\n";
+ print $hba
+ "hostssl certdb_dn all $servercidr cert clientname=DN map=dn\n",
+ "hostssl certdb_dn_re all $servercidr cert clientname=DN map=dnre\n",
+ "hostssl certdb_cn all $servercidr cert clientname=CN map=cn\n";
close $hba;
+
+ # Also set the ident maps. Note: fields with commas must be quoted
+ open my $map, ">", "$pgdata/pg_ident.conf";
+ print $map
+ "# MAPNAME SYSTEM-USERNAME PG-USERNAME\n",
+ "dn \"CN=ssltestuser-dn,OU=Testing,OU=Engineering,O=PGDG\" ssltestuser\n",
+ "dnre \"/^.*OU=Testing,.*\$\" ssltestuser\n",
+ "cn ssltestuser-dn ssltestuser\n";
+
return;
}
