[SSSD] [PATCH] IPA: Remove MPG groups if getgrgid was called before getpw()

Sumit Bose sbose at redhat.com
Wed Jul 22 08:01:31 UTC 2015


On Tue, Jul 21, 2015 at 09:41:46PM +0200, Jakub Hrozek wrote:
> Hi,
> 
> the attached patch fixes https://fedorahosted.org/sssd/ticket/2724 for
> me. I haven't yet heard back from the affected user, but they do have
> test packages available.

The patch is working as expected and fixes the issue for me as well.

Although it might not look as the most elegant way I think the approach
is ok because otherwise we would be to check for every group by GID
request if the GID might belong to a MPG which would require a user
lookup. This would slow down the requests where the GID belong to a
normal (non-MPG) group.

Nevertheless I wonder if we should check if the group name matches the
user name before deleting the group by GID to be on the safe side?

bye,
Sumit

> From b02129c29b35ff74aff537955ed3d38d477b9f3b Mon Sep 17 00:00:00 2001
> From: Jakub Hrozek <jhrozek at redhat.com>
> Date: Tue, 21 Jul 2015 11:44:03 +0200
> Subject: [PATCH] IPA: Remove MPG groups if getgrgid was called before getpw()
> 
> https://fedorahosted.org/sssd/ticket/2724
> 
> This bug only affects IPA clients that are connected to IPA servers with
> AD trust and ID mapping in effect.
> 
> If an IPA client calls getgrgid() for an ID that matches a user, the
> user's private group would be returned and stored as a group entry.
> 
> Subsequent queries for that user would fail, because MPG domains impose
> uniqueness restriction for both the ID and name space across groups and
> users.
> 
> To work around that, we remove the UPG groups in MPG domains during a
> group lookup.
> ---
>  src/providers/ipa/ipa_s2n_exop.c | 31 +++++++++++++++++++++++++++++--
>  1 file changed, 29 insertions(+), 2 deletions(-)
> 
> diff --git a/src/providers/ipa/ipa_s2n_exop.c b/src/providers/ipa/ipa_s2n_exop.c
> index 812a4bbd707faf5c184594b562c148d1e704fd58..88ae41f89c405966b61143d69cf5831ef8a50db2 100644
> --- a/src/providers/ipa/ipa_s2n_exop.c
> +++ b/src/providers/ipa/ipa_s2n_exop.c
> @@ -2005,8 +2005,35 @@ static errno_t ipa_s2n_save_objects(struct sss_domain_info *dom,
>                                     attrs->a.user.pw_dir, attrs->a.user.pw_shell,
>                                     NULL, attrs->sysdb_attrs, NULL,
>                                     timeout, now);
> -            if (ret != EOK) {
> -                DEBUG(SSSDBG_OP_FAILURE, "sysdb_store_user failed.\n");
> +            if (ret == EEXIST && dom->mpg == true) {
> +                /* This handles the case where getgrgid() was called for
> +                 * this user, so a group was created in the cache
> +                 */
> +                ret = sysdb_delete_group(dom, NULL, attrs->a.user.pw_uid);
> +                if (ret != EOK) {
> +                    DEBUG(SSSDBG_OP_FAILURE,
> +                          "sysdb_delete_group failed for MPG group [%d]: %s\n",
> +                          ret, sss_strerror(ret));
> +                    goto done;
> +                }
> +
> +                ret = sysdb_store_user(dom, name, NULL,
> +                                       attrs->a.user.pw_uid,
> +                                       gid, attrs->a.user.pw_gecos,
> +                                       attrs->a.user.pw_dir,
> +                                       attrs->a.user.pw_shell,
> +                                       NULL, attrs->sysdb_attrs, NULL,
> +                                       timeout, now);
> +                if (ret != EOK) {
> +                    DEBUG(SSSDBG_OP_FAILURE,
> +                          "sysdb_store_user failed for MPG user [%d]: %s\n",
> +                          ret, sss_strerror(ret));
> +                    goto done;
> +                }
> +            } else if (ret != EOK) {
> +                DEBUG(SSSDBG_OP_FAILURE,
> +                      "sysdb_store_user failed [%d]: %s\n",
> +                      ret, sss_strerror(ret));
>                  goto done;
>              }
>  
> -- 
> 2.4.3
> 

> _______________________________________________
> 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