| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Mon, 02 Feb 2009 20:29:57 -0500
From: Mimi Zohar <zohar@...ux.vnet.ibm.com>
To: "Serge E. Hallyn" <serge@...lyn.com>
Cc: linux-kernel@...r.kernel.org,
Andrew Morton <akpm@...ux-foundation.org>,
James Morris <jmorris@...ei.org>,
Christoph Hellwig <hch@...radead.org>,
Dave Hansen <dave@...ux.vnet.ibm.com>,
"<David Safford" <safford@...son.ibm.com>,
Serge Hallyn <serue@...ibm.com>, Mimi Zohar <zohar@...ibm.com>
Subject: Re: [PATCH 4/6] integrity: IMA policy
On Mon, 2009-02-02 at 17:40 -0600, Serge E. Hallyn wrote:
> Quoting Mimi Zohar (zohar@...ux.vnet.ibm.com):
> > Support for a user loadable policy through securityfs
> > with support for LSM specific policy data.
> >
> > Based on comments made by: Matt Helsley, Serge Hallyn
> > - replaced policy parsing code with version using strsep and match_token
> > - only replace default policy with a valid policy
> >
> > Signed-off-by: Mimi Zohar <zohar@...ibm.com>
>
> Apart from comments below,
>
> Acked-by: Serge Hallyn <serue@...ibm.com>
>
> > ---
> > diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
> > new file mode 100644
> > index 0000000..6434f0d
> > --- /dev/null
> > +++ b/Documentation/ABI/testing/ima_policy
> > @@ -0,0 +1,61 @@
> > +What: security/ima/policy
> > +Date: May 2008
> > +Contact: Mimi Zohar <zohar@...ibm.com>
> > +Description:
> > + The Trusted Computing Group(TCG) runtime Integrity
> > + Measurement Architecture(IMA) maintains a list of hash
> > + values of executables and other sensitive system files
> > + loaded into the run-time of this system. At runtime,
> > + the policy can be constrained based on LSM specific data.
> > + Policies are loaded into the securityfs file ima/policy
> > + by opening the file, writing the rules one at a time and
> > + then closing the file. The new policy takes effect after
> > + the file ima/policy is closed.
> > +
> > + rule format: action [condition ...]
> > +
> > + action: measure | dont_measure
> > + condition:= base | lsm
> > + base: [[func=] [mask=] [fsmagic=] [uid=]]
> > + lsm: [[subj_user=] [subj_role=] [subj_type=]
> > + [obj_user=] [obj_role=] [obj_type=]]
> > +
> > + base: func:= [BPRM_CHECK][FILE_MMAP][INODE_PERMISSION]
> > + mask:= [MAY_READ] [MAY_WRITE] [MAY_APPEND] [MAY_EXEC]
> > + fsmagic:= hex value
> > + uid:= decimal value
> > + lsm: are LSM specific
> > +
> > + default policy:
> > + # PROC_SUPER_MAGIC
> > + dont_measure fsmagic=0x9fa0
> > + # SYSFS_MAGIC
> > + dont_measure fsmagic=0x62656572
> > + # DEBUGFS_MAGIC
> > + dont_measure fsmagic=0x64626720
> > + # TMPFS_MAGIC
> > + dont_measure fsmagic=0x01021994
> > + # SECURITYFS_MAGIC
> > + dont_measure fsmagic=0x73636673
> > +
> > + measure func=BPRM_CHECK
> > + measure func=FILE_MMAP mask=MAY_EXEC
> > + measure func=INODE_PERM mask=MAY_READ uid=0
> > +
> > + The default policy measures all executables in bprm_check,
> > + all files mmapped executable in file_mmap, and all files
> > + open for read by root in inode_permission.
> > +
> > + Examples of LSM specific definitions:
> > +
> > + SELinux:
> > + # SELINUX_MAGIC
> > + dont_measure fsmagic=0xF97CFF8C
> > +
> > + dont_measure obj_type=var_log_t
> > + dont_measure obj_type=auditd_log_t
> > + measure subj_user=system_u func=INODE_PERM mask=MAY_READ
> > + measure subj_role=system_r func=INODE_PERM mask=MAY_READ
> > +
> > + Smack:
> > + measure subj_user=_ func=INODE_PERM mask=MAY_READ
> > diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
> > index 2a761c8..3d2b6ee 100644
> > --- a/security/integrity/ima/Kconfig
> > +++ b/security/integrity/ima/Kconfig
> > @@ -47,3 +47,9 @@ config IMA_AUDIT
> > auditing messages can be enabled with 'ima_audit=1' on
> > the kernel command line.
> >
> > +config IMA_LSM_RULES
> > + bool
> > + depends on IMA && (SECURITY_SELINUX || SECURITY_SMACK)
> > + default y
> > + help
> > + Disabling this option will disregard LSM based policy rules
> > diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> > index 236b74e..5b72cdb 100644
> > --- a/security/integrity/ima/ima.h
> > +++ b/security/integrity/ima/ima.h
> > @@ -138,4 +138,28 @@ enum ima_hooks { PATH_CHECK = 1, FILE_MMAP, BPRM_CHECK };
> > int ima_match_policy(struct inode *inode, enum ima_hooks func, int mask);
> > void ima_init_policy(void);
> > void ima_update_policy(void);
> > +int ima_parse_add_rule(char *);
> > +void ima_delete_rules(void);
> > +
> > +/* LSM based policy rules require audit */
> > +#ifdef CONFIG_IMA_LSM_RULES
> > +
> > +#define security_filter_rule_init security_audit_rule_init
> > +#define security_filter_rule_match security_audit_rule_match
> > +
> > +#else
> > +
> > +static inline int security_filter_rule_init(u32 field, u32 op, char *rulestr,
> > + void **lsmrule)
> > +{
> > + return -EINVAL;
> > +}
> > +
> > +static inline int security_filter_rule_match(u32 secid, u32 field, u32 op,
> > + void *lsmrule,
> > + struct audit_context *actx)
> > +{
> > + return -EINVAL;
> > +}
> > +#endif /* CONFIG_IMA_LSM_RULES */
> > #endif
> > diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
> > index 5044e4c..752a344 100644
> > --- a/security/integrity/ima/ima_fs.c
> > +++ b/security/integrity/ima/ima_fs.c
> > @@ -19,9 +19,11 @@
> > #include <linux/seq_file.h>
> > #include <linux/rculist.h>
> > #include <linux/rcupdate.h>
> > +#include <linux/parser.h>
> >
> > #include "ima.h"
> >
> > +static int valid_policy = 1;
> > #define TMPBUFLEN 12
> > static ssize_t ima_show_htable_value(char __user *buf, size_t count,
> > loff_t *ppos, atomic_long_t *val)
> > @@ -237,11 +239,66 @@ static struct file_operations ima_ascii_measurements_ops = {
> > .release = seq_release,
> > };
> >
> > +static ssize_t ima_write_policy(struct file *file, const char __user *buf,
> > + size_t datalen, loff_t *ppos)
> > +{
> > + char *data;
> > + int rc;
> > +
> > + if (datalen >= PAGE_SIZE)
> > + return -ENOMEM;
> > + if (*ppos != 0) {
> > + /* No partial writes. */
> > + return -EINVAL;
> > + }
> > + data = kmalloc(datalen + 1, GFP_KERNEL);
> > + if (!data)
> > + return -ENOMEM;
> > +
> > + if (copy_from_user(data, buf, datalen)) {
> > + kfree(data);
> > + return -EFAULT;
> > + }
> > + *(data + datalen) = '\0';
> > + rc = ima_parse_add_rule(data);
> > + if (rc < 0) {
> > + datalen = -EINVAL;
> > + valid_policy = 0;
> > + }
> > +
> > + kfree(data);
> > + return datalen;
> > +}
> > +
> > 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;
> > +
> > +/*
> > + * ima_release_policy - start using the new measure policy rules.
> > + *
> > + * Initially, ima_measure points to the default policy rules, now
> > + * point to the new policy rules, and remove the securityfs policy file.
> > + */
> > +static int ima_release_policy(struct inode *inode, struct file *file)
> > +{
> > + if (!valid_policy) {
> > + ima_delete_rules();
> > + return 0;
> > + }
> > + ima_update_policy();
> > + securityfs_remove(ima_policy);
> > + ima_policy = NULL;
> > + return 0;
> > +}
> > +
> > +static struct file_operations ima_measure_policy_ops = {
> > + .write = ima_write_policy,
> > + .release = ima_release_policy
> > +};
> >
> > int ima_fs_init(void)
> > {
> > @@ -276,13 +333,20 @@ int ima_fs_init(void)
> > if (!violations || IS_ERR(violations))
> > goto out;
> >
> > - return 0;
> > + ima_policy = securityfs_create_file("policy",
> > + S_IRUSR | S_IRGRP | S_IWUSR,
> > + ima_dir, NULL,
> > + &ima_measure_policy_ops);
> > + if (!ima_policy || IS_ERR(ima_policy))
> > + goto out;
>
> Of course, James' same comment applies here :)
Yes, got that one too.
> > +/**
> > + * ima_parse_add_rule - add a rule to measure_policy_rules
> > + * @rule - ima measurement policy rule
> > + *
> > + * Uses a mutex to protect the policy list from multiple concurrent writers.
> > + * Returns 0 on success, an error code on failure.
> > + */
> > +int ima_parse_add_rule(char *rule)
> > +{
> > + const char *op = "add_rule";
> > + struct ima_measure_rule_entry *entry;
> > + int result = 0;
> > + int audit_info = 0;
> > +
> > + /* Prevent installed policy from changing */
> > + if (ima_measure != &measure_default_rules) {
> > + integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL,
> > + NULL, op, "already exists",
> > + -EACCES, audit_info);
> > + return -EACCES;
> > + }
> > +
> > + entry = kzalloc(sizeof(*entry), GFP_KERNEL);
> > + if (!entry) {
> > + integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL,
> > + NULL, op, "-ENOMEM", -ENOMEM, audit_info);
> > + return -ENOMEM;
> > + }
> > +
> > + INIT_LIST_HEAD(&entry->list);
> > +
> > + result = ima_parse_rule(rule, entry);
> > + if (!result) {
> > + mutex_lock(&ima_measure_mutex);
> > + list_add_tail(&entry->list, &measure_policy_rules);
> > + mutex_unlock(&ima_measure_mutex);
> > + }
>
> Should you kfree(entry) if ima_parse_rule() failed?
yes, thanks.
Mimi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists