cr: define ckpt_debug if CONFIG_CHECKPOINT=n

[ patch 1 was a trivial non-security patch so if you didn't see it, you didn't miss anything ]

The RESTART_KEEP_LSM flag indicates that the LSM should attempt to reuse checkpointed security labels. It is always invalid when the LSM at restart differs from that at checkpoint. It is currently only usable for capabilities.

(For capabilities, restart without RESTART_KEEP_LSM is technically not implemented. There actually might be a use case for that, but the safety of it is dubious so for now we always re-create checkpointed capability sets whether RESTART_KEEP_LSM is specified or not)

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

---
 Documentation/checkpoint/readme.txt |   25 +++++++++++++++++++++++++
 checkpoint/checkpoint.c             |    6 ++++++
 checkpoint/restart.c                |   19 +++++++++++++++++++
 include/linux/checkpoint.h          |    6 ++++--
 include/linux/checkpoint_hdr.h      |    3 ++-
 include/linux/checkpoint_types.h    |    2 ++
 include/linux/security.h            |    8 ++++++++
 security/security.c                 |    5 +++++
 8 files changed, 71 insertions(+), 3 deletions(-)

Documentation/checkpoint/readme.txt +25 -0

index e84dc39..a60e001 100644
@@ -328,6 +328,31 @@ So that's why we don't want CAP_SYS_ADMIN required up-front. That way
 we will be forced to more carefully review each of those features.
 However, this can be controlled with a sysctl-variable.
 
+LSM
+===
+
+Security modules use custom labels on subjects and objects to
+further mediate access decisions beyond DAC controls.  When
+checkpoint applications, these labels are [ work in progress ]
+checkpointed along with the objects.  At restart, the
+RESTART_KEEP_LSM flag tells the kernel whether re-created objects
+whould keep their checkpointed labels, or get automatically
+recalculated labels.  Since checkpointed labels will only make
+sense to the same LSM which was active at checkpoint time,
+sys_restart() with the RESTART_KEEP_LSM flag will fail with
+-EINVAL if the LSM active at restart is not the same as that
+active at checkpoint.  If RESTART_KEEP_LSM is not specified,
+then objects will be given whatever default labels the LSM and
+their optional policy decide.  Of course, when RESTART_KEEP_LSM
+is specified, the LSM may choose a different label than the
+checkpointed one, or fail the entire restart if the caller
+does not have permission to create objects with the checkpointed
+label.
+
+It should always be safe to take a checkpoint of an application
+under LSM_1, and restart it without the RESTART_KEEP_LSM flag
+under LSM_2.
+
 
 Kernel interfaces
 =================

checkpoint/checkpoint.c +6 -0

index c19f812..70e3fac 100644
@@ -250,6 +250,12 @@ static int checkpoint_write_header(struct ckpt_ctx *ctx)
 	if (ret < 0)
 		return ret;
 
+	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;
+
 	return checkpoint_write_header_arch(ctx);
 }
 

checkpoint/restart.c +19 -0

index ed42b4b..f0ca1f6 100644
@@ -423,6 +423,25 @@ static int restore_read_header(struct ckpt_ctx *ctx)
 	if (ret < 0)
 		goto out;
 
+	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);
+			return -EINVAL;
+		}
+		/* to be implemented later, per-lsm */
+		if (strcmp(ctx->lsm_name, "lsm_none") != 0 &&
+				strcmp(ctx->lsm_name, "default") != 0) {
+			pr_warning("c/r: RESTART_KEEP_LSM unsupported for %s\n",
+					ctx->lsm_name);
+			return -ENOSYS;
+		}
+	}
+
 	ret = restore_read_header_arch(ctx);
  out:
 	kfree(uts);

include/linux/checkpoint.h +4 -2

index 06e3eb0..3a800a6 100644
@@ -10,7 +10,7 @@
  *  distribution for more details.
  */
 
-#define CHECKPOINT_VERSION  1
+#define CHECKPOINT_VERSION  2
 
 /* checkpoint user flags */
 #define CHECKPOINT_SUBTREE	0x1

@@ -18,6 +18,7 @@
 /* restart user flags */
 #define RESTART_TASKSELF	0x1
 #define RESTART_FROZEN		0x2
+#define RESTART_KEEP_LSM	0x4
 
 #ifdef __KERNEL__
 #ifdef CONFIG_CHECKPOINT

@@ -44,7 +45,8 @@
 
 /* ckpt_ctx: uflags */
 #define CHECKPOINT_USER_FLAGS		CHECKPOINT_SUBTREE
-#define RESTART_USER_FLAGS		(RESTART_TASKSELF | RESTART_FROZEN)
+#define RESTART_USER_FLAGS		(RESTART_TASKSELF | RESTART_FROZEN | \
+					 RESTART_KEEP_LSM)
 
 extern void exit_checkpoint(struct task_struct *tsk);
 

include/linux/checkpoint_hdr.h +2 -1

index 06bc6e2..2b166dc 100644
@@ -176,10 +176,11 @@ struct ckpt_hdr_header {
 	__u64 uflags;	/* uflags from checkpoint */
 
 	/*
-	 * the header is followed by three strings:
+	 * the header is followed by four strings:
 	 *   char release[const.uts_release_len];
 	 *   char version[const.uts_version_len];
 	 *   char machine[const.uts_machine_len];
+	 *   char lsm_name[SECURITY_NAME_MAX + 1]
 	 */
 } __attribute__((aligned(8)));
 

include/linux/checkpoint_types.h +2 -0

index a18846f..680750d 100644
@@ -19,6 +19,7 @@
 #include <linux/fs.h>
 #include <linux/ktime.h>
 #include <linux/wait.h>
