[Freeipa-devel] [PATCH] 584 migration: fix import of wsgiref.util

2014-04-01 Thread Petr Vobornik

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

2014-04-01 Thread Jan Pazdziora
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

2014-04-01 Thread Jan Pazdziora
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

2014-04-01 Thread Alexander Bokovoy

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

2014-04-01 Thread Tomas Babej

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

2014-04-01 Thread Jan Pazdziora
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

2014-04-01 Thread Martin Kosek
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

2014-04-01 Thread Tomas Babej

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

2014-04-01 Thread Tomas Babej
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

2014-04-01 Thread Alexander Bokovoy

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

2014-04-01 Thread Tomas Babej

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

2014-04-01 Thread Jan Pazdziora
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

2014-04-01 Thread Tomas Babej
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

2014-04-01 Thread Martin Kosek
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

2014-04-01 Thread Alexander Bokovoy

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

2014-04-01 Thread Simo Sorce
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

2014-04-01 Thread Martin Kosek
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?

2014-04-01 Thread Martin Kosek
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

2014-04-01 Thread Rob Crittenden

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

2014-04-01 Thread Petr Spacek

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