From 66c96b915dd4a82ebd4228cba61d7c4bae96cbca Mon Sep 17 00:00:00 2001
From: Fraser Tweedale <ftweedal@redhat.com>
Date: Fri, 16 Mar 2018 15:16:56 +1000
Subject: [PATCH] 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@redhat.com>
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 fec2dd931..50fd4d158 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 e0e028c5c..b172819b0 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 65a481541..73abba12a 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 78bb3635b..0db2de12b 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 74db9775c..fd1edc97e 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;
--
2.17.2