Commit 2b268c6c authored by Ruslan Kuprieiev's avatar Ruslan Kuprieiev Committed by Pavel Emelyanov

security: check additional groups,v5

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.
Signed-off-by: 's avatarRuslan Kuprieiev <kupruser@gmail.com>
Signed-off-by: 's avatarPavel Emelyanov <xemul@parallels.com>
parent a1e2da9b
...@@ -178,7 +178,8 @@ static int setup_opts_from_req(int sk, CriuOpts *req) ...@@ -178,7 +178,8 @@ static int setup_opts_from_req(int sk, CriuOpts *req)
return -1; return -1;
} }
restrict_uid(ids.uid, ids.gid); if (restrict_uid(ids.uid, ids.gid))
return -1;
if (fstat(sk, &st)) { if (fstat(sk, &st)) {
pr_perror("Can't get socket stat"); pr_perror("Can't get socket stat");
......
...@@ -173,7 +173,8 @@ int main(int argc, char *argv[]) ...@@ -173,7 +173,8 @@ int main(int argc, char *argv[])
BUILD_BUG_ON(PAGE_SIZE != PAGE_IMAGE_SIZE); BUILD_BUG_ON(PAGE_SIZE != PAGE_IMAGE_SIZE);
cr_pb_init(); cr_pb_init();
restrict_uid(getuid(), getgid()); if (restrict_uid(getuid(), getgid()))
return 1;
if (argc < 2) if (argc < 2)
goto usage; goto usage;
......
...@@ -24,7 +24,7 @@ extern int cr_check(void); ...@@ -24,7 +24,7 @@ extern int cr_check(void);
extern int cr_exec(int pid, char **opts); extern int cr_exec(int pid, char **opts);
extern int cr_dedup(void); 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; struct proc_status_creds;
extern bool may_dump(struct proc_status_creds *); extern bool may_dump(struct proc_status_creds *);
struct _CredsEntry; struct _CredsEntry;
......
#include <unistd.h> #include <unistd.h>
#include <pwd.h>
#include <grp.h>
#include <limits.h>
#include <stdlib.h>
#include <string.h>
#include "crtools.h" #include "crtools.h"
#include "proc_parse.h" #include "proc_parse.h"
#include "log.h" #include "log.h"
#include "xmalloc.h"
#include "bug.h"
#include "protobuf/creds.pb-c.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_uid, cr_gid;
static unsigned int cr_ngroups, *cr_groups;
/* /*
* Setup what user is requesting for dump (via rpc or using * Setup what user is requesting for dump (via rpc or using
...@@ -17,11 +26,36 @@ static unsigned int cr_uid, cr_gid; ...@@ -17,11 +26,36 @@ static unsigned int cr_uid, cr_gid;
* access to. (Or implement some trickier security policy). * 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_uid = uid;
cr_gid = gid; cr_gid = gid;
pwd = getpwuid(uid);
if (!pwd) {
pr_perror("Can't get password file entry");
return -1;
}
nbuf = NGROUPS_MAX;
if (getgrouplist(pwd->pw_name, pwd->pw_gid, buf, &nbuf) < 0) {
pr_perror("Can't get group list");
return -1;
}
cr_ngroups = nbuf;
cr_groups = xmalloc(cr_ngroups*sizeof(*cr_groups));
if (!cr_groups)
return -1;
memcpy(cr_groups, buf, cr_ngroups*sizeof(*cr_groups));
return 0;
} }
static bool check_ids(unsigned int crid, unsigned int rid, unsigned int eid, unsigned int sid) static bool check_ids(unsigned int crid, unsigned int rid, unsigned int eid, unsigned int sid)
...@@ -35,6 +69,59 @@ static bool check_ids(unsigned int crid, unsigned int rid, unsigned int eid, uns ...@@ -35,6 +69,59 @@ static bool check_ids(unsigned int crid, unsigned int rid, unsigned int eid, uns
return false; 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 rid, unsigned int eid, unsigned int sid)
{
if (cr_gid == 0)
return true;
if ((contains(cr_groups, cr_ngroups, rid) || cr_gid == rid) &&
(contains(cr_groups, cr_ngroups, eid) || cr_gid == eid) &&
(contains(cr_groups, cr_ngroups, sid) || cr_gid == sid))
return true;
pr_err("GID mismatch. User is absent in (%u,%u,%u)\n", rid, eid, sid);
return false;
}
/*
* There is no need to check groups on dump, because if uids and gids match
* then groups will match too. Btw, getting groups on dump is problematic.
* We can't parse proc, as it contains only first 32 groups. And we can't use
* getgrouplist, as it reads /etc/group which depends on the namespace.
*
* On restore we're getting groups from imgs and can check if user didn't add
* wrong groups by modifying images.
*/
static bool check_groups(unsigned int *groups, unsigned int ngroups)
{
int i;
if (cr_gid == 0)
return true;
for (i = 0; i < ngroups; ++i) {
if (!contains(cr_groups, cr_ngroups, groups[i])) {
pr_err("GID mismatch. User is absent in %u group\n",
groups[i]);
return false;
}
}
return true;
}
static bool check_caps(u32 *inh, u32 *eff, u32 *prm) static bool check_caps(u32 *inh, u32 *eff, u32 *prm)
{ {
int i; int i;
...@@ -62,13 +149,14 @@ static bool check_caps(u32 *inh, u32 *eff, u32 *prm) ...@@ -62,13 +149,14 @@ static bool check_caps(u32 *inh, u32 *eff, u32 *prm)
bool may_dump(struct proc_status_creds *creds) bool may_dump(struct proc_status_creds *creds)
{ {
return check_ids(cr_uid, creds->uids[0], creds->uids[1], creds->uids[2]) && 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]) && check_gids(creds->gids[0], creds->gids[1], creds->gids[2]) &&
check_caps(creds->cap_inh, creds->cap_eff, creds->cap_prm); check_caps(creds->cap_inh, creds->cap_eff, creds->cap_prm);
} }
bool may_restore(CredsEntry *creds) bool may_restore(CredsEntry *creds)
{ {
return check_ids(cr_uid, creds->uid, creds->euid, creds->suid) && return check_ids(cr_uid, creds->uid, creds->euid, creds->suid) &&
check_ids(cr_gid, creds->gid, creds->egid, creds->sgid) && check_gids(creds->gid, creds->egid, creds->sgid) &&
check_groups(creds->groups, creds->n_groups) &&
check_caps(creds->cap_inh, creds->cap_eff, creds->cap_prm); check_caps(creds->cap_inh, creds->cap_eff, creds->cap_prm);
} }
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment