On (10/08/16 20:59), Michal Židek wrote:
On 08/10/2016 08:36 PM, Lukas Slebodnik wrote:
> On (10/08/16 17:41), Michal Židek wrote:
> > Hi,
> >
> > see the attached patch.
> >
> > I modified the detection of duplicates when
> > extending the maps (sysdb_attr:ldap_attr).
> >
> > When we try to add entry to the map
> > that already exists in the map, then
> > without this patch we will fail.
> >
> > With this patch, we only fail if the
> > newly added extension would redefine
> > already existing entry in the map.
> >
> > Otherwise it is just skipped without
> > a failure (we just skip adding what
> > is already there).
> >
> > I created simple CI test for this (first
> > patch).
> >
> > Michal
>
> > From 5a2ef2a98e483701603a42bc50e9a11d8ee651ff Mon Sep 17 00:00:00 2001
> > From: =?UTF-8?q?Michal=20=C5=BDidek?= <mzidek(a)redhat.com>
> > Date: Wed, 10 Aug 2016 15:41:34 +0200
> > Subject: [PATCH 2/2] sdap: Skip exact duplicates when extending maps
> >
> > When extending map with entry that already
> > exists in the map in the exacty same form,
> > then there is no need to fail.
> >
> > We should only fail if we try to
> > change purpose of already used sysdb
> > attribute.
> >
> > Resolves:
> >
https://fedorahosted.org/sssd/ticket/3120
> > ---
> > src/providers/ldap/sdap.c | 41 +++++++++++++++++++++++++++++++++++------
> > 1 file changed, 35 insertions(+), 6 deletions(-)
> >
> > diff --git a/src/providers/ldap/sdap.c b/src/providers/ldap/sdap.c
> > index 97b8f12..e1cf70f 100644
> > --- a/src/providers/ldap/sdap.c
> > +++ b/src/providers/ldap/sdap.c
> > @@ -122,19 +122,39 @@ static errno_t split_extra_attr(TALLOC_CTX *mem_ctx,
> > return EOK;
> > }
> >
> > -static bool is_sysdb_duplicate(struct sdap_attr_map *map,
> > - int num_entries,
> > - const char *sysdb_attr)
> > +/* _already_in_map is set to true if the attribute
> > + * already exists in the map and is used for the same
> > + * LDAP attribute.
> > + *
> > + * _conflicts_with_map is set to true if the attribute
> > + * already exists in map, but is used for different
> > + * LDAP attribute.
> > + * */
> > +static void check_duplicate(struct sdap_attr_map *map,
> > + int num_entries,
> > + const char *sysdb_attr,
> > + const char *ldap_attr,
> > + bool *_already_in_map,
> > + bool *_conflicts_with_map)
> > {
> This function has 3 output boolean argumets:
> It would be better to return enum instead of
> adding new parametrs.
>
> LS
Ok, attached is version with enum.
Michal
I tried to rest use-case from ticket #3120
http://www.freeipa.org/page/Web_App_Authentication/Example_setup
but sssd_be crashed
(gdb) bt
#0 0x00007fc29afb8961 in __strncasecmp_l_avx () from /lib64/libc.so.6
#1 0x00007fc29f199ea0 in sysdb_attrs_get_el_ext (attrs=attrs@entry=0x7fc2a1adc740,
name=name@entry=0x0, alloc=alloc@entry=true, el=el@entry=0x7ffd0d466810) at
src/db/sysdb.c:290
#2 0x00007fc29f199fad in sysdb_attrs_get_el (attrs=attrs@entry=0x7fc2a1adc740,
name=name@entry=0x0, el=el@entry=0x7ffd0d466810) at src/db/sysdb.c:323
#3 0x00007fc28fe41400 in sdap_attrs_add_ldap_attr
(ldap_attrs=ldap_attrs@entry=0x7fc2a1adc740, attr_name=0x0, attr_desc=attr_desc@entry=0x0,
multivalued=multivalued@entry=true,
name=<optimized out>, attrs=attrs@entry=0x7fc2a1ac4860) at
src/providers/ldap/sdap_utils.c:40
#4 0x00007fc28fe1a2c7 in sdap_save_user (memctx=memctx@entry=0x7fc2a1adf600,
opts=0x7fc2a1a7eae0, dom=0x7fc2a1a54ae0, attrs=<optimized out>,
_usn_value=_usn_value@entry=0x0,
now=now@entry=0) at src/providers/ldap/sdap_async_users.c:482
#5 0x00007fc28fe2b667 in sdap_get_initgr_user (subreq=0x0) at
src/providers/ldap/sdap_async_initgroups.c:2961
#6 0x00007fc28fe13d99 in generic_ext_search_handler (subreq=0x0, opts=<optimized
out>) at src/providers/ldap/sdap_async.c:1688
#7 0x00007fc28fe16407 in sdap_get_generic_op_finished (op=<optimized out>,
reply=<optimized out>, error=<optimized out>, pvt=<optimized out>) at
src/providers/ldap/sdap_async.c:1578
#8 0x00007fc28fe14ded in sdap_process_message (ev=<optimized out>, sh=<optimized
out>, msg=0x7fc2a1aba9f0) at src/providers/ldap/sdap_async.c:353
#9 sdap_process_result (ev=<optimized out>, pvt=<optimized out>) at
src/providers/ldap/sdap_async.c:197
#10 0x00007fc29b85fb4f in tevent_common_loop_timer_delay () from /lib64/libtevent.so.0
#11 0x00007fc29b860b5a in epoll_event_loop_once () from /lib64/libtevent.so.0
#12 0x00007fc29b85f257 in std_event_context_init () from /lib64/libtevent.so.0
#13 0x00007fc29b85b40d in _tevent_loop_until () from /lib64/libtevent.so.0
#14 0x00007fc2a1a4bd20 in ?? ()
#15 0x00007fc29f1e7c47 in ?? () from /usr/lib64/sssd/libsss_util.so
#16 0x00007fc29b85f1f7 in std_event_loop_once () from /lib64/libtevent.so.0
#17 0x00007fc29f1cb7f3 in server_loop (main_ctx=0x7fc2a1a4d080) at src/util/server.c:702
#18 0x00007fc29fa45952 in main (argc=8, argv=<optimized out>) at
src/providers/data_provider_be.c:587
it crashed because one agruments from strcasecmp was NULL
(dereference of NULL pointer)
I guess that we hit the last value in user_map (zeroed structure)
In other words, opts->user_map_cnt does not match reallity.
(gdb) l 482
477 }
478 }
479 }
480
481 for (i = SDAP_FIRST_EXTRA_USER_AT; i < opts->user_map_cnt; i++) {
482 ret = sdap_attrs_add_list(attrs, opts->user_map[i].sys_name,
483 NULL, user_name, user_attrs);
484 if (ret) {
485 goto done;
486 }
NACK
LS