From: Guilherme M. <gui...@hp...> - 2017-05-11 14:00:50
|
The IMA policy rules and policy/appraise flags are now encapsulated on a new structure which completely describes the policy for a given namespace. The correct namespace structure is retrieved from a radix tree based on the namespace id in use by the process in the context whenever the IMA policy rules or flags are needed. The existent securityfs interface is reused to define policy per namespace. A new namespace file is used to create a folder for a given namespace id with a policy file which can then be used to define rules for that namespace. Patches 1, 2 and 4 qualify the file pathname considering multiple namespaces. Patch 3 adds a new kernel config which enables all the policy per namespace functionality. Patch 5 adds the namespace securityfs file which is the interface to define IMA policy per namespace. New policy file is creanted for each namespace and the policy securityfs mechanism is completely reused. Patche 7 adds a hook to fs/namespace.c to automatically delete all namespace IMA policy resources such as radix tree entry and securityfs files. Patches 8, 10, 11 and 14 are small implementation details Patches 6, 9, 12 are key changes to encapsulate all policy rules and flags in a structure per namespace. The correct structure is retrieved for the target namespace and the namespace rules are used on that context. Patch 13 adds the enforce_ns appraise mode which enables different appraise modes per namespace. Other areas might still need work to completely namespace IMA. For instance, EVM and templates per namespace are not yet covered. Guilherme Magalhaes (11): ima: qualify pathname in audit info record ima: qualify pathname in audit measurement record ima: qualify pathname in measurement file ima: add support to namespace securityfs file ima: store new namespace policy structure in a radix tree ima, fs: release namespace policy resources ima: new namespace policy structure to track initial namespace policy data ima: block initial namespace id on the namespace policy interface ima: delete namespace policy securityfs file in write-once mode ima: handling all policy flags per namespace using ima_ns_policy structure ima: appraise mode per namespace with new enforce_ns appraise mode fs/namespace.c | 4 + include/linux/integrity.h | 9 + security/integrity/ima/Kconfig | 8 + security/integrity/ima/ima.h | 78 ++++- security/integrity/ima/ima_api.c | 14 +- security/integrity/ima/ima_appraise.c | 30 +- security/integrity/ima/ima_fs.c | 454 ++++++++++++++++++++++++++++-- security/integrity/ima/ima_init.c | 13 +- security/integrity/ima/ima_main.c | 40 ++- security/integrity/ima/ima_policy.c | 210 +++++++++++--- security/integrity/ima/ima_template.c | 10 +- security/integrity/ima/ima_template_lib.c | 70 +++++ security/integrity/ima/ima_template_lib.h | 13 + security/integrity/integrity_audit.c | 5 + 14 files changed, 860 insertions(+), 98 deletions(-) -- 2.7.4 |
From: Guilherme M. <gui...@hp...> - 2017-05-11 14:00:58
|
Adding new fields (mount namespace id, file inode and device name) to uniquely identify a pathname considering different mount namespaces. The file inode on a given device is unique and these fields are required to identify a namespace id since this id can be released and later reused by a different namespace. Signed-off-by: Guilherme Magalhaes <gui...@hp...> --- security/integrity/ima/ima_api.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c index c2edba8..b05c1fd 100644 --- a/security/integrity/ima/ima_api.c +++ b/security/integrity/ima/ima_api.c @@ -18,6 +18,7 @@ #include <linux/fs.h> #include <linux/xattr.h> #include <linux/evm.h> +#include <linux/proc_ns.h> #include "ima.h" @@ -293,6 +294,7 @@ void ima_audit_measurement(struct integrity_iint_cache *iint, char hash[(iint->ima_hash->length * 2) + 1]; const char *algo_name = hash_algo_name[iint->ima_hash->algo]; char algo_hash[sizeof(hash) + strlen(algo_name) + 2]; + struct ns_common *ns; int i; if (iint->flags & IMA_AUDITED) @@ -312,6 +314,12 @@ void ima_audit_measurement(struct integrity_iint_cache *iint, audit_log_format(ab, " hash="); snprintf(algo_hash, sizeof(algo_hash), "%s:%s", algo_name, hash); audit_log_untrustedstring(ab, algo_hash); + ns = mntns_operations.get(current); + audit_log_format(ab, " mnt_ns=%u", ns->inum); + mntns_operations.put(ns); + audit_log_format(ab, " dev="); + audit_log_untrustedstring(ab, iint->inode->i_sb->s_id); + audit_log_format(ab, " ino=%lu", iint->inode->i_ino); audit_log_task_info(ab, current); audit_log_end(ab); -- 2.7.4 |
From: Guilherme M. <gui...@hp...> - 2017-05-11 14:00:59
|
Adding new field (mount namespace id, along with already existent file inode and device name) to uniquely identify a pathname considering different mount namespaces. The file inode on a given device is unique and these fields are required to identify a namespace id since this id can be released and later reused by a different namespace. Signed-off-by: Guilherme Magalhaes <gui...@hp...> --- security/integrity/integrity_audit.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/security/integrity/integrity_audit.c b/security/integrity/integrity_audit.c index 90987d1..e675e42 100644 --- a/security/integrity/integrity_audit.c +++ b/security/integrity/integrity_audit.c @@ -13,6 +13,7 @@ #include <linux/fs.h> #include <linux/gfp.h> #include <linux/audit.h> +#include <linux/proc_ns.h> #include "integrity.h" static int integrity_audit_info; @@ -52,8 +53,12 @@ void integrity_audit_msg(int audit_msgno, struct inode *inode, audit_log_format(ab, " comm="); audit_log_untrustedstring(ab, get_task_comm(name, current)); if (fname) { + struct ns_common *ns; audit_log_format(ab, " name="); audit_log_untrustedstring(ab, fname); + ns = mntns_operations.get(current); + audit_log_format(ab, " mnt_ns=%u", ns->inum); + mntns_operations.put(ns); } if (inode) { audit_log_format(ab, " dev="); -- 2.7.4 |
From: Guilherme M. <gui...@hp...> - 2017-05-11 14:01:04
|
Adding new fields (mount namespace id, file inode and device name) to uniquely identify a pathname in the measurement file considering multiple mount namespaces. The file inode on a given device is unique and these fields are required to identify a namespace id since this id can be released and later reused by a different namespace. These new fields are added to all measurement templates if CONFIG_IMA_PER_NAMESPACE is defined. There will still be one single measurement file even with multiple namespaces, since for the remote attestion a single and complete list is required. Signed-off-by: Guilherme Magalhaes <gui...@hp...> --- security/integrity/ima/Kconfig | 8 ++++ security/integrity/ima/ima.h | 12 ++++++ security/integrity/ima/ima_template.c | 10 ++++- security/integrity/ima/ima_template_lib.c | 70 +++++++++++++++++++++++++++++++ security/integrity/ima/ima_template_lib.h | 13 ++++++ 5 files changed, 111 insertions(+), 2 deletions(-) diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig index 370eb2f..7331ff6 100644 --- a/security/integrity/ima/Kconfig +++ b/security/integrity/ima/Kconfig @@ -219,3 +219,11 @@ config IMA_APPRAISE_SIGNED_INIT default n help This option requires user-space init to be signed. + +config IMA_PER_NAMESPACE + bool "Enable per mount-namespace handling of IMA policy." + depends on IMA + default n + help + This option enables another API in securityfs allowing IMA policies to + be defined per mount namespace. diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index b563fbd..42fb91ba 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -47,7 +47,19 @@ enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8 }; #define IMA_TEMPLATE_NUM_FIELDS_MAX 15 #define IMA_TEMPLATE_IMA_NAME "ima" +#define IMA_TEMPLATE_IMA_NG_NAME "ima-ng" +#define IMA_TEMPLATE_IMA_SIG_NAME "ima-sig" + +#ifndef CONFIG_IMA_PER_NAMESPACE #define IMA_TEMPLATE_IMA_FMT "d|n" +#define IMA_TEMPLATE_IMA_NG_FMT "d-ng|n-ng" +#define IMA_TEMPLATE_IMA_SIG_FMT "d-ng|n-ng|sig" +#else +#define IMA_TEMPLATE_IMA_FMT "nid|fi|dev|d|n" +#define IMA_TEMPLATE_IMA_NG_FMT "nid|fi|dev|d-ng|n-ng" +#define IMA_TEMPLATE_IMA_SIG_FMT "nid|fi|dev|d-ng|n-ng|sig" +#endif + /* current content of the policy */ extern int ima_policy_flag; diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c index cebb37c..db65c09 100644 --- a/security/integrity/ima/ima_template.c +++ b/security/integrity/ima/ima_template.c @@ -21,8 +21,8 @@ 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 = IMA_TEMPLATE_IMA_NG_NAME, .fmt = IMA_TEMPLATE_IMA_NG_FMT}, + {.name = IMA_TEMPLATE_IMA_SIG_NAME, .fmt = IMA_TEMPLATE_IMA_SIG_FMT}, {.name = "", .fmt = ""}, /* placeholder for a custom format */ }; @@ -40,6 +40,12 @@ static struct ima_template_field supported_fields[] = { .field_show = ima_show_template_string}, {.field_id = "sig", .field_init = ima_eventsig_init, .field_show = ima_show_template_sig}, + {.field_id = "nid", .field_init = ima_namespaceid_init, + .field_show = ima_show_namespaceid}, + {.field_id = "fi", .field_init = ima_filei_init, + .field_show = ima_show_filei}, + {.field_id = "dev", .field_init = ima_dev_init, + .field_show = ima_show_dev}, }; #define MAX_TEMPLATE_NAME_LEN 15 diff --git a/security/integrity/ima/ima_template_lib.c b/security/integrity/ima/ima_template_lib.c index f9ba37b..50cde10 100644 --- a/security/integrity/ima/ima_template_lib.c +++ b/security/integrity/ima/ima_template_lib.c @@ -14,6 +14,8 @@ */ #include "ima_template_lib.h" +#include <linux/proc_ns.h> +#include <linux/types.h> static bool ima_template_hash_algo_allowed(u8 algo) { @@ -330,3 +332,71 @@ int ima_eventsig_init(struct ima_event_data *event_data, out: return rc; } + +int ima_namespaceid_init(struct ima_event_data *event_data, + struct ima_field_data *field_data) +{ + u8 tmpbuf[64]; + struct ns_common *ns; + + ns = mntns_operations.get(current); + snprintf(tmpbuf, sizeof(tmpbuf), "mnt-ns=%u", ns->inum); + mntns_operations.put(ns); + + return ima_write_template_field_data(tmpbuf, strlen(tmpbuf), DATA_FMT_STRING, field_data); +} + +void ima_show_namespaceid(struct seq_file *m, enum ima_show_type show, + struct ima_field_data *field_data) +{ + ima_show_template_field_data(m, show, DATA_FMT_STRING, field_data); +} + +int ima_filei_init(struct ima_event_data *event_data, + struct ima_field_data *field_data) +{ + u8 tmpbuf[64]; + struct inode *inode; + int rc = 0; + + if (event_data->file) { + inode = file_inode(event_data->file); + snprintf(tmpbuf, sizeof(tmpbuf), "inode=%lu", inode->i_ino); + rc = ima_write_template_field_data(tmpbuf, strlen(tmpbuf), DATA_FMT_STRING, field_data); + } else { + pr_info("IMA: event file is NULL\n"); + } + + return rc; +} + +void ima_show_filei(struct seq_file *m, enum ima_show_type show, + struct ima_field_data *field_data) +{ + ima_show_template_field_data(m, show, DATA_FMT_STRING, field_data); +} + +int ima_dev_init(struct ima_event_data *event_data, + struct ima_field_data *field_data) +{ + u8 tmpbuf[64]; + struct inode *inode; + int rc = 0; + + if (event_data->file) { + inode = file_inode(event_data->file); + snprintf(tmpbuf, sizeof(tmpbuf), "dev=%s", inode->i_sb->s_id); //TODO: check untrusted string? see audit_log_n_untrustedstring() + tmpbuf[sizeof(tmpbuf) - 1] = 0; + rc = ima_write_template_field_data(tmpbuf, strlen(tmpbuf), DATA_FMT_STRING, field_data); + } else { + pr_info("IMA: event file is NULL\n"); + } + + return rc; +} + +void ima_show_dev(struct seq_file *m, enum ima_show_type show, + struct ima_field_data *field_data) +{ + ima_show_template_field_data(m, show, DATA_FMT_STRING, field_data); +} diff --git a/security/integrity/ima/ima_template_lib.h b/security/integrity/ima/ima_template_lib.h index c344530..cf6a6c7 100644 --- a/security/integrity/ima/ima_template_lib.h +++ b/security/integrity/ima/ima_template_lib.h @@ -26,6 +26,12 @@ 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); +void ima_show_namespaceid(struct seq_file *m, enum ima_show_type show, + struct ima_field_data *field_data); +void ima_show_filei(struct seq_file *m, enum ima_show_type show, + struct ima_field_data *field_data); +void ima_show_dev(struct seq_file *m, enum ima_show_type show, + struct ima_field_data *field_data); 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, @@ -36,4 +42,11 @@ int ima_eventname_ng_init(struct ima_event_data *event_data, struct ima_field_data *field_data); int ima_eventsig_init(struct ima_event_data *event_data, struct ima_field_data *field_data); +int ima_namespaceid_init(struct ima_event_data *event_data, + struct ima_field_data *field_data); +int ima_filei_init(struct ima_event_data *event_data, + struct ima_field_data *field_data); +int ima_dev_init(struct ima_event_data *event_data, + struct ima_field_data *field_data); + #endif /* __LINUX_IMA_TEMPLATE_LIB_H */ -- 2.7.4 |
From: Guilherme M. <gui...@hp...> - 2017-05-11 14:01:10
|
Release all namespace IMA policy resources when the mount namespace is released. This is the suggested mechanism to release namespace policy resources, but we still can discuss other methods to avoid cross-component changes. Signed-off-by: Guilherme Magalhaes <gui...@hp...> --- fs/namespace.c | 4 ++++ include/linux/integrity.h | 9 +++++++++ security/integrity/ima/ima_fs.c | 26 ++++++++++++++++++++++++++ 3 files changed, 39 insertions(+) diff --git a/fs/namespace.c b/fs/namespace.c index cc1375ef..80940998 100644 --- a/fs/namespace.c +++ b/fs/namespace.c @@ -15,6 +15,7 @@ #include <linux/user_namespace.h> #include <linux/namei.h> #include <linux/security.h> +#include <linux/integrity.h> #include <linux/cred.h> #include <linux/idr.h> #include <linux/init.h> /* init_rootfs */ @@ -3283,6 +3284,9 @@ void put_mnt_ns(struct mnt_namespace *ns) { if (!atomic_dec_and_test(&ns->count)) return; + + ima_mnt_namespace_dying(ns->ns.inum); + drop_collected_mounts(&ns->root->mnt); free_mnt_ns(ns); } diff --git a/include/linux/integrity.h b/include/linux/integrity.h index c2d6082..034d082 100644 --- a/include/linux/integrity.h +++ b/include/linux/integrity.h @@ -43,4 +43,13 @@ static inline void integrity_load_keys(void) } #endif /* CONFIG_INTEGRITY */ +#ifdef CONFIG_IMA_PER_NAMESPACE +extern void ima_mnt_namespace_dying(unsigned int ns_id); +#else +static inline void ima_mnt_namespace_dying(unsigned int ns_id) +{ + return; +} +#endif /* CONFIG_IMA_PER_NAMESPACE */ + #endif /* _LINUX_INTEGRITY_H */ diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c index ce6dcdf..56ba0ff 100644 --- a/security/integrity/ima/ima_fs.c +++ b/security/integrity/ima/ima_fs.c @@ -423,6 +423,7 @@ static ssize_t ima_write_policy(struct file *file, const char __user *buf, integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL, NULL, "policy_update", "signed policy required", 1, 0); + if (ima_appraise & IMA_APPRAISE_ENFORCE) result = -EACCES; } else { @@ -579,6 +580,31 @@ static int create_mnt_ns_directory(unsigned int ns_id) return result; } +/* + * ima_mnt_namespace_dying - releases all namespace policy resources + * It is called automatically when the namespace is released. + * @ns_id namespace id to be released + * + * Note: This function is called by put_mnt_ns() in the context + * of a namespace release. We need to make sure that a lock on + * this path is allowed. + */ +void ima_mnt_namespace_dying(unsigned int ns_id) +{ + struct ima_ns_policy *p; + + spin_lock(&ima_ns_policy_lock); + p = radix_tree_delete(&ima_ns_policy_mapping, ns_id); + + if (!p) { + spin_unlock(&ima_ns_policy_lock); + return; + } + + free_namespace_policy(p); + spin_unlock(&ima_ns_policy_lock); +} + static ssize_t handle_new_namespace_policy(const char *data, size_t datalen) { unsigned int ns_id; -- 2.7.4 |
From: Guilherme M. <gui...@hp...> - 2017-05-11 14:01:13
|
New ima_ns_policy structure to describe IMA policy data per namespace. Using a radix tree to map namespace ids to a respective ima_ns_policy structure. When it is needed to retrieve IMA policy rules/flags, the target ima_ns_policy structure is retrieved from the radix tree by getting the namespace id from the current context. Signed-off-by: Guilherme Magalhaes <gui...@hp...> --- security/integrity/ima/ima.h | 37 +++++++++++++++++ security/integrity/ima/ima_fs.c | 79 ++++++++++++++++++++++++++++++++++--- security/integrity/ima/ima_init.c | 2 + security/integrity/ima/ima_policy.c | 29 +++++++++----- 4 files changed, 133 insertions(+), 14 deletions(-) diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index 6e8ca8e..1c5c875 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -140,6 +140,21 @@ static inline void ima_load_kexec_buffer(void) {} */ extern bool ima_canonical_fmt; +/* Namespace policy globals */ +struct ima_ns_policy { + struct dentry *policy_dentry; + struct dentry *ns_dentry; + struct list_head *ima_rules; + struct list_head ima_policy_rules; + int ima_policy_flag; + int ima_appraise; +}; + +#ifdef CONFIG_IMA_PER_NAMESPACE +extern spinlock_t ima_ns_policy_lock; +extern struct radix_tree_root ima_ns_policy_mapping; +#endif + /* Internal IMA function definitions */ int ima_init(void); int ima_fs_init(void); @@ -166,6 +181,27 @@ 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); +#ifdef CONFIG_IMA_PER_NAMESPACE +static inline void ima_namespace_lock_init(void) { + spin_lock_init(&ima_ns_policy_lock); +} +static inline void ima_namespace_lock(void) { + spin_lock(&ima_ns_policy_lock); +} +static inline void ima_namespace_unlock(void) { + spin_unlock(&ima_ns_policy_lock); +} +#else +static inline void ima_namespace_lock_init(void) { + return; +} +static inline void ima_namespace_lock(void) { + return; +} +static inline void ima_namespace_unlock(void) { + return; +} +#endif /* * used to protect h_table and sha_table @@ -226,6 +262,7 @@ void ima_update_policy(void); void ima_update_policy_flag(void); ssize_t ima_parse_add_rule(char *); void ima_delete_rules(void); +void ima_free_policy_rules(struct list_head *policy_rules); int ima_check_policy(void); void *ima_policy_start(struct seq_file *m, loff_t *pos); void *ima_policy_next(struct seq_file *m, void *v, loff_t *pos); diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c index 6456407..ce6dcdf 100644 --- a/security/integrity/ima/ima_fs.c +++ b/security/integrity/ima/ima_fs.c @@ -275,6 +275,48 @@ static const struct file_operations ima_ascii_measurements_ops = { }; #ifdef CONFIG_IMA_PER_NAMESPACE +/* used for namespace policy rules initialization */ +static LIST_HEAD(empty_policy); + +static int allocate_namespace_policy(struct ima_ns_policy **ins, + struct dentry *policy_dentry, struct dentry *ns_dentry) +{ + int result; + struct ima_ns_policy *p; + + p = kmalloc(sizeof(struct ima_ns_policy), GFP_KERNEL); + if (!p) { + result = -ENOMEM; + goto out; + } + + p->policy_dentry = policy_dentry; + p->ns_dentry = ns_dentry; + p->ima_appraise = 0; + p->ima_policy_flag = 0; + INIT_LIST_HEAD(&p->ima_policy_rules); + /* namespace starts with empty rules and not pointing to + * ima_policy_rules */ + p->ima_rules = &empty_policy; + + result = 0; + *ins = p; + +out: + return result; +} + +static void free_namespace_policy(struct ima_ns_policy *ins) +{ + if (ins->policy_dentry) + securityfs_remove(ins->policy_dentry); + securityfs_remove(ins->ns_dentry); + + ima_free_policy_rules(&ins->ima_policy_rules); + + kfree(ins); +} + /* * check_mntns: check a mount namespace is valid * @@ -476,9 +518,11 @@ static int ima_release_policy(struct inode *inode, struct file *file) #ifndef CONFIG_IMA_WRITE_POLICY securityfs_remove(ima_policy); ima_policy = NULL; -#else - clear_bit(IMA_FS_BUSY, &ima_fs_flags); #endif + + /* always clear the busy flag so other namespaces can use it */ + clear_bit(IMA_FS_BUSY, &ima_fs_flags); + return 0; } @@ -500,11 +544,14 @@ static int create_mnt_ns_directory(unsigned int ns_id) int result; struct dentry *ns_dir, *ns_policy; char dir_name[64]; + struct ima_ns_policy *ins; snprintf(dir_name, sizeof(dir_name), "%u", ns_id); ns_dir = securityfs_create_dir(dir_name, ima_dir); if (IS_ERR(ns_dir)) { + /* TODO: handle EEXIST error, remove the folder and + continue the procedure */ result = PTR_ERR(ns_dir); goto out; } @@ -518,7 +565,15 @@ static int create_mnt_ns_directory(unsigned int ns_id) goto out; } - result = 0; + result = allocate_namespace_policy(&ins, ns_policy, ns_dir); + if (!result) { + result = radix_tree_insert(&ima_ns_policy_mapping, ns_id, ins); + if (result) + free_namespace_policy(ins); + } else { + securityfs_remove(ns_policy); + securityfs_remove(ns_dir); + } out: return result; @@ -528,6 +583,7 @@ static ssize_t handle_new_namespace_policy(const char *data, size_t datalen) { unsigned int ns_id; ssize_t result; + struct ima_ns_policy *ins; result = -EINVAL; @@ -536,21 +592,34 @@ static ssize_t handle_new_namespace_policy(const char *data, size_t datalen) goto out; } + rcu_read_lock(); + ins = radix_tree_lookup(&ima_ns_policy_mapping, ns_id); + rcu_read_unlock(); + if (ins) { + pr_info("IMA: directory for namespace id %u already created\n", ns_id); + result = datalen; + goto out; + } + + ima_namespace_lock(); if (check_mntns(ns_id)) { result = -ENOENT; pr_err("IMA: unused namespace id %u\n", ns_id); - goto out; + goto out_unlock; } result = create_mnt_ns_directory(ns_id); if (result != 0) { pr_err("IMA: namespace id %u directory creation failed\n", ns_id); - goto out; + goto out_unlock; } result = datalen; pr_info("IMA: directory created for namespace id %u\n", ns_id); +out_unlock: + ima_namespace_unlock(); + out: return result; } diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c index 2967d49..b557ee3 100644 --- a/security/integrity/ima/ima_init.c +++ b/security/integrity/ima/ima_init.c @@ -135,6 +135,8 @@ int __init ima_init(void) if (rc != 0) return rc; + ima_namespace_lock_init(); + ima_init_policy(); return ima_fs_init(); diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index aed47b7..2e8c3b7 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -47,6 +47,12 @@ int ima_policy_flag; static int temp_ima_appraise; +#ifdef CONFIG_IMA_PER_NAMESPACE +/* policy namespace map entries except the initial namespace policy */ +RADIX_TREE(ima_ns_policy_mapping, GFP_ATOMIC); +spinlock_t ima_ns_policy_lock; +#endif + #define MAX_LSM_RULES 6 enum lsm_rule_types { LSM_OBJ_USER, LSM_OBJ_ROLE, LSM_OBJ_TYPE, LSM_SUBJ_USER, LSM_SUBJ_ROLE, LSM_SUBJ_TYPE @@ -863,19 +869,12 @@ ssize_t ima_parse_add_rule(char *rule) return len; } -/** - * ima_delete_rules() called to cleanup invalid in-flight policy. - * We don't need locking as we operate on the temp list, which is - * different from the active one. There is also only one user of - * ima_delete_rules() at a time. - */ -void ima_delete_rules(void) +void ima_free_policy_rules(struct list_head *policy_rules) { struct ima_rule_entry *entry, *tmp; int i; - temp_ima_appraise = 0; - list_for_each_entry_safe(entry, tmp, &ima_temp_rules, list) { + list_for_each_entry_safe(entry, tmp, policy_rules, list) { for (i = 0; i < MAX_LSM_RULES; i++) kfree(entry->lsm[i].args_p); @@ -884,6 +883,18 @@ void ima_delete_rules(void) } } +/** + * ima_delete_rules() called to cleanup invalid in-flight policy. + * We don't need locking as we operate on the temp list, which is + * different from the active one. There is also only one user of + * ima_delete_rules() at a time. + */ +void ima_delete_rules(void) +{ + temp_ima_appraise = 0; + ima_free_policy_rules(&ima_temp_rules); +} + #ifdef CONFIG_IMA_READ_POLICY enum { mask_exec = 0, mask_write, mask_read, mask_append -- 2.7.4 |
From: Guilherme M. <gui...@hp...> - 2017-05-11 14:01:13
|
Creating the namespace securityfs file under ima folder. When a mount namespace id is written to the namespace file, a new folder is created and with a policy file for that specified namespace. Then, user defined policy for namespaces may be set by writing rules to this namespace policy file. With this interface, there is no need to give visibility for the securityfs inside mount namespaces or containers in userspace. Signed-off-by: Guilherme Magalhaes <gui...@hp...> --- security/integrity/ima/ima.h | 4 + security/integrity/ima/ima_fs.c | 183 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 187 insertions(+) diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index 42fb91ba..6e8ca8e 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -326,4 +326,8 @@ static inline int security_filter_rule_match(u32 secid, u32 field, u32 op, #define POLICY_FILE_FLAGS S_IWUSR #endif /* CONFIG_IMA_WRITE_POLICY */ +#ifdef CONFIG_IMA_PER_NAMESPACE +#define NAMESPACES_FILE_FLAGS S_IWUSR +#endif + #endif /* __LINUX_IMA_H */ diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c index ca303e5..6456407 100644 --- a/security/integrity/ima/ima_fs.c +++ b/security/integrity/ima/ima_fs.c @@ -23,6 +23,8 @@ #include <linux/rcupdate.h> #include <linux/parser.h> #include <linux/vmalloc.h> +#include <linux/proc_ns.h> +#include <linux/radix-tree.h> #include "ima.h" @@ -272,6 +274,40 @@ static const struct file_operations ima_ascii_measurements_ops = { .release = seq_release, }; +#ifdef CONFIG_IMA_PER_NAMESPACE +/* + * check_mntns: check a mount namespace is valid + * + * @ns_id: namespace id to be checked + * Returns 0 if the namespace is valid. + * + * Note: a better way to implement this check is needed. There are + * cases where the namespace id is valid but not in use by any process + * and then this implementation misses this case. Could we use an + * interface similar to what setns implements? + */ +static int check_mntns(unsigned int ns_id) +{ + struct task_struct *p; + int result = 1; + struct ns_common *ns; + + rcu_read_lock(); + for_each_process(p) { + ns = mntns_operations.get(p); + if (ns->inum == ns_id) { + result = 0; + mntns_operations.put(ns); + break; + } + mntns_operations.put(ns); + } + rcu_read_unlock(); + + return result; +} +#endif + static ssize_t ima_read_policy(char *path) { void *data; @@ -366,6 +402,9 @@ static struct dentry *ascii_runtime_measurements; static struct dentry *runtime_measurements_count; static struct dentry *violations; static struct dentry *ima_policy; +#ifdef CONFIG_IMA_PER_NAMESPACE +static struct dentry *ima_namespaces; +#endif enum ima_fs_flags { IMA_FS_BUSY, @@ -451,6 +490,139 @@ static const struct file_operations ima_measure_policy_ops = { .llseek = generic_file_llseek, }; +#ifdef CONFIG_IMA_PER_NAMESPACE +/* + * Assumes namespace id is in use by some process and this mapping + * does not exist in the map table. + */ +static int create_mnt_ns_directory(unsigned int ns_id) +{ + int result; + struct dentry *ns_dir, *ns_policy; + char dir_name[64]; + + snprintf(dir_name, sizeof(dir_name), "%u", ns_id); + + ns_dir = securityfs_create_dir(dir_name, ima_dir); + if (IS_ERR(ns_dir)) { + result = PTR_ERR(ns_dir); + goto out; + } + + ns_policy = securityfs_create_file("policy", POLICY_FILE_FLAGS, + ns_dir, NULL, + &ima_measure_policy_ops); + if (IS_ERR(ns_policy)) { + result = PTR_ERR(ns_policy); + securityfs_remove(ns_dir); + goto out; + } + + result = 0; + +out: + return result; +} + +static ssize_t handle_new_namespace_policy(const char *data, size_t datalen) +{ + unsigned int ns_id; + ssize_t result; + + result = -EINVAL; + + if (sscanf(data, "%u", &ns_id) != 1) { + pr_err("IMA: invalid namespace id: %s\n", data); + goto out; + } + + if (check_mntns(ns_id)) { + result = -ENOENT; + pr_err("IMA: unused namespace id %u\n", ns_id); + goto out; + } + + result = create_mnt_ns_directory(ns_id); + if (result != 0) { + pr_err("IMA: namespace id %u directory creation failed\n", ns_id); + goto out; + } + + result = datalen; + pr_info("IMA: directory created for namespace id %u\n", ns_id); + +out: + return result; +} + +static ssize_t ima_write_namespaces(struct file *file, const char __user *buf, + size_t datalen, loff_t *ppos) +{ + char *data; + ssize_t result; + + if (datalen >= PAGE_SIZE) + datalen = PAGE_SIZE - 1; + + /* No partial writes. */ + result = -EINVAL; + if (*ppos != 0) + goto out; + + result = -ENOMEM; + data = kmalloc(datalen + 1, GFP_KERNEL); + if (!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) + goto out_free; + + result = handle_new_namespace_policy(data, datalen); + + mutex_unlock(&ima_write_mutex); + +out_free: + kfree(data); +out: + return result; +} + +static int ima_open_namespaces(struct inode *inode, struct file *filp) +{ + if (!(filp->f_flags & O_WRONLY)) + return -EACCES; + + if (!capable(CAP_SYS_ADMIN)) + return -EPERM; + + if (test_and_set_bit(IMA_FS_BUSY, &ima_fs_flags)) + return -EBUSY; + return 0; +} + +static int ima_release_namespaces(struct inode *inode, struct file *file) +{ + clear_bit(IMA_FS_BUSY, &ima_fs_flags); + + return 0; +} + +static const struct file_operations ima_namespaces_ops = { + .open = ima_open_namespaces, + .write = ima_write_namespaces, + .read = seq_read, + .release = ima_release_namespaces, + .llseek = generic_file_llseek, +}; +#endif + int __init ima_fs_init(void) { ima_dir = securityfs_create_dir("ima", NULL); @@ -490,6 +662,14 @@ int __init ima_fs_init(void) if (IS_ERR(ima_policy)) goto out; +#ifdef CONFIG_IMA_PER_NAMESPACE + ima_namespaces = securityfs_create_file("namespaces", NAMESPACES_FILE_FLAGS, + ima_dir, NULL, + &ima_namespaces_ops); + if (IS_ERR(ima_namespaces)) + goto out; +#endif + return 0; out: securityfs_remove(violations); @@ -498,5 +678,8 @@ int __init ima_fs_init(void) securityfs_remove(binary_runtime_measurements); securityfs_remove(ima_dir); securityfs_remove(ima_policy); +#ifdef CONFIG_IMA_PER_NAMESPACE + securityfs_remove(ima_namespaces); +#endif return -1; } -- 2.7.4 |
From: Tycho A. <ty...@do...> - 2017-05-18 22:05:09
|
Hi Guilherme, On Thu, May 11, 2017 at 10:59:56AM -0300, Guilherme Magalhaes wrote: > +static int ima_open_namespaces(struct inode *inode, struct file *filp) > +{ > + if (!(filp->f_flags & O_WRONLY)) > + return -EACCES; > + > + if (!capable(CAP_SYS_ADMIN)) > + return -EPERM; > + > + if (test_and_set_bit(IMA_FS_BUSY, &ima_fs_flags)) > + return -EBUSY; It probably makes sense to do something like: if (!(ima_appraise & IMA_APPRAISE_NAMESPACE)) return -EINVAL; here. I'll keep playing around with this patchset and see if I have any other feedback. Cheers, Tycho |
From: Guilherme M. <gui...@hp...> - 2017-05-11 14:01:15
|
Adding the global ima_initial_namespace_policy which will be used when the initial namespace IMA policy data must be referred or when CONFIG_IMA_PER_NAMESPACE is not defined. New functions which will be used to retrieve the correct namespace IMA policy data from the radix tree map or from the ima_initial_namespace_policy. If the given namespace has not yet defined a private IMA policy, the IMA policy for that namespace falls back to the initial IMA policy by using ima_initial_namespace_policy. Signed-off-by: Guilherme Magalhaes <gui...@hp...> --- security/integrity/ima/ima.h | 6 ++ security/integrity/ima/ima_fs.c | 112 +++++++++++++++++++++++++++++------- security/integrity/ima/ima_policy.c | 72 +++++++++++++++++++++++ 3 files changed, 170 insertions(+), 20 deletions(-) diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index 1c5c875..20b927e 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -150,6 +150,7 @@ struct ima_ns_policy { int ima_appraise; }; +extern struct ima_ns_policy ima_initial_namespace_policy; #ifdef CONFIG_IMA_PER_NAMESPACE extern spinlock_t ima_ns_policy_lock; extern struct radix_tree_root ima_ns_policy_mapping; @@ -203,6 +204,11 @@ static inline void ima_namespace_unlock(void) { } #endif +/* IMA namespace function definitions */ +struct ima_ns_policy *ima_get_current_namespace_policy(void); +struct ima_ns_policy *ima_get_namespace_policy_from_inode(struct inode *inode); +struct ima_ns_policy *ima_get_policy_from_namespace(unsigned int ns_id); + /* * used to protect h_table and sha_table */ diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c index 56ba0ff..61f8da1 100644 --- a/security/integrity/ima/ima_fs.c +++ b/security/integrity/ima/ima_fs.c @@ -274,6 +274,22 @@ static const struct file_operations ima_ascii_measurements_ops = { .release = seq_release, }; +static struct dentry *ima_dir; +static struct dentry *binary_runtime_measurements; +static struct dentry *ascii_runtime_measurements; +static struct dentry *runtime_measurements_count; +static struct dentry *violations; +static struct dentry *ima_policy_initial_ns; +#ifdef CONFIG_IMA_PER_NAMESPACE +static struct dentry *ima_namespaces; +#endif + +enum ima_fs_flags { + IMA_FS_BUSY, +}; + +static unsigned long ima_fs_flags; + #ifdef CONFIG_IMA_PER_NAMESPACE /* used for namespace policy rules initialization */ static LIST_HEAD(empty_policy); @@ -348,6 +364,76 @@ static int check_mntns(unsigned int ns_id) return result; } + +/* + * ima_find_namespace_id_from_inode + * @policy_inode: the inode of the securityfs policy file for a given + * namespace + * + * Return 0 if the namespace id is not found in ima_ns_policy_mapping + */ +static unsigned int find_namespace_id_from_inode(struct inode *policy_inode) +{ + unsigned int ns_id = 0; +#ifdef CONFIG_IMA_PER_NAMESPACE + struct ima_ns_policy *ins; + void **slot; + struct radix_tree_iter iter; + + rcu_read_lock(); + radix_tree_for_each_slot(slot, &ima_ns_policy_mapping, &iter, 0) { + ins = radix_tree_deref_slot(slot); + if (unlikely(!ins)) + continue; + if (radix_tree_deref_retry(ins)) { + slot = radix_tree_iter_retry(&iter); + continue; + } + + if (ins->policy_dentry && ins->policy_dentry->d_inode == policy_inode) { + ns_id = iter.index; + break; + } + } + rcu_read_unlock(); +#endif + + return ns_id; +} + +/* + * get_namespace_policy_from_inode - Finds namespace mapping from + * securityfs policy file + * It is called to get the namespace policy reference when a seurityfs + * file such as the namespace or policy files are read or written. + * @inode: inode of the securityfs policy file under a namespace + * folder + * Expects the ima_ns_policy_lock already held + * + * Returns NULL if the namespace policy reference is not reliable once it + * probably was already released after a concurrent namespace release. + * Otherwise, the namespace policy reference is returned. + */ +struct ima_ns_policy *ima_get_namespace_policy_from_inode(struct inode *inode) +{ + unsigned int ns_id; + struct ima_ns_policy *ins; + + ns_id = find_namespace_id_from_inode(inode); +#ifdef CONFIG_IMA_PER_NAMESPACE + if (ns_id == 0 && + (!ima_policy_initial_ns || inode != ima_policy_initial_ns->d_inode)) { + /* ns_id == 0 refers to initial namespace, but inode refers to a + * namespaced policy file. It might be a race condition with + * namespace release, return invalid reference. */ + return NULL; + } +#endif + + ins = ima_get_policy_from_namespace(ns_id); + + return ins; +} #endif static ssize_t ima_read_policy(char *path) @@ -439,22 +525,6 @@ static ssize_t ima_write_policy(struct file *file, const char __user *buf, return result; } -static struct dentry *ima_dir; -static struct dentry *binary_runtime_measurements; -static struct dentry *ascii_runtime_measurements; -static struct dentry *runtime_measurements_count; -static struct dentry *violations; -static struct dentry *ima_policy; -#ifdef CONFIG_IMA_PER_NAMESPACE -static struct dentry *ima_namespaces; -#endif - -enum ima_fs_flags { - IMA_FS_BUSY, -}; - -static unsigned long ima_fs_flags; - #ifdef CONFIG_IMA_READ_POLICY static const struct seq_operations ima_policy_seqops = { .start = ima_policy_start, @@ -517,7 +587,7 @@ static int ima_release_policy(struct inode *inode, struct file *file) ima_update_policy(); #ifndef CONFIG_IMA_WRITE_POLICY - securityfs_remove(ima_policy); + securityfs_remove(ima_policy_initial_ns); ima_policy = NULL; #endif @@ -539,6 +609,8 @@ static const struct file_operations ima_measure_policy_ops = { /* * Assumes namespace id is in use by some process and this mapping * does not exist in the map table. + * @ns_id namespace id + * Expects ima_ns_policy_lock already held */ static int create_mnt_ns_directory(unsigned int ns_id) { @@ -751,10 +823,10 @@ int __init ima_fs_init(void) if (IS_ERR(violations)) goto out; - ima_policy = securityfs_create_file("policy", POLICY_FILE_FLAGS, + ima_policy_initial_ns = securityfs_create_file("policy", POLICY_FILE_FLAGS, ima_dir, NULL, &ima_measure_policy_ops); - if (IS_ERR(ima_policy)) + if (IS_ERR(ima_policy_initial_ns)) goto out; #ifdef CONFIG_IMA_PER_NAMESPACE @@ -772,7 +844,7 @@ int __init ima_fs_init(void) securityfs_remove(ascii_runtime_measurements); securityfs_remove(binary_runtime_measurements); securityfs_remove(ima_dir); - securityfs_remove(ima_policy); + securityfs_remove(ima_policy_initial_ns); #ifdef CONFIG_IMA_PER_NAMESPACE securityfs_remove(ima_namespaces); #endif diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index 2e8c3b7..8c0d4c9 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -20,6 +20,8 @@ #include <linux/rculist.h> #include <linux/genhd.h> #include <linux/seq_file.h> +#include <linux/radix-tree.h> +#include <linux/proc_ns.h> #include "ima.h" @@ -53,6 +55,17 @@ RADIX_TREE(ima_ns_policy_mapping, GFP_ATOMIC); spinlock_t ima_ns_policy_lock; #endif +/* initial namespace map entry, not added to the ima_ns_policy_mapping + * Used as policy fallback for namespaces without policy settings */ +struct ima_ns_policy ima_initial_namespace_policy = { + .policy_dentry = NULL, + .ns_dentry = NULL, + .ima_rules = NULL, + .ima_policy_rules = LIST_HEAD_INIT(ima_initial_namespace_policy.ima_policy_rules), + .ima_policy_flag = 0, + .ima_appraise = 0 + }; + #define MAX_LSM_RULES 6 enum lsm_rule_types { LSM_OBJ_USER, LSM_OBJ_ROLE, LSM_OBJ_TYPE, LSM_SUBJ_USER, LSM_SUBJ_ROLE, LSM_SUBJ_TYPE @@ -191,6 +204,65 @@ static int __init default_appraise_policy_setup(char *str) __setup("ima_appraise_tcb", default_appraise_policy_setup); /* + * ima_get_policy_from_namespace - Finds the ns_id mapping to namespace + * policy structure + * @ns_id: mount namespace id to look for in the policy mapping tree + * + * Returns either the given namespace policy data if mapped or the initial + * namespace data instead. + * + * Note that if a namespace has not a specific policy defined, it will + * fall back to the initial namespace policy. + */ +struct ima_ns_policy *ima_get_policy_from_namespace(unsigned int ns_id) +{ + struct ima_ns_policy *ins; + +#ifdef CONFIG_IMA_PER_NAMESPACE + rcu_read_lock(); + ins = radix_tree_lookup(&ima_ns_policy_mapping, ns_id); + rcu_read_unlock(); + + if (!ins) { + ins = &ima_initial_namespace_policy; + } +#else + ins = &ima_initial_namespace_policy; +#endif + + return ins; +} + +/* + * ima_get_current_namespace_policy - Finds the namespace policy mapping + * for the current task + * This function is called on the context of a syscall and then the namespace + * in use will not be released during this context. + */ +struct ima_ns_policy *ima_get_current_namespace_policy(void) +{ + struct ima_ns_policy *ins = NULL; +#ifdef CONFIG_IMA_PER_NAMESPACE + struct ns_common *ns; + + ns = mntns_operations.get(current); + if (ns) { + ins = ima_get_policy_from_namespace(ns->inum); + mntns_operations.put(ns); + } + if (!ins || (ins->ima_rules != &ins->ima_policy_rules)) { + /* if current namespace has no IMA policy, get the + * initial namespace policy */ + ins = &ima_initial_namespace_policy; + } +#else + ins = &ima_initial_namespace_policy; +#endif + + return ins; +} + +/* * The LSM policy can be reloaded, leaving the IMA LSM based rules referring * to the old, stale LSM policy. Update the IMA LSM based rules to reflect * the reloaded LSM policy. We assume the rules still exist; and BUG_ON() if -- 2.7.4 |
From: Guilherme M. <gui...@hp...> - 2017-05-11 14:01:22
|
The initial namespace policy is set through the existent interface in the ima/policy securityfs file. Block the initial namespace id when it is written to the ima/namespace securityfs file. Signed-off-by: Guilherme Magalhaes <gui...@hp...> --- security/integrity/ima/ima_fs.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c index 61f8da1..65c43e7 100644 --- a/security/integrity/ima/ima_fs.c +++ b/security/integrity/ima/ima_fs.c @@ -365,6 +365,16 @@ static int check_mntns(unsigned int ns_id) return result; } +static unsigned int initial_mntns_id; +static void get_initial_mntns_id(void) +{ + struct ns_common *ns; + + ns = mntns_operations.get(&init_task); + initial_mntns_id = ns->inum; + mntns_operations.put(ns); +} + /* * ima_find_namespace_id_from_inode * @policy_inode: the inode of the securityfs policy file for a given @@ -699,6 +709,12 @@ static ssize_t handle_new_namespace_policy(const char *data, size_t datalen) goto out; } + if (ns_id == initial_mntns_id) { + pr_err("IMA: invalid use of the initial mount namespace\n"); + result = -EINVAL; + goto out; + } + ima_namespace_lock(); if (check_mntns(ns_id)) { result = -ENOENT; @@ -835,6 +851,8 @@ int __init ima_fs_init(void) &ima_namespaces_ops); if (IS_ERR(ima_namespaces)) goto out; + + get_initial_mntns_id(); #endif return 0; -- 2.7.4 |
From: Guilherme M. <gui...@hp...> - 2017-05-11 14:01:22
|
When policy file is written and write-once is enabled, the policy file must be deleted. Select the namespace policy structure to get the correct policy file descriptor. Signed-off-by: Guilherme Magalhaes <gui...@hp...> --- security/integrity/ima/ima_fs.c | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c index 65c43e7..94e89fe 100644 --- a/security/integrity/ima/ima_fs.c +++ b/security/integrity/ima/ima_fs.c @@ -575,6 +575,7 @@ static int ima_open_policy(struct inode *inode, struct file *filp) static int ima_release_policy(struct inode *inode, struct file *file) { const char *cause = valid_policy ? "completed" : "failed"; + struct ima_ns_policy *ins; if ((file->f_flags & O_ACCMODE) == O_RDONLY) return seq_release(inode, file); @@ -595,15 +596,37 @@ static int ima_release_policy(struct inode *inode, struct file *file) return 0; } + /* get the namespace id from file->inode (policy file inode). + * We also need to synchronize this operation with concurrent namespace + * releasing. */ + ima_namespace_lock(); + ins = ima_get_namespace_policy_from_inode(inode); + if (!ins) { + /* the namespace is not valid anymore, discard new policy + * rules and exit */ + ima_delete_rules(); + valid_policy = 1; + clear_bit(IMA_FS_BUSY, &ima_fs_flags); + ima_namespace_unlock(); + return 0; + } + ima_update_policy(); #ifndef CONFIG_IMA_WRITE_POLICY - securityfs_remove(ima_policy_initial_ns); - ima_policy = NULL; + if (ins == &ima_initial_namespace_policy) { + securityfs_remove(ima_policy_initial_ns); + ima_policy_initial_ns = NULL; + } else { + securityfs_remove(ins->policy_dentry); + ins->policy_dentry = NULL; + } #endif /* always clear the busy flag so other namespaces can use it */ clear_bit(IMA_FS_BUSY, &ima_fs_flags); + ima_namespace_unlock(); + return 0; } -- 2.7.4 |
From: Guilherme M. <gui...@hp...> - 2017-05-11 14:01:35
|
Global ima_appraise still saves the initial appraise mode and it is used to initialize namespace.ima_appraise flag when a user defined policy is set. Globals moved into ima_ns_policy structure (namespace IMA policy private data): - ima_policy_flag - ima_appraise - ima_rules - ima_policy_rules Functions changed to take as parameter the correct ima_ns_policy structure. ima_initial_namespace_policy is initialized in ima_init_policy and stores the initial namespace IMA policy data. Replacing direct uses of ima_ns_policy lock with the ima_namespace_lock() and ima_namespace_unlock() functions. Signed-off-by: Guilherme Magalhaes <gui...@hp...> --- security/integrity/ima/ima.h | 17 +++--- security/integrity/ima/ima_api.c | 6 +- security/integrity/ima/ima_appraise.c | 21 +++++-- security/integrity/ima/ima_fs.c | 26 ++++++--- security/integrity/ima/ima_init.c | 11 +++- security/integrity/ima/ima_main.c | 36 ++++++++---- security/integrity/ima/ima_policy.c | 100 +++++++++++++++++++++++++--------- 7 files changed, 150 insertions(+), 67 deletions(-) diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h index 20b927e..fd5cfe9 100644 --- a/security/integrity/ima/ima.h +++ b/security/integrity/ima/ima.h @@ -61,9 +61,6 @@ enum tpm_pcrs { TPM_PCR0 = 0, TPM_PCR8 = 8 }; #endif -/* current content of the policy */ -extern int ima_policy_flag; - /* set during initialization */ extern int ima_initialized; extern int ima_used_chip; @@ -149,7 +146,6 @@ struct ima_ns_policy { int ima_policy_flag; int ima_appraise; }; - extern struct ima_ns_policy ima_initial_namespace_policy; #ifdef CONFIG_IMA_PER_NAMESPACE extern spinlock_t ima_ns_policy_lock; @@ -241,7 +237,7 @@ enum ima_hooks { /* LIM API function definitions */ int ima_get_action(struct inode *inode, int mask, - enum ima_hooks func, int *pcr); + enum ima_hooks func, int *pcr, struct ima_ns_policy *ins); 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, void *buf, loff_t size, @@ -262,10 +258,10 @@ const char *ima_d_path(const struct path *path, char **pathbuf, char *filename); /* IMA policy related functions */ int ima_match_policy(struct inode *inode, enum ima_hooks func, int mask, - int flags, int *pcr); + int flags, int *pcr, struct ima_ns_policy *ins); void ima_init_policy(void); -void ima_update_policy(void); -void ima_update_policy_flag(void); +void ima_update_policy(struct ima_ns_policy *ins); +void ima_update_policy_flag(struct ima_ns_policy *ins); ssize_t ima_parse_add_rule(char *); void ima_delete_rules(void); void ima_free_policy_rules(struct list_head *policy_rules); @@ -283,12 +279,13 @@ int ima_policy_show(struct seq_file *m, void *v); #define IMA_APPRAISE_FIRMWARE 0x10 #define IMA_APPRAISE_POLICY 0x20 + #ifdef CONFIG_IMA_APPRAISE 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 xattr_len, int opened, struct ima_ns_policy *ins); 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, @@ -304,7 +301,7 @@ static inline int ima_appraise_measurement(enum ima_hooks func, struct file *file, const unsigned char *filename, struct evm_ima_xattr_data *xattr_value, - int xattr_len, int opened) + int xattr_len, int opened, struct ima_ns_policy *ins) { return INTEGRITY_UNKNOWN; } diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c index b05c1fd..9aba542 100644 --- a/security/integrity/ima/ima_api.c +++ b/security/integrity/ima/ima_api.c @@ -173,13 +173,13 @@ void ima_add_violation(struct file *file, const unsigned char *filename, * Returns IMA_MEASURE, IMA_APPRAISE mask. * */ -int ima_get_action(struct inode *inode, int mask, enum ima_hooks func, int *pcr) +int ima_get_action(struct inode *inode, int mask, enum ima_hooks func, int *pcr, struct ima_ns_policy *ins) { int flags = IMA_MEASURE | IMA_AUDIT | IMA_APPRAISE; - flags &= ima_policy_flag; + flags &= ins->ima_policy_flag; - return ima_match_policy(inode, func, mask, flags, pcr); + return ima_match_policy(inode, func, mask, flags, pcr, ins); } /* diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index 1fd9539..510bb2f 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -26,6 +26,7 @@ static int __init default_appraise_setup(char *str) ima_appraise = IMA_APPRAISE_LOG; else if (strncmp(str, "fix", 3) == 0) ima_appraise = IMA_APPRAISE_FIX; + return 1; } @@ -38,10 +39,12 @@ __setup("ima_appraise=", default_appraise_setup); */ int ima_must_appraise(struct inode *inode, int mask, enum ima_hooks func) { - if (!ima_appraise) + struct ima_ns_policy *ins = ima_get_current_namespace_policy(); + + if (!ins->ima_appraise) return 0; - return ima_match_policy(inode, func, mask, IMA_APPRAISE, NULL); + return ima_match_policy(inode, func, mask, IMA_APPRAISE, NULL, ins); } static int ima_fix_xattr(struct dentry *dentry, @@ -189,7 +192,7 @@ 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 xattr_len, int opened, struct ima_ns_policy *ins) { static const char op[] = "appraise_data"; char *cause = "unknown"; @@ -273,7 +276,7 @@ int ima_appraise_measurement(enum ima_hooks func, out: if (status != INTEGRITY_PASS) { - if ((ima_appraise & IMA_APPRAISE_FIX) && + if ((ins->ima_appraise & IMA_APPRAISE_FIX) && (!xattr_value || xattr_value->type != EVM_IMA_XATTR_DIGSIG)) { if (!ima_fix_xattr(dentry, iint)) @@ -326,8 +329,11 @@ void ima_inode_post_setattr(struct dentry *dentry) struct inode *inode = d_backing_inode(dentry); struct integrity_iint_cache *iint; int must_appraise; + struct ima_ns_policy *ins; - if (!(ima_policy_flag & IMA_APPRAISE) || !S_ISREG(inode->i_mode) + ins = ima_get_current_namespace_policy(); + + if (!(ins->ima_policy_flag & IMA_APPRAISE) || !S_ISREG(inode->i_mode) || !(inode->i_opflags & IOP_XATTR)) return; @@ -363,8 +369,11 @@ static int ima_protect_xattr(struct dentry *dentry, const char *xattr_name, static void ima_reset_appraise_flags(struct inode *inode, int digsig) { struct integrity_iint_cache *iint; + struct ima_ns_policy *ins; + + ins = ima_get_current_namespace_policy(); - if (!(ima_policy_flag & IMA_APPRAISE) || !S_ISREG(inode->i_mode)) + if (!(ins->ima_policy_flag & IMA_APPRAISE) || !S_ISREG(inode->i_mode)) return; iint = integrity_iint_find(inode); diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c index 94e89fe..bc18722 100644 --- a/security/integrity/ima/ima_fs.c +++ b/security/integrity/ima/ima_fs.c @@ -308,7 +308,7 @@ static int allocate_namespace_policy(struct ima_ns_policy **ins, p->policy_dentry = policy_dentry; p->ns_dentry = ns_dentry; - p->ima_appraise = 0; + p->ima_appraise = ima_appraise; p->ima_policy_flag = 0; INIT_LIST_HEAD(&p->ima_policy_rules); /* namespace starts with empty rules and not pointing to @@ -488,6 +488,7 @@ static ssize_t ima_write_policy(struct file *file, const char __user *buf, { char *data; ssize_t result; + struct ima_ns_policy *ins; if (datalen >= PAGE_SIZE) datalen = PAGE_SIZE - 1; @@ -512,19 +513,30 @@ static ssize_t ima_write_policy(struct file *file, const char __user *buf, if (result < 0) goto out_free; + ima_namespace_lock(); + ins = ima_get_namespace_policy_from_inode(file->f_inode); + if (!ins) { + /* the namespace is not valid anymore, indicate the error + * and exit */ + result = -EINVAL; + goto out_unlock; + } + if (data[0] == '/') { result = ima_read_policy(data); - } else if (ima_appraise & IMA_APPRAISE_POLICY) { + } else if (ins->ima_appraise & IMA_APPRAISE_POLICY) { pr_err("IMA: signed policy file (specified as an absolute pathname) required\n"); integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL, NULL, "policy_update", "signed policy required", 1, 0); - if (ima_appraise & IMA_APPRAISE_ENFORCE) + if (ins->ima_appraise & IMA_APPRAISE_ENFORCE) result = -EACCES; } else { result = ima_parse_add_rule(data); } +out_unlock: + ima_namespace_unlock(); mutex_unlock(&ima_write_mutex); out_free: kfree(data); @@ -611,7 +623,7 @@ static int ima_release_policy(struct inode *inode, struct file *file) return 0; } - ima_update_policy(); + ima_update_policy(ins); #ifndef CONFIG_IMA_WRITE_POLICY if (ins == &ima_initial_namespace_policy) { securityfs_remove(ima_policy_initial_ns); @@ -698,16 +710,16 @@ void ima_mnt_namespace_dying(unsigned int ns_id) { struct ima_ns_policy *p; - spin_lock(&ima_ns_policy_lock); + ima_namespace_lock(); p = radix_tree_delete(&ima_ns_policy_mapping, ns_id); if (!p) { - spin_unlock(&ima_ns_policy_lock); + ima_namespace_unlock(); return; } free_namespace_policy(p); - spin_unlock(&ima_ns_policy_lock); + ima_namespace_unlock(); } static ssize_t handle_new_namespace_policy(const char *data, size_t datalen) diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c index b557ee3..f0bb196 100644 --- a/security/integrity/ima/ima_init.c +++ b/security/integrity/ima/ima_init.c @@ -96,11 +96,16 @@ static int __init ima_add_boot_aggregate(void) #ifdef CONFIG_IMA_LOAD_X509 void __init ima_load_x509(void) { - int unset_flags = ima_policy_flag & IMA_APPRAISE; + int unset_flags; + struct ima_ns_policy *ins; - ima_policy_flag &= ~unset_flags; + ins = ima_get_current_namespace_policy(); + + unset_flags = ins->ima_policy_flag & IMA_APPRAISE; + + ins->ima_policy_flag &= ~unset_flags; integrity_load_x509(INTEGRITY_KEYRING_IMA, CONFIG_IMA_X509_PATH); - ima_policy_flag |= unset_flags; + ins->ima_policy_flag |= unset_flags; } #endif diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 2aebb79..1b995bb 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -30,6 +30,7 @@ int ima_initialized; #ifdef CONFIG_IMA_APPRAISE +/* Used during IMA initialization only */ int ima_appraise = IMA_APPRAISE_ENFORCE; #else int ima_appraise; @@ -144,9 +145,13 @@ void ima_file_free(struct file *file) { struct inode *inode = file_inode(file); struct integrity_iint_cache *iint; + struct ima_ns_policy *ins; - if (!ima_policy_flag || !S_ISREG(inode->i_mode)) + ins = ima_get_current_namespace_policy(); + + if (!ins->ima_policy_flag || !S_ISREG(inode->i_mode)) { return; + } iint = integrity_iint_find(inode); if (!iint) @@ -170,17 +175,20 @@ static int process_measurement(struct file *file, char *buf, loff_t size, int xattr_len = 0; bool violation_check; enum hash_algo hash_algo; + struct ima_ns_policy *ins; - if (!ima_policy_flag || !S_ISREG(inode->i_mode)) + ins = ima_get_current_namespace_policy(); + + if (!ins->ima_policy_flag || !S_ISREG(inode->i_mode)) return 0; /* Return an IMA_MEASURE, IMA_APPRAISE, IMA_AUDIT action * bitmask based on the appraise/audit/measurement policy. * Included is the appraise submask. */ - action = ima_get_action(inode, mask, func, &pcr); + action = ima_get_action(inode, mask, func, &pcr, ins); violation_check = ((func == FILE_CHECK || func == MMAP_CHECK) && - (ima_policy_flag & IMA_MEASURE)); + (ins->ima_policy_flag & IMA_MEASURE)); if (!action && !violation_check) return 0; @@ -249,7 +257,7 @@ static int process_measurement(struct file *file, char *buf, loff_t size, xattr_value, xattr_len, pcr); if (action & IMA_APPRAISE_SUBMASK) rc = ima_appraise_measurement(func, iint, file, pathname, - xattr_value, xattr_len, opened); + xattr_value, xattr_len, opened, ins); if (action & IMA_AUDIT) ima_audit_measurement(iint, pathname); @@ -263,7 +271,7 @@ static int process_measurement(struct file *file, char *buf, loff_t size, __putname(pathbuf); out: inode_unlock(inode); - if ((rc && must_appraise) && (ima_appraise & IMA_APPRAISE_ENFORCE)) + if ((rc && must_appraise) && (ins->ima_appraise & IMA_APPRAISE_ENFORCE)) return -EACCES; return 0; } @@ -361,8 +369,10 @@ int ima_read_file(struct file *file, enum kernel_read_file_id read_id) { if (!file && read_id == READING_MODULE) { #ifndef CONFIG_MODULE_SIG_FORCE - if ((ima_appraise & IMA_APPRAISE_MODULES) && - (ima_appraise & IMA_APPRAISE_ENFORCE)) + struct ima_ns_policy *ins; + ins = ima_get_current_namespace_policy(); + if ((ins->ima_appraise & IMA_APPRAISE_MODULES) && + (ins->ima_appraise & IMA_APPRAISE_ENFORCE)) return -EACCES; /* INTEGRITY_UNKNOWN */ #endif return 0; /* We rely on module signature checking */ @@ -395,10 +405,13 @@ int ima_post_read_file(struct file *file, void *buf, loff_t size, enum kernel_read_file_id read_id) { enum ima_hooks func; + struct ima_ns_policy *ins; + + ins = ima_get_current_namespace_policy(); if (!file && read_id == READING_FIRMWARE) { - if ((ima_appraise & IMA_APPRAISE_FIRMWARE) && - (ima_appraise & IMA_APPRAISE_ENFORCE)) + if ((ins->ima_appraise & IMA_APPRAISE_FIRMWARE) && + (ins->ima_appraise & IMA_APPRAISE_ENFORCE)) return -EACCES; /* INTEGRITY_UNKNOWN */ return 0; } @@ -407,7 +420,7 @@ int ima_post_read_file(struct file *file, void *buf, loff_t size, return 0; if (!file || !buf || size == 0) { /* should never happen */ - if (ima_appraise & IMA_APPRAISE_ENFORCE) + if (ins->ima_appraise & IMA_APPRAISE_ENFORCE) return -EACCES; return 0; } @@ -425,7 +438,6 @@ static int __init init_ima(void) error = ima_init(); if (!error) { ima_initialized = 1; - ima_update_policy_flag(); } return error; } diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index 8c0d4c9..4ffb4ad 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -46,7 +46,7 @@ #define INVALID_PCR(a) (((a) < 0) || \ (a) >= (FIELD_SIZEOF(struct integrity_iint_cache, measured_pcrs) * 8)) -int ima_policy_flag; +/* used only during policy initialization and policy change */ static int temp_ima_appraise; #ifdef CONFIG_IMA_PER_NAMESPACE @@ -66,6 +66,7 @@ struct ima_ns_policy ima_initial_namespace_policy = { .ima_appraise = 0 }; + #define MAX_LSM_RULES 6 enum lsm_rule_types { LSM_OBJ_USER, LSM_OBJ_ROLE, LSM_OBJ_TYPE, LSM_SUBJ_USER, LSM_SUBJ_ROLE, LSM_SUBJ_TYPE @@ -166,11 +167,12 @@ static struct ima_rule_entry default_appraise_rules[] = { #endif }; +/* used only during policy setup of the initial namespace */ static LIST_HEAD(ima_default_rules); -static LIST_HEAD(ima_policy_rules); +/* used during policy setting and cleaned up for the next policy setting */ static LIST_HEAD(ima_temp_rules); -static struct list_head *ima_rules; +/* only used during setup of the initial namespace policy */ static int ima_policy __initdata; static int __init default_measure_policy_setup(char *str) @@ -268,13 +270,14 @@ struct ima_ns_policy *ima_get_current_namespace_policy(void) * the reloaded LSM policy. We assume the rules still exist; and BUG_ON() if * they don't. */ -static void ima_lsm_update_rules(void) +static void ima_lsm_update_rules(struct ima_ns_policy *ins) { struct ima_rule_entry *entry; int result; int i; - list_for_each_entry(entry, &ima_policy_rules, list) { + rcu_read_lock(); + list_for_each_entry_rcu(entry, &ins->ima_policy_rules, list) { for (i = 0; i < MAX_LSM_RULES; i++) { if (!entry->lsm[i].rule) continue; @@ -285,6 +288,7 @@ static void ima_lsm_update_rules(void) BUG_ON(!entry->lsm[i].rule); } } + rcu_read_unlock(); } /** @@ -297,7 +301,7 @@ static void ima_lsm_update_rules(void) * Returns true on rule match, false on failure. */ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode, - enum ima_hooks func, int mask) + enum ima_hooks func, int mask, struct ima_ns_policy *ins) { struct task_struct *tsk = current; const struct cred *cred = current_cred(); @@ -365,7 +369,7 @@ static bool ima_match_rules(struct ima_rule_entry *rule, struct inode *inode, } if ((rc < 0) && (!retried)) { retried = 1; - ima_lsm_update_rules(); + ima_lsm_update_rules(ins); goto retry; } if (!rc) @@ -412,18 +416,18 @@ static int get_subaction(struct ima_rule_entry *rule, enum ima_hooks func) * than writes so ima_match_policy() is classical RCU candidate. */ int ima_match_policy(struct inode *inode, enum ima_hooks func, int mask, - int flags, int *pcr) + int flags, int *pcr, struct ima_ns_policy *ins) { struct ima_rule_entry *entry; int action = 0, actmask = flags | (flags << 1); rcu_read_lock(); - list_for_each_entry_rcu(entry, ima_rules, list) { + list_for_each_entry_rcu(entry, ins->ima_rules, list) { if (!(entry->action & actmask)) continue; - if (!ima_match_rules(entry, inode, func, mask)) + if (!ima_match_rules(entry, inode, func, mask, ins)) continue; action |= entry->flags & IMA_ACTION_FLAGS; @@ -454,18 +458,20 @@ int ima_match_policy(struct inode *inode, enum ima_hooks func, int mask, * out of a function or not call the function in the first place * can be made earlier. */ -void ima_update_policy_flag(void) +void ima_update_policy_flag(struct ima_ns_policy *ins) { struct ima_rule_entry *entry; - list_for_each_entry(entry, ima_rules, list) { + rcu_read_lock(); + list_for_each_entry_rcu(entry, ins->ima_rules, list) { if (entry->action & IMA_DO_MASK) - ima_policy_flag |= entry->action; + ins->ima_policy_flag |= entry->action; } + rcu_read_unlock(); - ima_appraise |= temp_ima_appraise; - if (!ima_appraise) - ima_policy_flag &= ~IMA_APPRAISE; + ins->ima_appraise |= temp_ima_appraise; + if (!ins->ima_appraise) + ins->ima_policy_flag &= ~IMA_APPRAISE; } /** @@ -477,6 +483,7 @@ void ima_update_policy_flag(void) void __init ima_init_policy(void) { int i, measure_entries, appraise_entries; + struct ima_ns_policy *ins; /* if !ima_policy set entries = 0 so we load NO default rules */ measure_entries = ima_policy ? ARRAY_SIZE(dont_measure_rules) : 0; @@ -507,8 +514,13 @@ void __init ima_init_policy(void) temp_ima_appraise |= IMA_APPRAISE_POLICY; } - ima_rules = &ima_default_rules; - ima_update_policy_flag(); + ins = &ima_initial_namespace_policy; + + ins->ima_rules = &ima_default_rules; + ins->ima_appraise = ima_appraise; + + ima_update_policy_flag(ins); + temp_ima_appraise = 0; } /* Make sure we have a valid policy, at least containing some rules. */ @@ -530,14 +542,14 @@ int ima_check_policy(void) * Policy rules are never deleted so ima_policy_flag gets zeroed only once when * we switch from the default policy to user defined. */ -void ima_update_policy(void) +void ima_update_policy(struct ima_ns_policy *ins) { struct list_head *first, *last, *policy; /* append current policy with the new rules */ first = (&ima_temp_rules)->next; last = (&ima_temp_rules)->prev; - policy = &ima_policy_rules; + policy = &ins->ima_policy_rules; synchronize_rcu(); @@ -549,11 +561,14 @@ void ima_update_policy(void) /* prepare for the next policy rules addition */ INIT_LIST_HEAD(&ima_temp_rules); - if (ima_rules != policy) { - ima_policy_flag = 0; - ima_rules = policy; + if (ins->ima_rules != policy) { + ins->ima_policy_flag = 0; + ins->ima_rules = policy; + ins->ima_appraise = ima_appraise; } - ima_update_policy_flag(); + + ima_update_policy_flag(ins); + temp_ima_appraise = 0; } enum { @@ -964,6 +979,7 @@ void ima_free_policy_rules(struct list_head *policy_rules) void ima_delete_rules(void) { temp_ima_appraise = 0; + ima_free_policy_rules(&ima_temp_rules); } @@ -1002,28 +1018,49 @@ void *ima_policy_start(struct seq_file *m, loff_t *pos) { loff_t l = *pos; struct ima_rule_entry *entry; + struct ima_ns_policy *ins; + + ima_namespace_lock(); + ins = ima_get_namespace_policy_from_inode(m->file->f_inode); + if (!ins) { + ima_namespace_unlock(); + return NULL; + } rcu_read_lock(); - list_for_each_entry_rcu(entry, ima_rules, list) { + list_for_each_entry_rcu(entry, ins->ima_rules, list) { if (!l--) { rcu_read_unlock(); + ima_namespace_unlock(); return entry; } } rcu_read_unlock(); + ima_namespace_unlock(); return NULL; } void *ima_policy_next(struct seq_file *m, void *v, loff_t *pos) { struct ima_rule_entry *entry = v; + struct ima_ns_policy *ins; + void *p; + + ima_namespace_lock(); + ins = ima_get_namespace_policy_from_inode(m->file->f_inode); + if (!ins) { + ima_namespace_unlock(); + return NULL; + } rcu_read_lock(); entry = list_entry_rcu(entry->list.next, struct ima_rule_entry, list); rcu_read_unlock(); (*pos)++; - return (&entry->list == ima_rules) ? NULL : entry; + p = (&entry->list == ins->ima_rules) ? NULL : entry; + ima_namespace_unlock(); + return p; } void ima_policy_stop(struct seq_file *m, void *v) @@ -1082,6 +1119,16 @@ int ima_policy_show(struct seq_file *m, void *v) struct ima_rule_entry *entry = v; int i; char tbuf[64] = {0,}; + struct ima_ns_policy *ins; + + ima_namespace_lock(); + ins = ima_get_namespace_policy_from_inode(m->file->f_inode); + if (!ins) { + /* this namespace was release and the policy entry is not valid + * anymore */ + ima_namespace_unlock(); + return 0; + } rcu_read_lock(); @@ -1184,6 +1231,7 @@ int ima_policy_show(struct seq_file *m, void *v) seq_puts(m, "permit_directio "); rcu_read_unlock(); seq_puts(m, "\n"); + ima_namespace_unlock(); return 0; } #endif /* CONFIG_IMA_READ_POLICY */ -- 2.7.4 |
From: Magalhaes, G. (B. R&D-CL) <gui...@hp...> - 2017-05-11 14:54:04
|
I would like to replace part of the email below which briefly presents each one of the patches in this series. This is the right summary: -- Patches 1, 2 and 3 qualify the file pathname considering multiple namespaces. Patch 4 adds the namespace securityfs file which is the interface to define IMA policy per namespace. New policy file is created for each namespace and the policy securityfs mechanism is completely reused. Patch 6 adds a hook to fs/namespace.c to automatically delete all namespace IMA policy resources such as radix tree entry and securityfs files. Patches 8 and 9 are small implementation details Patches 5, 7, 10 are the key changes to encapsulate all policy rules and flags in a structure per namespace. The correct structure is retrieved for the target namespace and the namespace rules are used on that context. Patch 11 adds the enforce_ns appraise mode which enables different appraise modes per namespace. -- ---- Guilherme -----Original Message----- From: Magalhaes, Guilherme (Brazil R&D-CL) Sent: quinta-feira, 11 de maio de 2017 11:00 To: dmi...@gm...; zo...@li... Cc: vi...@ze...; jam...@or...; se...@ha...; lin...@vg...; lin...@vg...; lin...@li...; lin...@li...; lin...@vg...; ty...@do...; Souza, Joaquim (Brazil R&D-ECL) <joa...@hp...>; Edwards, Nigel <nig...@hp...>; Magalhaes, Guilherme (Brazil R&D-CL) <gui...@hp...> Subject: [RFC 00/11] ima: namespace support for IMA policy The IMA policy rules and policy/appraise flags are now encapsulated on a new structure which completely describes the policy for a given namespace. The correct namespace structure is retrieved from a radix tree based on the namespace id in use by the process in the context whenever the IMA policy rules or flags are needed. The existent securityfs interface is reused to define policy per namespace. A new namespace file is used to create a folder for a given namespace id with a policy file which can then be used to define rules for that namespace. Patches 1, 2 and 4 qualify the file pathname considering multiple namespaces. Patch 3 adds a new kernel config which enables all the policy per namespace functionality. Patch 5 adds the namespace securityfs file which is the interface to define IMA policy per namespace. New policy file is creanted for each namespace and the policy securityfs mechanism is completely reused. Patche 7 adds a hook to fs/namespace.c to automatically delete all namespace IMA policy resources such as radix tree entry and securityfs files. Patches 8, 10, 11 and 14 are small implementation details Patches 6, 9, 12 are key changes to encapsulate all policy rules and flags in a structure per namespace. The correct structure is retrieved for the target namespace and the namespace rules are used on that context. Patch 13 adds the enforce_ns appraise mode which enables different appraise modes per namespace. Other areas might still need work to completely namespace IMA. For instance, EVM and templates per namespace are not yet covered. Guilherme Magalhaes (11): ima: qualify pathname in audit info record ima: qualify pathname in audit measurement record ima: qualify pathname in measurement file ima: add support to namespace securityfs file ima: store new namespace policy structure in a radix tree ima, fs: release namespace policy resources ima: new namespace policy structure to track initial namespace policy data ima: block initial namespace id on the namespace policy interface ima: delete namespace policy securityfs file in write-once mode ima: handling all policy flags per namespace using ima_ns_policy structure ima: appraise mode per namespace with new enforce_ns appraise mode fs/namespace.c | 4 + include/linux/integrity.h | 9 + security/integrity/ima/Kconfig | 8 + security/integrity/ima/ima.h | 78 ++++- security/integrity/ima/ima_api.c | 14 +- security/integrity/ima/ima_appraise.c | 30 +- security/integrity/ima/ima_fs.c | 454 ++++++++++++++++++++++++++++-- security/integrity/ima/ima_init.c | 13 +- security/integrity/ima/ima_main.c | 40 ++- security/integrity/ima/ima_policy.c | 210 +++++++++++--- security/integrity/ima/ima_template.c | 10 +- security/integrity/ima/ima_template_lib.c | 70 +++++ security/integrity/ima/ima_template_lib.h | 13 + security/integrity/integrity_audit.c | 5 + 14 files changed, 860 insertions(+), 98 deletions(-) -- 2.7.4 |
From: Mimi Z. <zo...@li...> - 2017-05-24 20:14:02
|
On Thu, 2017-05-11 at 10:59 -0300, Guilherme Magalhaes wrote: > Creating the namespace securityfs file under ima folder. When a mount > namespace id is written to the namespace file, a new folder is created and > with a policy file for that specified namespace. Then, user defined policy > for namespaces may be set by writing rules to this namespace policy file. > With this interface, there is no need to give visibility for the securityfs > inside mount namespaces or containers in userspace. > > Signed-off-by: Guilherme Magalhaes <gui...@hp...> The design needs to be flexible enough for different types of containers, not just for when the orchestration layer provides the policy. With this design, the container owner has no control over the policy. One option is that we bind mount the securityfs/policy, so that root in the container will be allowed to read/write the policy. At some point, we might connect a vTPM to the container so that the container owner would be able to get a quote. For now even without a vTPM, the same mechanism would allow root within the container to read the measurement list. Mimi > --- > security/integrity/ima/ima.h | 4 + > security/integrity/ima/ima_fs.c | 183 ++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 187 insertions(+) > > diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h > index 42fb91ba..6e8ca8e 100644 > --- a/security/integrity/ima/ima.h > +++ b/security/integrity/ima/ima.h > @@ -326,4 +326,8 @@ static inline int security_filter_rule_match(u32 secid, u32 field, u32 op, > #define POLICY_FILE_FLAGS S_IWUSR > #endif /* CONFIG_IMA_WRITE_POLICY */ > > +#ifdef CONFIG_IMA_PER_NAMESPACE > +#define NAMESPACES_FILE_FLAGS S_IWUSR > +#endif > + > #endif /* __LINUX_IMA_H */ > diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c > index ca303e5..6456407 100644 > --- a/security/integrity/ima/ima_fs.c > +++ b/security/integrity/ima/ima_fs.c > @@ -23,6 +23,8 @@ > #include <linux/rcupdate.h> > #include <linux/parser.h> > #include <linux/vmalloc.h> > +#include <linux/proc_ns.h> > +#include <linux/radix-tree.h> > > #include "ima.h" > > @@ -272,6 +274,40 @@ static const struct file_operations ima_ascii_measurements_ops = { > .release = seq_release, > }; > > +#ifdef CONFIG_IMA_PER_NAMESPACE > +/* > + * check_mntns: check a mount namespace is valid > + * > + * @ns_id: namespace id to be checked > + * Returns 0 if the namespace is valid. > + * > + * Note: a better way to implement this check is needed. There are > + * cases where the namespace id is valid but not in use by any process > + * and then this implementation misses this case. Could we use an > + * interface similar to what setns implements? > + */ > +static int check_mntns(unsigned int ns_id) > +{ > + struct task_struct *p; > + int result = 1; > + struct ns_common *ns; > + > + rcu_read_lock(); > + for_each_process(p) { > + ns = mntns_operations.get(p); > + if (ns->inum == ns_id) { > + result = 0; > + mntns_operations.put(ns); > + break; > + } > + mntns_operations.put(ns); > + } > + rcu_read_unlock(); > + > + return result; > +} > +#endif > + > static ssize_t ima_read_policy(char *path) > { > void *data; > @@ -366,6 +402,9 @@ static struct dentry *ascii_runtime_measurements; > static struct dentry *runtime_measurements_count; > static struct dentry *violations; > static struct dentry *ima_policy; > +#ifdef CONFIG_IMA_PER_NAMESPACE > +static struct dentry *ima_namespaces; > +#endif > > enum ima_fs_flags { > IMA_FS_BUSY, > @@ -451,6 +490,139 @@ static const struct file_operations ima_measure_policy_ops = { > .llseek = generic_file_llseek, > }; > > +#ifdef CONFIG_IMA_PER_NAMESPACE > +/* > + * Assumes namespace id is in use by some process and this mapping > + * does not exist in the map table. > + */ > +static int create_mnt_ns_directory(unsigned int ns_id) > +{ > + int result; > + struct dentry *ns_dir, *ns_policy; > + char dir_name[64]; > + > + snprintf(dir_name, sizeof(dir_name), "%u", ns_id); > + > + ns_dir = securityfs_create_dir(dir_name, ima_dir); > + if (IS_ERR(ns_dir)) { > + result = PTR_ERR(ns_dir); > + goto out; > + } > + > + ns_policy = securityfs_create_file("policy", POLICY_FILE_FLAGS, > + ns_dir, NULL, > + &ima_measure_policy_ops); > + if (IS_ERR(ns_policy)) { > + result = PTR_ERR(ns_policy); > + securityfs_remove(ns_dir); > + goto out; > + } > + > + result = 0; > + > +out: > + return result; > +} > + > +static ssize_t handle_new_namespace_policy(const char *data, size_t datalen) > +{ > + unsigned int ns_id; > + ssize_t result; > + > + result = -EINVAL; > + > + if (sscanf(data, "%u", &ns_id) != 1) { > + pr_err("IMA: invalid namespace id: %s\n", data); > + goto out; > + } > + > + if (check_mntns(ns_id)) { > + result = -ENOENT; > + pr_err("IMA: unused namespace id %u\n", ns_id); > + goto out; > + } > + > + result = create_mnt_ns_directory(ns_id); > + if (result != 0) { > + pr_err("IMA: namespace id %u directory creation failed\n", ns_id); > + goto out; > + } > + > + result = datalen; > + pr_info("IMA: directory created for namespace id %u\n", ns_id); > + > +out: > + return result; > +} > + > +static ssize_t ima_write_namespaces(struct file *file, const char __user *buf, > + size_t datalen, loff_t *ppos) > +{ > + char *data; > + ssize_t result; > + > + if (datalen >= PAGE_SIZE) > + datalen = PAGE_SIZE - 1; > + > + /* No partial writes. */ > + result = -EINVAL; > + if (*ppos != 0) > + goto out; > + > + result = -ENOMEM; > + data = kmalloc(datalen + 1, GFP_KERNEL); > + if (!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) > + goto out_free; > + > + result = handle_new_namespace_policy(data, datalen); > + > + mutex_unlock(&ima_write_mutex); > + > +out_free: > + kfree(data); > +out: > + return result; > +} > + > +static int ima_open_namespaces(struct inode *inode, struct file *filp) > +{ > + if (!(filp->f_flags & O_WRONLY)) > + return -EACCES; > + > + if (!capable(CAP_SYS_ADMIN)) > + return -EPERM; > + > + if (test_and_set_bit(IMA_FS_BUSY, &ima_fs_flags)) > + return -EBUSY; > + return 0; > +} > + > +static int ima_release_namespaces(struct inode *inode, struct file *file) > +{ > + clear_bit(IMA_FS_BUSY, &ima_fs_flags); > + > + return 0; > +} > + > +static const struct file_operations ima_namespaces_ops = { > + .open = ima_open_namespaces, > + .write = ima_write_namespaces, > + .read = seq_read, > + .release = ima_release_namespaces, > + .llseek = generic_file_llseek, > +}; > +#endif > + > int __init ima_fs_init(void) > { > ima_dir = securityfs_create_dir("ima", NULL); > @@ -490,6 +662,14 @@ int __init ima_fs_init(void) > if (IS_ERR(ima_policy)) > goto out; > > +#ifdef CONFIG_IMA_PER_NAMESPACE > + ima_namespaces = securityfs_create_file("namespaces", NAMESPACES_FILE_FLAGS, > + ima_dir, NULL, > + &ima_namespaces_ops); > + if (IS_ERR(ima_namespaces)) > + goto out; > +#endif > + > return 0; > out: > securityfs_remove(violations); > @@ -498,5 +678,8 @@ int __init ima_fs_init(void) > securityfs_remove(binary_runtime_measurements); > securityfs_remove(ima_dir); > securityfs_remove(ima_policy); > +#ifdef CONFIG_IMA_PER_NAMESPACE > + securityfs_remove(ima_namespaces); > +#endif > return -1; > } |
From: John J. <joh...@ca...> - 2017-05-25 07:37:21
|
On 05/24/2017 01:12 PM, Mimi Zohar wrote: > On Thu, 2017-05-11 at 10:59 -0300, Guilherme Magalhaes wrote: >> Creating the namespace securityfs file under ima folder. When a mount >> namespace id is written to the namespace file, a new folder is created and >> with a policy file for that specified namespace. Then, user defined policy >> for namespaces may be set by writing rules to this namespace policy file. >> With this interface, there is no need to give visibility for the securityfs >> inside mount namespaces or containers in userspace. >> >> Signed-off-by: Guilherme Magalhaes <gui...@hp...> > > The design needs to be flexible enough for different types of > containers, not just for when the orchestration layer provides the > policy. With this design, the container owner has no control over the > policy. > > One option is that we bind mount the securityfs/policy, so that root > in the container will be allowed to read/write the policy. At some > point, we might connect a vTPM to the container so that the container > owner would be able to get a quote. For now even without a vTPM, the > same mechanism would allow root within the container to read the > measurement list. > I haven't looked at this enough yet on IMAs end, but another possible solution is using a symlink and a magic jump_link similar to what nsfs is doing. The patch series I posted out a couple of weeks ago [RFC][Patch 0/3] securityfs: add the ability to support symlinks adds symlink support to securityfs, and then patch 3/3 cribs from nsfs updating apparmorfs to use jump_link to "virtualize" the apparmor policy directory. This avoids needing to have the bind mount. I'll break the patch out more and repost so its easier to see if this approach might work for IMA. |
From: Mimi Z. <zo...@li...> - 2017-05-25 11:47:20
|
Hi John, On Thu, 2017-05-25 at 00:36 -0700, John Johansen wrote: > On 05/24/2017 01:12 PM, Mimi Zohar wrote: > > On Thu, 2017-05-11 at 10:59 -0300, Guilherme Magalhaes wrote: > >> Creating the namespace securityfs file under ima folder. When a mount > >> namespace id is written to the namespace file, a new folder is created and > >> with a policy file for that specified namespace. Then, user defined policy > >> for namespaces may be set by writing rules to this namespace policy file. > >> With this interface, there is no need to give visibility for the securityfs > >> inside mount namespaces or containers in userspace. > >> > >> Signed-off-by: Guilherme Magalhaes <gui...@hp...> > > > > The design needs to be flexible enough for different types of > > containers, not just for when the orchestration layer provides the > > policy. With this design, the container owner has no control over the > > policy. > > > > One option is that we bind mount the securityfs/policy, so that root > > in the container will be allowed to read/write the policy. At some > > point, we might connect a vTPM to the container so that the container > > owner would be able to get a quote. For now even without a vTPM, the > > same mechanism would allow root within the container to read the > > measurement list. > > > I haven't looked at this enough yet on IMAs end, but another possible solution > is using a symlink and a magic jump_link similar to what nsfs is doing. > > The patch series I posted out a couple of weeks ago > [RFC][Patch 0/3] securityfs: add the ability to support symlinks > > adds symlink support to securityfs, and then patch 3/3 cribs from nsfs > updating apparmorfs to use jump_link to "virtualize" the apparmor policy > directory. This avoids needing to have the bind mount. > > I'll break the patch out more and repost so its easier to see if this > approach might work for IMA. Sorry, I've been meaning to take a look at your patches, but just haven't gotten to it yet. This approach sounds really promising. thanks, Mimi |
From: Magalhaes, G. (B. R&D-CL) <gui...@hp...> - 2017-05-25 19:04:25
|
Mimi, With the securityfs symlink we would address the case of setting policy inside containers, but we still would need a way to set the IMA policy per namespace outside containers. So, the current proposed interface would address the latter case. As an alternative to symlinks, taking this patch set as base, and still considering setting policy inside containers (or inside namespaces in general), it is possible to bind mount the securityfs files into the containers, but it would be needed to prevent read/write access to the namespaced IMA policy files for processes not running on the same namespace. These mechanisms would not require a change in the proposed design. Do you think these mechanisms are enough for the flexibility you asked? Thanks. -- Guilherme -----Original Message----- From: Mimi Zohar [mailto:zo...@li...] Sent: quinta-feira, 25 de maio de 2017 08:46 To: John Johansen <joh...@ca...>; Magalhaes, Guilherme (Brazil R&D-CL) <gui...@hp...>; dmi...@gm... Cc: vi...@ze...; jam...@or...; se...@ha...; lin...@vg...; lin...@vg...; lin...@li...; lin...@li...; lin...@vg...; ty...@do...; Souza, Joaquim (Brazil R&D-ECL) <joa...@hp...>; Edwards, Nigel <nig...@hp...> Subject: Re: [RFC 04/11] ima: add support to namespace securityfs file Hi John, On Thu, 2017-05-25 at 00:36 -0700, John Johansen wrote: > On 05/24/2017 01:12 PM, Mimi Zohar wrote: > > On Thu, 2017-05-11 at 10:59 -0300, Guilherme Magalhaes wrote: > >> Creating the namespace securityfs file under ima folder. When a > >> mount namespace id is written to the namespace file, a new folder > >> is created and with a policy file for that specified namespace. > >> Then, user defined policy for namespaces may be set by writing rules to this namespace policy file. > >> With this interface, there is no need to give visibility for the > >> securityfs inside mount namespaces or containers in userspace. > >> > >> Signed-off-by: Guilherme Magalhaes <gui...@hp...> > > > > The design needs to be flexible enough for different types of > > containers, not just for when the orchestration layer provides the > > policy. With this design, the container owner has no control over > > the policy. > > > > One option is that we bind mount the securityfs/policy, so that root > > in the container will be allowed to read/write the policy. At some > > point, we might connect a vTPM to the container so that the > > container owner would be able to get a quote. For now even without > > a vTPM, the same mechanism would allow root within the container to > > read the measurement list. > > > I haven't looked at this enough yet on IMAs end, but another possible > solution is using a symlink and a magic jump_link similar to what nsfs is doing. > > The patch series I posted out a couple of weeks ago > [RFC][Patch 0/3] securityfs: add the ability to support symlinks > > adds symlink support to securityfs, and then patch 3/3 cribs from nsfs > updating apparmorfs to use jump_link to "virtualize" the apparmor > policy directory. This avoids needing to have the bind mount. > > I'll break the patch out more and repost so its easier to see if this > approach might work for IMA. Sorry, I've been meaning to take a look at your patches, but just haven't gotten to it yet. This approach sounds really promising. thanks, Mimi |
From: Mimi Z. <zo...@li...> - 2017-05-29 17:33:52
|
Hi Guilherme, (Wow, you should did Cc a lot of people.) On Thu, 2017-05-25 at 19:04 +0000, Magalhaes, Guilherme (Brazil R&D- CL) wrote: > Mimi, > With the securityfs symlink we would address the case of setting > policy inside containers, but we still would need a way to set the > IMA policy per namespace outside containers. So, the current > proposed interface would address the latter case. > As an alternative to symlinks, taking this patch set as base, and > still considering setting policy inside containers (or inside > namespaces in general), it is possible to bind mount the securityfs > files into the containers, but it would be needed to prevent > read/write access to the namespaced IMA policy files for processes > not running on the same namespace. > > These mechanisms would not require a change in the proposed design. > Do you think these mechanisms are enough for the flexibility you > asked? I'm really sorry Guileherme, but as I previously explained, IMA has many aspects to it - the original file measurements (IMA-measurement), file hash/signature appraisal (IMA-appraisal), and file audit messages (IMA-audit) used for security analytics/forensics, not the file system auditing. To namespace IMA properly requires addressing some underlying problems - securityfs, root privilege required for writing security xattrs, per namespace IMA keyring - to name a few. I understand wanting to namespace the appraisal aspect first, but when you asked me if anyone else is working on namespacing IMA at the moment, I suggested starting with the IMA-audit aspect for a reason. By beginning with the IMA-audit aspect, we could ignore, at least for the time being, some of these underlying problems, but define the basic namespacing architecture. By jumping to namespacing the appraisal aspect, you've simply avoided addressing the IMA namespacing architecture issues, which need to be addressed. Here are some, not by any means all, of the issues with namespacing IMA. - IMA-measurements: The namespacing design will need to support different types of environments. For example, depending on the environment, namespaces can come and go frequently. In such an environment do we really want all the namespace measurements to be included in the system measurement list, or should each namespace have its own measurement list? Should the namespace measurement policy be the same as the system measurement policy? Should the namespace (container) owner be allowed to modify their own policy? If a file matches a rule in both the namespace and system policies, should the file measurement be in both the namespace and the system measurement lists? - IMA-appraisal Assuming the IMA appraisal policy requires file signatures, signatures are verified based on the keys on the IMA keyring. When discussing namespacing IMA-appraisal, we might want different sets of keys in different namespaces. This requires separate keyrings for each namespace. In some use cases, we might want to include the keys on the system IMA keyring, while in other use cases we might not. Even if real root initially installs the files with their file signatures, stored as extended attributes, how will software be updated, as writing file signatures requires root privilege?Capabilities has recently added support to permit root in the namespace to write security.capability. Similarly when namespacing IMA, root in the namespace needs the ability to write security.ima. - IMA-audit: The IMA-audit messages can augment other file system security information used in security analytics/forensics. This information should be on a per namespace basis, meaning that each time a new file is accessed/executed, there needs to be a separate audit message, even if a message already exists in another namespace. Maintaining and cleaning up this per namespace cache information, allows development of the IMA namespace architecture independently of other issues. My original suggestion stands. Start with namespacing IMA- audit. Afterwards resolve the securityfs issues needed for namespacing IMA-measurement, and subsequently resolve the keyring and xattr issues for namespacing IMA-appraisal. Although other subsystems have already addressed some of the issues listed here, the advice to start with IMA-audit is still valid. Small incremental change does not imply without an overall design, but an overall design which is broken up in such a way (to ease review) that builds upon the previous change. As both Apparmor and IMA use securityfs for policy, it would be nice if their method for loading namespace policies would be similar too. Mimi |
From: Dr. G. W. <gr...@en...> - 2017-05-31 09:50:23
|
On Mon, May 29, 2017 at 01:32:38PM -0400, Mimi Zohar wrote: > Hi Guilherme, > > (Wow, you should did Cc a lot of people.) Indeed. We have namespaced a significant amount of the IMA code so we will continue the broadcast, under the assumption that this is of general interest to the community. Comments below. > On Thu, 2017-05-25 at 19:04 +0000, Magalhaes, Guilherme (Brazil R&D- > CL) wrote: > > Mimi, > > With the securityfs symlink we would address the case of setting > > policy inside containers, but we still would need a way to set the > > IMA policy per namespace outside containers. So, the current > > proposed interface would address the latter case. > > As an alternative to symlinks, taking this patch set as base, and > > still considering setting policy inside containers (or inside > > namespaces in general), it is possible to bind mount the securityfs > > files into the containers, but it would be needed to prevent > > read/write access to the namespaced IMA policy files for processes > > not running on the same namespace. > > > > These mechanisms would not require a change in the proposed design. > > Do you think these mechanisms are enough for the flexibility you > > asked? > > I'm really sorry Guileherme, but as I previously explained, IMA has > many aspects to it - the original file measurements (IMA-measurement), > file hash/signature appraisal (IMA-appraisal), and file audit messages > (IMA-audit) used for security analytics/forensics, not the file system > auditing.????To namespace IMA properly requires addressing some > underlying problems - securityfs, root privilege required for writing > security xattrs, per namespace IMA keyring - to name a few. > > I understand wanting to namespace the appraisal aspect first, but when > you asked me if anyone else is working on namespacing IMA at the > moment, I suggested starting with the IMA-audit aspect for a > reason.????By beginning with the IMA-audit aspect, we could ignore, at > least for the time being, some of these underlying problems, but > define the basic namespacing architecture.????By jumping to namespacing > the appraisal aspect, you've simply avoided addressing the IMA > namespacing architecture issues, which need to be addressed. > > Here are some, not by any means all, of the issues with namespacing > IMA. > > > - IMA-measurements: > > The namespacing design will need to support different types of > environments. For example, depending on the environment, namespaces > can come and go frequently. In such an environment do we really want > all the namespace measurements to be included in the system > measurement list, or should each namespace have its own measurement > list?????Should the namespace measurement policy be the same as the > system measurement policy?????Should the namespace (container) owner be > allowed to modify their own policy?????If a file matches a rule in both > the namespace and system policies, should the file measurement be in > both the namespace and the system measurement lists? > > > - IMA-appraisal > > Assuming the IMA appraisal policy requires file signatures, signatures > are verified based on the keys on the IMA keyring.????When discussing > namespacing IMA-appraisal, we might want different sets of keys in > different namespaces. This requires separate keyrings for each > namespace. ??In some use cases, we might want to include the keys on > the system IMA keyring, while in other use cases we might not. > > Even if real root initially installs the files with their file > signatures, stored as extended attributes, how will software be > updated, as writing file signatures requires root > privilege?Capabilities has recently added support to permit root in > the namespace to write security.capability.????Similarly when > namespacing IMA, root in the namespace needs the ability to write > security.ima. > > > - IMA-audit: > > The IMA-audit messages can augment other file system security > information used in security analytics/forensics.????This information > should be on a per namespace basis, meaning that each time a new file > is accessed/executed, there needs to be a separate audit message, even > if a message already exists in another namespace.????Maintaining and > cleaning up this per namespace cache information, allows development > of the IMA namespace architecture independently of other issues. > > My original suggestion stands. ??Start with namespacing IMA- > audit.????Afterwards resolve the securityfs issues needed for > namespacing IMA-measurement, and subsequently resolve the keyring and > xattr issues for namespacing IMA-appraisal. Although other subsystems > have already addressed some of the issues listed here, the advice to > start with IMA-audit is still valid. > > Small incremental change does not imply without an overall design, but > an overall design which is broken up in such a way (to ease review) > that builds upon the previous change. > > As both Apparmor and IMA use securityfs for policy, it would be nice > if their method for loading namespace policies would be similar too. Our deterministic platform modeling code is built on top of the IMA infrastructure. Our modeling implementation has namespace support so we ended up OS virtualizing a significant amount of the IMA infrastructure to implement container specific behavior modeling. So the following is under the moniker of been there, done that to some extent. As an aside, we refer to containers running in a modeled environment as 'canisters' since they are containers with a label. Based on our experiences, the community will want each namespace implementation to be as independent as possible. A primary benefit of namespaced implementations of integrity/behavior modeling is to enable more tractable deployments since the system can be run from a very small TCB whose behavior is rooted to a hardware integrity guarantee. System upgrades are way more tractable in this type of architecture. Each canister in our model has its own behavioral trajectory path which is the equivalent of a superset of the IMA measurement list. After the canister behavior is sealed, each canister reports forensics violations as new entries on the trajectory path. So forensic evaluations are enabled at the canister level, which is extremely valuable. Implementing all of this was fairly straight forward on top of the current IMA infrastructure. Mimi's recommendation to start with the audit layer is well taken, that is a solid pathway for determining how to generically address the relevant infrastructure issues. >From an implementation perspective we don't use bind mounts or the like. The securityfs pseudo-files all sense which namespace they are operating in which resulted in a much cleaner implementation, at least in our experience. We implemented a model where the behavioral eigenvectors (events) are surfaced through a new pseudo-file created for each canister. This pseudo-file is rooted in the /sys/fs/iso-identity pseudo-directory and named by the namespace inode number which was allocated for the canister namespace when the behavioral domain (namespace) is created. This could have been done as well in the securityfs filesystem but we were too {cramped for time,lazy} to implement polling and sysfs files gives you that for free. This seems to be a pretty flexible model for orchestration. We propagate the behavioral eigenvectors into individual SGX enclaves which monitor the behavior of each canister. The model would support propagating events into a vTPM as well. >From an orchestration perspective we plumbed all of this, including SGX support, into the most recent release of runc and it has all worked surprisingly well with respect to running existing containers. All you need to do is specify in the container config.json that the container is to run in an independent behavioral namespace. We do lament that runc was written in GO rather then C... :-) There are some rough edges which need polishing though. A more expressive and namespace specific policy engine needs to be addressed. In addition to the namespace implementation we implemented superblock independent content digests which closes what we believe is a rather significant measurement gap. Addressing network inode measurement is an entirely new and different can of worms as well. We have also spent a fair amount of time reflecting on Viro's rants with respect to IMA and all of its inherent ugliness... :-) Our major concern is what does the community want to do with respect to defining the CLONE_* flag which is used to signal that the integrity/behavior namespace should be unshared. We currently build on top of 4.4 LTS and we stole the last bit which was available. What are the thoughts with respect to expanding this resource as it won't be the last OS virtualization which is undertaken? > Mimi Hopefully the above reflections and experiences are useful datapoints. Have a good week. Greg As always, Dr. G.W. Wettstein, Ph.D. Enjellic Systems Development, LLC. 4206 N. 19th Ave. Specializing in information infra-structure Fargo, ND 58102 development. PH: 701-281-1686 FAX: 701-281-3949 EMAIL: gr...@en... ------------------------------------------------------------------------------ "We can't solve today's problems by using the same thinking we used in creating them." -- Einstein |
From: Magalhaes, G. (B. R&D-CL) <gui...@hp...> - 2017-08-10 14:17:24
|
Hi Mimi, > The namespacing design will need to support different types of > environments. For example, depending on the environment, namespaces > can come and go frequently. In such an environment do we really want > all the namespace measurements to be included in the system > measurement list, or should each namespace have its own measurement > list? Should the namespace measurement policy be the same as the > system measurement policy? Should the namespace (container) owner be > allowed to modify their own policy? If a file matches a rule in both > the namespace and system policies, should the file measurement be in > both the namespace and the system measurement lists? Based on our previous discussions, the IMA policy should be set independently for different namespaces and each namespace will have a measurement list. Based on that and considering now the issues related to namespacing the IMA policy interface, this interface to set policy in the host and inside namespaces could be solved entirely by leveraging the existent policy file. IMA then would detect the current namespace and assume the rules are added to that namespace. The root user, even when mapped to a non-root user outside the namespace, is able to set the namespace policy just like it is done in the host policy. Once the namespace policy is set, the same policy rules should be read back from the policy file, with the same uids that were set in the rule and that make sense inside the current user namespace, not the mapped uid to the 'kernel' uid. This point is important because currently the uids in the IMA policy rules are mapped to kernel uids at parsing time. Another required change for this proposed interface to set the policy is to allow read/write access to the policy file to all users, since a root user in a given user namespace may not be a root user outside that user namespace. The kernel would still block access to the policy file based on the SYS_ADMIN capability and any changes to the policy would be effective just for the current namespace. The securityfs access inside user namespaces is possible once the user id mapping is done correctly, the root user, as seen inside the namespace will be able to mount the securityfs. Again, the read/write access to all users must be granted for the policy file. The same changes could be made to the measurement list interface in securityfs when each namespace has its own measurement list. A management tool in user space might be very helpful to set or read the policy or measurement list files in the current namespace or its children namespaces and to map the uid considering the current and target namespaces. This tool could also relate containers in user space to its IMA state for policy and appraise mode. Does this design solve properly the basic architectural issues only related to namespacing the interface to set the IMA policy? I have a patch with these changes ready to be posted. -- Guilherme > -----Original Message----- > From: Mimi Zohar [mailto:zo...@li...] > Sent: segunda-feira, 29 de maio de 2017 14:33 > To: Magalhaes, Guilherme (Brazil R&D-CL) <gui...@hp...>; > John Johansen <joh...@ca...>; dmi...@gm... > Cc: vi...@ze...; jam...@or...; se...@ha...; > lin...@vg...; lin...@vg...; linux-ima- > de...@li...; lin...@li...; linux- > sec...@vg...; ty...@do...; Souza, Joaquim (Brazil > R&D-ECL) <joa...@hp...>; Edwards, Nigel <nig...@hp...> > Subject: Re: [RFC 04/11] ima: add support to namespace securityfs file > > Hi Guilherme, > > (Wow, you should did Cc a lot of people.) > > On Thu, 2017-05-25 at 19:04 +0000, Magalhaes, Guilherme (Brazil R&D- > CL) wrote: > > Mimi, > > With the securityfs symlink we would address the case of setting > > policy inside containers, but we still would need a way to set the > > IMA policy per namespace outside containers. So, the current > > proposed interface would address the latter case. > > As an alternative to symlinks, taking this patch set as base, and > > still considering setting policy inside containers (or inside > > namespaces in general), it is possible to bind mount the securityfs > > files into the containers, but it would be needed to prevent > > read/write access to the namespaced IMA policy files for processes > > not running on the same namespace. > > > > These mechanisms would not require a change in the proposed design. > > Do you think these mechanisms are enough for the flexibility you > > asked? > > I'm really sorry Guileherme, but as I previously explained, IMA has > many aspects to it - the original file measurements (IMA-measurement), > file hash/signature appraisal (IMA-appraisal), and file audit messages > (IMA-audit) used for security analytics/forensics, not the file system > auditing. To namespace IMA properly requires addressing some > underlying problems - securityfs, root privilege required for writing > security xattrs, per namespace IMA keyring - to name a few. > > I understand wanting to namespace the appraisal aspect first, but when > you asked me if anyone else is working on namespacing IMA at the > moment, I suggested starting with the IMA-audit aspect for a > reason. By beginning with the IMA-audit aspect, we could ignore, at > least for the time being, some of these underlying problems, but > define the basic namespacing architecture. By jumping to namespacing > the appraisal aspect, you've simply avoided addressing the IMA > namespacing architecture issues, which need to be addressed. > > Here are some, not by any means all, of the issues with namespacing > IMA. > > > - IMA-measurements: > > The namespacing design will need to support different types of > environments. For example, depending on the environment, namespaces > can come and go frequently. In such an environment do we really want > all the namespace measurements to be included in the system > measurement list, or should each namespace have its own measurement > list? Should the namespace measurement policy be the same as the > system measurement policy? Should the namespace (container) owner be > allowed to modify their own policy? If a file matches a rule in both > the namespace and system policies, should the file measurement be in > both the namespace and the system measurement lists? > > > - IMA-appraisal > > Assuming the IMA appraisal policy requires file signatures, signatures > are verified based on the keys on the IMA keyring. When discussing > namespacing IMA-appraisal, we might want different sets of keys in > different namespaces. This requires separate keyrings for each > namespace. In some use cases, we might want to include the keys on > the system IMA keyring, while in other use cases we might not. > > Even if real root initially installs the files with their file > signatures, stored as extended attributes, how will software be > updated, as writing file signatures requires root > privilege?Capabilities has recently added support to permit root in > the namespace to write security.capability. Similarly when > namespacing IMA, root in the namespace needs the ability to write > security.ima. > > > - IMA-audit: > > The IMA-audit messages can augment other file system security > information used in security analytics/forensics. This information > should be on a per namespace basis, meaning that each time a new file > is accessed/executed, there needs to be a separate audit message, even > if a message already exists in another namespace. Maintaining and > cleaning up this per namespace cache information, allows development > of the IMA namespace architecture independently of other issues. > > My original suggestion stands. Start with namespacing IMA- > audit. Afterwards resolve the securityfs issues needed for > namespacing IMA-measurement, and subsequently resolve the keyring and > xattr issues for namespacing IMA-appraisal. Although other subsystems > have already addressed some of the issues listed here, the advice to > start with IMA-audit is still valid. > > Small incremental change does not imply without an overall design, but > an overall design which is broken up in such a way (to ease review) > that builds upon the previous change. > > As both Apparmor and IMA use securityfs for policy, it would be nice > if their method for loading namespace policies would be similar too. > > Mimi |
From: Magalhaes, G. (B. R&D-CL) <gui...@hp...> - 2017-05-29 20:35:34
|
Hi Mimi, Thanks for the feedback. > (Wow, you should did Cc a lot of people.) Reducing the list for now. >I'm really sorry Guileherme, but as I previously explained, IMA has >many aspects to it - the original file measurements (IMA-measurement), >file hash/signature appraisal (IMA-appraisal), and file audit messages >(IMA-audit) used for security analytics/forensics, not the file system >auditing. To namespace IMA properly requires addressing some >underlying problems - >securityfs, root privilege required for writing >security xattrs, per namespace IMA keyring - to name a few. We really considered all these aspects and they are either already addressed on this patch series or planed as future work items. That means we already have in plans incremental changes without compromising our basic design. We should have sent a summary of the design assumptions before hand, sorry. So, let me take this opportunity to present some of them. I believe we should have a brief phone conference to clarify the plan and make sure we are all on the same page. >- IMA-measurements: > >The namespacing design will need to support different types of environments. > For example, depending on the environment, namespaces can come and go >frequently. In such an environment do we really want all the namespace >measurements to be included in the system measurement list, or should each >namespace have its own measurement list? As a first step, we suggest a single measurement list for the entire system. The reason is what remote attestation requires. We would need a solution for the small number of TPM PCR limitation. The vTPM is not a simple solution and would need some effort on its own to be integrated with containers. Basically, we would need to adapt vTPM from VMs to the containers reality, maybe protecting the software stack with Intel SGX or other similar technology. So, we considered again this is a future work item and still possible to evolve from our first implementation. >Should the namespace measurement policy be the same as the system >measurement policy? Should the namespace (container) owner be allowed to >modify their own policy? If a file matches a rule in both the namespace and >system policies, should the file measurement be in both the namespace and the >system measurement lists? Most of this patch series is about policy per namespace. So, basically we propose one policy per namespace, so each namespace (container) could define what to measure and appraise. On our design, until a new namespace defines a custom policy, it is subject to the system policy. So, we designed that a namespace will always follow an IMA policy no matter what and then the current design is flexible enough to cover different ways to assure a namespace follows a policy. The detail about who/where a custom measurement policy is defined, we already discussed previously about the possible options (securityfs symlink, bind mount, etc). Again, this detail doesn't compromise the proposed design. >- IMA-appraisal > >Assuming the IMA appraisal policy requires file signatures, signatures are >verified based on the keys on the IMA keyring. When discussing namespacing >IMA-appraisal, we might want different sets of keys in different namespaces. >This requires separate keyrings for each namespace. In some use cases, we >might want to include the keys on the system IMA keyring, while in other use >cases we might not. Good point. This still is a future work item we could address in a later patch. Additionally, each namespace may require a different measurement template, and template per namespace is not yet supported on this patch set, but already in plan. >Even if real root initially installs the files with their file signatures, stored as >extended attributes, how will software be updated, as writing file signatures >requires root privilege?Capabilities has recently added support to permit root >in the namespace to write security.capability. Similarly when namespacing >IMA root in the namespace needs the ability to write security.ima. If I understood the problem correctly, we just need to replace, in the IMA hooks, the capable() calls with ns_capable(). This will make sure the capability checks will consider the correct namespace setting for the current user namespace. For instance, a CAP_SYS_ADMIN could be enabled for a fake 'root' user inside a given namespace (container), and that is enough for this fake root to set security.ima (through evmctl possibly). >- IMA-audit: >The IMA-audit messages can augment other file system security information >used in security analytics/forensics. This information should be on a per >namespace basis, meaning that each time a new file is accessed/executed, >there needs to be a separate audit message, even if a message already exists >in another namespace. Maintaining and cleaning up this per namespace cache >information, allows development of the IMA namespace architecture >independently of other issues. My understanding is that the Audit subsystem manages on its own, by using a daemon per user namespace, the separation of messages per namespace. Audit API (audit_log_start, audit_log_end, etc) clients, on its turn, must generate the proper record and then use the Audit API to queue that record. What we did was to add enough fields to the record to properly identify a pathname, considering multiple mount namespaces. I thought this is what was missing on the IMA perspective. Please clarify. >As both Apparmor and IMA use securityfs for policy, it would be nice if their > method for loading namespace policies would be similar too. Ok, we need to take a look then. -- Guilherme |
From: John J. <joh...@ca...> - 2017-05-29 21:50:00
|
On 05/29/2017 01:34 PM, Magalhaes, Guilherme (Brazil R&D-CL) wrote: > Hi Mimi, > Thanks for the feedback. > >> (Wow, you should did Cc a lot of people.) > Reducing the list for now. > << snip >> > > >> As both Apparmor and IMA use securityfs for policy, it would be nice if their >> method for loading namespace policies would be similar too. > Ok, we need to take a look then. > Some context to help you AppArmor policy namespaces are hierarchical in layout. Where a parent can create children namespaces, and those children can create grand children. Within securityfs this hierarchy is visible from apparmor/policy/ AppArmor tracks which policy namespace a task is in, and this determines what policy is available to the task, both for enforcement and visibility. Generally a task with rights to manage policy can load policy into its namespace and that namespaces children, but a task in a child namespace can not load policy into a parent namespace, ideally it shouldn't even be able to see it. The goal being to allow tasks within a namespace to manage it/ think about as though it is the root system namespace. Probably the simplest example for this is LXD on ubuntu where it creates a child namespace for its container, and the container OS can load apparmor policy without worrying about the rest of systems apparmor policy. To achieve this apparmor had to "virtualize" most of its files so that they change input/output based on the policy namespace of the task that opens them. This works well for files but does not work for the policy directory trees. To deal with the policy directory tree we create a special apparmorfs which is not available to mount from userspace and we do a kern mount on it as part of our setup. apparmor/policy becoming a magic symlink that is dynamically resolved to the correct subtree in apparmorfs for a given policy namespace. The links look like apparmor/policy -> apparmorfs:[1234] This gives us the virtualization without having to mess with the mount namespace (bind mounts), without requiring changes to userspace around mounts, or management assumptions (securityfs), and it matches with what nsfs is doing, so there is precedence for this approach. I will further add, this is just our solution for dealing with the fs interface. There are currently insufficient hooks in the LSM for dealing with interactions between policy namespaces and the rest of the system namespaces. Lukasz Pawelczyk posted a set of 3 patches a while ago now, focused on needs for providing some namespacing smack. user_ns: 3 new LSM hooks for user namespace operations I have messed with these and they are worth looking at. I plan to prose a new revision of the patch series hopefully soon. |
From: Mimi Z. <zo...@li...> - 2017-05-30 02:28:20
|
On Mon, 2017-05-29 at 20:34 +0000, Magalhaes, Guilherme (Brazil R&D- CL) wrote: <snip> > >- IMA-audit: > > >The IMA-audit messages can augment other file system security information > >used in security analytics/forensics. This information should be on a per > >namespace basis, meaning that each time a new file is accessed/executed, > >there needs to be a separate audit message, even if a message already exists > >in another namespace. Maintaining and cleaning up this per namespace cache > >information, allows development of the IMA namespace architecture > >independently of other issues. > My understanding is that the Audit subsystem manages on its own, by using a > daemon per user namespace, the separation of messages per namespace. Audit > API (audit_log_start, audit_log_end, etc) clients, on its turn, must generate the > proper record and then use the Audit API to queue that record. What we did was > to add enough fields to the record to properly identify a pathname, considering > multiple mount namespaces. I thought this is what was missing on the IMA > perspective. Please clarify. I'm referring to the IMA-audit messages that were introduced in e7c568e0fd0c "ima: audit log hashes". Appending the namespace identification information to the existing information is not enough. The IMA-audit message needs to be generated the first time in each namespace. This requires storing the per namespace information separately from the common information, for example the file hash. The main difficulties are in how the common information vs. the per namespace information are stored, referenced, and cleaned up. Cleanup will need to address when either the namespace itself or a dentry is deleted. All of the different IMA aspects require storing, on a per namespace basis, IMA namespace specific information (eg. audit status within the namespace, measurement status within the namespace, appraisal results based on different policies/keys). Once the architecture is defined, namespacing the other IMA aspects should be straight forward. Mimi |
From: Magalhaes, G. (B. R&D-CL) <gui...@hp...> - 2017-06-14 14:31:29
|
Hi Mimi, Thanks for the further explanations. We now started to work on a solution for namespacing the IMA-audit as you indicated as a first step and still aiming a bottom-up design from this point to other IMA areas. Talk to you soon. -- Guilherme -----Original Message----- From: Mimi Zohar [mailto:zo...@li...] Sent: segunda-feira, 29 de maio de 2017 23:27 To: Magalhaes, Guilherme (Brazil R&D-CL) <gui...@hp...>; John Johansen <joh...@ca...>; dmi...@gm... Cc: lin...@li...; lin...@li...; ty...@do...; Souza, Joaquim (Brazil R&D-ECL) <joa...@hp...>; Edwards, Nigel <nig...@hp...> Subject: Re: [RFC 04/11] ima: add support to namespace securityfs file On Mon, 2017-05-29 at 20:34 +0000, Magalhaes, Guilherme (Brazil R&D- CL) wrote: <snip> > >- IMA-audit: > > >The IMA-audit messages can augment other file system security > >information used in security analytics/forensics. This information > >should be on a per namespace basis, meaning that each time a new file > >is accessed/executed, there needs to be a separate audit message, > >even if a message already exists in another namespace. Maintaining > >and cleaning up this per namespace cache information, allows > >development of the IMA namespace architecture independently of other issues. > My understanding is that the Audit subsystem manages on its own, by > using a daemon per user namespace, the separation of messages per > namespace. Audit API (audit_log_start, audit_log_end, etc) clients, on > its turn, must generate the proper record and then use the Audit API > to queue that record. What we did was to add enough fields to the > record to properly identify a pathname, considering multiple mount > namespaces. I thought this is what was missing on the IMA perspective. Please clarify. I'm referring to the IMA-audit messages that were introduced in e7c568e0fd0c "ima: audit log hashes". Appending the namespace identification information to the existing information is not enough. The IMA-audit message needs to be generated the first time in each namespace. This requires storing the per namespace information separately from the common information, for example the file hash. The main difficulties are in how the common information vs. the per namespace information are stored, referenced, and cleaned up. Cleanup will need to address when either the namespace itself or a dentry is deleted. All of the different IMA aspects require storing, on a per namespace basis, IMA namespace specific information (eg. audit status within the namespace, measurement status within the namespace, appraisal results based on different policies/keys). Once the architecture is defined, namespacing the other IMA aspects should be straight forward. Mimi |
From: Mimi Z. <zo...@li...> - 2017-06-14 18:16:14
|
Hi Guilherme, On Wed, 2017-06-14 at 14:31 +0000, Magalhaes, Guilherme (Brazil R&D- CL) wrote: > Hi Mimi, > Thanks for the further explanations. > We now started to work on a solution for namespacing the IMA-audit > as you indicated as a first step and still aiming a bottom-up design > from this point to other IMA areas. > Talk to you soon. Mehmet Kayaalp is rebasing Yuqiong Sun's (a former summer intern at IBM Research) IMA original namespacing patches to a more recent kernel. As soon as he finishes, we'll push them out as reference. Mimi |