| Message ID | 20190521000645.16227-2-prsriva02@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | add new ima hook ima_kexec_cmdline to measure kexec boot cmdline args | expand |
Hi Prakhar, On Mon, 2019-05-20 at 17:06 -0700, Prakhar Srivastava wrote: > Currently during kexec_file_load(soft reboot) the cmdline args > passed are not measured and the PCR values are not reset. This patch addresses not measuring the kexec boot cmdline. I don't see a reason for mentioning anything about the PCR values not being reset. Keep it simple. > This results in the new kernel to assume a secure boot was > followed. The boot args used to launch the new kernel need to be > measured and carried over to the next kernel to be used for > attestation. IMA supports only measuring files, no functionality > exists to measure a buffer(kexec cmdline). > > This change adds a new functionality to measure buffers > process_buffer_measurement which uses the hash of the buffer > instead of file hash to add an entry in the ima log. > A new ima hook ima_kexec_cmdline is also defined which calls > into process_buffer_measurement to add the kexec_cmdline args > to the log. > > A new policy KEXEC_CMDLINE is also defined to control measuring the > kexec_cmdline buffer. > This patch only adds IMA_MEASURE as a supported functionality. > > - A new ima hook ima_kexec_cmdline is defined to be called by the > kexec code. > - A new function process_buffer_measurement is defined to measure > the buffer hash into the ima log. > - A new func policy KEXEC_CMDLINE is defined to control the measurement. Missing is how to verify the digest of the measurement list kexec boot cmdline entry based on /proc/cmdline. Everything before the "root=" in the /proc entry needs to be removed before calculating the hash. Please include a sample shell command. Matthew's patch "IMA: Allow profiles to define the desired IMA template" will require changes to this patch. Please rebase this patch set on top of it, once Matthew addresses the comments and re- posts. (Reminder: the patch description should be 70 - 75 characters.) > > Signed-off-by: Prakhar Srivastava <prsriva02@gmail.com> > --- < snip > > --- a/security/integrity/ima/ima_main.c > +++ b/security/integrity/ima/ima_main.c > @@ -576,6 +576,83 @@ int ima_load_data(enum kernel_load_data_id id) > return 0; > } > > +/* > + * process_buffer_measurement - Measure the buffer to ima log. > + * @buf: pointer to the buffer that needs to be added to the log. > + * @size: size of buffer(in bytes). > + * @eventname: event name to be used for the buffer entry. > + * @cred: a pointer to a credentials structure for user validation. > + * @secid: the secid of the task to be validated. > + * > + * Based on policy, the buffer is measured into the ima log. > + */ > +static void process_buffer_measurement(const void *buf, int size, > + const char *eventname, const struct cred *cred, > + u32 secid) > +{ > + int ret = 0; > + struct ima_template_entry *entry = NULL; > + struct integrity_iint_cache tmp_iint, *iint = &tmp_iint; > + struct ima_event_data event_data = {iint, NULL, NULL, > + NULL, 0, NULL}; Thiago's clean up patch initializes only specific variables, as needed. Please initialize event_data like: struct ima_event_data event_data = {.iint = iint}; > + struct { > + struct ima_digest_data hdr; > + char digest[IMA_MAX_DIGEST_SIZE]; > + } hash; > + int violation = 0; > + int pcr = CONFIG_IMA_MEASURE_PCR_IDX; > + int action = 0; > + > + action = ima_get_action(NULL, cred, secid, 0, KEXEC_CMDLINE, &pcr); > + if (!(action & IMA_MEASURE)) > + goto out; > + > + memset(iint, 0, sizeof(*iint)); > + memset(&hash, 0, sizeof(hash)); > + > + event_data.filename = eventname; > + > + iint->ima_hash = &hash.hdr; > + iint->ima_hash->algo = ima_hash_algo; > + iint->ima_hash->length = hash_digest_size[ima_hash_algo]; > + > + ret = ima_calc_buffer_hash(buf, size, iint->ima_hash); > + if (ret < 0) > + goto out; > + > + ret = ima_alloc_init_template(&event_data, &entry); > + if (ret < 0) > + goto out; > + > + if (action & IMA_MEASURE) > + ret = ima_store_template(entry, violation, NULL, buf, pcr); > + > + if (ret < 0) { > + ima_free_template_entry(entry); > + } Remove brackets. Mimi > + > +out: > + return; > +}
diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy index 74c6702de74e..62e7cd687e9c 100644 --- a/Documentation/ABI/testing/ima_policy +++ b/Documentation/ABI/testing/ima_policy @@ -29,6 +29,7 @@ Description: base: func:= [BPRM_CHECK][MMAP_CHECK][CREDS_CHECK][FILE_CHECK][MODULE_CHECK] [FIRMWARE_CHECK] [KEXEC_KERNEL_CHECK] [KEXEC_INITRAMFS_CHECK] + [KEXEC_CMDLINE] mask:= [[^]MAY_READ] [[^]MAY_WRITE] [[^]MAY_APPEND] [[^]MAY_EXEC] fsmagic:= hex value diff --git a/include/linux/ima.h b/include/linux/ima.h index dc12fbcf484c..2e2c77280be8 100644 --- a/include/linux/ima.h +++ b/include/linux/ima.h @@ -26,6 +26,7 @@ extern int ima_read_file(struct file *file, enum kernel_read_file_id id); extern int ima_post_read_file(struct file *file, void *buf, loff_t size, enum kernel_read_file_id id); extern void ima_post_path_mknod(struct dentry *dentry); +extern void ima_kexec_cmdline(const void *buf, int size); #ifdef CONFIG_IMA_KEXEC extern void ima_add_kexec_buffer(struct kimage *image); @@ -92,6 +93,7 @@ static inline void ima_post_path_mknod(struct dentry *dentry) return; } +static inline void ima_kexec_cmdline(const void *buf, int size) {} #endif /* CONFIG_IMA */ #ifndef CONFIG_IMA_KEXEC diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index d213e835c498..226a26d8de09 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -184,6 +184,7 @@ static inline unsigned long ima_hash_key(u8 *digest) hook(KEXEC_KERNEL_CHECK) \ hook(KEXEC_INITRAMFS_CHECK) \ hook(POLICY_CHECK) \ + hook(KEXEC_CMDLINE) \ hook(MAX_CHECK) #define __ima_hook_enumify(ENUM) ENUM, diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c index c7505fb122d4..800d965232e5 100644 --- a/security/integrity/ima/ima_api.c +++ b/security/integrity/ima/ima_api.c @@ -169,6 +169,7 @@ void ima_add_violation(struct file *file, const unsigned char *filename, * subj=, obj=, type=, func=, mask=, fsmagic= * subj,obj, and type: are LSM specific. * func: FILE_CHECK | BPRM_CHECK | CREDS_CHECK | MMAP_CHECK | MODULE_CHECK + * | KEXEC_CMDLINE * mask: contains the permission mask * fsmagic: hex value * diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 357edd140c09..a88c28918a63 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -576,6 +576,83 @@ int ima_load_data(enum kernel_load_data_id id) return 0; } +/* + * process_buffer_measurement - Measure the buffer to ima log. + * @buf: pointer to the buffer that needs to be added to the log. + * @size: size of buffer(in bytes). + * @eventname: event name to be used for the buffer entry. + * @cred: a pointer to a credentials structure for user validation. + * @secid: the secid of the task to be validated. + * + * Based on policy, the buffer is measured into the ima log. + */ +static void process_buffer_measurement(const void *buf, int size, + const char *eventname, const struct cred *cred, + u32 secid) +{ + int ret = 0; + struct ima_template_entry *entry = NULL; + struct integrity_iint_cache tmp_iint, *iint = &tmp_iint; + struct ima_event_data event_data = {iint, NULL, NULL, + NULL, 0, NULL}; + struct { + struct ima_digest_data hdr; + char digest[IMA_MAX_DIGEST_SIZE]; + } hash; + int violation = 0; + int pcr = CONFIG_IMA_MEASURE_PCR_IDX; + int action = 0; + + action = ima_get_action(NULL, cred, secid, 0, KEXEC_CMDLINE, &pcr); + if (!(action & IMA_MEASURE)) + goto out; + + memset(iint, 0, sizeof(*iint)); + memset(&hash, 0, sizeof(hash)); + + event_data.filename = eventname; + + iint->ima_hash = &hash.hdr; + iint->ima_hash->algo = ima_hash_algo; + iint->ima_hash->length = hash_digest_size[ima_hash_algo]; + + ret = ima_calc_buffer_hash(buf, size, iint->ima_hash); + if (ret < 0) + goto out; + + ret = ima_alloc_init_template(&event_data, &entry); + if (ret < 0) + goto out; + + if (action & IMA_MEASURE) + ret = ima_store_template(entry, violation, NULL, buf, pcr); + + if (ret < 0) { + ima_free_template_entry(entry); + } + +out: + return; +} + +/** + * ima_kexec_cmdline - measure kexec cmdline boot args + * @buf: pointer to buffer + * @size: size of buffer + * + * Buffers can only be measured, not appraised. + */ +void ima_kexec_cmdline(const void *buf, int size) +{ + u32 secid; + + if (buf && size != 0) { + security_task_getsecid(current, &secid); + process_buffer_measurement(buf, size, "kexec-cmdline", + current_cred(), secid); + } +} + static int __init init_ima(void) { int error; diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index e0cc323f948f..413e5921b248 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -291,6 +291,13 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode, { int i; + /* only incase of KEXEC_CMDLINE, inode is NULL */ + if (func == KEXEC_CMDLINE) { + if ((rule->flags & IMA_FUNC) && + (rule->func == func) && (!inode)) + return true; + return false; + } if ((rule->flags & IMA_FUNC) && (rule->func != func && func != POST_SETATTR)) return false; @@ -869,6 +876,8 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) entry->func = KEXEC_INITRAMFS_CHECK; else if (strcmp(args[0].from, "POLICY_CHECK") == 0) entry->func = POLICY_CHECK; + else if (strcmp(args[0].from, "KEXEC_CMDLINE") == 0) + entry->func = KEXEC_CMDLINE; else result = -EINVAL; if (!result)
Currently during kexec_file_load(soft reboot) the cmdline args passed are not measured and the PCR values are not reset. This results in the new kernel to assume a secure boot was followed. The boot args used to launch the new kernel need to be measured and carried over to the next kernel to be used for attestation. IMA supports only measuring files, no functionality exists to measure a buffer(kexec cmdline). This change adds a new functionality to measure buffers process_buffer_measurement which uses the hash of the buffer instead of file hash to add an entry in the ima log. A new ima hook ima_kexec_cmdline is also defined which calls into process_buffer_measurement to add the kexec_cmdline args to the log. A new policy KEXEC_CMDLINE is also defined to control measuring the kexec_cmdline buffer. This patch only adds IMA_MEASURE as a supported functionality. - A new ima hook ima_kexec_cmdline is defined to be called by the kexec code. - A new function process_buffer_measurement is defined to measure the buffer hash into the ima log. - A new func policy KEXEC_CMDLINE is defined to control the measurement. Signed-off-by: Prakhar Srivastava <prsriva02@gmail.com> --- Documentation/ABI/testing/ima_policy | 1 + include/linux/ima.h | 2 + security/integrity/ima/ima.h | 1 + security/integrity/ima/ima_api.c | 1 + security/integrity/ima/ima_main.c | 77 ++++++++++++++++++++++++++++ security/integrity/ima/ima_policy.c | 9 ++++ 6 files changed, 91 insertions(+)