On 1/29/21 10:10 AM, Andrew Dunstan wrote: > On 1/28/21 5:10 PM, Andrew Dunstan wrote: >>> (I'd still recommend switching to use the RFC >>> flag to OpenSSL, to ease future improvements.) There should be a bunch >>> of warning documentation saying not to do anything more complex unless >>> you're an expert, and that the exact regular expression needed may >>> change depending on the TLS backend. >> I'll play around with the RFC flag. >> >> >>> If you want to add UTF-8 support to the above, so that things outside >>> ASCII can be matched nicely, then the ASN1_STRFLGS_ESC_MSB flag should >>> be removed from the _print_ex() call per OpenSSL documentation, and we >>> should investigate how it plays with other non-UTF-8 database >>> encodings. That seems like work but not a huge amount of it. >> How about if we remove ASN1_STRFLGS_ESC_MSB when the server encoding is >> UTF8? That should be an extremely simple change. >> >> >> > Of course, we don't have any idea what the database is at this stage, so > that wasn't well thought out. > > > I'm inclined at this stage just to turn on RFC2253. If someone wants to > deal with UTF8 (or other encodings) at a later stage they can. > >
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. Finding out how to generate the new cert without regenerating everything else was also fun :-) Here's what I did in a pristine source with patch applied, but where configure hasn't been run: cd src/test/ssl touch ../../Makefile.global rm -f ssl/client-dn.crt ssl/client-dn.key touch ssl/root_ca-certindex echo 01> ssl/root_ca.srl make ssl/client-dn.crt rm -rf ssl/*certindex* ssl/root_ca.srl ssl/new_certs_dir rm ../../Makefile.global Making incremental additions to the certificate set easier wouldn't be a bad thing. I wonder if we should really be setting 1 as the serial number, though. Might it not be better to use, say, `date +%Y%m%d01` rather like we do with catalog version numbers? 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 c4b9971a20..948c4f8aab 100644 --- a/doc/src/sgml/client-auth.sgml +++ b/doc/src/sgml/client-auth.sgml @@ -596,7 +596,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 @@ -610,6 +610,19 @@ 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, that is 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 comjunction with a username map. + </para> </listitem> </varlistentry> </variablelist> diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c index 545635f41a..d2e4c0b8a8 100644 --- a/src/backend/libpq/auth.c +++ b/src/backend/libpq/auth.c @@ -2848,12 +2848,15 @@ static int CheckCertAuth(Port *port) { int status_check_usermap = STATUS_ERROR; + char *peer_username; Assert(port->ssl); /* Make sure we have received a username in the certificate */ - if (port->peer_cn == NULL || - strlen(port->peer_cn) <= 0) + peer_username = port->hba->clientcertname == clientCertCN ? port->peer_cn : port->peer_dn; + + if (peer_username == NULL || + strlen(peer_username) <= 0) { ereport(LOG, (errmsg("certificate authentication failed for user \"%s\": client certificate contains no user name", @@ -2861,8 +2864,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) { /* @@ -2873,7 +2876,7 @@ 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", + (errmsg("certificate validation (clientcert=verify-full) failed for user \"%s\": CN/DN mismatch", port->user_name))); } } diff --git a/src/backend/libpq/be-secure-openssl.c b/src/backend/libpq/be-secure-openssl.c index 1e2ecc6e7a..83c0eb5006 100644 --- a/src/backend/libpq/be-secure-openssl.c +++ b/src/backend/libpq/be-secure-openssl.c @@ -523,22 +523,25 @@ 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 / 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_cn; + 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) { @@ -562,6 +565,36 @@ aloop: port->peer_cn = peer_cn; } + + bio = BIO_new(BIO_s_mem()); + if (!bio) + { + pfree(port->peer_cn); + port->peer_cn = NULL; + return -1; + } + /* 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; + } + + BIO_free(bio); + + port->peer_dn = peer_dn; + port->peer_cert_valid = true; } @@ -590,6 +623,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 4cf139a223..96293354d2 100644 --- a/src/backend/libpq/be-secure.c +++ b/src/backend/libpq/be-secure.c @@ -120,7 +120,7 @@ secure_open_server(Port *port) ereport(DEBUG2, (errmsg("SSL connection from \"%s\"", - port->peer_cn ? port->peer_cn : "(anonymous)"))); + port->peer_dn ? port->peer_dn : "(anonymous)"))); #endif return r; diff --git a/src/backend/libpq/hba.c b/src/backend/libpq/hba.c index 371dccb852..85d6f359bb 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..443b3560ef 100644 --- a/src/include/libpq/hba.h +++ b/src/include/libpq/hba.h @@ -69,7 +69,13 @@ typedef enum ClientCertMode clientCertOff, clientCertCA, clientCertFull -} ClientCertMode; +} ClientCertMode; + +typedef enum ClientCertName +{ + clientCertCN, + clientCertDN +} ClientCertName; typedef struct HbaLine { @@ -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 66a8673d93..c0988d2404 100644 --- a/src/include/libpq/libpq-be.h +++ b/src/include/libpq/libpq-be.h @@ -190,6 +190,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 93335b1ea2..43152a6d06 100644 --- a/src/test/ssl/Makefile +++ b/src/test/ssl/Makefile @@ -18,7 +18,7 @@ export with_openssl 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) \ @@ -88,6 +88,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/client-dn.config b/src/test/ssl/client-dn.config new file mode 100644 index 0000000000..ee2df54bd6 --- /dev/null +++ b/src/test/ssl/client-dn.config @@ -0,0 +1,16 @@ +# An OpenSSL format CSR config file for creating a client certificate. +# +# The certificate is for user "ssltestuser-dn" with a multi-part DN + +[ req ] +distinguished_name = req_distinguished_name +prompt = no + +[ req_distinguished_name ] +O = PGDG +0.OU = Engineering +1.OU = Testing +CN = ssltestuser-dn + +# no extensions in client certs +[ v3_req ] diff --git a/src/test/ssl/ssl/client-dn.crt b/src/test/ssl/ssl/client-dn.crt new file mode 100644 index 0000000000..a5343205a9 --- /dev/null +++ b/src/test/ssl/ssl/client-dn.crt @@ -0,0 +1,19 @@ +-----BEGIN CERTIFICATE----- +MIIDBjCCAe4CAQIwDQYJKoZIhvcNAQELBQAwQjFAMD4GA1UEAww3VGVzdCBDQSBm +b3IgUG9zdGdyZVNRTCBTU0wgcmVncmVzc2lvbiB0ZXN0IGNsaWVudCBjZXJ0czAe +Fw0yMTAxMzAyMDMyMTZaFw00ODA2MTcyMDMyMTZaMFAxDTALBgNVBAoMBFBHREcx +FDASBgNVBAsMC0VuZ2luZWVyaW5nMRAwDgYDVQQLDAdUZXN0aW5nMRcwFQYDVQQD +DA5zc2x0ZXN0dXNlci1kbjCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEB +ANYurUlM+KcR1FwC+6sQ2AFF5peLK31mZ0/gSEHkVXFsjxhKTB7BgLXK/aKIW0vo +VUVSbTor+wiLmNl0yVWPBKBtZzU1Ye0dG+sCoIUlvcBXN2NLI5G6cN75KNMNW12/ +Ju8opzqtD8vdHAMwuhESGkdg6QDrfMk/qD1T0d54fyWEBDChIjjdwGIiWNaYGQ1q +jG2CxlG6WB+vSoaSuLKSWjK4EZH3d2eEzlfGk3GmOFeAyUQpk2GD2/msWlapF4N1 +Qb1OmgEOuggIpLTOO9G9pfu1ctG5DgbtJd8w0TMgfK1ua9lgMme3MOYnruXHnHBq +cCQxu68z6t3m3mp9EaUSr7MCAwEAATANBgkqhkiG9w0BAQsFAAOCAQEAGkzqur0R +QmuBaCZOlXNZWuN+TyMa9es/lf0sLY+B9KYBzvTzYeB6QGrxUxfbcqBJHXjuseif +yxgzR/c/nW0QU4cFjdSc9q422Anr5KexrRPJTco1haZWJ+G+lY+OzfZsJ2RUDfhM +sWWrEzCzEdpFZ7iY5f6XrvcQo11DwKBTG6i62BYJb6vOYoC4ydCE5QedL5gJMoQS +jSpBjKWit1qW4vppopIYdG66A+xrMLUS+uqSVwyaoc//QYNL29WOauXxlqnxL0mG +LCLHgotMZeBD48vQyXlK8FsBVccbWgwWid02DSMliUdpsD2fEtmQB8Z8RaPfAOjC +zS06yylazfoT3A== +-----END CERTIFICATE----- diff --git a/src/test/ssl/ssl/client-dn.key b/src/test/ssl/ssl/client-dn.key new file mode 100644 index 0000000000..68e81ce043 --- /dev/null +++ b/src/test/ssl/ssl/client-dn.key @@ -0,0 +1,27 @@ +-----BEGIN RSA PRIVATE KEY----- +MIIEowIBAAKCAQEA1i6tSUz4pxHUXAL7qxDYAUXml4srfWZnT+BIQeRVcWyPGEpM +HsGAtcr9oohbS+hVRVJtOiv7CIuY2XTJVY8EoG1nNTVh7R0b6wKghSW9wFc3Y0sj +kbpw3vko0w1bXb8m7yinOq0Py90cAzC6ERIaR2DpAOt8yT+oPVPR3nh/JYQEMKEi +ON3AYiJY1pgZDWqMbYLGUbpYH69KhpK4spJaMrgRkfd3Z4TOV8aTcaY4V4DJRCmT +YYPb+axaVqkXg3VBvU6aAQ66CAiktM470b2l+7Vy0bkOBu0l3zDRMyB8rW5r2WAy +Z7cw5ieu5ceccGpwJDG7rzPq3ebean0RpRKvswIDAQABAoIBAEgZDFIJbAZpVQ/o +HSmXkO7UxeureGdNCmfz+r7bivuUbJLjp74OqzIG89w7hGgH/HRKa+RSG73jp/9D +deasLwWF7mEV6DH2Q4iXqMtJSheuBEITcBTFKuuT+e8ZpvDmwMdu6uQwj4mzk5Nr +WqcWbewrLiQWITppiEukpJf7/ej1awFX1z4yOb46bBBq6WfzmOki4wtLWL0fgm6r +9ovbEyAoYEhe5c5Uz8yvBMq8rUWBN5kDMKQncGFc89gIsfvoq5l8OPCPcYu6N3SC +QaI7dDnLUxKJ4ODZRRmgUzuKDZrBkmQ52wTi/tte0y6pFTWwU5ju0TDDT8BFKKoF +Jp7Qw+kCgYEA62PKnXeYIHSzqOd9eJhfnqGMVVw5Y+NuF4J2TAv6jZetLwz7gYB9 +28f4UlJktjwQweb1kTvP8XBGtkDQS19xrmhcGYHbMHOQIbQmQQNjwZZXcQ0FvcO3 +LhyeikdPo2R3pNLzZuHHg6pp5v79+jssXeYbGXvKPVFCw8lI+oeSG48CgYEA6O+G +ULK5+wnNfXUn524Ug/sxrbRvO+usHSPbAlLcjeTW3qiApwO3lb7lyDuErFUspF23 +WT6L4xnIugpmHO8Cklf6YFIsMNRDALDSnKHk/Hu+zIqiNzlPi0YQq+cCJEYDs+pU +EErn0tmS1sW/V5t4GblmvGJ/LK2ZFK+wCe7rJ50CgYA61uEI56IxSrq2F9d3U69j +OcKYe8skuu8EFWp4q+3fgvCZeEdOIc5UJ/JcsZfXLcCKl67+tNLP6V7jo+PtU1hZ +XmDXR2yA+gInSp7dVXmUJH6LFdQ/kTKy5hiDPDwd/bkijFCngPycXvbF9SuLZ0s5 +1ZEkl5sAJNXpluEVLtpI5QKBgAj3p6kVqFlEwFdzGi0rrLiEBB6W0q1w3jhk9/p3 +7Cu+QpNh00oat4eZSMlTmUD1KnnNbdCOut3sUTDwU4wLm4K8xlPM9gyPL2EobYNA +LEuYC/ld4O7VUv2eneewRgHVfDEB9WiHKbORUrjX9gzOXGpJG+5msFSs/jawqMtJ +Gl51AoGBAIYI8W/qJ1G4caDJWHEErjr5DsH5PaT58RhzqFEw/1oWGWXoyqoh5vLB +1nyZGrwcPrnfHzHkGn0P6pWgRQw0tPQW+kTw5q+9b6VsKXlS0jOUXILQ7X9SqAgc +gN2vIJcOTeWm3PF8Q90qweH+R2E75jffPvKIC+s1LXY8mDNjS3ib +-----END RSA PRIVATE KEY----- diff --git a/src/test/ssl/t/001_ssltests.pl b/src/test/ssl/t/001_ssltests.pl index fd2727b568..50d43e71b4 100644 --- a/src/test/ssl/t/001_ssltests.pl +++ b/src/test/ssl/t/001_ssltests.pl @@ -13,7 +13,7 @@ use SSLServer; if ($ENV{with_openssl} eq 'yes') { - plan tests => 93; + plan tests => 96; } else { @@ -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") @@ -435,6 +435,37 @@ 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; +$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" +); + +# same thing but with a regex +$dn_connstr =~ s/certdb_dn/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 =~ s/certdb_dn_re/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 f5987a003e..49aa1fb42b 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. @@ -205,7 +208,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; }