[SSSD] [PATCH] AD: replace GID/UID, do not add another one

Sumit Bose sbose at redhat.com
Wed Jan 9 16:41:54 UTC 2013


On Mon, Jan 07, 2013 at 03:06:59PM +0100, Jakub Hrozek wrote:
> On Mon, Jan 07, 2013 at 02:51:53PM +0100, Jakub Hrozek wrote:
> > On Sun, Jan 06, 2013 at 02:35:42PM +0100, Jakub Hrozek wrote:
> > > Please see the commit message. I was wondering whether to include a new
> > > sysdb function, but then I couldn't think of any other place in the code
> > > that needs to replace an attribute.
> > 
> > Self-nack, this doesn't work properly.
> 
> New patch is attached.

Two minor issues, otherwise the patch look good.

bye,
Sumit

> From 179d0ed3b4b8905a8d48099d799160c9fb8af9f2 Mon Sep 17 00:00:00 2001
> From: Jakub Hrozek <jhrozek at redhat.com>
> Date: Sun, 6 Jan 2013 16:04:32 +0100
> Subject: [PATCH] AD: replace GID/UID, do not add another one
> 
> The code would call sysdb_attrs_add_uint32 which added another UID or GID
> to the ID=0 we already downloaded from LDAP (0 is the default value) when
> ID-mapping an entry. This led to funky behaviour later on when we wanted
> to process the ID.
> ---
>  src/providers/ldap/sdap.c              | 31 +++++++++++++++++++++++++++++++
>  src/providers/ldap/sdap.h              |  2 ++
>  src/providers/ldap/sdap_async_groups.c |  8 +++-----
>  src/providers/ldap/sdap_async_users.c  |  7 +++++--
>  4 files changed, 41 insertions(+), 7 deletions(-)
> 
> diff --git a/src/providers/ldap/sdap.c b/src/providers/ldap/sdap.c
> index f5b1f95f0eaa8e6b5ea9d77c1d7226c05d366104..4cdc06c5474d7d49e7f8e78f74ae9c4a62e7a5e1 100644
> --- a/src/providers/ldap/sdap.c
> +++ b/src/providers/ldap/sdap.c
> @@ -1158,3 +1158,34 @@ int sdap_control_create(struct sdap_handle *sh, const char *oid, int iscritical,
>  
>      return ret;
>  }
> +
> +int sdap_replace_id(struct sysdb_attrs *entry, const char *attr, id_t val)
> +{
> +    char *str;
> +    errno_t ret;
> +    struct ldb_message_element *el;
> +
> +    ret = sysdb_attrs_get_el_ext(entry, attr, false, &el);
> +    if (ret == ENOENT) {
> +        return sysdb_attrs_add_uint32(entry, attr, val);
> +    } else if (ret) {
> +        DEBUG(SSSDBG_OP_FAILURE, ("No such attribute %s\n", attr));

The debug message is wrong.

> +        return ret;
> +    }
> +
> +    if (el->num_values != 1) {
> +        DEBUG(SSSDBG_OP_FAILURE,
> +              ("Expected 1 value for %s, got %d\n", attr, el->num_values));
> +        return EINVAL;
> +    }
> +
> +    str = talloc_asprintf(entry, "%llu", (unsigned long long) val);
> +    if (!str) {
> +        return ENOMEM;
> +    }
> +
> +    el->values[0].data = (uint8_t *) str;
> +    el->values[0].length = strlen(str);
> +
> +    return EOK;
> +}
> diff --git a/src/providers/ldap/sdap.h b/src/providers/ldap/sdap.h
> index d844ad6369dc19c82ea761afae746f9d4bd0ce82..d143657999f081dc067c43e0e7dedbb4a6ef3489 100644
> --- a/src/providers/ldap/sdap.h
> +++ b/src/providers/ldap/sdap.h
> @@ -488,6 +488,8 @@ int build_attrs_from_map(TALLOC_CTX *memctx,
>  int sdap_control_create(struct sdap_handle *sh, const char *oid, int iscritical,
>                          struct berval *value, int dupval, LDAPControl **ctrlp);
>  
> +int sdap_replace_id(struct sysdb_attrs *entry, const char *attr, id_t val);
> +
>  errno_t sdap_set_config_options_with_rootdse(struct sysdb_attrs *rootdse,
>                                               struct sdap_options *opts);
>  int sdap_get_server_opts_from_rootdse(TALLOC_CTX *memctx,
> diff --git a/src/providers/ldap/sdap_async_groups.c b/src/providers/ldap/sdap_async_groups.c
> index 26f36fb93f4316f5b610ee2251479444493f1158..e1e84c339618c5fded3a28693b99b2a27058c05a 100644
> --- a/src/providers/ldap/sdap_async_groups.c
> +++ b/src/providers/ldap/sdap_async_groups.c
> @@ -398,11 +398,9 @@ static int sdap_save_group(TALLOC_CTX *memctx,
>          /* Store the GID in the ldap_attrs so it doesn't get
>           * treated as a missing attribute from LDAP and removed.
>           */
> -        ret = sysdb_attrs_add_uint32(attrs, SYSDB_GIDNUM, gid);
> -        if (ret != EOK) {
> -            DEBUG(SSSDBG_MINOR_FAILURE,
> -                  ("Could not store GID: [%s]\n",
> -                   strerror(ret)));
> +        ret = sdap_replace_id(attrs, SYSDB_GIDNUM, gid);
> +        if (ret) {
> +            DEBUG(SSSDBG_OP_FAILURE, ("Cannot set the id-mapped GID\n"));
>              goto done;
>          }
>      } else {
> diff --git a/src/providers/ldap/sdap_async_users.c b/src/providers/ldap/sdap_async_users.c
> index f640b970a12ec2ecfa0af6c4ce9e49cffe8b9528..a0ccfb0d91de9ca71eda1ac62fe7a43f00a9baf5 100644
> --- a/src/providers/ldap/sdap_async_users.c
> +++ b/src/providers/ldap/sdap_async_users.c
> @@ -160,8 +160,11 @@ int sdap_save_user(TALLOC_CTX *memctx,
>          /* Store the UID in the ldap_attrs so it doesn't get
>           * treated as a missing attribute from LDAP and removed.
>           */
> -        ret = sysdb_attrs_add_uint32(attrs, SYSDB_UIDNUM, uid);
> -        if (ret != EOK) goto done;
> +        ret = sdap_replace_id(attrs, SYSDB_UIDNUM, uid);
> +        if (ret) {
> +            DEBUG(SSSDBG_OP_FAILURE, ("Cannot set the id-mapped GID\n"));

The debug message is wrong.

> +            goto done;
> +        }
>      } else {
>          ret = sysdb_attrs_get_uint32_t(attrs,
>                                         opts->user_map[SDAP_AT_USER_UID].sys_name,
> -- 
> 1.8.0.2
> 

> _______________________________________________
> sssd-devel mailing list
> sssd-devel at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel




More information about the sssd-devel mailing list