[CRIU] [PATCH 01/11] criu: Remove security

Pavel Emelyanov xemul at parallels.com
Tue Dec 15 11:23:54 PST 2015


We no longer support root-mode service and suid binaries, so
any artificial restrictions no longer make sense.

Signed-off-by: Pavel Emelyanov <xemul at parallels.com>
---
 Makefile.crtools   |   1 -
 cr-dump.c          |   1 -
 cr-restore.c       |   6 --
 cr-service.c       |   5 +-
 crtools.c          |   5 +-
 include/security.h |  13 ----
 log.c              |  13 ----
 ptrace.c           |   6 --
 security.c         | 184 -----------------------------------------------------
 9 files changed, 2 insertions(+), 232 deletions(-)
 delete mode 100644 include/security.h
 delete mode 100644 security.c

diff --git a/Makefile.crtools b/Makefile.crtools
index 77a7421..5788ef0 100644
--- a/Makefile.crtools
+++ b/Makefile.crtools
@@ -3,7 +3,6 @@ obj-y	+= mem.o
 obj-y	+= rst-malloc.o
 obj-y	+= cr-restore.o
 obj-y	+= crtools.o
-obj-y	+= security.o
 obj-y	+= image.o
 obj-y	+= image-desc.o
 obj-y	+= net.o
diff --git a/cr-dump.c b/cr-dump.c
index 5cc375d..bd7b446 100644
--- a/cr-dump.c
+++ b/cr-dump.c
@@ -77,7 +77,6 @@
 #include "sysfs_parse.h"
 #include "action-scripts.h"
 #include "aio.h"
-#include "security.h"
 #include "lsm.h"
 #include "seccomp.h"
 #include "seize.h"
diff --git a/cr-restore.c b/cr-restore.c
index 3c636b9..84e2ae2 100644
--- a/cr-restore.c
+++ b/cr-restore.c
@@ -73,7 +73,6 @@
 #include "file-lock.h"
 #include "action-scripts.h"
 #include "aio.h"
-#include "security.h"
 #include "lsm.h"
 #include "seccomp.h"
 #include "bitmap.h"
@@ -2375,11 +2374,6 @@ static CredsEntry *read_creds(int pid)
 		return NULL;
 	}
 
-	if (!may_restore(ce)) {
-		creds_entry__free_unpacked(ce, NULL);
-		return NULL;
-	}
-
 	return ce;
 }
 
diff --git a/cr-service.c b/cr-service.c
index 003f536..1f903b6 100644
--- a/cr-service.c
+++ b/cr-service.c
@@ -27,10 +27,10 @@
 #include "mount.h"
 #include "cgroup.h"
 #include "action-scripts.h"
-#include "security.h"
 #include "sockets.h"
 #include "irmap.h"
 #include "kerndat.h"
+#include "proc_parse.h"
 
 #include "setproctitle.h"
 
@@ -233,9 +233,6 @@ static int setup_opts_from_req(int sk, CriuOpts *req)
 		goto err;
 	}
 
-	if (restrict_uid(ids.uid, ids.gid))
-		goto err;
-
 	if (fstat(sk, &st)) {
 		pr_perror("Can't get socket stat");
 		goto err;
diff --git a/crtools.c b/crtools.c
index f357c0c..577cdeb 100644
--- a/crtools.c
+++ b/crtools.c
@@ -38,10 +38,10 @@
 #include "cgroup.h"
 #include "cpu.h"
 #include "action-scripts.h"
-#include "security.h"
 #include "irmap.h"
 #include "fault-injection.h"
 #include "lsm.h"
+#include "proc_parse.h"
 
 #include "setproctitle.h"
 
@@ -264,9 +264,6 @@ int main(int argc, char *argv[], char *envp[])
 		return 1;
 
 	cr_pb_init();
-	if (restrict_uid(getuid(), getgid()))
-		return 1;
-
 	setproctitle_init(argc, argv, envp);
 
 	if (argc < 2)
diff --git a/include/security.h b/include/security.h
deleted file mode 100644
index b21c8d9..0000000
--- a/include/security.h
+++ /dev/null
@@ -1,13 +0,0 @@
-#ifndef __CR_SECURITY_H__
-#define __CR_SECURITY_H__
-
-#include "proc_parse.h"
-#include "protobuf/creds.pb-c.h"
-
-extern int restrict_uid(unsigned int uid, unsigned int gid);
-extern bool may_dump(struct proc_status_creds *);
-extern bool may_restore(struct _CredsEntry *);
-extern bool cr_user_is_root(void);
-extern int cr_fchown(int fd);
-
-#endif /* __CR_SECURITY_H__ */
diff --git a/log.c b/log.c
index 448f854..1435401 100644
--- a/log.c
+++ b/log.c
@@ -17,7 +17,6 @@
 #include "util.h"
 #include "cr_options.h"
 #include "servicefd.h"
-#include "security.h"
 
 #define DEFAULT_LOGFD		STDERR_FILENO
 /* Enable timestamps if verbosity is increased from default */
@@ -86,12 +85,6 @@ int log_init(const char *output)
 			pr_perror("Can't create log file %s", output);
 			return -1;
 		}
