[SSSD] [PATCHES] Add SID related lookups to the NSS responder - part 3

Jakub Hrozek jhrozek at redhat.com
Tue May 14 16:07:05 UTC 2013


On Mon, May 13, 2013 at 12:33:46PM +0200, Sumit Bose wrote:
> Hi,
> 
> with these four patches the SID-to-name API can now also be used with
> the AD provider and for local IPA accounts. Since this goes beyond the
> functionality needed by the FreeIPA WebUI I send them in a separate
> series. The patches are also a requirement for using the PAC with the AD
> provider (https://fedorahosted.org/sssd/ticket/1558).
> 
> bye,
> Sumit

[PATCH 1/4] IPA: Always initialize ID mapping
>      sdap_auth_ctx->service = ipa_options->service->sdap;
>      ipa_options->auth_ctx->sdap_auth_ctx = sdap_auth_ctx;
>  
> -    ret = ipa_get_id_options(ipa_options, bectx->cdb, bectx->conf_path,
> -                             &sdap_auth_ctx->opts);
> -    if (ret != EOK) {
> +    if (ipa_options->id == NULL) {
> +        ret = EINVAL;
>          goto done;
>      }
> +    sdap_auth_ctx->opts = ipa_options->id;
>  

Ack, I would just like to move all assignments to sdap_auth_ctx together
and then assign to ipa_options->auth_ctx for better readability.

[PATCH 2/4] Handle SID strings in sdap_attrs_get_sid_str() as well
> +    if (el->values[0].length > 2 &&
> +        el->values[0].data[0] == 'S' &&
> +        el->values[0].data[1] == '-')
> +    {

Ack I will just reformat this opening bracket to conform to our coding
guidelines.

> +        sid_str = talloc_strndup(mem_ctx, (char *) el->values[0].data,
> +                                 el->values[0].length);
> +        if (sid_str == NULL) {
> +            DEBUG(SSSDBG_OP_FAILURE, ("talloc_strndup failed.\n"));
> +            return ENOMEM;
> +        }
> +    } else {
> +        err = sss_idmap_bin_sid_to_sid(idmap_ctx->map,
> +                                       el->values[0].data,
> +                                       el->values[0].length,
> +                                       &sid_str);
> +        if (err != IDMAP_SUCCESS) {
> +            DEBUG(SSSDBG_MINOR_FAILURE,
> +                  ("Could not convert SID: [%s]\n",
> +                   idmap_error_string(err)));
> +            return EIO;
> +        }
>      }
>  
>      *_sid_str = talloc_steal(mem_ctx, sid_str);
> -- 
> 1.7.7.6
> 

[PATCH 3/4] IPA: read user and group SID
Ack.

> From 8f59f19435d9d0e349c957f6abf698377f21a80f Mon Sep 17 00:00:00 2001
> From: Sumit Bose <sbose at redhat.com>
> Date: Tue, 7 May 2013 14:39:42 +0200
> Subject: [PATCH 4/4] Add SID related requests to the LDAP provider
> 
> The patch adds support for BE_REQ_BY_SECID and BE_REQ_USER_AND_GROUP to
> the LDAP provider. Since the AD and the IPA provider use the same code
> they support those request now as well.
> 
> Besides allowing that users and groups can be searched by the SID as
> well the new request allows to search users and groups in one run, i.e.
> if there is not user matching the search criteria groups are searched as
> well.
> ---
>  src/providers/ldap/ldap_id.c |  216 +++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 213 insertions(+), 3 deletions(-)
> 
> diff --git a/src/providers/ldap/ldap_id.c b/src/providers/ldap/ldap_id.c
> index 4d373a4..c4e8dbf 100644
> --- a/src/providers/ldap/ldap_id.c
> +++ b/src/providers/ldap/ldap_id.c
> @@ -136,6 +136,14 @@ struct tevent_req *users_get_send(TALLOC_CTX *memctx,
>              }
>          }
>          break;
> +    case BE_FILTER_SECID:
> +        attr_name = ctx->opts->user_map[SDAP_AT_USER_OBJECTSID].name;
> +
> +        ret = sss_filter_sanitize(state, name, &clean_name);
> +        if (ret != EOK) {
> +            goto fail;
> +        }
> +        break;
>      default:
>          ret = EINVAL;
>          goto fail;
> @@ -446,14 +454,22 @@ struct tevent_req *groups_get_send(TALLOC_CTX *memctx,
>              }
>          }
>          break;
> +    case BE_FILTER_SECID:
> +        attr_name = ctx->opts->user_map[SDAP_AT_USER_OBJECTSID].name;

We should be using the group_map here.

I think we should also check the attr validity, since this is generic
LDAP code and the LDAP provider doesn't set the option value itself.

> +static struct tevent_req *get_user_and_group_send(TALLOC_CTX *memctx,
> +                                                  struct tevent_context *ev,
> +                                                  struct sdap_id_ctx *id_ctx,
> +                                                  const char *filter_val,
> +                                                  int filter_type,
> +                                                  int attrs_type)
> +{
> +    struct tevent_req *req;
> +    struct tevent_req *subreq;
> +    struct get_user_and_group_state *state;
> +    int ret;
> +
> +    req = tevent_req_create(memctx, &state, struct get_user_and_group_state);
> +    if (req == NULL) {
> +        DEBUG(SSSDBG_OP_FAILURE, ("tevent_req_create failed.\n"));
> +        return NULL;
> +    }
> +
> +    state->ev = ev;
> +    state->id_ctx = id_ctx;
> +    state->dp_error = DP_ERR_FATAL;
> +
> +    state->op = sdap_id_op_create(state, state->id_ctx->conn_cache);
> +    if (!state->op) {
> +        DEBUG(2, ("sdap_id_op_create failed\n"));

Nitpick, but can you change the legacy debug level since the patch will
be changed anyway?

> +        ret = ENOMEM;
> +        goto fail;
> +    }
> +
> +    state->sysdb = state->id_ctx->be->domain->sysdb;
> +    state->domain = state->id_ctx->be->domain;
> +    state->filter_val = filter_val;
> +    state->filter_type = filter_type;
> +    state->attrs_type = attrs_type;
> +
> +    subreq = users_get_send(req, state->ev, state->id_ctx, state->filter_val,
> +                            state->filter_type, state->attrs_type);
> +    if (subreq == NULL) {
> +        DEBUG(SSSDBG_OP_FAILURE, ("users_get_send failed.\n"));
> +        ret = ENOMEM;
> +        goto fail;
> +    }
> +
> +    tevent_req_set_callback(subreq, get_user_and_group_users_done, req);
> +
> +    return req;
> +
> +fail:
> +    tevent_req_error(req, ret);
> +    tevent_req_post(req, ev);
> +    return req;
> +}
> +
> +static void get_user_and_group_users_done(struct tevent_req *subreq)
> +{
> +    struct tevent_req *req = tevent_req_callback_data(subreq,
> +                                                      struct tevent_req);
> +    struct get_user_and_group_state *state = tevent_req_data(req,
> +                                               struct get_user_and_group_state);
> +    int ret;
> +
> +    ret = users_get_recv(subreq, &state->dp_error);
> +    talloc_zfree(subreq);
> +
> +    if (ret == EOK) { /* Matching user found */
> +        tevent_req_done(req);
> +        return;
> +    }
> +

Here the code is retrying on all failures, would it be safer to only
retry on ENOENT?

> +    subreq = groups_get_send(req, state->ev, state->id_ctx, state->filter_val,
> +                            state->filter_type, state->attrs_type);
> +    if (subreq == NULL) {
> +        DEBUG(SSSDBG_OP_FAILURE, ("groups_get_send failed.\n"));
> +        tevent_req_error(req, ENOMEM);
> +        return;
> +    }
> +
> +    tevent_req_set_callback(subreq, get_user_and_group_groups_done, req);
> +}



More information about the sssd-devel mailing list