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?

(Still reviewing the code ...)

Mimi

> 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

Reply via email to