[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