Re: [SSSD] [PATCH] IPA: check ranges for collisions before saving them

[Sumit Bose]

On Wed, Feb 19, 2014 at 04:23:55PM +0100, Jakub Hrozek wrote:
>  On Wed, Feb 12, 2014 at 03:43:51PM +0100, Sumit Bose wrote:
>  > Hi,
>  > 
>  > currently we trust the idrange data we get from the IPA server. But it
>  > turned out that some checks are missing on the server
>  > (https://fedorahosted.org/freeipa/ticket/4137) so SSSD should check the
>  > input before saving the data to the cache.
>  > 
>  > The first two patches make some code which already exists available for
>  > other callers. The third contains the actual functionality.
>  > 
>  > bye,
>  > Sumit
>  
>  Hi, I rebased your patches on top of master, can you check if they look
>  OK to you and resend if they are fine? I'll continue testing with my
>  version in the meantime. 
sorry I forgot to send a version for master myself. The patches are
exactly the same as in my master tree.

Thanks

bye,
Sumit

[PATCH] IPA: check ranges for collisions before saving them

[Jakub Hrozek]

On Thu, Feb 20, 2014 at 10:34:44AM +0100, Sumit Bose wrote:
>  On Wed, Feb 19, 2014 at 04:23:55PM +0100, Jakub Hrozek wrote:
>  > On Wed, Feb 12, 2014 at 03:43:51PM +0100, Sumit Bose wrote:
>  > > Hi,
>  > > 
>  > > currently we trust the idrange data we get from the IPA server. But it
>  > > turned out that some checks are missing on the server
>  > > (https://fedorahosted.org/freeipa/ticket/4137) so SSSD should check the
>  > > input before saving the data to the cache.
>  > > 
>  > > The first two patches make some code which already exists available for
>  > > other callers. The third contains the actual functionality.
>  > > 
>  > > bye,
>  > > Sumit
>  > 
>  > Hi, I rebased your patches on top of master, can you check if they look
>  > OK to you and resend if they are fine? I'll continue testing with my
>  > version in the meantime.
>  
>  sorry I forgot to send a version for master myself. The patches are
>  exactly the same as in my master tree.
>  
>  Thanks
>  
>  bye,
>  Sumit 
I'm picking up these patches for review (and also testing patchwork at
the same time)

[PATCH] IPA: check ranges for collisions before saving them

[Jakub Hrozek]

On Thu, Feb 20, 2014 at 10:34:44AM +0100, Sumit Bose wrote:
>  On Wed, Feb 19, 2014 at 04:23:55PM +0100, Jakub Hrozek wrote:
>  > On Wed, Feb 12, 2014 at 03:43:51PM +0100, Sumit Bose wrote:
>  > > Hi,
>  > > 
>  > > currently we trust the idrange data we get from the IPA server. But it
>  > > turned out that some checks are missing on the server
>  > > (https://fedorahosted.org/freeipa/ticket/4137) so SSSD should check the
>  > > input before saving the data to the cache.
>  > > 
>  > > The first two patches make some code which already exists available for
>  > > other callers. The third contains the actual functionality.
>  > > 
>  > > bye,
>  > > Sumit
>  > 
>  > Hi, I rebased your patches on top of master, can you check if they look
>  > OK to you and resend if they are fine? I'll continue testing with my
>  > version in the meantime.
>  
>  sorry I forgot to send a version for master myself. The patches are
>  exactly the same as in my master tree.
>  
>  Thanks
>  
>  bye,
>  Sumit 
>  From 2def1c79630be290ec53a5c17871238b3fb388bb Mon Sep 17 00:00:00
> 2001
>  From: Sumit Bose <sbose(a)redhat.com&gt;
>  Date: Mon, 3 Feb 2014 13:30:35 +0100
>  Subject: [PATCH 1/3] IDMAP: add sss_idmap_check_collision(_ex) 
ACK

>  From 4b6b398e56da8453141a18882e39cfdca86d8941 Mon Sep 17 00:00:00
> 2001
>  From: Sumit Bose <sbose(a)redhat.com&gt;
>  Date: Fri, 7 Feb 2014 15:54:30 +0100
>  Subject: [PATCH 2/3] IPA: refactor idmap code and add test 
See two comments inline:

>  @@ -187,72 +245,36 @@ errno_t ipa_idmap_find_new_domain(struct
> sdap_idmap_ctx *idmap_ctx,
>       }
>   
>       for (c = 0; c < range_count; c++) {
>  -        r = range_list[c];
>  -
>  -        if (r->range_type == NULL) {
>  -            /* Older IPA servers might not have the range_type attribute, but
>  -             * only support local ranges and trusts with algorithmic mapping. */
>  -
>  -            if (r->trusted_dom_sid == NULL && r->secondary_base_rid != 0)
> {
>  -                /* local IPA domain */
>  -                rid = 0;
>  -                external_mapping = true;
>  -                name = idmap_ctx->id_ctx->be->domain->name;
>  -                sid = NULL;
>  -            } else if (r->trusted_dom_sid != NULL
>  -                    && r->secondary_base_rid == 0) {
>  -                /* trusted domain */
>  -                rid = r->base_rid;
>  -                external_mapping = false;
>  -                name = r->trusted_dom_sid;
>  -                sid = r->trusted_dom_sid;
>  -            } else {
>  -                DEBUG(SSSDBG_MINOR_FAILURE, "Cannot determine range type, " \
>  -                                             "skipping id ange [%s].\n",
>  -                                             r->name);
>  -                continue;
>  -            }
>  -        } else {
>  -            if (strcmp(r->range_type, IPA_RANGE_LOCAL) == 0) {
>  -                rid = 0;
>  -                external_mapping = true;
>  -                name = idmap_ctx->id_ctx->be->domain->name;
>  -                sid = NULL;
>  -            } else if (strcmp(r->range_type, IPA_RANGE_AD_TRUST_POSIX) == 0) {
>  -                rid = 0;
>  -                external_mapping = true;
>  -                name = r->trusted_dom_sid;
>  -                sid = r->trusted_dom_sid;
>  -            } else if (strcmp(r->range_type, IPA_RANGE_AD_TRUST) == 0) {
>  -                rid = r->base_rid;
>  -                external_mapping = false;
>  -                name = r->trusted_dom_sid;
>  -                sid = r->trusted_dom_sid;
>  -            } else {
>  -                DEBUG(SSSDBG_MINOR_FAILURE, "Range type [%s] not supported, "
> \
>  -                                             "skipping id range [%s].\n",
>  -                                             r->range_type, r->name);
>  -                continue;
>  -            }
>  +        ret = get_idmap_data_from_range(range_list[c],
>  +                                       
> idmap_ctx->id_ctx->be->domain->name,
>  +                                        &name, &sid, &rid, &range,
>  +                                        &external_mapping);
>  +        if (ret != EOK) {
>  +            DEBUG(SSSDBG_OP_FAILURE, "get_idmap_data_from_range failed for "
> \
>  +                                     "id range [%s], skipping.\n",
>  +                                     range_list[c]->name);
>  +            continue;
>           }
>   
>  -        range.min = r->base_id;
>  -        range.max = r->base_id + r->id_range_size -1;
>           err = sss_idmap_add_domain_ex(idmap_ctx->map, name, sid, &range,
>  -                                      r->name, rid, external_mapping);
>  -        if (err != IDMAP_SUCCESS && err != IDMAP_COLLISION) {
>  +                                      range_list[c]->name, rid,
>  +                                      external_mapping);
>  +        if (err != IDMAP_SUCCESS
>  +                && allow_collisions && err != IDMAP_COLLISION) { 
I don't think this condition is right, if allow_collisions is false,
then it never matches.

>               DEBUG(SSSDBG_CRIT_FAILURE, "Could not add range
> [%s] to ID map\n",
>  -                                        r->name);
>  +                                       range_list[c]->name);
>               ret = EIO;
>               goto done;
>           }
>       } 
[snip]

>  diff --git a/src/tests/cmocka/test_ipa_idmap.c
> b/src/tests/cmocka/test_ipa_idmap.c 
[snip]

>  +int main(int argc, const char *argv[])
>  +{
>  +    poptContext pc;
>  +    int opt;
>  +    struct poptOption long_options[] = {
>  +        POPT_AUTOHELP
>  +        SSSD_DEBUG_OPTS
>  +        POPT_TABLEEND
>  +    };
>  +
>  +    const UnitTest tests[] = {
>  +        //unit_test_setup_teardown(test_add_domain,
>  +        //                         test_sss_idmap_setup, test_sss_idmap_teardown),
It seems that no further patch includes test_add_domain either, did you
want to remove the commented out code?

>  +        unit_test(test_get_idmap_data_from_range),
>  +    };
>  +
>  +    /* Set debug level to invalid value so we can deside if -d 0 was used. */
>  +    debug_level = SSSDBG_INVALID;
>  +
>  +    pc = poptGetContext(argv[0], argc, argv, long_options, 0);
>  +    while((opt = poptGetNextOpt(pc)) != -1) {
>  +        switch(opt) {
>  +        default:
>  +            fprintf(stderr, "\nInvalid option %s: %s\n\n",
>  +                    poptBadOption(pc, 0), poptStrerror(opt));
>  +            poptPrintUsage(pc, stderr, 0);
>  +            return 1;
>  +        }
>  +    }
>  +    poptFreeContext(pc);
>  +
>  +    DEBUG_INIT(debug_level);
>  +
>  +    tests_set_cwd();
>  +
>  +    return run_tests(tests);
>  +}
>  -- 
>  1.8.5.3
>   
>  From 07ca39a31c5abde685481ef6fe993a0eb7a7f9b3 Mon Sep 17 00:00:00
> 2001
>  From: Sumit Bose <sbose(a)redhat.com&gt;
>  Date: Fri, 7 Feb 2014 18:17:09 +0100
>  Subject: [PATCH 3/3] IPA: check ranges for collisions before saving them 
ACK

When you resend the patches, can you also include the version for 1.11 ?
I think the initial submission would be easy to reuse..

[PATCH] IPA: check ranges for collisions before saving them

[Sumit Bose]

On Mon, Feb 24, 2014 at 07:12:00PM +0100, Jakub Hrozek wrote:
>  On Thu, Feb 20, 2014 at 10:34:44AM +0100, Sumit Bose wrote:
>  > On Wed, Feb 19, 2014 at 04:23:55PM +0100, Jakub Hrozek wrote:
>  > > On Wed, Feb 12, 2014 at 03:43:51PM +0100, Sumit Bose wrote:
>  > > > Hi,
>  > > > 
>  > > > currently we trust the idrange data we get from the IPA server. But it
>  > > > turned out that some checks are missing on the server
>  > > > (https://fedorahosted.org/freeipa/ticket/4137) so SSSD should check the
>  > > > input before saving the data to the cache.
>  > > > 
>  > > > The first two patches make some code which already exists available for
>  > > > other callers. The third contains the actual functionality.
>  > > > 
>  > > > bye,
>  > > > Sumit
>  > > 
>  > > Hi, I rebased your patches on top of master, can you check if they look
>  > > OK to you and resend if they are fine? I'll continue testing with my
>  > > version in the meantime.
>  > 
>  > sorry I forgot to send a version for master myself. The patches are
>  > exactly the same as in my master tree.
>  > 
>  > Thanks
>  > 
>  > bye,
>  > Sumit
>   
...

>  >          err = sss_idmap_add_domain_ex(idmap_ctx->map, name,
> sid, &range,
>  > -                                      r->name, rid, external_mapping);
>  > -        if (err != IDMAP_SUCCESS && err != IDMAP_COLLISION) {
>  > +                                      range_list[c]->name, rid,
>  > +                                      external_mapping);
>  > +        if (err != IDMAP_SUCCESS
>  > +                && allow_collisions && err != IDMAP_COLLISION) {
>  
>  I don't think this condition is right, if allow_collisions is false,
>  then it never matches. 
good catch, thank you, fixed.

>  
>  >              DEBUG(SSSDBG_CRIT_FAILURE, "Could not add range [%s] to ID
> map\n",
>  > -                                        r->name);
>  > +                                       range_list[c]->name);
>  >              ret = EIO;
>  >              goto done;
>  >          }
>  >      }
>  
>  [snip]
>  
>  > diff --git a/src/tests/cmocka/test_ipa_idmap.c b/src/tests/cmocka/test_ipa_idmap.c
>  
>  [snip]
>  
>  > +int main(int argc, const char *argv[])
>  > +{
>  > +    poptContext pc;
>  > +    int opt;
>  > +    struct poptOption long_options[] = {
>  > +        POPT_AUTOHELP
>  > +        SSSD_DEBUG_OPTS
>  > +        POPT_TABLEEND
>  > +    };
>  > +
>  > +    const UnitTest tests[] = {
>  > +        //unit_test_setup_teardown(test_add_domain,
>  > +        //                         test_sss_idmap_setup, test_sss_idmap_teardown),
>  
>  It seems that no further patch includes test_add_domain either, did you
>  want to remove the commented out code?
>   
yes, I removed it and added a new test to make sure the allow_collisions
check above is working as expected.

New version for master and 1.11 are attached.

bye,
Sumit

[PATCH] IPA: check ranges for collisions before saving them

[Jakub Hrozek]

On Tue, Feb 25, 2014 at 05:15:18PM +0100, Sumit Bose wrote:
>  On Mon, Feb 24, 2014 at 07:12:00PM +0100, Jakub Hrozek wrote:
>  > On Thu, Feb 20, 2014 at 10:34:44AM +0100, Sumit Bose wrote:
>  > > On Wed, Feb 19, 2014 at 04:23:55PM +0100, Jakub Hrozek wrote:
>  > > > On Wed, Feb 12, 2014 at 03:43:51PM +0100, Sumit Bose wrote:
>  > > > > Hi,
>  > > > > 
>  > > > > currently we trust the idrange data we get from the IPA server. But
> it
>  > > > > turned out that some checks are missing on the server
>  > > > > (https://fedorahosted.org/freeipa/ticket/4137) so SSSD should check
> the
>  > > > > input before saving the data to the cache.
>  > > > > 
>  > > > > The first two patches make some code which already exists available
> for
>  > > > > other callers. The third contains the actual functionality.
>  > > > > 
>  > > > > bye,
>  > > > > Sumit
>  > > > 
>  > > > Hi, I rebased your patches on top of master, can you check if they look
>  > > > OK to you and resend if they are fine? I'll continue testing with my
>  > > > version in the meantime.
>  > > 
>  > > sorry I forgot to send a version for master myself. The patches are
>  > > exactly the same as in my master tree.
>  > > 
>  > > Thanks
>  > > 
>  > > bye,
>  > > Sumit
>  > 
>  
>  ...
>  
>  > >          err = sss_idmap_add_domain_ex(idmap_ctx->map, name, sid,
> &range,
>  > > -                                      r->name, rid, external_mapping);
>  > > -        if (err != IDMAP_SUCCESS && err != IDMAP_COLLISION) {
>  > > +                                      range_list[c]->name, rid,
>  > > +                                      external_mapping);
>  > > +        if (err != IDMAP_SUCCESS
>  > > +                && allow_collisions && err != IDMAP_COLLISION)
> {
>  > 
>  > I don't think this condition is right, if allow_collisions is false,
>  > then it never matches.
>  
>  good catch, thank you, fixed.
>  
>  > 
>  > >              DEBUG(SSSDBG_CRIT_FAILURE, "Could not add range [%s] to ID
> map\n",
>  > > -                                        r->name);
>  > > +                                       range_list[c]->name);
>  > >              ret = EIO;
>  > >              goto done;
>  > >          }
>  > >      }
>  > 
>  > [snip]
>  > 
>  > > diff --git a/src/tests/cmocka/test_ipa_idmap.c
> b/src/tests/cmocka/test_ipa_idmap.c
>  > 
>  > [snip]
>  > 
>  > > +int main(int argc, const char *argv[])
>  > > +{
>  > > +    poptContext pc;
>  > > +    int opt;
>  > > +    struct poptOption long_options[] = {
>  > > +        POPT_AUTOHELP
>  > > +        SSSD_DEBUG_OPTS
>  > > +        POPT_TABLEEND
>  > > +    };
>  > > +
>  > > +    const UnitTest tests[] = {
>  > > +        //unit_test_setup_teardown(test_add_domain,
>  > > +        //                         test_sss_idmap_setup,
> test_sss_idmap_teardown),
>  > 
>  > It seems that no further patch includes test_add_domain either, did you
>  > want to remove the commented out code?
>  > 
>  
>  yes, I removed it and added a new test to make sure the allow_collisions
>  check above is working as expected.
>  
>  New version for master and 1.11 are attached.
>  
>  bye,
>  Sumit 
Thank you I will do some testing later today, so far I've submitted the
patches to Coverity.

[PATCH] IPA: check ranges for collisions before saving them

[Jakub Hrozek]

On Tue, Feb 25, 2014 at 05:29:53PM +0100, Jakub Hrozek wrote:
>  On Tue, Feb 25, 2014 at 05:15:18PM +0100, Sumit Bose wrote:
>  > On Mon, Feb 24, 2014 at 07:12:00PM +0100, Jakub Hrozek wrote:
>  > > On Thu, Feb 20, 2014 at 10:34:44AM +0100, Sumit Bose wrote:
>  > > > On Wed, Feb 19, 2014 at 04:23:55PM +0100, Jakub Hrozek wrote:
>  > > > > On Wed, Feb 12, 2014 at 03:43:51PM +0100, Sumit Bose wrote:
>  > > > > > Hi,
>  > > > > > 
>  > > > > > currently we trust the idrange data we get from the IPA server.
> But it
>  > > > > > turned out that some checks are missing on the server
>  > > > > > (https://fedorahosted.org/freeipa/ticket/4137) so SSSD should
> check the
>  > > > > > input before saving the data to the cache.
>  > > > > > 
>  > > > > > The first two patches make some code which already exists
> available for
>  > > > > > other callers. The third contains the actual functionality.
>  > > > > > 
>  > > > > > bye,
>  > > > > > Sumit
>  > > > > 
>  > > > > Hi, I rebased your patches on top of master, can you check if they
> look
>  > > > > OK to you and resend if they are fine? I'll continue testing with
> my
>  > > > > version in the meantime.
>  > > > 
>  > > > sorry I forgot to send a version for master myself. The patches are
>  > > > exactly the same as in my master tree.
>  > > > 
>  > > > Thanks
>  > > > 
>  > > > bye,
>  > > > Sumit
>  > > 
>  > 
>  > ...
>  > 
>  > > >          err = sss_idmap_add_domain_ex(idmap_ctx->map, name, sid,
> &range,
>  > > > -                                      r->name, rid,
> external_mapping);
>  > > > -        if (err != IDMAP_SUCCESS && err != IDMAP_COLLISION) {
>  > > > +                                      range_list[c]->name, rid,
>  > > > +                                      external_mapping);
>  > > > +        if (err != IDMAP_SUCCESS
>  > > > +                && allow_collisions && err !=
> IDMAP_COLLISION) {
>  > > 
>  > > I don't think this condition is right, if allow_collisions is false,
>  > > then it never matches.
>  > 
>  > good catch, thank you, fixed.
>  > 
>  > > 
>  > > >              DEBUG(SSSDBG_CRIT_FAILURE, "Could not add range [%s] to
> ID map\n",
>  > > > -                                        r->name);
>  > > > +                                       range_list[c]->name);
>  > > >              ret = EIO;
>  > > >              goto done;
>  > > >          }
>  > > >      }
>  > > 
>  > > [snip]
>  > > 
>  > > > diff --git a/src/tests/cmocka/test_ipa_idmap.c
> b/src/tests/cmocka/test_ipa_idmap.c
>  > > 
>  > > [snip]
>  > > 
>  > > > +int main(int argc, const char *argv[])
>  > > > +{
>  > > > +    poptContext pc;
>  > > > +    int opt;
>  > > > +    struct poptOption long_options[] = {
>  > > > +        POPT_AUTOHELP
>  > > > +        SSSD_DEBUG_OPTS
>  > > > +        POPT_TABLEEND
>  > > > +    };
>  > > > +
>  > > > +    const UnitTest tests[] = {
>  > > > +        //unit_test_setup_teardown(test_add_domain,
>  > > > +        //                         test_sss_idmap_setup,
> test_sss_idmap_teardown),
>  > > 
>  > > It seems that no further patch includes test_add_domain either, did you
>  > > want to remove the commented out code?
>  > > 
>  > 
>  > yes, I removed it and added a new test to make sure the allow_collisions
>  > check above is working as expected.
>  > 
>  > New version for master and 1.11 are attached.
>  > 
>  > bye,
>  > Sumit
>  
>  Thank you I will do some testing later today, so far I've submitted the
>  patches to Coverity. 
The patches look good to me and work fine.

ACK to all three.

[PATCH] IPA: check ranges for collisions before saving them

[Jakub Hrozek]

On Wed, Feb 26, 2014 at 02:56:51PM +0100, Jakub Hrozek wrote:
>  On Tue, Feb 25, 2014 at 05:29:53PM +0100, Jakub Hrozek wrote:
>  > On Tue, Feb 25, 2014 at 05:15:18PM +0100, Sumit Bose wrote:
>  > > On Mon, Feb 24, 2014 at 07:12:00PM +0100, Jakub Hrozek wrote:
>  > > > On Thu, Feb 20, 2014 at 10:34:44AM +0100, Sumit Bose wrote:
>  > > > > On Wed, Feb 19, 2014 at 04:23:55PM +0100, Jakub Hrozek wrote:
>  > > > > > On Wed, Feb 12, 2014 at 03:43:51PM +0100, Sumit Bose wrote:
>  > > > > > > Hi,
>  > > > > > > 
>  > > > > > > currently we trust the idrange data we get from the IPA
> server. But it
>  > > > > > > turned out that some checks are missing on the server
>  > > > > > > (https://fedorahosted.org/freeipa/ticket/4137) so SSSD
> should check the
>  > > > > > > input before saving the data to the cache.
>  > > > > > > 
>  > > > > > > The first two patches make some code which already exists
> available for
>  > > > > > > other callers. The third contains the actual
> functionality.
>  > > > > > > 
>  > > > > > > bye,
>  > > > > > > Sumit
>  > > > > > 
>  > > > > > Hi, I rebased your patches on top of master, can you check if
> they look
>  > > > > > OK to you and resend if they are fine? I'll continue testing
> with my
>  > > > > > version in the meantime.
>  > > > > 
>  > > > > sorry I forgot to send a version for master myself. The patches are
>  > > > > exactly the same as in my master tree.
>  > > > > 
>  > > > > Thanks
>  > > > > 
>  > > > > bye,
>  > > > > Sumit
>  > > > 
>  > > 
>  > > ...
>  > > 
>  > > > >          err = sss_idmap_add_domain_ex(idmap_ctx->map, name, sid,
> &range,
>  > > > > -                                      r->name, rid,
> external_mapping);
>  > > > > -        if (err != IDMAP_SUCCESS && err != IDMAP_COLLISION)
> {
>  > > > > +                                      range_list[c]->name, rid,
>  > > > > +                                      external_mapping);
>  > > > > +        if (err != IDMAP_SUCCESS
>  > > > > +                && allow_collisions && err !=
> IDMAP_COLLISION) {
>  > > > 
>  > > > I don't think this condition is right, if allow_collisions is false,
>  > > > then it never matches.
>  > > 
>  > > good catch, thank you, fixed.
>  > > 
>  > > > 
>  > > > >              DEBUG(SSSDBG_CRIT_FAILURE, "Could not add range
> [%s] to ID map\n",
>  > > > > -                                        r->name);
>  > > > > +                                       range_list[c]->name);
>  > > > >              ret = EIO;
>  > > > >              goto done;
>  > > > >          }
>  > > > >      }
>  > > > 
>  > > > [snip]
>  > > > 
>  > > > > diff --git a/src/tests/cmocka/test_ipa_idmap.c
> b/src/tests/cmocka/test_ipa_idmap.c
>  > > > 
>  > > > [snip]
>  > > > 
>  > > > > +int main(int argc, const char *argv[])
>  > > > > +{
>  > > > > +    poptContext pc;
>  > > > > +    int opt;
>  > > > > +    struct poptOption long_options[] = {
>  > > > > +        POPT_AUTOHELP
>  > > > > +        SSSD_DEBUG_OPTS
>  > > > > +        POPT_TABLEEND
>  > > > > +    };
>  > > > > +
>  > > > > +    const UnitTest tests[] = {
>  > > > > +        //unit_test_setup_teardown(test_add_domain,
>  > > > > +        //                         test_sss_idmap_setup,
> test_sss_idmap_teardown),
>  > > > 
>  > > > It seems that no further patch includes test_add_domain either, did you
>  > > > want to remove the commented out code?
>  > > > 
>  > > 
>  > > yes, I removed it and added a new test to make sure the allow_collisions
>  > > check above is working as expected.
>  > > 
>  > > New version for master and 1.11 are attached.
>  > > 
>  > > bye,
>  > > Sumit
>  > 
>  > Thank you I will do some testing later today, so far I've submitted the
>  > patches to Coverity.
>  
>  The patches look good to me and work fine.
>  
>  ACK to all three. 
Pushed to master and sssd-1-11