cr: add generic LSM c/r support (v4)

The checkpoint file header now has an 11-character string containing the name of the active LSM, following the uts info, and a variable length buffer type conaining LSM-specific version information (for instance a sha1sum of policy). Handle these.

Also add a -k (--keeplsm) flag to tell restart to set the RESTART_KEEP_LSM flag to sys_restart().

Signed-off-by Serge E. Hallyn <serue-r/[email protected]>

---
 restart.c |   32 +++++++++++++++++++++++++++++++-
 1 files changed, 31 insertions(+), 1 deletions(-)

restart.c +1 -0

index b810ca9..44ae252 100644
@@ -68,6 +68,7 @@ static char usage_str[] =
 "     --signal=SIG       send SIG to root task on SIGINT (default: SIGKILL\n"
 "                        to container root, SIGINT otherwise)\n"
 "  -w,--wait             wait for root task to termiate (default)\n"
+"  -k,--keeplsm          Try to recreate original LSM labels on all objects\n"
 "     --show-status      show exit status of root task (implies -w)\n"
 "     --copy-status      imitate exit status of root task (implies -w)\n"
 "  -W,--no-wait          do not wait for root task to terminate\n"

restart.c +8 -1

+
 static void usage(char *str)
 {
 	fprintf(stderr, "%s", str);

@@ -377,6 +380,7 @@ static void parse_args(struct args *args, int argc, char *argv[])
 		{ "self",	no_argument,		NULL, 6},
 		{ "signal",	required_argument,	NULL, 4 },
 		{ "inspect",	no_argument,		NULL, 5 },
+		{ "keeplsm",	no_argument,		NULL, 'k' },
 		{ "input",	required_argument,	NULL, 'i' },
 		{ "root",	required_argument,	NULL, 'r' },
 		{ "wait",	no_argument,		NULL, 'w' },

@@ -388,7 +392,7 @@ static void parse_args(struct args *args, int argc, char *argv[])
 		{ "debug",	no_argument,		NULL, 'd' },
 		{ NULL,		0,			NULL, 0 }
 	};
-	static char optc[] = "hdvpPwWF:r:i:";
+	static char optc[] = "hdvpkPwWF:r:i:";
 
 	int sig;
 

@@ -443,6 +447,9 @@ static void parse_args(struct args *args, int argc, char *argv[])
 		case 'w':
 			args->wait = 1;
 			break;
+		case 'k':
+			keep_lsm = RESTART_KEEP_LSM;
+			break;
 		case 'W':
 			args->wait = 0;
 			break;

@@ -927,6 +934,7 @@ static int ckpt_coordinator(struct ckpt_ctx *ctx)
 	if (ctx->args->freezer)
 		flags |= RESTART_FROZEN;
 
+	flags |= keep_lsm;
 	ret = restart(root_pid, STDIN_FILENO, flags);
 
 	if (ret < 0) {

@@ -1581,6 +1589,7 @@ static int ckpt_make_tree(struct ckpt_ctx *ctx, struct task *task)
 	if (task->flags & (TASK_GHOST | TASK_DEAD))
 		flags |= RESTART_GHOST;
 
+	flags |= keep_lsm;
 	/* on success this doesn't return */
 	ckpt_dbg("about to call sys_restart(), flags %#lx\n", flags);
 	ret = restart(0, STDIN_FILENO, flags);

restart.c +20 -0

 static int ckpt_read_header(struct ckpt_ctx *ctx)
 {
 	struct ckpt_hdr_header *h;

@@ -2090,6 +2100,16 @@ static int ckpt_read_header(struct ckpt_ctx *ctx)
 	if (ret < 0)
 		return ret;
 
+	ptr += ((struct ckpt_hdr *) ptr)->len;
+	ret = ckpt_read_obj_buffer(ctx, ptr, SECURITY_NAME_MAX + 1);
+	if (ret < 0)
+		return ret;
+
+	ptr += ((struct ckpt_hdr *) ptr)->len;
+	ret = ckpt_read_obj_type(ctx, ptr, 200, CKPT_HDR_LSM_INFO);
+	if (ret < 0)
+		return ret;
+
 	/* FIXME: skip version validation for now */
 
 	return 0;

@@ -2169,6 +2189,16 @@ static int ckpt_write_header(struct ckpt_ctx *ctx)
 	ptr += ((struct ckpt_hdr *) ptr)->len;
 	ret = ckpt_write_obj(ctx, (struct ckpt_hdr *) ptr);
 
+	if (ret < 0)
+		return ret;
+	ptr += ((struct ckpt_hdr *) ptr)->len;
+	ret = ckpt_write_obj(ctx, (struct ckpt_hdr *) ptr);
+	if (ret < 0)
+		return ret;
+
+	ptr += ((struct ckpt_hdr *) ptr)->len;
+	ret = ckpt_write_obj(ctx, (struct ckpt_hdr *) ptr);
+
 	return ret;
 }
 

(wasn't versioning the patchsets before, so randomly pick 4 as the version for this patchset...)

Documentation/checkpoint/readme.txt begins:
"""
Application checkpoint/restart is the ability to save the state of a running application so that it can later resume its execution from the time at which it was checkpointed.
"""

Kernel interfaces
=================

checkpoint/checkpoint.c +1 -0

index 1eeb557..6722035 100644
@@ -25,6 +25,7 @@
 #include <linux/magic.h>
 #include <linux/hrtimer.h>
 #include <linux/deferqueue.h>
+#include <linux/security.h>
 #include <linux/checkpoint.h>
 #include <linux/checkpoint_hdr.h>
 

@@ -351,6 +352,16 @@ static int checkpoint_write_header(struct ckpt_ctx *ctx)
 	if (ret < 0)
 		return ret;
 

checkpoint/checkpoint.c +0 -0

 	return checkpoint_write_header_arch(ctx);
 }
 

checkpoint/files.c +13 -0

index 27e29a0..570facb 100644
@@ -145,6 +145,19 @@ static int scan_fds(struct files_struct *files, int **fdtable)
 	return n;
 }
 