+#include <linux/security.h>
 
 struct ckpt_stats {
 	int uts_ns;

@@ -36,6 +37,7 @@ struct ckpt_ctx {
 	struct task_struct *root_task;		/* [container] root task */
 	struct nsproxy *root_nsproxy;		/* [container] root nsproxy */
 	struct task_struct *root_freezer;	/* [container] root task */
+	char lsm_name[SECURITY_NAME_MAX + 1];   /* security module at ckpt */
 
 	unsigned long kflags;	/* kerenl flags */
 	unsigned long uflags;	/* user flags */

include/linux/security.h +8 -0

index 5eff459..f1033a4 100644
@@ -1776,6 +1776,8 @@ int security_secid_to_secctx(u32 secid, char **secdata, u32 *seclen);
 int security_secctx_to_secid(const char *secdata, u32 seclen, u32 *secid);
 void security_release_secctx(char *secdata, u32 seclen);
 
+char *security_get_lsm_name(void);
+
 #else /* CONFIG_SECURITY */
 struct security_mnt_opts {
 };

@@ -1798,6 +1800,12 @@ static inline int security_init(void)
 	return 0;
 }
 
+#define DEFAULT_LSM_NAME "lsm_none"
+static inline char *security_get_lsm_name(void)
+{
+	return DEFAULT_LSM_NAME;
+}
+
 static inline int security_ptrace_may_access(struct task_struct *child,
 					     unsigned int mode)
 {

security/security.c +5 -0

index dc7674f..3829156 100644
@@ -122,6 +122,11 @@ int register_security(struct security_operations *ops)
 	return 0;
 }
 
+char *security_get_lsm_name(void)
+{
+	return security_ops->name;
+}
+
 /* Security operations */
 
 int security_ptrace_may_access(struct task_struct *child, unsigned int mode)

Quoted 3 lines · Serge E. Hallyn

[ patch 1 was a trivial non-security patch so if you didn't see it, you didn't miss anything ]

Quoted 5 lines

The RESTART_KEEP_LSM flag indicates that the LSM should attempt to reuse checkpointed security labels. It is always invalid when the LSM at restart differs from that at checkpoint. It is currently only usable for capabilities.

Can you imagine a scenario in which restoring a process on a system with a different LSM configuration makes any sense at all?

Goodness gracious, even if the "old" environment and the "new" are both SELinux and the policies are different I can't see how you could make any sort of claim that restoring the process is safe.

The same goes for having file based capabilities on one
and not on the other.

It seems that the check you've chosen is necessary, but far from sufficient.

Quoted 11 lines · Casey Schaufler

Quoted 9 lines · Serge E. Hallyn

[ patch 1 was a trivial non-security patch so if you didn't see it, you didn't miss anything ]

The RESTART_KEEP_LSM flag indicates that the LSM should attempt to reuse checkpointed security labels. It is always invalid when the LSM at restart differs from that at checkpoint. It is currently only usable for capabilities.

Quoted 3 lines

Can you imagine a scenario in which restoring a process on a system with a different LSM configuration makes any sense at all?

Without RESTART_KEEP_LSM absolutely.

With RESTART_KEEP_LSM, on a system with a different LSM loaded, certainly not.

With RESTART_KEEP_LSM, on a system with the same LSM but a different policy, yes I do. If any checkpointed contexts have been invalidated in the new policy, then restart with RESTART_KEEP_LSM should fail (*1). If the contexts are still valid, then it seems reasonable to assume that bin_t, user_t, etc, still basically mean what they meant before. No reason to refuse restart just because I loaded a policy module for postfix, imo.

I could add both an lsm-module and lsm-policy version to the checkpoint header, where the lsm-policy might be a sha1sum of the whole policy, but that seems like overkill, a lot of overhead, and probably a maintenance headache for the lsm-module version.

Quoted 5 lines

Goodness gracious, even if the "old" environment and the "new" are both SELinux and the policies are different I can't see how you could make any sort of claim that restoring the process is safe.

In what sense do you mean 'unsafe'? The initial creation or access to any checkpointed resource always happens with the sys_restart() caller's and existing object's contexts, so there should be no opportunity for accessing data which the old policy allowed but the new does not. It's possible that the task will fail because of a more restrictive new policy, but so be it.

Quoted 3 lines

The same goes for having file based capabilities on one
and not on the other.

A task running as uid 500 with cap_dac_read_search=ep could have been started at least 3 ways:
1. a uid 500 task executing a file with cap_dac_read_search=ep file caps
2. a root task executing a file which does prctl(PR_SET_KEEPCAPS), setuid(500), and drops the other caps,
3. a uid 500 task executing a setuid root version of the file in (2).

When we are restarting a task, we don't really care which of the above ways it might have gotten its privileges.

I certainly could be overlooking something - what do you see as the safety problem?

Quoted 3 lines

It seems that the check you've chosen is necessary, but far from sufficient.

Thanks much for giving this some thought. Without doubt this is tricky business, and I was definately hoping for some serious discussion.

thanks,
-serge

*1 - Hmm, in Smack, if the caller has CAP_MAC_ADMIN, then he
can load new, currently undefined types, right? I guess that
could be a problem, though again I assume the new type should
simply have no accesses and so shouldn't be unsafe.
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 17 lines · Serge E. Hallyn

Quoted 14 lines · Casey Schaufler

Quoted 9 lines · Serge E. Hallyn

[ patch 1 was a trivial non-security patch so if you didn't see it, you didn't miss anything ]

The RESTART_KEEP_LSM flag indicates that the LSM should attempt to reuse checkpointed security labels. It is always invalid when the LSM at restart differs from that at checkpoint. It is currently only usable for capabilities.

Can you imagine a scenario in which restoring a process on a system with a different LSM configuration makes any sense at all?

Quoted 2 lines

Without RESTART_KEEP_LSM absolutely.

Wow. I run a program on a system with the default LSM and get all sorts of passwords for network based services, then take it to an SELinux system that has a different firewall configuration based on the assumption that SELinux will prevent the program from getting that information. You'd allow that?

Wow. I run a program on a system with the default LSM and get all sorts of passwords for network based services, then take it to an

SELinux system that has a different firewall configuration based on the assumption that SELinux will prevent the program from getting that information. You'd allow that?

Quoted 11 lines

With RESTART_KEEP_LSM, on a system with a different LSM loaded, certainly not.

With RESTART_KEEP_LSM, on a system with the same LSM but a different policy, yes I do. If any checkpointed contexts have been invalidated in the new policy, then restart with RESTART_KEEP_LSM should fail (*1). If the contexts are still valid, then it seems reasonable to assume that bin_t, user_t, etc, still basically mean what they meant before. No reason to refuse restart just because I loaded a policy module for postfix, imo.

Again, Wow. How can you tell that a process that ran for a while under another policy can not have done anything that would make it unsafe under the current policy?

And what if the program is the
very one with the changed policy? How can you tell?

Quoted 13 lines

I could add both an lsm-module and lsm-policy version to the checkpoint header, where the lsm-policy might be a sha1sum of the whole policy, but that seems like overkill, a lot of overhead, and probably a maintenance headache for the lsm-module version.

Quoted 5 lines

Goodness gracious, even if the "old" environment and the "new" are both SELinux and the policies are different I can't see how you could make any sort of claim that restoring the process is safe.

Quoted 7 lines

In what sense do you mean 'unsafe'? The initial creation or access to any checkpointed resource always happens with the sys_restart() caller's and existing object's contexts, so there should be no opportunity for accessing data which the old policy allowed but the new does not. It's possible that the task will fail because of a more restrictive new policy, but so be it.

Now I'm no expert on SELinux policies, but if the change in policy was made to prevent the program from doing something that it has already done under the old policy so that it can safely allow the program to do something it hasn't done prior to being checkpointed, you have a big problem.

That's a big sentence. Let me clarify a little:

Let's say that my program, placebo, needs to do one of two dangerous things, either thing A early in the execution or thing B late in the execution. It would be perfectly safe for placebo to do either, but really dreadfully bad for it to do both. The program is of course oblivious to the policy, so it will try to do both even though it really only needs to do one.

The old policy allows placebo to do A and not B. The new policy allows B but not A. Either is safe. If I run placebo under the old policy, checkpoint it, and restart it under the new policy Bad Things happen. Even with both the old and the new policies being demonstrably safe.

Quoted 22 lines · Casey Schaufler

Quoted 20 lines · Serge E. Hallyn

Quoted 17 lines · Casey Schaufler

Quoted 10 lines · Serge E. Hallyn

[ patch 1 was a trivial non-security patch so if you didn't see it, you didn't miss anything ]

The RESTART_KEEP_LSM flag indicates that the LSM should attempt to reuse checkpointed security labels. It is always invalid when the LSM at restart differs from that at checkpoint. It is currently only usable for capabilities.

Can you imagine a scenario in which restoring a process on a system with a different LSM configuration makes any sense at all?

Quoted 9 lines

Quoted 3 lines

Without RESTART_KEEP_LSM absolutely.

Wow. I run a program on a system with the default LSM and get all sorts of passwords for network based services, then take it to an SELinux system that has a different firewall configuration based on the assumption that SELinux will prevent the program from getting that information. You'd allow that?

?

What exactly would refusing the restart prevent?

You already have the passwords in the checkpoint file, so you're not protecting those.

The program was running under xyz_t, which on the old system was allowed to make some change the fw configuration, but is not under the new. Then after restart, it will be labeled as xyz_t, and won't be allowed to make the change.

When a program running as abc_t calls sys_restart() do restart the task under xyz_t, then
1. everything which is re-created for the task will need to be allowed (under current policy) to abc_t.
2. abc_t will need to be allowed process_t:restore to xyz_t.

What exactly can the abc_t task do if allowed to restart, which it couldn't otherwise do?

(1) If it can read the checkpoint
files, it can access the checkpointed keys either way. (2) If the new policy allows xyz_t to do something it couldn't do before, and abc_t is allowed to transition to xyz_t, then abc_t could presumably start any program under xyz_t to do what it wants. That is limited by the need to be able to create a file of an appropriate entry type. That's why before I was thinking of passing the type of the checkpoint file to security_cred_restore() as well. and (3) If under the new policy xyz_t is NOT allowed to do something which the old policy could do, then after restart it won't be allowed to do it either.

Quoting your example again:

Quoted 3 lines

Wow. I run a program on a system with the default LSM and get all sorts of passwords for network based services, then take it to an

XXX Here you already have gotten the passwords under the old policy. So the restart has nothing to do with whether you can get the keys which let you get the information. Then,

Quoted 4 lines

SELinux system that has a different firewall configuration based on the assumption that SELinux will prevent the program from getting that information. You'd allow that?

XXX The sys_restart() process will make sure that the process calling sys_restart() is authorized to recreate anything (i.e. open connections, open files, etc) which the task had open at checkpoint, and anything done after checkpoint will still be authorized by the new policy.

So really the big problem I see is that I've introduced a way to disassociate the type you can restore to, from the file (and contents of the file) you can execute to restore into the type.

I think that can be helped by more liberal use of relabel_froms and passing the type of a checkpoint file along to security_cred_restore(). Passing the type of the checkpoint file allows us to at least ensure that the file was created by a process of a type which is also authorized to be a part of the chain to restarting a task of xyz_t. Adding the relabel_from to file restore()s may be necessary to prevent a leak of MAC perms. Again, the process calling sys_restore() must originally have been authorized to open the file with the requested permissions, and must be allowed

	allow restore_t target_file_t:file restore;
permissions.  But it seems reasonable to add to that the
requirement to be allowed to
	allow restore_t orig_file_t:file relabel_from;

Now in the meantime, please note that there is nothing stopping you from ensuring that only my_only_restore_t is allowed to transition to anything, and having the program which transitions to that domain and calls sys_restart() first compare policy versions.

Quoted 12 lines

Quoted 12 lines

With RESTART_KEEP_LSM, on a system with a different LSM loaded, certainly not.

With RESTART_KEEP_LSM, on a system with the same LSM but a different policy, yes I do. If any checkpointed contexts have been invalidated in the new policy, then restart with RESTART_KEEP_LSM should fail (*1). If the contexts are still valid, then it seems reasonable to assume that bin_t, user_t, etc, still basically mean what they meant before. No reason to refuse restart just because I loaded a policy module for postfix, imo.

Quoted 4 lines

Again, Wow. How can you tell that a process that ran for a while under another policy can not have done anything that would make it unsafe under the current policy?

If it 'did something' before checkpoint, then either
1. the system is already changed as a result, and whether we restore or not has no further effect.
or
2. the process ended up with some resource which it couldn't have gotten under the new policy. In that case, if the task doing sys_restart() is not allowed to re-create the resource (file handle to /etc/shadow, socket, whatever) then the checkpoint will fail. If the task doing sys_restart() *is* allowed to re-create it, but the target context xyz_t would not have been, then I don't consider that a leak. As I said above, it could compare policy versions in user-space ahead of time or the user could have used a more appropriate context from which to call sys_restart().

Quoted 3 lines

And what if the program is the
very one with the changed policy? How can you tell?

Well then either
1. the checkpoint was taken before it changed the policy, so it will try to do so again (and hopefully fail)
2. the checkpoint was taken after it changed the policy, so the policy hasn't actually changed between checkpoint and restart.

and in any case, if the program was authorized to change policy, then the program calling sys_restart() on it should have required some tremendously trusted type to begin with.

Quoted 1 line

...

Then I would argue the user (who has authorization to restart this beast and therefore is hopefully not malicious) should have compared policies before doing sys_restart...

It seems I could save a lot of time arguing by proposing that

1. each LSM keep a policy version (sha1sum or whatever suits it) updated at every policy update
2. a new security_app_restore() hook passes in the the checkpoint header, so that the LSM can choose to verify the checkpoint file type and the policy version. Checkpoint file type check might be done even if !(flags & RESTART_KEEP_LSM)...

but I'm still not convinced this isn't just better done by userspace if it wants to. Because we are not protecting against a malicious user, as he can get the info he got from checkpoint() using ptrace, so we are only protecting the user from himself.

Maybe I should wait a day to send this, since perhaps on an afternoon walk I'll see the error of my ways...

thanks,
-serge

Quoted 1 line · Serge E. Hallyn

Quoting Casey Schaufler ([email protected]):

Quoted 5 lines

What exactly would refusing the restart prevent?

You already have the passwords in the checkpoint file, so you're not protecting those.

I seem to be having some trouble presenting my point. It's not about the passwords. It's not about the firewall configuration. It's not even really about the SELinux policy. It's about the total security configuration of the system. This is a problem with checkpoint/restart in general and there isn't much we can do about reassigning user id and other bad things that can happen.

If the admin decides that an action is acceptable because the SELinux policy prevents some other action it had better be the case that a process was never allowed to perform that "other action". If you allow restart across policy changes you can't be sure that the process has not performed such an "other action". Processes are not stateless.

Quoted 13 lines

The program was running under xyz_t, which on the old system was allowed to make some change the fw configuration, but is not under the new. Then after restart, it will be labeled as xyz_t, and won't be allowed to make the change.

When a program running as abc_t calls sys_restart() do restart the task under xyz_t, then
1. everything which is re-created for the task will need to be allowed (under current policy) to abc_t.
2. abc_t will need to be allowed process_t:restore to xyz_t.

Quoted 2 lines

What exactly can the abc_t task do if allowed to restart, which it couldn't otherwise do?

That's the point. It does not have to do the bad thing, it has already been done. The cat is out of the bag. The process now has state that it couldn't get under the new policy.

The new policy
allows the process to do something that it could not had it started under the new policy.

Quoted 4 lines

Then I would argue the user (who has authorization to restart this beast and therefore is hopefully not malicious) should have compared policies before doing sys_restart...

I'm a firm believer in malicious users. That, and users who are sufficiently stupid to be indistinguishable from malicious users. That, and users who are sufficiently clever to do the same things that a malicious user would do in order to do something that looks like a really good idea, but that is not.

Quoted 4 lines · Casey Schaufler

Quoted 2 lines · Serge E. Hallyn

Quoting Casey Schaufler ([email protected]):

...

Quoted 13 lines

Quoted 6 lines

What exactly would refusing the restart prevent?

You already have the passwords in the checkpoint file, so you're not protecting those.

I seem to be having some trouble presenting my point. It's not about the passwords. It's not about the firewall configuration. It's not even really about the SELinux policy. It's about the total security configuration of the system. This is a problem with checkpoint/restart in general and there isn't much we can do about reassigning user id and other bad things that can happen.

Quoted 6 lines

If the admin decides that an action is acceptable because the SELinux policy prevents some other action it had better be the case that a process was never allowed to perform that "other action". If you allow restart across policy changes you can't be sure that the process has not performed such an "other action". Processes are not stateless.

No, but again there are only 3 kinds of state that I can think of that are relevant at restart:

1. calculations which were made before ckpt and after restart. For instance, maybe before the policy change the task could read key a but not key b. After it could read key b but not a. With both keys, it can rule the world.
In this case c/r changes nothing, bc the user could get the same info using ptrace.

2. system-wide changes which the task could do with the old policy, while it could do different once after. Doing both changes it can rule the world. Again, c/r changes nothing, bc the user could simply run the program twice to make both changes.

3. resources (file descriptions, whatever) which the task could create before the policy change but not after. Every resource will be re-opened at sys_restart() using the credentials of the caller.
This isn't perfect. It means that if the program started out privileged to open some file and then dropped privilege, then the program either will fail to restart, or will have to be restarted with extra privilege. And there's a decent chance that the sys_restart() caller will have more privs than the program anyway. But that is why I feel pretty strongly that it's up to the caller, in userspace, to worry about it and do policy checks if need be.

I am more worried about a user taking a checkpoint image and editing it, then pursuading a more privileged user to restart it for him. "see, the source was trustworthy and needed privs to start. can you restart it for me?"

The only thing to do about that is to eventually do TPM signing of checkpoint images.

Quoted 17 lines

Quoted 17 lines

The program was running under xyz_t, which on the old system was allowed to make some change the fw configuration, but is not under the new. Then after restart, it will be labeled as xyz_t, and won't be allowed to make the change.

When a program running as abc_t calls sys_restart() do restart the task under xyz_t, then
1. everything which is re-created for the task will need to be allowed (under current policy) to abc_t.
2. abc_t will need to be allowed process_t:restore to xyz_t.

What exactly can the abc_t task do if allowed to restart, which it couldn't otherwise do?

Quoted 4 lines

That's the point. It does not have to do the bad thing, it has already been done. The cat is out of the bag. The process now has state that it couldn't get under the new policy.

Again I claim that any state it couldn't get under the new policy, either it could get using ptrace anyway (i.e. a key), or it can't re-create the state because the new policy will prevent it.

Quoted 5 lines

The new policy
allows the process to do something that it could not had it started under the new policy.

And it still is re-starting under the new policy, though again (I keep stressing bc this worries me more) the authorization checks are done against the credentials at sys_restart(), not the checkpointed credentials.

...

Quoted 11 lines

Quoted 5 lines

Then I would argue the user (who has authorization to restart this beast and therefore is hopefully not malicious) should have compared policies before doing sys_restart...

I'm a firm believer in malicious users. That, and users who are sufficiently stupid to be indistinguishable from malicious users. That, and users who are sufficiently clever to do the same things that a malicious user would do in order to do something that looks like a really good idea, but that is not.

So do you think that adding a policy version check in the kernel at restart would help this?

Do you think providing a recipe for userspace to do the policy checks would help?

Along with a security_may_restart() hook
so that LSM policy can force restart to go through a helper that does these checks, if it wants to enforce that?

Quoted 3 lines · Serge E. Hallyn

Quoting Casey Schaufler ([email protected]):
So do you think that adding a policy version check in the kernel at restart would help this?

Quoted 2 lines

Do you think providing a recipe for userspace to do the policy checks would help?

If you are willing to trust the user space tools to get it right. It gets things really complicated building trust relationships when you have multiple policies to deal with. Look at the issues that labeled NFS is trying to deal with. How would the user space program know what the policy may have been when the process last ran?

Quoted 4 lines

Along with a security_may_restart() hook
so that LSM policy can force restart to go through a helper that does these checks, if it wants to enforce that?

I really don't know. As you're well aware, I've never bought into the sophistication of the SELinux policy mechanism. One of my major concerns has always been that the policy on two machines may be different. With CR you have a case where the policy on the same machine can be different over the life of a process.

I am much less concerned with Smack because all the protection it provides are subject/object relationships and those are pretty straight forward. Even there, if the old rule set allowed the process to read from a label the new rule set doesn't allow you could get burned.

In the end, I don't suppose that anything stronger than "requires privilege" is going to result in a system that's useful. So maybe your best bet is security_may_restart(), and let the LSM decide. Smack will most likely require CAP_MAC_ADMIN where as SELinux will do a check based on the half-dozen contexts mentioned elsewhere. I still don't think it is safe, but upon further reflection I'm convinced that from a security standpoint it can never be safe to do a restore. So we're debating one case, and a fringe one at that.

Let 'er rip.

Quoted 4 lines · Casey Schaufler

Quoted 3 lines · Serge E. Hallyn

Quoting Casey Schaufler ([email protected]):
So do you think that adding a policy version check in the kernel at restart would help this?

For the moment I intend to add a patch on top of these adding two security calls:

security_may_checkpoint(ctx) which will authorize the
ability to checkpoint at all, and

Quoted 11 lines · Serge E. Hallyn

Quoted 6 lines · Casey Schaufler

Quoted 4 lines · Serge E. Hallyn

Quoting Casey Schaufler ([email protected]):
So do you think that adding a policy version check in the kernel at restart would help this?

For the moment I intend to add a patch on top of these adding two security calls:

Quoted 3 lines

security_may_checkpoint(ctx) which will authorize the
ability to checkpoint at all, and

I meant:

security_may_restore(ctx).

I seem to be having some trouble presenting my point. It's not about the passwords. It's not about the firewall configuration. It's not even really about the SELinux policy. It's about the total security configuration of the system. This is a problem with checkpoint/restart in general and there isn't much we can do about reassigning user id and other bad things that can happen.

If the admin decides that an action is acceptable because the SELinux policy prevents some other action it had better be the case that a process was never allowed to perform that "other action". If you allow restart across policy changes you can't be sure that the process has not performed such an "other action". Processes are not stateless.

Quoted 7 lines · Casey Schaufler

I seem to be having some trouble presenting my point. It's not about the passwords. It's not about the firewall configuration. It's not even really about the SELinux policy. It's about the total security configuration of the system. This is a problem with checkpoint/restart in general and there isn't much we can do about reassigning user id and other bad things that can happen.

Quoted 6 lines

If the admin decides that an action is acceptable because the SELinux policy prevents some other action it had better be the case that a process was never allowed to perform that "other action". If you allow restart across policy changes you can't be sure that the process has not performed such an "other action". Processes are not stateless.

Of course we potentially have the same issue when changing a boolean, and when loading a new policy on a running system. In a realistic scenario, if you make a change that allows a process context to write to a public data store while at the same time denying read access to secret data then you also need to purge the contents of any files that the process may open for read/write.

Similar problems can also occur using Unix permissions and certain combinations of chmod with setuid/setgid bits. Of course most potential cases of triggering this issue with Unix permissions are extremely contrived and wouldn't be a likely attack scenario. The best example I can think of with Unix permissions is access to sound devices. If a system is configured to add the "audio" supplementary group to a user's session when they login at the console then they can keep a detached process running to maintain access to the audio devices (and anything else permitted to that group).

Checkpoint/restart has traditionally been interesting in the mainframe and supercomputer space. These environments have very different security profiles from a user desktop. No one at the [.......] National Supercomputer Centre cares if you can save your rogue game as soon as you pick up the Amulet of Yendor and restart it if you get killed on the way up. These environments are concerned with leaking data between the groups that have funded the facility, which is why they are very often customers of advanced access control technologies. I don't know that I see a really good security story for C/R in the desktop space, and as Russell points out, there are plenty of opportunities to exploit the feature. This is of course well outside the scope of what goes on within an LSM, where the specific issues are more or less contained.

Quoted 6 lines · Casey Schaufler

Checkpoint/restart has traditionally been interesting in the mainframe and supercomputer space. These environments have very different security profiles from a user desktop. No one at the [.......] National Supercomputer Centre cares if you can save your rogue game as soon as you pick up the Amulet of Yendor and restart it if you get killed on the way up. These environments

Quoted 9 lines

are concerned with leaking data between the groups that have funded the facility, which is why they are very often customers of advanced access control technologies. I don't know that I see a really good security story for C/R in the desktop space, and as Russell points out, there are plenty of opportunities to exploit the feature. This is of course well outside the scope of what goes on within an LSM, where the specific issues are more or less contained.

#1 I think you are shortsighted on this, there are many desktop scenarios where c/r has benefits.

Think linux terminal services, where
one has a large amount of backend machines and doesn't want to kill a user's session due to having to reboot a machine, or to do load balancing.

#2 I think all this discussion is making things more complicated than it needs to be.

There should be a simple policy decision.

Q. Would we allow this "user" (of the restart mechanism) to create a container and be root to it.

If the answer is yes, then any restart (even with a maliciously modified checkpoint file) should be allowed to proceed, as no differences between that and what it could do anyways.

If the answer is no, then any restart should not be allowed to proceed.

why? I imagine restart working something like this

a) create new container for restarted processes
b) restore processes
c) apply container LSM policy
d) let processes run

