On 18.5.2015 17:31, Petr Spacek wrote:
> Hello,
> 
> This patch is unrelated to metaDB but it should be merged before alpha, too.
> 
> Thank you for review!
> 
> Support unknown record types (RFC 3597).
> 
> Fallback to generic LDAP attribute "UnknownRecord;TYP256" if attempt to
> add specific attribute like "URIRecord" failed with 
> LDAP_OBJECT_CLASS_VIOLATION
> and always delete both attributes like "URIRecord" and 
> "UnknownRecord;TYPE256".
> 
> https://fedorahosted.org/bind-dyndb-ldap/ticket/157

Fixed version is attached. Version 1 could dereference NULL pointers in second
iteration of while loops.

-- 
Petr^2 Spacek
From 0e41cb1b3db9c08b891206b2d6a0a2113a4cf00a Mon Sep 17 00:00:00 2001
From: Petr Spacek <pspa...@redhat.com>
Date: Mon, 16 Mar 2015 22:40:36 +0100
Subject: [PATCH] Support unknown record types (RFC 3597).

Fallback to generic LDAP attribute "UnknownRecord;TYP256" if attempt to
add specific attribute like "URIRecord" failed with LDAP_OBJECT_CLASS_VIOLATION
and always delete both attributes like "URIRecord" and "UnknownRecord;TYPE256".

https://fedorahosted.org/bind-dyndb-ldap/ticket/157
---
 src/ldap_convert.c | 67 ++++++++++++++++++++++++++++++++-----
 src/ldap_convert.h | 13 ++++++--
 src/ldap_driver.c  |  9 ++---
 src/ldap_helper.c  | 96 +++++++++++++++++++++++++++++++++++++++++++-----------
 src/ldap_helper.h  |  5 +--
 5 files changed, 150 insertions(+), 40 deletions(-)

diff --git a/src/ldap_convert.c b/src/ldap_convert.c
index dde10d6e989159e9f6f5086a4a12bbd165b73646..0bdb83547bec74d0f2954a4e8443ef2b70347cf8 100644
--- a/src/ldap_convert.c
+++ b/src/ldap_convert.c
@@ -19,11 +19,13 @@
  */
 
 #include <isc/buffer.h>
+#include <isc/hex.h>
 #include <isc/mem.h>
 #include <isc/util.h>
 #include <isc/string.h>
 
 #include <dns/name.h>
+#include <dns/rdata.h>
 #include <dns/result.h>
 #include <dns/types.h>
 
@@ -394,33 +396,80 @@ ldap_attribute_to_rdatatype(const char *ldap_attribute, dns_rdatatype_t *rdtype)
 
 	/* Does attribute name end with RECORD_SUFFIX? */
 	if (strcasecmp(ldap_attribute + len - LDAP_RDATATYPE_SUFFIX_LEN,
-		       LDAP_RDATATYPE_SUFFIX))
+		       LDAP_RDATATYPE_SUFFIX) == 0) {
+		region.base = ldap_attribute;
+		region.length = len - LDAP_RDATATYPE_SUFFIX_LEN;
+	/* Does attribute name start with with UNKNOWN_PREFIX? */
+	} else if (strncasecmp(ldap_attribute,
+			      LDAP_RDATATYPE_UNKNOWN_PREFIX,
+			      LDAP_RDATATYPE_UNKNOWN_PREFIX_LEN) == 0) {
+		region.base = ldap_attribute + LDAP_RDATATYPE_UNKNOWN_PREFIX_LEN;
+		region.length = len - LDAP_RDATATYPE_UNKNOWN_PREFIX_LEN;
+	} else
 		return ISC_R_UNEXPECTED;
 
-	region.base = ldap_attribute;
-	region.length = len - LDAP_RDATATYPE_SUFFIX_LEN;
 	result = dns_rdatatype_fromtext(rdtype, (isc_textregion_t *)&region);
 	if (result != ISC_R_SUCCESS)
 		log_error_r("dns_rdatatype_fromtext() failed for attribute '%s'",
 			    ldap_attribute);
 
 	return result;
 }
 
