[Freeipa-devel] [PATCH 0158] Extend ipa-range-check DS plugin to handle range types
Alexander Bokovoy
abokovoy at redhat.com
Tue Mar 18 08:19:17 UTC 2014
On Mon, 17 Mar 2014, Tomas Babej wrote:
>Hi,
>
>The ipa-range-check plugin used to determine the range type depending
>on the value of the attributes such as RID or secondary RID base. This
>approached caused variety of issues since the portfolio of ID range
>types expanded.
>
>The patch makes sure the following rules are implemented:
> * No ID range pair can overlap on base ranges, with exception
> of two ipa-ad-trust-posix ranges belonging to the same forest
> * For any ID range pair of ranges belonging to the same domain:
> * Both ID ranges must be of the same type
> * For ranges of ipa-ad-trust type or ipa-local type:
> * Primary RID ranges can not overlap
> * For ranges of ipa-local type:
> * Primary and secondary RID ranges can not overlap
> * Secondary RID ranges cannot overlap
>
>For the implementation part, the plugin was extended with a domain ID
>to forest root domain ID mapping derivation capabilities.
>
>https://fedorahosted.org/freeipa/ticket/4137
>
>Test coverage coming soon!
I don't really like that you are using a list here. The list is built
and then destroyed in preop callback every time we process the range
object, so it is one-off operation. Now, when you fetch trust domain
objects to build the list, why can't you use an array directly?
Given that all you need the list for is to map domain to a trust (if
any) and trust name is the name of the root level domain, you can simply
make an array of a struct (name, root) where root is a and index to the
same array or -1 if this is the root domain itself.
>
>
>--
>Tomas Babej
>Associate Software Engeneer | Red Hat | Identity Management
>RHCE | Brno Site | IRC: tbabej | freeipa.org
>
>
>>From 0d038fb71f02fab5320e4843be80feb34c5c3303 Mon Sep 17 00:00:00 2001
>From: Tomas Babej <tbabej at redhat.com>
>Date: Wed, 5 Mar 2014 12:28:18 +0100
>Subject: [PATCH] Extend ipa-range-check DS plugin to handle range types
>
>The ipa-range-check plugin used to determine the range type depending
>on the value of the attributes such as RID or secondary RID base. This
>approached caused variety of issues since the portfolio of ID range
>types expanded.
>
>The patch makes sure the following rules are implemented:
> * No ID range pair can overlap on base ranges, with exception
> of two ipa-ad-trust-posix ranges belonging to the same forest
> * For any ID range pair of ranges belonging to the same domain:
> * Both ID ranges must be of the same type
> * For ranges of ipa-ad-trust type or ipa-local type:
> * Primary RID ranges can not overlap
> * For ranges of ipa-local type:
> * Primary and secondary RID ranges can not overlap
> * Secondary RID ranges cannot overlap
>
>For the implementation part, the plugin was extended with a domain ID
>to forest root domain ID mapping derivation capabilities.
>
>https://fedorahosted.org/freeipa/ticket/4137
>---
> .../ipa-range-check/ipa_range_check.c | 322 ++++++++++++++++++---
> 1 file changed, 284 insertions(+), 38 deletions(-)
>
>diff --git a/daemons/ipa-slapi-plugins/ipa-range-check/ipa_range_check.c b/daemons/ipa-slapi-plugins/ipa-range-check/ipa_range_check.c
>index 391e2259b6eced31fed399c927cfe44c1d3e237e..2360e42025a0146a43c3237914c8165d35714a68 100644
>--- a/daemons/ipa-slapi-plugins/ipa-range-check/ipa_range_check.c
>+++ b/daemons/ipa-slapi-plugins/ipa-range-check/ipa_range_check.c
>@@ -47,10 +47,17 @@
> #define IPA_CN "cn"
> #define IPA_BASE_ID "ipaBaseID"
> #define IPA_ID_RANGE_SIZE "ipaIDRangeSize"
>+#define IPA_RANGE_TYPE "ipaRangeType"
> #define IPA_BASE_RID "ipaBaseRID"
> #define IPA_SECONDARY_BASE_RID "ipaSecondaryBaseRID"
> #define IPA_DOMAIN_ID "ipaNTTrustedDomainSID"
> #define RANGES_FILTER "objectclass=ipaIDRange"
>+#define DOMAIN_ID_FILTER "ipaNTTrustedDomainSID=*"
>+
>+#define AD_TRUST_RANGE_TYPE "ipa-ad-trust"
>+#define AD_TRUST_POSIX_RANGE_TYPE "ipa-ad-trust-posix"
>+#define LOCAL_RANGE_TYPE "ipa-local"
>+
>
> #define IPA_PLUGIN_NAME "ipa-range-check"
> #define IPA_RANGE_CHECK_FEATURE_DESC "IPA ID range check plugin"
>@@ -72,13 +79,225 @@ struct ipa_range_check_ctx {
> struct range_info {
> char *name;
> char *domain_id;
>+ char *forest_root_id;
>+ char *id_range_type;
> uint32_t base_id;
> uint32_t id_range_size;
> uint32_t base_rid;
> uint32_t secondary_base_rid;
> };
>
>-static int slapi_entry_to_range_info(struct slapi_entry *entry,
>+struct domain_info {
>+ char *domain_id;
>+ char *forest_root_id;
>+ struct domain_info *next;
>+};
>+
>+
>+static struct domain_info* map_domain_to_root(struct domain_info *head,
>+ struct slapi_entry *domain,
>+ struct slapi_entry *root_domain){
>+
>+ struct domain_info* new_head = NULL;
>+ new_head = (struct domain_info*) malloc(sizeof(struct domain_info));
>+ if (new_head == NULL) {
>+ return NULL;
>+ }
>+
>+ new_head->domain_id = slapi_entry_attr_get_charptr(domain,
>+ IPA_DOMAIN_ID);
>+ new_head->forest_root_id = slapi_entry_attr_get_charptr(root_domain,
>+ IPA_DOMAIN_ID);
>+ new_head->next = head;
>+
>+ return new_head;
>+}
>+
>+/* Searches for the domain_info struct with the specified domain_id
>+ * in the linked list. Returns the forest root domain's ID, or NULL for
>+ * local ranges. */
>+
>+static char* get_forest_root_id(struct domain_info *head, char* domain_id) {
>+
>+ /* For local ranges there is no forest root domain,
>+ * so consider only ranges with domain_id set */
>+ if (domain_id != NULL) {
>+ while(head) {
>+ if (strcasecmp(head->domain_id, domain_id) == 0) {
>+ return head->forest_root_id;
>+ }
>+ head = head->next;
>+ }
>+ }
>+
>+ return NULL;
>+}
>+
>+
>+/*
>+ * This function builds a mapping from domain ID to forest
>+ * root domain ID.
>+ */
>+
>+static struct domain_info* build_domain_to_forest_root_map(struct domain_info *head,
>+ struct ipa_range_check_ctx *ctx){
>+
>+ Slapi_PBlock *root_domain_search_pb = NULL;
>+ Slapi_PBlock *child_domain_search_pb = NULL;
>+ Slapi_Entry **root_domain_entries = NULL;
>+ Slapi_Entry **child_domain_entries = NULL;
>+ Slapi_DN *base_dn = NULL;
>+ Slapi_RDN *rdn = NULL;
>+
>+ int search_result;
>+ int ret;
>+
>+ /* Set the base DN for the search to cn=ad, cn=trusts, $SUFFIX */
>+ base_dn = slapi_sdn_new_dn_byref(ctx->base_dn);
>+ if (base_dn == NULL) {
>+ LOG_FATAL("Failed to convert base DN.\n");
>+ ret = LDAP_OPERATIONS_ERROR;
>+ goto done;
>+ }
>+
>+ rdn = slapi_rdn_new();
>+ if (rdn == NULL) {
>+ LOG_FATAL("Failed to obtain RDN struct.\n");
>+ ret = LDAP_OPERATIONS_ERROR;
>+ goto done;
>+ }
>+
>+ slapi_rdn_add(rdn, "cn", "trusts");
>+ slapi_sdn_add_rdn(base_dn, rdn);
>+ slapi_rdn_done(rdn);
>+
>+ slapi_rdn_add(rdn, "cn", "ad");
>+ slapi_sdn_add_rdn(base_dn, rdn);
>+ slapi_rdn_done(rdn);
>+
>+ /* Allocate a new parameter block */
>+ root_domain_search_pb = slapi_pblock_new();
>+ if (root_domain_search_pb == NULL) {
>+ LOG_FATAL("Failed to create new pblock.\n");
>+ ret = LDAP_OPERATIONS_ERROR;
>+ //goto done;
>+ }
>+
>+ /* Search for all the root domains, note the LDAP_SCOPE_ONELEVEL */
>+ slapi_search_internal_set_pb(root_domain_search_pb,
>+ slapi_sdn_get_dn(base_dn),
>+ LDAP_SCOPE_ONELEVEL, DOMAIN_ID_FILTER,
>+ NULL, 0, NULL, NULL, ctx->plugin_id, 0);
>+
>+ ret = slapi_search_internal_pb(root_domain_search_pb);
>+ if (ret != 0) {
>+ LOG_FATAL("Starting internal search failed.\n");
>+ goto done;
>+ }
>+
>+ ret = slapi_pblock_get(root_domain_search_pb, SLAPI_PLUGIN_INTOP_RESULT, &search_result);
>+ if (ret != 0 || search_result != LDAP_SUCCESS) {
>+ LOG_FATAL("Internal search failed.\n");
>+ ret = LDAP_OPERATIONS_ERROR;
>+ goto done;
>+ }
>+
>+ ret = slapi_pblock_get(root_domain_search_pb, SLAPI_PLUGIN_INTOP_SEARCH_ENTRIES,
>+ &root_domain_entries);
>+
>+ if (ret != 0) {
>+ LOG_FATAL("Failed to read searched root domain entries.\n");
>+ goto done;
>+ }
>+
>+ if (root_domain_entries == NULL || root_domain_entries[0] == NULL) {
>+ LOG("No existing root domain entries.\n");
>+ ret = 0;
>+ goto done;
>+ }
>+
>+ /* now we iterate over root domains */
>+ for (int i = 0; root_domain_entries[i] != NULL; i++) {
>+ /* map root domain to itself */
>+ head = map_domain_to_root(head,
>+ root_domain_entries[i],
>+ root_domain_entries[i]);
>+ if (head == NULL) {
>+ LOG_FATAL("Error when allocating new domain_info struct.");
>+ goto child_done;
>+ }
>+
>+ /* let's reuse the variables */
>+ child_domain_search_pb = NULL;
>+ child_domain_entries = NULL;
>+
>+ /* allocate a new parameter block */
>+ child_domain_search_pb = slapi_pblock_new();
>+ if (child_domain_search_pb == NULL) {
>+ LOG_FATAL("Failed to create new pblock.\n");
>+ ret = LDAP_OPERATIONS_ERROR;
>+ goto child_done;
>+ }
>+
>+ /* search for all the child domains, note the LDAP_SCOPE_ONELEVEL */
>+ slapi_search_internal_set_pb(child_domain_search_pb,
>+ slapi_entry_get_dn_const(root_domain_entries[i]),
>+ LDAP_SCOPE_ONELEVEL, DOMAIN_ID_FILTER,
>+ NULL, 0, NULL, NULL, ctx->plugin_id, 0);
>+
>+ ret = slapi_search_internal_pb(child_domain_search_pb);
>+ if (ret != 0) {
>+ LOG_FATAL("Starting internal search failed.\n");
>+ goto child_done;
>+ }
>+
>+ ret = slapi_pblock_get(child_domain_search_pb, SLAPI_PLUGIN_INTOP_RESULT, &search_result);
>+ if (ret != 0 || search_result != LDAP_SUCCESS) {
>+ LOG_FATAL("Internal search failed.\n");
>+ ret = LDAP_OPERATIONS_ERROR;
>+ goto child_done;
>+ }
>+
>+ ret = slapi_pblock_get(child_domain_search_pb, SLAPI_PLUGIN_INTOP_SEARCH_ENTRIES,
>+ &child_domain_entries);
>+
>+ if (ret != 0) {
>+ LOG_FATAL("Failed to read searched child domain entries.\n");
>+ goto child_done;
>+ }
>+
>+ if (child_domain_entries == NULL || child_domain_entries[0] == NULL) {
>+ LOG("No existing child domain entries.\n");
>+ ret = 0;
>+ goto child_done;
>+ }
>+
>+ /* Now we iterate over child domains */
>+ for (int j = 0; child_domain_entries[j] != NULL; j++) {
>+
>+ /* map each child domain to the root domain */
>+ head = map_domain_to_root(head,
>+ child_domain_entries[j],
>+ root_domain_entries[i]);
>+
>+ }
>+
>+ child_done:
>+ slapi_free_search_results_internal(child_domain_search_pb);
>+ slapi_pblock_destroy(child_domain_search_pb);
>+ }
>+
>+done:
>+ slapi_free_search_results_internal(root_domain_search_pb);
>+ slapi_pblock_destroy(root_domain_search_pb);
>+ slapi_rdn_free(&rdn);
>+
>+ return head;
>+
>+}
>+
>+static int slapi_entry_to_range_info(struct domain_info *domain_info_head,
>+ struct slapi_entry *entry,
> struct range_info **_range)
> {
> int ret;
>@@ -97,6 +316,9 @@ static int slapi_entry_to_range_info(struct slapi_entry *entry,
> }
>
> range->domain_id = slapi_entry_attr_get_charptr(entry, IPA_DOMAIN_ID);
>+ range->id_range_type = slapi_entry_attr_get_charptr(entry, IPA_RANGE_TYPE);
>+ range->forest_root_id = get_forest_root_id(domain_info_head,
>+ range->domain_id);
>
> ul_val = slapi_entry_attr_get_ulong(entry, IPA_BASE_ID);
> if (ul_val == 0 || ul_val >= UINT32_MAX) {
>@@ -161,58 +383,67 @@ static bool intervals_overlap(uint32_t x, uint32_t base, uint32_t x_size, uint32
> * | | / \ |
> * new range: base rid sec_rid
> **/
>-static int ranges_overlap(struct range_info *r1, struct range_info *r2)
>+static int check_ranges(struct range_info *r1, struct range_info *r2)
> {
>+ /* Do not check overlaps of range with the range itself */
> if (r1->name != NULL && r2->name != NULL &&
> strcasecmp(r1->name, r2->name) == 0) {
> return 0;
> }
>
>- /* check if base range overlaps with existing base range */
>- if (intervals_overlap(r1->base_id, r2->base_id,
>- r1->id_range_size, r2->id_range_size)){
>- return 1;
>+ /* Check if base range overlaps with existing base range.
>+ * Exception: ipa-ad-trust-posix ranges from the same forest */
>+ if (!(strcasecmp(r1->id_range_type, AD_TRUST_POSIX_RANGE_TYPE) &&
>+ strcasecmp(r2->id_range_type, AD_TRUST_POSIX_RANGE_TYPE) &&
>+ r1->forest_root_id != NULL && r2->forest_root_id !=NULL &&
>+ strcasecmp(r1->forest_root_id, r2->forest_root_id) == 0)) {
>+
>+ if (intervals_overlap(r1->base_id, r2->base_id,
>+ r1->id_range_size, r2->id_range_size)){
>+ return 1;
>+ }
>+
> }
>
>- /* if both base_rid and secondary_base_rid = 0, the rid range is not set */
>- bool rid_ranges_set = (r1->base_rid != 0 || r1->secondary_base_rid != 0) &&
>- (r2->base_rid != 0 || r2->secondary_base_rid != 0);
>-
>- /**
>- * ipaNTTrustedDomainSID is not set for local ranges, use it to
>- * determine the type of the range **/
>- bool local_ranges = r1->domain_id == NULL && r2->domain_id == NULL;
>-
>+ /* Following checks apply for the ranges from the same domain */
> bool ranges_from_same_domain =
> (r1->domain_id == NULL && r2->domain_id == NULL) ||
> (r1->domain_id != NULL && r2->domain_id != NULL &&
> strcasecmp(r1->domain_id, r2->domain_id) == 0);
>
>- /**
>- * in case rid range is not set or ranges belong to different domains
>- * we can skip rid range tests as they are irrelevant **/
>- if (rid_ranges_set && ranges_from_same_domain){
>-
>- /* check if rid range overlaps with existing rid range */
>- if (intervals_overlap(r1->base_rid, r2->base_rid,
>- r1->id_range_size, r2->id_range_size))
>- return 2;
>-
>- /**
>- * The following 3 checks are relevant only if both ranges are local.
>- * Check if secondary rid range overlaps with existing secondary rid
>- * range. **/
>- if (local_ranges){
>+ if (ranges_from_same_domain) {
>+
>+ /* Ranges from the same domain must have the same type */
>+ if (strcasecmp(r1->id_range_type, r2->id_range_type) != 0) {
>+ return 6;
>+ }
>+
>+ /* For ipa-local or ipa-ad-trust range types primary RID ranges should
>+ * not overlap */
>+ if (strcasecmp(r1->id_range_type, AD_TRUST_RANGE_TYPE) == 0 ||
>+ strcasecmp(r1->id_range_type, LOCAL_RANGE_TYPE) == 0) {
>+
>+ /* Check if rid range overlaps with existing rid range */
>+ if (intervals_overlap(r1->base_rid, r2->base_rid,
>+ r1->id_range_size, r2->id_range_size))
>+ return 2;
>+ }
>+
>+ /* The following 3 checks are relevant only if both ranges are local. */
>+ if (strcasecmp(r1->id_range_type, LOCAL_RANGE_TYPE) == 0){
>+
>+ /* Check if secondary RID range overlaps with existing secondary or
>+ * primary RID range. */
> if (intervals_overlap(r1->secondary_base_rid,
> r2->secondary_base_rid, r1->id_range_size, r2->id_range_size))
> return 3;
>
>- /* check if rid range overlaps with existing secondary rid range */
>+ /* Check if RID range overlaps with existing secondary RID range */
> if (intervals_overlap(r1->base_rid, r2->secondary_base_rid,
> r1->id_range_size, r2->id_range_size))
> return 4;
>
>- /* check if secondary rid range overlaps with existing rid range */
>+ /* Check if secondary RID range overlaps with existing RID range */
> if (intervals_overlap(r1->secondary_base_rid, r2->base_rid,
> r1->id_range_size, r2->id_range_size))
> return 5;
>@@ -248,9 +479,10 @@ static int ipa_range_check_pre_op(Slapi_PBlock *pb, int modtype)
> int search_result;
> Slapi_Entry **search_entries = NULL;
> size_t c;
>- int no_overlap = 0;
>+ int ranges_valid = 0;
> const char *check_attr;
> char *errmsg = NULL;
>+ struct domain_info *domain_info_head = NULL;
>
> ret = slapi_pblock_get(pb, SLAPI_IS_REPLICATED_OPERATION, &is_repl_op);
> if (ret != 0) {
>@@ -337,7 +569,10 @@ static int ipa_range_check_pre_op(Slapi_PBlock *pb, int modtype)
> goto done;
> }
>
>- ret = slapi_entry_to_range_info(entry, &new_range);
>+ /* build a linked list of domain_info structs */
>+ domain_info_head = build_domain_to_forest_root_map(domain_info_head, ctx);
>+
>+ ret = slapi_entry_to_range_info(domain_info_head, entry, &new_range);
> if (ret != 0) {
> LOG_FATAL("Failed to convert LDAP entry to range struct.\n");
> goto done;
>@@ -381,19 +616,20 @@ static int ipa_range_check_pre_op(Slapi_PBlock *pb, int modtype)
> }
>
> for (c = 0; search_entries[c] != NULL; c++) {
>- ret = slapi_entry_to_range_info(search_entries[c], &old_range);
>+ ret = slapi_entry_to_range_info(domain_info_head, search_entries[c],
>+ &old_range);
> if (ret != 0) {
> LOG_FATAL("Failed to convert LDAP entry to range struct.\n");
> goto done;
> }
>
>- no_overlap = ranges_overlap(new_range, old_range);
>+ ranges_valid = check_ranges(new_range, old_range);
> free(old_range);
> old_range = NULL;
>- if (no_overlap != 0) {
>+ if (ranges_valid != 0) {
> ret = LDAP_CONSTRAINT_VIOLATION;
>
>- switch (no_overlap){
>+ switch (ranges_valid){
> case 1:
> errmsg = "New base range overlaps with existing base range.";
> break;
>@@ -409,6 +645,8 @@ static int ipa_range_check_pre_op(Slapi_PBlock *pb, int modtype)
> case 5:
> errmsg = "New secondary rid range overlaps with existing primary rid range.";
> break;
>+ case 6:
>+ errmsg = "New ID range has invalid type. All ranges in the same domain must be of the same type.";
> default:
> errmsg = "New range overlaps with existing one.";
> break;
>@@ -432,6 +670,14 @@ done:
> slapi_entry_free(entry);
> }
>
>+ /* Remove the domain info linked list from memory */
>+ struct domain_info *next;
>+ while(domain_info_head) {
>+ next = domain_info_head->next;
>+ free(domain_info_head);
>+ domain_info_head = next;
>+ }
>+
> if (ret != 0) {
> if (errmsg == NULL) {
> errmsg = "Range Check error";
>--
>1.8.5.3
>
>_______________________________________________
>Freeipa-devel mailing list
>Freeipa-devel at redhat.com
>https://www.redhat.com/mailman/listinfo/freeipa-devel
--
/ Alexander Bokovoy
More information about the Freeipa-devel
mailing list