why in this order, for the reasoning given before that permissions can change. This can actually be an issue with just a single user if you try to restore as that single user

i.e.
touch a
open(a)
chmod 000 a

and now try to restart, so shows the reliance on "root" in the context of the new container. its also possible to argue that if one does security model changes such as this, restart should fail, and therfore could switch by b/c above.

now getting back to the why.

what are the 2 primary attacks that have been mentioned

1) total host security policy
2) maliciously created checkpoint files

for #2, if we apply the policy decision I stated above, we shouldn't care about malicious checkpoint files (beyond their ability to trigger kernel bugs), as there's no fundamental difference between what a malicious user could do with a constructed checkpoint file and what our policy decision is based on (being root to the container).

for #1, why should this manner? if host policy (say re network connections?) would prevent some sort of functionality, this should cause restart to error out with a security permission error. restart
shouldn't ignore host policy in this case. Though I would add, this
shouldn't really matter re file system, as each containers fs should be isolated, and if they are shared and have different rules on different machines, I believe you have a much bigger fundamental security problem.

This then makes restart just an issue of how LSM policies can actually operate with multiple different containers that should be treated as fully independent machines running on a single host and with possibly different security requirements for each container, but not a checkpoint/restart issue in specific.

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 generic support for c/r of LSM credentials. Support for Smack and SELinux (and TOMOYO if appropriate) will be added later. Capabilities is already supported through generic creds code.