+/**
+ * Convert DNS rdata type to LDAP attribute name.
+ *
+ * @param[in]  rdtype
+ * @param[out] target   Output buffer with \0 terminated attribute name.
+ * @param[in]  size     Target size.
+ * @param[in]  unknown  ISC_TRUE = use generic syntax "UnknownRecord;TYPE65333",
+ *                      ISC_FALSE = use type-specific mnemonic like "ARecord"
+ */
 isc_result_t
 rdatatype_to_ldap_attribute(dns_rdatatype_t rdtype, char *target,
-			    unsigned int size)
+			    unsigned int size, isc_boolean_t unknown)
 {
 	isc_result_t result;
 	char rdtype_str[DNS_RDATATYPE_FORMATSIZE];
 
-	dns_rdatatype_format(rdtype, rdtype_str, DNS_RDATATYPE_FORMATSIZE);
-	CHECK(isc_string_copy(target, size, rdtype_str));
-	CHECK(isc_string_append(target, size, LDAP_RDATATYPE_SUFFIX));
-
-	return ISC_R_SUCCESS;
+	if (unknown) {
+		/* "UnknownRecord;TYPE65333" */
+		CHECK(isc_string_copy(target, size,
+				      LDAP_RDATATYPE_UNKNOWN_PREFIX));
+		snprintf(rdtype_str, sizeof(rdtype_str), "TYPE%u", rdtype);
+		CHECK(isc_string_append(target, size, rdtype_str));
+	} else {
+		/* "ARecord" */
+		dns_rdatatype_format(rdtype, rdtype_str, DNS_RDATATYPE_FORMATSIZE);
+		CHECK(isc_string_copy(target, size, rdtype_str));
+		CHECK(isc_string_append(target, size, LDAP_RDATATYPE_SUFFIX));
+	}
 
 cleanup:
 	return result;
 }
 
+/**
+ * Convert rdata to generic (RFC 3597) format.
+ */
+isc_result_t
+rdata_to_generic(dns_rdata_t *rdata, isc_buffer_t *target)
+{
+	isc_result_t result;
+	isc_region_t rdata_reg;
+	char buf[sizeof("\\# 65535")];
+
+	dns_rdata_toregion(rdata, &rdata_reg);
+	REQUIRE(rdata_reg.length <= 65535);
+
+	result = isc_string_printf(buf, sizeof(buf), "\\# %u", rdata_reg.length);
+	INSIST(result == ISC_R_SUCCESS);
+	isc_buffer_putstr(target, buf);
+	if (rdata_reg.length != 0U) {
+		isc_buffer_putstr(target, " ");
+		CHECK(isc_hex_totext(&rdata_reg, 0, "", target));
+	}
+
+cleanup:
+	return result;
+}
diff --git a/src/ldap_convert.h b/src/ldap_convert.h
index daa0db0b311dd51c05ade688715550d9f70759ac..3810468af5591dbef1776c5e1ca91764b53df126 100644
--- a/src/ldap_convert.h
+++ b/src/ldap_convert.h
@@ -28,8 +28,10 @@
 #include "zone_register.h"
 
 #define LDAP_ATTR_FORMATSIZE 32 /* "expected" maximum attribute name length */
