Serge E. Hallyn wrote:
> 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.
>
> 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)
[...]
> oct 19: At checkpoint, we insert the void* security into the
> objhash. The first time that we do so, we next write out
> the string representation of the context to the checkpoint
> image, along with the value of the objref for the void*
> security, and insert that into the objhash. Then at
> restart, when we read a LSM context, we read the objref
> which the void* security had at checkpoint, and we then
> insert the string context with that objref as well.
I hoped to see similar comment inlined in the code.
[...]
> @@ -159,8 +175,12 @@ int checkpoint_file_common(struct ckpt_ctx *ctx, struct file *file,
> if (h->f_credref < 0)
> return h->f_credref;
>
> - ckpt_debug("file %s credref %d", file->f_dentry->d_name.name,
> - h->f_credref);
> + ret = checkpoint_file_security(ctx, file, &h->f_secref);
> + if (ret < 0)
> + return ret;
How about a ckpt_write_err() here, or in checkpoint_file_security(),
or in security_checkpoint_obj(), or all of the above.
> +
> + ckpt_debug("file %s credref %d secref %d\n",
> + file->f_dentry->d_name.name, h->f_credref, h->f_secref);
[...]
> @@ -433,6 +464,22 @@ static struct ckpt_obj_ops ckpt_obj_ops[] = {
> .checkpoint = checkpoint_tty,
> .restore = restore_tty,
> },
> + /* LSM void *security on objhash - at checkpoint */
> + {
> + .obj_name = "SECURITY PTR",
> + .obj_type = CKPT_OBJ_SECURITY_PTR,
> + .ref_drop = obj_no_drop,
> + .ref_grab = obj_no_grab,
> + },
I really wish there was a comment explaining why it's ok to not
drop/grab the reference (because the security is always referenced
by an object that is itself grabbed, e.g. file, ipc etc ?).
> + /* LSM security strings - at restart */
> + {
> + .obj_name = "SECURITY STRING",
> + .obj_type = CKPT_OBJ_SECURITY,
> + .ref_grab = lsm_string_grab,
> + .ref_drop = lsm_string_drop,
> + .checkpoint = checkpoint_lsm_string,
> + .restore = restore_lsm_string_wrap,
> + },
> };
[...]
> @@ -376,6 +383,16 @@ struct ckpt_hdr_groupinfo {
> __u32 groups[0];
> } __attribute__((aligned(8)));
>
> +struct ckpt_hdr_lsm {
> + struct ckpt_hdr h;
> + __s32 ptrref;
> + __u8 sectype;
> + __u8 padding;
I don't think padding is necessary here...
> + /*
> + * This is followed by a string of size len+1,
> + * null-terminated
> + */
> +} __attribute__((aligned(8)));
[...]
> +/**
> + * 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
> + * @secref: We return the objref here
> + * different types of data for different types of objects.
> + *
> + * Returns the objref of the checkpointed ckpt_lsm_string 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,
> + int sectype, int *secref)
This function returns 0 for success or a negative error. It should
return the @secref instead of passing it by reference (see your
description of the return value above !)
[...]
> + /* Ask the LSM to apply a void*security to the object
> + * based on the checkpointed context string */
> + switch (sectype) {
> + case CKPT_SECURITY_IPC:
> + ret = security_ipc_restore(v, l->string);
Nit: I know it's not strictly necessary, but adding an explicit
type conversion here and in the other three seems cleaner
> + break;
> + case CKPT_SECURITY_MSG_MSG:
> + ret = security_msg_msg_restore(v, l->string);
> + break;
> + case CKPT_SECURITY_FILE:
> + ret = security_file_restore(v, l->string);
> + break;
> + case CKPT_SECURITY_CRED:
> + ret = security_cred_restore(ctx->file, v, l->string);
> + break;
> + default:
> + ret = -EINVAL;
> + }
> + if (ret)
> + ckpt_debug("sectype %d objref %d lsm's hook returned %d\n",
> + sectype, secref, ret);
> + if (ret == -EOPNOTSUPP)
> + ret = 0;
If you silently ignore EOPNOTSUPP, how can a user tell if a restarted
succeeded fully or by silently skipping the security part ?
Should the user approve this behavior (e.g. RESTART_SECURITY_LAX...) ?
Also, I suggest the s/EOPNOTSUPP/ENOSYS/ all over the patch.
> +
> + return ret;
> +}
> +#endif
Oren.
_______________________________________________
Containers mailing list
Containers@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/containers
