[CRIU] [PATCH] security: check additional gids
Ruslan Kuprieiev
kupruser at gmail.com
Mon Jun 23 05:12:04 PDT 2014
On 23.06.2014 14:18, Pavel Emelyanov wrote:
> 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.
>>
>> 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(-)
>>
>> diff --git a/cr-service.c b/cr-service.c
>> index dc0ff5a..7802e71 100644
>> --- a/cr-service.c
>> +++ b/cr-service.c
>> @@ -178,7 +178,8 @@ static int setup_opts_from_req(int sk, CriuOpts *req)
>> return -1;
>> }
>>
>> - restrict_uid(ids.uid, ids.gid);
>> + if (restrict_uid(ids.uid, ids.gid))
>> + return -1;
> Spaces got corrupted.
That's strange.
Try the attached one, please.
>> if (fstat(sk, &st)) {
>> pr_perror("Can't get socket stat");
>> diff --git a/crtools.c b/crtools.c
>> index 5684186..96ae590 100644
>> --- a/crtools.c
>> +++ b/crtools.c
>> @@ -173,7 +173,8 @@ int main(int argc, char *argv[])
>> BUILD_BUG_ON(PAGE_SIZE != PAGE_IMAGE_SIZE);
>>
>> cr_pb_init();
>> - restrict_uid(getuid(), getgid());
>> + if (restrict_uid(getuid(), getgid()))
>> + return 1;
>>
>> if (argc < 2)
>> goto usage;
>> diff --git a/include/crtools.h b/include/crtools.h
>> index c4ae580..75047fc 100644
>> --- a/include/crtools.h
>> +++ b/include/crtools.h
>> @@ -24,7 +24,7 @@ extern int cr_check(void);
>> extern int cr_exec(int pid, char **opts);
>> extern int cr_dedup(void);
>>
>> -extern void restrict_uid(unsigned int uid, unsigned int gid);
>> +extern int restrict_uid(unsigned int uid, unsigned int gid);
>> struct proc_status_creds;
>> extern bool may_dump(struct proc_status_creds *);
>> struct _CredsEntry;
>> diff --git a/security.c b/security.c
>> index d4b4230..3f3b73b 100644
>> --- a/security.c
>> +++ b/security.c
>> @@ -1,4 +1,9 @@
>> #include <unistd.h>
>> +#include <pwd.h>
>> +#include <grp.h>
>> +#include <limits.h>
>> +#include <stdlib.h>
>> +
>> #include "crtools.h"
>> #include "proc_parse.h"
>> #include "log.h"
>> @@ -8,7 +13,9 @@
>> /*
>> * UID and GID of user requesting for C/R
>> */
>> -static unsigned int cr_uid, cr_gid;
>> +static unsigned int cr_uid;
>> +static unsigned int cr_gids[NGROUPS_MAX];
>> +static int cr_gids_num;
>>
>> /*
>> * Setup what user is requesting for dump (via rpc or using
>> @@ -17,21 +24,68 @@ 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)
>> {
>> + 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) {
>> + pr_perror("Can't get group list of the user");
>> + return -1;
>> + }
>> +
>> + return 0;
>> }
>>
>> -static bool check_ids(unsigned int crid, unsigned int rid, unsigned int
>> eid, unsigned int sid)
>> +static bool check_uids(unsigned int crid, unsigned int rid, unsigned
>> int eid, unsigned int sid)
>> {
>> if (crid == 0)
>> return true;
>> if (crid == rid && crid == eid && crid == sid)
>> return true;
>>
>> - pr_err("UID/GID mismatch %u != (%u,%u,%u)\n", crid, rid, eid, sid);
>> + pr_err("UID mismatch %u != (%u,%u,%u)\n", crid, rid, eid, sid);
>> + return false;
>> +}
>> +
>> +static bool contains(unsigned int *crgids, unsigned int crgids_num,
>> unsigned int gid)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < crgids_num; ++i) {
>> + if (crgids[i] == gid)
>> + return true;
>> + }
>> +
>> + return false;
>> +}
>> +
>> +static bool check_gids(unsigned int cruid,
>> + unsigned int *crgids,
>> + unsigned int crgids_num,
>> + unsigned int rid,
>> + unsigned int eid,
>> + unsigned int sid)
>> +{
>> + if (contains(crgids, crgids_num, 0))
>> + return true;
>> +
>> + if (contains(crgids, crgids_num, rid) &&
>> + contains(crgids, crgids_num, eid) &&
>> + contains(crgids, crgids_num, sid))
>> + return true;
>> +
>> + pr_err("GID mismatch. User(uid %u, primary gid %u) is absent in
>> (%u,%u,%u)",
>> + cruid, crgids[0], rid, eid, sid);
>> return false;
>> }
>>
>> @@ -46,7 +100,7 @@ static bool check_caps(u32 *inh, u32 *eff, u32 *prm)
>> * security model.
>> */
>>
>> - if (cr_uid == 0 && cr_gid == 0)
>> + if (cr_uid == 0 && contains(cr_gids, cr_gids_num, 0))
>> return true;
>>
>> for (i = 0; i < CR_CAP_SIZE; i++) {
>> @@ -61,14 +115,14 @@ static bool check_caps(u32 *inh, u32 *eff, u32 *prm)
>>
>> bool may_dump(struct proc_status_creds *creds)
>> {
>> - return check_ids(cr_uid, creds->uids[0], creds->uids[1],
>> creds->uids[2]) &&
>> - check_ids(cr_gid, creds->gids[0], creds->gids[1],
>> creds->gids[2]) &&
>> + return check_uids(cr_uid, creds->uids[0], creds->uids[1],
>> creds->uids[2]) &&
>> + check_gids(cr_uid, cr_gids, cr_gids_num, creds->gids[0],
>> creds->gids[1], creds->gids[2]) &&
>> check_caps(creds->cap_inh, creds->cap_eff, creds->cap_prm);
>> }
>>
>> bool may_restore(CredsEntry *creds)
>> {
>> - return check_ids(cr_uid, creds->uid, creds->euid, creds->suid) &&
>> - check_ids(cr_gid, creds->gid, creds->egid, creds->sgid) &&
>> + return check_uids(cr_uid, creds->uid, creds->euid, creds->suid) &&
>> + check_gids(cr_uid, cr_gids, cr_gids_num, creds->gid,
>> creds->egid, creds->sgid) &&
>> check_caps(creds->cap_inh, creds->cap_eff, creds->cap_prm);
>> }
>>
More information about the CRIU
mailing list