+#ifdef CONFIG_SECURITY
+int checkpoint_file_security(struct ckpt_ctx *ctx, struct file *file)
+{
+	return security_checkpoint_obj(ctx, file->f_security,
+					CKPT_SECURITY_FILE);
+}
+#else
+int checkpoint_file_security(struct ckpt_ctx *ctx, struct file *file)
+{
+	return -EOPNOTSUPP;
+}
+#endif
+
 int checkpoint_file_common(struct ckpt_ctx *ctx, struct file *file,
 			   struct ckpt_hdr_file *h)
 {

checkpoint/files.c +2 -0

+	else if (h->f_secref < 0)
+		return h->f_secref;

checkpoint/files.c +17 -0

+	h = ckpt_hdr_get_type(ctx, sizeof(*h)+len+1, CKPT_HDR_SEC);
+	if (!h)
+		return -ENOMEM;
+	h->len = len;
+	h->sectype = l->sectype;
+	strncpy(h->string, l->string, len);
+	h->string[len] = '\0';
+	ret = ckpt_write_obj(ctx, &h->h);
+	ckpt_hdr_put(ctx, h);
+	return ret;
+}
+
+static int checkpoint_security(struct ckpt_ctx *ctx, void *ptr)
+{
+	return do_checkpoint_security(ctx, (struct ckpt_stored_lsm *) ptr);
+}
+

checkpoint/files.c +10 -1

+		ckpt_debug("restore_task_creds returned %d", ret);
+		return ret;
+	}
+	ret = restore_task_ns(ctx);
+	if (ret) {
+		ckpt_debug("restore_task_ns returned %d", ret);
 		return ret;
+	}
 
 	h = ckpt_read_obj_type(ctx, sizeof(*h), CKPT_HDR_TASK_OBJS);
-	if (IS_ERR(h))
+	if (IS_ERR(h)) {
+		ckpt_debug("Error fetching task obj");
 		return PTR_ERR(h);
+	}
 
 	ret = restore_obj_file_table(ctx, h->files_objref);
 	ckpt_debug("file_table: ret %d (%p)\n", ret, current->files);

checkpoint/restart.c +0 -0

index b12c8bd..bba2b45 100644
@@ -457,6 +457,31 @@ static int restore_read_header(struct ckpt_ctx *ctx)
 	if (ret < 0)
 		goto out;

checkpoint/restart.c +6 -0