-
-		if (cr_fchown(new_logfd)) {
-			pr_perror("Can't chown log file %s", output);
-			close(new_logfd);
-			return -1;
-		}
 	} else {
 		new_logfd = dup(DEFAULT_LOGFD);
 		if (new_logfd < 0) {
@@ -200,12 +193,6 @@ int write_pidfile(int pid)
 		return -1;
 	}
 
-	if (cr_fchown(fd)) {
-		pr_perror("Can't chown pidfile %s", opts.pidfile);
-		close(fd);
-		return -1;
-	}
-
 	dprintf(fd, "%d", pid);
 	close(fd);
 	return 0;
diff --git a/ptrace.c b/ptrace.c
index 3f24afb..4612ba3 100644
--- a/ptrace.c
+++ b/ptrace.c
@@ -20,7 +20,6 @@
 #include "ptrace.h"
 #include "proc_parse.h"
 #include "crtools.h"
-#include "security.h"
 #include "seccomp.h"
 
 int unseize_task(pid_t pid, int orig_st, int st)
@@ -188,11 +187,6 @@ try_again:
 	if (ret2)
 		goto err;
 
-	if (!may_dump(&cr)) {
-		pr_err("Check uid (pid: %d) failed\n", pid);
-		goto err;
-	}
-
 	if (ret < 0 || WIFEXITED(status) || WIFSIGNALED(status)) {
 		if (cr.state != 'Z') {
 			if (pid == getpid())
diff --git a/security.c b/security.c
deleted file mode 100644
index 693c575..0000000
--- a/security.c
+++ /dev/null
@@ -1,184 +0,0 @@
-#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, 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
- * suid bit on crtools). Later we would deny to dump/restore
- * a task, to which the original user doesn't have the direct
- * access to. (Or implement some trickier security policy).
- */
-
-int restrict_uid(unsigned int uid, unsigned int 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;
-
-	/* skip obtaining additional groups for root, as they don't matter */
-	if (cr_uid == 0 && cr_gid == 0)
-		return 0;
-
-	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_uids(unsigned int rid, unsigned int eid, unsigned int sid)
-{
-	if (cr_uid == 0)
-		return true;
-	if (cr_uid == rid && cr_uid == eid && cr_uid == sid)
-		return true;
-
-	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)
-{
-	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)
-{
-	int i;
-
-	/*
-	 * Impose the most strict requirements for now.
-	 * "Real" root user can use any caps, other users may
-	 * use none. Later we will implement more sophisticated
-	 * security model.
-	 */
-
-	if (cr_uid == 0 && cr_gid == 0)
-		return true;
-
-	for (i = 0; i < CR_CAP_SIZE; i++) {
-		if (inh[i] != 0 || eff[i] != 0 || prm[i] != 0) {
-			pr_err("CAPs not allowed for non-root user\n");
-			return false;
-		}
-	}
-
-	return true;
-}
-
-bool cr_user_is_root()
-{
-	return cr_uid == 0 && cr_gid == 0;
-}
-
-bool may_dump(struct proc_status_creds *creds)
-{
-	return check_uids(creds->uids[0], creds->uids[1], creds->uids[2]) &&
-		check_gids(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_uids(creds->uid, creds->euid, creds->suid) &&
-		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);
-}
-
-int cr_fchown(int fd)
-{
-	if (cr_user_is_root())
-		return 0;
-
-	if (fchown(fd, cr_uid, cr_gid)) {
-		pr_perror("Can't chown to (%u,%u)", cr_uid, cr_gid);
-		return -1;
-	}
-
-	return 0;
-}
-- 
1.9.3




More information about the CRIU mailing list