[CRIU] [PATCH] security: check additional gids

[Pavel Emelyanov]

On 06/23/2014 11:04 AM, Ruslan Kuprieiev wrote:
> Currently, we only check if process gids match primary gid of user.
> But user may have some additional groups. So lets check them too.

I believe you inverted this in the patch. We should check for additional
groups from image to be accessible by the caller, shouldn't we?

> Signed-off-by: Ruslan Kuprieiev <kupruser at gmail.com>
> ---
>   cr-service.c      |  3 ++-
>   crtools.c         |  3 ++-
>   include/crtools.h |  2 +-
>   security.c        | 74 
> +++++++++++++++++++++++++++++++++++++++++++++++--------
>   4 files changed, 69 insertions(+), 13 deletions(-)
> 
> -void restrict_uid(unsigned int uid, unsigned int gid)
> +int restrict_uid(unsigned int uid, unsigned int gid)
>   {
> +    struct passwd *pwd;
> +
>       pr_info("Restrict C/R with %u:%u uid\n", uid, gid);
>       cr_uid = uid;
> -    cr_gid = gid;
> +
> +    pwd = getpwuid(uid);
> +    if (!pwd) {
> +        pr_perror("Can't get user name");
> +        return -1;
> +    }
> +
> +    cr_gids_num = NGROUPS_MAX;
> +    if (getgrouplist(pwd->pw_name, gid, cr_gids, &cr_gids_num) < 0) {

These two reveal non run-time information about the caller, unlike
the rest of security code that work on the actual info.

> +        pr_perror("Can't get group list of the user");
> +        return -1;
> +    }
> +
> +    return 0;
>   }