+	/*
+	 * This is followed by a string of size len+1,
+	 * null-terminated
+	 */
+	__u8 string[0];
+} __attribute__((aligned(8)));
 /*
  * todo - keyrings and LSM
  * These may be better done with userspace help though

checkpoint/restart.c +43 -0

+/**
+ * security_checkpoint_obj - if first checkpoint of this void* security,
+ * then 1. ask the LSM for a string representing the context, 2. checkpoint
+ * that string
+ * @ctx: the checkpoint context
+ * @security: the void* security being checkpointed
+ * @sectype: indicates the type of object, because LSMs can (and do) store
+ * different types of data for different types of objects.
+ *
+ * Returns the objref of the checkpointed ckpt_stored_lsm representing the
+ * context, or -error on error.
+ *
+ * This is only used at checkpoint of course.
+ */
+int security_checkpoint_obj(struct ckpt_ctx *ctx, void *security,
+				unsigned sectype)
+{
+	int strref;
+	struct ckpt_stored_lsm *l;
+	char *str;
+	int len;
+
+	/*
+	 * to simplify the LSM code, short-cut a null security
+	 * here.
+	 */
+	if (!security)
+		return -EOPNOTSUPP;
+
+	switch (sectype) {
+	case CKPT_SECURITY_MSG_MSG:
+		str = security_msg_msg_checkpoint(security);
+		break;
+	case CKPT_SECURITY_IPC:
+		str = security_ipc_checkpoint(security);
+		break;
+	case CKPT_SECURITY_FILE:
+		str = security_file_checkpoint(security);
+		break;
+	case CKPT_SECURITY_CRED:
+		str = security_cred_checkpoint(security);
+		break;
+	default:

checkpoint/restart.c +4 -0

+	if (!l) {
+		kfree(str);
+		return -ENOMEM;
+	}

Quoted 10 lines · Serge E. Hallyn

(wasn't versioning the patchsets before, so randomly pick 4 as the version for this patchset...)

Documentation/checkpoint/readme.txt begins:
"""
Application checkpoint/restart is the ability to save the state of a running application so that it can later resume its execution from the time at which it was checkpointed.
"""

[...]

Quoted 18 lines

Kernel interfaces
=================

checkpoint/checkpoint.c +1 -0

index 1eeb557..6722035 100644
@@ -25,6 +25,7 @@
 #include <linux/magic.h>
 #include <linux/hrtimer.h>
 #include <linux/deferqueue.h>
+#include <linux/security.h>
 #include <linux/checkpoint.h>
 #include <linux/checkpoint_hdr.h>
 

@@ -351,6 +352,16 @@ static int checkpoint_write_header(struct ckpt_ctx *ctx)
 	if (ret < 0)
 		return ret;
 

checkpoint/checkpoint.c static int checkpoint_write_header(struct ckpt_ctx *ctx) · line 364

Quoted 10 lines

checkpoint/checkpoint.c +10 -0

+	memset(ctx->lsm_name, 0, SECURITY_NAME_MAX + 1);
+	strlcpy(ctx->lsm_name, security_get_lsm_name(), SECURITY_NAME_MAX + 1);
+	ret = ckpt_write_buffer(ctx, ctx->lsm_name, SECURITY_NAME_MAX + 1);
+	if (ret < 0)
+		return ret;
+
+	ret = security_checkpoint_header(ctx);
+	if (ret < 0)
+		return ret;
+

This is actually a case for a 'container-global' section that would appear after the header and before the rest of the image. (Would be useful also for network namespaces).

Quoted 31 lines

checkpoint/checkpoint.c +0 -0

 	return checkpoint_write_header_arch(ctx);
 }
 

checkpoint/files.c +13 -0

index 27e29a0..570facb 100644
@@ -145,6 +145,19 @@ static int scan_fds(struct files_struct *files, int **fdtable)
 	return n;
 }
 
+#ifdef CONFIG_SECURITY
+int checkpoint_file_security(struct ckpt_ctx *ctx, struct file *file)
+{
+	return security_checkpoint_obj(ctx, file->f_security,
+					CKPT_SECURITY_FILE);
+}
+#else
+int checkpoint_file_security(struct ckpt_ctx *ctx, struct file *file)
+{
+	return -EOPNOTSUPP;
+}
+#endif
+
 int checkpoint_file_common(struct ckpt_ctx *ctx, struct file *file,
 			   struct ckpt_hdr_file *h)
 {

@@ -159,6 +172,16 @@ int checkpoint_file_common(struct ckpt_ctx *ctx, struct file *file,
 	if (h->f_credref < 0)
 		return h->f_credref;
 

checkpoint/files.c int checkpoint_file_common(struct ckpt_ctx *ctx, struct file *file, · line 177

Quoted 3 lines

checkpoint/files.c +3 -0

+	h->f_secref = checkpoint_file_security(ctx, file);
+	if (h->f_secref == -EOPNOTSUPP)
+		h->f_secref = -1;

#define CKPT_SECURITY_NONE -1 ??

Quoted 2 lines

checkpoint/files.c +2 -0

+	else if (h->f_secref < 0)
+		return h->f_secref;

[...]

checkpoint/files.c

Quoted 10 lines

checkpoint/files.c +10 -0

+
+static int do_checkpoint_security(struct ckpt_ctx *ctx,
+				const struct ckpt_stored_lsm *l)
+{
+	int ret, len;
+	struct ckpt_hdr_lsm *h;
+
+	len = strlen(l->string);
+	if (len > PAGE_SIZE - sizeof(*h))
+		return -E2BIG;

A call to ckpt_write_err() here may be useful.

Quoted 22 lines

checkpoint/files.c +22 -0

+	h = ckpt_hdr_get_type(ctx, sizeof(*h)+len+1, CKPT_HDR_SEC);
+	if (!h)
+		return -ENOMEM;
+	h->len = len;
+	h->sectype = l->sectype;
+	strncpy(h->string, l->string, len);
+	h->string[len] = '\0';
+	ret = ckpt_write_obj(ctx, &h->h);
+	ckpt_hdr_put(ctx, h);
+	return ret;
+}
+
+static int checkpoint_security(struct ckpt_ctx *ctx, void *ptr)
+{
+	return do_checkpoint_security(ctx, (struct ckpt_stored_lsm *) ptr);
+}
+
+static struct ckpt_stored_lsm *do_restore_security(struct ckpt_ctx *ctx)
+{
+	struct ckpt_hdr_lsm *h;
+	struct ckpt_stored_lsm *l;
+

checkpoint/files.c

Quoted 5 lines

checkpoint/files.c +5 -0

+	h = ckpt_read_buf_type(ctx, PAGE_SIZE, CKPT_HDR_SEC);
+	if (IS_ERR(h))
+		return (struct ckpt_stored_lsm *)h;
+	if (h->len > h->h.len - sizeof(struct ckpt_hdr) ||
+				h->len > PAGE_SIZE) {

The second test (for h->len > PAGE_SIZE) is unnecessary - already done in ckpt_read_buf_type()

checkpoint/files.c

Quoted 7 lines

checkpoint/files.c +7 -0

+		ckpt_hdr_put(ctx, h);
+		return ERR_PTR(-EINVAL);
+	}
+	l = kzalloc(sizeof(*l), GFP_KERNEL);
+	if (!l) {
+		ckpt_hdr_put(ctx, h);
+		return ERR_PTR(-ENOMEM);

Nit: this part is common... set ret = -ENOMEM and use goto ?

[...]

checkpoint/files.c static int restore_task_objs(struct ckpt_ctx *ctx) · line 616

Quoted 8 lines

checkpoint/files.c +1 -3

@@ -608,14 +613,21 @@ static int restore_task_objs(struct ckpt_ctx *ctx)
 	 * referenced. See comment in checkpoint_task_objs.
 	 */
 	ret = restore_task_creds(ctx);
-	if (!ret)
-		ret = restore_task_ns(ctx);
-	if (ret < 0)
+	if (ret) {

Nit: (ret < 0) helps (me) read it.

Quoted 26 lines

checkpoint/files.c +10 -1

+		ckpt_debug("restore_task_creds returned %d", ret);
+		return ret;
+	}
+	ret = restore_task_ns(ctx);
+	if (ret) {
+		ckpt_debug("restore_task_ns returned %d", ret);
 		return ret;
+	}
 
 	h = ckpt_read_obj_type(ctx, sizeof(*h), CKPT_HDR_TASK_OBJS);
-	if (IS_ERR(h))
+	if (IS_ERR(h)) {
+		ckpt_debug("Error fetching task obj");
 		return PTR_ERR(h);
+	}
 
 	ret = restore_obj_file_table(ctx, h->files_objref);
 	ckpt_debug("file_table: ret %d (%p)\n", ret, current->files);

checkpoint/restart.c +0 -0

index b12c8bd..bba2b45 100644
@@ -457,6 +457,31 @@ static int restore_read_header(struct ckpt_ctx *ctx)
 	if (ret < 0)
 		goto out;
 

checkpoint/restart.c static int restore_read_header(struct ckpt_ctx *ctx) · line 468

Quoted 9 lines

checkpoint/restart.c +9 -0

+	ret = _ckpt_read_buffer(ctx, ctx->lsm_name, SECURITY_NAME_MAX + 1);
+	if (ret < 0)
+		goto out;
+	if (ctx->uflags & RESTART_KEEP_LSM) {
+		char *cur = security_get_lsm_name();
+		if (strncmp(cur, ctx->lsm_name, SECURITY_NAME_MAX + 1) != 0) {
+			pr_warning("c/r: checkpointed LSM %s, current is %s.\n",
+				ctx->lsm_name, cur);
+			ret = -EINVAL;

EPERM ?

[...]

checkpoint/restart.c struct ckpt_hdr_groupinfo { · line 289

Quoted 8 lines

checkpoint/restart.c +4 -0

@@ -277,6 +283,16 @@ struct ckpt_hdr_groupinfo {
 	__u32 groups[0];
 } __attribute__((aligned(8)));
 
+struct ckpt_hdr_lsm {
+	struct ckpt_hdr h;
+	__u8 sectype;
+	__u16 len;

Alignment ...

Quoted 10 lines

checkpoint/restart.c +6 -0

+	/*
+	 * This is followed by a string of size len+1,
+	 * null-terminated
+	 */
+	__u8 string[0];
+} __attribute__((aligned(8)));
 /*
  * todo - keyrings and LSM
  * These may be better done with userspace help though

checkpoint/restart.c struct ckpt_hdr_file { · line 412

Quoted 5 lines

checkpoint/restart.c +2 -0

@@ -392,6 +408,8 @@ struct ckpt_hdr_file {
 	__s32 f_credref;
 	__u64 f_pos;
 	__u64 f_version;
+	__s32 f_secref;
+	__u32 f_padding;

No need for padding here.

checkpoint/restart.c struct ckpt_hdr_ipc_perms { · line 656

Quoted 9 lines

checkpoint/restart.c +2 -0

 } __attribute__((aligned(8)));
 
 struct ckpt_hdr_file_generic {

@@ -634,6 +652,8 @@ struct ckpt_hdr_ipc_perms {
 	__u32 mode;
 	__u32 _padding;
 	__u64 seq;
+	__s32 sec_ref;
+	__u32 padding;

Ditto.

[...]

Quoted 4 lines

checkpoint/restart.c +0 -0

@@ -224,6 +234,16 @@ static struct msg_msg *restore_msg_contents_one(struct ckpt_ctx *ctx, int *clen)
 	msg->next = NULL;
 	pseg = &msg->next;
 

checkpoint/restart.c static struct msg_msg *restore_msg_contents_one(struct ckpt_ctx *ctx, int *clen) · line 241

Quoted 5 lines

checkpoint/restart.c +5 -0

+	if ((ctx->uflags & RESTART_KEEP_LSM) && (h->sec_ref != -1)) {
+		struct ckpt_stored_lsm *l = ckpt_obj_fetch(ctx, h->sec_ref,
+					CKPT_OBJ_SEC);
+		if (IS_ERR(l))
+			return (struct msg_msg *)l;

That's a leak. Need to 'goto out' instead.

checkpoint/restart.c static struct msg_msg *restore_msg_contents_one(struct ckpt_ctx *ctx, int *clen) · line 244

Quoted 3 lines

checkpoint/restart.c +3 -0

+		ret = security_msg_msg_restore(msg, l->string);
+		if (ret)
+			return ERR_PTR(ret);

Here too.

[...]

checkpoint/restart.c static struct cred *do_restore_cred(struct ckpt_ctx *ctx) · line 822

Quoted 9 lines

checkpoint/restart.c +5 -0

@@ -806,6 +815,15 @@ static struct cred *do_restore_cred(struct ckpt_ctx *ctx)
 	ret = cred_setfsgid(cred, h->fsgid, &oldgid);
 	if (oldgid != h->fsgid && ret < 0)
 		goto err_putcred;
+	if ((ctx->uflags & RESTART_KEEP_LSM) && (h->sec_ref != -1)) {
+		struct ckpt_stored_lsm *l = ckpt_obj_fetch(ctx, h->sec_ref,
+					CKPT_OBJ_SEC);
+		if (IS_ERR(l))
+			return (struct cred *)l;

And here too ?

Quoted 48 lines

checkpoint/restart.c +48 -0

+/**
+ * security_checkpoint_obj - if first checkpoint of this void* security,
+ * then 1. ask the LSM for a string representing the context, 2. checkpoint
+ * that string
+ * @ctx: the checkpoint context
+ * @security: the void* security being checkpointed
+ * @sectype: indicates the type of object, because LSMs can (and do) store
+ * different types of data for different types of objects.
+ *
+ * Returns the objref of the checkpointed ckpt_stored_lsm representing the
+ * context, or -error on error.
+ *
+ * This is only used at checkpoint of course.
+ */
+int security_checkpoint_obj(struct ckpt_ctx *ctx, void *security,
+				unsigned sectype)
+{
+	int strref;
+	struct ckpt_stored_lsm *l;
+	char *str;
+	int len;
+
+	/*
+	 * to simplify the LSM code, short-cut a null security
+	 * here.
+	 */
+	if (!security)
+		return -EOPNOTSUPP;
+
+	switch (sectype) {
+	case CKPT_SECURITY_MSG_MSG:
+		str = security_msg_msg_checkpoint(security);
+		break;
+	case CKPT_SECURITY_IPC:
+		str = security_ipc_checkpoint(security);
+		break;
+	case CKPT_SECURITY_FILE:
+		str = security_file_checkpoint(security);
+		break;
+	case CKPT_SECURITY_CRED:
+		str = security_cred_checkpoint(security);
+		break;
+	default:
+		str = ERR_PTR(-EINVAL);
+		break;
+	}
+	/* str will be alloc'ed for us by the LSM.  We will free it when
+	 * we clear out our hashtable */

checkpoint/restart.c

Quoted 5 lines

checkpoint/restart.c +5 -0

+	if (IS_ERR(str))
+		return PTR_ERR(str);
+
+	len = strlen(str);
+	l = kzalloc(sizeof(*l) + len + 1, GFP_KERNEL);

Probably a dumb question: why do you allocate with the size of the string when it merely points to the string ?

Quoted 8 lines

checkpoint/restart.c +8 -0

+	if (!l) {
+		kfree(str);
+		return -ENOMEM;
+	}
+	kref_init(&l->kref);
+	l->sectype = sectype;
+	l->string = str;
+

checkpoint/restart.c

Quoted 6 lines

checkpoint/restart.c +6 -0

+	strref = checkpoint_obj(ctx, l, CKPT_OBJ_SEC);
+	/* do we need to free l if strref was err? */
+	return strref;
+}
+
+#endif

Oren.

Quoted 12 lines · Oren Laadan

Quoted 10 lines · Serge E. Hallyn

(wasn't versioning the patchsets before, so randomly pick 4 as the version for this patchset...)

Documentation/checkpoint/readme.txt begins:
"""
Application checkpoint/restart is the ability to save the state of a running application so that it can later resume its execution from the time at which it was checkpointed.
"""

Quoted 5 lines

Quoted 5 lines

checkpoint/checkpoint.c +5 -0

+	memset(ctx->lsm_name, 0, SECURITY_NAME_MAX + 1);
+	strlcpy(ctx->lsm_name, security_get_lsm_name(), SECURITY_NAME_MAX + 1);
+	ret = ckpt_write_buffer(ctx, ctx->lsm_name, SECURITY_NAME_MAX + 1);
+	if (ret < 0)
+		return ret;

Quoted 8 lines

checkpoint/checkpoint.c static int checkpoint_write_header(struct ckpt_ctx *ctx) · line 364

Quoted 5 lines

checkpoint/checkpoint.c +5 -0

+
+	ret = security_checkpoint_header(ctx);
+	if (ret < 0)
+		return ret;
+

This is actually a case for a 'container-global' section that would appear after the header and before the rest of the image. (Would be useful also for network namespaces).

But LSM's are specifically not containerized, so this is a host property, not a container one.

Quoted 13 lines · Serge E. Hallyn

Quoted 12 lines · Oren Laadan

Quoted 10 lines · Serge E. Hallyn

(wasn't versioning the patchsets before, so randomly pick 4 as the version for this patchset...)

Documentation/checkpoint/readme.txt begins:
"""
Application checkpoint/restart is the ability to save the state of a running application so that it can later resume its execution from the time at which it was checkpointed.
"""

[...]

Quoted 14 lines

Quoted 13 lines

checkpoint/checkpoint.c static int checkpoint_write_header(struct ckpt_ctx *ctx) · line 364

Quoted 10 lines

checkpoint/checkpoint.c +10 -0

+	memset(ctx->lsm_name, 0, SECURITY_NAME_MAX + 1);
+	strlcpy(ctx->lsm_name, security_get_lsm_name(), SECURITY_NAME_MAX + 1);
+	ret = ckpt_write_buffer(ctx, ctx->lsm_name, SECURITY_NAME_MAX + 1);
+	if (ret < 0)
+		return ret;
+
+	ret = security_checkpoint_header(ctx);
+	if (ret < 0)
+		return ret;
+

This is actually a case for a 'container-global' section that would appear after the header and before the rest of the image. (Would be useful also for network namespaces).

Quoted 2 lines

But LSM's are specifically not containerized, so this is a host property, not a container one.

Hmmm... does that mean you can't apply one policy to one container and another policy to another ?

Anyway, it belongs to a 'global' section, that may have 2 parts:
host and container. (Putting it between header and arch-header seems weird...)

The header doesn't hold state, it is a declarative section about the properties of the original host (kernel and HW).

Documentation/checkpoint/readme.txt begins:
"""
Application checkpoint/restart is the ability to save the state of a running application so that it can later resume its execution from the time at which it was checkpointed.
"""

This patch adds the ability to checkpoint and restore selinux contexts for tasks, open files, and sysvipc objects. Contexts are checkpointed as strings. For tasks and files, where a security struct actually points to several contexts, all contexts are written out in one string, separated by ':::'.

security/selinux/hooks.c +0 -0

index 8d8b69c..4fb8b5d 100644

security/selinux/hooks.c +43 -0

+static inline int selinux_file_restore(struct file *file, char *ctx)
+{
+	char *s1, *s2;
+	__u32 sid1 = 0, sid2 = 0;
+	int ret = -EINVAL;
+	struct file_security_struct *fsec = file->f_security;
+
+	/*
+	 * Objhash made sure the string is null-terminated.
+	 * We make a copy so we can mangle it.
+	 */
+	s1 = kstrdup(ctx, GFP_KERNEL);
+	if (!s1)
+		return -ENOMEM;
+	s2 = strstr(s1, ":::");
+	if (!s2)
+		goto out;
+
+	*s2 = '\0';
+	s2 += 3;
+	if (*s2 == '\0')
+		goto out;
+
+	/* SECSID_NULL is not valid for file sids */
+	if (strlen(s1) == 0 || strlen(s2) == 0)
+		goto out;
+
+	ret = security_context_to_sid(s1, strlen(s1), &sid1);
+	if (ret)
+		goto out;
+	ret = security_context_to_sid(s2, strlen(s2), &sid2);
+	if (ret)
+		goto out;
+
+	if (sid1 && fsec->sid != sid1) {
+		ret = avc_has_perm(current_sid(), sid1, SECCLASS_FILE,
+					FILE__RESTORE, NULL);
+		if (ret)
+			goto out;
+		fsec->sid = sid1;
+	}
+
+#if 0

security/selinux/hooks.c +4 -0

@@ -3219,6 +3322,187 @@ static int selinux_task_create(unsigned long clone_flags)
 	return current_has_perm(current, PROCESS__FORK);
 }
 
+#define NUMTASKSIDS 6
+/*
+ * for cred context, we print:
+ *   osid, sid, exec_sid, create_sid, keycreate_sid, sockcreate_sid;

Quoted 21 lines · Serge E. Hallyn

Documentation/checkpoint/readme.txt begins:
"""
Application checkpoint/restart is the ability to save the state of a running application so that it can later resume its execution from the time at which it was checkpointed.
"""

This patch adds the ability to checkpoint and restore selinux contexts for tasks, open files, and sysvipc objects. Contexts are checkpointed as strings. For tasks and files, where a security struct actually points to several contexts, all contexts are written out in one string, separated by ':::'.

security/selinux/hooks.c +0 -0

index 8d8b69c..4fb8b5d 100644
@@ -2961,6 +2962,108 @@ static int selinux_file_permission(struct file *file, int mask)
 	return selinux_revalidate_file_permission(file, mask);
 }
 

security/selinux/hooks.c static int selinux_file_permission(struct file *file, int mask) · line 2971

Quoted 7 lines

security/selinux/hooks.c +7 -0

+/*
+ * for file context, we print both the fsec->sid and fsec->fown_sid
+ * as string representations, separated by ':::'
+ * We don't touch isid - if you wanted that set you shoulda set up the
+ * fs correctly.
+ */
+static inline char *selinux_file_checkpoint(void *security)

Why are these functions declared inline? In any event, just let the compiler figure out when to make them inline if appropriate.

Quoted 43 lines

security/selinux/hooks.c +43 -0

+static inline int selinux_file_restore(struct file *file, char *ctx)
+{
+	char *s1, *s2;
+	__u32 sid1 = 0, sid2 = 0;
+	int ret = -EINVAL;
+	struct file_security_struct *fsec = file->f_security;
+
+	/*
+	 * Objhash made sure the string is null-terminated.
+	 * We make a copy so we can mangle it.
+	 */
+	s1 = kstrdup(ctx, GFP_KERNEL);
+	if (!s1)
+		return -ENOMEM;
+	s2 = strstr(s1, ":::");
+	if (!s2)
+		goto out;
+
+	*s2 = '\0';
+	s2 += 3;
+	if (*s2 == '\0')
+		goto out;
+
+	/* SECSID_NULL is not valid for file sids */
+	if (strlen(s1) == 0 || strlen(s2) == 0)
+		goto out;
+
+	ret = security_context_to_sid(s1, strlen(s1), &sid1);
+	if (ret)
+		goto out;
+	ret = security_context_to_sid(s2, strlen(s2), &sid2);
+	if (ret)
+		goto out;
+
+	if (sid1 && fsec->sid != sid1) {
+		ret = avc_has_perm(current_sid(), sid1, SECCLASS_FILE,
+					FILE__RESTORE, NULL);
+		if (ret)
+			goto out;
+		fsec->sid = sid1;
+	}
+
+#if 0

security/selinux/hooks.c static int selinux_file_permission(struct file *file, int mask) · line 3024

Quoted 10 lines

security/selinux/hooks.c +10 -0

+	/* The following must wait - bc we don't yet support
+	 * c/r of fowners. */
+	if (sid2 && fsec->fown_sid != sid2) {
+		ret = avc_has_perm(current_sid(), sid2, SECCLASS_FILE,
+				FILE__FOWN_RESTORE, NULL);
+		if (ret)
+			goto out;
+	       fsec->fown_sid = sid2;
+	}
+#endif

I'm not sure why you can't enable this code yet. From your comment, it sounds like c/r doesn't yet handle the uid/euid in the fown_struct, but that doesn't mean you can't manage the fown_sid that we store in the file. We just leveraged the fact that a fown_struct is never outside the context of a containing struct file (at least from a send_sigio context) to avoid introducing a separate security field with its own lifecycle there.

Quoted 13 lines

security/selinux/hooks.c +9 -0

@@ -3219,6 +3322,187 @@ static int selinux_task_create(unsigned long clone_flags)
 	return current_has_perm(current, PROCESS__FORK);
 }
 
+#define NUMTASKSIDS 6
+/*
+ * for cred context, we print:
+ *   osid, sid, exec_sid, create_sid, keycreate_sid, sockcreate_sid;
+ * as string representations, separated by ':::'
+ */
+static inline char *selinux_cred_checkpoint(void *security)
+{
+	struct task_security_struct *tsec = security;

security/selinux/hooks.c static int selinux_task_create(unsigned long clone_flags) · line 3338

Quoted 5 lines

security/selinux/hooks.c +5 -0

+	char *stmp, *sfull = NULL;
+	__u32 slen, runlen;
+	int i, ret;
+	__u32 sids[NUMTASKSIDS] = { tsec->osid, tsec->sid, tsec->exec_sid,
+		tsec->create_sid, tsec->keycreate_sid, tsec->sockcreate_sid };

Why are you using __u32 rather than just u32? I think the former is only necessary in certain headers that might be shared with userspace?

Quoted 6 lines

security/selinux/hooks.c +6 -0

+static inline int selinux_cred_restore(struct file *file, struct cred *cred,
+					char *ctx)
+{
+	char *s, *s1, *s2 = NULL;
+	int ret = -EINVAL;
+	struct task_security_struct *tsec = cred->security;

security/selinux/hooks.c static int selinux_task_create(unsigned long clone_flags) · line 3348

Quoted 4 lines

security/selinux/hooks.c +4 -0

+	int i;
+	__u32 sids[NUMTASKSIDS];
+	struct inode *ctx_inode = file->f_dentry->d_inode;
+	struct avc_audit_data ad;

audit code was refactored in mainline to share common data structures and code among security modules, so e.g. this became common_audit_data. Not sure what your baseline is, but this happened in July.

Quoted 11 lines

security/selinux/hooks.c +10 -0

+	/*
+	 * Check that these transitions are allowed, and effect them.
+	 * XXX: Do these checks suffice?
+	 */
+	if (tsec->osid != sids[0]) {
+		/*
+		 * Are any security decisions made based on osid?
+		 * Is it worth adding a PROCESS_RESTORE_OSID permission
+		 * for this?
+		 */

security/selinux/hooks.c static int selinux_task_create(unsigned long clone_flags) · line 3359

Quoted 1 line

security/selinux/hooks.c +1 -0

+		 tsec->osid = sids[0];

osid is only used in kernel permission checks upon a SID transition, and then it has been updated to ->sid prior to the transition. Userspace however may use it in a permission check (obtained via getprevcon) on the caller when the program runs in its own domain, e.g. passwd(1). I suppose there should be a check for it, although possibly it could be the same as for the ->sid (i.e. PROCESS__RESTORE).

security/selinux/include/av_perm_to_string.h

Quoted 4 lines

security/selinux/include/av_perm_to_string.h +0 -0

index 31df1d7..78945e7 100644

<snip>

Just FYI, this file goes away with the dynamic class/perm discovery patch that I've been posting to selinux list.

Quoted 4 lines

security/selinux/include/av_permissions.h +0 -0

index d645192..8e066ac 100644

<snip>

Likewise, this file goes away and is replaced by an automatically generated one from the new classmap.h file, which is the only kernel file you'll need to modify to add new permissions.

Documentation/checkpoint/readme.txt begins:
"""
Application checkpoint/restart is the ability to save the state of a running application so that it can later resume its execution from the time at which it was checkpointed.
"""

This patch adds the ability to checkpoint and restore selinux contexts for tasks, open files, and sysvipc objects. Contexts are checkpointed as strings. For tasks and files, where a security struct actually points to several contexts, all contexts are written out in one string, separated by ':::'.

The default behaviors are to checkpoint contexts, but not to restore them. To attempt to restore them, sys_restart() must be given the RESTART_KEEP_LSM flag. If this is given then the caller of sys_restart() must have the new 'restore' permission to the target objclass, or for instance PROCESS__SETFSCREATE to itself to specify a create_sid.

There are some tests under cr_tests/selinux at
git://git.sr71.net/~hallyn/cr_tests.git.

A corresponding simple refpolicy (and /usr/share/selinux/devel/include) patch is needed.

The programs to checkpoint and restart (called 'checkpoint' and 'restart') come from git://git.ncl.cs.columbia.edu/pub/git/user-cr.git.
This patch applies against the checkpoint/restart-enabled kernel tree at git://git.ncl.cs.columbia.edu/pub/git/linux-cr.git/.

Changelog:
oct 01: Remove some debugging that is redundant with
avc log data.
sep 10: (Most addressing suggestions by Stephen Smalley)
1. change xyz_get_ctx() to xyz_checkpoint().
2. check entrypoint permission on cred_restore
3. always dec context length by 1
4. don't allow SECSID_NULL when that's not valid
5. when SECSID_NULL is valid, restore it
6. c/r task->osid
7. Just print nothing instead of 'null' for SECSID_NULL
8. sids are __u32, as are lenghts passed to sid_to_context.

Signed-off-by Serge E. Hallyn <[email protected]>

Cc Stephen Smalley <[email protected]>

---
 checkpoint/restart.c                         |    1 +
 kernel/cred.c                                |    2 +
 security/selinux/hooks.c                     |  371 ++++++++++++++++++++++++++
 security/selinux/include/av_perm_to_string.h |    4 +
 security/selinux/include/av_permissions.h    |    4 +
 5 files changed, 382 insertions(+), 0 deletions(-)

checkpoint/restart.c +0 -0

index 55bd2b5..008a116 100644

kernel/cred.c +11 -0

+static inline char *selinux_file_checkpoint(void *security)
+{
+	struct file_security_struct *fsec = security;
+	char *s1 = NULL, *s2 = NULL, *sfull;
+	__u32 len1, len2, lenfull;
+	int ret;
+
+	if (fsec->sid == 0 || fsec->fown_sid == 0)
+		return ERR_PTR(-EINVAL);
+
+	ret = security_sid_to_context(fsec->sid, &s1, &len1);

checkpoint/restart.c static int restore_read_header(struct ckpt_ctx *ctx) · line 475

Quoted 10 lines · Serge E. Hallyn

checkpoint/restart.c +1 -0

index 55bd2b5..008a116 100644
@@ -471,6 +471,7 @@ static int restore_read_header(struct ckpt_ctx *ctx)
 		/* to be implemented later, per-lsm */
 		if (strcmp(ctx->lsm_name, "lsm_none") != 0 &&
 				strcmp(ctx->lsm_name, "smack") != 0 &&
+				strcmp(ctx->lsm_name, "selinux") != 0 &&
 				strcmp(ctx->lsm_name, "default") != 0) {

If we expect this to grow, it may make sense to look it up in an array.

Quoted 10 lines

checkpoint/restart.c +0 -0

 			pr_warning("c/r: RESTART_KEEP_LSM unsupported for %s\n",
 					ctx->lsm_name);

kernel/cred.c +0 -0

index 06bc676..5eb09b8 100644
@@ -732,6 +732,8 @@ static int do_checkpoint_cred(struct ckpt_ctx *ctx, struct cred *cred)
 	if (!h)
 		return -ENOMEM;
 

kernel/cred.c static int do_checkpoint_cred(struct ckpt_ctx *ctx, struct cred *cred) · line 736

Quoted 2 lines

kernel/cred.c +2 -0

+	ckpt_debug("cred uid %d fsuid %d gid %d secref %d\n", cred->uid,
+		cred->fsuid, cred->gid, sec_ref);

Please place this (and a few others also in other patches) debug-only changes in a separate patch - it will make my life tremendously easier when merging v18-dev to a nice and clean v19.

[...]

Quoted 15 lines

kernel/cred.c +15 -0

+static inline char *selinux_file_checkpoint(void *security)
+{
+	struct file_security_struct *fsec = security;
+	char *s1 = NULL, *s2 = NULL, *sfull;
+	__u32 len1, len2, lenfull;
+	int ret;
+
+	if (fsec->sid == 0 || fsec->fown_sid == 0)
+		return ERR_PTR(-EINVAL);
+
+	ret = security_sid_to_context(fsec->sid, &s1, &len1);
+	if (ret)
+		return ERR_PTR(ret);
+	len1--;
+	ret = security_sid_to_context(fsec->fown_sid, &s2, &len2);

kernel/cred.c

Quoted 6 lines

kernel/cred.c +6 -0

+	if (ret) {
+		kfree(s1);
+		return ERR_PTR(ret);
+	}
+	len2--;
+	lenfull = len1+len2+3;

Would checkpatch.pl complain about no spaces here ?

Quoted 3 lines · Oren Laadan

Please place this (and a few others also in other patches) debug-only changes in a separate patch - it will make my life tremendously easier when merging v18-dev to a nice and clean v19.

Ok, I'll resend those just to containers@ to spare the lsm folks.