[Freeipa-devel] [PATCH] 584 migration: fix import of wsgiref.util
https://fedorahosted.org/freeipa/ticket/4293 -- Petr Vobornik From dc4eaf9d622b4eac9fb3d942b23083bfb903be9a Mon Sep 17 00:00:00 2001 From: Petr Vobornik pvobo...@redhat.com Date: Tue, 1 Apr 2014 08:45:08 +0200 Subject: [PATCH] migration: fix import of wsgiref.util https://fedorahosted.org/freeipa/ticket/4293 --- install/migration/migration.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/install/migration/migration.py b/install/migration/migration.py index 27e23a59f660c791c12948b4c40406d03b0f0966..acc1ee48dd8744f6abd2d7a59f0d5feb54fc93f9 100644 --- a/install/migration/migration.py +++ b/install/migration/migration.py @@ -23,7 +23,7 @@ Password migration script import cgi import errno import glob -import wsgiref +from wsgiref.util import request_uri from ipapython.ipa_log_manager import root_logger from ipapython.ipautil import get_ipa_basedn @@ -37,7 +37,7 @@ def wsgi_redirect(start_response, loc): return [] def get_ui_url(environ): -full_url = wsgiref.util.request_uri(environ) +full_url = request_uri(environ) index = full_url.rfind(environ.get('SCRIPT_NAME','')) if index == -1: raise ValueError('Cannot strip the script URL from full URL %s' % full_url) -- 1.8.5.3 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0161] ipa-range-check: Fix memory leaks when freeing range object
On Thu, Mar 27, 2014 at 01:14:52PM +0100, Tomas Babej wrote: Hi, When cleaning the range_info struct, simple free of the struct is not enough, we have to free contents of char pointers in the struct as well. https://fedorahosted.org/freeipa/ticket/4276 [...] 1 file changed, 13 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 0ef33e5869bbcb4f721394ce35e2338095bf5d36..c877a7dc445b31b3de085aa66028d7652df6b9cc 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 @@ -96,6 +96,15 @@ struct domain_info { struct domain_info *next; }; +static void free_range_info(struct range_info *range) { +if (range != NULL) { +slapi_ch_free_string((range-name)); +slapi_ch_free_string((range-domain_id)); +slapi_ch_free_string((range-forest_root_id)); +slapi_ch_free_string((range-id_range_type)); +free(range); +} +} In master, the range_info is struct range_info { char *name; char *domain_id; uint32_t base_id; uint32_t id_range_size; uint32_t base_rid; uint32_t secondary_base_rid; }; -- no forest_root_id and no id_range_type. So NACK for applying to master. Perhaps there is some dependency patch? -- Jan Pazdziora Principal Software Engineer, Identity Management Engineering, Red Hat ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] FreeIPA repository and it's committers
On Fri, Mar 28, 2014 at 01:14:31PM +0100, Martin Kosek wrote: This week there was a request to be added to the list of FreeIPA committers, which I denied as the person was not a member of the short list of selected trusted active core upstream developers doing the pushes. While that makes sense, I don't see why freeipa-docs needs to share the same list of committers. -- Jan Pazdziora Principal Software Engineer, Identity Management Engineering, Red Hat ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0161] ipa-range-check: Fix memory leaks when freeing range object
On Tue, 01 Apr 2014, Jan Pazdziora wrote: On Thu, Mar 27, 2014 at 01:14:52PM +0100, Tomas Babej wrote: Hi, When cleaning the range_info struct, simple free of the struct is not enough, we have to free contents of char pointers in the struct as well. https://fedorahosted.org/freeipa/ticket/4276 [...] 1 file changed, 13 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 0ef33e5869bbcb4f721394ce35e2338095bf5d36..c877a7dc445b31b3de085aa66028d7652df6b9cc 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 @@ -96,6 +96,15 @@ struct domain_info { struct domain_info *next; }; +static void free_range_info(struct range_info *range) { +if (range != NULL) { +slapi_ch_free_string((range-name)); +slapi_ch_free_string((range-domain_id)); +slapi_ch_free_string((range-forest_root_id)); +slapi_ch_free_string((range-id_range_type)); +free(range); +} +} In master, the range_info is struct range_info { char *name; char *domain_id; uint32_t base_id; uint32_t id_range_size; uint32_t base_rid; uint32_t secondary_base_rid; }; -- no forest_root_id and no id_range_type. So NACK for applying to master. Perhaps there is some dependency patch? I think Tomas took his own range check patch into account. I would also prefer splitting their dependencies. First make this one to work on master, then add forest_root_id and id_range_type freeing as part of the range check patches. -- / Alexander Bokovoy ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0161] ipa-range-check: Fix memory leaks when freeing range object
On 04/01/2014 09:11 AM, Alexander Bokovoy wrote: On Tue, 01 Apr 2014, Jan Pazdziora wrote: On Thu, Mar 27, 2014 at 01:14:52PM +0100, Tomas Babej wrote: Hi, When cleaning the range_info struct, simple free of the struct is not enough, we have to free contents of char pointers in the struct as well. https://fedorahosted.org/freeipa/ticket/4276 [...] 1 file changed, 13 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 0ef33e5869bbcb4f721394ce35e2338095bf5d36..c877a7dc445b31b3de085aa66028d7652df6b9cc 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 @@ -96,6 +96,15 @@ struct domain_info { struct domain_info *next; }; +static void free_range_info(struct range_info *range) { +if (range != NULL) { +slapi_ch_free_string((range-name)); +slapi_ch_free_string((range-domain_id)); +slapi_ch_free_string((range-forest_root_id)); +slapi_ch_free_string((range-id_range_type)); +free(range); +} +} In master, the range_info is struct range_info { char *name; char *domain_id; uint32_t base_id; uint32_t id_range_size; uint32_t base_rid; uint32_t secondary_base_rid; }; -- no forest_root_id and no id_range_type. So NACK for applying to master. Perhaps there is some dependency patch? I think Tomas took his own range check patch into account. I would also prefer splitting their dependencies. First make this one to work on master, then add forest_root_id and id_range_type freeing as part of the range check patches. Yes, that was the intention. Mistake on my part, I'll send updated patches. -- Tomas Babej Associate Software Engineer | Red Hat | Identity Management RHCE | Brno Site | IRC: tbabej | freeipa.org ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 584 migration: fix import of wsgiref.util
On Tue, Apr 01, 2014 at 08:47:25AM +0200, Petr Vobornik wrote: https://fedorahosted.org/freeipa/ticket/4293 -- Petr Vobornik From dc4eaf9d622b4eac9fb3d942b23083bfb903be9a Mon Sep 17 00:00:00 2001 From: Petr Vobornik pvobo...@redhat.com Date: Tue, 1 Apr 2014 08:45:08 +0200 Subject: [PATCH] migration: fix import of wsgiref.util https://fedorahosted.org/freeipa/ticket/4293 --- install/migration/migration.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/install/migration/migration.py b/install/migration/migration.py index 27e23a59f660c791c12948b4c40406d03b0f0966..acc1ee48dd8744f6abd2d7a59f0d5feb54fc93f9 100644 --- a/install/migration/migration.py +++ b/install/migration/migration.py @@ -23,7 +23,7 @@ Password migration script import cgi import errno import glob -import wsgiref +from wsgiref.util import request_uri from ipapython.ipa_log_manager import root_logger from ipapython.ipautil import get_ipa_basedn @@ -37,7 +37,7 @@ def wsgi_redirect(start_response, loc): return [] def get_ui_url(environ): -full_url = wsgiref.util.request_uri(environ) +full_url = request_uri(environ) Sadly, this antipattern seems needed even if the proper solution would be to fix wsgi to properly export util. Is there bug filed for that. Ack based on reading the code and wsgiref documentation where the same import style is used. -- Jan Pazdziora Principal Software Engineer, Identity Management Engineering, Red Hat ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] FreeIPA repository and it's committers
On 04/01/2014 09:05 AM, Jan Pazdziora wrote: On Fri, Mar 28, 2014 at 01:14:31PM +0100, Martin Kosek wrote: This week there was a request to be added to the list of FreeIPA committers, which I denied as the person was not a member of the short list of selected trusted active core upstream developers doing the pushes. While that makes sense, I don't see why freeipa-docs needs to share the same list of committers. It does not need to share the same list of committers. I requested a separate group: https://fedorahosted.org/fedora-infrastructure/ticket/4287 Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0161] ipa-range-check: Fix memory leaks when freeing range object
On 04/01/2014 09:17 AM, Tomas Babej wrote: On 04/01/2014 09:11 AM, Alexander Bokovoy wrote: On Tue, 01 Apr 2014, Jan Pazdziora wrote: On Thu, Mar 27, 2014 at 01:14:52PM +0100, Tomas Babej wrote: Hi, When cleaning the range_info struct, simple free of the struct is not enough, we have to free contents of char pointers in the struct as well. https://fedorahosted.org/freeipa/ticket/4276 [...] 1 file changed, 13 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 0ef33e5869bbcb4f721394ce35e2338095bf5d36..c877a7dc445b31b3de085aa66028d7652df6b9cc 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 @@ -96,6 +96,15 @@ struct domain_info { struct domain_info *next; }; +static void free_range_info(struct range_info *range) { +if (range != NULL) { +slapi_ch_free_string((range-name)); +slapi_ch_free_string((range-domain_id)); +slapi_ch_free_string((range-forest_root_id)); +slapi_ch_free_string((range-id_range_type)); +free(range); +} +} In master, the range_info is struct range_info { char *name; char *domain_id; uint32_t base_id; uint32_t id_range_size; uint32_t base_rid; uint32_t secondary_base_rid; }; -- no forest_root_id and no id_range_type. So NACK for applying to master. Perhaps there is some dependency patch? I think Tomas took his own range check patch into account. I would also prefer splitting their dependencies. First make this one to work on master, then add forest_root_id and id_range_type freeing as part of the range check patches. Yes, that was the intention. Mistake on my part, I'll send updated patches. Updated patch attached. -- Tomas Babej Associate Software Engineer | Red Hat | Identity Management RHCE | Brno Site | IRC: tbabej | freeipa.org From d6d0d8a05fe138ec93e6dc0c6015e59c040aca78 Mon Sep 17 00:00:00 2001 From: Tomas Babej tba...@redhat.com Date: Tue, 1 Apr 2014 09:20:09 +0200 Subject: [PATCH] ipa-range-check: Fix memory leaks when freeing range object When cleaning the range_info struct, simple free of the struct is not enough, we have to free contents of char pointers in the struct as well. https://fedorahosted.org/freeipa/ticket/4276 --- .../ipa-slapi-plugins/ipa-range-check/ipa_range_check.c | 16 1 file changed, 12 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 391e2259b6eced31fed399c927cfe44c1d3e237e..4f0640eafdaed38cbf72beccc146009a599fa108 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 @@ -78,6 +78,14 @@ struct range_info { uint32_t secondary_base_rid; }; +static void free_range_info(struct range_info *range) { +if (range != NULL) { +slapi_ch_free_string((range-name)); +slapi_ch_free_string((range-domain_id)); +free(range); +} +} + static int slapi_entry_to_range_info(struct slapi_entry *entry, struct range_info **_range) { @@ -131,7 +139,7 @@ static int slapi_entry_to_range_info(struct slapi_entry *entry, done: if (ret != 0) { -free(range); +free_range_info(range); } return ret; @@ -388,7 +396,7 @@ static int ipa_range_check_pre_op(Slapi_PBlock *pb, int modtype) } no_overlap = ranges_overlap(new_range, old_range); -free(old_range); +free_range_info(old_range); old_range = NULL; if (no_overlap != 0) { ret = LDAP_CONSTRAINT_VIOLATION; @@ -426,8 +434,8 @@ done: slapi_free_search_results_internal(search_pb); slapi_pblock_destroy(search_pb); slapi_sdn_free(dn); -free(old_range); -free(new_range); +free_range_info(old_range); +free_range_info(new_range); if (free_entry) { slapi_entry_free(entry); } -- 1.8.5.3 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0158] Extend ipa-range-check DS plugin to handle range types
A slightly new version, properly adds new attributes of the range_info struct to the free_range_info method. Should be applied on top of my 161 patch. On 03/27/2014 01:11 PM, Tomas Babej wrote: The updated version handles the ret variable properly. It also makes sure the memory is properly freed. On 03/18/2014 04:45 PM, Alexander Bokovoy wrote: On Tue, 18 Mar 2014, Tomas Babej wrote: On 03/18/2014 09:19 AM, Alexander Bokovoy wrote: 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. I don't see much benefit of using array over linked list in this case. In both cases, we would need to build the data container, and it would be one-off operation (once for the ADD/MOD operation). Additionaly, using linked list, I can only pass around the pointer to the head of the list, instead of passing around the array and it's size. If you make a {NULL, NULL} entry as the last one, no need to pass array size. But anyway... I reworked the part of the patch that fetches the data from the LDAP as we discussed. Instead of performing multiple LDAP searches, we do a single extra search per operation. See comments below. +static struct domain_info* build_domain_to_forest_root_map(struct domain_info *head, + struct ipa_range_check_ctx *ctx){ + +Slapi_PBlock *trusted_domain_search_pb = NULL; +Slapi_Entry **trusted_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); given that slapi_search_internal_set_pb() wants 'const char *base', why not use asprintf() instead? Here is what we do in ipa-kdb: ret = asprintf(base, cn=ad,cn=trusts,%s, ipactx-base); if (ret == -1) { ret = ENOMEM; goto done; } That's enough, IMHO. 389-ds internally will anyway create new sdn explicitly from a string you've passed. + +/* Allocate a new parameter block */ +trusted_domain_search_pb = slapi_pblock_new(); +if (trusted_domain_search_pb == NULL) { +LOG_FATAL(Failed to create new pblock.\n); +ret = LDAP_OPERATIONS_ERROR; in done: label you don't deal with 'ret' at all. Do you need it? +//goto done; is it goto or not? +} + +/* Search for all the root domains, note the LDAP_SCOPE_ONELEVEL */ +slapi_search_internal_set_pb(trusted_domain_search_pb, + slapi_sdn_get_dn(base_dn), here just use 'base_dn'. + LDAP_SCOPE_SUBTREE, DOMAIN_ID_FILTER, + NULL, 0, NULL, NULL, ctx-plugin_id, 0); + +ret = slapi_search_internal_pb(trusted_domain_search_pb); +if (ret != 0) { +LOG_FATAL(Starting internal search
Re: [Freeipa-devel] [PATCH 0158] Extend ipa-range-check DS plugin to handle range types
On Tue, 01 Apr 2014, Tomas Babej wrote: From 736b3f747188696fd4a46ca63d91a6cca942fd56 Mon Sep 17 00:00:00 2001 From: Tomas Babej tba...@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 -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 void free_domain_info(struct domain_info *info) { +if (info != NULL) { +slapi_ch_free_string((info-domain_id)); +slapi_ch_free_string((info-forest_root_id)); +free_domain_info(info-next); +free(info); +} +} Please, don't use recursion in the freeing part, there is really no pressing need to do so. Just use while() like you do in get_forest_root_id(): +/* 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; +} + -- / Alexander Bokovoy ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0158] Extend ipa-range-check DS plugin to handle range types
On 04/01/2014 10:40 AM, Alexander Bokovoy wrote: On Tue, 01 Apr 2014, Tomas Babej wrote: From 736b3f747188696fd4a46ca63d91a6cca942fd56 Mon Sep 17 00:00:00 2001 From: Tomas Babej tba...@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 -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 void free_domain_info(struct domain_info *info) { +if (info != NULL) { +slapi_ch_free_string((info-domain_id)); +slapi_ch_free_string((info-forest_root_id)); +free_domain_info(info-next); +free(info); +} +} Please, don't use recursion in the freeing part, there is really no pressing need to do so. Just use while() like you do in get_forest_root_id(): +/* 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; +} + Fixed, updated patch attached. -- Tomas Babej Associate Software Engineer | Red Hat | Identity Management RHCE | Brno Site | IRC: tbabej | freeipa.org From f98082d6cfd902417af94d7cd22d3cc85980a782 Mon Sep 17 00:00:00 2001 From: Tomas Babej tba...@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 | 306 ++--- 1 file changed, 263 insertions(+), 43 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 4f0640eafdaed38cbf72beccc146009a599fa108..da5169e6e9bf74d5fbbf3aea40ee3e1a2c8f6016 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 @@ -37,7 +37,10 @@ * All rights reserved. * END COPYRIGHT BLOCK **/ +#define _GNU_SOURCE /* See feature_test_macros(7) */ + #include stdlib.h +#include stdio.h #include errno.h #include stdbool.h #include dirsrv/slapi-plugin.h @@ -47,10 +50,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
Re: [Freeipa-devel] [PATCH 0161] ipa-range-check: Fix memory leaks when freeing range object
On Tue, Apr 01, 2014 at 10:05:39AM +0200, Tomas Babej wrote: Yes, that was the intention. Mistake on my part, I'll send updated patches. Updated patch attached. Ack based on reading the code and documentation for slapi_ch_free_string. -- Jan Pazdziora Principal Software Engineer, Identity Management Engineering, Red Hat ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH 0162] ipa-pwd-extop: Fix memory leak in ipapwd_pre_bind
Hi, We need to free the entry before returning from the function. https://fedorahosted.org/freeipa/ticket/4295 -- Tomas Babej Associate Software Engineer | Red Hat | Identity Management RHCE | Brno Site | IRC: tbabej | freeipa.org From 5149ce52f583ef234bde5e8b386567c377369e41 Mon Sep 17 00:00:00 2001 From: Tomas Babej tba...@redhat.com Date: Tue, 1 Apr 2014 12:48:57 +0200 Subject: [PATCH] ipa-pwd-extop: Fix memory leak in ipapwd_pre_bind We need to free the entry before returning from the function. https://fedorahosted.org/freeipa/ticket/4295 --- daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c | 1 + 1 file changed, 1 insertion(+) diff --git a/daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c b/daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c index c6e7509bb6f84e1996393ec8fdd5165ab5273095..def312ca8ac1966af9ac2c1e41a4685cbfed3899 100644 --- a/daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c +++ b/daemons/ipa-slapi-plugins/ipa-pwd-extop/prepost.c @@ -1414,6 +1414,7 @@ static int ipapwd_pre_bind(Slapi_PBlock *pb) /* Try to do OTP first. */ syncreq = sync_request_present(pb); if (!syncreq !ipapwd_pre_bind_otp(dn, entry, credentials)) { +slapi_entry_free(entry); slapi_send_ldap_result(pb, LDAP_INVALID_CREDENTIALS, NULL, NULL, 0, NULL); return 1; -- 1.8.5.3 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Read access to container entries
On 03/31/2014 06:01 PM, Simo Sorce wrote: On Mon, 2014-03-31 at 15:39 +0200, Martin Kosek wrote: On 03/31/2014 02:53 PM, Simo Sorce wrote: On Mon, 2014-03-31 at 10:41 +0200, Ludwig Krispenz wrote: ... 3) Add a special attribute to mark public containers, and add an ACI with a filter on that. Something like objectClass=ipaPublicContainer would do. there is one more option 4) add an allow aci for cn=accounts,$S and a deny aci for cn=*,cn=accounts,$S or uid=*,cn=accounts,$S We want to get rid of deny ACIs if at all possible. In general I think we should implement 1), there will be other scenarios where it could be useful. If something is needed imemdiately I would also prefer 3) I wonder, can we have an objectclass that defines no attributes ? Or do we always need to have a MAY at least ? This particular objectclass could have just one MUST attribute - cn. Similarly to what nsContainer has. Anyway I agree that the simplest solution would be to have an objectclass to filter on. But I see 2 options. 1. objectClass=ipaPublicContainer 2. objectClass=ipaPrivateContainer The problem with the second is adding a (!(objectclass=ipaPrivateContainer)) everywhere ... I already elaborated on that topic later in this thread, please check it. It also includes an attached list of container we already have. IMO most of containers we have will be public, rather than private as LDAP nsContainer's cn attribute is semantically not meant to contain secrets we want to hide. So instead of adding 61 ipaPublicContainer everywhere I would just allow reading nsContainers (cn+objectclass) anonymously + have ipaPrivateContainer available in case we need it (I am not aware of any such case though). Yeah sorry, I replied in order. I agree with your proposal of allowing (objectclass=nsContainer) and a targetfilter that simply excludes the cn=etc subtree. Simo. Ok. I just wonder if we really need the ipaPrivateContainer ACI exception. We may want to wait with such objectclass unless it is really needed. For now, it did not seem to me that there is any entry where it is needed. Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH 0162] ipa-pwd-extop: Fix memory leak in ipapwd_pre_bind
On Tue, 01 Apr 2014, Tomas Babej wrote: Hi, We need to free the entry before returning from the function. https://fedorahosted.org/freeipa/ticket/4295 ACK. -- / Alexander Bokovoy ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] Read access to container entries
On Tue, 2014-04-01 at 13:32 +0200, Martin Kosek wrote: On 03/31/2014 06:01 PM, Simo Sorce wrote: On Mon, 2014-03-31 at 15:39 +0200, Martin Kosek wrote: On 03/31/2014 02:53 PM, Simo Sorce wrote: On Mon, 2014-03-31 at 10:41 +0200, Ludwig Krispenz wrote: ... 3) Add a special attribute to mark public containers, and add an ACI with a filter on that. Something like objectClass=ipaPublicContainer would do. there is one more option 4) add an allow aci for cn=accounts,$S and a deny aci for cn=*,cn=accounts,$S or uid=*,cn=accounts,$S We want to get rid of deny ACIs if at all possible. In general I think we should implement 1), there will be other scenarios where it could be useful. If something is needed imemdiately I would also prefer 3) I wonder, can we have an objectclass that defines no attributes ? Or do we always need to have a MAY at least ? This particular objectclass could have just one MUST attribute - cn. Similarly to what nsContainer has. Anyway I agree that the simplest solution would be to have an objectclass to filter on. But I see 2 options. 1. objectClass=ipaPublicContainer 2. objectClass=ipaPrivateContainer The problem with the second is adding a (!(objectclass=ipaPrivateContainer)) everywhere ... I already elaborated on that topic later in this thread, please check it. It also includes an attached list of container we already have. IMO most of containers we have will be public, rather than private as LDAP nsContainer's cn attribute is semantically not meant to contain secrets we want to hide. So instead of adding 61 ipaPublicContainer everywhere I would just allow reading nsContainers (cn+objectclass) anonymously + have ipaPrivateContainer available in case we need it (I am not aware of any such case though). Yeah sorry, I replied in order. I agree with your proposal of allowing (objectclass=nsContainer) and a targetfilter that simply excludes the cn=etc subtree. Simo. Ok. I just wonder if we really need the ipaPrivateContainer ACI exception. We may want to wait with such objectclass unless it is really needed. For now, it did not seem to me that there is any entry where it is needed. I would hold on as well. Simo. -- Simo Sorce * Red Hat, Inc * New York ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH] 468 Make ipa-client-automount backwards compatible
ipa-client-automount calls automountlocation-show command during the process. Unfortunately, FreeIPA commands are forward compatible only and thus fail the installer. Similarly to ipa-client-install, call XML-RPC interface directly with version fixed to 2.0 (command was already available at that version) to fix the failure. https://fedorahosted.org/freeipa/ticket/4290 -- Martin Kosek mko...@redhat.com Supervisor, Software Engineering - Identity Management Team Red Hat Inc. From cebfd91869bdc22fa8f72c4e47d32cac73487e45 Mon Sep 17 00:00:00 2001 From: Martin Kosek mko...@redhat.com Date: Tue, 1 Apr 2014 16:23:14 +0200 Subject: [PATCH] Make ipa-client-automount backwards compatible ipa-client-automount calls automountlocation-show command during the process. Unfortunately, FreeIPA commands are forward compatible only and thus fail the installer. Similarly to ipa-client-install, call XML-RPC interface directly with version fixed to 2.0 (command was already available at that version) to fix the failure. https://fedorahosted.org/freeipa/ticket/4290 --- ipa-client/ipa-install/ipa-client-automount | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/ipa-client/ipa-install/ipa-client-automount b/ipa-client/ipa-install/ipa-client-automount index 62531bfe1d923b1705aed1187da6766b54c90a0c..77829b927e8c1772598d1a4e590c3f99977aa8eb 100755 --- a/ipa-client/ipa-install/ipa-client-automount +++ b/ipa-client/ipa-install/ipa-client-automount @@ -440,7 +440,12 @@ def main(): except errors.KerberosError, e: sys.exit('Cannot connect to the server due to ' + str(e)) try: -api.Command['automountlocation_show'](unicode(options.location)) +# Use the RPC directly so older servers are supported +result = api.Backend.xmlclient.forward( +'automountlocation_show', +unicode(options.location), +version=u'2.0', +) except errors.VersionError, e: sys.exit('This client is incompatible: ' + str(e)) except errors.NotFound: -- 1.8.5.3 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] OTP work, what's left?
On 03/28/2014 10:17 AM, Martin Kosek wrote: On 03/23/2014 10:26 PM, Alexander Bokovoy wrote: Hi! I've updated my COPR repo with current git master versions of FreeIPA and SSSD with few added patches on top that close OTP gaps (Nathaniel's patch 0038 and Jakub Hrozek's patch for password changes). With these patches we currently lack following parts of the OTP work: - OTP sync client. Still in development, patches and approach need additional review/discussion on the list - Password change in WebUI fails when OTP token exist for the user. More detailed examination is needed, I'm getting ACIError. http://copr.fedoraproject.org/coprs/abbra/freeipa-otp-unstable/ Alexander or Nathaniel, I see you progressed with the OTP development a lot, good job. Please provide a clean list of patches + information who acked what so that it can be pushed to master. Hint: OTP Patches thread is too chaotic for me to follow. Martin Hi Nathaniel, I did a quick search in the thread and it seems to me that at least following 2 patches are not merged (though appears to be ACKed): [PATCH 17/17] schema-compat: set precedence to 49 to allow OTP binds over compat tree [PATCH] freeipa.spec.in: update dependencies to 389-ds and selinux-policy Is that all that is left to be pushed from this long thread? Thanks, Martin ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
Re: [Freeipa-devel] [PATCH] 468 Make ipa-client-automount backwards compatible
Martin Kosek wrote: ipa-client-automount calls automountlocation-show command during the process. Unfortunately, FreeIPA commands are forward compatible only and thus fail the installer. Similarly to ipa-client-install, call XML-RPC interface directly with version fixed to 2.0 (command was already available at that version) to fix the failure. https://fedorahosted.org/freeipa/ticket/4290 ACK. Tested F-20 client against RHEL 6.5 server. rob ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel
[Freeipa-devel] [PATCH 0231] Fix record parsing to prevent child zone corruption
Hello, Fix record parsing to prevent child zone corruption. Child zone hosted on the same server as parent zone was corrupted by bug in update_record(). Child zone's apex was modified by update_records() instead of delegation records in the parent zone. https://fedorahosted.org/bind-dyndb-ldap/ticket/134 -- Petr^2 Spacek From 644d8e4d66107bd081dd0023f5b44d1c176861be Mon Sep 17 00:00:00 2001 From: Petr Spacek pspa...@redhat.com Date: Tue, 1 Apr 2014 18:38:35 +0200 Subject: [PATCH] Fix record parsing to prevent child zone corruption. Child zone hosted on the same server as parent zone was corrupted by bug in update_record(). Child zone's apex was modified by update_records() intead of delegation records in the parent zone. https://fedorahosted.org/bind-dyndb-ldap/ticket/134 Signed-off-by: Petr Spacek pspa...@redhat.com --- NEWS | 6 ++ src/ldap_helper.c | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/NEWS b/NEWS index d997df58dca5b77d84c0fafa2757cf49e15f7d65..e787e7f2d73e3e99d3d5c0d03b9ea92dff75b510 100644 --- a/NEWS +++ b/NEWS @@ -1,3 +1,9 @@ +4.2 + +[1] Record parsing was fixed to prevent child-zone data corruption in cases +where parent zone example.com was hosted on the same server as child zone +sub.example.com. (This bug was introduced in version 4.0.) + 4.1 [1] Fix few minor bugs in error handling found by static code analyzers. diff --git a/src/ldap_helper.c b/src/ldap_helper.c index 05951fccbc655aef20177ea4a905159141665800..678e9f8a52181a5c63c96d29da9b3e5ec3b1273d 100644 --- a/src/ldap_helper.c +++ b/src/ldap_helper.c @@ -4030,7 +4030,7 @@ update_restart: ldapdb = NULL; journal = NULL; ldapdb_rdatalist_destroy(mctx, rdatalist); - CHECK(zr_get_zone_dbs(inst-zone_register, name, ldapdb, rbtdb)); + CHECK(zr_get_zone_dbs(inst-zone_register, origin, ldapdb, rbtdb)); CHECK(dns_db_newversion(ldapdb, version)); CHECK(dns_db_findnode(rbtdb, name, ISC_TRUE, node)); -- 1.9.0 ___ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel