[PATCH] security: check additional gids

[Ruslan Kuprieiev]

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;
 
 	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);
 }
-- 
2.0.0


--------------070100020900040705030107--