[389-ds-base] branch 389-ds-base-1.3.9 updated: Ticket 49543 - fix certmap dn comparison

[git repository hosting]

This is an automated email from the git hooks/post-receive script.

mreynolds pushed a commit to branch 389-ds-base-1.3.9
in repository 389-ds-base.

The following commit(s) were added to refs/heads/389-ds-base-1.3.9 by this push:
     new 1a93d63  Ticket 49543 - fix certmap dn comparison
1a93d63 is described below

commit 1a93d63fa1fa95599c0dda1a1f3b2a72ab90d634
Author: Fraser Tweedale <ftweedal(a)redhat.com&gt;
AuthorDate: Fri Mar 16 15:16:56 2018 +1000

    Ticket 49543 - fix certmap dn comparison
    
    Bug Description: Differences in DN string representations between
    the value included in certmap.conf, and the stringified value of the
    Issuer DN produced by NSS, as well as buggy DN normalisation code in
    389 itself, cause 389 to wrongly reject the correct certmap
    configuration to use.  Authentication fails.  This behaviour was
    observed when there is an escaped comma in an attribute value.
    
    Fix Description: Instead of comparing stringified DNs, parse the DN
    represented in certmap.conf into an NSS CertNAME.  Use the NSS DN
    comparison routine when comparing certificate Issuer DNs against the
    certmap configurations.  Remove the buggy DN normalisation routine.
    
    https://pagure.io/389-ds-base/issue/49543
    
    Author: Fraser Tweedale <ftweedal(a)redhat.com&gt;
    
    Review by: ???
---
 include/ldaputil/certmap.h   |  20 +++---
 include/ldaputil/ldaputil.h  |   2 +-
 lib/ldaputil/cert.c          |  27 ++++++--
 lib/ldaputil/certmap.c       | 162 +++++++------------------------------------
 lib/ldaputil/examples/init.c |   3 +-
 5 files changed, 62 insertions(+), 152 deletions(-)

diff --git a/include/ldaputil/certmap.h b/include/ldaputil/certmap.h
index fec2dd9..50fd4d1 100644
--- a/include/ldaputil/certmap.h
+++ b/include/ldaputil/certmap.h
@@ -16,6 +16,7 @@
 /* What was extcmap.h begins ... */
 
 #include <ldap.h>
+#include <nss3/cert.h>
 
 #ifndef NSAPI_PUBLIC
 #define NSAPI_PUBLIC
@@ -156,7 +157,7 @@ typedef int (*CertVerifyFn_t)(void *cert, LDAP *ld, void
*certmap_info, LDAPMess
  *  otherwise return LDAPU_CERT_MAP_INITFN_FAILED.  The server startup will be
  *  aborted if the return value is not LDAPU_SUCCESS.
  */
-typedef int (*CertMapInitFn_t)(void *certmap_info, const char *issuerName, const char
*issuerDN, const char *libname);
+typedef int (*CertMapInitFn_t)(void *certmap_info, const char *issuerName, const CERTName
*issuerDN, const char *libname);
 
 /*
  * Refer to the description of the function ldapu_get_cert_ava_val
@@ -209,27 +210,30 @@ extern "C" {
 
 NSAPI_PUBLIC int ldapu_cert_to_ldap_entry(void *cert, LDAP *ld, const char *basedn,
LDAPMessage **res);
 
-NSAPI_PUBLIC int ldapu_set_cert_mapfn(const char *issuerDN,
+NSAPI_PUBLIC int ldapu_set_cert_mapfn(const CERTName *issuerDN,
                                       CertMapFn_t mapfn);
 
 
-NSAPI_PUBLIC CertMapFn_t ldapu_get_cert_mapfn(const char *issuerDN);
+NSAPI_PUBLIC CertMapFn_t ldapu_get_cert_mapfn(const CERTName *issuerDN);
 
-NSAPI_PUBLIC int ldapu_set_cert_searchfn(const char *issuerDN,
+NSAPI_PUBLIC int ldapu_set_cert_searchfn(const CERTName *issuerDN,
                                          CertSearchFn_t searchfn);
 
 
-NSAPI_PUBLIC CertSearchFn_t ldapu_get_cert_searchfn(const char *issuerDN);
+NSAPI_PUBLIC CertSearchFn_t ldapu_get_cert_searchfn(const CERTName *issuerDN);
 
-NSAPI_PUBLIC int ldapu_set_cert_verifyfn(const char *issuerDN,
+NSAPI_PUBLIC int ldapu_set_cert_verifyfn(const CERTName *issuerDN,
                                          CertVerifyFn_t verifyFn);
 
-NSAPI_PUBLIC CertVerifyFn_t ldapu_get_cert_verifyfn(const char *issuerDN);
+NSAPI_PUBLIC CertVerifyFn_t ldapu_get_cert_verifyfn(const CERTName *issuerDN);
 
 
 NSAPI_PUBLIC int ldapu_get_cert_subject_dn(void *cert, char **subjectDN);
 
 
+NSAPI_PUBLIC CERTName *ldapu_get_cert_issuer_dn_as_CERTName(CERTCertificate *cert);
+
+
 NSAPI_PUBLIC int ldapu_get_cert_issuer_dn(void *cert, char **issuerDN);
 
 
@@ -242,7 +246,7 @@ NSAPI_PUBLIC int ldapu_free_cert_ava_val(char **val);
 NSAPI_PUBLIC int ldapu_get_cert_der(void *cert, unsigned char **derCert, unsigned int
*len);
 
 
-NSAPI_PUBLIC int ldapu_issuer_certinfo(const char *issuerDN,
+NSAPI_PUBLIC int ldapu_issuer_certinfo(const CERTName *issuerDN,
                                        void **certmap_info);
 
 
diff --git a/include/ldaputil/ldaputil.h b/include/ldaputil/ldaputil.h
index e0e028c..b172819 100644
--- a/include/ldaputil/ldaputil.h
+++ b/include/ldaputil/ldaputil.h
@@ -48,7 +48,7 @@ enum
 typedef struct
 {
     char *issuerName;            /* issuer (symbolic/short) name */
-    char *issuerDN;              /* cert issuer's DN */
+    CERTName *issuerDN;          /* cert issuer's DN */
     LDAPUPropValList_t *propval; /* pointer to the prop-val pairs list */
     CertMapFn_t mapfn;           /* cert to ldapdn & filter mapping func */
     CertVerifyFn_t verifyfn;     /* verify cert function */
diff --git a/lib/ldaputil/cert.c b/lib/ldaputil/cert.c
index 65a4815..73abba1 100644
--- a/lib/ldaputil/cert.c
+++ b/lib/ldaputil/cert.c
@@ -54,15 +54,30 @@ ldapu_get_cert_subject_dn(void *cert_in, char **subjectDN)
     return *subjectDN ? LDAPU_SUCCESS : LDAPU_ERR_EXTRACT_SUBJECTDN_FAILED;
 }
 
+/*
+ * Return the Issuer DN as a CERTName.
+ * The CERTName is owned by the CERTCertificate.
+ */
+NSAPI_PUBLIC CERTName *
+ldapu_get_cert_issuer_dn_as_CERTName(CERTCertificate *cert_in)
+{
+    return &cert_in->issuer;
+}
+
+/*
+ * Return the Issuer DN as a string.
+ * The string should be freed by the caller.
+ */
 NSAPI_PUBLIC int
 ldapu_get_cert_issuer_dn(void *cert_in, char **issuerDN)
 {
-    CERTCertificate *cert = (CERTCertificate *)cert_in;
-    char *cert_issuer = CERT_NameToAscii(&cert->issuer);
-
-    *issuerDN = strdup(cert_issuer);
-    PR_Free(cert_issuer);
-
+    *issuerDN = NULL;
+    CERTName *dn = ldapu_get_cert_issuer_dn_as_CERTName((CERTCertificate *)cert_in);
+    if (dn != NULL) {
+        char *cert_issuer = CERT_NameToAscii(dn);
+        *issuerDN = strdup(cert_issuer);
+        PR_Free(cert_issuer);
+    }
     return *issuerDN ? LDAPU_SUCCESS : LDAPU_ERR_EXTRACT_ISSUERDN_FAILED;
 }
 
diff --git a/lib/ldaputil/certmap.c b/lib/ldaputil/certmap.c
index 78bb363..0db2de1 100644
--- a/lib/ldaputil/certmap.c
+++ b/lib/ldaputil/certmap.c
@@ -52,7 +52,6 @@ static char this_dllname[256];
 static const char *LIB_DIRECTIVE = "certmap";
 static const int LIB_DIRECTIVE_LEN = 7; /* strlen("LIB_DIRECTIVE") */
 
-static char *ldapu_dn_normalize(char *dn);
 static void *ldapu_propval_free(void *propval_in, void *arg);
 
 typedef struct
@@ -337,8 +336,13 @@ dbinfo_to_certinfo(DBConfDBInfo_t *db_info,
     certinfo->issuerName = db_info->dbname;
     db_info->dbname = 0;
 
-    certinfo->issuerDN = ldapu_dn_normalize(db_info->url);
-    db_info->url = 0;
+    /* Parse the Issuer DN. */
+    certinfo->issuerDN = CERT_AsciiToName(db_info->url);
+    if (NULL == certinfo->issuerDN                            /* invalid DN */
+            && ldapu_strcasecmp(db_info->url, "default") != 0 /* not
"default" */) {
+        rv = LDAPU_ERR_MALFORMED_SUBJECT_DN;
+        goto error;
+    }
 
     /* hijack actual prop-vals from dbinfo -- to avoid strdup calls */
     if (db_info->firstprop) {
@@ -890,24 +894,26 @@ ldapu_cert_searchfn_default(void *cert, LDAP *ld, void
*certmap_info_in, const c
 }
 
 NSAPI_PUBLIC int
-ldapu_issuer_certinfo(const char *issuerDN, void **certmap_info)
+ldapu_issuer_certinfo(const CERTName *issuerDN, void **certmap_info)
 {
     *certmap_info = 0;
 
-    if (!issuerDN || !*issuerDN || !ldapu_strcasecmp(issuerDN, "default")) {
-        *certmap_info = default_certmap_info;
-    } else if (certmap_listinfo) {
-        char *n_issuerDN = ldapu_dn_normalize(ldapu_strdup(issuerDN));
+    if (certmap_listinfo) {
         LDAPUListNode_t *cur = certmap_listinfo->head;
         while (cur) {
-            if (!ldapu_strcasecmp(n_issuerDN, ((LDAPUCertMapInfo_t
*)cur->info)->issuerDN)) {
+            LDAPUCertMapInfo_t *info = (LDAPUCertMapInfo_t *)cur->info;
+
+            if (NULL == info->issuerDN) {
+                /* no DN to compare to (probably the default certmap info) */
+                continue;
+            }
+
+            if (CERT_CompareName(issuerDN, info->issuerDN) == SECEqual) {
                 *certmap_info = cur->info;
                 break;
             }
             cur = cur->next;
         }
-        if (n_issuerDN)
-            ldapu_free(n_issuerDN);
     }
     return *certmap_info ? LDAPU_SUCCESS : LDAPU_FAILED;
 }
@@ -1128,7 +1134,7 @@ ldapu_cert_mapfn_default(void *cert_in, LDAP *ld
__attribute__((unused)), void *
 }
 
 NSAPI_PUBLIC int
-ldapu_set_cert_mapfn(const char *issuerDN,
+ldapu_set_cert_mapfn(const CERTName *issuerDN,
                      CertMapFn_t mapfn)
 {
     LDAPUCertMapInfo_t *certmap_info;
@@ -1161,7 +1167,7 @@ ldapu_get_cert_mapfn_sub(LDAPUCertMapInfo_t *certmap_info)
 }
 
 NSAPI_PUBLIC CertMapFn_t
-ldapu_get_cert_mapfn(const char *issuerDN)
+ldapu_get_cert_mapfn(const CERTName *issuerDN)
 {
     LDAPUCertMapInfo_t *certmap_info = 0;
 
@@ -1173,7 +1179,7 @@ ldapu_get_cert_mapfn(const char *issuerDN)
 }
 
 NSAPI_PUBLIC int
-ldapu_set_cert_searchfn(const char *issuerDN,
+ldapu_set_cert_searchfn(const CERTName *issuerDN,
                         CertSearchFn_t searchfn)
 {
     LDAPUCertMapInfo_t *certmap_info;
@@ -1206,7 +1212,7 @@ ldapu_get_cert_searchfn_sub(LDAPUCertMapInfo_t *certmap_info)
 }
 
 NSAPI_PUBLIC CertSearchFn_t
-ldapu_get_cert_searchfn(const char *issuerDN)
+ldapu_get_cert_searchfn(const CERTName *issuerDN)
 {
     LDAPUCertMapInfo_t *certmap_info = 0;
 
@@ -1218,7 +1224,7 @@ ldapu_get_cert_searchfn(const char *issuerDN)
 }
 
 NSAPI_PUBLIC int
-ldapu_set_cert_verifyfn(const char *issuerDN,
+ldapu_set_cert_verifyfn(const CERTName *issuerDN,
                         CertVerifyFn_t verifyfn)
 {
     LDAPUCertMapInfo_t *certmap_info;
@@ -1251,7 +1257,7 @@ ldapu_get_cert_verifyfn_sub(LDAPUCertMapInfo_t *certmap_info)
 }
 
 NSAPI_PUBLIC CertVerifyFn_t
-ldapu_get_cert_verifyfn(const char *issuerDN)
+ldapu_get_cert_verifyfn(const CERTName *issuerDN)
 {
     LDAPUCertMapInfo_t *certmap_info = 0;
 
@@ -1288,7 +1294,6 @@ static int ldapu_certinfo_copy (const LDAPUCertMapInfo_t *from,
 NSAPI_PUBLIC int
 ldapu_cert_to_ldap_entry(void *cert, LDAP *ld, const char *basedn, LDAPMessage **res)
 {
-    char *issuerDN = 0;
     char *ldapDN = 0;
     char *filter = 0;
     LDAPUCertMapInfo_t *certmap_info;
@@ -1308,14 +1313,14 @@ ldapu_cert_to_ldap_entry(void *cert, LDAP *ld, const char *basedn,
LDAPMessage *
         certmap_attrs[3] = 0;
     }
 
-    rv = ldapu_get_cert_issuer_dn(cert, &issuerDN);
+    CERTName *issuerDN = ldapu_get_cert_issuer_dn_as_CERTName(cert);
+    /*        ^ don't need to free this; it will be freed with ^ the cert */
 
-    if (rv != LDAPU_SUCCESS)
+    if (NULL == issuerDN)
         return LDAPU_ERR_NO_ISSUERDN_IN_CERT;
 
     /* don't free the certmap_info -- its a pointer to an internal structure */
     rv = ldapu_issuer_certinfo(issuerDN, (void **)&certmap_info);
-    free(issuerDN);
 
     if (!certmap_info)
         certmap_info = default_certmap_info;
@@ -1604,118 +1609,3 @@ ldapu_realloc(void *ptr, int size)
 {
     return realloc(ptr, size);
 }
-
-#define DNSEPARATOR(c) (c == ',' || c == ';')
-#define SEPARATOR(c) (c == ',' || c == ';' || c == '+')
-#define SPACE(c) (c == ' ' || c == '\n')
-#define NEEDSESCAPE(c) (c == '\\' || c == '"')
-#define B4TYPE 0
-#define INTYPE 1
-#define B4EQUAL 2
-#define B4VALUE 3
-#define INVALUE 4
-#define INQUOTEDVALUE 5
-#define B4SEPARATOR 6
-
-static char *
-ldapu_dn_normalize(char *dn)
-{
-    char *d, *s;
-    int state, gotesc;
-
-    gotesc = 0;
-    state = B4TYPE;
-    for (d = s = dn; *s; s++) {
-        switch (state) {
-        case B4TYPE:
-            if (!SPACE(*s)) {
-                state = INTYPE;
-                *d++ = *s;
-            }
-            break;
-        case INTYPE:
-            if (*s == '=') {
-                state = B4VALUE;
-                *d++ = *s;
-            } else if (SPACE(*s)) {
-                state = B4EQUAL;
-            } else {
-                *d++ = *s;
-            }
-            break;
-        case B4EQUAL:
-            if (*s == '=') {
-                state = B4VALUE;
-                *d++ = *s;
-            } else if (!SPACE(*s)) {
-                /* not a valid dn - but what can we do here? */
-                *d++ = *s;
-            }
-            break;
-        case B4VALUE:
-            if (*s == '"') {
-                state = INQUOTEDVALUE;
-                *d++ = *s;
-            } else if (!SPACE(*s)) {
-                state = INVALUE;
-                *d++ = *s;
-            }
-            break;
-        case INVALUE:
-            if (!gotesc && SEPARATOR(*s)) {
-                while (SPACE(*(d - 1)))
-                    d--;
-                state = B4TYPE;
-                if (*s == '+') {
-                    *d++ = *s;
-                } else {
-                    *d++ = ',';
-                }
-            } else if (gotesc && !NEEDSESCAPE(*s) &&
-                       !SEPARATOR(*s)) {
-                *--d = *s;
-                d++;
-            } else {
-                *d++ = *s;
-            }
-            break;
-        case INQUOTEDVALUE:
-            if (!gotesc && *s == '"') {
-                state = B4SEPARATOR;
-                *d++ = *s;
-            } else if (gotesc && !NEEDSESCAPE(*s)) {
-                *--d = *s;
-                d++;
-            } else {
-                *d++ = *s;
-            }
-            break;
-        case B4SEPARATOR:
-            if (SEPARATOR(*s)) {
-                state = B4TYPE;
-                if (*s == '+') {
-                    *d++ = *s;
-                } else {
-                    *d++ = ',';
-                }
-            }
-            break;
-        default:
-            break;
-        }
-        if (*s == '\\') {
-            gotesc = 1;
-        } else {
-            gotesc = 0;
-        }
-    }
-    *d = '\0';
-
-    /* Trim trailing spaces */
-    d--;
-    while (d >= dn && *d == ' ') {
-        *d-- = '\0';
-    }
-
-    return (dn);
-}
diff --git a/lib/ldaputil/examples/init.c b/lib/ldaputil/examples/init.c
index 74db977..fd1edc9 100644
--- a/lib/ldaputil/examples/init.c
+++ b/lib/ldaputil/examples/init.c
@@ -15,12 +15,13 @@
 #include <stdio.h>
 #include <string.h>
 #include <ctype.h>
+#include <nss3/cert.h>
 #include "certmap.h" /* Public Certmap API */
 #include "plugin.h"  /* must define extern "C" functions */
 
 
 NSAPI_PUBLIC int
-plugin_init_fn(void *certmap_info, const char *issuerName, const char *issuerDN, const
char *libname)
+plugin_init_fn(void *certmap_info, const char *issuerName, const CERTName *issuerDN,
const char *libname)
 {
     static int initialized = 0;
     int rv;

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.