Re: [Linux-ima-devel] [PATCH v6 07/10] ima: store the builtin/custom template definitions in a list
On Fri, Oct 21, 2016 at 5:44 AM, Thiago Jung Bauermann
wrote:
> From: Mimi Zohar
>
> The builtin and single custom templates are currently stored in an
> array. In preparation for being able to restore a measurement list
> containing multiple builtin/custom templates, this patch stores the
> builtin and custom templates as a linked list. This will permit
> defining more than one custom template per boot.
>
> Changelog v4:
> - fix "spinlock bad magic" BUG - reported by Dmitry Vyukov
>
> Changelog v3:
> - initialize template format list in ima_template_desc_current(), as it
> might be called during __setup before normal initialization. (kernel
> test robot)
> - remove __init annotation of ima_init_template_list()
>
> Changelog v2:
> - fix lookup_template_desc() preemption imbalance (kernel test robot)
>
> Signed-off-by: Mimi Zohar
> ---
> security/integrity/ima/ima.h | 2 ++
> security/integrity/ima/ima_main.c | 1 +
> security/integrity/ima/ima_template.c | 52
> +++
> 3 files changed, 44 insertions(+), 11 deletions(-)
>
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index 139dec67dcbf..6b0540ad189f 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -85,6 +85,7 @@ struct ima_template_field {
>
> /* IMA template descriptor definition */
> struct ima_template_desc {
> + struct list_head list;
> char *name;
> char *fmt;
> int num_fields;
> @@ -146,6 +147,7 @@ int ima_restore_measurement_list(loff_t bufsize, void
> *buf);
> int ima_measurements_show(struct seq_file *m, void *v);
> unsigned long ima_get_binary_runtime_size(void);
> int ima_init_template(void);
> +void ima_init_template_list(void);
>
> /*
> * used to protect h_table and sha_table
> diff --git a/security/integrity/ima/ima_main.c
> b/security/integrity/ima/ima_main.c
> index 423d111b3b94..50818c60538b 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -418,6 +418,7 @@ static int __init init_ima(void)
> {
> int error;
>
> + ima_init_template_list();
> hash_setup(CONFIG_IMA_DEFAULT_HASH);
> error = ima_init();
> if (!error) {
> diff --git a/security/integrity/ima/ima_template.c
> b/security/integrity/ima/ima_template.c
> index 37f972cb05fe..c0d808c20c40 100644
> --- a/security/integrity/ima/ima_template.c
> +++ b/security/integrity/ima/ima_template.c
> @@ -15,16 +15,20 @@
>
> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
>
> +#include
> #include "ima.h"
> #include "ima_template_lib.h"
>
> -static struct ima_template_desc defined_templates[] = {
> +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"},
> {.name = "ima-sig", .fmt = "d-ng|n-ng|sig"},
> {.name = "", .fmt = ""},/* placeholder for a custom format */
> };
>
> +static LIST_HEAD(defined_templates);
> +static DEFINE_SPINLOCK(template_list);
> +
> static struct ima_template_field supported_fields[] = {
> {.field_id = "d", .field_init = ima_eventdigest_init,
> .field_show = ima_show_template_digest},
> @@ -53,6 +57,8 @@ static int __init ima_template_setup(char *str)
> if (ima_template)
> return 1;
>
> + ima_init_template_list();
> +
> /*
> * Verify that a template with the supplied name exists.
> * If not, use CONFIG_IMA_DEFAULT_TEMPLATE.
> @@ -81,7 +87,7 @@ __setup("ima_template=", ima_template_setup);
>
> static int __init ima_template_fmt_setup(char *str)
> {
> - int num_templates = ARRAY_SIZE(defined_templates);
> + int num_templates = ARRAY_SIZE(builtin_templates);
>
> if (ima_template)
> return 1;
> @@ -92,22 +98,28 @@ static int __init ima_template_fmt_setup(char *str)
> return 1;
> }
>
> - defined_templates[num_templates - 1].fmt = str;
> - ima_template = defined_templates + num_templates - 1;
> + builtin_templates[num_templates - 1].fmt = str;
> + ima_template = builtin_templates + num_templates - 1;
> +
> return 1;
> }
> __setup("ima_template_fmt=", ima_template_fmt_setup);
>
> static struct ima_template_desc *lookup_template_desc(const char *name)
> {
> - int i;
> + struct ima_template_desc *template_desc;
> + int found = 0;
>
> - for (i = 0; i < ARRAY_SIZE(defined_templates); i++) {
> - if (strcmp(defined_templates[i].name, name) == 0)
> - return defined_templates + i;
> + rcu_read_lock();
> + list_for_each_entry_rcu(template_desc, &defined_templates, list) {
> + if ((strcmp(template_desc->name, name) == 0) ||
> + (strcmp(template_desc->fmt, name) == 0)) {
> + found = 1;
> + break;
Re: [Linux-ima-devel] [PATCH v6 04/10] ima: maintain memory size needed for serializing the measurement list
On Fri, Oct 21, 2016 at 5:44 AM, Thiago Jung Bauermann
wrote:
> From: Mimi Zohar
>
> In preparation for serializing the binary_runtime_measurements, this patch
> maintains the amount of memory required.
>
> Changelog v5:
> - replace CONFIG_KEXEC_FILE with architecture CONFIG_HAVE_IMA_KEXEC (Thiago)
>
> Changelog v3:
> - include the ima_kexec_hdr size in the binary_runtime_measurement size.
>
> Signed-off-by: Mimi Zohar
> ---
> security/integrity/ima/Kconfig | 12 +
> security/integrity/ima/ima.h | 1 +
> security/integrity/ima/ima_queue.c | 53
> --
> 3 files changed, 64 insertions(+), 2 deletions(-)
>
> diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
> index 5487827fa86c..370eb2f4dd37 100644
> --- a/security/integrity/ima/Kconfig
> +++ b/security/integrity/ima/Kconfig
> @@ -27,6 +27,18 @@ config IMA
> to learn more about IMA.
> If unsure, say N.
>
> +config IMA_KEXEC
> + bool "Enable carrying the IMA measurement list across a soft boot"
> + depends on IMA && TCG_TPM && HAVE_IMA_KEXEC
> + default n
> + help
> + TPM PCRs are only reset on a hard reboot. In order to validate
> + a TPM's quote after a soft boot, the IMA measurement list of the
> + running kernel must be saved and restored on boot.
> +
> + Depending on the IMA policy, the measurement list can grow to
> + be very large.
> +
> config IMA_MEASURE_PCR_IDX
> int
> depends on IMA
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index 51dc8d57d64d..ea1dcc452911 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -143,6 +143,7 @@ void ima_print_digest(struct seq_file *m, u8 *digest, u32
> size);
> struct ima_template_desc *ima_template_desc_current(void);
> int ima_restore_measurement_entry(struct ima_template_entry *entry);
> int ima_restore_measurement_list(loff_t bufsize, void *buf);
> +unsigned long ima_get_binary_runtime_size(void);
> int ima_init_template(void);
>
> /*
> diff --git a/security/integrity/ima/ima_queue.c
> b/security/integrity/ima/ima_queue.c
> index 12d1b040bca9..3a3cc2a45645 100644
> --- a/security/integrity/ima/ima_queue.c
> +++ b/security/integrity/ima/ima_queue.c
> @@ -29,6 +29,11 @@
> #define AUDIT_CAUSE_LEN_MAX 32
>
> LIST_HEAD(ima_measurements); /* list of all measurements */
> +#ifdef CONFIG_IMA_KEXEC
> +static unsigned long binary_runtime_size;
> +#else
> +static unsigned long binary_runtime_size = ULONG_MAX;
> +#endif
>
> /* key: inode (before secure-hashing a file) */
> struct ima_h_table ima_htable = {
> @@ -64,6 +69,24 @@ static struct ima_queue_entry *ima_lookup_digest_entry(u8
> *digest_value,
> return ret;
> }
>
> +/*
> + * Calculate the memory required for serializing a single
> + * binary_runtime_measurement list entry, which contains a
> + * couple of variable length fields (e.g template name and data).
> + */
> +static int get_binary_runtime_size(struct ima_template_entry *entry)
> +{
> + int size = 0;
> +
> + size += sizeof(u32);/* pcr */
> + size += sizeof(entry->digest);
> + size += sizeof(int);/* template name size field */
> + size += strlen(entry->template_desc->name);
> + size += sizeof(entry->template_data_len);
> + size += entry->template_data_len;
> + return size;
> +}
> +
strlen returns len without '\0'. I cannot see how you would know how
to read it back?
> /* ima_add_template_entry helper function:
> * - Add template entry to the measurement list and hash table, for
> * all entries except those carried across kexec.
> @@ -90,9 +113,30 @@ static int ima_add_digest_entry(struct ima_template_entry
> *entry, int flags)
> key = ima_hash_key(entry->digest);
> hlist_add_head_rcu(&qe->hnext, &ima_htable.queue[key]);
> }
> +
> + if (binary_runtime_size != ULONG_MAX) {
> + int size;
> +
> + size = get_binary_runtime_size(entry);
> + binary_runtime_size = (binary_runtime_size < ULONG_MAX -
> size) ?
> +binary_runtime_size + size : ULONG_MAX;
> + }
> return 0;
> }
>
> +/*
> + * Return the amount of memory required for serializing the
> + * entire binary_runtime_measurement list, including the ima_kexec_hdr
> + * structure.
> + */
> +unsigned long ima_get_binary_runtime_size(void)
> +{
> + if (binary_runtime_size >= (ULONG_MAX - sizeof(struct ima_kexec_hdr)))
> + return ULONG_MAX;
> + else
> + return binary_runtime_size + sizeof(struct ima_kexec_hdr);
> +};
> +
> static int ima_pcr_extend(const u8 *hash, int pcr)
> {
> int result = 0;
> @@ -106,8 +150,13 @@ static int ima_pcr_extend(const u8 *hash, int pcr)
> return result;
> }
>
> -/* Add template entry to the measurement list and hash tab
Re: [Linux-ima-devel] [PATCH v6 03/10] ima: permit duplicate measurement list entries
On Fri, Oct 21, 2016 at 5:44 AM, Thiago Jung Bauermann
wrote:
> From: Mimi Zohar
>
> Measurements carried across kexec need to be added to the IMA
> measurement list, but should not prevent measurements of the newly
> booted kernel from being added to the measurement list. This patch
> adds support for allowing duplicate measurements.
>
> The "boot_aggregate" measurement entry is the delimiter between soft
> boots.
>
> Signed-off-by: Mimi Zohar
> ---
> security/integrity/ima/ima_queue.c | 15 +--
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/security/integrity/ima/ima_queue.c
> b/security/integrity/ima/ima_queue.c
> index 4b1bb7787839..12d1b040bca9 100644
> --- a/security/integrity/ima/ima_queue.c
> +++ b/security/integrity/ima/ima_queue.c
> @@ -65,11 +65,12 @@ static struct ima_queue_entry *ima_lookup_digest_entry(u8
> *digest_value,
> }
>
> /* ima_add_template_entry helper function:
> - * - Add template entry to measurement list and hash table.
> + * - Add template entry to the measurement list and hash table, for
> + * all entries except those carried across kexec.
> *
> * (Called with ima_extend_list_mutex held.)
> */
> -static int ima_add_digest_entry(struct ima_template_entry *entry)
> +static int ima_add_digest_entry(struct ima_template_entry *entry, int flags)
> {
> struct ima_queue_entry *qe;
> unsigned int key;
> @@ -85,8 +86,10 @@ static int ima_add_digest_entry(struct ima_template_entry
> *entry)
> list_add_tail_rcu(&qe->later, &ima_measurements);
>
> atomic_long_inc(&ima_htable.len);
> - key = ima_hash_key(entry->digest);
> - hlist_add_head_rcu(&qe->hnext, &ima_htable.queue[key]);
> + if (flags) {
It looks lile "bool", not flags in fact.
> + key = ima_hash_key(entry->digest);
> + hlist_add_head_rcu(&qe->hnext, &ima_htable.queue[key]);
> + }
> return 0;
> }
>
> @@ -126,7 +129,7 @@ int ima_add_template_entry(struct ima_template_entry
> *entry, int violation,
> }
> }
>
> - result = ima_add_digest_entry(entry);
> + result = ima_add_digest_entry(entry, 1);
> if (result < 0) {
> audit_cause = "ENOMEM";
> audit_info = 0;
> @@ -155,7 +158,7 @@ int ima_restore_measurement_entry(struct
> ima_template_entry *entry)
> int result = 0;
>
> mutex_lock(&ima_extend_list_mutex);
> - result = ima_add_digest_entry(entry);
> + result = ima_add_digest_entry(entry, 0);
> mutex_unlock(&ima_extend_list_mutex);
> return result;
> }
> --
> 2.7.4
>
>
> --
> Check out the vibrant tech community on one of the world's most
> engaging tech sites, SlashDot.org! http://sdm.link/slashdot
> ___
> Linux-ima-devel mailing list
> linux-ima-de...@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/linux-ima-devel
--
Thanks,
Dmitry
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v3 19/22] ima: support for kexec image and initramfs
On Thu, Feb 11, 2016 at 4:08 AM, Mimi Zohar wrote:
> On Thu, 2016-02-11 at 01:55 +0200, Dmitry Kasatkin wrote:
>> On Feb 11, 2016 1:22 AM, "Mimi Zohar" wrote:
>> >
>> > On Wed, 2016-02-10 at 23:09 +0200, Dmitry Kasatkin wrote:
>> > > On Wed, Feb 3, 2016 at 9:06 PM, Mimi Zohar
>> wrote:
>
>> > > >
>> > > > - if (read_id == READING_FIRMWARE)
>> > > > + switch (read_id) {
>> > > > + case READING_FIRMWARE:
>> > > > func = FIRMWARE_CHECK;
>> > > > - else if (read_id == READING_MODULE)
>> > > > + break;
>> > > > + case READING_MODULE:
>> > > > func = MODULE_CHECK;
>> > > > + break;
>> > > > + case READING_KEXEC_IMAGE:
>> > > > + func = KEXEC_CHECK;
>> > > > + break;
>> > > > + case READING_KEXEC_INITRAMFS:
>> > > > + func = INITRAMFS_CHECK;
>> > > > + break;
>> > > > + default:
>> > > > + func = FILE_CHECK;
>> > > > + break;
>> > > > + }
>> > > >
>> > >
>> > > I would define a separate function like "int ima_read_id_to_func(id)"
>> > > which search over the map
>> > >
>> > > Something like...
>> > >
>> > > struct
>> > > {
>> > > int id;
>> > > int func;
>> > > } map[] = {
>> > > { .id = READING_FIRMWARE, .fun = FIRMWARE_CHECK },
>> > >...
>> > > { -1, 0 }
>> > > };
>> > >
>> >
>> > So we stay with the duplication (option 1), but clean it up. That works
>> > for me.
>> >
>>
>> Actually it may be simpler.
>> Just define int idmap[MAX_ID] and assign to every id corresponding func.
>> It will be quick and simple.
>
> Unlike the ima_read_id_to_func() above or the original switch/case
> statement, this method assumes the kernel_read_file_id enumeration stays
> in sync with ima_hooks.
Actually not necessary. You can use array initialization by index,
then you do not need to worry about sync...
static int idmap[] = {
[READING_FIRMWARE] = FIRMWARE_CHECK,
[READING_MODULE] = MODULE_CHECK,
...
};
> In terms of the ima_read_id_to_func()
> function, it would iterate over the map[] to find the corresponding .id,
> whereas the current switch/case is a direct lookup.
>
switch is also iteration.
Dmitry
Actually
> Perhaps we should defer making a change for now.
>
> Mimi
>
--
Thanks,
Dmitry
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v3 19/22] ima: support for kexec image and initramfs
On Wed, Feb 3, 2016 at 9:06 PM, Mimi Zohar wrote:
> Add IMA policy support for measuring/appraising the kexec image and
> initramfs.
>
> Moving the enumeration to the vfs layer simplified the patches, allowing
> the IMA changes, for the most part, to be separated from the other
> changes. Unfortunately, passing either a kernel_read_file_id or a
> ima_hooks enumeration within IMA is messy.
>
> Option 1: duplicate kernel_read_file enumeration in ima_hooks
>
> enum kernel_read_file_id {
> ...
> READING_KEXEC_IMAGE,
> READING_KEXEC_INITRAMFS,
> READING_MAX_ID
>
> enum ima_hooks {
> ...
> KEXEC_CHECK
> INITRAMFS_CHECK
>
> Option 2: define ima_hooks as extension of kernel_read_file
> eg: enum ima_hooks {
> FILE_CHECK = READING_MAX_ID,
> MMAP_CHECK,
>
> In order to pass both kernel_read_file_id and ima_hooks values, we
> would need to specify a struct containing a union.
>
> struct caller_id {
> union {
> enum ima_hooks func_id;
> enum kernel_read_file_id read_id;
> };
> };
>
> Option 3: incorportate the ima_hooks enumeration into kernel_read_file_id,
> perhaps changing the enumeration name.
>
> For now, duplicate the new READING_KEXEC_IMAGE/INITRAMFS in ima_hooks.
>
> Signed-off-by: Mimi Zohar
> ---
> Documentation/ABI/testing/ima_policy | 2 +-
> security/integrity/ima/ima.h | 2 ++
> security/integrity/ima/ima_main.c| 19 ---
> security/integrity/ima/ima_policy.c | 13 -
> 4 files changed, 31 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/ABI/testing/ima_policy
> b/Documentation/ABI/testing/ima_policy
> index 0a378a8..e80f767 100644
> --- a/Documentation/ABI/testing/ima_policy
> +++ b/Documentation/ABI/testing/ima_policy
> @@ -26,7 +26,7 @@ Description:
> option: [[appraise_type=]] [permit_directio]
>
> base: func:=
> [BPRM_CHECK][MMAP_CHECK][FILE_CHECK][MODULE_CHECK]
> - [FIRMWARE_CHECK]
> + [FIRMWARE_CHECK] [KEXEC_CHECK]
> [INITRAMFS_CHECK]
> mask:= [[^]MAY_READ] [[^]MAY_WRITE] [[^]MAY_APPEND]
>[[^]MAY_EXEC]
> fsmagic:= hex value
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index a5d2592..832e62a 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -147,6 +147,8 @@ enum ima_hooks {
> POST_SETATTR,
> MODULE_CHECK,
> FIRMWARE_CHECK,
> + KEXEC_CHECK,
> + INITRAMFS_CHECK,
> MAX_CHECK
> };
>
> diff --git a/security/integrity/ima/ima_main.c
> b/security/integrity/ima/ima_main.c
> index 1e91d94..ccf9526 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -355,7 +355,7 @@ int ima_read_file(struct file *file, enum
> kernel_read_file_id read_id)
> int ima_post_read_file(struct file *file, void *buf, loff_t size,
>enum kernel_read_file_id read_id)
> {
> - enum ima_hooks func = FILE_CHECK;
> + enum ima_hooks func;
>
> if (!file && read_id == READING_FIRMWARE) {
> if ((ima_appraise & IMA_APPRAISE_FIRMWARE) &&
> @@ -373,10 +373,23 @@ int ima_post_read_file(struct file *file, void *buf,
> loff_t size,
> return 0;
> }
>
> - if (read_id == READING_FIRMWARE)
> + switch (read_id) {
> + case READING_FIRMWARE:
> func = FIRMWARE_CHECK;
> - else if (read_id == READING_MODULE)
> + break;
> + case READING_MODULE:
> func = MODULE_CHECK;
> + break;
> + case READING_KEXEC_IMAGE:
> + func = KEXEC_CHECK;
> + break;
> + case READING_KEXEC_INITRAMFS:
> + func = INITRAMFS_CHECK;
> + break;
> + default:
> + func = FILE_CHECK;
> + break;
> + }
>
I would define a separate function like "int ima_read_id_to_func(id)"
which search over the map
Something like...
struct
{
int id;
int func;
} map[] = {
{ .id = READING_FIRMWARE, .fun = FIRMWARE_CHECK },
...
{ -1, 0 }
};
Dmitry
> return process_measurement(file, buf, size, MAY_READ, func, 0);
> }
> diff --git a/security/integrity/ima/ima_policy.c
> b/security/integrity/ima/ima_policy.c
> index 7571ce8..d02560e 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -612,6 +612,10 @@ static int ima_parse_rule(char *rule, struct
> ima_rule_entry *entry)
> entry->func = MMAP_CHECK;
> else if (strcmp(args[0].from, "BPRM_CHECK") == 0)
> entry->func = BPRM_CHECK;
> + else if (strcmp(args[0].from, "KEXEC_CHECK") == 0)
> +
Re: [PATCH v3 11/22] ima: define a new hook to measure and appraise a file already in memory
On Wed, Feb 3, 2016 at 9:06 PM, Mimi Zohar wrote:
> This patch defines a new IMA hook ima_post_read_file() for measuring
> and appraising files read by the kernel. The caller loads the file into
> memory before calling this function, which calculates the hash followed by
> the normal IMA policy based processing.
>
> Changelog v3:
> - rename ima_hash_and_process_file() to ima_post_read_file()
>
> v1:
> - To simplify patch review, separate the IMA changes from the kexec
> and initramfs changes in "ima: measure and appraise kexec image and
> initramfs" patch. This patch contains just the IMA changes. The
> kexec and initramfs changes are with the rest of the kexec changes
> in "kexec: replace call to copy_file_from_fd() with kernel version".
>
> Signed-off-by: Mimi Zohar
Acked-by: Dmitry Kasatkin
> ---
> include/linux/ima.h | 8 +++
> include/linux/security.h | 1 +
> security/integrity/ima/ima.h | 4 +++-
> security/integrity/ima/ima_api.c | 6 +++--
> security/integrity/ima/ima_appraise.c | 2 +-
> security/integrity/ima/ima_main.c | 45
> ---
> security/integrity/ima/ima_policy.c | 1 +
> security/integrity/integrity.h| 7 --
> security/security.c | 7 +-
> 9 files changed, 66 insertions(+), 15 deletions(-)
>
> diff --git a/include/linux/ima.h b/include/linux/ima.h
> index 120ccc5..d29a6a2 100644
> --- a/include/linux/ima.h
> +++ b/include/linux/ima.h
> @@ -20,6 +20,8 @@ extern void ima_file_free(struct file *file);
> extern int ima_file_mmap(struct file *file, unsigned long prot);
> extern int ima_module_check(struct file *file);
> extern int ima_fw_from_file(struct file *file, char *buf, size_t size);
> +extern int ima_post_read_file(struct file *file, void *buf, loff_t size,
> + enum kernel_read_file_id id);
>
> #else
> static inline int ima_bprm_check(struct linux_binprm *bprm)
> @@ -52,6 +54,12 @@ static inline int ima_fw_from_file(struct file *file, char
> *buf, size_t size)
> return 0;
> }
>
> +static inline int ima_post_read_file(struct file *file, void *buf, loff_t
> size,
> +enum kernel_read_file_id id)
> +{
> + return 0;
> +}
> +
> #endif /* CONFIG_IMA */
>
> #ifdef CONFIG_IMA_APPRAISE
> diff --git a/include/linux/security.h b/include/linux/security.h
> index b68ce94..d920718 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -24,6 +24,7 @@
>
> #include
> #include
> +#include
> #include
> #include
> #include
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index 2c5262f..0b7134c 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -19,6 +19,7 @@
>
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -152,7 +153,8 @@ enum ima_hooks {
> int ima_get_action(struct inode *inode, int mask, enum ima_hooks func);
> int ima_must_measure(struct inode *inode, int mask, enum ima_hooks func);
> int ima_collect_measurement(struct integrity_iint_cache *iint,
> - struct file *file, enum hash_algo algo);
> + struct file *file, void *buf, loff_t size,
> + enum hash_algo algo);
> void ima_store_measurement(struct integrity_iint_cache *iint, struct file
> *file,
>const unsigned char *filename,
>struct evm_ima_xattr_data *xattr_value,
> diff --git a/security/integrity/ima/ima_api.c
> b/security/integrity/ima/ima_api.c
> index 8750254..370e42d 100644
> --- a/security/integrity/ima/ima_api.c
> +++ b/security/integrity/ima/ima_api.c
> @@ -188,7 +188,8 @@ int ima_get_action(struct inode *inode, int mask, enum
> ima_hooks func)
> * Return 0 on success, error code otherwise
> */
> int ima_collect_measurement(struct integrity_iint_cache *iint,
> - struct file *file, enum hash_algo algo)
> + struct file *file, void *buf, loff_t size,
> + enum hash_algo algo)
> {
> const char *audit_cause = "failed";
> struct inode *inode = file_inode(file);
> @@ -210,7 +211,8 @@ int ima_collect_measurement(struct integrity_iint_cache
> *iint,
>
> hash.hdr.algo = algo;
>
> - result = ima_calc_file_hash(file, &hash.hdr);
> + result = (!buf) ? ima_calc_file_hash(file, &hash.hdr) :
> + ima_calc_buffer_hash(
Re: [PATCH v3 22/22] ima: require signed IMA policy
On Wed, Feb 3, 2016 at 9:06 PM, Mimi Zohar wrote:
> Require the IMA policy to be signed when additional rules can be added.
>
> v1:
> - initialize the policy flag
> - include IMA_APPRAISE_POLICY in the policy flag
>
> Signed-off-by: Mimi Zohar
Acked-by: Dmitry Kasatkin
> ---
> security/integrity/ima/ima_policy.c | 7 +++
> 1 file changed, 7 insertions(+)
>
> diff --git a/security/integrity/ima/ima_policy.c
> b/security/integrity/ima/ima_policy.c
> index 39a811a..ba0f6dc 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -129,6 +129,10 @@ static struct ima_rule_entry default_appraise_rules[] = {
> {.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},
> +#ifdef CONFIG_IMA_WRITE_POLICY
> + {.action = APPRAISE, .func = POLICY_CHECK,
> + .flags = IMA_FUNC | IMA_DIGSIG_REQUIRED},
> +#endif
> #ifndef CONFIG_IMA_APPRAISE_SIGNED_INIT
> {.action = APPRAISE, .fowner = GLOBAL_ROOT_UID, .flags = IMA_FOWNER},
> #else
> @@ -412,9 +416,12 @@ void __init ima_init_policy(void)
> for (i = 0; i < appraise_entries; i++) {
> list_add_tail(&default_appraise_rules[i].list,
> &ima_default_rules);
> + if (default_appraise_rules[i].func == POLICY_CHECK)
> + temp_ima_appraise |= IMA_APPRAISE_POLICY;
> }
>
> ima_rules = &ima_default_rules;
> + ima_update_policy_flag();
> }
>
> /* Make sure we have a valid policy, at least containing some rules. */
> --
> 2.1.0
>
--
Thanks,
Dmitry
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v3 21/22] ima: measure and appraise the IMA policy itself
On Wed, Feb 3, 2016 at 9:06 PM, Mimi Zohar wrote:
> Add support for measuring and appraising the IMA policy itself.
>
> Signed-off-by: Mimi Zohar
Acked-by: Dmitry Kasatkin
But from Documentation/CodingStyle
if (condition)
do_this();
else
do_that();
This does not apply if only one branch of a conditional statement is a single
statement; in the latter case use braces in both branches:
if (condition) {
do_this();
do_that();
} else {
otherwise();
}
You have similar issue in other patches as well...
Dmitry
> ---
> security/integrity/ima/ima.h| 2 ++
> security/integrity/ima/ima_fs.c | 9 -
> security/integrity/ima/ima_main.c | 3 +++
> security/integrity/ima/ima_policy.c | 10 +-
> 4 files changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index 832e62a..6685968 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -149,6 +149,7 @@ enum ima_hooks {
> FIRMWARE_CHECK,
> KEXEC_CHECK,
> INITRAMFS_CHECK,
> + POLICY_CHECK,
> MAX_CHECK
> };
>
> @@ -191,6 +192,7 @@ int ima_policy_show(struct seq_file *m, void *v);
> #define IMA_APPRAISE_LOG 0x04
> #define IMA_APPRAISE_MODULES 0x08
> #define IMA_APPRAISE_FIRMWARE 0x10
> +#define IMA_APPRAISE_POLICY0x20
>
> #ifdef CONFIG_IMA_APPRAISE
> int ima_appraise_measurement(enum ima_hooks func,
> diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
> index 00ccd67..7b15e80 100644
> --- a/security/integrity/ima/ima_fs.c
> +++ b/security/integrity/ima/ima_fs.c
> @@ -325,7 +325,14 @@ static ssize_t ima_write_policy(struct file *file, const
> char __user *buf,
>
> if (data[0] == '/')
> result = ima_read_policy(data);
> - else
> + else if (ima_appraise & IMA_APPRAISE_POLICY) {
> + pr_err("IMA: signed policy required\n");
> + integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL, NULL,
> + "policy_update", "signed policy required",
> + 1, 0);
> + if (ima_appraise & IMA_APPRAISE_ENFORCE)
> + result = -EACCES;
> + } else
> result = ima_parse_add_rule(data);
> mutex_unlock(&ima_write_mutex);
> out_free:
> diff --git a/security/integrity/ima/ima_main.c
> b/security/integrity/ima/ima_main.c
> index ccf9526..497a6f2 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -386,6 +386,9 @@ int ima_post_read_file(struct file *file, void *buf,
> loff_t size,
> case READING_KEXEC_INITRAMFS:
> func = INITRAMFS_CHECK;
> break;
> + case READING_POLICY:
> + func = POLICY_CHECK;
> + break;
> default:
> func = FILE_CHECK;
> break;
> diff --git a/security/integrity/ima/ima_policy.c
> b/security/integrity/ima/ima_policy.c
> index d02560e..39a811a 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -114,6 +114,7 @@ static struct ima_rule_entry default_measurement_rules[]
> = {
> .uid = GLOBAL_ROOT_UID, .flags = IMA_FUNC | IMA_INMASK | IMA_UID},
> {.action = MEASURE, .func = MODULE_CHECK, .flags = IMA_FUNC},
> {.action = MEASURE, .func = FIRMWARE_CHECK, .flags = IMA_FUNC},
> + {.action = MEASURE, .func = POLICY_CHECK, .flags = IMA_FUNC},
> };
>
> static struct ima_rule_entry default_appraise_rules[] = {
> @@ -616,6 +617,8 @@ static int ima_parse_rule(char *rule, struct
> ima_rule_entry *entry)
> entry->func = KEXEC_CHECK;
> else if (strcmp(args[0].from, "INITRAMFS_CHECK") == 0)
> entry->func = INITRAMFS_CHECK;
> + else if (strcmp(args[0].from, "POLICY_CHECK") == 0)
> + entry->func = POLICY_CHECK;
> else
> result = -EINVAL;
> if (!result)
> @@ -774,6 +777,8 @@ static int ima_parse_rule(char *rule, struct
> ima_rule_entry *entry)
> temp_ima_appraise |= IMA_APPRAISE_MODULES;
> else if (entry->func == FIRMWARE_CHECK)
> temp_ima_appraise |= IMA_APPRAISE_FIRMWARE;
> + else if (entry->func == POLICY_CHECK)
> + temp_ima_appraise |= IMA_APPRAISE_POLICY;
> audit_log_format(
Re: [PATCH v3 17/22] ima: remove firmware and module specific cached status info
On Wed, Feb 3, 2016 at 9:06 PM, Mimi Zohar wrote:
> Each time a file is read by the kernel, the file should be re-measured and
> the file signature re-appraised, based on policy. As there is no need to
> preserve the status information, this patch replaces the firmware and
> module specific cache status with a generic one named read_file.
>
> This change simplifies adding support for other files read by the kernel.
>
> Signed-off-by: Mimi Zohar
> ---
> security/integrity/iint.c | 4 ++--
> security/integrity/ima/ima.h | 3 ++-
> security/integrity/ima/ima_appraise.c | 35
> ---
> security/integrity/ima/ima_policy.c | 9 -
> security/integrity/integrity.h| 16
> 5 files changed, 28 insertions(+), 39 deletions(-)
>
> diff --git a/security/integrity/iint.c b/security/integrity/iint.c
> index 8f1ab37..345b759 100644
> --- a/security/integrity/iint.c
> +++ b/security/integrity/iint.c
> @@ -77,7 +77,7 @@ static void iint_free(struct integrity_iint_cache *iint)
> iint->ima_file_status = INTEGRITY_UNKNOWN;
> iint->ima_mmap_status = INTEGRITY_UNKNOWN;
> iint->ima_bprm_status = INTEGRITY_UNKNOWN;
> - iint->ima_module_status = INTEGRITY_UNKNOWN;
> + iint->ima_read_status = INTEGRITY_UNKNOWN;
> iint->evm_status = INTEGRITY_UNKNOWN;
> kmem_cache_free(iint_cache, iint);
> }
> @@ -157,7 +157,7 @@ static void init_once(void *foo)
> iint->ima_file_status = INTEGRITY_UNKNOWN;
> iint->ima_mmap_status = INTEGRITY_UNKNOWN;
> iint->ima_bprm_status = INTEGRITY_UNKNOWN;
> - iint->ima_module_status = INTEGRITY_UNKNOWN;
> + iint->ima_read_status = INTEGRITY_UNKNOWN;
> iint->evm_status = INTEGRITY_UNKNOWN;
> }
>
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index 0b7134c..a5d2592 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -144,9 +144,10 @@ enum ima_hooks {
> FILE_CHECK = 1,
> MMAP_CHECK,
> BPRM_CHECK,
> + POST_SETATTR,
> MODULE_CHECK,
> FIRMWARE_CHECK,
> - POST_SETATTR
> + MAX_CHECK
> };
>
> /* LIM API function definitions */
> diff --git a/security/integrity/ima/ima_appraise.c
> b/security/integrity/ima/ima_appraise.c
> index cb0d0ff..6b4694a 100644
> --- a/security/integrity/ima/ima_appraise.c
> +++ b/security/integrity/ima/ima_appraise.c
> @@ -74,13 +74,12 @@ enum integrity_status ima_get_cache_status(struct
> integrity_iint_cache *iint,
> return iint->ima_mmap_status;
> case BPRM_CHECK:
> return iint->ima_bprm_status;
> - case MODULE_CHECK:
> - return iint->ima_module_status;
> - case FIRMWARE_CHECK:
> - return iint->ima_firmware_status;
> case FILE_CHECK:
> - default:
> + case POST_SETATTR:
> return iint->ima_file_status;
> + case MODULE_CHECK ... MAX_CHECK - 1:
Will LLVM clang handles this range?
Otherwise it can be just like:
case MODULE_CHECK ... MAX_CHECK :
> + default:
> + return iint->ima_read_status;
> }
> }
>
> @@ -95,15 +94,14 @@ static void ima_set_cache_status(struct
> integrity_iint_cache *iint,
> case BPRM_CHECK:
> iint->ima_bprm_status = status;
> break;
> - case MODULE_CHECK:
> - iint->ima_module_status = status;
> - break;
> - case FIRMWARE_CHECK:
> - iint->ima_firmware_status = status;
> - break;
> case FILE_CHECK:
> - default:
> + case POST_SETATTR:
> iint->ima_file_status = status;
> + break;
> + case MODULE_CHECK ... MAX_CHECK - 1:
> + default:
> + iint->ima_read_status = status;
> + break;
> }
> }
>
> @@ -117,15 +115,14 @@ static void ima_cache_flags(struct integrity_iint_cache
> *iint,
> case BPRM_CHECK:
> iint->flags |= (IMA_BPRM_APPRAISED | IMA_APPRAISED);
> break;
> - case MODULE_CHECK:
> - iint->flags |= (IMA_MODULE_APPRAISED | IMA_APPRAISED);
> - break;
> - case FIRMWARE_CHECK:
> - iint->flags |= (IMA_FIRMWARE_APPRAISED | IMA_APPRAISED);
> - break;
> case FILE_CHECK:
> - default:
> + case POST_SETATTR:
> iint->flags |= (IMA_FILE_APPRAISED | IMA_APPRAISED);
> + break;
> + case MODULE_CHECK ... MAX_CHECK - 1:
> + default:
> + iint->flags |= (IMA_READ_APPRAISED | IMA_APPRAISED);
> + break;
> }
> }
>
> diff --git a/security/integrity/ima/ima_policy.c
> b/security/integrity/ima/ima_policy.c
> index cfbe86f..7571ce8 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -300,13
Re: [PATCH v3 10/22] ima: calculate the hash of a buffer using aynchronous hash(ahash)
On Wed, Feb 3, 2016 at 9:06 PM, Mimi Zohar wrote:
> Setting up ahash has some overhead. Only use ahash to calculate the
> hash of a buffer, if the buffer is larger than ima_ahash_minsize.
>
> Signed-off-by: Mimi Zohar
Acked-by: Dmitry Kasatkin
> ---
> security/integrity/ima/ima_crypto.c | 75
> -
> 1 file changed, 73 insertions(+), 2 deletions(-)
>
> diff --git a/security/integrity/ima/ima_crypto.c
> b/security/integrity/ima/ima_crypto.c
> index fccb6ce..38f2ed8 100644
> --- a/security/integrity/ima/ima_crypto.c
> +++ b/security/integrity/ima/ima_crypto.c
> @@ -519,6 +519,63 @@ int ima_calc_field_array_hash(struct ima_field_data
> *field_data,
> return rc;
> }
>
> +static int calc_buffer_ahash_atfm(const void *buf, loff_t len,
> + struct ima_digest_data *hash,
> + struct crypto_ahash *tfm)
> +{
> + struct ahash_request *req;
> + struct scatterlist sg;
> + struct ahash_completion res;
> + int rc, ahash_rc = 0;
> +
> + hash->length = crypto_ahash_digestsize(tfm);
> +
> + req = ahash_request_alloc(tfm, GFP_KERNEL);
> + if (!req)
> + return -ENOMEM;
> +
> + init_completion(&res.completion);
> + ahash_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG |
> + CRYPTO_TFM_REQ_MAY_SLEEP,
> + ahash_complete, &res);
> +
> + rc = ahash_wait(crypto_ahash_init(req), &res);
> + if (rc)
> + goto out;
> +
> + sg_init_one(&sg, buf, len);
> + ahash_request_set_crypt(req, &sg, NULL, len);
> +
> + ahash_rc = crypto_ahash_update(req);
> +
> + /* wait for the update request to complete */
> + rc = ahash_wait(ahash_rc, &res);
> + if (!rc) {
> + ahash_request_set_crypt(req, NULL, hash->digest, 0);
> + rc = ahash_wait(crypto_ahash_final(req), &res);
> + }
> +out:
> + ahash_request_free(req);
> + return rc;
> +}
> +
> +static int calc_buffer_ahash(const void *buf, loff_t len,
> +struct ima_digest_data *hash)
> +{
> + struct crypto_ahash *tfm;
> + int rc;
> +
> + tfm = ima_alloc_atfm(hash->algo);
> + if (IS_ERR(tfm))
> + return PTR_ERR(tfm);
> +
> + rc = calc_buffer_ahash_atfm(buf, len, hash, tfm);
> +
> + ima_free_atfm(tfm);
> +
> + return rc;
> +}
> +
> static int calc_buffer_shash_tfm(const void *buf, loff_t size,
> struct ima_digest_data *hash,
> struct crypto_shash *tfm)
> @@ -550,8 +607,8 @@ static int calc_buffer_shash_tfm(const void *buf, loff_t
> size,
> return rc;
> }
>
> -int ima_calc_buffer_hash(const void *buf, loff_t len,
> -struct ima_digest_data *hash)
> +static int calc_buffer_shash(const void *buf, loff_t len,
> +struct ima_digest_data *hash)
> {
> struct crypto_shash *tfm;
> int rc;
> @@ -566,6 +623,20 @@ int ima_calc_buffer_hash(const void *buf, loff_t len,
> return rc;
> }
>
> +int ima_calc_buffer_hash(const void *buf, loff_t len,
> +struct ima_digest_data *hash)
> +{
> + int rc;
> +
> + if (ima_ahash_minsize && len >= ima_ahash_minsize) {
> + rc = calc_buffer_ahash(buf, len, hash);
> + if (!rc)
> + return 0;
> + }
> +
> + return calc_buffer_shash(buf, len, hash);
> +}
> +
> static void __init ima_pcrread(int idx, u8 *pcr)
> {
> if (!ima_used_chip)
> --
> 2.1.0
>
--
Thanks,
Dmitry
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v3 03/22] ima: use "ima_hooks" enum as function argument
On Wed, Feb 3, 2016 at 9:06 PM, Mimi Zohar wrote:
> Cleanup the function arguments by using "ima_hooks" enumerator as needed.
>
> Signed-off-by: Mimi Zohar
Acked-by: Dmitry Kasatkin
> ---
> security/integrity/ima/ima.h | 25 +
> security/integrity/ima/ima_api.c | 6 +++---
> security/integrity/ima/ima_appraise.c | 13 +++--
> security/integrity/ima/ima_main.c | 14 +++---
> security/integrity/ima/ima_policy.c | 6 +++---
> 5 files changed, 37 insertions(+), 27 deletions(-)
>
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index fb8da36..b7e7935 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -137,9 +137,18 @@ static inline unsigned long ima_hash_key(u8 *digest)
> return hash_long(*digest, IMA_HASH_BITS);
> }
>
> +enum ima_hooks {
> + FILE_CHECK = 1,
> + MMAP_CHECK,
> + BPRM_CHECK,
> + MODULE_CHECK,
> + FIRMWARE_CHECK,
> + POST_SETATTR
> +};
> +
> /* LIM API function definitions */
> -int ima_get_action(struct inode *inode, int mask, int function);
> -int ima_must_measure(struct inode *inode, int mask, int function);
> +int ima_get_action(struct inode *inode, int mask, enum ima_hooks func);
> +int ima_must_measure(struct inode *inode, int mask, enum ima_hooks func);
> int ima_collect_measurement(struct integrity_iint_cache *iint,
> struct file *file, enum hash_algo algo);
> void ima_store_measurement(struct integrity_iint_cache *iint, struct file
> *file,
> @@ -156,8 +165,6 @@ void ima_free_template_entry(struct ima_template_entry
> *entry);
> const char *ima_d_path(struct path *path, char **pathbuf);
>
> /* IMA policy related functions */
> -enum ima_hooks { FILE_CHECK = 1, MMAP_CHECK, BPRM_CHECK, MODULE_CHECK,
> FIRMWARE_CHECK, POST_SETATTR };
> -
> int ima_match_policy(struct inode *inode, enum ima_hooks func, int mask,
> int flags);
> void ima_init_policy(void);
> @@ -179,21 +186,22 @@ int ima_policy_show(struct seq_file *m, void *v);
> #define IMA_APPRAISE_FIRMWARE 0x10
>
> #ifdef CONFIG_IMA_APPRAISE
> -int ima_appraise_measurement(int func, struct integrity_iint_cache *iint,
> +int ima_appraise_measurement(enum ima_hooks func,
> +struct integrity_iint_cache *iint,
> struct file *file, const unsigned char *filename,
> struct evm_ima_xattr_data *xattr_value,
> int xattr_len, int opened);
> int ima_must_appraise(struct inode *inode, int mask, enum ima_hooks func);
> void ima_update_xattr(struct integrity_iint_cache *iint, struct file *file);
> enum integrity_status ima_get_cache_status(struct integrity_iint_cache *iint,
> - int func);
> + enum ima_hooks func);
> enum hash_algo ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value,
> int xattr_len);
> int ima_read_xattr(struct dentry *dentry,
>struct evm_ima_xattr_data **xattr_value);
>
> #else
> -static inline int ima_appraise_measurement(int func,
> +static inline int ima_appraise_measurement(enum ima_hooks func,
>struct integrity_iint_cache *iint,
>struct file *file,
>const unsigned char *filename,
> @@ -215,7 +223,8 @@ static inline void ima_update_xattr(struct
> integrity_iint_cache *iint,
> }
>
> static inline enum integrity_status ima_get_cache_status(struct
> integrity_iint_cache
> -*iint, int func)
> +*iint,
> +enum ima_hooks func)
> {
> return INTEGRITY_UNKNOWN;
> }
> diff --git a/security/integrity/ima/ima_api.c
> b/security/integrity/ima/ima_api.c
> index e7c7a5d..8750254 100644
> --- a/security/integrity/ima/ima_api.c
> +++ b/security/integrity/ima/ima_api.c
> @@ -156,7 +156,7 @@ err_out:
> * ima_get_action - appraise & measure decision based on policy.
> * @inode: pointer to inode to measure
> * @mask: contains the permission mask (MAY_READ, MAY_WRITE, MAY_EXECUTE)
> - * @function: calling function (FILE_CHECK, BPRM_CHECK, MMAP_CHECK,
> MODULE_CHECK)
> + * @func: caller identifier
> *
> * The policy is defined in terms of keypairs:
> * subj=, obj=, type=, func=, mask=, fsmagic=
> @@ -168,13 +16
Re: [PATCH v3 02/22] ima: refactor ima_policy_show() to display "ima_hooks" rules
On Wed, Feb 3, 2016 at 9:06 PM, Mimi Zohar wrote:
> Define and call a function to display the "ima_hooks" rules.
>
> Signed-off-by: Mimi Zohar
Acked-by: Dmitry Kasatkin
> ---
> security/integrity/ima/ima_policy.c | 63
> +
> 1 file changed, 36 insertions(+), 27 deletions(-)
>
> diff --git a/security/integrity/ima/ima_policy.c
> b/security/integrity/ima/ima_policy.c
> index e0e18cc..43b6425 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -903,6 +903,40 @@ void ima_policy_stop(struct seq_file *m, void *v)
> #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;
> + default:
> + snprintf(tbuf, sizeof(tbuf), "%d", func);
> + seq_printf(m, pt(Opt_func), tbuf);
> + break;
> + }
> + seq_puts(m, " ");
> +}
> +
> int ima_policy_show(struct seq_file *m, void *v)
> {
> struct ima_rule_entry *entry = v;
> @@ -924,33 +958,8 @@ int ima_policy_show(struct seq_file *m, void *v)
>
> seq_puts(m, " ");
>
> - if (entry->flags & IMA_FUNC) {
> - switch (entry->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;
> - default:
> - snprintf(tbuf, sizeof(tbuf), "%d", entry->func);
> - seq_printf(m, pt(Opt_func), tbuf);
> - break;
> - }
> - seq_puts(m, " ");
> - }
> + if (entry->flags & IMA_FUNC)
> + policy_func_show(m, entry->func);
>
> if (entry->flags & IMA_MASK) {
> if (entry->mask & MAY_EXEC)
> --
> 2.1.0
>
--
Thanks,
Dmitry
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
RE: [PATCH v3 20/22] ima: load policy using path
From: Petko Manolov [pet...@mip-labs.com]
Sent: Monday, February 08, 2016 12:35 PM
To: Dmitry Kasatkin
Cc: Mimi Zohar; linux-security-mod...@vger.kernel.org; Luis R. Rodriguez;
kexec@lists.infradead.org; linux-modu...@vger.kernel.org;
fsde...@vger.kernel.org; David Howells; David Woodhouse; Kees Cook; Dmitry
Torokhov; Dmitry Kasatkin; Eric Biederman; Rusty Russell; Dmitry Kasatkin
Subject: Re: [PATCH v3 20/22] ima: load policy using path
On 16-02-08 09:58:16, Dmitry Kasatkin wrote:
>
>
> From: Petko Manolov [pet...@mip-labs.com]
> Sent: Sunday, February 07, 2016 9:59 PM
> To: Mimi Zohar
> Cc: linux-security-mod...@vger.kernel.org; Luis R. Rodriguez;
> kexec@lists.infradead.org; linux-modu...@vger.kernel.org;
> fsde...@vger.kernel.org; David Howells; David Woodhouse; Kees Cook; Dmitry
> Torokhov; Dmitry Kasatkin; Eric Biederman; Rusty Russell; Dmitry Kasatkin;
> Dmitry Kasatkin
> Subject: Re: [PATCH v3 20/22] ima: load policy using path
>
> On 16-02-03 14:06:28, Mimi Zohar wrote:
> > From: Dmitry Kasatkin
> >
> > We currently cannot do appraisal or signature vetting of IMA policies
> > since we currently can only load IMA policies by writing the contents
> > of the policy directly in, as follows:
> >
> > cat policy-file > /ima/policy
> >
> > If we provide the kernel the path to the IMA policy so it can load
> > the policy itself it'd be able to later appraise or vet the file
> > signature if it has one. This patch adds support to load the IMA
> > policy with a given path as follows:
> >
> > echo /etc/ima/ima_policy > /sys/kernel/security/ima/policy
> >
> > Changelog v3:
> > - moved kernel_read_file_from_path() to a separate patch
> > v2:
> > - after re-ordering the patches, replace calling integrity_kernel_read()
> > to read the file with kernel_read_file_from_path() (Mimi)
> > - Patch description re-written by Luis R. Rodriguez
> >
> > Signed-off-by: Dmitry Kasatkin
> > Signed-off-by: Mimi Zohar
> > ---
> > include/linux/fs.h | 1 +
> > security/integrity/ima/ima_fs.c | 43
> > +++--
> > 2 files changed, 42 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/linux/fs.h b/include/linux/fs.h
> > index d4d556e..b648e6d 100644
> > --- a/include/linux/fs.h
> > +++ b/include/linux/fs.h
> > @@ -2531,6 +2531,7 @@ enum kernel_read_file_id {
> > READING_MODULE,
> > READING_KEXEC_IMAGE,
> > READING_KEXEC_INITRAMFS,
> > + READING_POLICY,
> > READING_MAX_ID
> > };
> >
> > diff --git a/security/integrity/ima/ima_fs.c
> > b/security/integrity/ima/ima_fs.c
> > index f355231..00ccd67 100644
> > --- a/security/integrity/ima/ima_fs.c
> > +++ b/security/integrity/ima/ima_fs.c
> > @@ -22,6 +22,7 @@
> > #include
> > #include
> > #include
> > +#include
> >
> > #include "ima.h"
> >
> > @@ -258,6 +259,41 @@ static const struct file_operations
> > ima_ascii_measurements_ops = {
> > .release = seq_release,
> > };
> >
> > +static ssize_t ima_read_policy(char *path)
> > +{
> > + void *data;
> > + char *datap;
> > + loff_t size;
> > + int rc, pathlen = strlen(path);
> > +
> > + char *p;
> > +
> > + /* remove \n */
> > + datap = path;
> > + strsep(&datap, "\n");
> > +
> > + rc = kernel_read_file_from_path(path, &data, &size, 0,
> > READING_POLICY);
> > + if (rc < 0)
> > + return rc;
> > +
> > + datap = data;
> > + while (size > 0 && (p = strsep(&datap, "\n"))) {
> > + pr_debug("rule: %s\n", p);
> > + rc = ima_parse_add_rule(p);
> > + if (rc < 0)
> > + break;
> > + size -= rc;
> > + }
> > +
> > + vfree(data);
> > + if (rc < 0)
> > + return rc;
> > + else if (size)
> > + return -EINVAL;
> > + else
> > + return pathlen;
> > +}
> > +
> > static ssize_t ima_write_policy(struct file *file, const char __user *buf,
> > size_t datalen, loff_t *ppos)
> > {
> > @@ -286,9 +322,12 @@ static ssize_t ima_write_policy(struct file *file,
> > const char __user *buf,
> > result = mutex_lock_interruptible(&ima_write_mutex);
> > if (result < 0)
> > goto out_free;
> > - result = ima_parse_add_rule(data);
> > - mutex_unlock(&ima_write_mutex);
> >
> > + if (data[0] == '/')
>
> >It seems that if we feed relative path to ima_policy the update will fail...
>
> Yes, i think it is always a good idea to pass absolute path.
What if we at least emit a warning so people know what's wrong?
Petko
DK: May be a good idea to print that loading policy by path or not.
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
RE: [PATCH v3 20/22] ima: load policy using path
From: Petko Manolov [pet...@mip-labs.com]
Sent: Sunday, February 07, 2016 9:59 PM
To: Mimi Zohar
Cc: linux-security-mod...@vger.kernel.org; Luis R. Rodriguez;
kexec@lists.infradead.org; linux-modu...@vger.kernel.org;
fsde...@vger.kernel.org; David Howells; David Woodhouse; Kees Cook; Dmitry
Torokhov; Dmitry Kasatkin; Eric Biederman; Rusty Russell; Dmitry Kasatkin;
Dmitry Kasatkin
Subject: Re: [PATCH v3 20/22] ima: load policy using path
On 16-02-03 14:06:28, Mimi Zohar wrote:
> From: Dmitry Kasatkin
>
> We currently cannot do appraisal or signature vetting of IMA policies
> since we currently can only load IMA policies by writing the contents
> of the policy directly in, as follows:
>
> cat policy-file > /ima/policy
>
> If we provide the kernel the path to the IMA policy so it can load
> the policy itself it'd be able to later appraise or vet the file
> signature if it has one. This patch adds support to load the IMA
> policy with a given path as follows:
>
> echo /etc/ima/ima_policy > /sys/kernel/security/ima/policy
>
> Changelog v3:
> - moved kernel_read_file_from_path() to a separate patch
> v2:
> - after re-ordering the patches, replace calling integrity_kernel_read()
> to read the file with kernel_read_file_from_path() (Mimi)
> - Patch description re-written by Luis R. Rodriguez
>
> Signed-off-by: Dmitry Kasatkin
> Signed-off-by: Mimi Zohar
> ---
> include/linux/fs.h | 1 +
> security/integrity/ima/ima_fs.c | 43
> +++--
> 2 files changed, 42 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index d4d556e..b648e6d 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2531,6 +2531,7 @@ enum kernel_read_file_id {
> READING_MODULE,
> READING_KEXEC_IMAGE,
> READING_KEXEC_INITRAMFS,
> + READING_POLICY,
> READING_MAX_ID
> };
>
> diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
> index f355231..00ccd67 100644
> --- a/security/integrity/ima/ima_fs.c
> +++ b/security/integrity/ima/ima_fs.c
> @@ -22,6 +22,7 @@
> #include
> #include
> #include
> +#include
>
> #include "ima.h"
>
> @@ -258,6 +259,41 @@ static const struct file_operations
> ima_ascii_measurements_ops = {
> .release = seq_release,
> };
>
> +static ssize_t ima_read_policy(char *path)
> +{
> + void *data;
> + char *datap;
> + loff_t size;
> + int rc, pathlen = strlen(path);
> +
> + char *p;
> +
> + /* remove \n */
> + datap = path;
> + strsep(&datap, "\n");
> +
> + rc = kernel_read_file_from_path(path, &data, &size, 0, READING_POLICY);
> + if (rc < 0)
> + return rc;
> +
> + datap = data;
> + while (size > 0 && (p = strsep(&datap, "\n"))) {
> + pr_debug("rule: %s\n", p);
> + rc = ima_parse_add_rule(p);
> + if (rc < 0)
> + break;
> + size -= rc;
> + }
> +
> + vfree(data);
> + if (rc < 0)
> + return rc;
> + else if (size)
> + return -EINVAL;
> + else
> + return pathlen;
> +}
> +
> static ssize_t ima_write_policy(struct file *file, const char __user *buf,
> size_t datalen, loff_t *ppos)
> {
> @@ -286,9 +322,12 @@ static ssize_t ima_write_policy(struct file *file, const
> char __user *buf,
> result = mutex_lock_interruptible(&ima_write_mutex);
> if (result < 0)
> goto out_free;
> - result = ima_parse_add_rule(data);
> - mutex_unlock(&ima_write_mutex);
>
> + if (data[0] == '/')
>It seems that if we feed relative path to ima_policy the update will fail...
Yes, i think it is always a good idea to pass absolute path.
Dmitry
> + result = ima_read_policy(data);
> + else
> + result = ima_parse_add_rule(data);
> + mutex_unlock(&ima_write_mutex);
> out_free:
> kfree(data);
> out:
> --
> 2.1.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe
> linux-security-module" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
Re: [RFC PATCH v2 01/11] ima: separate 'security.ima' reading functionality from collect
On Thu, Jan 21, 2016 at 3:19 PM, Mimi Zohar wrote: > On Tue, 2016-01-19 at 22:00 +0200, Dmitry Kasatkin wrote: >> Hi Mimi, >> >> Please change >> >> Signed-off-by: Dmitry Kasatkin > > I'll make the change here and in the other patches as well. > > Mimi > Thanks. -- Thanks, Dmitry ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [RFC PATCH v2 01/11] ima: separate 'security.ima' reading functionality from collect
Hi Mimi,
Please change
Signed-off-by: Dmitry Kasatkin
Thanks
Dmitry
On Mon, Jan 18, 2016 at 5:11 PM, Mimi Zohar wrote:
> From: Dmitry Kasatkin
>
> Instead of passing pointers to pointers to ima_collect_measurent() to
> read and return the 'security.ima' xattr value, this patch moves the
> functionality to the calling process_measurement() to directly read
> the xattr and pass only the hash algo to the ima_collect_measurement().
>
> Signed-off-by: Dmitry Kasatkin
> Signed-off-by: Mimi Zohar
> ---
> security/integrity/ima/ima.h | 15 +++
> security/integrity/ima/ima_api.c | 15 +++
> security/integrity/ima/ima_appraise.c | 25 ++---
> security/integrity/ima/ima_crypto.c | 2 +-
> security/integrity/ima/ima_init.c | 2 +-
> security/integrity/ima/ima_main.c | 11 +++
> security/integrity/ima/ima_template.c | 2 --
> security/integrity/ima/ima_template_lib.c | 1 -
> 8 files changed, 33 insertions(+), 40 deletions(-)
>
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index 585af61..fb8da36 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -23,6 +23,7 @@
> #include
> #include
> #include
> +#include
>
> #include "../integrity.h"
>
> @@ -140,9 +141,7 @@ static inline unsigned long ima_hash_key(u8 *digest)
> int ima_get_action(struct inode *inode, int mask, int function);
> int ima_must_measure(struct inode *inode, int mask, int function);
> int ima_collect_measurement(struct integrity_iint_cache *iint,
> - struct file *file,
> - struct evm_ima_xattr_data **xattr_value,
> - int *xattr_len);
> + struct file *file, enum hash_algo algo);
> void ima_store_measurement(struct integrity_iint_cache *iint, struct file
> *file,
>const unsigned char *filename,
>struct evm_ima_xattr_data *xattr_value,
> @@ -188,8 +187,8 @@ int ima_must_appraise(struct inode *inode, int mask, enum
> ima_hooks func);
> void ima_update_xattr(struct integrity_iint_cache *iint, struct file *file);
> enum integrity_status ima_get_cache_status(struct integrity_iint_cache *iint,
>int func);
> -void ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value, int xattr_len,
> - struct ima_digest_data *hash);
> +enum hash_algo ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value,
> +int xattr_len);
> int ima_read_xattr(struct dentry *dentry,
>struct evm_ima_xattr_data **xattr_value);
>
> @@ -221,10 +220,10 @@ static inline enum integrity_status
> ima_get_cache_status(struct integrity_iint_c
> return INTEGRITY_UNKNOWN;
> }
>
> -static inline void ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value,
> -int xattr_len,
> -struct ima_digest_data *hash)
> +static inline enum hash_algo
> +ima_get_hash_algo(struct evm_ima_xattr_data *xattr_value, int xattr_len)
> {
> + return ima_hash_algo;
> }
>
> static inline int ima_read_xattr(struct dentry *dentry,
> diff --git a/security/integrity/ima/ima_api.c
> b/security/integrity/ima/ima_api.c
> index 1d950fb..e7c7a5d 100644
> --- a/security/integrity/ima/ima_api.c
> +++ b/security/integrity/ima/ima_api.c
> @@ -18,7 +18,7 @@
> #include
> #include
> #include
> -#include
> +
> #include "ima.h"
>
> /*
> @@ -188,9 +188,7 @@ int ima_get_action(struct inode *inode, int mask, int
> function)
> * Return 0 on success, error code otherwise
> */
> int ima_collect_measurement(struct integrity_iint_cache *iint,
> - struct file *file,
> - struct evm_ima_xattr_data **xattr_value,
> - int *xattr_len)
> + struct file *file, enum hash_algo algo)
> {
> const char *audit_cause = "failed";
> struct inode *inode = file_inode(file);
> @@ -201,9 +199,6 @@ int ima_collect_measurement(struct integrity_iint_cache
> *iint,
> char digest[IMA_MAX_DIGEST_SIZE];
> } hash;
>
> - if (xattr_value)
> - *xattr_len = ima_read_xattr(file->f_path.dentry, xattr_value);
> -
> if (!(iint->flags & IMA_COLLECTED)) {
> u64 i_version = file_inode(file)->i_version;
>
> @@ -213,11 +208,7 @@ int ima_collect_measurement(struct
Re: [RFC PATCH v2 03/11] ima: provide buffer hash calculation function
On Mon, Jan 18, 2016 at 5:11 PM, Mimi Zohar wrote:
> From: Dmitry Kasatkin
>
> This patch provides convenient buffer hash calculation function.
>
> Changelog:
> - rewrite to support loff_t sized buffers - Mimi
> (based on Fenguang Wu's testing)
>
> Signed-off-by: Dmitry Kasatkin
> Signed-off-by: Mimi Zohar
> ---
> security/integrity/ima/ima.h| 2 ++
> security/integrity/ima/ima_crypto.c | 47
> +
> 2 files changed, 49 insertions(+)
>
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index fb8da36..de53631 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -107,6 +107,8 @@ int ima_add_template_entry(struct ima_template_entry
> *entry, int violation,
>const char *op, struct inode *inode,
>const unsigned char *filename);
> int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash);
> +int ima_calc_buffer_hash(const void *buf, loff_t len,
> +struct ima_digest_data *hash);
> int ima_calc_field_array_hash(struct ima_field_data *field_data,
> struct ima_template_desc *desc, int num_fields,
> struct ima_digest_data *hash);
> diff --git a/security/integrity/ima/ima_crypto.c
> b/security/integrity/ima/ima_crypto.c
> index fb30ce4..8d86281 100644
> --- a/security/integrity/ima/ima_crypto.c
> +++ b/security/integrity/ima/ima_crypto.c
> @@ -519,6 +519,53 @@ int ima_calc_field_array_hash(struct ima_field_data
> *field_data,
> return rc;
> }
>
> +static int calc_buffer_shash_tfm(const void *buf, loff_t size,
> + struct ima_digest_data *hash,
> + struct crypto_shash *tfm)
> +{
> + SHASH_DESC_ON_STACK(shash, tfm);
> + unsigned int len;
> + loff_t offset = 0;
> + int rc;
> +
> + shash->tfm = tfm;
> + shash->flags = 0;
> +
> + hash->length = crypto_shash_digestsize(tfm);
> +
> + rc = crypto_shash_init(shash);
> + if (rc != 0)
> + return rc;
> +
> + len = size < PAGE_SIZE ? size : PAGE_SIZE;
> + while (offset < size) {
> + rc = crypto_shash_update(shash, buf + offset, len);
> + if (rc)
> + break;
> + offset += len;
> + }
> +
Hello Mimi,
May be this was my earlier patch, but it seems to have a problem of
accessing beyond end of buffer using the same len.
When buffer always padded by zeros it is not a problem, but it is a bug.
This seems to be better version.
while (size) {
len = size < PAGE_SIZE ? size : PAGE_SIZE;
rc = crypto_shash_update(shash, buf, len);
if (rc)
break;
buf += len;
size -= len;
}
Please change my sign-of to: dmitry.kasat...@huawei.com
Thanks.
Dmitry
> + if (!rc)
> + rc = crypto_shash_final(shash, hash->digest);
> + return rc;
> +}
> +
> +int ima_calc_buffer_hash(const void *buf, loff_t len,
> +struct ima_digest_data *hash)
> +{
> + struct crypto_shash *tfm;
> + int rc;
> +
> + tfm = ima_alloc_tfm(hash->algo);
> + if (IS_ERR(tfm))
> + return PTR_ERR(tfm);
> +
> + rc = calc_buffer_shash_tfm(buf, len, hash, tfm);
> +
> + ima_free_tfm(tfm);
> + return rc;
> +}
> +
> static void __init ima_pcrread(int idx, u8 *pcr)
> {
> if (!ima_used_chip)
> --
> 2.1.0
>
--
Thanks,
Dmitry
___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec
