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

[Sumit Bose]

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 
> 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
> [e-mail]
> https://lists.fedorahosted.org/mailman/listinfo/sssd-devel