[PATCH] ipa_server_mode: write capaths to krb5 include file

[Sumit Bose]

Hi,

the first two attached patches should fix
https://fedorahosted.org/sssd/ticket/2093 and make
https://fedorahosted.org/sssd/ticket/2080 invalid. The third fixes wrong
return code in one of the functions I touched with in the other patches.

bye,
Sumit

[Jakub Hrozek]

On Thu, Sep 26, 2013 at 11:22:06AM +0200, Sumit Bose wrote:
> Hi,
>
> the first two attached patches should fix
> https://fedorahosted.org/sssd/ticket/2093 and make
> https://fedorahosted.org/sssd/ticket/2080 invalid. The third fixes wrong
> return code in one of the functions I touched with in the other patches.
>
> bye,
> Sumit
> if (!ldb_dn_validate(dn)) {
>    DEBUG(SSSDBG_OP_FAILURE, ("Original DN [%s] is not a valid DN.\n",
>                              orig_dn));
>    ret = EINVAL;
>    goto done;
> }
> if (ldb_dn_get_comp_num(dn) < 5) {
>    /* we are only interested in the member domain objects */
>    ret = EOK;
>    goto done;
> }
> val = ldb_dn_get_component_val(dn, 3);
> if (strncasecmp("trusts", (const char *) val->data, val->length) != 0) {
>    DEBUG(SSSDBG_TRACE_FUNC,
>          ("4th component is not 'trust', nothing to do.\n"));
>    ret = EOK;
>    goto done;
> }
> val = ldb_dn_get_component_val(dn, 2);
> if (strncasecmp("ad", (const char *) val->data, val->length) != 0) {
>    DEBUG(SSSDBG_TRACE_FUNC,
>          ("3rd component is not 'ad', nothing to do.\n"));
>    ret = EOK;
>    goto done;
> }
> val = ldb_dn_get_component_val(dn, 1);
> forest = talloc_strndup(mem_ctx, (const char *) val->data, val->length);
> if (forest == NULL) {
>    DEBUG(SSSDBG_OP_FAILURE, ("talloc_strndup failed.\n"));
>    ret = ENOMEM;
>    goto done;
> }

I think there is an off-by-one error here. In my setup, the original DN
is:

cn=AD.EXAMPLE.COM,cn=ad,cn=trusts,dc=ipatest,dc=example,dc=com

But "val = ldb_dn_get_component_val(dn, 3);" is "ipatest" in my case, so
the strcmp never matches. So I think the indexes should say "2, 1, 0".

It would also be nice to print the original DN in a DEBUG message.

[Sumit Bose]

On Thu, Sep 26, 2013 at 07:24:03PM +0200, Jakub Hrozek wrote:
> On Thu, Sep 26, 2013 at 11:22:06AM +0200, Sumit Bose wrote:
> > Hi,
> >
> > the first two attached patches should fix
> > https://fedorahosted.org/sssd/ticket/2093 and make
> > https://fedorahosted.org/sssd/ticket/2080 invalid. The third fixes wrong
> > return code in one of the functions I touched with in the other patches.
> >
> > bye,
> > Sumit
> > if (!ldb_dn_validate(dn)) {
> >    DEBUG(SSSDBG_OP_FAILURE, ("Original DN [%s] is not a valid DN.\n",
> >                              orig_dn));
> >    ret = EINVAL;
> >    goto done;
> > }
> > if (ldb_dn_get_comp_num(dn) < 5) {
> >    /* we are only interested in the member domain objects */
> >    ret = EOK;
> >    goto done;
> > }
> > val = ldb_dn_get_component_val(dn, 3);
> > if (strncasecmp("trusts", (const char *) val->data, val->length) != 0) {
> >    DEBUG(SSSDBG_TRACE_FUNC,
> >          ("4th component is not 'trust', nothing to do.\n"));
> >    ret = EOK;
> >    goto done;
> > }
> > val = ldb_dn_get_component_val(dn, 2);
> > if (strncasecmp("ad", (const char *) val->data, val->length) != 0) {
> >    DEBUG(SSSDBG_TRACE_FUNC,
> >          ("3rd component is not 'ad', nothing to do.\n"));
> >    ret = EOK;
> >    goto done;
> > }
> > val = ldb_dn_get_component_val(dn, 1);
> > forest = talloc_strndup(mem_ctx, (const char *) val->data, val->length);
> > if (forest == NULL) {
> >    DEBUG(SSSDBG_OP_FAILURE, ("talloc_strndup failed.\n"));
> >    ret = ENOMEM;
> >    goto done;
> > }
>
> I think there is an off-by-one error here. In my setup, the original DN
> is:
>
> cn=AD.EXAMPLE.COM,cn=ad,cn=trusts,dc=ipatest,dc=example,dc=com
>
> But "val = ldb_dn_get_component_val(dn, 3);" is "ipatest" in my case, so
> the strcmp never matches. So I think the indexes should say "2, 1, 0".
>
> It would also be nice to print the original DN in a DEBUG message.