This patch supports ipc_perm, msg_msg, cred (task) and file ->security fields. Inodes, superblocks, netif, and xfrm currently are restored not through sys_restart() but through container creation, and so the security fields should be done then as well. Network should be added when network c/r is added.

Briefly, all security fields must be exported by the LSM as a simple null-terminated string. They are checkpointed through the security_checkpoint_obj() helper, because we must pass it an extra sectype field. Splitting SECURITY_OBJ_SEC into one type per object type would not work because, in Smack, one void* security is used for all object types.

But we must pass the sectype field because in
SELinux a different type of structure is stashed in each object type.

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

---
 checkpoint/checkpoint.c          |    1 +
 checkpoint/files.c               |   30 +++++++++
 checkpoint/objhash.c             |   93 +++++++++++++++++++++++++++
 include/linux/checkpoint_hdr.h   |   20 ++++++
 include/linux/checkpoint_types.h |    7 ++
 include/linux/security.h         |  128 ++++++++++++++++++++++++++++++++++++++
 ipc/checkpoint.c                 |   34 +++++++---
 ipc/checkpoint_msg.c             |   26 +++++++-
 ipc/checkpoint_sem.c             |    4 +-
 ipc/checkpoint_shm.c             |    4 +-
 ipc/util.h                       |    6 +-
 kernel/cred.c                    |   20 ++++++-
 security/capability.c            |   48 ++++++++++++++
 security/security.c              |  105 +++++++++++++++++++++++++++++++
 14 files changed, 506 insertions(+), 20 deletions(-)

checkpoint/checkpoint.c +1 -0

index 70e3fac..9dbb33c 100644
@@ -24,6 +24,7 @@
 #include <linux/utsname.h>
 #include <linux/magic.h>
 #include <linux/hrtimer.h>
+#include <linux/security.h>
 #include <linux/checkpoint.h>
 #include <linux/checkpoint_hdr.h>
 

checkpoint/files.c +30 -0

