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 *)®ion); 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, ®ion); /* 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