ah, sorry, I should have said that Alexander's latest FreeIPA patches
are needed here. They will store the member domains below the forest
root object, e.g.
cn=member.dom,cn=AD.EXAMPLE.COM,cn=ad,cn=trusts,dc=ipatest,dc=example,dc=com
For those domains the patch will set the forest name which in this case
is AD.EXAMPLE.COM.

bye,
Sumit
> sssd-devel mailing list
> [e-mail]
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

[Jakub Hrozek]

On Thu, Sep 26, 2013 at 09:45:12PM +0200, Sumit Bose wrote:
> On Thu, Sep 26, 2013 at 07:24:03PM +0200, Jakub Hrozek wrote:
> > On Thu, Sep 26, 2013 at 11:22:06AM +0200, Sumit Bose wrote:
> > > Hi,
> > >
> > > the first two attached patches should fix
> > > https://fedorahosted.org/sssd/ticket/2093 and make
> > > https://fedorahosted.org/sssd/ticket/2080 invalid. The third fixes wrong
> > > return code in one of the functions I touched with in the other patches.
> > >
> > > bye,
> > > Sumit
> > > if (!ldb_dn_validate(dn)) {
> > >    DEBUG(SSSDBG_OP_FAILURE, ("Original DN [%s] is not a valid DN.\n",
> > >                              orig_dn));
> > >    ret = EINVAL;
> > >    goto done;
> > > }
> > > if (ldb_dn_get_comp_num(dn) < 5) {
> > >    /* we are only interested in the member domain objects */
> > >    ret = EOK;
> > >    goto done;
> > > }
> > > val = ldb_dn_get_component_val(dn, 3);
> > > if (strncasecmp("trusts", (const char *) val->data, val->length) != 0) {
> > >    DEBUG(SSSDBG_TRACE_FUNC,
> > >          ("4th component is not 'trust', nothing to do.\n"));
> > >    ret = EOK;
> > >    goto done;
> > > }
> > > val = ldb_dn_get_component_val(dn, 2);
> > > if (strncasecmp("ad", (const char *) val->data, val->length) != 0) {
> > >    DEBUG(SSSDBG_TRACE_FUNC,
> > >          ("3rd component is not 'ad', nothing to do.\n"));
> > >    ret = EOK;
> > >    goto done;
> > > }
> > > val = ldb_dn_get_component_val(dn, 1);
> > > forest = talloc_strndup(mem_ctx, (const char *) val->data, val->length);
> > > if (forest == NULL) {
> > >    DEBUG(SSSDBG_OP_FAILURE, ("talloc_strndup failed.\n"));
> > >    ret = ENOMEM;
> > >    goto done;
> > > }
> >
> > I think there is an off-by-one error here. In my setup, the original DN
> > is:
> >
> > cn=AD.EXAMPLE.COM,cn=ad,cn=trusts,dc=ipatest,dc=example,dc=com
> >
> > But "val = ldb_dn_get_component_val(dn, 3);" is "ipatest" in my case, so
> > the strcmp never matches. So I think the indexes should say "2, 1, 0".
> >
> > It would also be nice to print the original DN in a DEBUG message.
>
> ah, sorry, I should have said that Alexander's latest FreeIPA patches
> are needed here. They will store the member domains below the forest
> root object, e.g.
> cn=member.dom,cn=AD.EXAMPLE.COM,cn=ad,cn=trusts,dc=ipatest,dc=example,dc=com
> For those domains the patch will set the forest name which in this case
> is AD.EXAMPLE.COM.
>
> bye,
> Sumit

Ah, then the code is fine.

ACK.

But is should be noted that I only tested the file is generated and
contains the [capaths] section, not any actual functionality it brings.

[Jakub Hrozek]

