From 33ce9549cfa1e71d77bc91a2e67e65d693e2e53f Mon Sep 17 00:00:00 2001 From: Mimi Zohar Date: Mon, 24 Apr 2017 12:04:09 -0400 Subject: ima: extend the "ima_policy" boot command line to support multiple policies Add support for providing multiple builtin policies on the "ima_policy=" boot command line. Use "|" as the delimitor separating the policy names. Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_policy.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) (limited to 'security/integrity') diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index 3ab1067db624..0ddc41389a9c 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -170,19 +170,24 @@ static int __init default_measure_policy_setup(char *str) } __setup("ima_tcb", default_measure_policy_setup); +static bool ima_use_appraise_tcb __initdata; static int __init policy_setup(char *str) { - if (ima_policy) - return 1; + char *p; - if (strcmp(str, "tcb") == 0) - ima_policy = DEFAULT_TCB; + while ((p = strsep(&str, " |\n")) != NULL) { + if (*p == ' ') + continue; + if ((strcmp(p, "tcb") == 0) && !ima_policy) + ima_policy = DEFAULT_TCB; + else if (strcmp(p, "appraise_tcb") == 0) + ima_use_appraise_tcb = 1; + } return 1; } __setup("ima_policy=", policy_setup); -static bool ima_use_appraise_tcb __initdata; static int __init default_appraise_policy_setup(char *str) { ima_use_appraise_tcb = 1; -- cgit v1.2.3 From 503ceaef8e2e7dbbdb04a867acc6fe4c548ede7f Mon Sep 17 00:00:00 2001 From: Mimi Zohar Date: Fri, 21 Apr 2017 18:58:27 -0400 Subject: ima: define a set of appraisal rules requiring file signatures The builtin "ima_appraise_tcb" policy should require file signatures for at least a few of the hooks (eg. kernel modules, firmware, and the kexec kernel image), but changing it would break the existing userspace/kernel ABI. This patch defines a new builtin policy named "secure_boot", which can be specified on the "ima_policy=" boot command line, independently or in conjunction with the "ima_appraise_tcb" policy, by specifing ima_policy="appraise_tcb | secure_boot". The new appraisal rules requiring file signatures will be added prior to the "ima_appraise_tcb" rules. Signed-off-by: Mimi Zohar Changelog: - Reference secure boot in the new builtin policy name. (Thiago Bauermann) --- Documentation/admin-guide/kernel-parameters.txt | 6 +++++- security/integrity/ima/ima_policy.c | 26 ++++++++++++++++++++++++- 2 files changed, 30 insertions(+), 2 deletions(-) (limited to 'security/integrity') diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt index 9b4381fee877..e438a1fca554 100644 --- a/Documentation/admin-guide/kernel-parameters.txt +++ b/Documentation/admin-guide/kernel-parameters.txt @@ -1478,7 +1478,7 @@ ima_policy= [IMA] The builtin policies to load during IMA setup. - Format: "tcb | appraise_tcb" + Format: "tcb | appraise_tcb | secure_boot" The "tcb" policy measures all programs exec'd, files mmap'd for exec, and all files opened with the read @@ -1489,6 +1489,10 @@ all files owned by root. (This is the equivalent of ima_appraise_tcb.) + The "secure_boot" policy appraises the integrity + of files (eg. kexec kernel image, kernel modules, + firmware, policy, etc) based on file signatures. + ima_tcb [IMA] Deprecated. Use ima_policy= instead. Load a policy which meets the needs of the Trusted Computing Base. This means IMA will measure all diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index 0ddc41389a9c..3653c86c70df 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -153,6 +153,17 @@ static struct ima_rule_entry default_appraise_rules[] __ro_after_init = { #endif }; +static struct ima_rule_entry secure_boot_rules[] __ro_after_init = { + {.action = APPRAISE, .func = MODULE_CHECK, + .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED}, + {.action = APPRAISE, .func = FIRMWARE_CHECK, + .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED}, + {.action = APPRAISE, .func = KEXEC_KERNEL_CHECK, + .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED}, + {.action = APPRAISE, .func = POLICY_CHECK, + .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED}, +}; + static LIST_HEAD(ima_default_rules); static LIST_HEAD(ima_policy_rules); static LIST_HEAD(ima_temp_rules); @@ -171,6 +182,7 @@ static int __init default_measure_policy_setup(char *str) __setup("ima_tcb", default_measure_policy_setup); static bool ima_use_appraise_tcb __initdata; +static bool ima_use_secure_boot __initdata; static int __init policy_setup(char *str) { char *p; @@ -182,6 +194,8 @@ static int __init policy_setup(char *str) ima_policy = DEFAULT_TCB; else if (strcmp(p, "appraise_tcb") == 0) ima_use_appraise_tcb = 1; + else if (strcmp(p, "secure_boot") == 0) + ima_use_secure_boot = 1; } return 1; @@ -410,12 +424,14 @@ void ima_update_policy_flag(void) */ void __init ima_init_policy(void) { - int i, measure_entries, appraise_entries; + int i, measure_entries, appraise_entries, secure_boot_entries; /* if !ima_policy set entries = 0 so we load NO default rules */ measure_entries = ima_policy ? ARRAY_SIZE(dont_measure_rules) : 0; appraise_entries = ima_use_appraise_tcb ? ARRAY_SIZE(default_appraise_rules) : 0; + secure_boot_entries = ima_use_secure_boot ? + ARRAY_SIZE(secure_boot_rules) : 0; for (i = 0; i < measure_entries; i++) list_add_tail(&dont_measure_rules[i].list, &ima_default_rules); @@ -434,6 +450,14 @@ void __init ima_init_policy(void) break; } + /* + * Insert the appraise rules requiring file signatures, prior to + * any other appraise rules. + */ + for (i = 0; i < secure_boot_entries; i++) + list_add_tail(&secure_boot_rules[i].list, + &ima_default_rules); + for (i = 0; i < appraise_entries; i++) { list_add_tail(&default_appraise_rules[i].list, &ima_default_rules); -- cgit v1.2.3 From e1f5e01f4b035ced1c71b40866e4e5c0508fbb0b Mon Sep 17 00:00:00 2001 From: Mimi Zohar Date: Mon, 24 Apr 2017 22:06:49 -0400 Subject: ima: define Kconfig IMA_APPRAISE_BOOTPARAM option Permit enabling the different "ima_appraise=" modes (eg. log, fix) from the boot command line. Signed-off-by: Mimi Zohar --- security/integrity/ima/Kconfig | 8 ++++++++ security/integrity/ima/ima_appraise.c | 2 ++ 2 files changed, 10 insertions(+) (limited to 'security/integrity') diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig index 370eb2f4dd37..8b688a26033d 100644 --- a/security/integrity/ima/Kconfig +++ b/security/integrity/ima/Kconfig @@ -155,6 +155,14 @@ config IMA_APPRAISE If unsure, say N. +config IMA_APPRAISE_BOOTPARAM + bool "ima_appraise boot parameter" + depends on IMA_APPRAISE + default y + help + This option enables the different "ima_appraise=" modes + (eg. fix, log) from the boot command line. + config IMA_TRUSTED_KEYRING bool "Require all keys on the .ima keyring be signed (deprecated)" depends on IMA_APPRAISE && SYSTEM_TRUSTED_KEYRING diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index 5d0785cfe063..ac546df73afc 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -20,12 +20,14 @@ static int __init default_appraise_setup(char *str) { +#ifdef CONFIG_IMA_APPRAISE_BOOTPARAM if (strncmp(str, "off", 3) == 0) ima_appraise = 0; else if (strncmp(str, "log", 3) == 0) ima_appraise = IMA_APPRAISE_LOG; else if (strncmp(str, "fix", 3) == 0) ima_appraise = IMA_APPRAISE_FIX; +#endif return 1; } -- cgit v1.2.3 From 6f6723e21589f4594bb72b27ddbb2f75defb33bb Mon Sep 17 00:00:00 2001 From: Mimi Zohar Date: Mon, 24 Apr 2017 22:43:52 -0400 Subject: ima: define is_ima_appraise_enabled() Only return enabled if in enforcing mode, not fix or log modes. Signed-off-by: Mimi Zohar Changes: - Define is_ima_appraise_enabled() as a bool (Thiago Bauermann) --- include/linux/ima.h | 6 ++++++ security/integrity/ima/ima_appraise.c | 10 ++++++++++ 2 files changed, 16 insertions(+) (limited to 'security/integrity') diff --git a/include/linux/ima.h b/include/linux/ima.h index 7f6952f8d6aa..0e4647e0eb60 100644 --- a/include/linux/ima.h +++ b/include/linux/ima.h @@ -75,11 +75,17 @@ static inline void ima_add_kexec_buffer(struct kimage *image) #endif #ifdef CONFIG_IMA_APPRAISE +extern bool is_ima_appraise_enabled(void); extern void ima_inode_post_setattr(struct dentry *dentry); extern int ima_inode_setxattr(struct dentry *dentry, const char *xattr_name, const void *xattr_value, size_t xattr_value_len); extern int ima_inode_removexattr(struct dentry *dentry, const char *xattr_name); #else +static inline bool is_ima_appraise_enabled(void) +{ + return 0; +} + static inline void ima_inode_post_setattr(struct dentry *dentry) { return; diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index ac546df73afc..7fe0566142d8 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -33,6 +33,16 @@ static int __init default_appraise_setup(char *str) __setup("ima_appraise=", default_appraise_setup); +/* + * is_ima_appraise_enabled - return appraise status + * + * Only return enabled, if not in ima_appraise="fix" or "log" modes. + */ +bool is_ima_appraise_enabled(void) +{ + return (ima_appraise & IMA_APPRAISE_ENFORCE) ? 1 : 0; +} + /* * ima_must_appraise - set appraise flag * -- cgit v1.2.3 From 38d192684e8b1811c352c208447d565f8f0a309f Mon Sep 17 00:00:00 2001 From: Ben Hutchings Date: Tue, 2 May 2017 19:27:00 +0100 Subject: IMA: Correct Kconfig dependencies for hash selection IMA uses the hash algorithm too early to be able to use a module. Require the selected hash algorithm to be built-in. Signed-off-by: Ben Hutchings Signed-off-by: Mimi Zohar --- security/integrity/ima/Kconfig | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'security/integrity') diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig index 8b688a26033d..35ef69312811 100644 --- a/security/integrity/ima/Kconfig +++ b/security/integrity/ima/Kconfig @@ -96,19 +96,19 @@ choice config IMA_DEFAULT_HASH_SHA1 bool "SHA1 (default)" - depends on CRYPTO_SHA1 + depends on CRYPTO_SHA1=y config IMA_DEFAULT_HASH_SHA256 bool "SHA256" - depends on CRYPTO_SHA256 && !IMA_TEMPLATE + depends on CRYPTO_SHA256=y && !IMA_TEMPLATE config IMA_DEFAULT_HASH_SHA512 bool "SHA512" - depends on CRYPTO_SHA512 && !IMA_TEMPLATE + depends on CRYPTO_SHA512=y && !IMA_TEMPLATE config IMA_DEFAULT_HASH_WP512 bool "WP512" - depends on CRYPTO_WP512 && !IMA_TEMPLATE + depends on CRYPTO_WP512=y && !IMA_TEMPLATE endchoice config IMA_DEFAULT_HASH -- cgit v1.2.3 From 5d659f286d58d76064168c4cd7aad61a30d20c44 Mon Sep 17 00:00:00 2001 From: Tycho Andersen Date: Fri, 5 May 2017 11:15:47 -0600 Subject: ima: fix up #endif comments While reading the code, I noticed that these #endif comments don't match how they're actually nested. This patch fixes that. Signed-off-by: Tycho Andersen Signed-off-by: Mimi Zohar --- security/integrity/ima/ima.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'security/integrity') diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index b563fbd4d122..d26a30e37d13 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -306,12 +306,12 @@ static inline int security_filter_rule_match(u32 secid, u32 field, u32 op, { return -EINVAL; } -#endif /* CONFIG_IMA_TRUSTED_KEYRING */ +#endif /* CONFIG_IMA_LSM_RULES */ #ifdef CONFIG_IMA_READ_POLICY #define POLICY_FILE_FLAGS (S_IWUSR | S_IRUSR) #else #define POLICY_FILE_FLAGS S_IWUSR -#endif /* CONFIG_IMA_WRITE_POLICY */ +#endif /* CONFIG_IMA_READ_POLICY */ #endif /* __LINUX_IMA_H */ -- cgit v1.2.3 From b4e280304ddb2fb5b6970524e901fc8ae8ec6337 Mon Sep 17 00:00:00 2001 From: Geliang Tang Date: Sat, 6 May 2017 23:40:18 +0800 Subject: ima: use memdup_user_nul Use memdup_user_nul() helper instead of open-coding to simplify the code. Signed-off-by: Geliang Tang Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_fs.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) (limited to 'security/integrity') diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c index ca303e5d2b94..ad491c51e833 100644 --- a/security/integrity/ima/ima_fs.c +++ b/security/integrity/ima/ima_fs.c @@ -323,16 +323,11 @@ static ssize_t ima_write_policy(struct file *file, const char __user *buf, if (*ppos != 0) goto out; - result = -ENOMEM; - data = kmalloc(datalen + 1, GFP_KERNEL); - if (!data) + data = memdup_user_nul(buf, datalen); + if (IS_ERR(data)) { + result = PTR_ERR(data); goto out; - - *(data + datalen) = '\0'; - - result = -EFAULT; - if (copy_from_user(data, buf, datalen)) - goto out_free; + } result = mutex_lock_interruptible(&ima_write_mutex); if (result < 0) -- cgit v1.2.3 From 82e3bb4d44be21daefe8af857a68d3c9118c1048 Mon Sep 17 00:00:00 2001 From: Laura Abbott Date: Tue, 9 May 2017 11:25:27 -0700 Subject: ima: Add cgroups2 to the defaults list cgroups2 is beginning to show up in wider usage. Add it to the default nomeasure/noappraise list like other filesystems. Signed-off-by: Laura Abbott Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_policy.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'security/integrity') diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index 3653c86c70df..0acd68decb17 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -96,6 +96,8 @@ static struct ima_rule_entry dont_measure_rules[] __ro_after_init = { {.action = DONT_MEASURE, .fsmagic = SELINUX_MAGIC, .flags = IMA_FSMAGIC}, {.action = DONT_MEASURE, .fsmagic = CGROUP_SUPER_MAGIC, .flags = IMA_FSMAGIC}, + {.action = DONT_MEASURE, .fsmagic = CGROUP2_SUPER_MAGIC, + .flags = IMA_FSMAGIC}, {.action = DONT_MEASURE, .fsmagic = NSFS_MAGIC, .flags = IMA_FSMAGIC} }; @@ -139,6 +141,7 @@ static struct ima_rule_entry default_appraise_rules[] __ro_after_init = { {.action = DONT_APPRAISE, .fsmagic = SELINUX_MAGIC, .flags = IMA_FSMAGIC}, {.action = DONT_APPRAISE, .fsmagic = NSFS_MAGIC, .flags = IMA_FSMAGIC}, {.action = DONT_APPRAISE, .fsmagic = CGROUP_SUPER_MAGIC, .flags = IMA_FSMAGIC}, + {.action = DONT_APPRAISE, .fsmagic = CGROUP2_SUPER_MAGIC, .flags = IMA_FSMAGIC}, #ifdef CONFIG_IMA_WRITE_POLICY {.action = APPRAISE, .func = POLICY_CHECK, .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED}, -- cgit v1.2.3 From b17fd9ecf854e8f695e911d3ff9e1fe33bb1c76c Mon Sep 17 00:00:00 2001 From: Roberto Sassu Date: Tue, 16 May 2017 14:53:41 +0200 Subject: ima: introduce ima_parse_buf() ima_parse_buf() takes as input the buffer start and end pointers, and stores the result in a static array of ima_field_data structures, where the len field contains the length parsed from the buffer, and the data field contains the address of the buffer just after the length. Optionally, the function returns the current value of the buffer pointer and the number of array elements written. A bitmap has been added as parameter of ima_parse_buf() to handle the cases where the length is not prepended to data. Each bit corresponds to an element of the ima_field_data array. If a bit is set, the length is not parsed from the buffer, but is read from the corresponding element of the array (the length must be set before calling the function). ima_parse_buf() can perform three checks upon request by callers, depending on the enforce mask passed to it: - ENFORCE_FIELDS: matching of number of fields (length-data combination) - there must be enough data in the buffer to parse the number of fields requested (output: current value of buffer pointer) - ENFORCE_BUFEND: matching of buffer end - the ima_field_data array must be large enough to contain lengths and data pointers for the amount of data requested (output: number of fields written) - ENFORCE_FIELDS | ENFORCE_BUFEND: matching of both Use cases - measurement entry header: ENFORCE_FIELDS | ENFORCE_BUFEND - four fields must be parsed: pcr, digest, template name, template data - ENFORCE_BUFEND is enforced only for the last measurement entry - template digest (Crypto Agile): ENFORCE_BUFEND - since only the total template digest length is known, the function parses length-data combinations until the buffer end is reached - template data: ENFORCE_FIELDS | ENFORCE_BUFEND - since the number of fields and the total template data length are known, the function can perform both checks Signed-off-by: Roberto Sassu Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_template_lib.c | 61 +++++++++++++++++++++++++++++++ security/integrity/ima/ima_template_lib.h | 6 +++ 2 files changed, 67 insertions(+) (limited to 'security/integrity') diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c index f9ba37b3928d..28af43f63572 100644 --- a/security/integrity/ima/ima_template_lib.c +++ b/security/integrity/ima/ima_template_lib.c @@ -159,6 +159,67 @@ void ima_show_template_sig(struct seq_file *m, enum ima_show_type show, ima_show_template_field_data(m, show, DATA_FMT_HEX, field_data); } +/** + * ima_parse_buf() - Parses lengths and data from an input buffer + * @bufstartp: Buffer start address. + * @bufendp: Buffer end address. + * @bufcurp: Pointer to remaining (non-parsed) data. + * @maxfields: Length of fields array. + * @fields: Array containing lengths and pointers of parsed data. + * @curfields: Number of array items containing parsed data. + * @len_mask: Bitmap (if bit is set, data length should not be parsed). + * @enforce_mask: Check if curfields == maxfields and/or bufcurp == bufendp. + * @bufname: String identifier of the input buffer. + * + * Return: 0 on success, -EINVAL on error. + */ +int ima_parse_buf(void *bufstartp, void *bufendp, void **bufcurp, + int maxfields, struct ima_field_data *fields, int *curfields, + unsigned long *len_mask, int enforce_mask, char *bufname) +{ + void *bufp = bufstartp; + int i; + + for (i = 0; i < maxfields; i++) { + if (len_mask == NULL || !test_bit(i, len_mask)) { + if (bufp > (bufendp - sizeof(u32))) + break; + + fields[i].len = *(u32 *)bufp; + if (ima_canonical_fmt) + fields[i].len = le32_to_cpu(fields[i].len); + + bufp += sizeof(u32); + } + + if (bufp > (bufendp - fields[i].len)) + break; + + fields[i].data = bufp; + bufp += fields[i].len; + } + + if ((enforce_mask & ENFORCE_FIELDS) && i != maxfields) { + pr_err("%s: nr of fields mismatch: expected: %d, current: %d\n", + bufname, maxfields, i); + return -EINVAL; + } + + if ((enforce_mask & ENFORCE_BUFEND) && bufp != bufendp) { + pr_err("%s: buf end mismatch: expected: %p, current: %p\n", + bufname, bufendp, bufp); + return -EINVAL; + } + + if (curfields) + *curfields = i; + + if (bufcurp) + *bufcurp = bufp; + + return 0; +} + static int ima_eventdigest_init_common(u8 *digest, u32 digestsize, u8 hash_algo, struct ima_field_data *field_data) { diff --git a/security/integrity/ima/ima_template_lib.h b/security/integrity/ima/ima_template_lib.h index c344530c1d69..6a3d8b831deb 100644 --- a/security/integrity/ima/ima_template_lib.h +++ b/security/integrity/ima/ima_template_lib.h @@ -18,6 +18,9 @@ #include #include "ima.h" +#define ENFORCE_FIELDS 0x00000001 +#define ENFORCE_BUFEND 0x00000002 + void ima_show_template_digest(struct seq_file *m, enum ima_show_type show, struct ima_field_data *field_data); void ima_show_template_digest_ng(struct seq_file *m, enum ima_show_type show, @@ -26,6 +29,9 @@ void ima_show_template_string(struct seq_file *m, enum ima_show_type show, struct ima_field_data *field_data); void ima_show_template_sig(struct seq_file *m, enum ima_show_type show, struct ima_field_data *field_data); +int ima_parse_buf(void *bufstartp, void *bufendp, void **bufcurp, + int maxfields, struct ima_field_data *fields, int *curfields, + unsigned long *len_mask, int enforce_mask, char *bufname); int ima_eventdigest_init(struct ima_event_data *event_data, struct ima_field_data *field_data); int ima_eventname_init(struct ima_event_data *event_data, -- cgit v1.2.3 From 47fdee60b47fc2836b256761ab60ada26788b323 Mon Sep 17 00:00:00 2001 From: Roberto Sassu Date: Tue, 16 May 2017 14:53:42 +0200 Subject: ima: use ima_parse_buf() to parse measurements headers The binary_hdr_v1 and binary_data_v1 structures defined in ima_restore_measurement_list() have been replaced with an array of four ima_field_data structures where pcr, digest, template name and template data lengths and pointers are stored. The length of pcr and digest in the ima_field_data array and the bits in the bitmap are set before ima_parse_buf() is called. The ENFORCE_FIELDS bit is set for all entries except the last one (there is still data to parse), and ENFORCE_BUFEND is set only for the last entry. Signed-off-by: Roberto Sassu Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_template.c | 80 ++++++++++++----------------------- 1 file changed, 28 insertions(+), 52 deletions(-) (limited to 'security/integrity') diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c index cebb37c63629..624e2a1d0f89 100644 --- a/security/integrity/ima/ima_template.c +++ b/security/integrity/ima/ima_template.c @@ -19,6 +19,9 @@ #include "ima.h" #include "ima_template_lib.h" +enum header_fields { HDR_PCR, HDR_DIGEST, HDR_TEMPLATE_NAME, + HDR_TEMPLATE_DATA, HDR__LAST }; + static struct ima_template_desc builtin_templates[] = { {.name = IMA_TEMPLATE_IMA_NAME, .fmt = IMA_TEMPLATE_IMA_FMT}, {.name = "ima-ng", .fmt = "d-ng|n-ng"}, @@ -337,27 +340,19 @@ static int ima_restore_template_data(struct ima_template_desc *template_desc, /* Restore the serialized binary measurement list without extending PCRs. */ int ima_restore_measurement_list(loff_t size, void *buf) { - struct binary_hdr_v1 { - u32 pcr; - u8 digest[TPM_DIGEST_SIZE]; - u32 template_name_len; - char template_name[0]; - } __packed; char template_name[MAX_TEMPLATE_NAME_LEN]; - struct binary_data_v1 { - u32 template_data_size; - char template_data[0]; - } __packed; - struct ima_kexec_hdr *khdr = buf; - struct binary_hdr_v1 *hdr_v1; - struct binary_data_v1 *data_v1; + struct ima_field_data hdr[HDR__LAST] = { + [HDR_PCR] = {.len = sizeof(u32)}, + [HDR_DIGEST] = {.len = TPM_DIGEST_SIZE}, + }; void *bufp = buf + sizeof(*khdr); void *bufendp; struct ima_template_entry *entry; struct ima_template_desc *template_desc; + DECLARE_BITMAP(hdr_mask, HDR__LAST); unsigned long count = 0; int ret = 0; @@ -380,6 +375,10 @@ int ima_restore_measurement_list(loff_t size, void *buf) return -EINVAL; } + bitmap_zero(hdr_mask, HDR__LAST); + bitmap_set(hdr_mask, HDR_PCR, 1); + bitmap_set(hdr_mask, HDR_DIGEST, 1); + /* * ima kexec buffer prefix: version, buffer size, count * v1 format: pcr, digest, template-name-len, template-name, @@ -387,31 +386,25 @@ int ima_restore_measurement_list(loff_t size, void *buf) */ bufendp = buf + khdr->buffer_size; while ((bufp < bufendp) && (count++ < khdr->count)) { - hdr_v1 = bufp; - if (bufp > (bufendp - sizeof(*hdr_v1))) { - pr_err("attempting to restore partial measurement\n"); - ret = -EINVAL; - break; - } - bufp += sizeof(*hdr_v1); + int enforce_mask = ENFORCE_FIELDS; - if (ima_canonical_fmt) - hdr_v1->template_name_len = - le32_to_cpu(hdr_v1->template_name_len); + enforce_mask |= (count == khdr->count) ? ENFORCE_BUFEND : 0; + ret = ima_parse_buf(bufp, bufendp, &bufp, HDR__LAST, hdr, NULL, + hdr_mask, enforce_mask, "entry header"); + if (ret < 0) + break; - if ((hdr_v1->template_name_len >= MAX_TEMPLATE_NAME_LEN) || - (bufp > (bufendp - hdr_v1->template_name_len))) { + if (hdr[HDR_TEMPLATE_NAME].len >= MAX_TEMPLATE_NAME_LEN) { pr_err("attempting to restore a template name \ that is too long\n"); ret = -EINVAL; break; } - data_v1 = bufp += (u_int8_t)hdr_v1->template_name_len; /* template name is not null terminated */ - memcpy(template_name, hdr_v1->template_name, - hdr_v1->template_name_len); - template_name[hdr_v1->template_name_len] = 0; + memcpy(template_name, hdr[HDR_TEMPLATE_NAME].data, + hdr[HDR_TEMPLATE_NAME].len); + template_name[hdr[HDR_TEMPLATE_NAME].len] = 0; if (strcmp(template_name, "ima") == 0) { pr_err("attempting to restore an unsupported \ @@ -441,34 +434,17 @@ int ima_restore_measurement_list(loff_t size, void *buf) break; } - if (bufp > (bufendp - sizeof(data_v1->template_data_size))) { - pr_err("restoring the template data size failed\n"); - ret = -EINVAL; - break; - } - bufp += (u_int8_t) sizeof(data_v1->template_data_size); - - if (ima_canonical_fmt) - data_v1->template_data_size = - le32_to_cpu(data_v1->template_data_size); - - if (bufp > (bufendp - data_v1->template_data_size)) { - pr_err("restoring the template data failed\n"); - ret = -EINVAL; - break; - } - bufp += data_v1->template_data_size; - ret = ima_restore_template_data(template_desc, - data_v1->template_data, - data_v1->template_data_size, + hdr[HDR_TEMPLATE_DATA].data, + hdr[HDR_TEMPLATE_DATA].len, &entry); if (ret < 0) break; - memcpy(entry->digest, hdr_v1->digest, TPM_DIGEST_SIZE); - entry->pcr = - !ima_canonical_fmt ? hdr_v1->pcr : le32_to_cpu(hdr_v1->pcr); + memcpy(entry->digest, hdr[HDR_DIGEST].data, + hdr[HDR_DIGEST].len); + entry->pcr = !ima_canonical_fmt ? *(hdr[HDR_PCR].data) : + le32_to_cpu(*(hdr[HDR_PCR].data)); ret = ima_restore_measurement_entry(entry); if (ret < 0) break; -- cgit v1.2.3 From 28a8dc41279de2a8a635df51ad33d3cee7e0c0d1 Mon Sep 17 00:00:00 2001 From: Roberto Sassu Date: Tue, 16 May 2017 14:53:43 +0200 Subject: ima: use ima_parse_buf() to parse template data The binary_field_data structure definition has been removed from ima_restore_template_data(). The lengths and data pointers are directly stored into the template_data array of the ima_template_entry structure. For template data, both the number of fields and buffer end checks can be done, as these information are known (respectively from the template descriptor, and from the measurement header field). Signed-off-by: Roberto Sassu Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_template.c | 44 +++++++++++------------------------ 1 file changed, 13 insertions(+), 31 deletions(-) (limited to 'security/integrity') diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c index 624e2a1d0f89..7412d0291ab9 100644 --- a/security/integrity/ima/ima_template.c +++ b/security/integrity/ima/ima_template.c @@ -277,13 +277,6 @@ static int ima_restore_template_data(struct ima_template_desc *template_desc, int template_data_size, struct ima_template_entry **entry) { - struct binary_field_data { - u32 len; - u8 data[0]; - } __packed; - - struct binary_field_data *field_data; - int offset = 0; int ret = 0; int i; @@ -293,30 +286,19 @@ static int ima_restore_template_data(struct ima_template_desc *template_desc, if (!*entry) return -ENOMEM; + ret = ima_parse_buf(template_data, template_data + template_data_size, + NULL, template_desc->num_fields, + (*entry)->template_data, NULL, NULL, + ENFORCE_FIELDS | ENFORCE_BUFEND, "template data"); + if (ret < 0) { + kfree(*entry); + return ret; + } + (*entry)->template_desc = template_desc; for (i = 0; i < template_desc->num_fields; i++) { - field_data = template_data + offset; - - /* Each field of the template data is prefixed with a length. */ - if (offset > (template_data_size - sizeof(*field_data))) { - pr_err("Restoring the template field failed\n"); - ret = -EINVAL; - break; - } - offset += sizeof(*field_data); - - if (ima_canonical_fmt) - field_data->len = le32_to_cpu(field_data->len); - - if (offset > (template_data_size - field_data->len)) { - pr_err("Restoring the template field data failed\n"); - ret = -EINVAL; - break; - } - offset += field_data->len; - - (*entry)->template_data[i].len = field_data->len; - (*entry)->template_data_len += sizeof(field_data->len); + struct ima_field_data *field_data = &(*entry)->template_data[i]; + u8 *data = field_data->data; (*entry)->template_data[i].data = kzalloc(field_data->len + 1, GFP_KERNEL); @@ -324,8 +306,8 @@ static int ima_restore_template_data(struct ima_template_desc *template_desc, ret = -ENOMEM; break; } - memcpy((*entry)->template_data[i].data, field_data->data, - field_data->len); + memcpy((*entry)->template_data[i].data, data, field_data->len); + (*entry)->template_data_len += sizeof(field_data->len); (*entry)->template_data_len += field_data->len; } -- cgit v1.2.3 From e4586c79d4ba24a02f63a17e49207007c3bbdaea Mon Sep 17 00:00:00 2001 From: Roberto Sassu Date: Tue, 16 May 2017 14:53:47 +0200 Subject: ima: fix get_binary_runtime_size() Remove '+ 1' from 'size += strlen(entry->template_desc->name) + 1;', as the template name is sent to userspace without the '\0' character. Signed-off-by: Roberto Sassu Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_queue.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'security/integrity') diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c index d9aa5ab71204..a02a86d51102 100644 --- a/security/integrity/ima/ima_queue.c +++ b/security/integrity/ima/ima_queue.c @@ -81,7 +81,7 @@ static int get_binary_runtime_size(struct ima_template_entry *entry) size += sizeof(u32); /* pcr */ size += sizeof(entry->digest); size += sizeof(int); /* template name size field */ - size += strlen(entry->template_desc->name) + 1; + size += strlen(entry->template_desc->name); size += sizeof(entry->template_data_len); size += entry->template_data_len; return size; -- cgit v1.2.3 From bb543e3959b5909e7b5db4a216018c634a9d9898 Mon Sep 17 00:00:00 2001 From: Thiago Jung Bauermann Date: Wed, 7 Jun 2017 22:49:10 -0300 Subject: integrity: Small code improvements These changes are too small to warrant their own patches: The keyid and sig_size members of struct signature_v2_hdr are in BE format, so use a type that makes this assumption explicit. Also, use beXX_to_cpu instead of __beXX_to_cpu to read them. Change integrity_kernel_read to take a void * buffer instead of char * buffer, so that callers don't have to use a cast if they provide a buffer that isn't a char *. Add missing #endif comment in ima.h pointing out which macro it refers to. Add missing fall through comment in ima_appraise.c. Constify mask_tokens and func_tokens arrays. Signed-off-by: Thiago Jung Bauermann Signed-off-by: Mimi Zohar --- security/integrity/digsig_asymmetric.c | 4 ++-- security/integrity/iint.c | 2 +- security/integrity/ima/ima.h | 2 +- security/integrity/ima/ima_appraise.c | 1 + security/integrity/ima/ima_policy.c | 4 ++-- security/integrity/integrity.h | 7 ++++--- 6 files changed, 11 insertions(+), 9 deletions(-) (limited to 'security/integrity') diff --git a/security/integrity/digsig_asymmetric.c b/security/integrity/digsig_asymmetric.c index 80052ed8d467..ab6a029062a1 100644 --- a/security/integrity/digsig_asymmetric.c +++ b/security/integrity/digsig_asymmetric.c @@ -92,13 +92,13 @@ int asymmetric_verify(struct key *keyring, const char *sig, siglen -= sizeof(*hdr); - if (siglen != __be16_to_cpu(hdr->sig_size)) + if (siglen != be16_to_cpu(hdr->sig_size)) return -EBADMSG; if (hdr->hash_algo >= HASH_ALGO__LAST) return -ENOPKG; - key = request_asymmetric_key(keyring, __be32_to_cpu(hdr->keyid)); + key = request_asymmetric_key(keyring, be32_to_cpu(hdr->keyid)); if (IS_ERR(key)) return PTR_ERR(key); diff --git a/security/integrity/iint.c b/security/integrity/iint.c index c710d22042f9..6fc888ca468e 100644 --- a/security/integrity/iint.c +++ b/security/integrity/iint.c @@ -182,7 +182,7 @@ security_initcall(integrity_iintcache_init); * */ int integrity_kernel_read(struct file *file, loff_t offset, - char *addr, unsigned long count) + void *addr, unsigned long count) { mm_segment_t old_fs; char __user *buf = (char __user *)addr; diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index d26a30e37d13..215a93c41b51 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -284,7 +284,7 @@ static inline int ima_read_xattr(struct dentry *dentry, return 0; } -#endif +#endif /* CONFIG_IMA_APPRAISE */ /* LSM based policy rules require audit */ #ifdef CONFIG_IMA_LSM_RULES diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index 7fe0566142d8..ea36a4f134f4 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -240,6 +240,7 @@ int ima_appraise_measurement(enum ima_hooks func, case IMA_XATTR_DIGEST_NG: /* first byte contains algorithm id */ hash_start = 1; + /* fall through */ case IMA_XATTR_DIGEST: if (iint->flags & IMA_DIGSIG_REQUIRED) { cause = "IMA-signature-required"; diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index 0acd68decb17..949ad3858327 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -965,7 +965,7 @@ enum { mask_exec = 0, mask_write, mask_read, mask_append }; -static char *mask_tokens[] = { +static const char *const mask_tokens[] = { "MAY_EXEC", "MAY_WRITE", "MAY_READ", @@ -979,7 +979,7 @@ enum { func_policy }; -static char *func_tokens[] = { +static const char *const func_tokens[] = { "FILE_CHECK", "MMAP_CHECK", "BPRM_CHECK", diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h index 24520b4ef3b0..a53e7e4ab06c 100644 --- a/security/integrity/integrity.h +++ b/security/integrity/integrity.h @@ -92,8 +92,8 @@ struct signature_v2_hdr { uint8_t type; /* xattr type */ uint8_t version; /* signature format version */ uint8_t hash_algo; /* Digest algorithm [enum hash_algo] */ - uint32_t keyid; /* IMA key identifier - not X509/PGP specific */ - uint16_t sig_size; /* signature size */ + __be32 keyid; /* IMA key identifier - not X509/PGP specific */ + __be16 sig_size; /* signature size */ uint8_t sig[0]; /* signature payload */ } __packed; @@ -118,7 +118,8 @@ struct integrity_iint_cache { struct integrity_iint_cache *integrity_iint_find(struct inode *inode); int integrity_kernel_read(struct file *file, loff_t offset, - char *addr, unsigned long count); + void *addr, unsigned long count); + int __init integrity_read_file(const char *path, char **data); #define INTEGRITY_KEYRING_EVM 0 -- cgit v1.2.3 From 2663218ba6e3dd6f27df9664e00fa3eb63be3a3f Mon Sep 17 00:00:00 2001 From: Thiago Jung Bauermann Date: Wed, 7 Jun 2017 22:49:11 -0300 Subject: ima: Simplify policy_func_show. If the func_tokens array uses the same indices as enum ima_hooks, policy_func_show can be a lot simpler, and the func_* enum becomes unnecessary. Also, if we use the same macro trick used by kernel_read_file_id_str we can use one hooks list for both the enum and the string array, making sure they are always in sync (suggested by Mimi Zohar). Finally, by using the printf pattern for the function token directly instead of using the pt macro we can simplify policy_func_show even further and avoid needing a temporary buffer. Signed-off-by: Thiago Jung Bauermann Signed-off-by: Mimi Zohar --- security/integrity/ima/ima.h | 25 +++++++++------- security/integrity/ima/ima_policy.c | 58 ++++--------------------------------- 2 files changed, 21 insertions(+), 62 deletions(-) (limited to 'security/integrity') diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index 215a93c41b51..d52b487ad259 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -172,17 +172,22 @@ static inline unsigned long ima_hash_key(u8 *digest) return hash_long(*digest, IMA_HASH_BITS); } +#define __ima_hooks(hook) \ + hook(NONE) \ + hook(FILE_CHECK) \ + hook(MMAP_CHECK) \ + hook(BPRM_CHECK) \ + hook(POST_SETATTR) \ + hook(MODULE_CHECK) \ + hook(FIRMWARE_CHECK) \ + hook(KEXEC_KERNEL_CHECK) \ + hook(KEXEC_INITRAMFS_CHECK) \ + hook(POLICY_CHECK) \ + hook(MAX_CHECK) +#define __ima_hook_enumify(ENUM) ENUM, + enum ima_hooks { - FILE_CHECK = 1, - MMAP_CHECK, - BPRM_CHECK, - POST_SETATTR, - MODULE_CHECK, - FIRMWARE_CHECK, - KEXEC_KERNEL_CHECK, - KEXEC_INITRAMFS_CHECK, - POLICY_CHECK, - MAX_CHECK + __ima_hooks(__ima_hook_enumify) }; /* LIM API function definitions */ diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index 949ad3858327..f4436626ccb7 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -972,23 +972,10 @@ static const char *const mask_tokens[] = { "MAY_APPEND" }; -enum { - func_file = 0, func_mmap, func_bprm, - func_module, func_firmware, func_post, - func_kexec_kernel, func_kexec_initramfs, - func_policy -}; +#define __ima_hook_stringify(str) (#str), static const char *const func_tokens[] = { - "FILE_CHECK", - "MMAP_CHECK", - "BPRM_CHECK", - "MODULE_CHECK", - "FIRMWARE_CHECK", - "POST_SETATTR", - "KEXEC_KERNEL_CHECK", - "KEXEC_INITRAMFS_CHECK", - "POLICY_CHECK" + __ima_hooks(__ima_hook_stringify) }; void *ima_policy_start(struct seq_file *m, loff_t *pos) @@ -1025,49 +1012,16 @@ void ima_policy_stop(struct seq_file *m, void *v) #define pt(token) policy_tokens[token + Opt_err].pattern #define mt(token) mask_tokens[token] -#define ft(token) func_tokens[token] /* * policy_func_show - display the ima_hooks policy rule */ static void policy_func_show(struct seq_file *m, enum ima_hooks func) { - char tbuf[64] = {0,}; - - switch (func) { - case FILE_CHECK: - seq_printf(m, pt(Opt_func), ft(func_file)); - break; - case MMAP_CHECK: - seq_printf(m, pt(Opt_func), ft(func_mmap)); - break; - case BPRM_CHECK: - seq_printf(m, pt(Opt_func), ft(func_bprm)); - break; - case MODULE_CHECK: - seq_printf(m, pt(Opt_func), ft(func_module)); - break; - case FIRMWARE_CHECK: - seq_printf(m, pt(Opt_func), ft(func_firmware)); - break; - case POST_SETATTR: - seq_printf(m, pt(Opt_func), ft(func_post)); - break; - case KEXEC_KERNEL_CHECK: - seq_printf(m, pt(Opt_func), ft(func_kexec_kernel)); - break; - case KEXEC_INITRAMFS_CHECK: - seq_printf(m, pt(Opt_func), ft(func_kexec_initramfs)); - break; - case POLICY_CHECK: - seq_printf(m, pt(Opt_func), ft(func_policy)); - break; - default: - snprintf(tbuf, sizeof(tbuf), "%d", func); - seq_printf(m, pt(Opt_func), tbuf); - break; - } - seq_puts(m, " "); + if (func > 0 && func < MAX_CHECK) + seq_printf(m, "func=%s ", func_tokens[func]); + else + seq_printf(m, "func=%d ", func); } int ima_policy_show(struct seq_file *m, void *v) -- cgit v1.2.3 From 915d9d255defeba80e1331a2b8bb8a79c0ca4db7 Mon Sep 17 00:00:00 2001 From: Thiago Jung Bauermann Date: Wed, 7 Jun 2017 22:49:12 -0300 Subject: ima: Log the same audit cause whenever a file has no signature If the file doesn't have an xattr, ima_appraise_measurement sets cause to "missing-hash" while if there's an xattr but it's a digest instead of a signature it sets cause to "IMA-signature-required". Fix it by setting cause to "IMA-signature-required" in both cases. Signed-off-by: Thiago Jung Bauermann Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_appraise.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'security/integrity') diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index ea36a4f134f4..809ba70fbbbf 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -217,7 +217,8 @@ int ima_appraise_measurement(enum ima_hooks func, if (rc && rc != -ENODATA) goto out; - cause = "missing-hash"; + cause = iint->flags & IMA_DIGSIG_REQUIRED ? + "IMA-signature-required" : "missing-hash"; status = INTEGRITY_NOLABEL; if (opened & FILE_CREATED) iint->flags |= IMA_NEW_FILE; -- cgit v1.2.3