[CRIU] [PATCH] security: check additional groups
Ruslan Kuprieiev
kupruser at gmail.com
Mon Jun 30 07:35:24 PDT 2014
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) check that user has all groups from the process grouplist.
Signed-off-by: Ruslan Kuprieiev <kupruser at gmail.com>
---
cr-service.c | 3 +-
crtools.c | 3 +-
include/crtools.h | 2 +-
include/proc_parse.h | 2 ++
proc_parse.c | 34 +++++++++++++++++--
security.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++------
6 files changed, 122 insertions(+), 16 deletions(-)
diff --git a/cr-service.c b/cr-service.c
index 64ce751..9504c14 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;
if (fstat(sk, &st)) {
pr_perror("Can't get socket stat");
diff --git a/crtools.c b/crtools.c
index c3a2e27..c184dba 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/include/proc_parse.h b/include/proc_parse.h
index b153328..a13ec14 100644
--- a/include/proc_parse.h
+++ b/include/proc_parse.h
@@ -83,6 +83,8 @@ struct proc_pid_stat {
struct proc_status_creds {
unsigned int uids[4];
unsigned int gids[4];
+ unsigned int *groups;
+ unsigned int ngroups;
u32 cap_inh[PROC_CAP_SIZE];
u32 cap_prm[PROC_CAP_SIZE];
diff --git a/proc_parse.c b/proc_parse.c
index f2ea897..2712432 100644
--- a/proc_parse.c
+++ b/proc_parse.c
@@ -689,6 +689,29 @@ static int ids_parse(char *str, unsigned int *arr)
return 0;
}
+static int groups_parse(char *str, unsigned int **arr, unsigned int *arr_size)
+{
+ char *end;
+ unsigned int size = 0;
+ unsigned int *buf = NULL;
+
+ end = str - 1;
+ /* "Groups:" string has a space before '\n' */
+ while (*end != ' ' && *(end+1) != '\n') {
+ size++;
+ buf = xrealloc(buf, size*sizeof(*buf));
+ if (!buf)
+ return -1;
+
+ buf[size-1] = strtol(end + 1, &end, 10);
+ }
+
+ *arr = buf;
+ *arr_size = size;
+
+ return 0;
+}
+
static int cap_parse(char *str, unsigned int *res)
{
int i, ret;
@@ -715,7 +738,7 @@ int parse_pid_status(pid_t pid, struct proc_status_creds *cr)
return -1;
}
- while (done < 6 && fgets(str, sizeof(str), f)) {
+ while (done < 7 && fgets(str, sizeof(str), f)) {
if (!strncmp(str, "Uid:", 4)) {
if (ids_parse(str + 5, cr->uids))
goto err_parse;
@@ -730,6 +753,13 @@ int parse_pid_status(pid_t pid, struct proc_status_creds *cr)
done++;
}
+ if (!strncmp(str, "Groups:", 7)) {
+ if (groups_parse(str + 8, &cr->groups, &cr->ngroups))
+ goto err_parse;
+
+ done++;
+ }
+
if (!strncmp(str, "CapInh:", 7)) {
if (cap_parse(str + 8, cr->cap_inh))
goto err_parse;
@@ -759,7 +789,7 @@ int parse_pid_status(pid_t pid, struct proc_status_creds *cr)
}
}
- if (done != 6) {
+ if (done != 7) {
err_parse:
pr_err("Error parsing proc status file\n");
fclose(f);
diff --git a/security.c b/security.c
index d4b4230..bbda294 100644
--- a/security.c
+++ b/security.c
@@ -1,14 +1,22 @@
#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 "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,24 +25,88 @@ 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;
+ }
+
+ 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_uids(unsigned int rid, unsigned int eid, unsigned int sid)
{
- if (crid == 0)
+ if (cr_uid == 0)
return true;
- if (crid == rid && crid == eid && crid == sid)
+ if (cr_uid == rid && cr_uid == eid && cr_uid == 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", cr_uid, 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 rid, unsigned int eid, unsigned int sid,
+ unsigned int *groups, unsigned int ngroups)
+{
+ int i;
+
+ if (cr_gid == 0)
+ return true;
+
+ if (!(contains(cr_groups, cr_ngroups, rid) &&
+ contains(cr_groups, cr_ngroups, eid) &&
+ contains(cr_groups, cr_ngroups, sid))) {
+ pr_err("GID mismatch. User is absent in (%u,%u,%u)",
+ rid, eid, sid);
+ return false;
+ }
+
+ 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)
{
int i;
@@ -61,14 +133,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(creds->uids[0], creds->uids[1], creds->uids[2]) &&
+ check_gids(creds->gids[0], creds->gids[1], creds->gids[2], creds->groups, creds->ngroups) &&
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(creds->uid, creds->euid, creds->suid) &&
+ check_gids(creds->gid, creds->egid, creds->sgid, creds->groups, creds->n_groups) &&
check_caps(creds->cap_inh, creds->cap_eff, creds->cap_prm);
}
--
1.8.3.2
More information about the CRIU
mailing list