On Thu, Sep 26, 2013 at 10:11:21PM +0200, Jakub Hrozek wrote:
> On Thu, Sep 26, 2013 at 09:45:12PM +0200, Sumit Bose wrote:
> > On Thu, Sep 26, 2013 at 07:24:03PM +0200, Jakub Hrozek wrote:
> > > On Thu, Sep 26, 2013 at 11:22:06AM +0200, Sumit Bose wrote:
> > > > Hi,
> > > >
> > > > the first two attached patches should fix
> > > > https://fedorahosted.org/sssd/ticket/2093 and make
> > > > https://fedorahosted.org/sssd/ticket/2080 invalid. The third fixes wrong
> > > > return code in one of the functions I touched with in the other patches.
> > > >
> > > > bye,
> > > > Sumit
> > > > if (!ldb_dn_validate(dn)) {
> > > >    DEBUG(SSSDBG_OP_FAILURE, ("Original DN [%s] is not a valid DN.\n",
> > > >                              orig_dn));
> > > >    ret = EINVAL;
> > > >    goto done;
> > > > }
> > > > if (ldb_dn_get_comp_num(dn) < 5) {
> > > >    /* we are only interested in the member domain objects */
> > > >    ret = EOK;
> > > >    goto done;
> > > > }
> > > > val = ldb_dn_get_component_val(dn, 3);
> > > > if (strncasecmp("trusts", (const char *) val->data, val->length) != 0) {
> > > >    DEBUG(SSSDBG_TRACE_FUNC,
> > > >          ("4th component is not 'trust', nothing to do.\n"));
> > > >    ret = EOK;
> > > >    goto done;
> > > > }
> > > > val = ldb_dn_get_component_val(dn, 2);
> > > > if (strncasecmp("ad", (const char *) val->data, val->length) != 0) {
> > > >    DEBUG(SSSDBG_TRACE_FUNC,
> > > >          ("3rd component is not 'ad', nothing to do.\n"));
> > > >    ret = EOK;
> > > >    goto done;
> > > > }
> > > > val = ldb_dn_get_component_val(dn, 1);
> > > > forest = talloc_strndup(mem_ctx, (const char *) val->data, val->length);
> > > > if (forest == NULL) {
> > > >    DEBUG(SSSDBG_OP_FAILURE, ("talloc_strndup failed.\n"));
> > > >    ret = ENOMEM;
> > > >    goto done;
> > > > }
> > >
> > > I think there is an off-by-one error here. In my setup, the original DN
> > > is:
> > >
> > > cn=AD.EXAMPLE.COM,cn=ad,cn=trusts,dc=ipatest,dc=example,dc=com
> > >
> > > But "val = ldb_dn_get_component_val(dn, 3);" is "ipatest" in my case, so
> > > the strcmp never matches. So I think the indexes should say "2, 1, 0".
> > >
> > > It would also be nice to print the original DN in a DEBUG message.
> >
> > ah, sorry, I should have said that Alexander's latest FreeIPA patches
> > are needed here. They will store the member domains below the forest
> > root object, e.g.
> > cn=member.dom,cn=AD.EXAMPLE.COM,cn=ad,cn=trusts,dc=ipatest,dc=example,dc=com
> > For those domains the patch will set the forest name which in this case
> > is AD.EXAMPLE.COM.
> >
> > bye,
> > Sumit
>
> Ah, then the code is fine.
>
> ACK.
>
> But is should be noted that I only tested the file is generated and
> contains the [capaths] section, not any actual functionality it brings.

Actually it seems that the sysdb tests are not happy:
(gdb) r
Starting program: /dev/shm/sssd/.libs/sysdb-tests 
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/usr/lib64/libthread_db.so.1".
Warning: LDB_MODULES_PATH is not set, will use LDB plugins installed in system paths.
Running suite(s): sysdb
ldb: unable to dlopen /usr/lib64/ldb/modules/ldb/memberof.la : /usr/lib64/ldb/modules/ldb/memberof.la: invalid ELF header

Program received signal SIGSEGV, Segmentation fault.
__strcasecmp_l_avx () at ../sysdeps/x86_64/multiarch/strcmp-sse42.S:164
164             movdqu  (%rdi), %xmm1
(gdb) bt
#0  __strcasecmp_l_avx () at ../sysdeps/x86_64/multiarch/strcmp-sse42.S:164
#1  0x00007ffff7facc16 in sysdb_update_subdomains (domain=0x57ec70) at /home/remote/jhrozek/devel/sssd/src/db/sysdb_subdomains.c:164
#2  0x0000000000417752 in test_sysdb_subdomain_create (_i=0) at /home/remote/jhrozek/devel/sssd/src/tests/sysdb-tests.c:4558
#3  0x00007ffff7bd7d12 in tcase_run_tfun_nofork (sr=sr@entry=0x42b180, tc=tc@entry=0x42ac00, i=i@entry=0, tfun=0x42ad40, tfun=0x42ad40) at check_run.c:332
#4  0x00007ffff7bd7f6e in srunner_iterate_tcase_tfuns (tc=0x42ac00, sr=0x42b180) at check_run.c:192
#5  srunner_run_tcase (tc=0x42ac00, sr=0x42b180) at check_run.c:318
#6  srunner_iterate_suites (print_mode=<optimized out>, tcname=0x0, sname=0x0, sr=0x42b180) at check_run.c:161
#7  srunner_run (sr=0x42b180, sname=0x0, tcname=0x0, print_mode=<optimized out>) at check_run.c:596
#8  0x000000000041a42b in main (argc=1, argv=0x7fffffffdc48) at /home/remote/jhrozek/devel/sssd/src/tests/sysdb-tests.c:5369

[Sumit Bose]

On Thu, Sep 26, 2013 at 10:27:07PM +0200, Jakub Hrozek wrote:
> On Thu, Sep 26, 2013 at 10:11:21PM +0200, Jakub Hrozek wrote:
> > On Thu, Sep 26, 2013 at 09:45:12PM +0200, Sumit Bose wrote:
> > > On Thu, Sep 26, 2013 at 07:24:03PM +0200, Jakub Hrozek wrote:
> > > > On Thu, Sep 26, 2013 at 11:22:06AM +0200, Sumit Bose wrote:
> > > > > Hi,
> > > > >
> > > > > the first two attached patches should fix
> > > > > https://fedorahosted.org/sssd/ticket/2093 and make
> > > > > https://fedorahosted.org/sssd/ticket/2080 invalid. The third fixes wrong
> > > > > return code in one of the functions I touched with in the other patches.
> > > > >
> > > > > bye,
> > > > > Sumit
> > > > > if (!ldb_dn_validate(dn)) {
> > > > >    DEBUG(SSSDBG_OP_FAILURE, ("Original DN [%s] is not a valid DN.\n",
> > > > >                              orig_dn));
> > > > >    ret = EINVAL;
> > > > >    goto done;
> > > > > }
> > > > > if (ldb_dn_get_comp_num(dn) < 5) {
> > > > >    /* we are only interested in the member domain objects */
> > > > >    ret = EOK;
> > > > >    goto done;
> > > > > }
> > > > > val = ldb_dn_get_component_val(dn, 3);
> > > > > if (strncasecmp("trusts", (const char *) val->data, val->length) != 0) {
> > > > >    DEBUG(SSSDBG_TRACE_FUNC,
> > > > >          ("4th component is not 'trust', nothing to do.\n"));
> > > > >    ret = EOK;
> > > > >    goto done;
> > > > > }
> > > > > val = ldb_dn_get_component_val(dn, 2);
> > > > > if (strncasecmp("ad", (const char *) val->data, val->length) != 0) {
> > > > >    DEBUG(SSSDBG_TRACE_FUNC,
> > > > >          ("3rd component is not 'ad', nothing to do.\n"));
> > > > >    ret = EOK;
> > > > >    goto done;
> > > > > }
> > > > > val = ldb_dn_get_component_val(dn, 1);
> > > > > forest = talloc_strndup(mem_ctx, (const char *) val->data, val->length);
> > > > > if (forest == NULL) {
> > > > >    DEBUG(SSSDBG_OP_FAILURE, ("talloc_strndup failed.\n"));
> > > > >    ret = ENOMEM;
> > > > >    goto done;
> > > > > }
> > > >
> > > > I think there is an off-by-one error here. In my setup, the original DN
> > > > is:
> > > >
> > > > cn=AD.EXAMPLE.COM,cn=ad,cn=trusts,dc=ipatest,dc=example,dc=com
> > > >
> > > > But "val = ldb_dn_get_component_val(dn, 3);" is "ipatest" in my case, so
> > > > the strcmp never matches. So I think the indexes should say "2, 1, 0".
> > > >
> > > > It would also be nice to print the original DN in a DEBUG message.
> > >
> > > ah, sorry, I should have said that Alexander's latest FreeIPA patches
> > > are needed here. They will store the member domains below the forest
> > > root object, e.g.
> > > cn=member.dom,cn=AD.EXAMPLE.COM,cn=ad,cn=trusts,dc=ipatest,dc=example,dc=com
> > > For those domains the patch will set the forest name which in this case
> > > is AD.EXAMPLE.COM.
> > >
> > > bye,
> > > Sumit
> >
> > Ah, then the code is fine.
> >
> > ACK.
> >
> > But is should be noted that I only tested the file is generated and
> > contains the [capaths] section, not any actual functionality it brings.
>
> Actually it seems that the sysdb tests are not happy:
> (gdb) r
> Starting program: /dev/shm/sssd/.libs/sysdb-tests 
> [Thread debugging using libthread_db enabled]
> Using host libthread_db library "/usr/lib64/libthread_db.so.1".
> Warning: LDB_MODULES_PATH is not set, will use LDB plugins installed in system paths.
> Running suite(s): sysdb
> ldb: unable to dlopen /usr/lib64/ldb/modules/ldb/memberof.la : /usr/lib64/ldb/modules/ldb/memberof.la: invalid ELF header
>
> Program received signal SIGSEGV, Segmentation fault.
> __strcasecmp_l_avx () at ../sysdeps/x86_64/multiarch/strcmp-sse42.S:164
> 164             movdqu  (%rdi), %xmm1
> (gdb) bt
> #0  __strcasecmp_l_avx () at ../sysdeps/x86_64/multiarch/strcmp-sse42.S:164
> #1  0x00007ffff7facc16 in sysdb_update_subdomains (domain=0x57ec70) at /home/remote/jhrozek/devel/sssd/src/db/sysdb_subdomains.c:164
> #2  0x0000000000417752 in test_sysdb_subdomain_create (_i=0) at /home/remote/jhrozek/devel/sssd/src/tests/sysdb-tests.c:4558
> #3  0x00007ffff7bd7d12 in tcase_run_tfun_nofork (sr=sr@entry=0x42b180, tc=tc@entry=0x42ac00, i=i@entry=0, tfun=0x42ad40, tfun=0x42ad40) at check_run.c:332
> #4  0x00007ffff7bd7f6e in srunner_iterate_tcase_tfuns (tc=0x42ac00, sr=0x42b180) at check_run.c:192
> #5  srunner_run_tcase (tc=0x42ac00, sr=0x42b180) at check_run.c:318
> #6  srunner_iterate_suites (print_mode=<optimized out>, tcname=0x0, sname=0x0, sr=0x42b180) at check_run.c:161
> #7  srunner_run (sr=0x42b180, sname=0x0, tcname=0x0, print_mode=<optimized out>) at check_run.c:596
> #8  0x000000000041a42b in main (argc=1, argv=0x7fffffffdc48) at /home/remote/jhrozek/devel/sssd/src/tests/sysdb-tests.c:5369

