From 6d7a6feac48b1970c4cd127ee65d4c487acbb5e9 Mon Sep 17 00:00:00 2001 From: Andrew Dunstan Date: Mon, 29 Mar 2021 15:31:22 -0400 Subject: [PATCH] Allow matching the DN of a client certificate for authentication Currently we only recognize the Common Name (CN) of a certificate's subject to be matched against the user name. Thus certificates with subjects '/OU=eng/CN=fred' and '/OU=sales/CN=fred' will have the same connection rights. This patch provides an option to match the whole Distinguished Name (DN) instead of just the CN. On any hba line using client certificate identity, there is an option 'clientname' which can have values of 'DN' or 'CN'. The default is 'CN', the current procedure. The DN is matched against the RFC2253 formatted DN, which looks like 'CN=fred,OU=eng'. This facility of probably best used in conjunction with an ident map. Discussion: https://postgr.es/m/92e70110-9273-d93c-5913-0bccb6562740@dunslane.net Reviewed-By: Michael Paquier, Daniel Gustafsson, Jacob Champion --- doc/src/sgml/client-auth.sgml | 24 ++++++++++- src/backend/libpq/auth.c | 34 ++++++++++++--- src/backend/libpq/be-secure-openssl.c | 61 ++++++++++++++++++++++++--- src/backend/libpq/be-secure.c | 5 ++- src/backend/libpq/hba.c | 31 ++++++++++++++ src/include/libpq/hba.h | 7 +++ src/include/libpq/libpq-be.h | 1 + src/test/ssl/Makefile | 9 +++- src/test/ssl/client-dn.config | 16 +++++++ src/test/ssl/ssl/client-dn.crt | 19 +++++++++ src/test/ssl/ssl/client-dn.key | 27 ++++++++++++ src/test/ssl/t/001_ssltests.pl | 34 ++++++++++++++- src/test/ssl/t/SSLServer.pm | 16 +++++++ 13 files changed, 266 insertions(+), 18 deletions(-) create mode 100644 src/test/ssl/client-dn.config create mode 100644 src/test/ssl/ssl/client-dn.crt create mode 100644 src/test/ssl/ssl/client-dn.key 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 database user - 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 clientcert, which can be specified in any hostssl record. This option can be set to verify-ca or @@ -612,6 +612,28 @@ hostnogssenc database userhostssl entries. + + On any record using client certificate authentication (i.e. one + using the cert authentication method or one + using the clientcert option), you can specify + which part of the client certificate credentials to match using + the clientname option. This option can have one + of two values. If you specify clientname=CN, which + is the default, the username is matched against the certificate's + Common Name (CN). If instead you specify + clientname=DN the username is matched against the + entire Distinguished Name (DN) of the certificate. + This option is probably best used in conjunction with a username map. + The comparison is done with the DN in + RFC 2253 + format. To see the DN of a client certificate + in this format, do + +openssl x509 -in myclient.crt -noout --subject -nameopt RFC2253 | sed "s/^subject=//" + + Care needs to be taken when using this option, especially when using + regular expression matching against the DN. + diff --git a/src/backend/libpq/auth.c b/src/backend/libpq/auth.c index 994251e7d9..9dc28e19aa 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 = NULL; Assert(port->ssl); + /* select the correct field to compare */ + switch (port->hba->clientcertname) + { + case clientCertDN: + peer_username = port->peer_dn; + break; + case clientCertCN: + 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))); + switch (port->hba->clientcertname) + { + case clientCertDN: + ereport(LOG, + (errmsg("certificate validation (clientcert=verify-full) failed for user \"%s\": DN mismatch", + port->user_name))); + break; + case clientCertCN: + 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..40deab13c7 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,47 @@ 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); + if (BIO_get_mem_ptr(bio, &bio_buf) <= 0) + { + BIO_free(bio); + pfree(port->peer_cn); + port->peer_cn = NULL; + return -1; + } + 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 +663,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/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..f2f6a62dad --- /dev/null +++ b/src/test/ssl/ssl/client-dn.crt @@ -0,0 +1,19 @@ +-----BEGIN CERTIFICATE----- +MIIDBjCCAe4CAQEwDQYJKoZIhvcNAQELBQAwQjFAMD4GA1UEAww3VGVzdCBDQSBm +b3IgUG9zdGdyZVNRTCBTU0wgcmVncmVzc2lvbiB0ZXN0IGNsaWVudCBjZXJ0czAe +Fw0yMTAzMDUyMDUyNDVaFw00ODA3MjEyMDUyNDVaMFAxDTALBgNVBAoMBFBHREcx +FDASBgNVBAsMC0VuZ2luZWVyaW5nMRAwDgYDVQQLDAdUZXN0aW5nMRcwFQYDVQQD +DA5zc2x0ZXN0dXNlci1kbjCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEB +AMRLriq2Sh8+N4bhVtRUp/MAEsLQK6u/GotMSmiSr9K31YBYOvNzw8liKt4Rmnh5 +zmsdXJBW8erPNpkUAy9tFRCAx0YobhWCSfyX3orEdrhDrLFihA62zXQC69T0u4Yp +PSXGd0yCAcOZERQ4CQVgqnsh7Kmx5QaQnqxaz4OVPArWFJP4RQBT/l+r+kCeAn6h +qvbSbxY3FoCElQq0EF5x1F2pjL+HcBvjeI+GP430gVeJJX0RaG14Fp4v9MQT6zv/ +gvvjHC8l7YSJUROjeUzLZpUnj/ik4yrtT4av/TDGTSOpGs5qEATqk4hxAUEWw6TJ +RoLh3Oq2N5KuzDmKBBskLX0CAwEAATANBgkqhkiG9w0BAQsFAAOCAQEAL2H54oyx +pNkcgFF79lwc4c/Jda7j0wrZQIw5CWwO0MdCozJGRIEAA5WXA8b5THo1ZkaWv+sh +lWnCOflBtGnEpD7dUpMW9lxGL5clMeMf3CoNYBb7zBofm+oTJytCzXHNftB4hCZj +pvN79bNT4msWbmxDyi75nfbEfzK1BKnfCg+DWBBjEnHC8VzgDq6ACN6FEoyFb+fr +dlDoof+S7k8jYAzhxwySI5DnMzr9OIwnepWfx9HENsasAighc8vFSEouShvsOlYS +L0OIb9Tn6M5q1tWoLHulQsQYDPzaO/1M7ubsr5xCx1ReDK4gaNwS3YXn/2KE9Kco +aKCrL89AjQrJPA== +-----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..1d67ef0408 --- /dev/null +++ b/src/test/ssl/ssl/client-dn.key @@ -0,0 +1,27 @@ +-----BEGIN RSA PRIVATE KEY----- +MIIEowIBAAKCAQEAxEuuKrZKHz43huFW1FSn8wASwtArq78ai0xKaJKv0rfVgFg6 +83PDyWIq3hGaeHnOax1ckFbx6s82mRQDL20VEIDHRihuFYJJ/JfeisR2uEOssWKE +DrbNdALr1PS7hik9JcZ3TIIBw5kRFDgJBWCqeyHsqbHlBpCerFrPg5U8CtYUk/hF +AFP+X6v6QJ4CfqGq9tJvFjcWgISVCrQQXnHUXamMv4dwG+N4j4Y/jfSBV4klfRFo +bXgWni/0xBPrO/+C++McLyXthIlRE6N5TMtmlSeP+KTjKu1Phq/9MMZNI6kazmoQ +BOqTiHEBQRbDpMlGguHc6rY3kq7MOYoEGyQtfQIDAQABAoIBABqL3Zb7JhUJlfrQ +uKxocnojdWYRPwawBof2Hk38IHkP0XjU9cv8yOqQMxnrKYfHeUn1I5KFn5vQwCJ9 +mVytlN6xe8GaMCEKiLT3WOpNXXzX8h/fIdrXj/tzda9MFZw0MYfNSk73egOYzL1+ +QoIOq5+RW+8rFr0Hi93lPhEeeotAYWDQgx9Ye/NSW6vK2m47hdBKf9SBsWs+Vafa +mC9Bf4LQqRYSJZee1zDwIh+Om7/JTsjMZYU0/lpycRz7V5uHbamXKlOXF54ow3Wn +CJ9eVVWo7sb3CaeJ0p2sHIFp89ybMQ2vvmNr6aJNtZWd5WYxsjKs40rVq6DiUlFn +T6CK7uECgYEA/Ks4/OnZnorhaHwYTs0LqiPSM7oZw4qchCNDMoE3WngsaZoWUKmr +2JTY6uYP/B+oWgwPBdDiPRDeGqtVNZSAVsZEDMbiqZxwHaLi9OKJ7sKgK8Q6ANV1 +q5qgH1yXXygWhlol/Nf9bbnGWWoN+33zvnADeKRcT/1gZLEQpJ46DHUCgYEAxuIx +k/EOOT9kyC5WrBDY3l7veb/WGRQgXTXiCJaO4d7IYh8UpUXlg0ZYF4RfeKRsSd07 +n9QdW6ImrtDloNyG6HnDknYsPRUs8JcuuyrxaOsZ/p9LS76ItNV7gzREf4N/7jrD +c6TJappgXm+dgXg6ENuyk05hzjT6qdvm9V80m+kCgYEA7kfXRYSP61lT/AJTtjTf +FEQV3xxZYbRdqKvMmluLxTDhyXE8LDPm0SiGbPgsCPwd+1W18SktwqMeoo4DnLUA +V1VBJb+GUKgsf3Z2jLT7mYRIIx46CUFFaGE5MnpScrXOkEOB4bIb2RfCu94tc4gz +jtv6GhL+z5zHBA6MAIMLgWUCgYAlynNLPkHKpP4cf5mehnD/CCEPDGG9UDK6I3P4 +18r8pl2DL463vOlYoXQ5u8B8ZxngizY6L48Ii244R59qipzj7cc4vFW5oZ1xdfi+ +PfGzUwEUfeZL1T+axPn8O2FMrYsQlH/xKH3RUNZA+4p9QIAgFe7/yKQTD8QVpKBl +PZr8iQKBgBjdrgMt1Az98ECXJCjM4uui2S9UenNQVmhmxgZUpHqfNk+WEvIIthDi +FEJPSTHyhTI9XIrhhwNkW3UZMjMndAiNylXGfJdr/xGwLM57t5HhGgljSboV7Mnw +RFnh2FZxa3i/8g+4lAPZNwU0W/JU46wgg4C2Eu/Ne7jA8XUXYu9t +-----END RSA PRIVATE KEY----- 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; } -- 2.30.2