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

[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.)

/tmp # whoami
whoami: unknown uid 0
/tmp # groups
0groups: unknown ID 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
>>
> 


-- 
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by the Linux Foundation.