769039
From 66c96b915dd4a82ebd4228cba61d7c4bae96cbca Mon Sep 17 00:00:00 2001
769039
From: Fraser Tweedale <ftweedal@redhat.com>
769039
Date: Fri, 16 Mar 2018 15:16:56 +1000
769039
Subject: [PATCH] Ticket 49543 - fix certmap dn comparison
769039
769039
Bug Description: Differences in DN string representations between
769039
the value included in certmap.conf, and the stringified value of the
769039
Issuer DN produced by NSS, as well as buggy DN normalisation code in
769039
389 itself, cause 389 to wrongly reject the correct certmap
769039
configuration to use.  Authentication fails.  This behaviour was
769039
observed when there is an escaped comma in an attribute value.
769039
769039
Fix Description: Instead of comparing stringified DNs, parse the DN
769039
represented in certmap.conf into an NSS CertNAME.  Use the NSS DN
769039
comparison routine when comparing certificate Issuer DNs against the
769039
certmap configurations.  Remove the buggy DN normalisation routine.
769039
769039
https://pagure.io/389-ds-base/issue/49543
769039
769039
Author: Fraser Tweedale <ftweedal@redhat.com>
769039
769039
Review by: ???
769039
---
769039
 include/ldaputil/certmap.h   |  20 +++--
769039
 include/ldaputil/ldaputil.h  |   2 +-
769039
 lib/ldaputil/cert.c          |  27 ++++--
769039
 lib/ldaputil/certmap.c       | 162 ++++++-----------------------------
769039
 lib/ldaputil/examples/init.c |   3 +-
769039
 5 files changed, 62 insertions(+), 152 deletions(-)
769039
769039
diff --git a/include/ldaputil/certmap.h b/include/ldaputil/certmap.h
769039
index fec2dd931..50fd4d158 100644
769039
--- a/include/ldaputil/certmap.h
769039
+++ b/include/ldaputil/certmap.h
769039
@@ -16,6 +16,7 @@
769039
 /* What was extcmap.h begins ... */
769039
 
769039
 #include <ldap.h>
769039
+#include <nss3/cert.h>
769039
 
769039
 #ifndef NSAPI_PUBLIC
769039
 #define NSAPI_PUBLIC
