On 04/22/2014 12:50 PM, Tomas Babej wrote:
> On 04/17/2014 02:44 PM, Alexander Bokovoy wrote:
>> You replace this by
>> range->base_rid_set = (slapi_entry_attr_find(entry, IPA_BASE_RID,
> &attr) == -1);
>
> You probably meant "== 0". Fixed.
>
>> I know that is was in your original code, but can we get numbers
>> replaced by an enum? I'd prefer to see symbolic names used instead of
>> numbers.
> Fixed in a separate patch 0178 (attached).
>
>> Please expand the message here, may be something like
>> LOG("Empty forest root map as trusts are not enabled on this IPA
> server\n");
>
> Fixed.
>
> Updated patchset attached.
>
> Tomas
I amended the commit message in the patch 178, whole patchset attached.
--
Tomas Babej
Associate Software Engineer | Red Hat | Identity Management
RHCE | Brno Site | IRC: tbabej | freeipa.org
>From 2afc058870fea26a5079dd5d09994e2cb7196fe6 Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Wed, 16 Apr 2014 17:28:34 +0200
Subject: [PATCH] ipa_range_check: Fix typo when comparing strings using
strcasecmp
Part of: https://fedorahosted.org/freeipa/ticket/4137
---
daemons/ipa-slapi-plugins/ipa-range-check/ipa_range_check.c | 8 ++++----
1 file changed, 4 insertions(+), 4 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 e9f69674ccb45dff9fdd4d78c11b6e90846d179f..b55624912d4de976a036e5fd2531373addcccefc 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
@@ -387,10 +387,10 @@ static int check_ranges(struct range_info *r1, struct range_info *r2)
/* 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 (!((strcasecmp(r1->id_range_type, AD_TRUST_POSIX_RANGE_TYPE) == 0) &&
+ (strcasecmp(r2->id_range_type, AD_TRUST_POSIX_RANGE_TYPE) == 0) &&
+ (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)){
--
1.8.5.3
>From e5add061260ccff3a32c2510094a4b8fe5e7acc4 Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Wed, 16 Apr 2014 17:26:07 +0200
Subject: [PATCH] ipa_range_check: Do not fail when no trusted domain is
available
When building the domain to forest root map, we need to take the case
of IPA server having no trusted domains configured at all. Do not abort
the checks, but return an empty map instead.
Part of: https://fedorahosted.org/freeipa/ticket/4137
---
daemons/ipa-slapi-plugins/ipa-range-check/ipa_range_check.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 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 9a51d4196d4fdee20d053f40fc1b4a79f24df806..e9f69674ccb45dff9fdd4d78c11b6e90846d179f 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
@@ -173,6 +173,8 @@ static int build_domain_to_forest_root_map(struct domain_info **head,
int search_result;
int ret = 0;
+ LOG("Building forest root map \n");
+
/* Set the base DN for the search to cn=ad, cn=trusts, $SUFFIX */
ret = asprintf(&base, "cn=ad,cn=trusts,%s", ctx->base_dn);
if (ret == -1) {
@@ -211,8 +213,14 @@ static int build_domain_to_forest_root_map(struct domain_info **head,
ret = slapi_pblock_get(trusted_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;
+
+ /* If the search for the trusted domains fails,
+ * AD Trust support on IPA server is not available */
+
+ LOG("Empty forest root map as trusts are not enabled on this IPA server.\n");
+ ret = 0;
+ *head = NULL;
+
goto done;
}
--
1.8.5.3
>From e9e2b123adeebdbe5623285e83e64b28b0b91647 Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Wed, 16 Apr 2014 17:22:46 +0200
Subject: [PATCH] ipa_range_check: Make a new copy of forest_root_id attribute
for range_info struct
Not making a new copy of this attribute creates multiple frees caused by multiple
pointers to the same forest_root_id from all the range_info structs for all the
domains belonging to given forest.
Part of: https://fedorahosted.org/freeipa/ticket/4137
---
daemons/ipa-slapi-plugins/ipa-range-check/ipa_range_check.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
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 fca4819a0889c4e43d76e8b9bcd2a6751aa3efc7..9a51d4196d4fdee20d053f40fc1b4a79f24df806 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
@@ -147,7 +147,7 @@ static char* get_forest_root_id(struct domain_info *head, char* domain_id) {
if (domain_id != NULL) {
while(head) {
if (strcasecmp(head->domain_id, domain_id) == 0) {
- return head->forest_root_id;
+ return slapi_ch_strdup(head->forest_root_id);
}
head = head->next;
}
--
1.8.5.3
>From 98b1599f942573173574ed6cf20116e62f47d019 Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Wed, 16 Apr 2014 17:20:55 +0200
Subject: [PATCH] ipa_range_check: Connect the new node of the linked list
Part of: https://fedorahosted.org/freeipa/ticket/4137
---
daemons/ipa-slapi-plugins/ipa-range-check/ipa_range_check.c | 1 +
1 file changed, 1 insertion(+)
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 ea7658ed6fb96ee2cbf63a3f0fbeecd758312301..fca4819a0889c4e43d76e8b9bcd2a6751aa3efc7 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
@@ -131,6 +131,7 @@ static int map_domain_to_root(struct domain_info **head,
new_head->forest_root_id = slapi_entry_attr_get_charptr(root_domain,
IPA_DOMAIN_ID);
new_head->next = *head;
+ *head = new_head;
return 0;
}
--
1.8.5.3
>From 79d057773cf2d685f2cf1d682f2d329b75dae8be Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Wed, 16 Apr 2014 17:15:55 +0200
Subject: [PATCH] ipa_range_check: Use special attributes to determine presence
of RID bases
The slapi_entry_attr_get_ulong which is used to get value of the RID base
attributes returns 0 in case the attribute is not set at all. We need
to distinguish this situation from the situation where RID base attributes
are present, but deliberately set to 0.
Otherwise this can cause false negative results of checks in the range_check
plugin.
Part of: https://fedorahosted.org/freeipa/ticket/4137
---
.../ipa-range-check/ipa_range_check.c | 30 +++++++++++++++-------
1 file changed, 21 insertions(+), 9 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 da5169e6e9bf74d5fbbf3aea40ee3e1a2c8f6016..ea7658ed6fb96ee2cbf63a3f0fbeecd758312301 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
@@ -88,6 +88,8 @@ struct range_info {
uint32_t id_range_size;
uint32_t base_rid;
uint32_t secondary_base_rid;
+ bool base_rid_set;
+ bool secondary_base_rid_set;
};
static void free_range_info(struct range_info *range) {
@@ -281,6 +283,7 @@ static int slapi_entry_to_range_info(struct domain_info *domain_info_head,
int ret;
unsigned long ul_val;
struct range_info *range = NULL;
+ Slapi_Attr *attr;
range = calloc(1, sizeof(struct range_info));
if (range == NULL) {
@@ -326,6 +329,10 @@ static int slapi_entry_to_range_info(struct domain_info *domain_info_head,
}
range->secondary_base_rid = ul_val;
+ /* slapi_entry_attr_find return 0 if requested attribute is present in entry */
+ range->base_rid_set = (slapi_entry_attr_find(entry, IPA_BASE_RID, &attr) == 0);
+ range->secondary_base_rid_set = (slapi_entry_attr_find(entry, IPA_SECONDARY_BASE_RID, &attr) == 0);
+
*_range = range;
ret = 0;
@@ -398,12 +405,14 @@ static int check_ranges(struct range_info *r1, struct range_info *r2)
/* 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))
+ /* Check if primary rid range overlaps with existing primary rid range */
+ if ((r1->base_rid_set && r2->base_rid_set) &&
+ intervals_overlap(r1->base_rid, r2->base_rid,
+ r1->id_range_size, r2->id_range_size))
return 2;
}
@@ -412,18 +421,21 @@ static int check_ranges(struct range_info *r1, struct range_info *r2)
/* 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))
+ if ((r1->secondary_base_rid_set && r2->secondary_base_rid_set) &&
+ 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 */
- if (intervals_overlap(r1->base_rid, r2->secondary_base_rid,
- r1->id_range_size, r2->id_range_size))
+ if ((r1->base_rid_set && r2->secondary_base_rid_set) &&
+ 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 */
- if (intervals_overlap(r1->secondary_base_rid, r2->base_rid,
- r1->id_range_size, r2->id_range_size))
+ if ((r1->secondary_base_rid_set && r2->base_rid_set) &&
+ intervals_overlap(r1->secondary_base_rid, r2->base_rid,
+ r1->id_range_size, r2->id_range_size))
return 5;
}
}
--
1.8.5.3
>From 369161c7123c353d7b03cb187c9a66783bb347f1 Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Tue, 22 Apr 2014 12:34:12 +0200
Subject: [PATCH] ipa_range_check: Change range_check return values from int to
range_check_result_t enum
Using integers for return values that are used for complex casing can be fragile
and typo-prone. Change range_check function to return range_check_result_t enum,
whose values properly describes each of the range_check results.
Part of: https://fedorahosted.org/freeipa/ticket/4137
---
.../ipa-range-check/ipa_range_check.c | 42 +++++++++++++---------
1 file changed, 26 insertions(+), 16 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 b55624912d4de976a036e5fd2531373addcccefc..d659dc7d8b510cb5719dc9ff1db2ef55e2d237c4 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
@@ -79,6 +79,16 @@ struct ipa_range_check_ctx {
const char *base_dn;
};
+typedef enum {
+ RANGE_CHECK_OK,
+ RANGE_CHECK_BASE_OVERLAP,
+ RANGE_CHECK_PRIMARY_PRIMARY_RID_OVERLAP,
+ RANGE_CHECK_SECONDARY_SECONDARY_RID_OVERLAP,
+ RANGE_CHECK_PRIMARY_SECONDARY_RID_OVERLAP,
+ RANGE_CHECK_SECONDARY_PRIMARY_RID_OVERLAP,
+ RANGE_CHECK_DIFFERENT_TYPE_IN_DOMAIN,
+} range_check_result_t;
+
struct range_info {
char *name;
char *domain_id;
@@ -377,12 +387,12 @@ static bool intervals_overlap(uint32_t x, uint32_t base, uint32_t x_size, uint32
* | | / \ |
* new range: base rid sec_rid
**/
-static int check_ranges(struct range_info *r1, struct range_info *r2)
+static range_check_result_t 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;
+ return RANGE_CHECK_OK;
}
/* Check if base range overlaps with existing base range.
@@ -394,7 +404,7 @@ static int check_ranges(struct range_info *r1, struct range_info *r2)
if (intervals_overlap(r1->base_id, r2->base_id,
r1->id_range_size, r2->id_range_size)){
- return 1;
+ return RANGE_CHECK_BASE_OVERLAP;
}
}
@@ -409,7 +419,7 @@ static int check_ranges(struct range_info *r1, struct range_info *r2)
/* Ranges from the same domain must have the same type */
if (strcasecmp(r1->id_range_type, r2->id_range_type) != 0) {
- return 6;
+ return RANGE_CHECK_DIFFERENT_TYPE_IN_DOMAIN;
}
/* For ipa-local or ipa-ad-trust range types primary RID ranges should
@@ -422,7 +432,7 @@ static int check_ranges(struct range_info *r1, struct range_info *r2)
if ((r1->base_rid_set && r2->base_rid_set) &&
intervals_overlap(r1->base_rid, r2->base_rid,
r1->id_range_size, r2->id_range_size))
- return 2;
+ return RANGE_CHECK_PRIMARY_PRIMARY_RID_OVERLAP;
}
/* The following 3 checks are relevant only if both ranges are local. */
@@ -433,23 +443,23 @@ static int check_ranges(struct range_info *r1, struct range_info *r2)
if ((r1->secondary_base_rid_set && r2->secondary_base_rid_set) &&
intervals_overlap(r1->secondary_base_rid, r2->secondary_base_rid,
r1->id_range_size, r2->id_range_size))
- return 3;
+ return RANGE_CHECK_SECONDARY_SECONDARY_RID_OVERLAP;
/* Check if RID range overlaps with existing secondary RID range */
if ((r1->base_rid_set && r2->secondary_base_rid_set) &&
intervals_overlap(r1->base_rid, r2->secondary_base_rid,
r1->id_range_size, r2->id_range_size))
- return 4;
+ return RANGE_CHECK_PRIMARY_SECONDARY_RID_OVERLAP;
/* Check if secondary RID range overlaps with existing RID range */
if ((r1->secondary_base_rid_set && r2->base_rid_set) &&
intervals_overlap(r1->secondary_base_rid, r2->base_rid,
r1->id_range_size, r2->id_range_size))
- return 5;
+ return RANGE_CHECK_SECONDARY_PRIMARY_RID_OVERLAP;
}
}
- return 0;
+ return RANGE_CHECK_OK;
}
static int ipa_range_check_start(Slapi_PBlock *pb)
@@ -629,26 +639,26 @@ static int ipa_range_check_pre_op(Slapi_PBlock *pb, int modtype)
ranges_valid = check_ranges(new_range, old_range);
free_range_info(old_range);
old_range = NULL;
- if (ranges_valid != 0) {
+ if (ranges_valid != RANGE_CHECK_OK) {
ret = LDAP_CONSTRAINT_VIOLATION;
switch (ranges_valid){
- case 1:
+ case RANGE_CHECK_BASE_OVERLAP:
errmsg = "New base range overlaps with existing base range.";
break;
- case 2:
+ case RANGE_CHECK_PRIMARY_PRIMARY_RID_OVERLAP:
errmsg = "New primary rid range overlaps with existing primary rid range.";
break;
- case 3:
+ case RANGE_CHECK_SECONDARY_SECONDARY_RID_OVERLAP:
errmsg = "New secondary rid range overlaps with existing secondary rid range.";
break;
- case 4:
+ case RANGE_CHECK_PRIMARY_SECONDARY_RID_OVERLAP:
errmsg = "New primary rid range overlaps with existing secondary rid range.";
break;
- case 5:
+ case RANGE_CHECK_SECONDARY_PRIMARY_RID_OVERLAP:
errmsg = "New secondary rid range overlaps with existing primary rid range.";
break;
- case 6:
+ case RANGE_CHECK_DIFFERENT_TYPE_IN_DOMAIN:
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.";
--
1.8.5.3
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
