On 15-10-19 14:21:42, Mimi Zohar wrote:
> On Fri, 2015-10-16 at 22:31 +0300, Petko Manolov wrote:
> > When in development it is useful to read back the IMA policy. This patch
> > provides the functionality. However, this is a potential security hole so
> > it should not be used in production-grade kernels.
>
> Like the other IMA securityfs files, only root would be able to read it.
> Once we start allowing additional rules to be appended to the policy,
> being able to view the resulting policy is important. Is there a reason
> for limiting this option to development?
I have not considered allowing non-root users to read the policy - i was merely
cleaning up the Zbigniew's patch. I guess it might be useful to be able to
read
the policy when in development mode.
Petko
> > Signed-off-by: Petko Manolov <pet...@mip-labs.com>
> > ---
> > security/integrity/ima/Kconfig | 13 +++
> > security/integrity/ima/ima.h | 13 ++-
> > security/integrity/ima/ima_fs.c | 29 +++++-
> > security/integrity/ima/ima_policy.c | 190
> > ++++++++++++++++++++++++++++++++++++
> > 4 files changed, 239 insertions(+), 6 deletions(-)
> >
> > diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
> > index 235b3c2..040778f 100644
> > --- a/security/integrity/ima/Kconfig
> > +++ b/security/integrity/ima/Kconfig
> > @@ -121,6 +121,19 @@ config IMA_WRITE_POLICY
> >
> > If unsure, say N.
> >
> > +config IMA_READ_POLICY
> > + bool "Enable reading back the current IMA policy"
> > + depends on IMA
> > + default n
> > + help
> > + When developing and debugging an IMA enabled system it is often
> > + useful to be able to read the IMA policy. This patch allows
> > + for doing so. However, being able to read the IMA policy is
> > + considered insecure and is strongly discouraged for
> > + production-grade kernels.
> > +
> > + If unsure, say N.
> > +
> > config IMA_APPRAISE
> > bool "Appraise integrity measurements"
> > depends on IMA
> > diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> > index e2a60c3..ebc3554 100644
> > --- a/security/integrity/ima/ima.h
> > +++ b/security/integrity/ima/ima.h
> > @@ -166,6 +166,10 @@ 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_policy_start(struct seq_file *m, loff_t *pos);
> > +void *ima_policy_next(struct seq_file *m, void *v, loff_t *pos);
> > +void ima_policy_stop(struct seq_file *m, void *v);
> > +int ima_policy_show(struct seq_file *m, void *v);
> >
> > /* Appraise integrity measurements */
> > #define IMA_APPRAISE_ENFORCE 0x01
> > @@ -263,4 +267,11 @@ static inline int ima_init_keyring(const unsigned int
> > id)
> > return 0;
> > }
> > #endif /* CONFIG_IMA_TRUSTED_KEYRING */
> > -#endif
> > +
> > +#ifdef CONFIG_IMA_READ_POLICY
> > +#define POLICY_FILE_FLAGS (S_IWUSR | S_IRUSR)
> > +#else
> > +#define POLICY_FILE_FLAGS S_IWUSR
> > +#endif /* CONFIG_IMA_WRITE_POLICY */
> > +
> > +#endif /* __LINUX_IMA_H */
> > diff --git a/security/integrity/ima/ima_fs.c
> > b/security/integrity/ima/ima_fs.c
> > index a3cf5c0..85bd129 100644
> > --- a/security/integrity/ima/ima_fs.c
> > +++ b/security/integrity/ima/ima_fs.c
> > @@ -311,14 +311,29 @@ enum ima_fs_flags {
> >
> > static unsigned long ima_fs_flags;
> >
> > +#ifdef CONFIG_IMA_READ_POLICY
> > +static const struct seq_operations ima_policy_seqops = {
> > + .start = ima_policy_start,
> > + .next = ima_policy_next,
> > + .stop = ima_policy_stop,
> > + .show = ima_policy_show,
> > +};
> > +#endif
> > +
> > /*
> > * ima_open_policy: sequentialize access to the policy file
> > */
> > static int ima_open_policy(struct inode *inode, struct file *filp)
> > {
> > - /* No point in being allowed to open it if you aren't going to write */
> > - if (!(filp->f_flags & O_WRONLY))
> > + if (!(filp->f_flags & O_WRONLY)) {
> > +#ifndef CONFIG_IMA_READ_POLICY
> > return -EACCES;
> > +#else
> > + if (!capable(CAP_SYS_ADMIN))
> > + return -EPERM;
> > + return seq_open(filp, &ima_policy_seqops);
> > +#endif
> > + }
> > if (test_and_set_bit(IMA_FS_BUSY, &ima_fs_flags))
> > return -EBUSY;
> > return 0;
> > @@ -334,6 +349,10 @@ 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";
> > + int policy_write = (file->f_flags & O_WRONLY);
> > +
> > + if (!policy_write)
> > + return 0;
> >
> > pr_info("IMA: policy update %s\n", cause);
> > integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL, NULL,
> > @@ -342,9 +361,9 @@ static int ima_release_policy(struct inode *inode,
> > struct file *file)
> > if (!valid_policy) {
> > ima_delete_rules();
> > valid_policy = 1;
> > - clear_bit(IMA_FS_BUSY, &ima_fs_flags);
> > return 0;
> > }
> > +
> > ima_update_policy();
> > #ifndef CONFIG_IMA_WRITE_POLICY
> > securityfs_remove(ima_policy);
> > @@ -358,6 +377,7 @@ static int ima_release_policy(struct inode *inode,
> > struct file *file)
> > static const struct file_operations ima_measure_policy_ops = {
> > .open = ima_open_policy,
> > .write = ima_write_policy,
> > + .read = seq_read,
> > .release = ima_release_policy,
> > .llseek = generic_file_llseek,
> > };
> > @@ -395,8 +415,7 @@ int __init ima_fs_init(void)
> > if (IS_ERR(violations))
> > goto out;
> >
> > - ima_policy = securityfs_create_file("policy",
> > - S_IWUSR,
> > + ima_policy = securityfs_create_file("policy", POLICY_FILE_FLAGS,
> > ima_dir, NULL,
> > &ima_measure_policy_ops);
> > if (IS_ERR(ima_policy))
> > diff --git a/security/integrity/ima/ima_policy.c
> > b/security/integrity/ima/ima_policy.c
> > index 7ace4e4..29961d7 100644
> > --- a/security/integrity/ima/ima_policy.c
> > +++ b/security/integrity/ima/ima_policy.c
> > @@ -18,6 +18,7 @@
> > #include <linux/slab.h>
> > #include <linux/rculist.h>
> > #include <linux/genhd.h>
> > +#include <linux/seq_file.h>
> >
> > #include "ima.h"
> >
> > @@ -487,6 +488,35 @@ static match_table_t policy_tokens = {
> > {Opt_err, NULL}
> > };
> >
> > +enum {
> > + mask_err = -1,
> > + mask_exec = 1, mask_write, mask_read, mask_append
> > +};
> > +
> > +static match_table_t mask_tokens = {
> > + {mask_exec, "MAY_EXEC"},
> > + {mask_write, "MAY_WRITE"},
> > + {mask_read, "MAY_READ"},
> > + {mask_append, "MAY_APPEND"},
> > + {mask_err, NULL}
> > +};
> > +
> > +enum {
> > + func_err = -1,
> > + func_file = 1, func_mmap, func_bprm,
> > + func_module, func_firmware, func_post
> > +};
> > +
> > +static match_table_t func_tokens = {
> > + {func_file, "FILE_CHECK"},
> > + {func_mmap, "MMAP_CHECK"},
> > + {func_bprm, "BPRM_CHECK"},
> > + {func_module, "MODULE_CHECK"},
> > + {func_firmware, "FIRMWARE_CHECK"},
> > + {func_post, "POST_SETATTR"},
> > + {func_err, NULL}
> > +};
> > +
> > static int ima_lsm_rule_init(struct ima_rule_entry *entry,
> > substring_t *args, int lsm_rule, int audit_type)
> > {
> > @@ -829,3 +859,163 @@ void ima_delete_rules(void)
> > kfree(entry);
> > }
> > }
> > +
> > +void *ima_policy_start(struct seq_file *m, loff_t *pos)
> > +{
> > + loff_t l = *pos;
> > + struct ima_rule_entry *entry;
> > +
> > + rcu_read_lock();
> > + list_for_each_entry_rcu(entry, ima_rules, list) {
> > + if (!l--) {
> > + rcu_read_unlock();
> > + return entry;
> > + }
> > + }
> > + rcu_read_unlock();
> > + return NULL;
> > +}
> > +
> > +void *ima_policy_next(struct seq_file *m, void *v, loff_t *pos)
> > +{
> > + struct ima_rule_entry *entry = v;
> > +
> > + 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;
> > +}
> > +
> > +void ima_policy_stop(struct seq_file *m, void *v)
> > +{
> > +}
> > +
> > +#define pt(token) policy_tokens[token-1].pattern
> > +#define mt(token) mask_tokens[token-1].pattern
> > +#define ft(token) func_tokens[token-1].pattern
> > +
> > +int ima_policy_show(struct seq_file *m, void *v)
> > +{
> > + struct ima_rule_entry *entry = v;
> > + int i = 0;
> > +
> > + rcu_read_lock();
> > +
> > + if (entry->action & MEASURE)
> > + seq_puts(m, pt(Opt_measure));
> > + if (entry->action & DONT_MEASURE)
> > + seq_puts(m, pt(Opt_dont_measure));
> > + if (entry->action & APPRAISE)
> > + seq_puts(m, pt(Opt_appraise));
> > + if (entry->action & DONT_APPRAISE)
> > + seq_puts(m, pt(Opt_dont_appraise));
> > + if (entry->action & AUDIT)
> > + seq_puts(m, pt(Opt_audit));
> > +
> > + seq_puts(m, " ");
> > +
> > + if (entry->flags & IMA_FUNC) {
> > + switch (entry->func) {
> > + case FILE_CHECK:
> > + seq_printf(m, pt(Opt_func), ft(func_file));
> > + break;
> > + case MMAP_CHECK:
> > + seq_printf(m, pt(Opt_func), ft(func_mmap));
> > + break;
> > + case BPRM_CHECK:
> > + seq_printf(m, pt(Opt_func), ft(func_bprm));
> > + break;
> > + case MODULE_CHECK:
> > + seq_printf(m, pt(Opt_func), ft(func_module));
> > + break;
> > + case FIRMWARE_CHECK:
> > + seq_printf(m, pt(Opt_func), ft(func_firmware));
> > + break;
> > + case POST_SETATTR:
> > + seq_printf(m, pt(Opt_func), ft(func_post));
> > + break;
> > + default:
> > + seq_printf(m, "func=%d", entry->func);
> > + break;
> > + }
> > + seq_puts(m, " ");
> > + }
> > +
> > + if (entry->flags & IMA_MASK) {
> > + if (entry->mask & MAY_EXEC)
> > + seq_printf(m, pt(Opt_mask), mt(mask_exec));
> > + if (entry->mask & MAY_WRITE)
> > + seq_printf(m, pt(Opt_mask), mt(mask_write));
> > + if (entry->mask & MAY_READ)
> > + seq_printf(m, pt(Opt_mask), mt(mask_read));
> > + if (entry->mask & MAY_APPEND)
> > + seq_printf(m, pt(Opt_mask), mt(mask_append));
> > + seq_puts(m, " ");
> > + }
> > +
> > + if (entry->flags & IMA_FSMAGIC) {
> > + seq_printf(m, "fsmagic=0x%lx", entry->fsmagic);
> > + seq_puts(m, " ");
> > + }
> > +
> > + if (entry->flags & IMA_FSUUID) {
> > + seq_puts(m, "fsuuid=");
> > + for (i = 0; i < ARRAY_SIZE(entry->fsuuid); ++i) {
> > + switch (i) {
> > + case 4:
> > + case 6:
> > + case 8:
> > + case 10:
> > + seq_puts(m, "-");
> > + }
> > + seq_printf(m, "%x", entry->fsuuid[i]);
> > + }
> > + seq_puts(m, " ");
> > + }
> > +
> > + if (entry->flags & IMA_UID) {
> > + seq_printf(m, "uid=%d", __kuid_val(entry->uid));
> > + seq_puts(m, " ");
> > + }
> > +
> > + if (entry->flags & IMA_FOWNER) {
> > + seq_printf(m, "fowner=%d", __kuid_val(entry->fowner));
> > + seq_puts(m, " ");
> > + }
> > +
> > + for (i = 0; i < MAX_LSM_RULES; i++) {
> > + if (entry->lsm[i].rule) {
> > + switch (i) {
> > + case LSM_OBJ_USER:
> > + seq_printf(m, pt(Opt_obj_user),
> > + (char *)entry->lsm[i].args_p);
> > + break;
> > + case LSM_OBJ_ROLE:
> > + seq_printf(m, pt(Opt_obj_role),
> > + (char *)entry->lsm[i].args_p);
> > + break;
> > + case LSM_OBJ_TYPE:
> > + seq_printf(m, pt(Opt_obj_type),
> > + (char *)entry->lsm[i].args_p);
> > + break;
> > + case LSM_SUBJ_USER:
> > + seq_printf(m, pt(Opt_subj_user),
> > + (char *)entry->lsm[i].args_p);
> > + break;
> > + case LSM_SUBJ_ROLE:
> > + seq_printf(m, pt(Opt_subj_role),
> > + (char *)entry->lsm[i].args_p);
> > + break;
> > + case LSM_SUBJ_TYPE:
> > + seq_printf(m, pt(Opt_subj_type),
> > + (char *)entry->lsm[i].args_p);
> > + break;
> > + }
> > + }
> > + }
> > + rcu_read_unlock();
> > + seq_puts(m, "\n");
> > + return 0;
> > +}
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe
> linux-security-module" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe
linux-security-module" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
