Compare DNs by comparing their string representations

A binary comparison will not work in case the contents is the same but
the ASN.1 type differ (e.g. PrintableString vs UTF8String). Such
variations are permitted so we need to handle them.

Signed-off-by: Pierre Ossman <ossman@cendio.se>

diff --git a/.gitignore b/.gitignore
index 7c397c517cf3f21dd0b359f2ae8a36ffca031950..c72694eba75f235eca15d1361fd8101e4a0947b9 100644 (file)
--- a/.gitignore
+++ b/.gitignore
@@ -870,6 +870,7 @@ tests/x509-extensions
 tests/x509-verify-with-crl
 tests/x509_altname
 tests/x509cert
+tests/x509cert-dntypes
 tests/x509cert-invalid
 tests/x509cert-tl
 tests/x509dn

diff --git a/lib/pkcs11.c b/lib/pkcs11.c
index 8b65212a50cd439ff526ced58f7db05675464a04..d03bf6e44406a4094a9737535de5bf757d827dc7 100644 (file)
--- a/lib/pkcs11.c
+++ b/lib/pkcs11.c
@@ -4141,6 +4141,8 @@ find_cert_cb(struct ck_function_list *module, struct pkcs11_session_info *sinfo,
                        a_vals++;
                }
 
+               /* This doesn't do a proper comparison, see
+                * _gnutls_x509_compare_raw_dn() */
                if (priv->dn.size > 0) {
                        a[a_vals].type = CKA_SUBJECT;
                        a[a_vals].value = priv->dn.data;
@@ -4155,6 +4157,7 @@ find_cert_cb(struct ck_function_list *module, struct pkcs11_session_info *sinfo,
                        a_vals++;
                }
 
+               /* Same problem as for priv->dn */
                if (priv->issuer_dn.size > 0) {
                        a[a_vals].type = CKA_ISSUER;
                        a[a_vals].value = priv->issuer_dn.data;

diff --git a/lib/x509/dn.c b/lib/x509/dn.c
index 8d428e9b3f603d57f55fbf7cea20ce6c79d4569d..33739e2271a4c50c072423157befa6722f0552d6 100644 (file)
--- a/lib/x509/dn.c
+++ b/lib/x509/dn.c
@@ -982,13 +982,56 @@ int
 _gnutls_x509_compare_raw_dn(const gnutls_datum_t * dn1,
                            const gnutls_datum_t * dn2)
 {
+       int ret;
+       gnutls_datum_t str1, str2;
+
+       /* Simple case of completely identical? */
+
+       if (dn1->size == dn2->size) {
+               if (memcmp(dn1->data, dn2->data, dn2->size) == 0) {
+                       return 1;
+               }
+       }
+
+       /* RFC5280 (https://tools.ietf.org/html/rfc5280#section-7.1)
+        * requires that the LDAP StringPrep profile and caseIgnoreMatch
+        * must be used for this comparison. We do not use that but
+        * instead we do a simpler comparison that ignores the tags used
+        * such as `UTF8String` and `PrintableString`. */
 
-       if (dn1->size != dn2->size) {
+       if ((dn1->size == 0) || (dn2->size == 0)) {
+               gnutls_assert();
+               return 0;
+       }
+
+       ret = gnutls_x509_rdn_get2(dn1, &str1, 0);
+       if (ret < 0) {
+               gnutls_assert();
                return 0;
        }
-       if (memcmp(dn1->data, dn2->data, dn2->size) != 0) {
+
+       ret = gnutls_x509_rdn_get2(dn2, &str2, 0);
+       if (ret < 0) {
                gnutls_assert();
+               _gnutls_free_datum(&str1);
                return 0;
        }
-       return 1;               /* they match */
+
+       if (str1.size != str2.size) {
+               ret = 0;
+               goto cleanup;
+       }
+       if (memcmp(str1.data, str2.data, str2.size) != 0) {
+               gnutls_assert();
+               ret = 0;
+               goto cleanup;
+       }
+
+       ret = 1;                /* they match */
+
+cleanup:
+       _gnutls_free_datum(&str1);
+       _gnutls_free_datum(&str2);
+
+       return ret;
 }

diff --git a/tests/Makefile.am b/tests/Makefile.am
index b5a3eb673e7643451e6d61a3a39dfbb30d8e757e..16bf7fa7ee96f7f21fea8e36ea4f3ecf412c5214 100644 (file)
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -218,7 +218,7 @@ ctests += mini-record-2 simple gnutls_hmac_fast set_pkcs12_cred cert certuniquei
         sign-verify-newapi sign-verify-deterministic iov aead-cipher-vec \
         tls13-without-timeout-func buffer status-request-revoked \
         set_x509_ocsp_multi_cli kdf-api keylog-func \
-        dtls_hello_random_value tls_hello_random_value
+        dtls_hello_random_value tls_hello_random_value x509cert-dntypes
 
 if HAVE_SECCOMP_TESTS
 ctests += dtls-with-seccomp tls-with-seccomp dtls-client-with-seccomp tls-client-with-seccomp

diff --git a/tests/x509cert-dntypes.c b/tests/x509cert-dntypes.c
new file mode 100644 (file)
index 0000000..10d7950
--- /dev/null
+++ b/tests/x509cert-dntypes.c
@@ -0,0 +1,134 @@
+/*
+ * Copyright (C) 2020 Pierre Ossman for Cendio AB
+ *
+ * Author: Pierre Ossman
+ *
+ * This file is part of GnuTLS.
+ *
+ * GnuTLS is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * GnuTLS is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with GnuTLS; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/types.h>
+#include <unistd.h>
+#include <gnutls/gnutls.h>
+#include <gnutls/x509.h>
+
+#include "utils.h"
+
+static void tls_log_func(int level, const char *str)
+{
+       fprintf(stderr, "<%d>| %s", level, str);
+}
+
+/* the issuer/subject connection between the server cert and the CA
+ * cert uses different ASN.1 types, which is uncommon but allowed */
+
+static unsigned char server_pem[] =
+       "-----BEGIN CERTIFICATE-----\n"
+       "MIIDZTCCAc2gAwIBAgIUB7aVTQvtbBpOEtKELkBkLViM0eIwDQYJKoZIhvcNAQEL\n"
+       "BQAwEjEQMA4GA1UEAwwHVGVzdCBDQTAeFw0yMDAzMjYxMDE4NTdaFw0yMTAzMjYx\n"
+       "MDE4NTdaMBYxFDASBgNVBAMMC1Rlc3QgY2xpZW50MIIBIjANBgkqhkiG9w0BAQEF\n"
+       "AAOCAQ8AMIIBCgKCAQEAviqj5S/xe39agbMnq/oPAQmdIhalB17Ewc3AZlD8n+zQ\n"
+       "scPDNvnk4gxSeSXePtXmh0OaGcBKbMAkjiyo2gPBmV3ay34LQuk97nJxE2TUAWMm\n"
+       "S8yFwP3yoE+GZ5eYjv+HGQxeAP9uHLjho/jHjVGgUOCVv1QjsKyRx8Tuvy9TH3ON\n"
+       "DuMPw3Jmnq0OhLy2+SjU0ug5jxfWJvnfeGoFzRgalmWGyoAQsH9bqha/D44QSen+\n"
+       "Zbbt/A4uNIILAENYuHXEfvpmBuZPpocOb6h2huGbp6iHZfdZUHso37UmWT6PXh+2\n"
+       "dASPaCpAr3bURBhnEsQM43njb8METZewMeoQxwZC0QIDAQABoy8wLTAMBgNVHRMB\n"
+       "Af8EAjAAMB0GA1UdDgQWBBSb3h7ZbajS/2RWx2a7hTVSkur0FDANBgkqhkiG9w0B\n"
+       "AQsFAAOCAYEAPfwyvOwNEjIvlifjBVhiWmrtZAS2YaY9jqFnaA2PvYY2FVyC3AMu\n"
+       "3BGAorau/4DL3P92/9SlygEmBQpqCq+AJnQRH6WKFT4avAOmw3yc0++st+DhGK0I\n"
+       "6Cr69WccVi0Kmxi1XP4dpPDWSuVCOP6rGc3ulgEH83xF4ZL+3qVA9Fihsie3ZZme\n"
+       "7mqWOznVO1MZHLDFIUEoRdOSin5bIkl7FPOCZqMsWRM41GuA1h4aX/X5dLeqRW1c\n"
+       "mJ5CNRWwPIPcwgqeldFnx07svCv9QseUDaIw+C9vZOlgfIgp0qeYoR6fsD38WcUC\n"
+       "eJPsOUwhdhMcw+/PM16iwzd89dI+PCecFY9FeLh9YeihZm0DnG8L0To1Y2ry+WRf\n"
+       "w5knR3FReHPcelymvSKZSEG0d/KKHXBeKWgcrCrdnn4ya71eblsNzO3vnxB5k0Zj\n"
+       "WcQ3wfeftQKDEIuaRHUP6B4zx2teJWMWvJLcXuavoqo0z3L5EN74RztCpnP9ykSH\n"
+       "ZsYWoJ3aelFv\n"
+       "-----END CERTIFICATE-----\n";
+
+static unsigned char ca_pem[] =
+       "-----BEGIN CERTIFICATE-----\n"
+       "MIID5DCCAkygAwIBAgIUB4lnLAeQ20wlYbqt5ykgvWOPNzgwDQYJKoZIhvcNAQEL\n"
+       "BQAwEjEQMA4GA1UEAxMHVGVzdCBDQTAeFw0yMDAzMjYxMDI0MjhaFw0yMDAzMjcx\n"
+       "MDI0MjhaMBIxEDAOBgNVBAMTB1Rlc3QgQ0EwggGiMA0GCSqGSIb3DQEBAQUAA4IB\n"
+       "jwAwggGKAoIBgQCt9z/noU7qCPquzzgwNvu/rwXyIvxmqdWhpfpBOmVq8wpgUDUU\n"
+       "cQ94F65UfTo3EcYXCoDs43E4Wo8KmF5YQM2xK+LrH28XmpL3z+NoQGaZoUVrMWp6\n"
+       "rbIeoGZvITaaGn2uEbGT7iRkBUdS4wOjUT13IxpG8cM4d0i0DIsqSlUPnQCfyMqf\n"
+       "jsVhO9IQsn7qMo0+2nNCI5JqblEXRvL39hHzJMOsq1NRqZO1Zjt9HCIB7m7Q42Jx\n"
+       "e8zm7RzTiBFVKecxb5h4mmt3tUZQ0Kjd94yE6ARSE0rULmO+6H7hgI6sU8vqfSFe\n"
+       "DimQ5mPReumBRDcErX+c7bRGPRul41kAB8XvPmAHG8xCepjH8xrgY/FeVBQT74xm\n"
+       "MEYQaxdGpa8Azx6MZCrZOI0rzu+zI0CBQGE1h1Xk8HBozrn/G2OOAZcXyzHzq56R\n"
+       "Z52zEQYFZmKH9tHTDI6fMfo8clr7esb/wmgEOt/lJYE9IMJrzUh+IwWuowdYaDVj\n"
+       "nMrboUBVepmBKSUCAwEAAaMyMDAwDwYDVR0TAQH/BAUwAwEB/zAdBgNVHQ4EFgQU\n"
+       "rhkYiczAkbCcVfNr67VGGaqilbgwDQYJKoZIhvcNAQELBQADggGBAGYiUTKdYBXk\n"
+       "lZFIhZkCc33/lCgJw2mSrdAd+xJmJonRPy3qmYy3HniOmQdRVqResLALubz89VjJ\n"
+       "dSeokujFrlNtb4CygojseqTsxWgeZlKjLU3tJ/Xn+DFIiP7k9+WPW7KFIIW0fq61\n"
+       "MAI0lKjqpC8sJTlXoJemDw9MW/380nKr+K1YY3arRzsSHEIeA54xOggKEwvgz11A\n"
+       "47xT83WoLwFQ4e9LZfCsL/M51lsLHAlJzDKyTTeSxCi/C6kUIzx8QyxHKYgBuNxz\n"
+       "8vVLY/YzUv/l5ELYQ9gkAX0vZWdw7pqASUY8yvbzImrWqjFAHeN3zK687Ke9uppS\n"
+       "dmjvPwvTK+SKm++NR8YCwb3xqHQHMYHV3lxjlOhaN6rxBW0l4gtvb2FMlhcljiZ+\n"
+       "tF2ObVwEs6nqJSGrzubp0os+WmnbVSCaHz9jnRWb68C87mXCZkbA7FTSKJOVuqRM\n"
+       "vVTcHQ7jwGQ2/SvikndFQ53zi2j9o/jTOiFv29rEOeHu67UAiFSi2A==\n"
+       "-----END CERTIFICATE-----\n";
+
+const gnutls_datum_t server = { server_pem, sizeof(server_pem)-1 };
+const gnutls_datum_t ca = { ca_pem, sizeof(ca_pem)-1 };
+
+void doit(void)
+{
+       int ret;
+       gnutls_x509_crt_t server_crt, ca_crt;
+
+       /* this must be called once in the program
+        */
+       global_init();
+
+       gnutls_global_set_log_function(tls_log_func);
+       if (debug)
+               gnutls_global_set_log_level(6);
+
+       gnutls_x509_crt_init(&server_crt);
+
+       ret =
+           gnutls_x509_crt_import(server_crt, &server, GNUTLS_X509_FMT_PEM);
+       if (ret < 0)
+               fail("gnutls_x509_crt_import");
+
+       gnutls_x509_crt_init(&ca_crt);
+
+       ret =
+           gnutls_x509_crt_import(ca_crt, &ca, GNUTLS_X509_FMT_PEM);
+       if (ret < 0)
+               fail("gnutls_x509_crt_import");
+
+       ret = gnutls_x509_crt_check_issuer(server_crt, ca_crt);
+       if (!ret)
+               fail("gnutls_x509_crt_check_issuer");
+
+       gnutls_x509_crt_deinit(ca_crt);
+       gnutls_x509_crt_deinit(server_crt);
+
+       gnutls_global_deinit();
+
+       if (debug)
+               success("success");
+}