sorry I haven't seen this on my 32bit VM, but I reproduce it on 64bits.

If have added a check to prevent this. As you suggested I also added a
debug message with the current original DN and a comment explaining the
IPA tree layout with respect to member domains in a forest. All changes
are in the first patch, the others are unchanged.

bye,
Sumit
> sssd-devel mailing list
> [e-mail]
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel

[Jakub Hrozek]

On Fri, Sep 27, 2013 at 10:14:23AM +0200, Sumit Bose wrote:
> On Thu, Sep 26, 2013 at 10:27:07PM +0200, Jakub Hrozek wrote:
> > On Thu, Sep 26, 2013 at 10:11:21PM +0200, Jakub Hrozek wrote:
> > > On Thu, Sep 26, 2013 at 09:45:12PM +0200, Sumit Bose wrote:
> > > > On Thu, Sep 26, 2013 at 07:24:03PM +0200, Jakub Hrozek wrote:
> > > > > On Thu, Sep 26, 2013 at 11:22:06AM +0200, Sumit Bose wrote:
> > > > > > Hi,
> > > > > >
> > > > > > the first two attached patches should fix
> > > > > > https://fedorahosted.org/sssd/ticket/2093 and make
> > > > > > https://fedorahosted.org/sssd/ticket/2080 invalid. The third fixes wrong
> > > > > > return code in one of the functions I touched with in the other patches.
> > > > > >
> > > > > > bye,
> > > > > > Sumit
> > > > > > if (!ldb_dn_validate(dn)) {
> > > > > >    DEBUG(SSSDBG_OP_FAILURE, ("Original DN [%s] is not a valid DN.\n",
> > > > > >                              orig_dn));
> > > > > >    ret = EINVAL;
> > > > > >    goto done;
> > > > > > }
> > > > > > if (ldb_dn_get_comp_num(dn) < 5) {
> > > > > >    /* we are only interested in the member domain objects */
> > > > > >    ret = EOK;
> > > > > >    goto done;
> > > > > > }
> > > > > > val = ldb_dn_get_component_val(dn, 3);
> > > > > > if (strncasecmp("trusts", (const char *) val->data, val->length) != 0) {
> > > > > >    DEBUG(SSSDBG_TRACE_FUNC,
> > > > > >          ("4th component is not 'trust', nothing to do.\n"));
> > > > > >    ret = EOK;
> > > > > >    goto done;
> > > > > > }
> > > > > > val = ldb_dn_get_component_val(dn, 2);
> > > > > > if (strncasecmp("ad", (const char *) val->data, val->length) != 0) {
> > > > > >    DEBUG(SSSDBG_TRACE_FUNC,
> > > > > >          ("3rd component is not 'ad', nothing to do.\n"));
> > > > > >    ret = EOK;
> > > > > >    goto done;
> > > > > > }
> > > > > > val = ldb_dn_get_component_val(dn, 1);
> > > > > > forest = talloc_strndup(mem_ctx, (const char *) val->data, val->length);
> > > > > > if (forest == NULL) {
> > > > > >    DEBUG(SSSDBG_OP_FAILURE, ("talloc_strndup failed.\n"));
> > > > > >    ret = ENOMEM;
> > > > > >    goto done;
> > > > > > }
> > > > >
> > > > > I think there is an off-by-one error here. In my setup, the original DN
> > > > > is:
> > > > >
> > > > > cn=AD.EXAMPLE.COM,cn=ad,cn=trusts,dc=ipatest,dc=example,dc=com
> > > > >
> > > > > But "val = ldb_dn_get_component_val(dn, 3);" is "ipatest" in my case, so
> > > > > the strcmp never matches. So I think the indexes should say "2, 1, 0".
> > > > >
> > > > > It would also be nice to print the original DN in a DEBUG message.
> > > >
> > > > ah, sorry, I should have said that Alexander's latest FreeIPA patches
> > > > are needed here. They will store the member domains below the forest
> > > > root object, e.g.
> > > > cn=member.dom,cn=AD.EXAMPLE.COM,cn=ad,cn=trusts,dc=ipatest,dc=example,dc=com
> > > > For those domains the patch will set the forest name which in this case
> > > > is AD.EXAMPLE.COM.
> > > >
> > > > bye,
> > > > Sumit
> > >
> > > Ah, then the code is fine.
> > >
> > > ACK.
> > >
> > > But is should be noted that I only tested the file is generated and
> > > contains the [capaths] section, not any actual functionality it brings.
> >
> > Actually it seems that the sysdb tests are not happy:
> > (gdb) r
> > Starting program: /dev/shm/sssd/.libs/sysdb-tests 
> > [Thread debugging using libthread_db enabled]
> > Using host libthread_db library "/usr/lib64/libthread_db.so.1".
> > Warning: LDB_MODULES_PATH is not set, will use LDB plugins installed in system paths.
> > Running suite(s): sysdb
> > ldb: unable to dlopen /usr/lib64/ldb/modules/ldb/memberof.la : /usr/lib64/ldb/modules/ldb/memberof.la: invalid ELF header
> >
> > Program received signal SIGSEGV, Segmentation fault.
> > __strcasecmp_l_avx () at ../sysdeps/x86_64/multiarch/strcmp-sse42.S:164
> > 164             movdqu  (%rdi), %xmm1
> > (gdb) bt
> > #0  __strcasecmp_l_avx () at ../sysdeps/x86_64/multiarch/strcmp-sse42.S:164
> > #1  0x00007ffff7facc16 in sysdb_update_subdomains (domain=0x57ec70) at /home/remote/jhrozek/devel/sssd/src/db/sysdb_subdomains.c:164
> > #2  0x0000000000417752 in test_sysdb_subdomain_create (_i=0) at /home/remote/jhrozek/devel/sssd/src/tests/sysdb-tests.c:4558
> > #3  0x00007ffff7bd7d12 in tcase_run_tfun_nofork (sr=sr@entry=0x42b180, tc=tc@entry=0x42ac00, i=i@entry=0, tfun=0x42ad40, tfun=0x42ad40) at check_run.c:332
> > #4  0x00007ffff7bd7f6e in srunner_iterate_tcase_tfuns (tc=0x42ac00, sr=0x42b180) at check_run.c:192
> > #5  srunner_run_tcase (tc=0x42ac00, sr=0x42b180) at check_run.c:318
> > #6  srunner_iterate_suites (print_mode=<optimized out>, tcname=0x0, sname=0x0, sr=0x42b180) at check_run.c:161
> > #7  srunner_run (sr=0x42b180, sname=0x0, tcname=0x0, print_mode=<optimized out>) at check_run.c:596
> > #8  0x000000000041a42b in main (argc=1, argv=0x7fffffffdc48) at /home/remote/jhrozek/devel/sssd/src/tests/sysdb-tests.c:5369
>
> sorry I haven't seen this on my 32bit VM, but I reproduce it on 64bits.
>
> If have added a check to prevent this. As you suggested I also added a
> debug message with the current original DN and a comment explaining the
> IPA tree layout with respect to member domains in a forest. All changes
> are in the first patch, the others are unchanged.
>
> bye,
> Sumit

