[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