On 6.5.2013 17:40, Tomas Hozza wrote:
On 04/08/2013 07:45 PM, Petr Spacek wrote:
Generalize attribute_name<->rdata_type conversions.
Attribute names are generated on-the-fly: String "Record" is appended
to textual representation of DNS RDATA type.
String "Record" is cut down from the attribute name during
attribute name to rdata type conversion.
From now, the plugin doesn't add artificial limitation to supported
record types.
ACK.
The patch looks good. (I didn't do functional test)
Cosmetic issue:
I think it would be good to dynamically allocate "mod_type" in LDAPMod
in every case and include the "mod_type" memory freeing in
free_ldapmod() function. Now one has to be be careful when it is
statically or dynamically allocated. Before it was static in every case.
It is good idea. This version of the patch contains ldap_mod_create() function
which allocates the whole structure including mod_type of fixed size. All
writes to mod_type checks the array length, so it should not cause any harm.
The function modify_soa_record() still uses statically allocated LDAPMod
structure with statically allocated strings for mod_type, but the LDAPMod
structure never leave this function. There are no calls to ldap_mod_create()
and ldap_mod_free(), so I think it is obvious.
Tbabej, please try to dynamically update some A records with sync_ptr enabled.
(And of course the support for some new type, like TLSA.)
Thank you!
--
Petr^2 Spacek
From 121ca310271b655f05d030b0a841007184de8fcd Mon Sep 17 00:00:00 2001
From: Petr Spacek <pspa...@redhat.com>
Date: Mon, 8 Apr 2013 19:03:43 +0200
Subject: [PATCH] Generalize attribute_name<->rdata_type conversions.
Attribute names are generated on-the-fly: String "Record" is appended
to textual representation of DNS RDATA type.
String "Record" is cut down from the attribute name during
attribute name to rdata type conversion.
From now, the plugin doesn't add artificial limitation to supported
record types.
Signed-off-by: Petr Spacek <pspa...@redhat.com>
---
src/ldap_convert.c | 70 +++++++++++++-------------------
src/ldap_convert.h | 10 ++++-
src/ldap_helper.c | 116 +++++++++++++++++++++++++++--------------------------
3 files changed, 95 insertions(+), 101 deletions(-)
diff --git a/src/ldap_convert.c b/src/ldap_convert.c
index b1d9a18b4afa83a6fc10b348b1657d73a667a027..f0901a3c333caa0e62205b8fe1eb04261e35eea8 100644
--- a/src/ldap_convert.c
+++ b/src/ldap_convert.c
@@ -24,7 +24,6 @@
#include <isc/string.h>
#include <dns/name.h>
-#include <dns/rdatatype.h>
#include <dns/result.h>
#include <dns/types.h>
@@ -41,28 +40,6 @@
#include "util.h"
#include "zone_register.h"
-/*
- * Consistency must be preserved in these tables.
- * ldap_dns_records[i] must always corespond to dns_records[i]
- */
-const char *ldap_dns_records[] = {
- "ARecord", "AAAARecord", "A6Record", "NSRecord",
- "CNAMERecord", "PTRRecord", "SRVRecord", "TXTRecord", "MXRecord",
- "MDRecord", "HINFORecord", "MINFORecord", "AFSDBRecord", "SIGRecord",
- "KEYRecord", "LOCRecord", "NXTRecord", "NAPTRRecord", "KXRecord",
- "CERTRecord", "DNAMERecord", "DSRecord", "SSHFPRecord",
- "RRSIGRecord", "NSECRecord", NULL
-};
-
-const char *dns_records[] = {
- "A", "AAAA", "A6", "NS",
- "CNAME", "PTR", "SRV", "TXT", "MX",
- "MD", "HINFO", "MINFO", "AFSDB", "SIG",
- "KEY", "LOC", "NXT", "NAPTR", "KX",
- "CERT", "DNAME", "DS", "SSHFP",
- "RRSIG", "NSEC", NULL
-};
-
static isc_result_t explode_dn(const char *dn, char ***explodedp, int notypes);
static isc_result_t explode_rdn(const char *rdn, char ***explodedp,
int notypes);
@@ -436,45 +413,52 @@ cleanup:
return result;
}
+/**
+ * Convert attribute name to dns_rdatatype.
+ *
+ * @param[in] ldap_attribute String with attribute name terminated by \0.
+ * @param[out] rdtype
+ */
isc_result_t
ldap_attribute_to_rdatatype(const char *ldap_attribute, dns_rdatatype_t *rdtype)
{
isc_result_t result;
- unsigned i;
+ unsigned len;
isc_consttextregion_t region;
- for (i = 0; ldap_dns_records[i] != NULL; i++) {
- if (!strcasecmp(ldap_attribute, ldap_dns_records[i]))
- break;
- }
- if (dns_records[i] == NULL)
- return ISC_R_NOTFOUND;
+ len = strlen(ldap_attribute);
+ if (len <= LDAP_RDATATYPE_SUFFIX_LEN)
+ return ISC_R_UNEXPECTEDEND;
- region.base = dns_records[i];
- region.length = strlen(region.base);
+ /* Does attribute name end with RECORD_SUFFIX? */
+ if (strcasecmp(ldap_attribute + len - LDAP_RDATATYPE_SUFFIX_LEN,
+ LDAP_RDATATYPE_SUFFIX))
+ 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("dns_rdatatype_fromtext() failed");
+ log_error_r("dns_rdatatype_fromtext() failed for attribute '%s'",
+ ldap_attribute);
return result;
}
isc_result_t
-rdatatype_to_ldap_attribute(dns_rdatatype_t rdtype, const char **target)
+rdatatype_to_ldap_attribute(dns_rdatatype_t rdtype, char *target,
+ unsigned int size)
{
- unsigned i;
+ isc_result_t result;
char rdtype_str[DNS_RDATATYPE_FORMATSIZE];
dns_rdatatype_format(rdtype, rdtype_str, DNS_RDATATYPE_FORMATSIZE);
- for (i = 0; dns_records[i] != NULL; i++) {
- if (!strcmp(rdtype_str, dns_records[i]))
- break;
- }
- if (ldap_dns_records[i] == NULL)
- return ISC_R_NOTFOUND;
-
- *target = ldap_dns_records[i];
+ CHECK(isc_string_copy(target, size, rdtype_str));
+ CHECK(isc_string_append(target, size, LDAP_RDATATYPE_SUFFIX));
return ISC_R_SUCCESS;
+
+cleanup:
+ return result;
}
diff --git a/src/ldap_convert.h b/src/ldap_convert.h
index bcbe395dc3de684de62341f436310e34ee0d90ee..0ca1a148a3e762bbf36f7ff9e09c08db0a0dd65a 100644
--- a/src/ldap_convert.h
+++ b/src/ldap_convert.h
@@ -22,10 +22,15 @@
#define _LD_LDAP_CONVERT_H_
#include <dns/types.h>
+#include <dns/rdatatype.h>
#include "str.h"
#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)
+
/*
* Convert LDAP DN 'dn', to dns_name_t 'target'. 'target' needs to be
* initialized with dns_name_init() before the call and freed by the caller
@@ -41,8 +46,9 @@ isc_result_t dnsname_to_dn(zone_register_t *zr, dns_name_t *name,
isc_result_t ldap_attribute_to_rdatatype(const char *ldap_record,
dns_rdatatype_t *rdtype);
-isc_result_t rdatatype_to_ldap_attribute(dns_rdatatype_t rdtype,
- const char **target);
+isc_result_t
+rdatatype_to_ldap_attribute(dns_rdatatype_t rdtype, char *target,
+ unsigned int size);
isc_result_t dn_to_text(const char *dn, ld_string_t *target,
ld_string_t *origin);
diff --git a/src/ldap_helper.c b/src/ldap_helper.c
index c9afdf159cd304fb17fd63082849980e3cb4a3e7..6beb5cf1ab48c572046072739f1fbfb9cb4af59e 100644
--- a/src/ldap_helper.c
+++ b/src/ldap_helper.c
@@ -320,7 +320,7 @@ static isc_result_t ldap_rdttl_to_ldapmod(isc_mem_t *mctx,
dns_rdatalist_t *rdlist, LDAPMod **changep);
static isc_result_t ldap_rdatalist_to_ldapmod(isc_mem_t *mctx,
dns_rdatalist_t *rdlist, LDAPMod **changep, int mod_op);
-static void free_ldapmod(isc_mem_t *mctx, LDAPMod **changep);
+
static isc_result_t ldap_rdata_to_char_array(isc_mem_t *mctx,
dns_rdata_t *rdata_head, char ***valsp);
static void free_char_array(isc_mem_t *mctx, char ***valsp);
@@ -2656,45 +2656,8 @@ cleanup:
return result;
}
-static isc_result_t
-ldap_rdatalist_to_ldapmod(isc_mem_t *mctx, dns_rdatalist_t *rdlist,
- LDAPMod **changep, int mod_op)
-{
- isc_result_t result;
- LDAPMod *change = NULL;
- char **vals = NULL;
- const char *attr_name_c;
- char *attr_name;
-
-
- REQUIRE(changep != NULL && *changep == NULL);
-
- CHECKED_MEM_GET_PTR(mctx, change);
- ZERO_PTR(change);
-
- result = rdatatype_to_ldap_attribute(rdlist->type, &attr_name_c);
- if (result != ISC_R_SUCCESS) {
- result = ISC_R_FAILURE;
- goto cleanup;
- }
- DE_CONST(attr_name_c, attr_name);
- CHECK(ldap_rdata_to_char_array(mctx, HEAD(rdlist->rdata), &vals));
-
- change->mod_op = mod_op;
- change->mod_type = attr_name;
- change->mod_values = vals;
-
- *changep = change;
- return ISC_R_SUCCESS;
-
-cleanup:
- free_ldapmod(mctx, &change);
-
- return result;
-}
-
static void
-free_ldapmod(isc_mem_t *mctx, LDAPMod **changep)
+ldap_mod_free(isc_mem_t *mctx, LDAPMod **changep)
{
LDAPMod *change;
@@ -2705,12 +2668,61 @@ free_ldapmod(isc_mem_t *mctx, LDAPMod **changep)
return;
free_char_array(mctx, &change->mod_values);
+ if (change->mod_type != NULL)
+ SAFE_MEM_PUT(mctx, change->mod_type, LDAP_ATTR_FORMATSIZE);
SAFE_MEM_PUT_PTR(mctx, change);
*changep = NULL;
}
static isc_result_t
+ldap_mod_create(isc_mem_t *mctx, LDAPMod **changep)
+{
+ LDAPMod *change = NULL;
+ isc_result_t result;
+
+ REQUIRE(changep != NULL && *changep == NULL);
+
+ CHECKED_MEM_GET_PTR(mctx, change);
+ ZERO_PTR(change);
+ CHECKED_MEM_GET(mctx, change->mod_type, LDAP_ATTR_FORMATSIZE);
+
+ *changep = change;
+ return ISC_R_SUCCESS;
+
+cleanup:
+ if (change != NULL)
+ SAFE_MEM_PUT_PTR(mctx, change);
+
+ return result;
+}
+
+static isc_result_t
+ldap_rdatalist_to_ldapmod(isc_mem_t *mctx, dns_rdatalist_t *rdlist,
+ LDAPMod **changep, int mod_op)
+{
+ 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));
+
+ change->mod_op = mod_op;
+ change->mod_values = vals;
+
+ *changep = change;
+ return ISC_R_SUCCESS;
+
+cleanup:
+ ldap_mod_free(mctx, &change);
+
+ return result;
+}
+
+static isc_result_t
ldap_rdata_to_char_array(isc_mem_t *mctx, dns_rdata_t *rdata_head,
char ***valsp)
{
@@ -2791,11 +2803,9 @@ ldap_rdttl_to_ldapmod(isc_mem_t *mctx, dns_rdatalist_t *rdlist,
CHECK(str_new(mctx, &ttlval));
CHECK(str_sprintf(ttlval, "%d", rdlist->ttl));
- CHECKED_MEM_GET_PTR(mctx, change);
- ZERO_PTR(change);
-
+ CHECK(ldap_mod_create(mctx, &change));
change->mod_op = LDAP_MOD_REPLACE;
- change->mod_type = "dnsTTL";
+ CHECK(isc_string_copy(change->mod_type, LDAP_ATTR_FORMATSIZE, "dnsTTL"));
CHECKED_MEM_ALLOCATE(mctx, vals, 2 * sizeof(char *));
memset(vals, 0, 2 * sizeof(char *));
@@ -2808,7 +2818,7 @@ ldap_rdttl_to_ldapmod(isc_mem_t *mctx, dns_rdatalist_t *rdlist,
cleanup:
if (ttlval) str_destroy(&ttlval);
- if (change && result != ISC_R_SUCCESS) free_ldapmod(mctx, &change);
+ if (change && result != ISC_R_SUCCESS) ldap_mod_free(mctx, &change);
return result;
}
@@ -3172,17 +3182,12 @@ ldap_sync_ptr(ldap_instance_t *ldap_inst, dns_name_t *a_name,
CLEANUP_WITH(DNS_R_SERVFAIL);
/* Fill the LDAPMod change structure up. */
- CHECKED_MEM_GET_PTR(mctx, change[0]);
- ZERO_PTR(change[0]);
+ CHECK(ldap_mod_create(mctx, &change[0]));
/* Do the same action what has been done with A/AAAA record. */
change[0]->mod_op = mod_op;
- char *attr_name;
- const char *attr_name_c;
- CHECK(rdatatype_to_ldap_attribute(dns_rdatatype_ptr, &attr_name_c));
-
- DE_CONST(attr_name_c, attr_name);
- change[0]->mod_type = attr_name;
+ CHECK(rdatatype_to_ldap_attribute(dns_rdatatype_ptr, change[0]->mod_type,
+ LDAP_ATTR_FORMATSIZE));
CHECKED_MEM_ALLOCATE(mctx, vals, 2 * sizeof(char *));
memset(vals, 0, 2 * sizeof(char *));
@@ -3202,7 +3207,7 @@ cleanup:
if (dns_name_dynamic(&zone_name))
dns_name_free(&zone_name, mctx);
str_destroy(&ptr_dn);
- if (change[0] != NULL) free_ldapmod(mctx, &change[0]);
+ ldap_mod_free(mctx, &change[0]);
return result;
}
@@ -3293,9 +3298,8 @@ modify_ldap_common(dns_name_t *owner, ldap_instance_t *ldap_inst,
cleanup:
str_destroy(&owner_dn);
-
- free_ldapmod(mctx, &change[0]);
- free_ldapmod(mctx, &change[1]);
+ ldap_mod_free(mctx, &change[0]);
+ ldap_mod_free(mctx, &change[1]);
free_char_array(mctx, &vals);
dns_name_free(&zone_name, mctx);
--
1.7.11.7
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