769039
@@ -156,7 +157,7 @@ typedef int (*CertVerifyFn_t)(void *cert, LDAP *ld, void *certmap_info, LDAPMess
769039
  *  otherwise return LDAPU_CERT_MAP_INITFN_FAILED.  The server startup will be
769039
  *  aborted if the return value is not LDAPU_SUCCESS.
769039
  */
769039
-typedef int (*CertMapInitFn_t)(void *certmap_info, const char *issuerName, const char *issuerDN, const char *libname);
769039
+typedef int (*CertMapInitFn_t)(void *certmap_info, const char *issuerName, const CERTName *issuerDN, const char *libname);
769039
 
769039
 /*
769039
  * Refer to the description of the function ldapu_get_cert_ava_val
769039
@@ -209,27 +210,30 @@ extern "C" {
769039
 
769039
 NSAPI_PUBLIC int ldapu_cert_to_ldap_entry(void *cert, LDAP *ld, const char *basedn, LDAPMessage **res);
769039
 
769039
-NSAPI_PUBLIC int ldapu_set_cert_mapfn(const char *issuerDN,
769039
+NSAPI_PUBLIC int ldapu_set_cert_mapfn(const CERTName *issuerDN,
769039
                                       CertMapFn_t mapfn);
769039
 
769039
 
769039
-NSAPI_PUBLIC CertMapFn_t ldapu_get_cert_mapfn(const char *issuerDN);
769039
+NSAPI_PUBLIC CertMapFn_t ldapu_get_cert_mapfn(const CERTName *issuerDN);
769039
 
769039
-NSAPI_PUBLIC int ldapu_set_cert_searchfn(const char *issuerDN,
769039
+NSAPI_PUBLIC int ldapu_set_cert_searchfn(const CERTName *issuerDN,
769039
                                          CertSearchFn_t searchfn);
769039
 
769039
 
769039
-NSAPI_PUBLIC CertSearchFn_t ldapu_get_cert_searchfn(const char *issuerDN);
769039
+NSAPI_PUBLIC CertSearchFn_t ldapu_get_cert_searchfn(const CERTName *issuerDN);
769039
 
769039
-NSAPI_PUBLIC int ldapu_set_cert_verifyfn(const char *issuerDN,
769039
+NSAPI_PUBLIC int ldapu_set_cert_verifyfn(const CERTName *issuerDN,
769039
                                          CertVerifyFn_t verifyFn);
769039
 
769039
-NSAPI_PUBLIC CertVerifyFn_t ldapu_get_cert_verifyfn(const char *issuerDN);
769039
+NSAPI_PUBLIC CertVerifyFn_t ldapu_get_cert_verifyfn(const CERTName *issuerDN);
769039
 
769039
 
769039
 NSAPI_PUBLIC int ldapu_get_cert_subject_dn(void *cert, char **subjectDN);
769039
 
769039
 
769039
+NSAPI_PUBLIC CERTName *ldapu_get_cert_issuer_dn_as_CERTName(CERTCertificate *cert);
769039
+
769039
+
769039
 NSAPI_PUBLIC int ldapu_get_cert_issuer_dn(void *cert, char **issuerDN);
769039
 
769039
 
769039
@@ -242,7 +246,7 @@ NSAPI_PUBLIC int ldapu_free_cert_ava_val(char **val);
769039
 NSAPI_PUBLIC int ldapu_get_cert_der(void *cert, unsigned char **derCert, unsigned int *len);
769039
 
769039
 
769039
-NSAPI_PUBLIC int ldapu_issuer_certinfo(const char *issuerDN,
769039
+NSAPI_PUBLIC int ldapu_issuer_certinfo(const CERTName *issuerDN,
769039
                                        void **certmap_info);
769039
 
769039
 
769039
diff --git a/include/ldaputil/ldaputil.h b/include/ldaputil/ldaputil.h
769039
index e0e028c5c..b172819b0 100644
769039
--- a/include/ldaputil/ldaputil.h
769039
+++ b/include/ldaputil/ldaputil.h
769039
@@ -48,7 +48,7 @@ enum
769039
 typedef struct
769039
 {
769039
     char *issuerName;            /* issuer (symbolic/short) name */
769039
-    char *issuerDN;              /* cert issuer's DN */
769039
+    CERTName *issuerDN;          /* cert issuer's DN */
769039
     LDAPUPropValList_t *propval; /* pointer to the prop-val pairs list */
769039
     CertMapFn_t mapfn;           /* cert to ldapdn & filter mapping func */
769039
     CertVerifyFn_t verifyfn;     /* verify cert function */
769039
diff --git a/lib/ldaputil/cert.c b/lib/ldaputil/cert.c
769039
index 65a481541..73abba12a 100644
769039
--- a/lib/ldaputil/cert.c
769039
+++ b/lib/ldaputil/cert.c
769039
@@ -54,15 +54,30 @@ ldapu_get_cert_subject_dn(void *cert_in, char **subjectDN)
769039
     return *subjectDN ? LDAPU_SUCCESS : LDAPU_ERR_EXTRACT_SUBJECTDN_FAILED;
769039
 }
769039
 
769039
+/*
769039
+ * Return the Issuer DN as a CERTName.
769039
+ * The CERTName is owned by the CERTCertificate.
769039
+ */
769039
+NSAPI_PUBLIC CERTName *
769039
+ldapu_get_cert_issuer_dn_as_CERTName(CERTCertificate *cert_in)
769039
+{
769039
+    return &cert_in->issuer;
769039
+}
769039
+
769039
+/*
769039
+ * Return the Issuer DN as a string.
769039
+ * The string should be freed by the caller.
769039
+ */
769039
 NSAPI_PUBLIC int
769039
 ldapu_get_cert_issuer_dn(void *cert_in, char **issuerDN)
769039
 {
769039
-    CERTCertificate *cert = (CERTCertificate *)cert_in;
769039
-    char *cert_issuer = CERT_NameToAscii(&cert->issuer);
769039
-
769039
-    *issuerDN = strdup(cert_issuer);
769039
-    PR_Free(cert_issuer);
769039
-
769039
+    *issuerDN = NULL;
769039
+    CERTName *dn = ldapu_get_cert_issuer_dn_as_CERTName((CERTCertificate *)cert_in);
769039
+    if (dn != NULL) {
769039
+        char *cert_issuer = CERT_NameToAscii(dn);
769039
+        *issuerDN = strdup(cert_issuer);
769039
+        PR_Free(cert_issuer);
769039
+    }
769039
     return *issuerDN ? LDAPU_SUCCESS : LDAPU_ERR_EXTRACT_ISSUERDN_FAILED;
769039
 }
