On Fri, Oct 25, 2013 at 12:58:23PM +0200, Pavel Březina wrote:
On 10/25/2013 10:55 AM, Pavel Březina wrote:
>On 10/24/2013 08:40 PM, Jakub Hrozek wrote:
>>On Wed, Oct 02, 2013 at 04:13:32PM +0200, Pavel Březina wrote:
>>>On 10/01/2013 09:54 PM, Jakub Hrozek wrote:
>>>>On Tue, Sep 24, 2013 at 03:17:47PM +0200, Pavel Březina wrote:
>>>>>On 09/24/2013 01:32 PM, Jakub Hrozek wrote:
>>>>>>On Wed, Sep 11, 2013 at 02:40:14PM +0200, Pavel Březina wrote:
>>>>>>>https://fedorahosted.org/sssd/ticket/2064
>>>>>>>
>>>>>>>These patch set depends on: [PATCH] ad: store group in
correct
>>>>>>>tree on initgroups via tokenGroups
>>>>>>>
>>>>>>>You can also pull it with all dependencies from my
repository:
>>>>>>>fedorapeople.org:public_git/sssd.git #ad-groups
>>>>>>>
>>>>>>>The fundamental changes in this patch set are: - lookup
groups
>>>>>>>in global catalog - pick up member domain from its
originalDN
>>>>>>
>>>>>>>From 0273d17f24eac7b60dfc0515a9e3b97ad16d1199 Mon Sep 17
>>>>>>>00:00:00 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?=
>>>>>>><pbrezina(a)redhat.com> Date: Mon, 9 Sep 2013 15:52:03
+0200
>>>>>>>Subject: [PATCH 1/9] ad: shortcut if possible during get
object
>>>>>>>by ID or SID
>>>>>>>
>>>>>>>When getByID or getBySID comes from responder, the request
>>>>>>>doesn't necessarily have to contain correct domain,
since
>>>>>>>responder iterates over all domains until it finds a match.
>>>>>>>
>>>>>>>Every domain has its own ID range, so we can simply shortcut
>>>>>>>if domain does not match and avoid LDAP round trip.
Responder
>>>>>>>will continue with next domain until it finds the correct
one.
>>>>>>
>>>>>>This patch seems OK to me, but I'd like a second look from
>>>>>>someone who understands the ranges better (which is probably
>>>>>>Sumit)
>>>>>>
>>>>>>>From f74d4637980438032649dfbf079fa6c839862586 Mon Sep 17
>>>>>>>00:00:00 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?=
>>>>>>><pbrezina(a)redhat.com> Date: Tue, 10 Sep 2013 10:40:06
+0200
>>>>>>>Subject: [PATCH 2/9] ad: simplify get_conn_list()
>>>>>>>
>>>>>>>It was originally design to return list of connection
objects,
>>>>>>>it really always work with only one connection.
>>>>>>
>>>>>>I'd like to review this patch and the following along with
my
>>>>>>patches to look up POSIX IDs in GC, they touch the same code.
>>>>>>
>>>>>>>From ad5dc9e7557ef605fc5d7fc759e5cb6c2f9a148c Mon Sep 17
>>>>>>>00:00:00 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?=
>>>>>>><pbrezina(a)redhat.com> Date: Tue, 10 Sep 2013 14:45:50
+0200
>>>>>>>Subject: [PATCH 4/9] sdap_domain_add(): fix possible memory
>>>>>>>leak
>>>>>>
>>>>>>ACK.
>>>>>>
>>>>>>>From 9f2c212e01700289d70002c8c39b732ca6c11cee Mon Sep 17
>>>>>>>00:00:00 2001 From: =?UTF-8?q?Pavel=20B=C5=99ezina?=
>>>>>>><pbrezina(a)redhat.com> Date: Tue, 10 Sep 2013 14:45:52
+0200
>>>>>>>Subject: [PATCH 5/9] sdap: store base dn in sdap_domain
>>>>>>>
>>>>>>>Groups may contain members from different domains.
Remembering
>>>>>>>base dn in domain object gives us the ability to simply
lookup
>>>>>>>correct domain by comparing object dn with domain base dn.
>>>>>>>
>>>>>>>Resolves:
https://fedorahosted.org/sssd/ticket/2064
>>>>>>
>>>>>>I haven't tested these patches yet.
>>>>>
>>>>>I'm sending rebased version of my patches.
>>>>>
>>>>>[PATCH 4/9] sdap_domain_add(): fix possible memory leak was removed
>>>>>from the patch set since recent Sumit's patch removed the code I
>>>>>fixed :-)
>>>>
>>>>Hi,
>>>>
>>>>can you check if patches #6 and #7 still apply after the recent
>>>>changes in 1.11 ? We actually do use the LDAP fallback now..
>>>
>>>They won't apply, but I think it is quite all right to just skip them.
>>>The purpose of these patches was to always contact GC for get_group and
>>>initgroups.
>>>
>>>We always contact GC first at the moment and having LDAP as fallback is
>>>fine for groups. If there will be a member from different domain, we
>>>will just fail - but if there won't be foreign member it will work.
>>
>>Hi,
>>
>>In general the patches look good to me but I think the first two can be
>>simplified.
>>
>>Instead of magically trying to find the base DN from the entry's
>>original DN, I think we can simplify it to:
>>
>>struct sdap_domain *
>>sdap_domain_get_by_dn(struct sdap_options *opts,
>> const char *dn)
>>{
>> struct sdap_domain *sditer = NULL;
>>
>> DLIST_FOR_EACH(sditer, opts->sdom) {
>> if (sss_ldap_dn_in_search_bases(tmp_ctx, dn,
>>sditer->search_bases, NULL) ||
>> sss_ldap_dn_in_search_bases(tmp_ctx, dn,
>>sditer->user_search_bases, NULL) ||
>> sss_ldap_dn_in_search_bases(tmp_ctx, dn,
>>sditer->group_search_bases, NULL)) {
>> return sditer;
>> }
>> }
>>
>> return NULL;
>>}
>>
>>Then you won't need to store the base DN, no need for the magic with
>>strstr
>>and the code will work even for custom search bases.
>
>Hi,
>new patches are attached. I just added all of the search bases.
And here's one more iteration. While working on token groups patches
I found a bug in the first patch. I tried to shortcut using id
mapping even if id mapping is disabled. This made the posix groups
unresolvable since id mapping returned an error when trying to map
GID to SID.
From d2224259b331c2e6887ce7b8816f1949342e539a Mon Sep 17 00:00:00
2001
From: =?UTF-8?q?Pavel=20B=C5=99ezina?= <pbrezina(a)redhat.com>
Date: Mon, 9 Sep 2013 15:52:03 +0200
Subject: [PATCH 1/5] ad: shortcut if possible during get object by ID or SID
When getByID or getBySID comes from responder, the request doesn't
necessarily have to contain correct domain, since responder iterates
over all domains until it finds a match.
Every domain has its own ID range, so we can simply shortcut if
domain does not match and avoid LDAP round trip. Responder will
continue with next domain until it finds the correct one.
---
src/providers/ad/ad_id.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 46 insertions(+)
diff --git a/src/providers/ad/ad_id.c b/src/providers/ad/ad_id.c
index f09b9c6fe239b9b602f53e8afce641a4d07be6e9..6ac10bba82ba71fde2a963f3ade23a49a20d705e
100644
--- a/src/providers/ad/ad_id.c
+++ b/src/providers/ad/ad_id.c
@@ -20,10 +20,12 @@
along with this program. If not, see <
http://www.gnu.org/licenses/>.
*/
#include "util/util.h"
+#include "util/strtonum.h"
#include "providers/ad/ad_common.h"
#include "providers/ad/ad_id.h"
#include "providers/ad/ad_domain_info.h"
#include "providers/ldap/sdap_async_enum.h"
+#include "providers/ldap/sdap_idmap.h"
struct ad_handle_acct_info_state {
struct be_req *breq;
@@ -224,17 +226,61 @@ ad_account_info_handler(struct be_req *be_req)
struct sss_domain_info *dom;
struct sdap_domain *sdom;
struct sdap_id_conn_ctx **clist;
+ struct sdap_idmap_ctx *idmap_ctx;
+ enum idmap_error_code map_err;
+ uint32_t id;
+ char *sid = NULL;
errno_t ret;
ad_ctx = talloc_get_type(be_ctx->bet_info[BET_ID].pvt_bet_data,
struct ad_id_ctx);
ar = talloc_get_type(be_req_get_data(be_req), struct be_acct_req);
sdap_id_ctx = ad_ctx->sdap_id_ctx;
+ idmap_ctx = sdap_id_ctx->opts->idmap_ctx;
if (be_is_offline(be_ctx)) {
return be_req_terminate(be_req, DP_ERR_OFFLINE, EAGAIN, "Offline");
}
+ /* Try to shortcut if this is ID or SID search and it belongs to
+ * other domain range than is in ar->domain. */
+ if (dp_opt_get_bool(idmap_ctx->id_ctx->opts->basic, SDAP_ID_MAPPING)) {
Please use sdap_idmap_domain_has_algorithmic_mapping() instead. In the
case of trusted AD domains SDAP_ID_MAPPING is not set and the mapping
type depends on the type to the related IPA idranges.
bye,
Sumit
+ if (ar->filter_type == BE_FILTER_IDNUM
+ || ar->filter_type == BE_FILTER_SECID) {
+ if (ar->filter_type == BE_FILTER_IDNUM) {
+ id = strtouint32(ar->filter_value, NULL, 10);
+ if (errno != EOK) {
+ ret = EINVAL;
+ goto fail;
+ }
+