-#define LDAP_RDATATYPE_SUFFIX     "Record"
-#define LDAP_RDATATYPE_SUFFIX_LEN (sizeof(LDAP_RDATATYPE_SUFFIX) - 1)
+#define LDAP_RDATATYPE_SUFFIX		"Record"
+#define LDAP_RDATATYPE_SUFFIX_LEN	(sizeof(LDAP_RDATATYPE_SUFFIX) - 1)
+#define LDAP_RDATATYPE_UNKNOWN_PREFIX	"UnknownRecord;"
+#define LDAP_RDATATYPE_UNKNOWN_PREFIX_LEN	(sizeof(LDAP_RDATATYPE_UNKNOWN_PREFIX) - 1)
 
 /*
  * Convert LDAP DN 'dn', to dns_name_t 'target'. 'target' needs to be
@@ -54,7 +56,12 @@ isc_result_t ldap_attribute_to_rdatatype(const char *ldap_record,
 
 isc_result_t
 rdatatype_to_ldap_attribute(dns_rdatatype_t rdtype, char *target,
-			    unsigned int size) ATTR_NONNULLS ATTR_CHECKRESULT;
+			    unsigned int size, isc_boolean_t unknown)
+			    ATTR_NONNULLS ATTR_CHECKRESULT;
+
+isc_result_t
+rdata_to_generic(dns_rdata_t *rdata, isc_buffer_t *target)
+		ATTR_NONNULLS ATTR_CHECKRESULT;
 
 isc_result_t dn_to_text(const char *dn, ld_string_t *target,
 			ld_string_t *origin) ATTR_NONNULL(1, 2) ATTR_CHECKRESULT;
diff --git a/src/ldap_driver.c b/src/ldap_driver.c
index 46729f9dad69ce7906693aaef845cbb1354248c5..f0ab5566484ecafe1566b0c17dd4a2ae30b8bf46 100644
--- a/src/ldap_driver.c
+++ b/src/ldap_driver.c
@@ -607,14 +607,11 @@ deleterdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version,
 	dns_fixedname_t fname;
 	dns_name_t *zname = NULL;
 	isc_boolean_t empty_node;
-	char attr_name[LDAP_ATTR_FORMATSIZE];
-	dns_rdatalist_t fake_rdlist; /* required by remove_values_from_ldap */
 	isc_result_t result;
 
 	REQUIRE(VALID_LDAPDB(ldapdb));
 
 	dns_fixedname_init(&fname);
-	dns_rdatalist_init(&fake_rdlist);
 	zname = dns_db_origin(ldapdb->rbtdb);
 
 	result = dns_db_deleterdataset(ldapdb->rbtdb, node, version, type,
@@ -632,10 +629,8 @@ deleterdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version,
 		CHECK(remove_entry_from_ldap(dns_fixedname_name(&fname), zname,
 					     ldapdb->ldap_inst));
 	} else {
-		CHECK(rdatatype_to_ldap_attribute(type, attr_name,
-						  LDAP_ATTR_FORMATSIZE));
-		CHECK(remove_attr_from_ldap(dns_fixedname_name(&fname), zname,
-					    ldapdb->ldap_inst, attr_name));
+		CHECK(remove_rdtype_from_ldap(dns_fixedname_name(&fname), zname,
+					    ldapdb->ldap_inst, type));
 	}
 
 cleanup:
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index 45c70edfb72a9212e60da5cae9c8818bc9d52071..7408b74fcb3f415a279c7b51adcc33e96e1e8b2d 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -311,10 +311,15 @@ static isc_result_t handle_connection_error(ldap_instance_t *ldap_inst,
 static isc_result_t ldap_rdttl_to_ldapmod(isc_mem_t *mctx,
 		dns_rdatalist_t *rdlist, LDAPMod **changep) ATTR_NONNULLS ATTR_CHECKRESULT;
 static isc_result_t ldap_rdatalist_to_ldapmod(isc_mem_t *mctx,
-		dns_rdatalist_t *rdlist, LDAPMod **changep, int mod_op) ATTR_NONNULLS ATTR_CHECKRESULT;
+		dns_rdatalist_t *rdlist, LDAPMod **changep, int mod_op,
+		isc_boolean_t unknown) ATTR_NONNULLS ATTR_CHECKRESULT;
 
 static isc_result_t ldap_rdata_to_char_array(isc_mem_t *mctx,
-		dns_rdata_t *rdata_head, char ***valsp) ATTR_NONNULLS ATTR_CHECKRESULT;
+					     dns_rdata_t *rdata_head,
+					     isc_boolean_t unknown,
+					     char ***valsp)
+					     ATTR_NONNULLS ATTR_CHECKRESULT;
+
 static void free_char_array(isc_mem_t *mctx, char ***valsp) ATTR_NONNULLS;
 static isc_result_t modify_ldap_common(dns_name_t *owner, dns_name_t *zone, ldap_instance_t *ldap_inst,
 		dns_rdatalist_t *rdlist, int mod_op, isc_boolean_t delete_node) ATTR_NONNULLS ATTR_CHECKRESULT;
@@ -3039,6 +3044,14 @@ reconnect:
 	return result;
 }
 
+/**
+ * Apply LDAP modifications.
+ *
+ * @retval ISC_R_SUCCESS
+ * @retval DNS_R_UNKNOWN = LDAP_OBJECT_CLASS_VIOLATION, most likely an attribute
+ *                       for a DNS RR type cannot be added because it is not
+ *                       present in the LDAP schema.
+ */
 isc_result_t ATTR_NONNULLS ATTR_CHECKRESULT
 ldap_modify_do(ldap_instance_t *ldap_inst, const char *dn, LDAPMod **mods,
 		isc_boolean_t delete_node)
@@ -3129,6 +3142,9 @@ retry:
 	}
 
 	log_ldap_error(ldap_conn->handle, "while %s entry '%s'", operation_str, dn);
+	/* attempt to manipulate attribute failed - likely a unknown RR type */
+	if (err_code == LDAP_OBJECT_CLASS_VIOLATION)
+		CLEANUP_WITH(DNS_R_UNKNOWN);
 
 	/* do not error out if we are trying to delete an
 	 * unexisting attribute */
@@ -3189,18 +3205,25 @@ cleanup:
 	return result;
 }
 
+/**
+ * @param[in]  rdlist
+ * @param[out] changep
+ * @param[in]  mod_op  LDAP operation as integer used in LDAPMod.
+ * @param[in]  generic Use generic (RFC 3597) syntax.
+ */
 static isc_result_t ATTR_NONNULLS ATTR_CHECKRESULT
 ldap_rdatalist_to_ldapmod(isc_mem_t *mctx, dns_rdatalist_t *rdlist,
-			  LDAPMod **changep, int mod_op)
+			  LDAPMod **changep, int mod_op, isc_boolean_t unknown)
 {
 	isc_result_t result;
 	LDAPMod *change = NULL;
 	char **vals = NULL;
 
 	CHECK(ldap_mod_create(mctx, &change));
 	CHECK(rdatatype_to_ldap_attribute(rdlist->type, change->mod_type,
-					  LDAP_ATTR_FORMATSIZE));
-	CHECK(ldap_rdata_to_char_array(mctx, HEAD(rdlist->rdata), &vals));
+					  LDAP_ATTR_FORMATSIZE, unknown));
+	CHECK(ldap_rdata_to_char_array(mctx, HEAD(rdlist->rdata), unknown,
+				       &vals));
 
 	change->mod_op = mod_op;
 	change->mod_values = vals;
@@ -3214,9 +3237,15 @@ cleanup:
 	return result;
 }
 
