[CRIU] [PATCH 1/3] security: check additional groups,v5

[Ruslan Kuprieiev]

06.08.2014 16:43, Christopher Covington пишет:
> On 08/06/2014 09:22 AM, Ruslan Kuprieiev wrote:
>> Hi Christopher,
>>
>> this is a bit unexpected issue for me=).
>> Could you provide more info about your case?
>> Are you using something like /etc/nsswitch.conf?
> I'm running simulators for various architectures such as x86_64, ARM, and
> AArch64. I've cross-compiled root filesystems with pretty much just what's
> necessary to boot, run a benchmark, use perf and other tools to profile it,
> and dump and restore checkpoints with CRIU (I also keep around debug tools
> like strace, but if everything worked all the time I wouldn't need them ;).
> When someone attaches to the serial console they simply get a shell, no
> password required. In /etc/inittab I have:
>
> console::respawn:-getty -n -l mylogin -L console 115200 xterm
>
> Where mylogin is simply:
>
> sh -l
>
> (Because I want /etc/profile to be sourced.)

Wow, cool!

> /tmp # whoami
> whoami: unknown uid 0
> /tmp # groups
> 0groups: unknown ID 0

So you are running criu only as uid 0, gid 0 , right?
If so, I can create patch to just skip getting groups for root(uid=0,gid=0),
additional groups don't matter for uid 0, gid 0.

> I could create /etc/passwd and /etc/group files, but they wouldn't have a
> group list of greater than one that the new code seems to be intended to handle.
>
> Thanks,
> Christopher
>
>> 06.08.2014 16:04, Christopher Covington пишет:
>>> Hi,
>>>
>>> This change breaks `criu check` and `criu dump` for me because I don't have an
>>> /etc/passwd.
>>>
>>> On 07/14/2014 02:24 PM, Ruslan Kuprieiev wrote:
>>>> Currently, we only check if process gids match primary gid of user.
>>>> But process and user have additional groups too. So lets:
>>>>        1) check that process rgid,egid and sgid are in the user's grouplist.
>>>>        2) on restore check that user has all groups from the images.
>>>> --- a/security.c
>>>> +++ b/security.c
>>>> @@ -1,14 +1,23 @@
>>>>    #include <unistd.h>
>>>> +#include <pwd.h>
>>>> +#include <grp.h>
>>>> +#include <limits.h>
>>>> +#include <stdlib.h>
>>>> +#include <string.h>
>>>> +
>>>>    #include "crtools.h"
>>>>    #include "proc_parse.h"
>>>>    #include "log.h"
>>>> +#include "xmalloc.h"
>>>> +#include "bug.h"
>>>>      #include "protobuf/creds.pb-c.h"
>>>>      /*
>>>> - * UID and GID of user requesting for C/R
>>>> + * UID, GID and groups of user requesting for C/R
>>>>     */
>>>>    static unsigned int cr_uid, cr_gid;
>>>> +static unsigned int cr_ngroups, *cr_groups;
>>>>      /*
>>>>     * Setup what user is requesting for dump (via rpc or using
>>>> @@ -17,11 +26,36 @@ static unsigned int cr_uid, cr_gid;
>>>>     * access to. (Or implement some trickier security policy).
>>>>     */
>>>>    -void restrict_uid(unsigned int uid, unsigned int gid)
>>>> +int restrict_uid(unsigned int uid, unsigned int gid)
>>>>    {
>>>> -    pr_info("Restrict C/R with %u:%u uid\n", uid, gid);
>>>> +    struct passwd *pwd;
>>>> +    unsigned int buf[NGROUPS_MAX];
>>>> +    int nbuf;
>>>> +
>>>> +    pr_info("Restrict C/R with %u:%u uid:gid\n", uid, gid);
>>>>        cr_uid = uid;
>>>>        cr_gid = gid;
>>>> +
>>>> +    pwd = getpwuid(uid);
>>>> +    if (!pwd) {
>>>> +        pr_perror("Can't get password file entry");
>>>> +        return -1;
>>>> +    }
>>> Could CRIU just revert to the old behavior rather than exiting if /etc/passwd
>>> doesn't exist?
>>>
>>> Thanks,
>>> Christopher
>>>
>