769039
 
769039
diff --git a/lib/ldaputil/certmap.c b/lib/ldaputil/certmap.c
769039
index 78bb3635b..0db2de12b 100644
769039
--- a/lib/ldaputil/certmap.c
769039
+++ b/lib/ldaputil/certmap.c
769039
@@ -52,7 +52,6 @@ static char this_dllname[256];
769039
 static const char *LIB_DIRECTIVE = "certmap";
769039
 static const int LIB_DIRECTIVE_LEN = 7; /* strlen("LIB_DIRECTIVE") */
769039
 
769039
-static char *ldapu_dn_normalize(char *dn);
769039
 static void *ldapu_propval_free(void *propval_in, void *arg);
769039
 
769039
 typedef struct
769039
@@ -337,8 +336,13 @@ dbinfo_to_certinfo(DBConfDBInfo_t *db_info,
769039
     certinfo->issuerName = db_info->dbname;
769039
     db_info->dbname = 0;
769039
 
769039
-    certinfo->issuerDN = ldapu_dn_normalize(db_info->url);
769039
-    db_info->url = 0;
769039
+    /* Parse the Issuer DN. */
769039
+    certinfo->issuerDN = CERT_AsciiToName(db_info->url);
769039
+    if (NULL == certinfo->issuerDN                            /* invalid DN */
769039
+            && ldapu_strcasecmp(db_info->url, "default") != 0 /* not "default" */) {
769039
+        rv = LDAPU_ERR_MALFORMED_SUBJECT_DN;
769039
+        goto error;
769039
+    }
769039
 
769039
     /* hijack actual prop-vals from dbinfo -- to avoid strdup calls */
769039
     if (db_info->firstprop) {
769039
@@ -890,24 +894,26 @@ ldapu_cert_searchfn_default(void *cert, LDAP *ld, void *certmap_info_in, const c
769039
 }
769039
 
769039
 NSAPI_PUBLIC int
769039
-ldapu_issuer_certinfo(const char *issuerDN, void **certmap_info)
769039
+ldapu_issuer_certinfo(const CERTName *issuerDN, void **certmap_info)
769039
 {
769039
     *certmap_info = 0;
769039
 
769039
-    if (!issuerDN || !*issuerDN || !ldapu_strcasecmp(issuerDN, "default")) {
769039
-        *certmap_info = default_certmap_info;
769039
-    } else if (certmap_listinfo) {
769039
-        char *n_issuerDN = ldapu_dn_normalize(ldapu_strdup(issuerDN));
769039
+    if (certmap_listinfo) {
769039
         LDAPUListNode_t *cur = certmap_listinfo->head;
769039
         while (cur) {
769039
-            if (!ldapu_strcasecmp(n_issuerDN, ((LDAPUCertMapInfo_t *)cur->info)->issuerDN)) {
769039
+            LDAPUCertMapInfo_t *info = (LDAPUCertMapInfo_t *)cur->info;
769039
+
769039
+            if (NULL == info->issuerDN) {
769039
+                /* no DN to compare to (probably the default certmap info) */
769039
+                continue;
769039
+            }
769039
+
769039
+            if (CERT_CompareName(issuerDN, info->issuerDN) == SECEqual) {
769039
                 *certmap_info = cur->info;
769039
                 break;
769039
             }
769039
             cur = cur->next;
769039
         }
769039
-        if (n_issuerDN)
769039
-            ldapu_free(n_issuerDN);
769039
     }
769039
     return *certmap_info ? LDAPU_SUCCESS : LDAPU_FAILED;
769039
 }
769039
@@ -1128,7 +1134,7 @@ ldapu_cert_mapfn_default(void *cert_in, LDAP *ld __attribute__((unused)), void *
769039
 }
769039
 
769039
 NSAPI_PUBLIC int
769039
-ldapu_set_cert_mapfn(const char *issuerDN,
769039
+ldapu_set_cert_mapfn(const CERTName *issuerDN,
769039
                      CertMapFn_t mapfn)
769039
 {
769039
     LDAPUCertMapInfo_t *certmap_info;
769039
@@ -1161,7 +1167,7 @@ ldapu_get_cert_mapfn_sub(LDAPUCertMapInfo_t *certmap_info)
769039
 }
769039
 
769039
 NSAPI_PUBLIC CertMapFn_t
769039
-ldapu_get_cert_mapfn(const char *issuerDN)
769039
+ldapu_get_cert_mapfn(const CERTName *issuerDN)
769039
 {
769039
     LDAPUCertMapInfo_t *certmap_info = 0;
769039
 
769039
@@ -1173,7 +1179,7 @@ ldapu_get_cert_mapfn(const char *issuerDN)
769039
 }
769039
 
769039
 NSAPI_PUBLIC int
769039
-ldapu_set_cert_searchfn(const char *issuerDN,
769039
+ldapu_set_cert_searchfn(const CERTName *issuerDN,
769039
                         CertSearchFn_t searchfn)
769039
 {
769039
     LDAPUCertMapInfo_t *certmap_info;
769039
@@ -1206,7 +1212,7 @@ ldapu_get_cert_searchfn_sub(LDAPUCertMapInfo_t *certmap_info)
769039
 }
769039
 
769039
 NSAPI_PUBLIC CertSearchFn_t
769039
-ldapu_get_cert_searchfn(const char *issuerDN)
769039
+ldapu_get_cert_searchfn(const CERTName *issuerDN)
769039
 {
769039
     LDAPUCertMapInfo_t *certmap_info = 0;
769039
 
769039
@@ -1218,7 +1224,7 @@ ldapu_get_cert_searchfn(const char *issuerDN)
769039
 }
769039
 
769039
 NSAPI_PUBLIC int
769039
-ldapu_set_cert_verifyfn(const char *issuerDN,
769039
+ldapu_set_cert_verifyfn(const CERTName *issuerDN,
769039
                         CertVerifyFn_t verifyfn)
769039
 {
769039
     LDAPUCertMapInfo_t *certmap_info;
769039
@@ -1251,7 +1257,7 @@ ldapu_get_cert_verifyfn_sub(LDAPUCertMapInfo_t *certmap_info)
769039
 }
769039
 
769039
 NSAPI_PUBLIC CertVerifyFn_t
769039
-ldapu_get_cert_verifyfn(const char *issuerDN)
769039
+ldapu_get_cert_verifyfn(const CERTName *issuerDN)
769039
 {
769039
     LDAPUCertMapInfo_t *certmap_info = 0;
769039
 
769039
@@ -1288,7 +1294,6 @@ static int ldapu_certinfo_copy (const LDAPUCertMapInfo_t *from,
769039
 NSAPI_PUBLIC int
769039
 ldapu_cert_to_ldap_entry(void *cert, LDAP *ld, const char *basedn, LDAPMessage **res)
769039
 {
769039
-    char *issuerDN = 0;
769039
     char *ldapDN = 0;
769039
     char *filter = 0;
769039
     LDAPUCertMapInfo_t *certmap_info;
769039
@@ -1308,14 +1313,14 @@ ldapu_cert_to_ldap_entry(void *cert, LDAP *ld, const char *basedn, LDAPMessage *
769039
         certmap_attrs[3] = 0;
769039
     }
769039
 
769039
-    rv = ldapu_get_cert_issuer_dn(cert, &issuerDN);
769039
+    CERTName *issuerDN = ldapu_get_cert_issuer_dn_as_CERTName(cert);
769039
+    /*        ^ don't need to free this; it will be freed with ^ the cert */
769039
 
769039
-    if (rv != LDAPU_SUCCESS)
769039
+    if (NULL == issuerDN)
769039
         return LDAPU_ERR_NO_ISSUERDN_IN_CERT;
769039
 
769039
     /* don't free the certmap_info -- its a pointer to an internal structure */
769039
     rv = ldapu_issuer_certinfo(issuerDN, (void **)&certmap_info);
769039
-    free(issuerDN);
769039
 
769039
     if (!certmap_info)
769039
         certmap_info = default_certmap_info;
769039
@@ -1604,118 +1609,3 @@ ldapu_realloc(void *ptr, int size)
769039
 {
769039
     return realloc(ptr, size);
769039
 }
769039
-
769039
-#define DNSEPARATOR(c) (c == ',' || c == ';')
769039
-#define SEPARATOR(c) (c == ',' || c == ';' || c == '+')
769039
-#define SPACE(c) (c == ' ' || c == '\n')
769039
-#define NEEDSESCAPE(c) (c == '\\' || c == '"')
769039
-#define B4TYPE 0
769039
-#define INTYPE 1
769039
-#define B4EQUAL 2
769039
-#define B4VALUE 3
769039
-#define INVALUE 4
769039
-#define INQUOTEDVALUE 5
769039
-#define B4SEPARATOR 6
769039
-
769039
-static char *
769039
-ldapu_dn_normalize(char *dn)
769039
-{
769039
-    char *d, *s;
769039
-    int state, gotesc;
769039
-
769039
-    gotesc = 0;
769039
-    state = B4TYPE;
769039
-    for (d = s = dn; *s; s++) {
769039
-        switch (state) {
769039
-        case B4TYPE:
769039
-            if (!SPACE(*s)) {
769039
-                state = INTYPE;
769039
-                *d++ = *s;
769039
-            }
769039
-            break;
769039
-        case INTYPE:
769039
-            if (*s == '=') {
769039
-                state = B4VALUE;
769039
-                *d++ = *s;
769039
-            } else if (SPACE(*s)) {
769039
-                state = B4EQUAL;
769039
-            } else {
769039
-                *d++ = *s;
769039
-            }
769039
-            break;
769039
-        case B4EQUAL:
769039
-            if (*s == '=') {
769039
-                state = B4VALUE;
769039
-                *d++ = *s;
769039
-            } else if (!SPACE(*s)) {
769039
-                /* not a valid dn - but what can we do here? */
769039
-                *d++ = *s;
769039
-            }
769039
-            break;
769039
-        case B4VALUE:
769039
-            if (*s == '"') {
769039
-                state = INQUOTEDVALUE;
769039
-                *d++ = *s;
769039
-            } else if (!SPACE(*s)) {
769039
-                state = INVALUE;
769039
-                *d++ = *s;
769039
-            }
769039
-            break;
769039
-        case INVALUE:
769039
-            if (!gotesc && SEPARATOR(*s)) {
769039
-                while (SPACE(*(d - 1)))
769039
-                    d--;
769039
-                state = B4TYPE;
769039
-                if (*s == '+') {
769039
-                    *d++ = *s;
769039
-                } else {
769039
-                    *d++ = ',';
769039
-                }
769039
-            } else if (gotesc && !NEEDSESCAPE(*s) &&
769039
-                       !SEPARATOR(*s)) {
769039
-                *--d = *s;
769039
-                d++;
769039
-            } else {
769039
-                *d++ = *s;
769039
-            }
769039
-            break;
769039
-        case INQUOTEDVALUE:
769039
-            if (!gotesc && *s == '"') {
769039
-                state = B4SEPARATOR;
769039
-                *d++ = *s;
769039
-            } else if (gotesc && !NEEDSESCAPE(*s)) {
769039
-                *--d = *s;
769039
-                d++;
769039
-            } else {
769039
-                *d++ = *s;
769039
-            }
769039
-            break;
769039
-        case B4SEPARATOR:
769039
-            if (SEPARATOR(*s)) {
769039
-                state = B4TYPE;
769039
-                if (*s == '+') {
769039
-                    *d++ = *s;
769039
-                } else {
769039
-                    *d++ = ',';
769039
-                }
769039
-            }
769039
-            break;
769039
-        default:
769039
-            break;
769039
-        }
769039
-        if (*s == '\\') {
769039
-            gotesc = 1;
769039
-        } else {
769039
-            gotesc = 0;
769039
-        }
769039
-    }
769039
-    *d = '\0';
769039
-
769039
-    /* Trim trailing spaces */
769039
-    d--;
769039
-    while (d >= dn && *d == ' ') {
769039
-        *d-- = '\0';
769039
-    }
769039
-
769039
-    return (dn);
769039
-}
769039
diff --git a/lib/ldaputil/examples/init.c b/lib/ldaputil/examples/init.c
769039
index 74db9775c..fd1edc97e 100644
769039
--- a/lib/ldaputil/examples/init.c
769039
+++ b/lib/ldaputil/examples/init.c
769039
@@ -15,12 +15,13 @@
769039
 #include <stdio.h>
769039
 #include <string.h>
769039
 #include <ctype.h>
769039
+#include <nss3/cert.h>
769039
 #include "certmap.h" /* Public Certmap API */
769039
 #include "plugin.h"  /* must define extern "C" functions */
769039
 
769039
 
769039
 NSAPI_PUBLIC int
769039
-plugin_init_fn(void *certmap_info, const char *issuerName, const char *issuerDN, const char *libname)
769039
+plugin_init_fn(void *certmap_info, const char *issuerName, const CERTName *issuerDN, const char *libname)
769039
 {
769039
     static int initialized = 0;
769039
     int rv;
769039
-- 
769039
2.17.2
769039