capaths are still written OK, code builds and tests pass.

ACK

[Jakub Hrozek]

On Fri, Sep 27, 2013 at 10:32:58AM +0200, Jakub Hrozek wrote:
> On Fri, Sep 27, 2013 at 10:14:23AM +0200, Sumit Bose wrote:
> > On Thu, Sep 26, 2013 at 10:27:07PM +0200, Jakub Hrozek wrote:
> > > On Thu, Sep 26, 2013 at 10:11:21PM +0200, Jakub Hrozek wrote:
> > > > On Thu, Sep 26, 2013 at 09:45:12PM +0200, Sumit Bose wrote:
> > > > > On Thu, Sep 26, 2013 at 07:24:03PM +0200, Jakub Hrozek wrote:
> > > > > > On Thu, Sep 26, 2013 at 11:22:06AM +0200, Sumit Bose wrote:
> > > > > > > Hi,
> > > > > > > 
> > > > > > > the first two attached patches should fix
> > > > > > > https://fedorahosted.org/sssd/ticket/2093 and make
> > > > > > > https://fedorahosted.org/sssd/ticket/2080 invalid. The third fixes wrong
> > > > > > > return code in one of the functions I touched with in the other patches.
> > > > > > > 
> > > > > > > bye,
> > > > > > > Sumit
> > > > > >
> > > > > > > +    if (!ldb_dn_validate(dn)) {
> > > > > > > +        DEBUG(SSSDBG_OP_FAILURE, ("Original DN [%s] is not a valid DN.\n",
> > > > > > > +                                  orig_dn));
> > > > > > > +        ret = EINVAL;
> > > > > > > +        goto done;
> > > > > > > +    }
> > > > > > > +
> > > > > > > +    if (ldb_dn_get_comp_num(dn) < 5) {
> > > > > > > +        /* we are only interested in the member domain objects */
> > > > > > > +        ret = EOK;
> > > > > > > +        goto done;
> > > > > > > +    }
> > > > > > > +
> > > > > > > +    val = ldb_dn_get_component_val(dn, 3);
> > > > > > > +    if (strncasecmp("trusts", (const char *) val->data, val->length) != 0) {
> > > > > > > +        DEBUG(SSSDBG_TRACE_FUNC,
> > > > > > > +              ("4th component is not 'trust', nothing to do.\n"));
> > > > > > > +        ret = EOK;
> > > > > > > +        goto done;
> > > > > > > +    }
> > > > > > > +
> > > > > > > +    val = ldb_dn_get_component_val(dn, 2);
> > > > > > > +    if (strncasecmp("ad", (const char *) val->data, val->length) != 0) {
> > > > > > > +        DEBUG(SSSDBG_TRACE_FUNC,
> > > > > > > +              ("3rd component is not 'ad', nothing to do.\n"));
> > > > > > > +        ret = EOK;
> > > > > > > +        goto done;
> > > > > > > +    }
> > > > > > > +
> > > > > > > +    val = ldb_dn_get_component_val(dn, 1);
> > > > > > > +    forest = talloc_strndup(mem_ctx, (const char *) val->data, val->length);
> > > > > > > +    if (forest == NULL) {
> > > > > > > +        DEBUG(SSSDBG_OP_FAILURE, ("talloc_strndup failed.\n"));
> > > > > > > +        ret = ENOMEM;
> > > > > > > +        goto done;
> > > > > > > +    }
> > > > > >
> > > > > > I think there is an off-by-one error here. In my setup, the original DN
> > > > > > is:
> > > > > >
> > > > > > cn=AD.EXAMPLE.COM,cn=ad,cn=trusts,dc=ipatest,dc=example,dc=com
> > > > > >
> > > > > > But "val = ldb_dn_get_component_val(dn, 3);" is "ipatest" in my case, so
> > > > > > the strcmp never matches. So I think the indexes should say "2, 1, 0".
> > > > > >
> > > > > > It would also be nice to print the original DN in a DEBUG message.
> > > > >
> > > > > ah, sorry, I should have said that Alexander's latest FreeIPA patches
> > > > > are needed here. They will store the member domains below the forest
> > > > > root object, e.g.
> > > > > cn=member.dom,cn=AD.EXAMPLE.COM,cn=ad,cn=trusts,dc=ipatest,dc=example,dc=com
> > > > > For those domains the patch will set the forest name which in this case
> > > > > is AD.EXAMPLE.COM.
> > > > >
> > > > > bye,
> > > > > Sumit
> > > >
> > > > Ah, then the code is fine.
> > > >
> > > > ACK.
> > > >
> > > > But is should be noted that I only tested the file is generated and
> > > > contains the [capaths] section, not any actual functionality it brings.
> > >
> > > Actually it seems that the sysdb tests are not happy:
> > > (gdb) r
> > > Starting program: /dev/shm/sssd/.libs/sysdb-tests 
> > > [Thread debugging using libthread_db enabled]
> > > Using host libthread_db library "/usr/lib64/libthread_db.so.1".
> > > Warning: LDB_MODULES_PATH is not set, will use LDB plugins installed in system paths.
> > > Running suite(s): sysdb
> > > ldb: unable to dlopen /usr/lib64/ldb/modules/ldb/memberof.la : /usr/lib64/ldb/modules/ldb/memberof.la: invalid ELF header
> > >
> > > Program received signal SIGSEGV, Segmentation fault.
> > > __strcasecmp_l_avx () at ../sysdeps/x86_64/multiarch/strcmp-sse42.S:164
> > > 164             movdqu  (%rdi), %xmm1
> > > (gdb) bt
> > > #0  __strcasecmp_l_avx () at ../sysdeps/x86_64/multiarch/strcmp-sse42.S:164
> > > #1  0x00007ffff7facc16 in sysdb_update_subdomains (domain=0x57ec70) at /home/remote/jhrozek/devel/sssd/src/db/sysdb_subdomains.c:164
> > > #2  0x0000000000417752 in test_sysdb_subdomain_create (_i=0) at /home/remote/jhrozek/devel/sssd/src/tests/sysdb-tests.c:4558
> > > #3  0x00007ffff7bd7d12 in tcase_run_tfun_nofork (sr=sr@entry=0x42b180, tc=tc@entry=0x42ac00, i=i@entry=0, tfun=0x42ad40, tfun=0x42ad40) at check_run.c:332
> > > #4  0x00007ffff7bd7f6e in srunner_iterate_tcase_tfuns (tc=0x42ac00, sr=0x42b180) at check_run.c:192
> > > #5  srunner_run_tcase (tc=0x42ac00, sr=0x42b180) at check_run.c:318
> > > #6  srunner_iterate_suites (print_mode=<optimized out>, tcname=0x0, sname=0x0, sr=0x42b180) at check_run.c:161
> > > #7  srunner_run (sr=0x42b180, sname=0x0, tcname=0x0, print_mode=<optimized out>) at check_run.c:596
> > > #8  0x000000000041a42b in main (argc=1, argv=0x7fffffffdc48) at /home/remote/jhrozek/devel/sssd/src/tests/sysdb-tests.c:5369
> >
> > sorry I haven't seen this on my 32bit VM, but I reproduce it on 64bits.
> >
> > If have added a check to prevent this. As you suggested I also added a
> > debug message with the current original DN and a comment explaining the
> > IPA tree layout with respect to member domains in a forest. All changes
> > are in the first patch, the others are unchanged.
> >
> > bye,
> > Sumit
>
> capaths are still written OK, code builds and tests pass.
>
> ACK

Pushed to master and sssd-1-11