index 204055b..a26951a 100644
@@ -143,6 +143,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)
 {

@@ -155,6 +168,12 @@ int checkpoint_file_common(struct ckpt_ctx *ctx, struct file *file,
 	if (h->f_credref < 0)
 		return h->f_credref;
 
+	h->f_secref = checkpoint_file_security(ctx, file);
+	if (h->f_secref == -EOPNOTSUPP)
+		h->f_secref = -1;
+	else if (h->f_secref < 0)
+		return h->f_secref;
+
 	/* FIX: need also file->f_owner, etc */
 
 	return 0;

@@ -481,6 +500,17 @@ int restore_file_common(struct ckpt_ctx *ctx, struct file *file,
 	put_cred(file->f_cred);
 	file->f_cred = get_cred(cred);
 
+	if ((ctx->uflags & RESTART_KEEP_LSM) && (h->f_secref != -1)) {
+		struct ckpt_stored_lsm *l = ckpt_obj_fetch(ctx, h->f_secref,
+					CKPT_OBJ_SEC);
+		if (IS_ERR(l))
+			return PTR_ERR(l);
+
+		ret = security_file_restore(file, l->string);
+		if (ret)
+			return ret;
+	}
+
 	/* safe to set 1st arg (fd) to 0, as command is F_SETFL */
 	ret = vfs_fcntl(0, F_SETFL, h->f_flags & CKPT_SETFL_MASK, file);
 	if (ret < 0)

checkpoint/objhash.c +93 -0

index a9a10d1..2b0ead4 100644
@@ -16,6 +16,7 @@
 #include <linux/file.h>
 #include <linux/fdtable.h>
 #include <linux/sched.h>
+#include <linux/kref.h>
 #include <linux/ipc_namespace.h>
 #include <linux/user_namespace.h>
 #include <linux/checkpoint.h>

@@ -246,6 +247,89 @@ static void obj_sock_drop(void *ptr)
 	sock_put((struct sock *) ptr);
 }
 
+static void obj_free_sec(struct kref *kref)
+{
+	struct ckpt_stored_lsm *s = container_of(kref, struct ckpt_stored_lsm,
+					kref);
+	kfree(s->string);
+	kfree(s);
+}
+
+static int obj_sec_grab(void *ptr)
+{
+	struct ckpt_stored_lsm *s = ptr;
+	kref_get(&s->kref);
+	return 0;
+}
+
+static void obj_sec_drop(void *ptr)
+{
+	struct ckpt_stored_lsm *s = ptr;
+	kref_put(&s->kref, obj_free_sec);
+}
+
+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;
+	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;
+
+	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) {
+		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);
+	}
+	l->string = kzalloc(h->len + 1, GFP_KERNEL);
+	if (!l->string) {
+		kfree(l);
+		ckpt_hdr_put(ctx, h);
+		return ERR_PTR(-ENOMEM);
+	}
+	kref_init(&l->kref);
+	l->sectype = h->sectype;
+	strncpy(l->string, h->string, h->len);
+	ckpt_hdr_put(ctx, h);
+	return l;
+}
+
+static void *restore_security(struct ckpt_ctx *ctx)
+{
+	return (void *) do_restore_security(ctx);
+}
+
 static struct ckpt_obj_ops ckpt_obj_ops[] = {
 	/* ignored object */
 	{

@@ -382,6 +466,15 @@ static struct ckpt_obj_ops ckpt_obj_ops[] = {
 		.ref_drop = obj_sock_drop,
 		.ref_grab = obj_sock_grab,
 	},
+	/* LSM security labels */
+	{
+		.obj_name = "LSM",
+		.obj_type = CKPT_OBJ_SEC,
+		.ref_drop = obj_sec_drop,
+		.ref_grab = obj_sec_grab,
+		.checkpoint = checkpoint_security,
+		.restore = restore_security,
+	},
 };
 
 

include/linux/checkpoint_hdr.h +20 -0

index 2b166dc..729ca33 100644
@@ -53,6 +53,7 @@ enum {
 	CKPT_HDR_BUFFER,
 	CKPT_HDR_STRING,
 	CKPT_HDR_OBJREF,
+	CKPT_HDR_SEC,
 
 	CKPT_HDR_TREE = 101,
 	CKPT_HDR_TASK,

@@ -136,6 +137,7 @@ enum obj_type {
 	CKPT_OBJ_USER,
 	CKPT_OBJ_GROUPINFO,
 	CKPT_OBJ_SOCK,
+	CKPT_OBJ_SEC,
 	CKPT_OBJ_MAX
 };
 

@@ -250,6 +252,8 @@ struct ckpt_hdr_cred {
 	__u32 gid, sgid, egid, fsgid;
 	__s32 user_ref;
 	__s32 groupinfo_ref;
+	__s32 sec_ref;
+	__u32 padding;
 	struct ckpt_capabilities cap_s;
 } __attribute__((aligned(8)));
 

@@ -262,6 +266,16 @@ struct ckpt_hdr_groupinfo {
 	__u32 groups[0];
 } __attribute__((aligned(8)));
 
+struct ckpt_hdr_lsm {
+	struct ckpt_hdr h;
+	__u8 sectype;
+	__u16 len;
+	/*
+	 * 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

@@ -357,6 +371,8 @@ struct ckpt_hdr_file {
 	__s32 f_credref;
 	__u64 f_pos;
 	__u64 f_version;
+	__s32 f_secref;
+	__u32 f_padding;
 } __attribute__((aligned(8)));
 
 struct ckpt_hdr_file_generic {

@@ -595,6 +611,8 @@ struct ckpt_hdr_ipc_perms {
 	__u32 mode;
 	__u32 _padding;
 	__u64 seq;
+	__s32 sec_ref;
+	__u32 padding;
 } __attribute__((aligned(8)));
 
 struct ckpt_hdr_ipc_shm {

@@ -628,6 +646,8 @@ struct ckpt_hdr_ipc_msg_msg {
 	struct ckpt_hdr h;
 	__s32 m_type;
 	__u32 m_ts;
+	__s32 sec_ref;
+	__u32 padding;
 } __attribute__((aligned(8)));
 
 struct ckpt_hdr_ipc_sem {

include/linux/checkpoint_types.h +7 -0

index 680750d..182878b 100644
@@ -73,6 +73,13 @@ struct ckpt_ctx {
 	struct ckpt_stats stats;	/* statistics */
 };
 
+/* stored on hashtable */
+struct ckpt_stored_lsm {
+	struct kref kref;
+	int sectype;
+	char *string;
+};
+
 #endif /* __KERNEL__ */
 
 #endif /* _LINUX_CHECKPOINT_TYPES_H_ */

include/linux/security.h +128 -0

index f1033a4..61f224f 100644
@@ -554,6 +554,15 @@ static inline void security_free_mnt_opts(struct security_mnt_opts *opts)
  *	created.
  *	@file contains the file structure to secure.
  *	Return 0 if the hook is successful and permission is granted.
+ * @file_get_ctx:
+ *	Return a string representing the security context on a file.
+ *	@security contains the security field.
+ *	Returns a char* which the caller will free, or -error on error.
+ * @file_restore:
+ *	Set a security context on a file according to the checkpointed context.
+ *	@file contains the file.
+ *	@ctx contains a string representation of the checkpointed context.
+ *	Returns 0 on success, -error on failure.
  * @file_free_security:
  *	Deallocate and free any security structures stored in file->f_security.
  *	@file contains the file structure being modified.

@@ -633,6 +642,16 @@ static inline void security_free_mnt_opts(struct security_mnt_opts *opts)
  *	manual page for definitions of the @clone_flags.
  *	@clone_flags contains the flags indicating what should be shared.
  *	Return 0 if permission is granted.
+ * @cred_get_ctx:
+ *	Return a string representing the security context on the task cred.
+ *	@security contains the security field.
+ *	Returns a char* which the caller will free, or -error on error.
+ * @cred_restore:
+ *	Set a security context on a task cred according to the checkpointed
+ *	context.
+ *	@cred contains the cred.
+ *	@ctx contains a string representation of the checkpointed context.
+ *	Returns 0 on success, -error on failure.
  * @cred_free:
  *	@cred points to the credentials.
  *	Deallocate and clear the cred->security field in a set of credentials.

@@ -1081,6 +1100,19 @@ static inline void security_free_mnt_opts(struct security_mnt_opts *opts)
  *	@ipcp contains the kernel IPC permission structure.
  *	@secid contains a pointer to the location where result will be saved.
  *	In case of failure, @secid will be set to zero.
+ * @ipc_get_ctx:
+ *	Return a string representing the security context on the IPC
+ *	permission structure.
+ *	@security contains the security field.
+ *	Returns a char* which the caller will free, or -error on error.
+ * @ipc_restore:
+ *	Set a security context on a IPC permission structure according to
+ *	the checkpointed context.
+ *	@ipcp contains the IPC permission structure, which will have
+ *	already been allocated and initialized when the IPC structure was
+ *	created.
+ *	@ctx contains a string representation of the checkpointed context.
+ *	Returns 0 on success, -error on failure.
  *
  * Security hooks for individual messages held in System V IPC message queues
  * @msg_msg_alloc_security:

@@ -1089,6 +1121,16 @@ static inline void security_free_mnt_opts(struct security_mnt_opts *opts)
  *	created.
  *	@msg contains the message structure to be modified.
  *	Return 0 if operation was successful and permission is granted.
+ * @msg_msg_get_ctx:
+ *	Return a string representing the security context on an msg_msg
+ *	struct.
+ *	@security contains the security field
+ *	Returns a char* which the caller will free, or -error on error.
+ * @msg_msg_restore:
+ *	Set msg_msg->security according to the checkpointed context.
+ *	@msg contains the message structure to be modified.
+ *	@ctx contains a string representation of the checkpointed context.
+ *	Return 0 on success, -error on failure.
  * @msg_msg_free_security:
  *	Deallocate the security structure for this message.
  *	@msg contains the message structure to be modified.

@@ -1443,6 +1485,8 @@ struct security_operations {
 
 	int (*file_permission) (struct file *file, int mask);
 	int (*file_alloc_security) (struct file *file);
+	char *(*file_get_ctx) (void *security);
+	int (*file_restore) (struct file *file, char *ctx);
 	void (*file_free_security) (struct file *file);
 	int (*file_ioctl) (struct file *file, unsigned int cmd,
 			   unsigned long arg);

@@ -1463,6 +1507,8 @@ struct security_operations {
 	int (*dentry_open) (struct file *file, const struct cred *cred);
 
 	int (*task_create) (unsigned long clone_flags);
+	char *(*cred_get_ctx) (void *security);
+	int (*cred_restore) (struct cred *cred, char *ctx);
 	void (*cred_free) (struct cred *cred);
 	int (*cred_prepare)(struct cred *new, const struct cred *old,
 			    gfp_t gfp);

@@ -1496,8 +1542,12 @@ struct security_operations {
 
 	int (*ipc_permission) (struct kern_ipc_perm *ipcp, short flag);
 	void (*ipc_getsecid) (struct kern_ipc_perm *ipcp, u32 *secid);
+	char *(*ipc_get_ctx) (void *security);
+	int (*ipc_restore) (struct kern_ipc_perm *ipcp, char *ctx);
 
 	int (*msg_msg_alloc_security) (struct msg_msg *msg);
+	char *(*msg_msg_get_ctx) (void *security);
+	int (*msg_msg_restore) (struct msg_msg *msg, char *ctx);
 	void (*msg_msg_free_security) (struct msg_msg *msg);
 
 	int (*msg_queue_alloc_security) (struct msg_queue *msq);

@@ -1701,6 +1751,8 @@ int security_inode_listsecurity(struct inode *inode, char *buffer, size_t buffer
 void security_inode_getsecid(const struct inode *inode, u32 *secid);
 int security_file_permission(struct file *file, int mask);
 int security_file_alloc(struct file *file);
+char *security_file_get_ctx(void *security);
+int security_file_restore(struct file *file, char *ctx);
 void security_file_free(struct file *file);
 int security_file_ioctl(struct file *file, unsigned int cmd, unsigned long arg);
 int security_file_mmap(struct file *file, unsigned long reqprot,

@@ -1716,6 +1768,8 @@ int security_file_send_sigiotask(struct task_struct *tsk,
 int security_file_receive(struct file *file);
 int security_dentry_open(struct file *file, const struct cred *cred);
 int security_task_create(unsigned long clone_flags);
+char *security_cred_get_ctx(void *security);
+int security_cred_restore(struct cred *cred, char *ctx);
 void security_cred_free(struct cred *cred);
 int security_prepare_creds(struct cred *new, const struct cred *old, gfp_t gfp);
 void security_commit_creds(struct cred *new, const struct cred *old);

@@ -1746,7 +1800,11 @@ int security_task_prctl(int option, unsigned long arg2, unsigned long arg3,
 void security_task_to_inode(struct task_struct *p, struct inode *inode);
 int security_ipc_permission(struct kern_ipc_perm *ipcp, short flag);
 void security_ipc_getsecid(struct kern_ipc_perm *ipcp, u32 *secid);
+char *security_ipc_get_ctx(void *security);
+int security_ipc_restore(struct kern_ipc_perm *ipcp, char *ctx);
 int security_msg_msg_alloc(struct msg_msg *msg);
+char *security_msg_msg_get_ctx(void *security);
+int security_msg_msg_restore(struct msg_msg *msg, char *ctx);
 void security_msg_msg_free(struct msg_msg *msg);
 int security_msg_queue_alloc(struct msg_queue *msq);
 void security_msg_queue_free(struct msg_queue *msq);

@@ -2190,6 +2248,19 @@ static inline int security_file_alloc(struct file *file)
 	return 0;
 }
 
+static inline char *security_file_get_ctx(void *security)
+{
+	/* this shouldn't ever get called if SECURITY=n */
+	return ERR_PTR(-EINVAL);
+}
+
+static inline int security_file_restore(struct file *file, char *ctx)
+{
+	/* we're asked to recreate security contexts for an LSM which had
+	 * contexts, but CONFIG_SECURITY=n now! */
+	return -EINVAL;
+}
+
 static inline void security_file_free(struct file *file)
 { }
 

@@ -2256,6 +2327,19 @@ static inline int security_task_create(unsigned long clone_flags)
 	return 0;
 }
 
+static inline char *security_cred_get_ctx(void *security)
+{
+	/* this shouldn't ever get called if SECURITY=n */
+	return ERR_PTR(-EINVAL);
+}
+
+static inline int security_cred_restore(struct cred *cred, char *ctx)
+{
+	/* we're asked to recreate security contexts for an LSM which had
+	 * contexts, but CONFIG_SECURITY=n now! */
+	return -EINVAL;
+}
+
 static inline void security_cred_free(struct cred *cred)
 { }
 

@@ -2398,11 +2482,37 @@ static inline void security_ipc_getsecid(struct kern_ipc_perm *ipcp, u32 *secid)
 	*secid = 0;
 }
 
+static inline char *security_ipc_get_ctx(void *security)
+{
+	/* this shouldn't ever get called if SECURITY=n */
+	return ERR_PTR(-EINVAL);
+}
+
+static inline int security_ipc_restore(struct kern_ipc_perm *ipcp, char *ctx)
+{
+	/* we're asked to recreate security contexts for an LSM which had
+	 * contexts, but CONFIG_SECURITY=n now! */
+	return -EINVAL;
+}
+
 static inline int security_msg_msg_alloc(struct msg_msg *msg)
 {
 	return 0;
 }
 
+static inline char *security_msg_msg_get_ctx(void *security)
+{
+	/* this shouldn't ever get called if SECURITY=n */
+	return ERR_PTR(-EINVAL);
+}
+
+static inline int security_msg_msg_restore(struct msg_msg *msg, char *ctx)
+{
+	/* we're asked to recreate security contexts for an LSM which had
+	 * contexts, but CONFIG_SECURITY=n now! */
+	return -EINVAL;
+}
+
 static inline void security_msg_msg_free(struct msg_msg *msg)
 { }
 

@@ -2987,5 +3097,23 @@ static inline void free_secdata(void *secdata)
 { }
 #endif /* CONFIG_SECURITY */
 
+#ifdef CONFIG_CHECKPOINT
+#define CKPT_SECURITY_MSG_MSG	1
+#define CKPT_SECURITY_IPC	2
+#define CKPT_SECURITY_FILE	3
+#define CKPT_SECURITY_CRED	4
+#define CKPT_SECURITY_MAX	4
+
+#ifdef CONFIG_SECURITY
+int security_checkpoint_obj(struct ckpt_ctx *ctx, void *security,
+				unsigned sectype);
+#else
+static inline int security_checkpoint_obj(struct ckpt_ctx *ctx, void *security,
+				unsigned sectype)
+{ return -1; }
+#endif /* CONFIG_SECURITY */
+
+#endif /* CONFIG_CHECKPOINT */
+
 #endif /* ! __LINUX_SECURITY_H */
 

ipc/checkpoint.c +24 -10

index 8e6e9ba..c0d3f13 100644
@@ -31,7 +31,8 @@ static char *ipc_ind_to_str[] = { "sem", "msg", "shm" };
  * Checkpoint
  */
 
-int checkpoint_fill_ipc_perms(struct ckpt_hdr_ipc_perms *h,
+int checkpoint_fill_ipc_perms(struct ckpt_ctx *ctx,
+			      struct ckpt_hdr_ipc_perms *h,
 			      struct kern_ipc_perm *perm)
 {
 	if (ipcperms(perm, S_IROTH))

@@ -45,6 +46,15 @@ int checkpoint_fill_ipc_perms(struct ckpt_hdr_ipc_perms *h,
 	h->cgid = perm->cgid;
 	h->mode = perm->mode & S_IRWXUGO;
 	h->seq = perm->seq;
+	if (perm->security) {
+		h->sec_ref = security_checkpoint_obj(ctx, perm->security,
+					CKPT_SECURITY_IPC);
+		if (h->sec_ref == -EOPNOTSUPP)
+			h->sec_ref = -1;
+		else if (h->sec_ref < 0)
+			return h->sec_ref;
+	} else
+		h->sec_ref = -1;
 
 	return 0;
 }

@@ -176,7 +186,8 @@ static int validate_created_perms(struct ckpt_hdr_ipc_perms *h)
 	return 1;
 }
 
-int restore_load_ipc_perms(struct ckpt_hdr_ipc_perms *h,
+int restore_load_ipc_perms(struct ckpt_ctx *ctx,
+			   struct ckpt_hdr_ipc_perms *h,
 			   struct kern_ipc_perm *perm)
 {
 	if (h->id < 0)

@@ -205,14 +216,17 @@ int restore_load_ipc_perms(struct ckpt_hdr_ipc_perms *h,
 	perm->cgid = h->cgid;
 	perm->mode = h->mode;
 	perm->seq = h->seq;
-	/*
-	 * Todo: restore perm->security.
-	 * At the moment it gets set by security_x_alloc() called through
-	 * ipcget()->ipcget_public()->ops-.getnew (->nequeue for instance)
-	 * We will want to ask the LSM to consider resetting the
-	 * checkpointed ->security, based on current_security(),
-	 * the checkpointed ->security, and the checkpoint file context.
-	 */
+
+	if ((ctx->uflags & RESTART_KEEP_LSM) && (h->sec_ref != -1)) {
+		int ret;
+		struct ckpt_stored_lsm *l = ckpt_obj_fetch(ctx, h->sec_ref,
+					CKPT_OBJ_SEC);
+		if (IS_ERR(l))
+			return PTR_ERR(l);
+		ret = security_ipc_restore(perm, l->string);
+		if (ret)
+			return ret;
+	}
 
 	return 0;
 }

ipc/checkpoint_msg.c +23 -3

index b933c19..24f4097 100644
@@ -37,7 +37,7 @@ static int fill_ipc_msg_hdr(struct ckpt_ctx *ctx,
 
 	ipc_lock_by_ptr(&msq->q_perm);
 
-	ret = checkpoint_fill_ipc_perms(&h->perms, &msq->q_perm);
+	ret = checkpoint_fill_ipc_perms(ctx, &h->perms, &msq->q_perm);
 	if (ret < 0)
 		goto unlock;
 

@@ -64,13 +64,23 @@ static int checkpoint_msg_contents(struct ckpt_ctx *ctx, struct msg_msg *msg)
 	struct msg_msgseg *seg;
 	int total, len;
 	int ret;
-
+	int sec_ref = -1;
+
+	if (msg->security) {
+		sec_ref = security_checkpoint_obj(ctx, msg->security,
+						CKPT_SECURITY_MSG_MSG);
+		if (sec_ref == -EOPNOTSUPP)
+			sec_ref = -1;
+		else if (sec_ref < 0)
+			return sec_ref;
+	}
 	h = ckpt_hdr_get_type(ctx, sizeof(*h), CKPT_HDR_IPC_MSG_MSG);
 	if (!h)
 		return -ENOMEM;
 
 	h->m_type = msg->m_type;
 	h->m_ts = msg->m_ts;
+	h->sec_ref = sec_ref;
 
 	ret = ckpt_write_obj(ctx, &h->h);
 	ckpt_hdr_put(ctx, h);

@@ -177,7 +187,7 @@ static int load_ipc_msg_hdr(struct ckpt_ctx *ctx,
 {
 	int ret = 0;
 
-	ret = restore_load_ipc_perms(&h->perms, &msq->q_perm);
+	ret = restore_load_ipc_perms(ctx, &h->perms, &msq->q_perm);
 	if (ret < 0)
 		return ret;
 

@@ -224,6 +234,16 @@ static struct msg_msg *restore_msg_contents_one(struct ckpt_ctx *ctx, int *clen)
 	msg->next = NULL;
 	pseg = &msg->next;
 
+	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;
+		ret = security_msg_msg_restore(msg, l->string);
+		if (ret)
+			return ERR_PTR(ret);
+	}
+
 	ret = _ckpt_read_buffer(ctx, (msg + 1), len);
 	if (ret < 0)
 		goto out;

ipc/checkpoint_sem.c +2 -2

index 76eb2b9..53a19ed 100644
@@ -37,7 +37,7 @@ static int fill_ipc_sem_hdr(struct ckpt_ctx *ctx,
 
 	ipc_lock_by_ptr(&sem->sem_perm);
 
-	ret = checkpoint_fill_ipc_perms(&h->perms, &sem->sem_perm);
+	ret = checkpoint_fill_ipc_perms(ctx, &h->perms, &sem->sem_perm);
 	if (ret < 0)
 		goto unlock;
 

@@ -113,7 +113,7 @@ static int load_ipc_sem_hdr(struct ckpt_ctx *ctx,
 {
 	int ret = 0;
 
-	ret = restore_load_ipc_perms(&h->perms, &sem->sem_perm);
+	ret = restore_load_ipc_perms(ctx, &h->perms, &sem->sem_perm);
 	if (ret < 0)
 		return ret;
 

ipc/checkpoint_shm.c +2 -2

index ad78aa3..571d9bb 100644
@@ -41,7 +41,7 @@ static int fill_ipc_shm_hdr(struct ckpt_ctx *ctx,
 
 	ipc_lock_by_ptr(&shp->shm_perm);
 
-	ret = checkpoint_fill_ipc_perms(&h->perms, &shp->shm_perm);
+	ret = checkpoint_fill_ipc_perms(ctx, &h->perms, &shp->shm_perm);
 	if (ret < 0)
 		goto unlock;
 

@@ -166,7 +166,7 @@ static int load_ipc_shm_hdr(struct ckpt_ctx *ctx,
 {
 	int ret;
 
-	ret = restore_load_ipc_perms(&h->perms, &shp->shm_perm);
+	ret = restore_load_ipc_perms(ctx, &h->perms, &shp->shm_perm);
 	if (ret < 0)
 		return ret;
 

ipc/util.h +4 -2

index aa35aaa..93a1ba3 100644
@@ -199,9 +199,11 @@ void freeary(struct ipc_namespace *ns, struct kern_ipc_perm *ipcp);
 
 
 #ifdef CONFIG_CHECKPOINT
-extern int checkpoint_fill_ipc_perms(struct ckpt_hdr_ipc_perms *h,
+extern int checkpoint_fill_ipc_perms(struct ckpt_ctx *ctx,
+				     struct ckpt_hdr_ipc_perms *h,
 				     struct kern_ipc_perm *perm);
-extern int restore_load_ipc_perms(struct ckpt_hdr_ipc_perms *h,
+extern int restore_load_ipc_perms(struct ckpt_ctx *ctx,
+				  struct ckpt_hdr_ipc_perms *h,
 				  struct kern_ipc_perm *perm);
 
 extern int checkpoint_ipc_shm(int id, void *p, void *data);

kernel/cred.c +19 -1

index 27e02ca..e43ba45 100644
@@ -709,7 +709,7 @@ int cred_setfsgid(struct cred *new, gid_t gid, gid_t *old_fsgid)
 static int do_checkpoint_cred(struct ckpt_ctx *ctx, const struct cred *cred)
 {
 	int ret;
-	int groupinfo_ref, user_ref;
+	int groupinfo_ref, user_ref, sec_ref = -1;
 	struct ckpt_hdr_cred *h;
 
 	groupinfo_ref = checkpoint_obj(ctx, cred->group_info,

@@ -719,6 +719,14 @@ static int do_checkpoint_cred(struct ckpt_ctx *ctx, const struct cred *cred)
 	user_ref = checkpoint_obj(ctx, cred->user, CKPT_OBJ_USER);
 	if (user_ref < 0)
 		return user_ref;
+#ifdef CONFIG_SECURITY
+	sec_ref = security_checkpoint_obj(ctx, cred->security,
+					CKPT_SECURITY_CRED);
+	if (sec_ref == -EOPNOTSUPP)
+		sec_ref = -1;
+	else if (sec_ref < 0)
+		return sec_ref;
+#endif  /* else cred->security doesn't exist and sec_ref = -1 */
 
 	h = ckpt_hdr_get_type(ctx, sizeof(*h), CKPT_HDR_CRED);
 	if (!h)

@@ -733,6 +741,7 @@ static int do_checkpoint_cred(struct ckpt_ctx *ctx, const struct cred *cred)
 	h->sgid = cred->sgid;
 	h->egid = cred->egid;
 	h->fsgid = cred->fsgid;
+	h->sec_ref = sec_ref;
 
 	checkpoint_capabilities(&h->cap_s, cred);
 

@@ -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;
+		ret = security_cred_restore(cred, l->string);
+		if (ret)
+			goto err_putcred;
+	}
 	ret = restore_capabilities(&h->cap_s, cred);
 	if (ret)
 		goto err_putcred;

security/capability.c +48 -0

index 21b6cea..28e6495 100644
@@ -315,6 +315,16 @@ static int cap_file_permission(struct file *file, int mask)
 	return 0;
 }
 
+static inline char *cap_file_get_ctx(void *security)
+{
+	return ERR_PTR(-EOPNOTSUPP);
+}
+
+static int cap_file_restore(struct file *file, char *ctx)
+{
+	return -EOPNOTSUPP;
+}
+
 static int cap_file_alloc_security(struct file *file)
 {
 	return 0;

@@ -382,6 +392,16 @@ static int cap_task_create(unsigned long clone_flags)
 	return 0;
 }
 
+static char *cap_cred_get_ctx(void *security)
+{
+	return ERR_PTR(-EOPNOTSUPP);
+}
+
+static int cap_cred_restore(struct cred *cred, char *ctx)
+{
+	return -EOPNOTSUPP;
+}
+
 static void cap_cred_free(struct cred *cred)
 {
 }

@@ -485,11 +505,31 @@ static void cap_ipc_getsecid(struct kern_ipc_perm *ipcp, u32 *secid)
 	*secid = 0;
 }
 
+static char *cap_ipc_get_ctx(void *security)
+{
+	return ERR_PTR(-EOPNOTSUPP);
+}
+
+static int cap_ipc_restore(struct kern_ipc_perm *ipcp, char *ctx)
+{
+	return -EOPNOTSUPP;
+}
+
 static int cap_msg_msg_alloc_security(struct msg_msg *msg)
 {
 	return 0;
 }
 
+static inline char *cap_msg_msg_get_ctx(void *security)
+{
+	return ERR_PTR(-EOPNOTSUPP);
+}
+
+static int cap_msg_msg_restore(struct msg_msg *msg, char *ctx)
+{
+	return -EOPNOTSUPP;
+}
+
 static void cap_msg_msg_free_security(struct msg_msg *msg)
 {
 }

@@ -937,6 +977,8 @@ void security_fixup_ops(struct security_operations *ops)
 	set_to_cap_if_null(ops, path_truncate);
 #endif
 	set_to_cap_if_null(ops, file_permission);
+	set_to_cap_if_null(ops, file_get_ctx);
+	set_to_cap_if_null(ops, file_restore);
 	set_to_cap_if_null(ops, file_alloc_security);
 	set_to_cap_if_null(ops, file_free_security);
 	set_to_cap_if_null(ops, file_ioctl);

@@ -949,6 +991,8 @@ void security_fixup_ops(struct security_operations *ops)
 	set_to_cap_if_null(ops, file_receive);
 	set_to_cap_if_null(ops, dentry_open);
 	set_to_cap_if_null(ops, task_create);
+	set_to_cap_if_null(ops, cred_get_ctx);
+	set_to_cap_if_null(ops, cred_restore);
 	set_to_cap_if_null(ops, cred_free);
 	set_to_cap_if_null(ops, cred_prepare);
 	set_to_cap_if_null(ops, cred_commit);

@@ -975,7 +1019,11 @@ void security_fixup_ops(struct security_operations *ops)
 	set_to_cap_if_null(ops, task_to_inode);
 	set_to_cap_if_null(ops, ipc_permission);
 	set_to_cap_if_null(ops, ipc_getsecid);
+	set_to_cap_if_null(ops, ipc_get_ctx);
+	set_to_cap_if_null(ops, ipc_restore);
 	set_to_cap_if_null(ops, msg_msg_alloc_security);
+	set_to_cap_if_null(ops, msg_msg_get_ctx);
+	set_to_cap_if_null(ops, msg_msg_restore);
 	set_to_cap_if_null(ops, msg_msg_free_security);
 	set_to_cap_if_null(ops, msg_queue_alloc_security);
 	set_to_cap_if_null(ops, msg_queue_free_security);

security/security.c +76 -0

index 3829156..6bafb9e 100644
@@ -630,6 +630,16 @@ int security_file_alloc(struct file *file)
 	return security_ops->file_alloc_security(file);
 }
 
+char *security_file_get_ctx(void *security)
+{
+	return security_ops->file_get_ctx(security);
+}
+
+int security_file_restore(struct file *file, char *ctx)
+{
+	return security_ops->file_restore(file, ctx);
+}
+
 void security_file_free(struct file *file)
 {
 	security_ops->file_free_security(file);

@@ -689,6 +699,16 @@ int security_task_create(unsigned long clone_flags)
 	return security_ops->task_create(clone_flags);
 }
 
+char *security_cred_get_ctx(void *security)
+{
+	return security_ops->cred_get_ctx(security);
+}
+
+int security_cred_restore(struct cred *cred, char *ctx)
+{
+	return security_ops->cred_restore(cred, ctx);
+}
+
 void security_cred_free(struct cred *cred)
 {
 	security_ops->cred_free(cred);

@@ -824,11 +844,31 @@ void security_ipc_getsecid(struct kern_ipc_perm *ipcp, u32 *secid)
 	security_ops->ipc_getsecid(ipcp, secid);
 }
 
+char *security_ipc_get_ctx(void *security)
+{
+	return security_ops->ipc_get_ctx(security);
+}
+
+int security_ipc_restore(struct kern_ipc_perm *ipcp, char *ctx)
+{
+	return security_ops->ipc_restore(ipcp, ctx);
+}
+
 int security_msg_msg_alloc(struct msg_msg *msg)
 {
 	return security_ops->msg_msg_alloc_security(msg);
 }
 
+char *security_msg_msg_get_ctx(void *security)
+{
+	return security_ops->msg_msg_get_ctx(security);
+}
+
+int security_msg_msg_restore(struct msg_msg *msg, char *ctx)
+{
+	return security_ops->msg_msg_restore(msg, ctx);
+}
+
 void security_msg_msg_free(struct msg_msg *msg)
 {
 	security_ops->msg_msg_free_security(msg);

@@ -1249,3 +1289,68 @@ int security_audit_rule_match(u32 secid, u32 field, u32 op, void *lsmrule,
 }
 
 #endif /* CONFIG_AUDIT */
+
+#ifdef CONFIG_CHECKPOINT
+
+#include <linux/checkpoint.h>
+
+/**
+ * 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;
+
+	switch (sectype) {
+	case CKPT_SECURITY_MSG_MSG:
+		str = security_msg_msg_get_ctx(security);
+		break;
+	case CKPT_SECURITY_IPC:
+		str = security_ipc_get_ctx(security);
+		break;
+	case CKPT_SECURITY_FILE:
+		str = security_file_get_ctx(security);

security/security.c +19 -0

+	if (IS_ERR(str))
+		return PTR_ERR(str);
+
+	len = strlen(str);
+	l = kzalloc(sizeof(*l) + len + 1, GFP_KERNEL);
+	if (!l) {
+		kfree(str);
+		return -ENOMEM;
+	}
+	kref_init(&l->kref);
+	l->sectype = sectype;
+	l->string = str;
+
+	strref = checkpoint_obj(ctx, l, CKPT_OBJ_SEC);
+	/* do we need to free l if strref was err? */
+	return strref;
+}
+
+#endif

Serge E. Hallyn wrote:

Quoted 6 lines

Briefly, all security fields must be exported by the LSM as a simple null-terminated string. They are checkpointed through the security_checkpoint_obj() helper, because we must pass it an extra sectype field. Splitting SECURITY_OBJ_SEC into one type per object type would not work because, in Smack, one void* security is used for all object types.

I do not understand why the Smack behavior is a limitation here.

Quoted 2 lines

But we must pass the sectype field because in
SELinux a different type of structure is stashed in each object type.

But each can be expressed as a context, can't it?

security/security.c int security_audit_rule_match(u32 secid, u32 field, u32 op, void *lsmrule, · line 1337

Quoted 3 lines

security/security.c +2 -0

+	/* str will be alloc'ed for us by the LSM.  We will free it when
+	 * we clear out our hashtable */

Why do you think that you need a copy? Sure, SELinux always gives you a copy, but Smack keeps "contexts" around and making a copy is not only unnecessary, but wasteful. If you free the "context" with the appropriate call (security_release_secctx) you will get the "free allocated memory" behavior desired by SELinux and the "do nothing" behavior of Smack. For free, assuming that you also fix your Smack hook so that it works in the way Smack deems "Correct".

Quoted 2 lines · Casey Schaufler

Serge E. Hallyn wrote:

...

Quoted 7 lines

Quoted 7 lines

Briefly, all security fields must be exported by the LSM as a simple null-terminated string. They are checkpointed through the security_checkpoint_obj() helper, because we must pass it an extra sectype field. Splitting SECURITY_OBJ_SEC into one type per object type would not work because, in Smack, one void* security is used for all object types.

Quoted 2 lines

I do not understand why the Smack behavior is a limitation here.

Well it's not the Smack behavior, it's the combination of Smack's and SELinux's behavior :)

In the end what I have here is probably best anyway - what is stored in the object hash is not a security struct as known by any LSM, but a generic object label. So at object_hash_types[CKPT_OBJ_SEC]->restore(), we don't re-create an actual security struct, but just a string which is later used by security_xyztype_restore() to create an actual label.

Quoted 6 lines

Quoted 4 lines

But we must pass the sectype field because in
SELinux a different type of structure is stashed in each object type.

But each can be expressed as a context, can't it?

A set of contexts (root_u:root_r:root_t:::system_u:system_r\
:system_t::...).

There would be a problem if it were stored as a more structured type, and if the ->restore handler wanted to re-create an actual task_security_struct, ipc_security_struct, etc. So the last paragraph in the patch intro was just trying to explain why the intermediate layer, storing a generic string on the c/r object hash, needs to be there. The thing that is not possible is to place the actual void *security or a struct task_security_struct on the objhash.

...

Quoted 12 lines

security/security.c int security_audit_rule_match(u32 secid, u32 field, u32 op, void *lsmrule, · line 1337

Quoted 4 lines

security/security.c +2 -0

+	/* str will be alloc'ed for us by the LSM.  We will free it when
+	 * we clear out our hashtable */

Why do you think that you need a copy? Sure, SELinux always gives you a copy, but Smack keeps "contexts" around and making a copy is not only unnecessary, but wasteful. If you free the "context" with the appropriate call (security_release_secctx) you will get the "free allocated memory" behavior desired by SELinux and the "do nothing" behavior of Smack. For free, assuming that you also fix your Smack hook so that it works in the way Smack deems "Correct".

Hmm, that should be doable. Mind you these are not the same as secctx's returned by secid_to_secctx.

Though selinux_release_secctx
is implemented as a simple kfree, so it would 'just work.' I'm not sure if it's better to just re-use it, or introduce a new security_release_context() that does the same thing...

thanks,
-serge

Quoted 7 lines · Serge E. Hallyn

Quoted 2 lines · Casey Schaufler

But each can be expressed as a context, can't it?

A set of contexts (root_u:root_r:root_t:::system_u:system_r\
:system_t::...).

Quoted 9 lines

There would be a problem if it were stored as a more structured type, and if the ->restore handler wanted to re-create an actual task_security_struct, ipc_security_struct, etc. So the last paragraph in the patch intro was just trying to explain why the intermediate layer, storing a generic string on the c/r object hash, needs to be there. The thing that is not possible is to place the actual void *security or a struct task_security_struct on the objhash.

Right. Now why do you need a set of contexts?

Quoted 16 lines

...

Quoted 12 lines

security/security.c int security_audit_rule_match(u32 secid, u32 field, u32 op, void *lsmrule, · line 1337

Quoted 4 lines

security/security.c +2 -0

+	/* str will be alloc'ed for us by the LSM.  We will free it when
+	 * we clear out our hashtable */

Why do you think that you need a copy? Sure, SELinux always gives you a copy, but Smack keeps "contexts" around and making a copy is not only unnecessary, but wasteful. If you free the "context" with the appropriate call (security_release_secctx) you will get the "free allocated memory" behavior desired by SELinux and the "do nothing" behavior of Smack. For free, assuming that you also fix your Smack hook so that it works in the way Smack deems "Correct".

Quoted 2 lines

Hmm, that should be doable. Mind you these are not the same as secctx's returned by secid_to_secctx.

Now why is that? If they are different things, what are they?

What is the difference between a secctx and a context? I got a bit confused because the word "context" has been used to refer to the thing represented by a secctx for a long time.

Quoted 20 lines · Casey Schaufler

Quoted 18 lines · Serge E. Hallyn

Quoted 3 lines · Casey Schaufler

But each can be expressed as a context, can't it?

A set of contexts (root_u:root_r:root_t:::system_u:system_r\
:system_t::...).

There would be a problem if it were stored as a more structured type, and if the ->restore handler wanted to re-create an actual task_security_struct, ipc_security_struct, etc. So the last paragraph in the patch intro was just trying to explain why the intermediate layer, storing a generic string on the c/r object hash, needs to be there. The thing that is not possible is to place the actual void *security or a struct task_security_struct on the objhash.

Quoted 2 lines

Right. Now why do you need a set of contexts?

Because for SELinux, for instance, when checkpointing a security context for a task, we want to checkpoint the actual context, the fscreate context, the sockcreate context, keycreate context, and the task create (exec_create) context.

Quoted 21 lines

Quoted 18 lines

...

Quoted 4 lines

Quoted 4 lines

security/security.c +2 -0

+	/* str will be alloc'ed for us by the LSM.  We will free it when
+	 * we clear out our hashtable */

Quoted 10 lines

Quoted 1 line

Why do you think that you need a copy? Sure, SELinux always gives you a copy, but Smack keeps "contexts" around and making a copy is not only unnecessary, but wasteful. If you free the "context" with the appropriate call (security_release_secctx) you will get the "free allocated memory" behavior desired by SELinux and the "do nothing" behavior of Smack. For free, assuming that you also fix your Smack hook so that it works in the way Smack deems "Correct".

Quoted 3 lines

Hmm, that should be doable. Mind you these are not the same as secctx's returned by secid_to_secctx.

Quoted 7 lines

Now why is that? If they are different things, what are they?

What is the difference between a secctx and a context? I got a bit confused because the word "context" has been used to refer to the thing represented by a secctx for a long time.

I know, I know, I should come up with a better name. But while an selinux context would be

root_u:root_r:root_t

the blob I have to checkpoint for a task would perhaps be

root_u:root_r:root_t:::null:::null::null:::user_u:serge_r:serge_t:::null

Quoted 6 lines · Serge E. Hallyn

Quoting Casey Schaufler ([email protected]):
I know, I know, I should come up with a better name. But while an selinux context would be

root_u:root_r:root_t

Quoted 4 lines

the blob I have to checkpoint for a task would perhaps be

root_u:root_r:root_t:::null:::null::null:::user_u:serge_r:serge_t:::null

What you really want is a textual representation of the security blob if I read this correctly.

Seems like you could call this either a
"blob string" or a "context collection" or a "checkpoint string".

Quoted 25 lines · Serge E. Hallyn

Quoted 22 lines · Casey Schaufler

Quoted 8 lines · Serge E. Hallyn

Quoted 3 lines · Casey Schaufler

But each can be expressed as a context, can't it?

A set of contexts (root_u:root_r:root_t:::system_u:system_r\
:system_t::...).

Quoted 10 lines

There would be a problem if it were stored as a more structured type, and if the ->restore handler wanted to re-create an actual task_security_struct, ipc_security_struct, etc. So the last paragraph in the patch intro was just trying to explain why the intermediate layer, storing a generic string on the c/r object hash, needs to be there. The thing that is not possible is to place the actual void *security or a struct task_security_struct on the objhash.

Right. Now why do you need a set of contexts?

Quoted 5 lines

Because for SELinux, for instance, when checkpointing a security context for a task, we want to checkpoint the actual context, the fscreate context, the sockcreate context, keycreate context, and the task create (exec_create) context.

My. That is quite a lot of contexts to keep track of.

Quoted 38 lines · Casey Schaufler

Quoted 36 lines · Serge E. Hallyn

Quoted 24 lines · Casey Schaufler

Quoted 10 lines · Serge E. Hallyn

Quoted 4 lines · Casey Schaufler

But each can be expressed as a context, can't it?

A set of contexts (root_u:root_r:root_t:::system_u:system_r\
:system_t::...).

Quoted 11 lines

There would be a problem if it were stored as a more structured type, and if the ->restore handler wanted to re-create an actual task_security_struct, ipc_security_struct, etc. So the last paragraph in the patch intro was just trying to explain why the intermediate layer, storing a generic string on the c/r object hash, needs to be there. The thing that is not possible is to place the actual void *security or a struct task_security_struct on the objhash.

Quoted 3 lines

Right. Now why do you need a set of contexts?

Because for SELinux, for instance, when checkpointing a security context for a task, we want to checkpoint the actual context, the fscreate context, the sockcreate context, keycreate context, and the task create (exec_create) context.

Quoted 2 lines

My. That is quite a lot of contexts to keep track of.

Doesn't Smack also have at least one case where it supports multiple distinct contexts for different purposes on a single object (e.g. sockets)?

Quoted 40 lines · Stephen Smalley

Quoted 38 lines · Casey Schaufler

Quoted 36 lines · Serge E. Hallyn

Quoted 24 lines · Casey Schaufler

Quoted 10 lines · Serge E. Hallyn

Quoted 4 lines · Casey Schaufler

But each can be expressed as a context, can't it?

A set of contexts (root_u:root_r:root_t:::system_u:system_r\
:system_t::...).

Quoted 11 lines

There would be a problem if it were stored as a more structured type, and if the ->restore handler wanted to re-create an actual task_security_struct, ipc_security_struct, etc. So the last paragraph in the patch intro was just trying to explain why the intermediate layer, storing a generic string on the c/r object hash, needs to be there. The thing that is not possible is to place the actual void *security or a struct task_security_struct on the objhash.

Quoted 3 lines

Right. Now why do you need a set of contexts?

Because for SELinux, for instance, when checkpointing a security context for a task, we want to checkpoint the actual context, the fscreate context, the sockcreate context, keycreate context, and the task create (exec_create) context.

Quoted 7 lines

Quoted 2 lines

My. That is quite a lot of contexts to keep track of.

Doesn't Smack also have at least one case where it supports multiple distinct contexts for different purposes on a single object (e.g. sockets)?

Smack does support associating labels with incoming and/or outgoing packets from a particular socket. I haven't looked carefully at how the checkpoint/restart scheme is handling sockets in general, so I couldn't say if its going to get the

rest of it right, either. In any case, that's something that is strictly in the realm of privileged processes for which checkpoint/restart is going to be one hairy potato.

Processes that use this mechanism will be quite rare. The Smack port multiplexer (smackpolyport) that I'll be talking about in Portland and a small set of security enforcing applications should be about it.

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html