+/**
+ * Convert list of DNS Rdata to array of LDAP values.
+ *
+ * @param[in]  unknown  ISC_TRUE  = use generic (RFC 3597) format,
+ *                      ISC_FALSE = use record-specific syntax (if available).
+ */
 static isc_result_t ATTR_NONNULLS ATTR_CHECKRESULT
 ldap_rdata_to_char_array(isc_mem_t *mctx, dns_rdata_t *rdata_head,
-			 char ***valsp)
+			 isc_boolean_t unknown, char ***valsp)
 {
 	isc_result_t result;
 	char **vals;
@@ -3238,12 +3267,16 @@ ldap_rdata_to_char_array(isc_mem_t *mctx, dns_rdata_t *rdata_head,
 
 	rdata = rdata_head;
 	for (i = 0; i < rdata_count && rdata != NULL; i++) {
-		DECLARE_BUFFER(buffer, DNS_RDATA_MAXLENGTH);
+		DECLARE_BUFFER(buffer, /* RFC 3597 format is hex string */
+			       DNS_RDATA_MAXLENGTH * 2 + sizeof("\\# 65535 "));
 		isc_region_t region;
 
 		/* Convert rdata to text. */
 		INIT_BUFFER(buffer);
-		CHECK(dns_rdata_totext(rdata, NULL, &buffer));
+		if (unknown == ISC_FALSE)
+			CHECK(dns_rdata_totext(rdata, NULL, &buffer));
+		else
+			CHECK(rdata_to_generic(rdata, &buffer));
 		isc_buffer_usedregion(&buffer, &region);
 
 		/* Now allocate the string with the right size. */
@@ -3378,6 +3411,7 @@ modify_ldap_common(dns_name_t *owner, dns_name_t *zone, ldap_instance_t *ldap_in
 	char *zone_dn = NULL;
 	settings_set_t *zone_settings = NULL;
 	int af; /* address family */
+	isc_boolean_t unknown_type = ISC_FALSE;
 
 	/*
 	 * Find parent zone entry and check if Dynamic Update is allowed.
@@ -3396,6 +3430,7 @@ modify_ldap_common(dns_name_t *owner, dns_name_t *zone, ldap_instance_t *ldap_in
 	}
 
 	CHECK(dn_to_dnsname(mctx, zone_dn, &zone_name, NULL, NULL));
+	INSIST(dns_name_equal(zone, &zone_name) == ISC_TRUE);
 
 	result = zr_get_zone_settings(ldap_inst->zone_register, &zone_name,
 				      &zone_settings);
@@ -3415,13 +3450,22 @@ modify_ldap_common(dns_name_t *owner, dns_name_t *zone, ldap_instance_t *ldap_in
 		goto cleanup;
 	}
 
-	CHECK(ldap_rdatalist_to_ldapmod(mctx, rdlist, &change[0], mod_op));
 	if (mod_op == LDAP_MOD_ADD) {
 		/* for now always replace the ttl on add */
 		CHECK(ldap_rdttl_to_ldapmod(mctx, rdlist, &change[1]));
 	}
 
-	CHECK(ldap_modify_do(ldap_inst, str_buf(owner_dn), change, delete_node));
+	/* First, try to store data into named attribute like "URIRecord".
+	 * If that fails, try to store the data into "UnknownRecord;TYPE256". */
+	unknown_type = ISC_FALSE;
+	do {
+		ldap_mod_free(mctx, &change[0]);
+		CHECK(ldap_rdatalist_to_ldapmod(mctx, rdlist, &change[0],
+						mod_op, unknown_type));
+		result = ldap_modify_do(ldap_inst, str_buf(owner_dn), change,
+					delete_node);
+		unknown_type = !unknown_type; /* try again with unknown type */
+	} while (result == DNS_R_UNKNOWN && unknown_type == ISC_TRUE);
 
 	/* Keep the PTR of corresponding A/AAAA record synchronized. */
 	if (rdlist->type == dns_rdatatype_a || rdlist->type == dns_rdatatype_aaaa) {
@@ -3438,6 +3482,7 @@ modify_ldap_common(dns_name_t *owner, dns_name_t *zone, ldap_instance_t *ldap_in
 		log_debug(3, "sync PTR is enabled for zone '%s'", zone_dn);
 
 		af = (rdlist->type == dns_rdatatype_a) ? AF_INET : AF_INET6;
+		/* Following call will not work if A/AAAA records are unknown. */
 		result = sync_ptr_init(mctx, ldap_inst->view->zonetable,
 				       ldap_inst->zone_register, owner, af,
 				       change[0]->mod_values[0], rdlist->ttl,
@@ -3475,22 +3520,35 @@ remove_values_from_ldap(dns_name_t *owner, dns_name_t *zone, ldap_instance_t *ld
 				  delete_node);
 }
 
+/**
+ * Delete named attribute 'URIRecord'
+ * and equivalent attribute 'UnknownRecord;TYPE256' too.
+ */
 isc_result_t
-remove_attr_from_ldap(dns_name_t *owner, dns_name_t *zone, ldap_instance_t *ldap_inst,
-		      const char *attr) {
+remove_rdtype_from_ldap(dns_name_t *owner, dns_name_t *zone,
+		      ldap_instance_t *ldap_inst, dns_rdatatype_t type) {
+	char attr[LDAP_ATTR_FORMATSIZE];
 	LDAPMod *change[2] = { NULL };
 	ld_string_t *dn = NULL;
 	isc_result_t result;
+	isc_boolean_t unknown_type = ISC_FALSE;
 
 	CHECK(str_new(ldap_inst->mctx, &dn));
-
-	CHECK(ldap_mod_create(ldap_inst->mctx, &change[0]));
-	change[0]->mod_op = LDAP_MOD_DELETE;
-	CHECK(isc_string_copy(change[0]->mod_type, LDAP_ATTR_FORMATSIZE, attr));
-	change[0]->mod_vals.modv_strvals = NULL; /* delete all values from given attribute */
-
 	CHECK(dnsname_to_dn(ldap_inst->zone_register, owner, zone, dn));
-	CHECK(ldap_modify_do(ldap_inst, str_buf(dn), change, ISC_FALSE));
+
+	do {
+		CHECK(ldap_mod_create(ldap_inst->mctx, &change[0]));
+		change[0]->mod_op = LDAP_MOD_DELETE;
+		/* delete all values from given attribute */
+		change[0]->mod_vals.modv_strvals = NULL;
+		CHECK(rdatatype_to_ldap_attribute(type, attr, sizeof(attr),
+						  unknown_type));
+		CHECK(isc_string_copy(change[0]->mod_type, LDAP_ATTR_FORMATSIZE,
+				      attr));
+		CHECK(ldap_modify_do(ldap_inst, str_buf(dn), change, ISC_FALSE));
+		ldap_mod_free(ldap_inst->mctx, &change[0]);
+		unknown_type = !unknown_type;
+	} while (unknown_type == ISC_TRUE);
 
 cleanup:
 	ldap_mod_free(ldap_inst->mctx, &change[0]);
diff --git a/src/ldap_helper.h b/src/ldap_helper.h
index 834b30d5b5b4bf61ebee561734998ce0b442f081..aaac8d6790c44b74a4b4a7c93f9a3eacc2a0339a 100644
--- a/src/ldap_helper.h
+++ b/src/ldap_helper.h
@@ -76,8 +76,9 @@ remove_values_from_ldap(dns_name_t *owner, dns_name_t *zone, ldap_instance_t *ld
 		dns_rdatalist_t *rdlist, isc_boolean_t delete_node) ATTR_NONNULLS;
 
 isc_result_t
-remove_attr_from_ldap(dns_name_t *owner, dns_name_t *zone, ldap_instance_t *ldap_inst,
-		      const char *attr) ATTR_NONNULLS;
+remove_rdtype_from_ldap(dns_name_t *owner, dns_name_t *zone,
+		      ldap_instance_t *ldap_inst, dns_rdatatype_t type)
+		      ATTR_NONNULLS;
 
 isc_result_t
 remove_entry_from_ldap(dns_name_t *owner, dns_name_t *zone, ldap_instance_t *ldap_inst) ATTR_NONNULLS;
-- 
2.1.0

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to