* [PATCH 0/8] integrity: fixes and new features
@ 2014-02-28 14:59 Dmitry Kasatkin
2014-02-28 14:59 ` [PATCH 1/8] ima: fix erronous removal of security.ima xattr Dmitry Kasatkin
` (7 more replies)
0 siblings, 8 replies; 31+ messages in thread
From: Dmitry Kasatkin @ 2014-02-28 14:59 UTC (permalink / raw)
To: linux-security-module, zohar
Cc: jmorris, linux-kernel, casey.schaufler, dmitry.kasatkin,
Dmitry Kasatkin
Hi,
This patchset contains bug fixes, cleanups and new features
for integrity subsytem.
- Dmitry
Dmitry Kasatkin (8):
ima: fix erronous removal of security.ima xattr
integrity: fix checkpatch errors
ima: return d_name.name if d_path fails
evm: EVM does not use MD5
ima: skip memory allocation for empty files
evm: enable key retention service automatically
evm: introduce EVM hmac attribute list
evm: introduce EVM hmac xattr list
security/integrity/evm/Kconfig | 35 +++++++++-----
security/integrity/evm/evm.h | 41 +++++++++-------
security/integrity/evm/evm_crypto.c | 14 +++---
security/integrity/evm/evm_main.c | 90 +++++++++++++++++++++--------------
security/integrity/iint.c | 2 +-
security/integrity/ima/ima_api.c | 10 ++--
security/integrity/ima/ima_crypto.c | 22 +++++----
security/integrity/ima/ima_fs.c | 6 +--
security/integrity/ima/ima_main.c | 11 ++---
security/integrity/ima/ima_policy.c | 75 +++++++++++++++--------------
security/integrity/ima/ima_queue.c | 4 +-
security/integrity/ima/ima_template.c | 14 +++---
security/integrity/integrity_audit.c | 4 +-
13 files changed, 183 insertions(+), 145 deletions(-)
--
1.8.3.2
^ permalink raw reply [flat|nested] 31+ messages in thread* [PATCH 1/8] ima: fix erronous removal of security.ima xattr 2014-02-28 14:59 [PATCH 0/8] integrity: fixes and new features Dmitry Kasatkin @ 2014-02-28 14:59 ` Dmitry Kasatkin 2014-03-03 13:43 ` Mimi Zohar 2014-02-28 14:59 ` [PATCH 2/8] integrity: fix checkpatch errors Dmitry Kasatkin ` (6 subsequent siblings) 7 siblings, 1 reply; 31+ messages in thread From: Dmitry Kasatkin @ 2014-02-28 14:59 UTC (permalink / raw) To: linux-security-module, zohar Cc: jmorris, linux-kernel, casey.schaufler, dmitry.kasatkin, Dmitry Kasatkin ima_inode_post_setattr() calls ima_must_appraise() to check if file needs to be appraised. If it is not then it removes security.ima xattr. With original policy matching code it might happen that even file needs to be appraised with FILE_CHECK hook, it might not be for POST_SETATTR hook. 'security.ima' might be erronously removed. This patch treats POST_SETATTR as special wildcard function and will cause ima_must_appraise() to be true if any of the hooks rules matches. security.ima will not be removed if any of the hooks would require appraisal. Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com> --- security/integrity/ima/ima_policy.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index d7c97a4..947cdbe 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -167,9 +167,11 @@ static bool ima_match_rules(struct ima_rule_entry *rule, const struct cred *cred = current_cred(); int i; - if ((rule->flags & IMA_FUNC) && rule->func != func) + if ((rule->flags & IMA_FUNC) && + (rule->func != func && func != POST_SETATTR)) return false; - if ((rule->flags & IMA_MASK) && rule->mask != mask) + if ((rule->flags & IMA_MASK) && + (rule->mask != mask && func != POST_SETATTR)) return false; if ((rule->flags & IMA_FSMAGIC) && rule->fsmagic != inode->i_sb->s_magic) -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH 1/8] ima: fix erronous removal of security.ima xattr 2014-02-28 14:59 ` [PATCH 1/8] ima: fix erronous removal of security.ima xattr Dmitry Kasatkin @ 2014-03-03 13:43 ` Mimi Zohar 0 siblings, 0 replies; 31+ messages in thread From: Mimi Zohar @ 2014-03-03 13:43 UTC (permalink / raw) To: Dmitry Kasatkin Cc: linux-security-module, jmorris, linux-kernel, casey.schaufler, dmitry.kasatkin On Fri, 2014-02-28 at 16:59 +0200, Dmitry Kasatkin wrote: > ima_inode_post_setattr() calls ima_must_appraise() to check if > file needs to be appraised. If it is not then it removes security.ima > xattr. With original policy matching code it might happen that even > file needs to be appraised with FILE_CHECK hook, it might not be > for POST_SETATTR hook. 'security.ima' might be erronously removed. > > This patch treats POST_SETATTR as special wildcard function and will > cause ima_must_appraise() to be true if any of the hooks rules matches. > security.ima will not be removed if any of the hooks would require > appraisal. > > Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com> Thanks! Mimi > --- > security/integrity/ima/ima_policy.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c > index d7c97a4..947cdbe 100644 > --- a/security/integrity/ima/ima_policy.c > +++ b/security/integrity/ima/ima_policy.c > @@ -167,9 +167,11 @@ static bool ima_match_rules(struct ima_rule_entry *rule, > const struct cred *cred = current_cred(); > int i; > > - if ((rule->flags & IMA_FUNC) && rule->func != func) > + if ((rule->flags & IMA_FUNC) && > + (rule->func != func && func != POST_SETATTR)) > return false; > - if ((rule->flags & IMA_MASK) && rule->mask != mask) > + if ((rule->flags & IMA_MASK) && > + (rule->mask != mask && func != POST_SETATTR)) > return false; > if ((rule->flags & IMA_FSMAGIC) > && rule->fsmagic != inode->i_sb->s_magic) ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 2/8] integrity: fix checkpatch errors 2014-02-28 14:59 [PATCH 0/8] integrity: fixes and new features Dmitry Kasatkin 2014-02-28 14:59 ` [PATCH 1/8] ima: fix erronous removal of security.ima xattr Dmitry Kasatkin @ 2014-02-28 14:59 ` Dmitry Kasatkin 2014-02-28 15:16 ` Dmitry Kasatkin 2014-03-03 13:41 ` Mimi Zohar 2014-02-28 14:59 ` [PATCH 3/8] ima: return d_name.name if d_path fails Dmitry Kasatkin ` (5 subsequent siblings) 7 siblings, 2 replies; 31+ messages in thread From: Dmitry Kasatkin @ 2014-02-28 14:59 UTC (permalink / raw) To: linux-security-module, zohar Cc: jmorris, linux-kernel, casey.schaufler, dmitry.kasatkin, Dmitry Kasatkin Unfixed checkpatch errors make it difficult to see new errors.. This patch fix them. Some lines with over 80 chars remained unchanged to improve code readability. Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com> --- security/integrity/evm/evm.h | 28 +++++++------- security/integrity/evm/evm_crypto.c | 4 +- security/integrity/iint.c | 2 +- security/integrity/ima/ima_api.c | 8 ++-- security/integrity/ima/ima_crypto.c | 2 +- security/integrity/ima/ima_fs.c | 6 +-- security/integrity/ima/ima_main.c | 4 +- security/integrity/ima/ima_policy.c | 69 +++++++++++++++++------------------ security/integrity/ima/ima_queue.c | 4 +- security/integrity/ima/ima_template.c | 14 +++---- security/integrity/integrity_audit.c | 4 +- 11 files changed, 72 insertions(+), 73 deletions(-) diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h index 30bd1ec..37c88dd 100644 --- a/security/integrity/evm/evm.h +++ b/security/integrity/evm/evm.h @@ -32,19 +32,19 @@ extern struct crypto_shash *hash_tfm; /* List of EVM protected security xattrs */ extern char *evm_config_xattrnames[]; -extern int evm_init_key(void); -extern int evm_update_evmxattr(struct dentry *dentry, - const char *req_xattr_name, - const char *req_xattr_value, - size_t req_xattr_value_len); -extern int evm_calc_hmac(struct dentry *dentry, const char *req_xattr_name, - const char *req_xattr_value, - size_t req_xattr_value_len, char *digest); -extern int evm_calc_hash(struct dentry *dentry, const char *req_xattr_name, - const char *req_xattr_value, - size_t req_xattr_value_len, char *digest); -extern int evm_init_hmac(struct inode *inode, const struct xattr *xattr, - char *hmac_val); -extern int evm_init_secfs(void); +int evm_init_key(void); +int evm_update_evmxattr(struct dentry *dentry, + const char *req_xattr_name, + const char *req_xattr_value, + size_t req_xattr_value_len); +int evm_calc_hmac(struct dentry *dentry, const char *req_xattr_name, + const char *req_xattr_value, + size_t req_xattr_value_len, char *digest); +int evm_calc_hash(struct dentry *dentry, const char *req_xattr_name, + const char *req_xattr_value, + size_t req_xattr_value_len, char *digest); +int evm_init_hmac(struct inode *inode, const struct xattr *xattr, + char *hmac_val); +int evm_init_secfs(void); #endif diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c index 9bd329f..babd862 100644 --- a/security/integrity/evm/evm_crypto.c +++ b/security/integrity/evm/evm_crypto.c @@ -105,13 +105,13 @@ static void hmac_add_misc(struct shash_desc *desc, struct inode *inode, umode_t mode; } hmac_misc; - memset(&hmac_misc, 0, sizeof hmac_misc); + memset(&hmac_misc, 0, sizeof(hmac_misc)); hmac_misc.ino = inode->i_ino; hmac_misc.generation = inode->i_generation; hmac_misc.uid = from_kuid(&init_user_ns, inode->i_uid); hmac_misc.gid = from_kgid(&init_user_ns, inode->i_gid); hmac_misc.mode = inode->i_mode; - crypto_shash_update(desc, (const u8 *)&hmac_misc, sizeof hmac_misc); + crypto_shash_update(desc, (const u8 *)&hmac_misc, sizeof(hmac_misc)); if (evm_hmac_version > 1) crypto_shash_update(desc, inode->i_sb->s_uuid, sizeof(inode->i_sb->s_uuid)); diff --git a/security/integrity/iint.c b/security/integrity/iint.c index c49d3f1..a521edf 100644 --- a/security/integrity/iint.c +++ b/security/integrity/iint.c @@ -151,7 +151,7 @@ static void init_once(void *foo) { struct integrity_iint_cache *iint = foo; - memset(iint, 0, sizeof *iint); + memset(iint, 0, sizeof(*iint)); iint->version = 0; iint->flags = 0UL; iint->ima_file_status = INTEGRITY_UNKNOWN; diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c index 393b9d4..c6b4a73 100644 --- a/security/integrity/ima/ima_api.c +++ b/security/integrity/ima/ima_api.c @@ -160,10 +160,10 @@ err_out: * @function: calling function (FILE_CHECK, BPRM_CHECK, MMAP_CHECK, MODULE_CHECK) * * The policy is defined in terms of keypairs: - * subj=, obj=, type=, func=, mask=, fsmagic= + * subj=, obj=, type=, func=, mask=, fsmagic= * subj,obj, and type: are LSM specific. - * func: FILE_CHECK | BPRM_CHECK | MMAP_CHECK | MODULE_CHECK - * mask: contains the permission mask + * func: FILE_CHECK | BPRM_CHECK | MMAP_CHECK | MODULE_CHECK + * mask: contains the permission mask * fsmagic: hex value * * Returns IMA_MEASURE, IMA_APPRAISE mask. @@ -248,7 +248,7 @@ int ima_collect_measurement(struct integrity_iint_cache *iint, * * We only get here if the inode has not already been measured, * but the measurement could already exist: - * - multiple copies of the same file on either the same or + * - multiple copies of the same file on either the same or * different filesystems. * - the inode was previously flushed as well as the iint info, * containing the hashing info. diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c index 9999057..d257e36 100644 --- a/security/integrity/ima/ima_crypto.c +++ b/security/integrity/ima/ima_crypto.c @@ -10,7 +10,7 @@ * the Free Software Foundation, version 2 of the License. * * File: ima_crypto.c - * Calculates md5/sha1 file hash, template hash, boot-aggreate hash + * Calculates md5/sha1 file hash, template hash, boot-aggreate hash */ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c index 468a3ba..da92fcc 100644 --- a/security/integrity/ima/ima_fs.c +++ b/security/integrity/ima/ima_fs.c @@ -133,14 +133,14 @@ static int ima_measurements_show(struct seq_file *m, void *v) * PCR used is always the same (config option) in * little-endian format */ - ima_putc(m, &pcr, sizeof pcr); + ima_putc(m, &pcr, sizeof(pcr)); /* 2nd: template digest */ ima_putc(m, e->digest, TPM_DIGEST_SIZE); /* 3rd: template name size */ namelen = strlen(e->template_desc->name); - ima_putc(m, &namelen, sizeof namelen); + ima_putc(m, &namelen, sizeof(namelen)); /* 4th: template name */ ima_putc(m, e->template_desc->name, namelen); @@ -292,7 +292,7 @@ static atomic_t policy_opencount = ATOMIC_INIT(1); /* * ima_open_policy: sequentialize access to the policy file */ -static int ima_open_policy(struct inode * inode, struct file * filp) +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)) diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 149ee11..50413d0 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -71,10 +71,10 @@ __setup("ima_hash=", hash_setup); * ima_rdwr_violation_check * * Only invalidate the PCR for measured files: - * - Opening a file for write when already open for read, + * - Opening a file for write when already open for read, * results in a time of measure, time of use (ToMToU) error. * - Opening a file for read when already open for write, - * could result in a file measurement error. + * could result in a file measurement error. * */ static void ima_rdwr_violation_check(struct file *file) diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c index 947cdbe..41021b4 100644 --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -7,7 +7,7 @@ * the Free Software Foundation, version 2 of the License. * * ima_policy.c - * - initialize default measure policy rules + * - initialize default measure policy rules * */ #include <linux/module.h> @@ -21,8 +21,8 @@ #include "ima.h" /* flags definitions */ -#define IMA_FUNC 0x0001 -#define IMA_MASK 0x0002 +#define IMA_FUNC 0x0001 +#define IMA_MASK 0x0002 #define IMA_FSMAGIC 0x0004 #define IMA_UID 0x0008 #define IMA_FOWNER 0x0010 @@ -69,35 +69,35 @@ struct ima_rule_entry { * and running executables. */ static struct ima_rule_entry default_rules[] = { - {.action = DONT_MEASURE,.fsmagic = PROC_SUPER_MAGIC,.flags = IMA_FSMAGIC}, - {.action = DONT_MEASURE,.fsmagic = SYSFS_MAGIC,.flags = IMA_FSMAGIC}, - {.action = DONT_MEASURE,.fsmagic = DEBUGFS_MAGIC,.flags = IMA_FSMAGIC}, - {.action = DONT_MEASURE,.fsmagic = TMPFS_MAGIC,.flags = IMA_FSMAGIC}, - {.action = DONT_MEASURE,.fsmagic = DEVPTS_SUPER_MAGIC,.flags = IMA_FSMAGIC}, - {.action = DONT_MEASURE,.fsmagic = BINFMTFS_MAGIC,.flags = IMA_FSMAGIC}, - {.action = DONT_MEASURE,.fsmagic = SECURITYFS_MAGIC,.flags = IMA_FSMAGIC}, - {.action = DONT_MEASURE,.fsmagic = SELINUX_MAGIC,.flags = IMA_FSMAGIC}, - {.action = MEASURE,.func = MMAP_CHECK,.mask = MAY_EXEC, + {.action = DONT_MEASURE, .fsmagic = PROC_SUPER_MAGIC, .flags = IMA_FSMAGIC}, + {.action = DONT_MEASURE, .fsmagic = SYSFS_MAGIC, .flags = IMA_FSMAGIC}, + {.action = DONT_MEASURE, .fsmagic = DEBUGFS_MAGIC, .flags = IMA_FSMAGIC}, + {.action = DONT_MEASURE, .fsmagic = TMPFS_MAGIC, .flags = IMA_FSMAGIC}, + {.action = DONT_MEASURE, .fsmagic = DEVPTS_SUPER_MAGIC, .flags = IMA_FSMAGIC}, + {.action = DONT_MEASURE, .fsmagic = BINFMTFS_MAGIC, .flags = IMA_FSMAGIC}, + {.action = DONT_MEASURE, .fsmagic = SECURITYFS_MAGIC, .flags = IMA_FSMAGIC}, + {.action = DONT_MEASURE, .fsmagic = SELINUX_MAGIC, .flags = IMA_FSMAGIC}, + {.action = MEASURE, .func = MMAP_CHECK, .mask = MAY_EXEC, .flags = IMA_FUNC | IMA_MASK}, - {.action = MEASURE,.func = BPRM_CHECK,.mask = MAY_EXEC, + {.action = MEASURE, .func = BPRM_CHECK, .mask = MAY_EXEC, .flags = IMA_FUNC | IMA_MASK}, - {.action = MEASURE,.func = FILE_CHECK,.mask = MAY_READ,.uid = GLOBAL_ROOT_UID, + {.action = MEASURE, .func = FILE_CHECK, .mask = MAY_READ, .uid = GLOBAL_ROOT_UID, .flags = IMA_FUNC | IMA_MASK | IMA_UID}, - {.action = MEASURE,.func = MODULE_CHECK, .flags = IMA_FUNC}, + {.action = MEASURE, .func = MODULE_CHECK, .flags = IMA_FUNC}, }; static struct ima_rule_entry default_appraise_rules[] = { - {.action = DONT_APPRAISE,.fsmagic = PROC_SUPER_MAGIC,.flags = IMA_FSMAGIC}, - {.action = DONT_APPRAISE,.fsmagic = SYSFS_MAGIC,.flags = IMA_FSMAGIC}, - {.action = DONT_APPRAISE,.fsmagic = DEBUGFS_MAGIC,.flags = IMA_FSMAGIC}, - {.action = DONT_APPRAISE,.fsmagic = TMPFS_MAGIC,.flags = IMA_FSMAGIC}, - {.action = DONT_APPRAISE,.fsmagic = RAMFS_MAGIC,.flags = IMA_FSMAGIC}, - {.action = DONT_APPRAISE,.fsmagic = DEVPTS_SUPER_MAGIC,.flags = IMA_FSMAGIC}, - {.action = DONT_APPRAISE,.fsmagic = BINFMTFS_MAGIC,.flags = IMA_FSMAGIC}, - {.action = DONT_APPRAISE,.fsmagic = SECURITYFS_MAGIC,.flags = IMA_FSMAGIC}, - {.action = DONT_APPRAISE,.fsmagic = SELINUX_MAGIC,.flags = IMA_FSMAGIC}, - {.action = DONT_APPRAISE,.fsmagic = CGROUP_SUPER_MAGIC,.flags = IMA_FSMAGIC}, - {.action = APPRAISE,.fowner = GLOBAL_ROOT_UID,.flags = IMA_FOWNER}, + {.action = DONT_APPRAISE, .fsmagic = PROC_SUPER_MAGIC, .flags = IMA_FSMAGIC}, + {.action = DONT_APPRAISE, .fsmagic = SYSFS_MAGIC, .flags = IMA_FSMAGIC}, + {.action = DONT_APPRAISE, .fsmagic = DEBUGFS_MAGIC, .flags = IMA_FSMAGIC}, + {.action = DONT_APPRAISE, .fsmagic = TMPFS_MAGIC, .flags = IMA_FSMAGIC}, + {.action = DONT_APPRAISE, .fsmagic = RAMFS_MAGIC, .flags = IMA_FSMAGIC}, + {.action = DONT_APPRAISE, .fsmagic = DEVPTS_SUPER_MAGIC, .flags = IMA_FSMAGIC}, + {.action = DONT_APPRAISE, .fsmagic = BINFMTFS_MAGIC, .flags = IMA_FSMAGIC}, + {.action = DONT_APPRAISE, .fsmagic = SECURITYFS_MAGIC, .flags = IMA_FSMAGIC}, + {.action = DONT_APPRAISE, .fsmagic = SELINUX_MAGIC, .flags = IMA_FSMAGIC}, + {.action = DONT_APPRAISE, .fsmagic = CGROUP_SUPER_MAGIC, .flags = IMA_FSMAGIC}, + {.action = APPRAISE, .fowner = GLOBAL_ROOT_UID, .flags = IMA_FOWNER}, }; static LIST_HEAD(ima_default_rules); @@ -122,12 +122,12 @@ static int __init default_appraise_policy_setup(char *str) } __setup("ima_appraise_tcb", default_appraise_policy_setup); -/* +/* * Although the IMA policy does not change, 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. + * Update the IMA LSM based rules to reflect the reloaded LSM policy. * We assume the rules still exist; and BUG_ON() if they don't. */ static void ima_lsm_update_rules(void) @@ -218,7 +218,7 @@ retry: retried = 1; ima_lsm_update_rules(); goto retry; - } + } if (!rc) return false; } @@ -234,7 +234,7 @@ static int get_subaction(struct ima_rule_entry *rule, int func) if (!(rule->flags & IMA_FUNC)) return IMA_FILE_APPRAISE; - switch(func) { + switch (func) { case MMAP_CHECK: return IMA_MMAP_APPRAISE; case BPRM_CHECK: @@ -306,7 +306,7 @@ void __init ima_init_policy(void) measure_entries = ima_use_tcb ? ARRAY_SIZE(default_rules) : 0; appraise_entries = ima_use_appraise_tcb ? ARRAY_SIZE(default_appraise_rules) : 0; - + for (i = 0; i < measure_entries + appraise_entries; i++) { if (i < measure_entries) list_add_tail(&default_rules[i].list, @@ -522,8 +522,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) break; } - result = strict_strtoul(args[0].from, 16, - &entry->fsmagic); + result = kstrtoul(args[0].from, 16, &entry->fsmagic); if (!result) entry->flags |= IMA_FSMAGIC; break; @@ -549,7 +548,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) break; } - result = strict_strtoul(args[0].from, 10, &lnum); + result = kstrtoul(args[0].from, 10, &lnum); if (!result) { entry->uid = make_kuid(current_user_ns(), (uid_t)lnum); if (!uid_valid(entry->uid) || (((uid_t)lnum) != lnum)) @@ -566,7 +565,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) break; } - result = strict_strtoul(args[0].from, 10, &lnum); + result = kstrtoul(args[0].from, 10, &lnum); if (!result) { entry->fowner = make_kuid(current_user_ns(), (uid_t)lnum); if (!uid_valid(entry->fowner) || (((uid_t)lnum) != lnum)) diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c index 91128b4..552705d 100644 --- a/security/integrity/ima/ima_queue.c +++ b/security/integrity/ima/ima_queue.c @@ -117,7 +117,7 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation, mutex_lock(&ima_extend_list_mutex); if (!violation) { - memcpy(digest, entry->digest, sizeof digest); + memcpy(digest, entry->digest, sizeof(digest)); if (ima_lookup_digest_entry(digest)) { audit_cause = "hash_exists"; result = -EEXIST; @@ -133,7 +133,7 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation, } if (violation) /* invalidate pcr */ - memset(digest, 0xff, sizeof digest); + memset(digest, 0xff, sizeof(digest)); tpmresult = ima_pcr_extend(digest); if (tpmresult != 0) { diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c index 9a4a0d1..a076a96 100644 --- a/security/integrity/ima/ima_template.c +++ b/security/integrity/ima/ima_template.c @@ -22,20 +22,20 @@ static struct ima_template_desc defined_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-ng", .fmt = "d-ng|n-ng"}, + {.name = "ima-sig", .fmt = "d-ng|n-ng|sig"}, }; static struct ima_template_field supported_fields[] = { - {.field_id = "d",.field_init = ima_eventdigest_init, + {.field_id = "d", .field_init = ima_eventdigest_init, .field_show = ima_show_template_digest}, - {.field_id = "n",.field_init = ima_eventname_init, + {.field_id = "n", .field_init = ima_eventname_init, .field_show = ima_show_template_string}, - {.field_id = "d-ng",.field_init = ima_eventdigest_ng_init, + {.field_id = "d-ng", .field_init = ima_eventdigest_ng_init, .field_show = ima_show_template_digest_ng}, - {.field_id = "n-ng",.field_init = ima_eventname_ng_init, + {.field_id = "n-ng", .field_init = ima_eventname_ng_init, .field_show = ima_show_template_string}, - {.field_id = "sig",.field_init = ima_eventsig_init, + {.field_id = "sig", .field_init = ima_eventsig_init, .field_show = ima_show_template_sig}, }; diff --git a/security/integrity/integrity_audit.c b/security/integrity/integrity_audit.c index 793d7be..aab9fa5 100644 --- a/security/integrity/integrity_audit.c +++ b/security/integrity/integrity_audit.c @@ -7,7 +7,7 @@ * the Free Software Foundation, version 2 of the License. * * File: integrity_audit.c - * Audit calls for the integrity subsystem + * Audit calls for the integrity subsystem */ #include <linux/fs.h> @@ -22,7 +22,7 @@ static int __init integrity_audit_setup(char *str) { unsigned long audit; - if (!strict_strtoul(str, 0, &audit)) + if (!kstrtoul(str, 0, &audit)) integrity_audit_info = audit ? 1 : 0; return 1; } -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH 2/8] integrity: fix checkpatch errors 2014-02-28 14:59 ` [PATCH 2/8] integrity: fix checkpatch errors Dmitry Kasatkin @ 2014-02-28 15:16 ` Dmitry Kasatkin 2014-03-03 13:41 ` Mimi Zohar 1 sibling, 0 replies; 31+ messages in thread From: Dmitry Kasatkin @ 2014-02-28 15:16 UTC (permalink / raw) To: linux-security-module, zohar Cc: jmorris, linux-kernel, casey.schaufler, dmitry.kasatkin This patch is on the top of Joe Perches patch. - Dmitry On 28/02/14 16:59, Dmitry Kasatkin wrote: > Unfixed checkpatch errors make it difficult to see new errors.. > This patch fix them. > Some lines with over 80 chars remained unchanged to improve > code readability. > > Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com> > --- > security/integrity/evm/evm.h | 28 +++++++------- > security/integrity/evm/evm_crypto.c | 4 +- > security/integrity/iint.c | 2 +- > security/integrity/ima/ima_api.c | 8 ++-- > security/integrity/ima/ima_crypto.c | 2 +- > security/integrity/ima/ima_fs.c | 6 +-- > security/integrity/ima/ima_main.c | 4 +- > security/integrity/ima/ima_policy.c | 69 +++++++++++++++++------------------ > security/integrity/ima/ima_queue.c | 4 +- > security/integrity/ima/ima_template.c | 14 +++---- > security/integrity/integrity_audit.c | 4 +- > 11 files changed, 72 insertions(+), 73 deletions(-) > > diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h > index 30bd1ec..37c88dd 100644 > --- a/security/integrity/evm/evm.h > +++ b/security/integrity/evm/evm.h > @@ -32,19 +32,19 @@ extern struct crypto_shash *hash_tfm; > /* List of EVM protected security xattrs */ > extern char *evm_config_xattrnames[]; > > -extern int evm_init_key(void); > -extern int evm_update_evmxattr(struct dentry *dentry, > - const char *req_xattr_name, > - const char *req_xattr_value, > - size_t req_xattr_value_len); > -extern int evm_calc_hmac(struct dentry *dentry, const char *req_xattr_name, > - const char *req_xattr_value, > - size_t req_xattr_value_len, char *digest); > -extern int evm_calc_hash(struct dentry *dentry, const char *req_xattr_name, > - const char *req_xattr_value, > - size_t req_xattr_value_len, char *digest); > -extern int evm_init_hmac(struct inode *inode, const struct xattr *xattr, > - char *hmac_val); > -extern int evm_init_secfs(void); > +int evm_init_key(void); > +int evm_update_evmxattr(struct dentry *dentry, > + const char *req_xattr_name, > + const char *req_xattr_value, > + size_t req_xattr_value_len); > +int evm_calc_hmac(struct dentry *dentry, const char *req_xattr_name, > + const char *req_xattr_value, > + size_t req_xattr_value_len, char *digest); > +int evm_calc_hash(struct dentry *dentry, const char *req_xattr_name, > + const char *req_xattr_value, > + size_t req_xattr_value_len, char *digest); > +int evm_init_hmac(struct inode *inode, const struct xattr *xattr, > + char *hmac_val); > +int evm_init_secfs(void); > > #endif > diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c > index 9bd329f..babd862 100644 > --- a/security/integrity/evm/evm_crypto.c > +++ b/security/integrity/evm/evm_crypto.c > @@ -105,13 +105,13 @@ static void hmac_add_misc(struct shash_desc *desc, struct inode *inode, > umode_t mode; > } hmac_misc; > > - memset(&hmac_misc, 0, sizeof hmac_misc); > + memset(&hmac_misc, 0, sizeof(hmac_misc)); > hmac_misc.ino = inode->i_ino; > hmac_misc.generation = inode->i_generation; > hmac_misc.uid = from_kuid(&init_user_ns, inode->i_uid); > hmac_misc.gid = from_kgid(&init_user_ns, inode->i_gid); > hmac_misc.mode = inode->i_mode; > - crypto_shash_update(desc, (const u8 *)&hmac_misc, sizeof hmac_misc); > + crypto_shash_update(desc, (const u8 *)&hmac_misc, sizeof(hmac_misc)); > if (evm_hmac_version > 1) > crypto_shash_update(desc, inode->i_sb->s_uuid, > sizeof(inode->i_sb->s_uuid)); > diff --git a/security/integrity/iint.c b/security/integrity/iint.c > index c49d3f1..a521edf 100644 > --- a/security/integrity/iint.c > +++ b/security/integrity/iint.c > @@ -151,7 +151,7 @@ static void init_once(void *foo) > { > struct integrity_iint_cache *iint = foo; > > - memset(iint, 0, sizeof *iint); > + memset(iint, 0, sizeof(*iint)); > iint->version = 0; > iint->flags = 0UL; > iint->ima_file_status = INTEGRITY_UNKNOWN; > diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c > index 393b9d4..c6b4a73 100644 > --- a/security/integrity/ima/ima_api.c > +++ b/security/integrity/ima/ima_api.c > @@ -160,10 +160,10 @@ err_out: > * @function: calling function (FILE_CHECK, BPRM_CHECK, MMAP_CHECK, MODULE_CHECK) > * > * The policy is defined in terms of keypairs: > - * subj=, obj=, type=, func=, mask=, fsmagic= > + * subj=, obj=, type=, func=, mask=, fsmagic= > * subj,obj, and type: are LSM specific. > - * func: FILE_CHECK | BPRM_CHECK | MMAP_CHECK | MODULE_CHECK > - * mask: contains the permission mask > + * func: FILE_CHECK | BPRM_CHECK | MMAP_CHECK | MODULE_CHECK > + * mask: contains the permission mask > * fsmagic: hex value > * > * Returns IMA_MEASURE, IMA_APPRAISE mask. > @@ -248,7 +248,7 @@ int ima_collect_measurement(struct integrity_iint_cache *iint, > * > * We only get here if the inode has not already been measured, > * but the measurement could already exist: > - * - multiple copies of the same file on either the same or > + * - multiple copies of the same file on either the same or > * different filesystems. > * - the inode was previously flushed as well as the iint info, > * containing the hashing info. > diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c > index 9999057..d257e36 100644 > --- a/security/integrity/ima/ima_crypto.c > +++ b/security/integrity/ima/ima_crypto.c > @@ -10,7 +10,7 @@ > * the Free Software Foundation, version 2 of the License. > * > * File: ima_crypto.c > - * Calculates md5/sha1 file hash, template hash, boot-aggreate hash > + * Calculates md5/sha1 file hash, template hash, boot-aggreate hash > */ > > #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c > index 468a3ba..da92fcc 100644 > --- a/security/integrity/ima/ima_fs.c > +++ b/security/integrity/ima/ima_fs.c > @@ -133,14 +133,14 @@ static int ima_measurements_show(struct seq_file *m, void *v) > * PCR used is always the same (config option) in > * little-endian format > */ > - ima_putc(m, &pcr, sizeof pcr); > + ima_putc(m, &pcr, sizeof(pcr)); > > /* 2nd: template digest */ > ima_putc(m, e->digest, TPM_DIGEST_SIZE); > > /* 3rd: template name size */ > namelen = strlen(e->template_desc->name); > - ima_putc(m, &namelen, sizeof namelen); > + ima_putc(m, &namelen, sizeof(namelen)); > > /* 4th: template name */ > ima_putc(m, e->template_desc->name, namelen); > @@ -292,7 +292,7 @@ static atomic_t policy_opencount = ATOMIC_INIT(1); > /* > * ima_open_policy: sequentialize access to the policy file > */ > -static int ima_open_policy(struct inode * inode, struct file * filp) > +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)) > diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c > index 149ee11..50413d0 100644 > --- a/security/integrity/ima/ima_main.c > +++ b/security/integrity/ima/ima_main.c > @@ -71,10 +71,10 @@ __setup("ima_hash=", hash_setup); > * ima_rdwr_violation_check > * > * Only invalidate the PCR for measured files: > - * - Opening a file for write when already open for read, > + * - Opening a file for write when already open for read, > * results in a time of measure, time of use (ToMToU) error. > * - Opening a file for read when already open for write, > - * could result in a file measurement error. > + * could result in a file measurement error. > * > */ > static void ima_rdwr_violation_check(struct file *file) > diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c > index 947cdbe..41021b4 100644 > --- a/security/integrity/ima/ima_policy.c > +++ b/security/integrity/ima/ima_policy.c > @@ -7,7 +7,7 @@ > * the Free Software Foundation, version 2 of the License. > * > * ima_policy.c > - * - initialize default measure policy rules > + * - initialize default measure policy rules > * > */ > #include <linux/module.h> > @@ -21,8 +21,8 @@ > #include "ima.h" > > /* flags definitions */ > -#define IMA_FUNC 0x0001 > -#define IMA_MASK 0x0002 > +#define IMA_FUNC 0x0001 > +#define IMA_MASK 0x0002 > #define IMA_FSMAGIC 0x0004 > #define IMA_UID 0x0008 > #define IMA_FOWNER 0x0010 > @@ -69,35 +69,35 @@ struct ima_rule_entry { > * and running executables. > */ > static struct ima_rule_entry default_rules[] = { > - {.action = DONT_MEASURE,.fsmagic = PROC_SUPER_MAGIC,.flags = IMA_FSMAGIC}, > - {.action = DONT_MEASURE,.fsmagic = SYSFS_MAGIC,.flags = IMA_FSMAGIC}, > - {.action = DONT_MEASURE,.fsmagic = DEBUGFS_MAGIC,.flags = IMA_FSMAGIC}, > - {.action = DONT_MEASURE,.fsmagic = TMPFS_MAGIC,.flags = IMA_FSMAGIC}, > - {.action = DONT_MEASURE,.fsmagic = DEVPTS_SUPER_MAGIC,.flags = IMA_FSMAGIC}, > - {.action = DONT_MEASURE,.fsmagic = BINFMTFS_MAGIC,.flags = IMA_FSMAGIC}, > - {.action = DONT_MEASURE,.fsmagic = SECURITYFS_MAGIC,.flags = IMA_FSMAGIC}, > - {.action = DONT_MEASURE,.fsmagic = SELINUX_MAGIC,.flags = IMA_FSMAGIC}, > - {.action = MEASURE,.func = MMAP_CHECK,.mask = MAY_EXEC, > + {.action = DONT_MEASURE, .fsmagic = PROC_SUPER_MAGIC, .flags = IMA_FSMAGIC}, > + {.action = DONT_MEASURE, .fsmagic = SYSFS_MAGIC, .flags = IMA_FSMAGIC}, > + {.action = DONT_MEASURE, .fsmagic = DEBUGFS_MAGIC, .flags = IMA_FSMAGIC}, > + {.action = DONT_MEASURE, .fsmagic = TMPFS_MAGIC, .flags = IMA_FSMAGIC}, > + {.action = DONT_MEASURE, .fsmagic = DEVPTS_SUPER_MAGIC, .flags = IMA_FSMAGIC}, > + {.action = DONT_MEASURE, .fsmagic = BINFMTFS_MAGIC, .flags = IMA_FSMAGIC}, > + {.action = DONT_MEASURE, .fsmagic = SECURITYFS_MAGIC, .flags = IMA_FSMAGIC}, > + {.action = DONT_MEASURE, .fsmagic = SELINUX_MAGIC, .flags = IMA_FSMAGIC}, > + {.action = MEASURE, .func = MMAP_CHECK, .mask = MAY_EXEC, > .flags = IMA_FUNC | IMA_MASK}, > - {.action = MEASURE,.func = BPRM_CHECK,.mask = MAY_EXEC, > + {.action = MEASURE, .func = BPRM_CHECK, .mask = MAY_EXEC, > .flags = IMA_FUNC | IMA_MASK}, > - {.action = MEASURE,.func = FILE_CHECK,.mask = MAY_READ,.uid = GLOBAL_ROOT_UID, > + {.action = MEASURE, .func = FILE_CHECK, .mask = MAY_READ, .uid = GLOBAL_ROOT_UID, > .flags = IMA_FUNC | IMA_MASK | IMA_UID}, > - {.action = MEASURE,.func = MODULE_CHECK, .flags = IMA_FUNC}, > + {.action = MEASURE, .func = MODULE_CHECK, .flags = IMA_FUNC}, > }; > > static struct ima_rule_entry default_appraise_rules[] = { > - {.action = DONT_APPRAISE,.fsmagic = PROC_SUPER_MAGIC,.flags = IMA_FSMAGIC}, > - {.action = DONT_APPRAISE,.fsmagic = SYSFS_MAGIC,.flags = IMA_FSMAGIC}, > - {.action = DONT_APPRAISE,.fsmagic = DEBUGFS_MAGIC,.flags = IMA_FSMAGIC}, > - {.action = DONT_APPRAISE,.fsmagic = TMPFS_MAGIC,.flags = IMA_FSMAGIC}, > - {.action = DONT_APPRAISE,.fsmagic = RAMFS_MAGIC,.flags = IMA_FSMAGIC}, > - {.action = DONT_APPRAISE,.fsmagic = DEVPTS_SUPER_MAGIC,.flags = IMA_FSMAGIC}, > - {.action = DONT_APPRAISE,.fsmagic = BINFMTFS_MAGIC,.flags = IMA_FSMAGIC}, > - {.action = DONT_APPRAISE,.fsmagic = SECURITYFS_MAGIC,.flags = IMA_FSMAGIC}, > - {.action = DONT_APPRAISE,.fsmagic = SELINUX_MAGIC,.flags = IMA_FSMAGIC}, > - {.action = DONT_APPRAISE,.fsmagic = CGROUP_SUPER_MAGIC,.flags = IMA_FSMAGIC}, > - {.action = APPRAISE,.fowner = GLOBAL_ROOT_UID,.flags = IMA_FOWNER}, > + {.action = DONT_APPRAISE, .fsmagic = PROC_SUPER_MAGIC, .flags = IMA_FSMAGIC}, > + {.action = DONT_APPRAISE, .fsmagic = SYSFS_MAGIC, .flags = IMA_FSMAGIC}, > + {.action = DONT_APPRAISE, .fsmagic = DEBUGFS_MAGIC, .flags = IMA_FSMAGIC}, > + {.action = DONT_APPRAISE, .fsmagic = TMPFS_MAGIC, .flags = IMA_FSMAGIC}, > + {.action = DONT_APPRAISE, .fsmagic = RAMFS_MAGIC, .flags = IMA_FSMAGIC}, > + {.action = DONT_APPRAISE, .fsmagic = DEVPTS_SUPER_MAGIC, .flags = IMA_FSMAGIC}, > + {.action = DONT_APPRAISE, .fsmagic = BINFMTFS_MAGIC, .flags = IMA_FSMAGIC}, > + {.action = DONT_APPRAISE, .fsmagic = SECURITYFS_MAGIC, .flags = IMA_FSMAGIC}, > + {.action = DONT_APPRAISE, .fsmagic = SELINUX_MAGIC, .flags = IMA_FSMAGIC}, > + {.action = DONT_APPRAISE, .fsmagic = CGROUP_SUPER_MAGIC, .flags = IMA_FSMAGIC}, > + {.action = APPRAISE, .fowner = GLOBAL_ROOT_UID, .flags = IMA_FOWNER}, > }; > > static LIST_HEAD(ima_default_rules); > @@ -122,12 +122,12 @@ static int __init default_appraise_policy_setup(char *str) > } > __setup("ima_appraise_tcb", default_appraise_policy_setup); > > -/* > +/* > * Although the IMA policy does not change, 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. > + * Update the IMA LSM based rules to reflect the reloaded LSM policy. > * We assume the rules still exist; and BUG_ON() if they don't. > */ > static void ima_lsm_update_rules(void) > @@ -218,7 +218,7 @@ retry: > retried = 1; > ima_lsm_update_rules(); > goto retry; > - } > + } > if (!rc) > return false; > } > @@ -234,7 +234,7 @@ static int get_subaction(struct ima_rule_entry *rule, int func) > if (!(rule->flags & IMA_FUNC)) > return IMA_FILE_APPRAISE; > > - switch(func) { > + switch (func) { > case MMAP_CHECK: > return IMA_MMAP_APPRAISE; > case BPRM_CHECK: > @@ -306,7 +306,7 @@ void __init ima_init_policy(void) > measure_entries = ima_use_tcb ? ARRAY_SIZE(default_rules) : 0; > appraise_entries = ima_use_appraise_tcb ? > ARRAY_SIZE(default_appraise_rules) : 0; > - > + > for (i = 0; i < measure_entries + appraise_entries; i++) { > if (i < measure_entries) > list_add_tail(&default_rules[i].list, > @@ -522,8 +522,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) > break; > } > > - result = strict_strtoul(args[0].from, 16, > - &entry->fsmagic); > + result = kstrtoul(args[0].from, 16, &entry->fsmagic); > if (!result) > entry->flags |= IMA_FSMAGIC; > break; > @@ -549,7 +548,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) > break; > } > > - result = strict_strtoul(args[0].from, 10, &lnum); > + result = kstrtoul(args[0].from, 10, &lnum); > if (!result) { > entry->uid = make_kuid(current_user_ns(), (uid_t)lnum); > if (!uid_valid(entry->uid) || (((uid_t)lnum) != lnum)) > @@ -566,7 +565,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) > break; > } > > - result = strict_strtoul(args[0].from, 10, &lnum); > + result = kstrtoul(args[0].from, 10, &lnum); > if (!result) { > entry->fowner = make_kuid(current_user_ns(), (uid_t)lnum); > if (!uid_valid(entry->fowner) || (((uid_t)lnum) != lnum)) > diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c > index 91128b4..552705d 100644 > --- a/security/integrity/ima/ima_queue.c > +++ b/security/integrity/ima/ima_queue.c > @@ -117,7 +117,7 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation, > > mutex_lock(&ima_extend_list_mutex); > if (!violation) { > - memcpy(digest, entry->digest, sizeof digest); > + memcpy(digest, entry->digest, sizeof(digest)); > if (ima_lookup_digest_entry(digest)) { > audit_cause = "hash_exists"; > result = -EEXIST; > @@ -133,7 +133,7 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation, > } > > if (violation) /* invalidate pcr */ > - memset(digest, 0xff, sizeof digest); > + memset(digest, 0xff, sizeof(digest)); > > tpmresult = ima_pcr_extend(digest); > if (tpmresult != 0) { > diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c > index 9a4a0d1..a076a96 100644 > --- a/security/integrity/ima/ima_template.c > +++ b/security/integrity/ima/ima_template.c > @@ -22,20 +22,20 @@ > > static struct ima_template_desc defined_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-ng", .fmt = "d-ng|n-ng"}, > + {.name = "ima-sig", .fmt = "d-ng|n-ng|sig"}, > }; > > static struct ima_template_field supported_fields[] = { > - {.field_id = "d",.field_init = ima_eventdigest_init, > + {.field_id = "d", .field_init = ima_eventdigest_init, > .field_show = ima_show_template_digest}, > - {.field_id = "n",.field_init = ima_eventname_init, > + {.field_id = "n", .field_init = ima_eventname_init, > .field_show = ima_show_template_string}, > - {.field_id = "d-ng",.field_init = ima_eventdigest_ng_init, > + {.field_id = "d-ng", .field_init = ima_eventdigest_ng_init, > .field_show = ima_show_template_digest_ng}, > - {.field_id = "n-ng",.field_init = ima_eventname_ng_init, > + {.field_id = "n-ng", .field_init = ima_eventname_ng_init, > .field_show = ima_show_template_string}, > - {.field_id = "sig",.field_init = ima_eventsig_init, > + {.field_id = "sig", .field_init = ima_eventsig_init, > .field_show = ima_show_template_sig}, > }; > > diff --git a/security/integrity/integrity_audit.c b/security/integrity/integrity_audit.c > index 793d7be..aab9fa5 100644 > --- a/security/integrity/integrity_audit.c > +++ b/security/integrity/integrity_audit.c > @@ -7,7 +7,7 @@ > * the Free Software Foundation, version 2 of the License. > * > * File: integrity_audit.c > - * Audit calls for the integrity subsystem > + * Audit calls for the integrity subsystem > */ > > #include <linux/fs.h> > @@ -22,7 +22,7 @@ static int __init integrity_audit_setup(char *str) > { > unsigned long audit; > > - if (!strict_strtoul(str, 0, &audit)) > + if (!kstrtoul(str, 0, &audit)) > integrity_audit_info = audit ? 1 : 0; > return 1; > } ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 2/8] integrity: fix checkpatch errors 2014-02-28 14:59 ` [PATCH 2/8] integrity: fix checkpatch errors Dmitry Kasatkin 2014-02-28 15:16 ` Dmitry Kasatkin @ 2014-03-03 13:41 ` Mimi Zohar 2014-03-03 14:01 ` Dmitry Kasatkin 2014-03-04 2:02 ` Mimi Zohar 1 sibling, 2 replies; 31+ messages in thread From: Mimi Zohar @ 2014-03-03 13:41 UTC (permalink / raw) To: Dmitry Kasatkin Cc: linux-security-module, jmorris, linux-kernel, casey.schaufler, dmitry.kasatkin On Fri, 2014-02-28 at 16:59 +0200, Dmitry Kasatkin wrote: > Unfixed checkpatch errors make it difficult to see new errors.. > This patch fix them. A number of these errors are a result of inconsistencies between Lindent and checkpatch. This patch uses the checkpatch preferences. > Some lines with over 80 chars remained unchanged to improve > code readability. > > Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com> This patch doesn't apply cleanly to either linux-integrity/next-fixes or to linux-security/next. thanks, Mimi > --- > security/integrity/evm/evm.h | 28 +++++++------- > security/integrity/evm/evm_crypto.c | 4 +- > security/integrity/iint.c | 2 +- > security/integrity/ima/ima_api.c | 8 ++-- > security/integrity/ima/ima_crypto.c | 2 +- > security/integrity/ima/ima_fs.c | 6 +-- > security/integrity/ima/ima_main.c | 4 +- > security/integrity/ima/ima_policy.c | 69 +++++++++++++++++------------------ > security/integrity/ima/ima_queue.c | 4 +- > security/integrity/ima/ima_template.c | 14 +++---- > security/integrity/integrity_audit.c | 4 +- > 11 files changed, 72 insertions(+), 73 deletions(-) > > diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h > index 30bd1ec..37c88dd 100644 > --- a/security/integrity/evm/evm.h > +++ b/security/integrity/evm/evm.h > @@ -32,19 +32,19 @@ extern struct crypto_shash *hash_tfm; > /* List of EVM protected security xattrs */ > extern char *evm_config_xattrnames[]; > > -extern int evm_init_key(void); > -extern int evm_update_evmxattr(struct dentry *dentry, > - const char *req_xattr_name, > - const char *req_xattr_value, > - size_t req_xattr_value_len); > -extern int evm_calc_hmac(struct dentry *dentry, const char *req_xattr_name, > - const char *req_xattr_value, > - size_t req_xattr_value_len, char *digest); > -extern int evm_calc_hash(struct dentry *dentry, const char *req_xattr_name, > - const char *req_xattr_value, > - size_t req_xattr_value_len, char *digest); > -extern int evm_init_hmac(struct inode *inode, const struct xattr *xattr, > - char *hmac_val); > -extern int evm_init_secfs(void); > +int evm_init_key(void); > +int evm_update_evmxattr(struct dentry *dentry, > + const char *req_xattr_name, > + const char *req_xattr_value, > + size_t req_xattr_value_len); > +int evm_calc_hmac(struct dentry *dentry, const char *req_xattr_name, > + const char *req_xattr_value, > + size_t req_xattr_value_len, char *digest); > +int evm_calc_hash(struct dentry *dentry, const char *req_xattr_name, > + const char *req_xattr_value, > + size_t req_xattr_value_len, char *digest); > +int evm_init_hmac(struct inode *inode, const struct xattr *xattr, > + char *hmac_val); > +int evm_init_secfs(void); > > #endif > diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c > index 9bd329f..babd862 100644 > --- a/security/integrity/evm/evm_crypto.c > +++ b/security/integrity/evm/evm_crypto.c > @@ -105,13 +105,13 @@ static void hmac_add_misc(struct shash_desc *desc, struct inode *inode, > umode_t mode; > } hmac_misc; > > - memset(&hmac_misc, 0, sizeof hmac_misc); > + memset(&hmac_misc, 0, sizeof(hmac_misc)); > hmac_misc.ino = inode->i_ino; > hmac_misc.generation = inode->i_generation; > hmac_misc.uid = from_kuid(&init_user_ns, inode->i_uid); > hmac_misc.gid = from_kgid(&init_user_ns, inode->i_gid); > hmac_misc.mode = inode->i_mode; > - crypto_shash_update(desc, (const u8 *)&hmac_misc, sizeof hmac_misc); > + crypto_shash_update(desc, (const u8 *)&hmac_misc, sizeof(hmac_misc)); > if (evm_hmac_version > 1) > crypto_shash_update(desc, inode->i_sb->s_uuid, > sizeof(inode->i_sb->s_uuid)); > diff --git a/security/integrity/iint.c b/security/integrity/iint.c > index c49d3f1..a521edf 100644 > --- a/security/integrity/iint.c > +++ b/security/integrity/iint.c > @@ -151,7 +151,7 @@ static void init_once(void *foo) > { > struct integrity_iint_cache *iint = foo; > > - memset(iint, 0, sizeof *iint); > + memset(iint, 0, sizeof(*iint)); > iint->version = 0; > iint->flags = 0UL; > iint->ima_file_status = INTEGRITY_UNKNOWN; > diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c > index 393b9d4..c6b4a73 100644 > --- a/security/integrity/ima/ima_api.c > +++ b/security/integrity/ima/ima_api.c > @@ -160,10 +160,10 @@ err_out: > * @function: calling function (FILE_CHECK, BPRM_CHECK, MMAP_CHECK, MODULE_CHECK) > * > * The policy is defined in terms of keypairs: > - * subj=, obj=, type=, func=, mask=, fsmagic= > + * subj=, obj=, type=, func=, mask=, fsmagic= > * subj,obj, and type: are LSM specific. > - * func: FILE_CHECK | BPRM_CHECK | MMAP_CHECK | MODULE_CHECK > - * mask: contains the permission mask > + * func: FILE_CHECK | BPRM_CHECK | MMAP_CHECK | MODULE_CHECK > + * mask: contains the permission mask > * fsmagic: hex value > * > * Returns IMA_MEASURE, IMA_APPRAISE mask. > @@ -248,7 +248,7 @@ int ima_collect_measurement(struct integrity_iint_cache *iint, > * > * We only get here if the inode has not already been measured, > * but the measurement could already exist: > - * - multiple copies of the same file on either the same or > + * - multiple copies of the same file on either the same or > * different filesystems. > * - the inode was previously flushed as well as the iint info, > * containing the hashing info. > diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c > index 9999057..d257e36 100644 > --- a/security/integrity/ima/ima_crypto.c > +++ b/security/integrity/ima/ima_crypto.c > @@ -10,7 +10,7 @@ > * the Free Software Foundation, version 2 of the License. > * > * File: ima_crypto.c > - * Calculates md5/sha1 file hash, template hash, boot-aggreate hash > + * Calculates md5/sha1 file hash, template hash, boot-aggreate hash > */ > > #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c > index 468a3ba..da92fcc 100644 > --- a/security/integrity/ima/ima_fs.c > +++ b/security/integrity/ima/ima_fs.c > @@ -133,14 +133,14 @@ static int ima_measurements_show(struct seq_file *m, void *v) > * PCR used is always the same (config option) in > * little-endian format > */ > - ima_putc(m, &pcr, sizeof pcr); > + ima_putc(m, &pcr, sizeof(pcr)); > > /* 2nd: template digest */ > ima_putc(m, e->digest, TPM_DIGEST_SIZE); > > /* 3rd: template name size */ > namelen = strlen(e->template_desc->name); > - ima_putc(m, &namelen, sizeof namelen); > + ima_putc(m, &namelen, sizeof(namelen)); > > /* 4th: template name */ > ima_putc(m, e->template_desc->name, namelen); > @@ -292,7 +292,7 @@ static atomic_t policy_opencount = ATOMIC_INIT(1); > /* > * ima_open_policy: sequentialize access to the policy file > */ > -static int ima_open_policy(struct inode * inode, struct file * filp) > +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)) > diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c > index 149ee11..50413d0 100644 > --- a/security/integrity/ima/ima_main.c > +++ b/security/integrity/ima/ima_main.c > @@ -71,10 +71,10 @@ __setup("ima_hash=", hash_setup); > * ima_rdwr_violation_check > * > * Only invalidate the PCR for measured files: > - * - Opening a file for write when already open for read, > + * - Opening a file for write when already open for read, > * results in a time of measure, time of use (ToMToU) error. > * - Opening a file for read when already open for write, > - * could result in a file measurement error. > + * could result in a file measurement error. > * > */ > static void ima_rdwr_violation_check(struct file *file) > diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c > index 947cdbe..41021b4 100644 > --- a/security/integrity/ima/ima_policy.c > +++ b/security/integrity/ima/ima_policy.c > @@ -7,7 +7,7 @@ > * the Free Software Foundation, version 2 of the License. > * > * ima_policy.c > - * - initialize default measure policy rules > + * - initialize default measure policy rules > * > */ > #include <linux/module.h> > @@ -21,8 +21,8 @@ > #include "ima.h" > > /* flags definitions */ > -#define IMA_FUNC 0x0001 > -#define IMA_MASK 0x0002 > +#define IMA_FUNC 0x0001 > +#define IMA_MASK 0x0002 > #define IMA_FSMAGIC 0x0004 > #define IMA_UID 0x0008 > #define IMA_FOWNER 0x0010 > @@ -69,35 +69,35 @@ struct ima_rule_entry { > * and running executables. > */ > static struct ima_rule_entry default_rules[] = { > - {.action = DONT_MEASURE,.fsmagic = PROC_SUPER_MAGIC,.flags = IMA_FSMAGIC}, > - {.action = DONT_MEASURE,.fsmagic = SYSFS_MAGIC,.flags = IMA_FSMAGIC}, > - {.action = DONT_MEASURE,.fsmagic = DEBUGFS_MAGIC,.flags = IMA_FSMAGIC}, > - {.action = DONT_MEASURE,.fsmagic = TMPFS_MAGIC,.flags = IMA_FSMAGIC}, > - {.action = DONT_MEASURE,.fsmagic = DEVPTS_SUPER_MAGIC,.flags = IMA_FSMAGIC}, > - {.action = DONT_MEASURE,.fsmagic = BINFMTFS_MAGIC,.flags = IMA_FSMAGIC}, > - {.action = DONT_MEASURE,.fsmagic = SECURITYFS_MAGIC,.flags = IMA_FSMAGIC}, > - {.action = DONT_MEASURE,.fsmagic = SELINUX_MAGIC,.flags = IMA_FSMAGIC}, > - {.action = MEASURE,.func = MMAP_CHECK,.mask = MAY_EXEC, > + {.action = DONT_MEASURE, .fsmagic = PROC_SUPER_MAGIC, .flags = IMA_FSMAGIC}, > + {.action = DONT_MEASURE, .fsmagic = SYSFS_MAGIC, .flags = IMA_FSMAGIC}, > + {.action = DONT_MEASURE, .fsmagic = DEBUGFS_MAGIC, .flags = IMA_FSMAGIC}, > + {.action = DONT_MEASURE, .fsmagic = TMPFS_MAGIC, .flags = IMA_FSMAGIC}, > + {.action = DONT_MEASURE, .fsmagic = DEVPTS_SUPER_MAGIC, .flags = IMA_FSMAGIC}, > + {.action = DONT_MEASURE, .fsmagic = BINFMTFS_MAGIC, .flags = IMA_FSMAGIC}, > + {.action = DONT_MEASURE, .fsmagic = SECURITYFS_MAGIC, .flags = IMA_FSMAGIC}, > + {.action = DONT_MEASURE, .fsmagic = SELINUX_MAGIC, .flags = IMA_FSMAGIC}, > + {.action = MEASURE, .func = MMAP_CHECK, .mask = MAY_EXEC, > .flags = IMA_FUNC | IMA_MASK}, > - {.action = MEASURE,.func = BPRM_CHECK,.mask = MAY_EXEC, > + {.action = MEASURE, .func = BPRM_CHECK, .mask = MAY_EXEC, > .flags = IMA_FUNC | IMA_MASK}, > - {.action = MEASURE,.func = FILE_CHECK,.mask = MAY_READ,.uid = GLOBAL_ROOT_UID, > + {.action = MEASURE, .func = FILE_CHECK, .mask = MAY_READ, .uid = GLOBAL_ROOT_UID, > .flags = IMA_FUNC | IMA_MASK | IMA_UID}, > - {.action = MEASURE,.func = MODULE_CHECK, .flags = IMA_FUNC}, > + {.action = MEASURE, .func = MODULE_CHECK, .flags = IMA_FUNC}, > }; > > static struct ima_rule_entry default_appraise_rules[] = { > - {.action = DONT_APPRAISE,.fsmagic = PROC_SUPER_MAGIC,.flags = IMA_FSMAGIC}, > - {.action = DONT_APPRAISE,.fsmagic = SYSFS_MAGIC,.flags = IMA_FSMAGIC}, > - {.action = DONT_APPRAISE,.fsmagic = DEBUGFS_MAGIC,.flags = IMA_FSMAGIC}, > - {.action = DONT_APPRAISE,.fsmagic = TMPFS_MAGIC,.flags = IMA_FSMAGIC}, > - {.action = DONT_APPRAISE,.fsmagic = RAMFS_MAGIC,.flags = IMA_FSMAGIC}, > - {.action = DONT_APPRAISE,.fsmagic = DEVPTS_SUPER_MAGIC,.flags = IMA_FSMAGIC}, > - {.action = DONT_APPRAISE,.fsmagic = BINFMTFS_MAGIC,.flags = IMA_FSMAGIC}, > - {.action = DONT_APPRAISE,.fsmagic = SECURITYFS_MAGIC,.flags = IMA_FSMAGIC}, > - {.action = DONT_APPRAISE,.fsmagic = SELINUX_MAGIC,.flags = IMA_FSMAGIC}, > - {.action = DONT_APPRAISE,.fsmagic = CGROUP_SUPER_MAGIC,.flags = IMA_FSMAGIC}, > - {.action = APPRAISE,.fowner = GLOBAL_ROOT_UID,.flags = IMA_FOWNER}, > + {.action = DONT_APPRAISE, .fsmagic = PROC_SUPER_MAGIC, .flags = IMA_FSMAGIC}, > + {.action = DONT_APPRAISE, .fsmagic = SYSFS_MAGIC, .flags = IMA_FSMAGIC}, > + {.action = DONT_APPRAISE, .fsmagic = DEBUGFS_MAGIC, .flags = IMA_FSMAGIC}, > + {.action = DONT_APPRAISE, .fsmagic = TMPFS_MAGIC, .flags = IMA_FSMAGIC}, > + {.action = DONT_APPRAISE, .fsmagic = RAMFS_MAGIC, .flags = IMA_FSMAGIC}, > + {.action = DONT_APPRAISE, .fsmagic = DEVPTS_SUPER_MAGIC, .flags = IMA_FSMAGIC}, > + {.action = DONT_APPRAISE, .fsmagic = BINFMTFS_MAGIC, .flags = IMA_FSMAGIC}, > + {.action = DONT_APPRAISE, .fsmagic = SECURITYFS_MAGIC, .flags = IMA_FSMAGIC}, > + {.action = DONT_APPRAISE, .fsmagic = SELINUX_MAGIC, .flags = IMA_FSMAGIC}, > + {.action = DONT_APPRAISE, .fsmagic = CGROUP_SUPER_MAGIC, .flags = IMA_FSMAGIC}, > + {.action = APPRAISE, .fowner = GLOBAL_ROOT_UID, .flags = IMA_FOWNER}, > }; > > static LIST_HEAD(ima_default_rules); > @@ -122,12 +122,12 @@ static int __init default_appraise_policy_setup(char *str) > } > __setup("ima_appraise_tcb", default_appraise_policy_setup); > > -/* > +/* > * Although the IMA policy does not change, 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. > + * Update the IMA LSM based rules to reflect the reloaded LSM policy. > * We assume the rules still exist; and BUG_ON() if they don't. > */ > static void ima_lsm_update_rules(void) > @@ -218,7 +218,7 @@ retry: > retried = 1; > ima_lsm_update_rules(); > goto retry; > - } > + } > if (!rc) > return false; > } > @@ -234,7 +234,7 @@ static int get_subaction(struct ima_rule_entry *rule, int func) > if (!(rule->flags & IMA_FUNC)) > return IMA_FILE_APPRAISE; > > - switch(func) { > + switch (func) { > case MMAP_CHECK: > return IMA_MMAP_APPRAISE; > case BPRM_CHECK: > @@ -306,7 +306,7 @@ void __init ima_init_policy(void) > measure_entries = ima_use_tcb ? ARRAY_SIZE(default_rules) : 0; > appraise_entries = ima_use_appraise_tcb ? > ARRAY_SIZE(default_appraise_rules) : 0; > - > + > for (i = 0; i < measure_entries + appraise_entries; i++) { > if (i < measure_entries) > list_add_tail(&default_rules[i].list, > @@ -522,8 +522,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) > break; > } > > - result = strict_strtoul(args[0].from, 16, > - &entry->fsmagic); > + result = kstrtoul(args[0].from, 16, &entry->fsmagic); > if (!result) > entry->flags |= IMA_FSMAGIC; > break; > @@ -549,7 +548,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) > break; > } > > - result = strict_strtoul(args[0].from, 10, &lnum); > + result = kstrtoul(args[0].from, 10, &lnum); > if (!result) { > entry->uid = make_kuid(current_user_ns(), (uid_t)lnum); > if (!uid_valid(entry->uid) || (((uid_t)lnum) != lnum)) > @@ -566,7 +565,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) > break; > } > > - result = strict_strtoul(args[0].from, 10, &lnum); > + result = kstrtoul(args[0].from, 10, &lnum); > if (!result) { > entry->fowner = make_kuid(current_user_ns(), (uid_t)lnum); > if (!uid_valid(entry->fowner) || (((uid_t)lnum) != lnum)) > diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c > index 91128b4..552705d 100644 > --- a/security/integrity/ima/ima_queue.c > +++ b/security/integrity/ima/ima_queue.c > @@ -117,7 +117,7 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation, > > mutex_lock(&ima_extend_list_mutex); > if (!violation) { > - memcpy(digest, entry->digest, sizeof digest); > + memcpy(digest, entry->digest, sizeof(digest)); > if (ima_lookup_digest_entry(digest)) { > audit_cause = "hash_exists"; > result = -EEXIST; > @@ -133,7 +133,7 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation, > } > > if (violation) /* invalidate pcr */ > - memset(digest, 0xff, sizeof digest); > + memset(digest, 0xff, sizeof(digest)); > > tpmresult = ima_pcr_extend(digest); > if (tpmresult != 0) { > diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c > index 9a4a0d1..a076a96 100644 > --- a/security/integrity/ima/ima_template.c > +++ b/security/integrity/ima/ima_template.c > @@ -22,20 +22,20 @@ > > static struct ima_template_desc defined_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-ng", .fmt = "d-ng|n-ng"}, > + {.name = "ima-sig", .fmt = "d-ng|n-ng|sig"}, > }; > > static struct ima_template_field supported_fields[] = { > - {.field_id = "d",.field_init = ima_eventdigest_init, > + {.field_id = "d", .field_init = ima_eventdigest_init, > .field_show = ima_show_template_digest}, > - {.field_id = "n",.field_init = ima_eventname_init, > + {.field_id = "n", .field_init = ima_eventname_init, > .field_show = ima_show_template_string}, > - {.field_id = "d-ng",.field_init = ima_eventdigest_ng_init, > + {.field_id = "d-ng", .field_init = ima_eventdigest_ng_init, > .field_show = ima_show_template_digest_ng}, > - {.field_id = "n-ng",.field_init = ima_eventname_ng_init, > + {.field_id = "n-ng", .field_init = ima_eventname_ng_init, > .field_show = ima_show_template_string}, > - {.field_id = "sig",.field_init = ima_eventsig_init, > + {.field_id = "sig", .field_init = ima_eventsig_init, > .field_show = ima_show_template_sig}, > }; > > diff --git a/security/integrity/integrity_audit.c b/security/integrity/integrity_audit.c > index 793d7be..aab9fa5 100644 > --- a/security/integrity/integrity_audit.c > +++ b/security/integrity/integrity_audit.c > @@ -7,7 +7,7 @@ > * the Free Software Foundation, version 2 of the License. > * > * File: integrity_audit.c > - * Audit calls for the integrity subsystem > + * Audit calls for the integrity subsystem > */ > > #include <linux/fs.h> > @@ -22,7 +22,7 @@ static int __init integrity_audit_setup(char *str) > { > unsigned long audit; > > - if (!strict_strtoul(str, 0, &audit)) > + if (!kstrtoul(str, 0, &audit)) > integrity_audit_info = audit ? 1 : 0; > return 1; > } ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 2/8] integrity: fix checkpatch errors 2014-03-03 13:41 ` Mimi Zohar @ 2014-03-03 14:01 ` Dmitry Kasatkin 2014-03-03 14:23 ` Mimi Zohar 2014-03-04 2:02 ` Mimi Zohar 1 sibling, 1 reply; 31+ messages in thread From: Dmitry Kasatkin @ 2014-03-03 14:01 UTC (permalink / raw) To: Mimi Zohar Cc: Dmitry Kasatkin, linux-security-module, James Morris, linux-kernel@vger.kernel.org, casey.schaufler On Mon, Mar 3, 2014 at 3:41 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote: > On Fri, 2014-02-28 at 16:59 +0200, Dmitry Kasatkin wrote: >> Unfixed checkpatch errors make it difficult to see new errors.. >> This patch fix them. > > A number of these errors are a result of inconsistencies between Lindent > and checkpatch. This patch uses the checkpatch preferences. > >> Some lines with over 80 chars remained unchanged to improve >> code readability. >> >> Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com> > > This patch doesn't apply cleanly to either linux-integrity/next-fixes or > to linux-security/next. > > thanks, > Hi, Did you apply Joe Perches's patch what you signed-off first? - Dmitry > Mimi > >> --- >> security/integrity/evm/evm.h | 28 +++++++------- >> security/integrity/evm/evm_crypto.c | 4 +- >> security/integrity/iint.c | 2 +- >> security/integrity/ima/ima_api.c | 8 ++-- >> security/integrity/ima/ima_crypto.c | 2 +- >> security/integrity/ima/ima_fs.c | 6 +-- >> security/integrity/ima/ima_main.c | 4 +- >> security/integrity/ima/ima_policy.c | 69 +++++++++++++++++------------------ >> security/integrity/ima/ima_queue.c | 4 +- >> security/integrity/ima/ima_template.c | 14 +++---- >> security/integrity/integrity_audit.c | 4 +- >> 11 files changed, 72 insertions(+), 73 deletions(-) >> >> diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h >> index 30bd1ec..37c88dd 100644 >> --- a/security/integrity/evm/evm.h >> +++ b/security/integrity/evm/evm.h >> @@ -32,19 +32,19 @@ extern struct crypto_shash *hash_tfm; >> /* List of EVM protected security xattrs */ >> extern char *evm_config_xattrnames[]; >> >> -extern int evm_init_key(void); >> -extern int evm_update_evmxattr(struct dentry *dentry, >> - const char *req_xattr_name, >> - const char *req_xattr_value, >> - size_t req_xattr_value_len); >> -extern int evm_calc_hmac(struct dentry *dentry, const char *req_xattr_name, >> - const char *req_xattr_value, >> - size_t req_xattr_value_len, char *digest); >> -extern int evm_calc_hash(struct dentry *dentry, const char *req_xattr_name, >> - const char *req_xattr_value, >> - size_t req_xattr_value_len, char *digest); >> -extern int evm_init_hmac(struct inode *inode, const struct xattr *xattr, >> - char *hmac_val); >> -extern int evm_init_secfs(void); >> +int evm_init_key(void); >> +int evm_update_evmxattr(struct dentry *dentry, >> + const char *req_xattr_name, >> + const char *req_xattr_value, >> + size_t req_xattr_value_len); >> +int evm_calc_hmac(struct dentry *dentry, const char *req_xattr_name, >> + const char *req_xattr_value, >> + size_t req_xattr_value_len, char *digest); >> +int evm_calc_hash(struct dentry *dentry, const char *req_xattr_name, >> + const char *req_xattr_value, >> + size_t req_xattr_value_len, char *digest); >> +int evm_init_hmac(struct inode *inode, const struct xattr *xattr, >> + char *hmac_val); >> +int evm_init_secfs(void); >> >> #endif >> diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c >> index 9bd329f..babd862 100644 >> --- a/security/integrity/evm/evm_crypto.c >> +++ b/security/integrity/evm/evm_crypto.c >> @@ -105,13 +105,13 @@ static void hmac_add_misc(struct shash_desc *desc, struct inode *inode, >> umode_t mode; >> } hmac_misc; >> >> - memset(&hmac_misc, 0, sizeof hmac_misc); >> + memset(&hmac_misc, 0, sizeof(hmac_misc)); >> hmac_misc.ino = inode->i_ino; >> hmac_misc.generation = inode->i_generation; >> hmac_misc.uid = from_kuid(&init_user_ns, inode->i_uid); >> hmac_misc.gid = from_kgid(&init_user_ns, inode->i_gid); >> hmac_misc.mode = inode->i_mode; >> - crypto_shash_update(desc, (const u8 *)&hmac_misc, sizeof hmac_misc); >> + crypto_shash_update(desc, (const u8 *)&hmac_misc, sizeof(hmac_misc)); >> if (evm_hmac_version > 1) >> crypto_shash_update(desc, inode->i_sb->s_uuid, >> sizeof(inode->i_sb->s_uuid)); >> diff --git a/security/integrity/iint.c b/security/integrity/iint.c >> index c49d3f1..a521edf 100644 >> --- a/security/integrity/iint.c >> +++ b/security/integrity/iint.c >> @@ -151,7 +151,7 @@ static void init_once(void *foo) >> { >> struct integrity_iint_cache *iint = foo; >> >> - memset(iint, 0, sizeof *iint); >> + memset(iint, 0, sizeof(*iint)); >> iint->version = 0; >> iint->flags = 0UL; >> iint->ima_file_status = INTEGRITY_UNKNOWN; >> diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c >> index 393b9d4..c6b4a73 100644 >> --- a/security/integrity/ima/ima_api.c >> +++ b/security/integrity/ima/ima_api.c >> @@ -160,10 +160,10 @@ err_out: >> * @function: calling function (FILE_CHECK, BPRM_CHECK, MMAP_CHECK, MODULE_CHECK) >> * >> * The policy is defined in terms of keypairs: >> - * subj=, obj=, type=, func=, mask=, fsmagic= >> + * subj=, obj=, type=, func=, mask=, fsmagic= >> * subj,obj, and type: are LSM specific. >> - * func: FILE_CHECK | BPRM_CHECK | MMAP_CHECK | MODULE_CHECK >> - * mask: contains the permission mask >> + * func: FILE_CHECK | BPRM_CHECK | MMAP_CHECK | MODULE_CHECK >> + * mask: contains the permission mask >> * fsmagic: hex value >> * >> * Returns IMA_MEASURE, IMA_APPRAISE mask. >> @@ -248,7 +248,7 @@ int ima_collect_measurement(struct integrity_iint_cache *iint, >> * >> * We only get here if the inode has not already been measured, >> * but the measurement could already exist: >> - * - multiple copies of the same file on either the same or >> + * - multiple copies of the same file on either the same or >> * different filesystems. >> * - the inode was previously flushed as well as the iint info, >> * containing the hashing info. >> diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c >> index 9999057..d257e36 100644 >> --- a/security/integrity/ima/ima_crypto.c >> +++ b/security/integrity/ima/ima_crypto.c >> @@ -10,7 +10,7 @@ >> * the Free Software Foundation, version 2 of the License. >> * >> * File: ima_crypto.c >> - * Calculates md5/sha1 file hash, template hash, boot-aggreate hash >> + * Calculates md5/sha1 file hash, template hash, boot-aggreate hash >> */ >> >> #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt >> diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c >> index 468a3ba..da92fcc 100644 >> --- a/security/integrity/ima/ima_fs.c >> +++ b/security/integrity/ima/ima_fs.c >> @@ -133,14 +133,14 @@ static int ima_measurements_show(struct seq_file *m, void *v) >> * PCR used is always the same (config option) in >> * little-endian format >> */ >> - ima_putc(m, &pcr, sizeof pcr); >> + ima_putc(m, &pcr, sizeof(pcr)); >> >> /* 2nd: template digest */ >> ima_putc(m, e->digest, TPM_DIGEST_SIZE); >> >> /* 3rd: template name size */ >> namelen = strlen(e->template_desc->name); >> - ima_putc(m, &namelen, sizeof namelen); >> + ima_putc(m, &namelen, sizeof(namelen)); >> >> /* 4th: template name */ >> ima_putc(m, e->template_desc->name, namelen); >> @@ -292,7 +292,7 @@ static atomic_t policy_opencount = ATOMIC_INIT(1); >> /* >> * ima_open_policy: sequentialize access to the policy file >> */ >> -static int ima_open_policy(struct inode * inode, struct file * filp) >> +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)) >> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c >> index 149ee11..50413d0 100644 >> --- a/security/integrity/ima/ima_main.c >> +++ b/security/integrity/ima/ima_main.c >> @@ -71,10 +71,10 @@ __setup("ima_hash=", hash_setup); >> * ima_rdwr_violation_check >> * >> * Only invalidate the PCR for measured files: >> - * - Opening a file for write when already open for read, >> + * - Opening a file for write when already open for read, >> * results in a time of measure, time of use (ToMToU) error. >> * - Opening a file for read when already open for write, >> - * could result in a file measurement error. >> + * could result in a file measurement error. >> * >> */ >> static void ima_rdwr_violation_check(struct file *file) >> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c >> index 947cdbe..41021b4 100644 >> --- a/security/integrity/ima/ima_policy.c >> +++ b/security/integrity/ima/ima_policy.c >> @@ -7,7 +7,7 @@ >> * the Free Software Foundation, version 2 of the License. >> * >> * ima_policy.c >> - * - initialize default measure policy rules >> + * - initialize default measure policy rules >> * >> */ >> #include <linux/module.h> >> @@ -21,8 +21,8 @@ >> #include "ima.h" >> >> /* flags definitions */ >> -#define IMA_FUNC 0x0001 >> -#define IMA_MASK 0x0002 >> +#define IMA_FUNC 0x0001 >> +#define IMA_MASK 0x0002 >> #define IMA_FSMAGIC 0x0004 >> #define IMA_UID 0x0008 >> #define IMA_FOWNER 0x0010 >> @@ -69,35 +69,35 @@ struct ima_rule_entry { >> * and running executables. >> */ >> static struct ima_rule_entry default_rules[] = { >> - {.action = DONT_MEASURE,.fsmagic = PROC_SUPER_MAGIC,.flags = IMA_FSMAGIC}, >> - {.action = DONT_MEASURE,.fsmagic = SYSFS_MAGIC,.flags = IMA_FSMAGIC}, >> - {.action = DONT_MEASURE,.fsmagic = DEBUGFS_MAGIC,.flags = IMA_FSMAGIC}, >> - {.action = DONT_MEASURE,.fsmagic = TMPFS_MAGIC,.flags = IMA_FSMAGIC}, >> - {.action = DONT_MEASURE,.fsmagic = DEVPTS_SUPER_MAGIC,.flags = IMA_FSMAGIC}, >> - {.action = DONT_MEASURE,.fsmagic = BINFMTFS_MAGIC,.flags = IMA_FSMAGIC}, >> - {.action = DONT_MEASURE,.fsmagic = SECURITYFS_MAGIC,.flags = IMA_FSMAGIC}, >> - {.action = DONT_MEASURE,.fsmagic = SELINUX_MAGIC,.flags = IMA_FSMAGIC}, >> - {.action = MEASURE,.func = MMAP_CHECK,.mask = MAY_EXEC, >> + {.action = DONT_MEASURE, .fsmagic = PROC_SUPER_MAGIC, .flags = IMA_FSMAGIC}, >> + {.action = DONT_MEASURE, .fsmagic = SYSFS_MAGIC, .flags = IMA_FSMAGIC}, >> + {.action = DONT_MEASURE, .fsmagic = DEBUGFS_MAGIC, .flags = IMA_FSMAGIC}, >> + {.action = DONT_MEASURE, .fsmagic = TMPFS_MAGIC, .flags = IMA_FSMAGIC}, >> + {.action = DONT_MEASURE, .fsmagic = DEVPTS_SUPER_MAGIC, .flags = IMA_FSMAGIC}, >> + {.action = DONT_MEASURE, .fsmagic = BINFMTFS_MAGIC, .flags = IMA_FSMAGIC}, >> + {.action = DONT_MEASURE, .fsmagic = SECURITYFS_MAGIC, .flags = IMA_FSMAGIC}, >> + {.action = DONT_MEASURE, .fsmagic = SELINUX_MAGIC, .flags = IMA_FSMAGIC}, >> + {.action = MEASURE, .func = MMAP_CHECK, .mask = MAY_EXEC, >> .flags = IMA_FUNC | IMA_MASK}, >> - {.action = MEASURE,.func = BPRM_CHECK,.mask = MAY_EXEC, >> + {.action = MEASURE, .func = BPRM_CHECK, .mask = MAY_EXEC, >> .flags = IMA_FUNC | IMA_MASK}, >> - {.action = MEASURE,.func = FILE_CHECK,.mask = MAY_READ,.uid = GLOBAL_ROOT_UID, >> + {.action = MEASURE, .func = FILE_CHECK, .mask = MAY_READ, .uid = GLOBAL_ROOT_UID, >> .flags = IMA_FUNC | IMA_MASK | IMA_UID}, >> - {.action = MEASURE,.func = MODULE_CHECK, .flags = IMA_FUNC}, >> + {.action = MEASURE, .func = MODULE_CHECK, .flags = IMA_FUNC}, >> }; >> >> static struct ima_rule_entry default_appraise_rules[] = { >> - {.action = DONT_APPRAISE,.fsmagic = PROC_SUPER_MAGIC,.flags = IMA_FSMAGIC}, >> - {.action = DONT_APPRAISE,.fsmagic = SYSFS_MAGIC,.flags = IMA_FSMAGIC}, >> - {.action = DONT_APPRAISE,.fsmagic = DEBUGFS_MAGIC,.flags = IMA_FSMAGIC}, >> - {.action = DONT_APPRAISE,.fsmagic = TMPFS_MAGIC,.flags = IMA_FSMAGIC}, >> - {.action = DONT_APPRAISE,.fsmagic = RAMFS_MAGIC,.flags = IMA_FSMAGIC}, >> - {.action = DONT_APPRAISE,.fsmagic = DEVPTS_SUPER_MAGIC,.flags = IMA_FSMAGIC}, >> - {.action = DONT_APPRAISE,.fsmagic = BINFMTFS_MAGIC,.flags = IMA_FSMAGIC}, >> - {.action = DONT_APPRAISE,.fsmagic = SECURITYFS_MAGIC,.flags = IMA_FSMAGIC}, >> - {.action = DONT_APPRAISE,.fsmagic = SELINUX_MAGIC,.flags = IMA_FSMAGIC}, >> - {.action = DONT_APPRAISE,.fsmagic = CGROUP_SUPER_MAGIC,.flags = IMA_FSMAGIC}, >> - {.action = APPRAISE,.fowner = GLOBAL_ROOT_UID,.flags = IMA_FOWNER}, >> + {.action = DONT_APPRAISE, .fsmagic = PROC_SUPER_MAGIC, .flags = IMA_FSMAGIC}, >> + {.action = DONT_APPRAISE, .fsmagic = SYSFS_MAGIC, .flags = IMA_FSMAGIC}, >> + {.action = DONT_APPRAISE, .fsmagic = DEBUGFS_MAGIC, .flags = IMA_FSMAGIC}, >> + {.action = DONT_APPRAISE, .fsmagic = TMPFS_MAGIC, .flags = IMA_FSMAGIC}, >> + {.action = DONT_APPRAISE, .fsmagic = RAMFS_MAGIC, .flags = IMA_FSMAGIC}, >> + {.action = DONT_APPRAISE, .fsmagic = DEVPTS_SUPER_MAGIC, .flags = IMA_FSMAGIC}, >> + {.action = DONT_APPRAISE, .fsmagic = BINFMTFS_MAGIC, .flags = IMA_FSMAGIC}, >> + {.action = DONT_APPRAISE, .fsmagic = SECURITYFS_MAGIC, .flags = IMA_FSMAGIC}, >> + {.action = DONT_APPRAISE, .fsmagic = SELINUX_MAGIC, .flags = IMA_FSMAGIC}, >> + {.action = DONT_APPRAISE, .fsmagic = CGROUP_SUPER_MAGIC, .flags = IMA_FSMAGIC}, >> + {.action = APPRAISE, .fowner = GLOBAL_ROOT_UID, .flags = IMA_FOWNER}, >> }; >> >> static LIST_HEAD(ima_default_rules); >> @@ -122,12 +122,12 @@ static int __init default_appraise_policy_setup(char *str) >> } >> __setup("ima_appraise_tcb", default_appraise_policy_setup); >> >> -/* >> +/* >> * Although the IMA policy does not change, 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. >> + * Update the IMA LSM based rules to reflect the reloaded LSM policy. >> * We assume the rules still exist; and BUG_ON() if they don't. >> */ >> static void ima_lsm_update_rules(void) >> @@ -218,7 +218,7 @@ retry: >> retried = 1; >> ima_lsm_update_rules(); >> goto retry; >> - } >> + } >> if (!rc) >> return false; >> } >> @@ -234,7 +234,7 @@ static int get_subaction(struct ima_rule_entry *rule, int func) >> if (!(rule->flags & IMA_FUNC)) >> return IMA_FILE_APPRAISE; >> >> - switch(func) { >> + switch (func) { >> case MMAP_CHECK: >> return IMA_MMAP_APPRAISE; >> case BPRM_CHECK: >> @@ -306,7 +306,7 @@ void __init ima_init_policy(void) >> measure_entries = ima_use_tcb ? ARRAY_SIZE(default_rules) : 0; >> appraise_entries = ima_use_appraise_tcb ? >> ARRAY_SIZE(default_appraise_rules) : 0; >> - >> + >> for (i = 0; i < measure_entries + appraise_entries; i++) { >> if (i < measure_entries) >> list_add_tail(&default_rules[i].list, >> @@ -522,8 +522,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) >> break; >> } >> >> - result = strict_strtoul(args[0].from, 16, >> - &entry->fsmagic); >> + result = kstrtoul(args[0].from, 16, &entry->fsmagic); >> if (!result) >> entry->flags |= IMA_FSMAGIC; >> break; >> @@ -549,7 +548,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) >> break; >> } >> >> - result = strict_strtoul(args[0].from, 10, &lnum); >> + result = kstrtoul(args[0].from, 10, &lnum); >> if (!result) { >> entry->uid = make_kuid(current_user_ns(), (uid_t)lnum); >> if (!uid_valid(entry->uid) || (((uid_t)lnum) != lnum)) >> @@ -566,7 +565,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) >> break; >> } >> >> - result = strict_strtoul(args[0].from, 10, &lnum); >> + result = kstrtoul(args[0].from, 10, &lnum); >> if (!result) { >> entry->fowner = make_kuid(current_user_ns(), (uid_t)lnum); >> if (!uid_valid(entry->fowner) || (((uid_t)lnum) != lnum)) >> diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c >> index 91128b4..552705d 100644 >> --- a/security/integrity/ima/ima_queue.c >> +++ b/security/integrity/ima/ima_queue.c >> @@ -117,7 +117,7 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation, >> >> mutex_lock(&ima_extend_list_mutex); >> if (!violation) { >> - memcpy(digest, entry->digest, sizeof digest); >> + memcpy(digest, entry->digest, sizeof(digest)); >> if (ima_lookup_digest_entry(digest)) { >> audit_cause = "hash_exists"; >> result = -EEXIST; >> @@ -133,7 +133,7 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation, >> } >> >> if (violation) /* invalidate pcr */ >> - memset(digest, 0xff, sizeof digest); >> + memset(digest, 0xff, sizeof(digest)); >> >> tpmresult = ima_pcr_extend(digest); >> if (tpmresult != 0) { >> diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c >> index 9a4a0d1..a076a96 100644 >> --- a/security/integrity/ima/ima_template.c >> +++ b/security/integrity/ima/ima_template.c >> @@ -22,20 +22,20 @@ >> >> static struct ima_template_desc defined_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-ng", .fmt = "d-ng|n-ng"}, >> + {.name = "ima-sig", .fmt = "d-ng|n-ng|sig"}, >> }; >> >> static struct ima_template_field supported_fields[] = { >> - {.field_id = "d",.field_init = ima_eventdigest_init, >> + {.field_id = "d", .field_init = ima_eventdigest_init, >> .field_show = ima_show_template_digest}, >> - {.field_id = "n",.field_init = ima_eventname_init, >> + {.field_id = "n", .field_init = ima_eventname_init, >> .field_show = ima_show_template_string}, >> - {.field_id = "d-ng",.field_init = ima_eventdigest_ng_init, >> + {.field_id = "d-ng", .field_init = ima_eventdigest_ng_init, >> .field_show = ima_show_template_digest_ng}, >> - {.field_id = "n-ng",.field_init = ima_eventname_ng_init, >> + {.field_id = "n-ng", .field_init = ima_eventname_ng_init, >> .field_show = ima_show_template_string}, >> - {.field_id = "sig",.field_init = ima_eventsig_init, >> + {.field_id = "sig", .field_init = ima_eventsig_init, >> .field_show = ima_show_template_sig}, >> }; >> >> diff --git a/security/integrity/integrity_audit.c b/security/integrity/integrity_audit.c >> index 793d7be..aab9fa5 100644 >> --- a/security/integrity/integrity_audit.c >> +++ b/security/integrity/integrity_audit.c >> @@ -7,7 +7,7 @@ >> * the Free Software Foundation, version 2 of the License. >> * >> * File: integrity_audit.c >> - * Audit calls for the integrity subsystem >> + * Audit calls for the integrity subsystem >> */ >> >> #include <linux/fs.h> >> @@ -22,7 +22,7 @@ static int __init integrity_audit_setup(char *str) >> { >> unsigned long audit; >> >> - if (!strict_strtoul(str, 0, &audit)) >> + if (!kstrtoul(str, 0, &audit)) >> integrity_audit_info = audit ? 1 : 0; >> return 1; >> } > > -- Thanks, Dmitry ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 2/8] integrity: fix checkpatch errors 2014-03-03 14:01 ` Dmitry Kasatkin @ 2014-03-03 14:23 ` Mimi Zohar 0 siblings, 0 replies; 31+ messages in thread From: Mimi Zohar @ 2014-03-03 14:23 UTC (permalink / raw) To: Dmitry Kasatkin Cc: Dmitry Kasatkin, linux-security-module, James Morris, linux-kernel@vger.kernel.org, casey.schaufler On Mon, 2014-03-03 at 16:01 +0200, Dmitry Kasatkin wrote: > On Mon, Mar 3, 2014 at 3:41 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote: > > On Fri, 2014-02-28 at 16:59 +0200, Dmitry Kasatkin wrote: > >> Unfixed checkpatch errors make it difficult to see new errors.. > >> This patch fix them. > > > > A number of these errors are a result of inconsistencies between Lindent > > and checkpatch. This patch uses the checkpatch preferences. > > > >> Some lines with over 80 chars remained unchanged to improve > >> code readability. > >> > >> Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com> > > > > This patch doesn't apply cleanly to either linux-integrity/next-fixes or > > to linux-security/next. > > > > thanks, > > > > Hi, > > Did you apply Joe Perches's patch what you signed-off first? Thanks for the reminder. I hadn't applied Joe's patch, but that didn't resolve it. Mimi ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 2/8] integrity: fix checkpatch errors 2014-03-03 13:41 ` Mimi Zohar 2014-03-03 14:01 ` Dmitry Kasatkin @ 2014-03-04 2:02 ` Mimi Zohar 1 sibling, 0 replies; 31+ messages in thread From: Mimi Zohar @ 2014-03-04 2:02 UTC (permalink / raw) To: Dmitry Kasatkin Cc: linux-security-module, jmorris, linux-kernel, casey.schaufler, dmitry.kasatkin On Mon, 2014-03-03 at 08:41 -0500, Mimi Zohar wrote: > On Fri, 2014-02-28 at 16:59 +0200, Dmitry Kasatkin wrote: > > Unfixed checkpatch errors make it difficult to see new errors.. > > This patch fix them. Between checkpatch changes (eg. sizeof) and inconsistencies between Lindent and checkpatch, unfixed checkpatch errors make it difficult to see new errors. This patch fixes them. checkpatch doesn't complain about 'evm.h'. Please include the reason for the change in the patch description. > > Some lines with over 80 chars remained unchanged to improve > > code readability. > > > > Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com> > > This patch doesn't apply cleanly to either linux-integrity/next-fixes or > to linux-security/next. Thanks for rebasing against linux-integrity/next-fixes. Mimi > > > --- > > security/integrity/evm/evm.h | 28 +++++++------- > > security/integrity/evm/evm_crypto.c | 4 +- > > security/integrity/iint.c | 2 +- > > security/integrity/ima/ima_api.c | 8 ++-- > > security/integrity/ima/ima_crypto.c | 2 +- > > security/integrity/ima/ima_fs.c | 6 +-- > > security/integrity/ima/ima_main.c | 4 +- > > security/integrity/ima/ima_policy.c | 69 +++++++++++++++++------------------ > > security/integrity/ima/ima_queue.c | 4 +- > > security/integrity/ima/ima_template.c | 14 +++---- > > security/integrity/integrity_audit.c | 4 +- > > 11 files changed, 72 insertions(+), 73 deletions(-) > > > > diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h > > index 30bd1ec..37c88dd 100644 > > --- a/security/integrity/evm/evm.h > > +++ b/security/integrity/evm/evm.h > > @@ -32,19 +32,19 @@ extern struct crypto_shash *hash_tfm; > > /* List of EVM protected security xattrs */ > > extern char *evm_config_xattrnames[]; > > > > -extern int evm_init_key(void); > > -extern int evm_update_evmxattr(struct dentry *dentry, > > - const char *req_xattr_name, > > - const char *req_xattr_value, > > - size_t req_xattr_value_len); > > -extern int evm_calc_hmac(struct dentry *dentry, const char *req_xattr_name, > > - const char *req_xattr_value, > > - size_t req_xattr_value_len, char *digest); > > -extern int evm_calc_hash(struct dentry *dentry, const char *req_xattr_name, > > - const char *req_xattr_value, > > - size_t req_xattr_value_len, char *digest); > > -extern int evm_init_hmac(struct inode *inode, const struct xattr *xattr, > > - char *hmac_val); > > -extern int evm_init_secfs(void); > > +int evm_init_key(void); > > +int evm_update_evmxattr(struct dentry *dentry, > > + const char *req_xattr_name, > > + const char *req_xattr_value, > > + size_t req_xattr_value_len); > > +int evm_calc_hmac(struct dentry *dentry, const char *req_xattr_name, > > + const char *req_xattr_value, > > + size_t req_xattr_value_len, char *digest); > > +int evm_calc_hash(struct dentry *dentry, const char *req_xattr_name, > > + const char *req_xattr_value, > > + size_t req_xattr_value_len, char *digest); > > +int evm_init_hmac(struct inode *inode, const struct xattr *xattr, > > + char *hmac_val); > > +int evm_init_secfs(void); > > > > #endif > > diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c > > index 9bd329f..babd862 100644 > > --- a/security/integrity/evm/evm_crypto.c > > +++ b/security/integrity/evm/evm_crypto.c > > @@ -105,13 +105,13 @@ static void hmac_add_misc(struct shash_desc *desc, struct inode *inode, > > umode_t mode; > > } hmac_misc; > > > > - memset(&hmac_misc, 0, sizeof hmac_misc); > > + memset(&hmac_misc, 0, sizeof(hmac_misc)); > > hmac_misc.ino = inode->i_ino; > > hmac_misc.generation = inode->i_generation; > > hmac_misc.uid = from_kuid(&init_user_ns, inode->i_uid); > > hmac_misc.gid = from_kgid(&init_user_ns, inode->i_gid); > > hmac_misc.mode = inode->i_mode; > > - crypto_shash_update(desc, (const u8 *)&hmac_misc, sizeof hmac_misc); > > + crypto_shash_update(desc, (const u8 *)&hmac_misc, sizeof(hmac_misc)); > > if (evm_hmac_version > 1) > > crypto_shash_update(desc, inode->i_sb->s_uuid, > > sizeof(inode->i_sb->s_uuid)); > > diff --git a/security/integrity/iint.c b/security/integrity/iint.c > > index c49d3f1..a521edf 100644 > > --- a/security/integrity/iint.c > > +++ b/security/integrity/iint.c > > @@ -151,7 +151,7 @@ static void init_once(void *foo) > > { > > struct integrity_iint_cache *iint = foo; > > > > - memset(iint, 0, sizeof *iint); > > + memset(iint, 0, sizeof(*iint)); > > iint->version = 0; > > iint->flags = 0UL; > > iint->ima_file_status = INTEGRITY_UNKNOWN; > > diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c > > index 393b9d4..c6b4a73 100644 > > --- a/security/integrity/ima/ima_api.c > > +++ b/security/integrity/ima/ima_api.c > > @@ -160,10 +160,10 @@ err_out: > > * @function: calling function (FILE_CHECK, BPRM_CHECK, MMAP_CHECK, MODULE_CHECK) > > * > > * The policy is defined in terms of keypairs: > > - * subj=, obj=, type=, func=, mask=, fsmagic= > > + * subj=, obj=, type=, func=, mask=, fsmagic= > > * subj,obj, and type: are LSM specific. > > - * func: FILE_CHECK | BPRM_CHECK | MMAP_CHECK | MODULE_CHECK > > - * mask: contains the permission mask > > + * func: FILE_CHECK | BPRM_CHECK | MMAP_CHECK | MODULE_CHECK > > + * mask: contains the permission mask > > * fsmagic: hex value > > * > > * Returns IMA_MEASURE, IMA_APPRAISE mask. > > @@ -248,7 +248,7 @@ int ima_collect_measurement(struct integrity_iint_cache *iint, > > * > > * We only get here if the inode has not already been measured, > > * but the measurement could already exist: > > - * - multiple copies of the same file on either the same or > > + * - multiple copies of the same file on either the same or > > * different filesystems. > > * - the inode was previously flushed as well as the iint info, > > * containing the hashing info. > > diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c > > index 9999057..d257e36 100644 > > --- a/security/integrity/ima/ima_crypto.c > > +++ b/security/integrity/ima/ima_crypto.c > > @@ -10,7 +10,7 @@ > > * the Free Software Foundation, version 2 of the License. > > * > > * File: ima_crypto.c > > - * Calculates md5/sha1 file hash, template hash, boot-aggreate hash > > + * Calculates md5/sha1 file hash, template hash, boot-aggreate hash > > */ > > > > #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > > diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c > > index 468a3ba..da92fcc 100644 > > --- a/security/integrity/ima/ima_fs.c > > +++ b/security/integrity/ima/ima_fs.c > > @@ -133,14 +133,14 @@ static int ima_measurements_show(struct seq_file *m, void *v) > > * PCR used is always the same (config option) in > > * little-endian format > > */ > > - ima_putc(m, &pcr, sizeof pcr); > > + ima_putc(m, &pcr, sizeof(pcr)); > > > > /* 2nd: template digest */ > > ima_putc(m, e->digest, TPM_DIGEST_SIZE); > > > > /* 3rd: template name size */ > > namelen = strlen(e->template_desc->name); > > - ima_putc(m, &namelen, sizeof namelen); > > + ima_putc(m, &namelen, sizeof(namelen)); > > > > /* 4th: template name */ > > ima_putc(m, e->template_desc->name, namelen); > > @@ -292,7 +292,7 @@ static atomic_t policy_opencount = ATOMIC_INIT(1); > > /* > > * ima_open_policy: sequentialize access to the policy file > > */ > > -static int ima_open_policy(struct inode * inode, struct file * filp) > > +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)) > > diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c > > index 149ee11..50413d0 100644 > > --- a/security/integrity/ima/ima_main.c > > +++ b/security/integrity/ima/ima_main.c > > @@ -71,10 +71,10 @@ __setup("ima_hash=", hash_setup); > > * ima_rdwr_violation_check > > * > > * Only invalidate the PCR for measured files: > > - * - Opening a file for write when already open for read, > > + * - Opening a file for write when already open for read, > > * results in a time of measure, time of use (ToMToU) error. > > * - Opening a file for read when already open for write, > > - * could result in a file measurement error. > > + * could result in a file measurement error. > > * > > */ > > static void ima_rdwr_violation_check(struct file *file) > > diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c > > index 947cdbe..41021b4 100644 > > --- a/security/integrity/ima/ima_policy.c > > +++ b/security/integrity/ima/ima_policy.c > > @@ -7,7 +7,7 @@ > > * the Free Software Foundation, version 2 of the License. > > * > > * ima_policy.c > > - * - initialize default measure policy rules > > + * - initialize default measure policy rules > > * > > */ > > #include <linux/module.h> > > @@ -21,8 +21,8 @@ > > #include "ima.h" > > > > /* flags definitions */ > > -#define IMA_FUNC 0x0001 > > -#define IMA_MASK 0x0002 > > +#define IMA_FUNC 0x0001 > > +#define IMA_MASK 0x0002 > > #define IMA_FSMAGIC 0x0004 > > #define IMA_UID 0x0008 > > #define IMA_FOWNER 0x0010 > > @@ -69,35 +69,35 @@ struct ima_rule_entry { > > * and running executables. > > */ > > static struct ima_rule_entry default_rules[] = { > > - {.action = DONT_MEASURE,.fsmagic = PROC_SUPER_MAGIC,.flags = IMA_FSMAGIC}, > > - {.action = DONT_MEASURE,.fsmagic = SYSFS_MAGIC,.flags = IMA_FSMAGIC}, > > - {.action = DONT_MEASURE,.fsmagic = DEBUGFS_MAGIC,.flags = IMA_FSMAGIC}, > > - {.action = DONT_MEASURE,.fsmagic = TMPFS_MAGIC,.flags = IMA_FSMAGIC}, > > - {.action = DONT_MEASURE,.fsmagic = DEVPTS_SUPER_MAGIC,.flags = IMA_FSMAGIC}, > > - {.action = DONT_MEASURE,.fsmagic = BINFMTFS_MAGIC,.flags = IMA_FSMAGIC}, > > - {.action = DONT_MEASURE,.fsmagic = SECURITYFS_MAGIC,.flags = IMA_FSMAGIC}, > > - {.action = DONT_MEASURE,.fsmagic = SELINUX_MAGIC,.flags = IMA_FSMAGIC}, > > - {.action = MEASURE,.func = MMAP_CHECK,.mask = MAY_EXEC, > > + {.action = DONT_MEASURE, .fsmagic = PROC_SUPER_MAGIC, .flags = IMA_FSMAGIC}, > > + {.action = DONT_MEASURE, .fsmagic = SYSFS_MAGIC, .flags = IMA_FSMAGIC}, > > + {.action = DONT_MEASURE, .fsmagic = DEBUGFS_MAGIC, .flags = IMA_FSMAGIC}, > > + {.action = DONT_MEASURE, .fsmagic = TMPFS_MAGIC, .flags = IMA_FSMAGIC}, > > + {.action = DONT_MEASURE, .fsmagic = DEVPTS_SUPER_MAGIC, .flags = IMA_FSMAGIC}, > > + {.action = DONT_MEASURE, .fsmagic = BINFMTFS_MAGIC, .flags = IMA_FSMAGIC}, > > + {.action = DONT_MEASURE, .fsmagic = SECURITYFS_MAGIC, .flags = IMA_FSMAGIC}, > > + {.action = DONT_MEASURE, .fsmagic = SELINUX_MAGIC, .flags = IMA_FSMAGIC}, > > + {.action = MEASURE, .func = MMAP_CHECK, .mask = MAY_EXEC, > > .flags = IMA_FUNC | IMA_MASK}, > > - {.action = MEASURE,.func = BPRM_CHECK,.mask = MAY_EXEC, > > + {.action = MEASURE, .func = BPRM_CHECK, .mask = MAY_EXEC, > > .flags = IMA_FUNC | IMA_MASK}, > > - {.action = MEASURE,.func = FILE_CHECK,.mask = MAY_READ,.uid = GLOBAL_ROOT_UID, > > + {.action = MEASURE, .func = FILE_CHECK, .mask = MAY_READ, .uid = GLOBAL_ROOT_UID, > > .flags = IMA_FUNC | IMA_MASK | IMA_UID}, > > - {.action = MEASURE,.func = MODULE_CHECK, .flags = IMA_FUNC}, > > + {.action = MEASURE, .func = MODULE_CHECK, .flags = IMA_FUNC}, > > }; > > > > static struct ima_rule_entry default_appraise_rules[] = { > > - {.action = DONT_APPRAISE,.fsmagic = PROC_SUPER_MAGIC,.flags = IMA_FSMAGIC}, > > - {.action = DONT_APPRAISE,.fsmagic = SYSFS_MAGIC,.flags = IMA_FSMAGIC}, > > - {.action = DONT_APPRAISE,.fsmagic = DEBUGFS_MAGIC,.flags = IMA_FSMAGIC}, > > - {.action = DONT_APPRAISE,.fsmagic = TMPFS_MAGIC,.flags = IMA_FSMAGIC}, > > - {.action = DONT_APPRAISE,.fsmagic = RAMFS_MAGIC,.flags = IMA_FSMAGIC}, > > - {.action = DONT_APPRAISE,.fsmagic = DEVPTS_SUPER_MAGIC,.flags = IMA_FSMAGIC}, > > - {.action = DONT_APPRAISE,.fsmagic = BINFMTFS_MAGIC,.flags = IMA_FSMAGIC}, > > - {.action = DONT_APPRAISE,.fsmagic = SECURITYFS_MAGIC,.flags = IMA_FSMAGIC}, > > - {.action = DONT_APPRAISE,.fsmagic = SELINUX_MAGIC,.flags = IMA_FSMAGIC}, > > - {.action = DONT_APPRAISE,.fsmagic = CGROUP_SUPER_MAGIC,.flags = IMA_FSMAGIC}, > > - {.action = APPRAISE,.fowner = GLOBAL_ROOT_UID,.flags = IMA_FOWNER}, > > + {.action = DONT_APPRAISE, .fsmagic = PROC_SUPER_MAGIC, .flags = IMA_FSMAGIC}, > > + {.action = DONT_APPRAISE, .fsmagic = SYSFS_MAGIC, .flags = IMA_FSMAGIC}, > > + {.action = DONT_APPRAISE, .fsmagic = DEBUGFS_MAGIC, .flags = IMA_FSMAGIC}, > > + {.action = DONT_APPRAISE, .fsmagic = TMPFS_MAGIC, .flags = IMA_FSMAGIC}, > > + {.action = DONT_APPRAISE, .fsmagic = RAMFS_MAGIC, .flags = IMA_FSMAGIC}, > > + {.action = DONT_APPRAISE, .fsmagic = DEVPTS_SUPER_MAGIC, .flags = IMA_FSMAGIC}, > > + {.action = DONT_APPRAISE, .fsmagic = BINFMTFS_MAGIC, .flags = IMA_FSMAGIC}, > > + {.action = DONT_APPRAISE, .fsmagic = SECURITYFS_MAGIC, .flags = IMA_FSMAGIC}, > > + {.action = DONT_APPRAISE, .fsmagic = SELINUX_MAGIC, .flags = IMA_FSMAGIC}, > > + {.action = DONT_APPRAISE, .fsmagic = CGROUP_SUPER_MAGIC, .flags = IMA_FSMAGIC}, > > + {.action = APPRAISE, .fowner = GLOBAL_ROOT_UID, .flags = IMA_FOWNER}, > > }; > > > > static LIST_HEAD(ima_default_rules); > > @@ -122,12 +122,12 @@ static int __init default_appraise_policy_setup(char *str) > > } > > __setup("ima_appraise_tcb", default_appraise_policy_setup); > > > > -/* > > +/* > > * Although the IMA policy does not change, 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. > > + * Update the IMA LSM based rules to reflect the reloaded LSM policy. > > * We assume the rules still exist; and BUG_ON() if they don't. > > */ > > static void ima_lsm_update_rules(void) > > @@ -218,7 +218,7 @@ retry: > > retried = 1; > > ima_lsm_update_rules(); > > goto retry; > > - } > > + } > > if (!rc) > > return false; > > } > > @@ -234,7 +234,7 @@ static int get_subaction(struct ima_rule_entry *rule, int func) > > if (!(rule->flags & IMA_FUNC)) > > return IMA_FILE_APPRAISE; > > > > - switch(func) { > > + switch (func) { > > case MMAP_CHECK: > > return IMA_MMAP_APPRAISE; > > case BPRM_CHECK: > > @@ -306,7 +306,7 @@ void __init ima_init_policy(void) > > measure_entries = ima_use_tcb ? ARRAY_SIZE(default_rules) : 0; > > appraise_entries = ima_use_appraise_tcb ? > > ARRAY_SIZE(default_appraise_rules) : 0; > > - > > + > > for (i = 0; i < measure_entries + appraise_entries; i++) { > > if (i < measure_entries) > > list_add_tail(&default_rules[i].list, > > @@ -522,8 +522,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) > > break; > > } > > > > - result = strict_strtoul(args[0].from, 16, > > - &entry->fsmagic); > > + result = kstrtoul(args[0].from, 16, &entry->fsmagic); > > if (!result) > > entry->flags |= IMA_FSMAGIC; > > break; > > @@ -549,7 +548,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) > > break; > > } > > > > - result = strict_strtoul(args[0].from, 10, &lnum); > > + result = kstrtoul(args[0].from, 10, &lnum); > > if (!result) { > > entry->uid = make_kuid(current_user_ns(), (uid_t)lnum); > > if (!uid_valid(entry->uid) || (((uid_t)lnum) != lnum)) > > @@ -566,7 +565,7 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry) > > break; > > } > > > > - result = strict_strtoul(args[0].from, 10, &lnum); > > + result = kstrtoul(args[0].from, 10, &lnum); > > if (!result) { > > entry->fowner = make_kuid(current_user_ns(), (uid_t)lnum); > > if (!uid_valid(entry->fowner) || (((uid_t)lnum) != lnum)) > > diff --git a/security/integrity/ima/ima_queue.c b/security/integrity/ima/ima_queue.c > > index 91128b4..552705d 100644 > > --- a/security/integrity/ima/ima_queue.c > > +++ b/security/integrity/ima/ima_queue.c > > @@ -117,7 +117,7 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation, > > > > mutex_lock(&ima_extend_list_mutex); > > if (!violation) { > > - memcpy(digest, entry->digest, sizeof digest); > > + memcpy(digest, entry->digest, sizeof(digest)); > > if (ima_lookup_digest_entry(digest)) { > > audit_cause = "hash_exists"; > > result = -EEXIST; > > @@ -133,7 +133,7 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation, > > } > > > > if (violation) /* invalidate pcr */ > > - memset(digest, 0xff, sizeof digest); > > + memset(digest, 0xff, sizeof(digest)); > > > > tpmresult = ima_pcr_extend(digest); > > if (tpmresult != 0) { > > diff --git a/security/integrity/ima/ima_template.c b/security/integrity/ima/ima_template.c > > index 9a4a0d1..a076a96 100644 > > --- a/security/integrity/ima/ima_template.c > > +++ b/security/integrity/ima/ima_template.c > > @@ -22,20 +22,20 @@ > > > > static struct ima_template_desc defined_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-ng", .fmt = "d-ng|n-ng"}, > > + {.name = "ima-sig", .fmt = "d-ng|n-ng|sig"}, > > }; > > > > static struct ima_template_field supported_fields[] = { > > - {.field_id = "d",.field_init = ima_eventdigest_init, > > + {.field_id = "d", .field_init = ima_eventdigest_init, > > .field_show = ima_show_template_digest}, > > - {.field_id = "n",.field_init = ima_eventname_init, > > + {.field_id = "n", .field_init = ima_eventname_init, > > .field_show = ima_show_template_string}, > > - {.field_id = "d-ng",.field_init = ima_eventdigest_ng_init, > > + {.field_id = "d-ng", .field_init = ima_eventdigest_ng_init, > > .field_show = ima_show_template_digest_ng}, > > - {.field_id = "n-ng",.field_init = ima_eventname_ng_init, > > + {.field_id = "n-ng", .field_init = ima_eventname_ng_init, > > .field_show = ima_show_template_string}, > > - {.field_id = "sig",.field_init = ima_eventsig_init, > > + {.field_id = "sig", .field_init = ima_eventsig_init, > > .field_show = ima_show_template_sig}, > > }; > > > > diff --git a/security/integrity/integrity_audit.c b/security/integrity/integrity_audit.c > > index 793d7be..aab9fa5 100644 > > --- a/security/integrity/integrity_audit.c > > +++ b/security/integrity/integrity_audit.c > > @@ -7,7 +7,7 @@ > > * the Free Software Foundation, version 2 of the License. > > * > > * File: integrity_audit.c > > - * Audit calls for the integrity subsystem > > + * Audit calls for the integrity subsystem > > */ > > > > #include <linux/fs.h> > > @@ -22,7 +22,7 @@ static int __init integrity_audit_setup(char *str) > > { > > unsigned long audit; > > > > - if (!strict_strtoul(str, 0, &audit)) > > + if (!kstrtoul(str, 0, &audit)) > > integrity_audit_info = audit ? 1 : 0; > > return 1; > > } > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-security-module" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 3/8] ima: return d_name.name if d_path fails 2014-02-28 14:59 [PATCH 0/8] integrity: fixes and new features Dmitry Kasatkin 2014-02-28 14:59 ` [PATCH 1/8] ima: fix erronous removal of security.ima xattr Dmitry Kasatkin 2014-02-28 14:59 ` [PATCH 2/8] integrity: fix checkpatch errors Dmitry Kasatkin @ 2014-02-28 14:59 ` Dmitry Kasatkin 2014-03-03 13:45 ` Mimi Zohar 2014-02-28 14:59 ` [PATCH 4/8] evm: EVM does not use MD5 Dmitry Kasatkin ` (4 subsequent siblings) 7 siblings, 1 reply; 31+ messages in thread From: Dmitry Kasatkin @ 2014-02-28 14:59 UTC (permalink / raw) To: linux-security-module, zohar Cc: jmorris, linux-kernel, casey.schaufler, dmitry.kasatkin, Dmitry Kasatkin Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com> --- security/integrity/ima/ima_api.c | 2 +- security/integrity/ima/ima_main.c | 7 +------ 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c index c6b4a73..ba9e4d7 100644 --- a/security/integrity/ima/ima_api.c +++ b/security/integrity/ima/ima_api.c @@ -332,5 +332,5 @@ const char *ima_d_path(struct path *path, char **pathbuf) pathname = NULL; } } - return pathname; + return pathname ?: (const char *)path->dentry->d_name.name; } diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c index 50413d0..52ac6cf 100644 --- a/security/integrity/ima/ima_main.c +++ b/security/integrity/ima/ima_main.c @@ -79,7 +79,6 @@ __setup("ima_hash=", hash_setup); */ static void ima_rdwr_violation_check(struct file *file) { - struct dentry *dentry = file->f_path.dentry; struct inode *inode = file_inode(file); fmode_t mode = file->f_mode; int must_measure; @@ -111,8 +110,6 @@ out: return; pathname = ima_d_path(&file->f_path, &pathbuf); - if (!pathname || strlen(pathname) > IMA_EVENT_NAME_LEN_MAX) - pathname = dentry->d_name.name; if (send_tomtou) ima_add_violation(file, pathname, "invalid_pcr", "ToMToU"); @@ -220,9 +217,7 @@ static int process_measurement(struct file *file, const char *filename, if (rc != 0) goto out_digsig; - pathname = !filename ? ima_d_path(&file->f_path, &pathbuf) : filename; - if (!pathname) - pathname = (const char *)file->f_dentry->d_name.name; + pathname = filename ?: ima_d_path(&file->f_path, &pathbuf); if (action & IMA_MEASURE) ima_store_measurement(iint, file, pathname, -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH 3/8] ima: return d_name.name if d_path fails 2014-02-28 14:59 ` [PATCH 3/8] ima: return d_name.name if d_path fails Dmitry Kasatkin @ 2014-03-03 13:45 ` Mimi Zohar 0 siblings, 0 replies; 31+ messages in thread From: Mimi Zohar @ 2014-03-03 13:45 UTC (permalink / raw) To: Dmitry Kasatkin Cc: linux-security-module, jmorris, linux-kernel, casey.schaufler, dmitry.kasatkin On Fri, 2014-02-28 at 16:59 +0200, Dmitry Kasatkin wrote: > Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com> Nice refactoring. Please add a patch description. Mimi > --- > security/integrity/ima/ima_api.c | 2 +- > security/integrity/ima/ima_main.c | 7 +------ > 2 files changed, 2 insertions(+), 7 deletions(-) > > diff --git a/security/integrity/ima/ima_api.c b/security/integrity/ima/ima_api.c > index c6b4a73..ba9e4d7 100644 > --- a/security/integrity/ima/ima_api.c > +++ b/security/integrity/ima/ima_api.c > @@ -332,5 +332,5 @@ const char *ima_d_path(struct path *path, char **pathbuf) > pathname = NULL; > } > } > - return pathname; > + return pathname ?: (const char *)path->dentry->d_name.name; > } > diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c > index 50413d0..52ac6cf 100644 > --- a/security/integrity/ima/ima_main.c > +++ b/security/integrity/ima/ima_main.c > @@ -79,7 +79,6 @@ __setup("ima_hash=", hash_setup); > */ > static void ima_rdwr_violation_check(struct file *file) > { > - struct dentry *dentry = file->f_path.dentry; > struct inode *inode = file_inode(file); > fmode_t mode = file->f_mode; > int must_measure; > @@ -111,8 +110,6 @@ out: > return; > > pathname = ima_d_path(&file->f_path, &pathbuf); > - if (!pathname || strlen(pathname) > IMA_EVENT_NAME_LEN_MAX) > - pathname = dentry->d_name.name; > > if (send_tomtou) > ima_add_violation(file, pathname, "invalid_pcr", "ToMToU"); > @@ -220,9 +217,7 @@ static int process_measurement(struct file *file, const char *filename, > if (rc != 0) > goto out_digsig; > > - pathname = !filename ? ima_d_path(&file->f_path, &pathbuf) : filename; > - if (!pathname) > - pathname = (const char *)file->f_dentry->d_name.name; > + pathname = filename ?: ima_d_path(&file->f_path, &pathbuf); > > if (action & IMA_MEASURE) > ima_store_measurement(iint, file, pathname, ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 4/8] evm: EVM does not use MD5 2014-02-28 14:59 [PATCH 0/8] integrity: fixes and new features Dmitry Kasatkin ` (2 preceding siblings ...) 2014-02-28 14:59 ` [PATCH 3/8] ima: return d_name.name if d_path fails Dmitry Kasatkin @ 2014-02-28 14:59 ` Dmitry Kasatkin 2014-03-04 2:02 ` Mimi Zohar 2014-02-28 14:59 ` [PATCH 5/8] ima: skip memory allocation for empty files Dmitry Kasatkin ` (3 subsequent siblings) 7 siblings, 1 reply; 31+ messages in thread From: Dmitry Kasatkin @ 2014-02-28 14:59 UTC (permalink / raw) To: linux-security-module, zohar Cc: jmorris, linux-kernel, casey.schaufler, dmitry.kasatkin, Dmitry Kasatkin Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com> --- security/integrity/evm/Kconfig | 1 - 1 file changed, 1 deletion(-) diff --git a/security/integrity/evm/Kconfig b/security/integrity/evm/Kconfig index fea9749..5aa9103 100644 --- a/security/integrity/evm/Kconfig +++ b/security/integrity/evm/Kconfig @@ -2,7 +2,6 @@ config EVM boolean "EVM support" depends on SECURITY && KEYS && (TRUSTED_KEYS=y || TRUSTED_KEYS=n) select CRYPTO_HMAC - select CRYPTO_MD5 select CRYPTO_SHA1 select ENCRYPTED_KEYS default n -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH 4/8] evm: EVM does not use MD5 2014-02-28 14:59 ` [PATCH 4/8] evm: EVM does not use MD5 Dmitry Kasatkin @ 2014-03-04 2:02 ` Mimi Zohar 0 siblings, 0 replies; 31+ messages in thread From: Mimi Zohar @ 2014-03-04 2:02 UTC (permalink / raw) To: Dmitry Kasatkin Cc: linux-security-module, jmorris, linux-kernel, casey.schaufler, dmitry.kasatkin On Fri, 2014-02-28 at 16:59 +0200, Dmitry Kasatkin wrote: > Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com> Missing patch description ... Mimi > --- > security/integrity/evm/Kconfig | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/security/integrity/evm/Kconfig b/security/integrity/evm/Kconfig > index fea9749..5aa9103 100644 > --- a/security/integrity/evm/Kconfig > +++ b/security/integrity/evm/Kconfig > @@ -2,7 +2,6 @@ config EVM > boolean "EVM support" > depends on SECURITY && KEYS && (TRUSTED_KEYS=y || TRUSTED_KEYS=n) > select CRYPTO_HMAC > - select CRYPTO_MD5 > select CRYPTO_SHA1 > select ENCRYPTED_KEYS > default n ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 5/8] ima: skip memory allocation for empty files 2014-02-28 14:59 [PATCH 0/8] integrity: fixes and new features Dmitry Kasatkin ` (3 preceding siblings ...) 2014-02-28 14:59 ` [PATCH 4/8] evm: EVM does not use MD5 Dmitry Kasatkin @ 2014-02-28 14:59 ` Dmitry Kasatkin 2014-03-04 2:03 ` Mimi Zohar 2014-02-28 14:59 ` [PATCH 6/8] evm: enable key retention service automatically Dmitry Kasatkin ` (2 subsequent siblings) 7 siblings, 1 reply; 31+ messages in thread From: Dmitry Kasatkin @ 2014-02-28 14:59 UTC (permalink / raw) To: linux-security-module, zohar Cc: jmorris, linux-kernel, casey.schaufler, dmitry.kasatkin, Dmitry Kasatkin Memory allocation is unnecessary for empty files. This patch finalize the hash without memory allocation. Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com> --- security/integrity/ima/ima_crypto.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c index d257e36..1bde8e6 100644 --- a/security/integrity/ima/ima_crypto.c +++ b/security/integrity/ima/ima_crypto.c @@ -87,16 +87,20 @@ static int ima_calc_file_hash_tfm(struct file *file, if (rc != 0) return rc; - rbuf = kzalloc(PAGE_SIZE, GFP_KERNEL); - if (!rbuf) { - rc = -ENOMEM; + i_size = i_size_read(file_inode(file)); + + if (i_size == 0) goto out; - } + + rbuf = kzalloc(PAGE_SIZE, GFP_KERNEL); + if (!rbuf) + return -ENOMEM; + if (!(file->f_mode & FMODE_READ)) { file->f_mode |= FMODE_READ; read = 1; } - i_size = i_size_read(file_inode(file)); + while (offset < i_size) { int rbuf_len; @@ -113,12 +117,12 @@ static int ima_calc_file_hash_tfm(struct file *file, if (rc) break; } - kfree(rbuf); - if (!rc) - rc = crypto_shash_final(&desc.shash, hash->digest); if (read) file->f_mode &= ~FMODE_READ; + kfree(rbuf); out: + if (!rc) + rc = crypto_shash_final(&desc.shash, hash->digest); return rc; } -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH 5/8] ima: skip memory allocation for empty files 2014-02-28 14:59 ` [PATCH 5/8] ima: skip memory allocation for empty files Dmitry Kasatkin @ 2014-03-04 2:03 ` Mimi Zohar 0 siblings, 0 replies; 31+ messages in thread From: Mimi Zohar @ 2014-03-04 2:03 UTC (permalink / raw) To: Dmitry Kasatkin Cc: linux-security-module, jmorris, linux-kernel, casey.schaufler, dmitry.kasatkin On Fri, 2014-02-28 at 16:59 +0200, Dmitry Kasatkin wrote: > Memory allocation is unnecessary for empty files. > This patch finalize the hash without memory allocation. > > Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com> Thanks. Mimi > --- > security/integrity/ima/ima_crypto.c | 20 ++++++++++++-------- > 1 file changed, 12 insertions(+), 8 deletions(-) > > diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c > index d257e36..1bde8e6 100644 > --- a/security/integrity/ima/ima_crypto.c > +++ b/security/integrity/ima/ima_crypto.c > @@ -87,16 +87,20 @@ static int ima_calc_file_hash_tfm(struct file *file, > if (rc != 0) > return rc; > > - rbuf = kzalloc(PAGE_SIZE, GFP_KERNEL); > - if (!rbuf) { > - rc = -ENOMEM; > + i_size = i_size_read(file_inode(file)); > + > + if (i_size == 0) > goto out; > - } > + > + rbuf = kzalloc(PAGE_SIZE, GFP_KERNEL); > + if (!rbuf) > + return -ENOMEM; > + > if (!(file->f_mode & FMODE_READ)) { > file->f_mode |= FMODE_READ; > read = 1; > } > - i_size = i_size_read(file_inode(file)); > + > while (offset < i_size) { > int rbuf_len; > > @@ -113,12 +117,12 @@ static int ima_calc_file_hash_tfm(struct file *file, > if (rc) > break; > } > - kfree(rbuf); > - if (!rc) > - rc = crypto_shash_final(&desc.shash, hash->digest); > if (read) > file->f_mode &= ~FMODE_READ; > + kfree(rbuf); > out: > + if (!rc) > + rc = crypto_shash_final(&desc.shash, hash->digest); > return rc; > } > ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 6/8] evm: enable key retention service automatically 2014-02-28 14:59 [PATCH 0/8] integrity: fixes and new features Dmitry Kasatkin ` (4 preceding siblings ...) 2014-02-28 14:59 ` [PATCH 5/8] ima: skip memory allocation for empty files Dmitry Kasatkin @ 2014-02-28 14:59 ` Dmitry Kasatkin 2014-03-04 2:02 ` Mimi Zohar 2014-02-28 14:59 ` [PATCH 7/8] evm: introduce EVM hmac attribute list Dmitry Kasatkin 2014-02-28 14:59 ` [PATCH 8/8] evm: introduce EVM hmac xattr list Dmitry Kasatkin 7 siblings, 1 reply; 31+ messages in thread From: Dmitry Kasatkin @ 2014-02-28 14:59 UTC (permalink / raw) To: linux-security-module, zohar Cc: jmorris, linux-kernel, casey.schaufler, dmitry.kasatkin, Dmitry Kasatkin If keys are not enabled, EVM is not visible in the configuration menu. It may be difficult to figure out what to do unless you really know. Other subsystems as NFS, CIFS select keys automatically. This patch does the same. Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com> --- security/integrity/evm/Kconfig | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/security/integrity/evm/Kconfig b/security/integrity/evm/Kconfig index 5aa9103..d35b491 100644 --- a/security/integrity/evm/Kconfig +++ b/security/integrity/evm/Kconfig @@ -1,9 +1,10 @@ config EVM boolean "EVM support" - depends on SECURITY && KEYS && (TRUSTED_KEYS=y || TRUSTED_KEYS=n) + depends on SECURITY + select KEYS + select ENCRYPTED_KEYS select CRYPTO_HMAC select CRYPTO_SHA1 - select ENCRYPTED_KEYS default n help EVM protects a file's security extended attributes against -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH 6/8] evm: enable key retention service automatically 2014-02-28 14:59 ` [PATCH 6/8] evm: enable key retention service automatically Dmitry Kasatkin @ 2014-03-04 2:02 ` Mimi Zohar 2014-03-04 14:10 ` Dmitry Kasatkin 0 siblings, 1 reply; 31+ messages in thread From: Mimi Zohar @ 2014-03-04 2:02 UTC (permalink / raw) To: Dmitry Kasatkin Cc: linux-security-module, jmorris, linux-kernel, casey.schaufler, dmitry.kasatkin On Fri, 2014-02-28 at 16:59 +0200, Dmitry Kasatkin wrote: > If keys are not enabled, EVM is not visible in the configuration menu. > It may be difficult to figure out what to do unless you really know. > > Other subsystems as NFS, CIFS select keys automatically. > This patch does the same. > > Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com> > --- > security/integrity/evm/Kconfig | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/security/integrity/evm/Kconfig b/security/integrity/evm/Kconfig > index 5aa9103..d35b491 100644 > --- a/security/integrity/evm/Kconfig > +++ b/security/integrity/evm/Kconfig > @@ -1,9 +1,10 @@ > config EVM > boolean "EVM support" > - depends on SECURITY && KEYS && (TRUSTED_KEYS=y || TRUSTED_KEYS=n) Including KEYS is fine, but the trusted-keys dependency is still required. If trusted-keys is enabled, then the TPM and trusted-keys must be builtin. Mimi > + depends on SECURITY > + select KEYS > + select ENCRYPTED_KEYS > select CRYPTO_HMAC > select CRYPTO_SHA1 > - select ENCRYPTED_KEYS > default n > help > EVM protects a file's security extended attributes against ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 6/8] evm: enable key retention service automatically 2014-03-04 2:02 ` Mimi Zohar @ 2014-03-04 14:10 ` Dmitry Kasatkin 2014-03-04 19:25 ` Mimi Zohar 0 siblings, 1 reply; 31+ messages in thread From: Dmitry Kasatkin @ 2014-03-04 14:10 UTC (permalink / raw) To: Mimi Zohar Cc: Dmitry Kasatkin, linux-security-module, James Morris, linux-kernel@vger.kernel.org, casey.schaufler On Tue, Mar 4, 2014 at 4:02 AM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote: > On Fri, 2014-02-28 at 16:59 +0200, Dmitry Kasatkin wrote: >> If keys are not enabled, EVM is not visible in the configuration menu. >> It may be difficult to figure out what to do unless you really know. >> >> Other subsystems as NFS, CIFS select keys automatically. >> This patch does the same. >> >> Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com> >> --- >> security/integrity/evm/Kconfig | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/security/integrity/evm/Kconfig b/security/integrity/evm/Kconfig >> index 5aa9103..d35b491 100644 >> --- a/security/integrity/evm/Kconfig >> +++ b/security/integrity/evm/Kconfig >> @@ -1,9 +1,10 @@ >> config EVM >> boolean "EVM support" >> - depends on SECURITY && KEYS && (TRUSTED_KEYS=y || TRUSTED_KEYS=n) > > Including KEYS is fine, but the trusted-keys dependency is still > required. If trusted-keys is enabled, then the TPM and trusted-keys > must be builtin. > Hello. EVM does not depend on trusted keys, but on encrypted keys. There is no need for "&& (TRUSTED_KEYS=y || TRUSTED_KEYS=n)" Header file already provides all necessary compile time dependencies... ------------------------------------------------------------------------------------------------- #if defined(CONFIG_TRUSTED_KEYS) || \ (defined(CONFIG_TRUSTED_KEYS_MODULE) && defined(CONFIG_ENCRYPTED_KEYS_MODULE)) extern struct key *request_trusted_key(const char *trusted_desc, u8 **master_key, size_t *master_keylen); #else static inline struct key *request_trusted_key(const char *trusted_desc, u8 **master_key, size_t *master_keylen) { return ERR_PTR(-EOPNOTSUPP); } #endif ------------------------------------------------------------------------------------------------- - Dmitry > Mimi > >> + depends on SECURITY >> + select KEYS >> + select ENCRYPTED_KEYS >> select CRYPTO_HMAC >> select CRYPTO_SHA1 >> - select ENCRYPTED_KEYS >> default n >> help >> EVM protects a file's security extended attributes against > > -- Thanks, Dmitry ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 6/8] evm: enable key retention service automatically 2014-03-04 14:10 ` Dmitry Kasatkin @ 2014-03-04 19:25 ` Mimi Zohar 0 siblings, 0 replies; 31+ messages in thread From: Mimi Zohar @ 2014-03-04 19:25 UTC (permalink / raw) To: Dmitry Kasatkin Cc: Dmitry Kasatkin, linux-security-module, James Morris, linux-kernel@vger.kernel.org, casey.schaufler On Tue, 2014-03-04 at 16:10 +0200, Dmitry Kasatkin wrote: > On Tue, Mar 4, 2014 at 4:02 AM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote: > > On Fri, 2014-02-28 at 16:59 +0200, Dmitry Kasatkin wrote: > >> If keys are not enabled, EVM is not visible in the configuration menu. > >> It may be difficult to figure out what to do unless you really know. > >> > >> Other subsystems as NFS, CIFS select keys automatically. > >> This patch does the same. > >> > >> Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com> > >> --- > >> security/integrity/evm/Kconfig | 5 +++-- > >> 1 file changed, 3 insertions(+), 2 deletions(-) > >> > >> diff --git a/security/integrity/evm/Kconfig b/security/integrity/evm/Kconfig > >> index 5aa9103..d35b491 100644 > >> --- a/security/integrity/evm/Kconfig > >> +++ b/security/integrity/evm/Kconfig > >> @@ -1,9 +1,10 @@ > >> config EVM > >> boolean "EVM support" > >> - depends on SECURITY && KEYS && (TRUSTED_KEYS=y || TRUSTED_KEYS=n) > > > > Including KEYS is fine, but the trusted-keys dependency is still > > required. If trusted-keys is enabled, then the TPM and trusted-keys > > must be builtin. > > > > Hello. > > EVM does not depend on trusted keys, but on encrypted keys. > There is no need for "&& (TRUSTED_KEYS=y || TRUSTED_KEYS=n)" > > Header file already provides all necessary compile time dependencies... > > ------------------------------------------------------------------------------------------------- > #if defined(CONFIG_TRUSTED_KEYS) || \ > (defined(CONFIG_TRUSTED_KEYS_MODULE) && defined(CONFIG_ENCRYPTED_KEYS_MODULE)) > extern struct key *request_trusted_key(const char *trusted_desc, > u8 **master_key, size_t *master_keylen); > #else > static inline struct key *request_trusted_key(const char *trusted_desc, > u8 **master_key, > size_t *master_keylen) > { > return ERR_PTR(-EOPNOTSUPP); > } > #endif > ------------------------------------------------------------------------------------------------- Either in the patch description or the changelog, please note the reason for removing the builtin TRUSTED_KEY dependency. thanks, Mimi ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 7/8] evm: introduce EVM hmac attribute list 2014-02-28 14:59 [PATCH 0/8] integrity: fixes and new features Dmitry Kasatkin ` (5 preceding siblings ...) 2014-02-28 14:59 ` [PATCH 6/8] evm: enable key retention service automatically Dmitry Kasatkin @ 2014-02-28 14:59 ` Dmitry Kasatkin 2014-03-04 2:09 ` Mimi Zohar 2014-02-28 14:59 ` [PATCH 8/8] evm: introduce EVM hmac xattr list Dmitry Kasatkin 7 siblings, 1 reply; 31+ messages in thread From: Dmitry Kasatkin @ 2014-02-28 14:59 UTC (permalink / raw) To: linux-security-module, zohar Cc: jmorris, linux-kernel, casey.schaufler, dmitry.kasatkin, Dmitry Kasatkin This patch replaces using of hmac version configuration parameter with attribute list. It allows to build kernels which works with previously labeled filesystems. Currently supported attribute is 'fsuuid' which is equivalent of former version 2. Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com> --- security/integrity/evm/Kconfig | 19 ++++++++++--------- security/integrity/evm/evm.h | 4 +++- security/integrity/evm/evm_crypto.c | 2 +- security/integrity/evm/evm_main.c | 21 ++++++++++++++++++++- 4 files changed, 34 insertions(+), 12 deletions(-) diff --git a/security/integrity/evm/Kconfig b/security/integrity/evm/Kconfig index d35b491..2be51fa 100644 --- a/security/integrity/evm/Kconfig +++ b/security/integrity/evm/Kconfig @@ -12,15 +12,16 @@ config EVM If you are unsure how to answer this question, answer N. -config EVM_HMAC_VERSION - int "EVM HMAC version" - depends on EVM - default 2 - help - This options adds EVM HMAC version support. - 1 - original version - 2 - add per filesystem unique identifier (UUID) (default) +config EVM_HMAC_ATTRS + string "HMAC attributes" + default "fsuuid" + help + This options allows to specify list of optional attributes included into HMAC + calculation. It makes it possible easily upgrade to newer kernels. + + Default value is 'fsuuid', which is former version 2. + if blank, it is equivalent of version 1 WARNING: changing the HMAC calculation method or adding additional info to the calculation, requires existing EVM - labeled file systems to be relabeled. + labeled file systems to be relabeled. diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h index 37c88dd..c8fa0aa 100644 --- a/security/integrity/evm/evm.h +++ b/security/integrity/evm/evm.h @@ -24,11 +24,13 @@ extern int evm_initialized; extern char *evm_hmac; extern char *evm_hash; -extern int evm_hmac_version; +extern int evm_hmac_attrs; extern struct crypto_shash *hmac_tfm; extern struct crypto_shash *hash_tfm; +#define EVM_HMAC_ATTR_FSUUID 0x0001 + /* List of EVM protected security xattrs */ extern char *evm_config_xattrnames[]; diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c index babd862..ab034e5 100644 --- a/security/integrity/evm/evm_crypto.c +++ b/security/integrity/evm/evm_crypto.c @@ -112,7 +112,7 @@ static void hmac_add_misc(struct shash_desc *desc, struct inode *inode, hmac_misc.gid = from_kgid(&init_user_ns, inode->i_gid); hmac_misc.mode = inode->i_mode; crypto_shash_update(desc, (const u8 *)&hmac_misc, sizeof(hmac_misc)); - if (evm_hmac_version > 1) + if (evm_hmac_attrs & EVM_HMAC_ATTR_FSUUID) crypto_shash_update(desc, inode->i_sb->s_uuid, sizeof(inode->i_sb->s_uuid)); crypto_shash_final(desc, digest); diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index 996092f..9c05929 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -32,7 +32,7 @@ static char *integrity_status_msg[] = { }; char *evm_hmac = "hmac(sha1)"; char *evm_hash = "sha1"; -int evm_hmac_version = CONFIG_EVM_HMAC_VERSION; +int evm_hmac_attrs; char *evm_config_xattrnames[] = { #ifdef CONFIG_SECURITY_SELINUX @@ -57,6 +57,19 @@ static int __init evm_set_fixmode(char *str) } __setup("evm=", evm_set_fixmode); +static int __init evm_init_config(void) +{ + char *attrs = CONFIG_EVM_HMAC_ATTRS; + char *p; + + while ((p = strsep(&attrs, ", \t"))) { + if (!strcmp(p, "fsuuid")) + evm_hmac_attrs |= EVM_HMAC_ATTR_FSUUID; + } + pr_info("HMAC attrs: 0x%x\n", evm_hmac_attrs); + return 0; +} + static int evm_find_protected_xattrs(struct dentry *dentry) { struct inode *inode = dentry->d_inode; @@ -432,6 +445,12 @@ static int __init init_evm(void) { int error; + error = evm_init_config(); + if (error < 0) { + pr_info("Error parsing config lists\n"); + goto err; + } + error = evm_init_secfs(); if (error < 0) { pr_info("Error registering secfs\n"); -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH 7/8] evm: introduce EVM hmac attribute list 2014-02-28 14:59 ` [PATCH 7/8] evm: introduce EVM hmac attribute list Dmitry Kasatkin @ 2014-03-04 2:09 ` Mimi Zohar 2014-03-04 14:20 ` Dmitry Kasatkin 0 siblings, 1 reply; 31+ messages in thread From: Mimi Zohar @ 2014-03-04 2:09 UTC (permalink / raw) To: Dmitry Kasatkin Cc: linux-security-module, jmorris, linux-kernel, casey.schaufler, dmitry.kasatkin On Fri, 2014-02-28 at 16:59 +0200, Dmitry Kasatkin wrote: > This patch replaces using of hmac version configuration parameter > with attribute list. It allows to build kernels which works with > previously labeled filesystems. > > Currently supported attribute is 'fsuuid' which is equivalent of > former version 2. > > Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com> Please include the new boot command line option in Documentation/kernel-parameters.txt. Mimi > --- > security/integrity/evm/Kconfig | 19 ++++++++++--------- > security/integrity/evm/evm.h | 4 +++- > security/integrity/evm/evm_crypto.c | 2 +- > security/integrity/evm/evm_main.c | 21 ++++++++++++++++++++- > 4 files changed, 34 insertions(+), 12 deletions(-) > > diff --git a/security/integrity/evm/Kconfig b/security/integrity/evm/Kconfig > index d35b491..2be51fa 100644 > --- a/security/integrity/evm/Kconfig > +++ b/security/integrity/evm/Kconfig > @@ -12,15 +12,16 @@ config EVM > > If you are unsure how to answer this question, answer N. > > -config EVM_HMAC_VERSION > - int "EVM HMAC version" > - depends on EVM > - default 2 > - help > - This options adds EVM HMAC version support. > - 1 - original version > - 2 - add per filesystem unique identifier (UUID) (default) > +config EVM_HMAC_ATTRS > + string "HMAC attributes" > + default "fsuuid" > + help > + This options allows to specify list of optional attributes included into HMAC > + calculation. It makes it possible easily upgrade to newer kernels. > + > + Default value is 'fsuuid', which is former version 2. > + if blank, it is equivalent of version 1 > > WARNING: changing the HMAC calculation method or adding > additional info to the calculation, requires existing EVM > - labeled file systems to be relabeled. > + labeled file systems to be relabeled. > diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h > index 37c88dd..c8fa0aa 100644 > --- a/security/integrity/evm/evm.h > +++ b/security/integrity/evm/evm.h > @@ -24,11 +24,13 @@ > extern int evm_initialized; > extern char *evm_hmac; > extern char *evm_hash; > -extern int evm_hmac_version; > +extern int evm_hmac_attrs; > > extern struct crypto_shash *hmac_tfm; > extern struct crypto_shash *hash_tfm; > > +#define EVM_HMAC_ATTR_FSUUID 0x0001 > + > /* List of EVM protected security xattrs */ > extern char *evm_config_xattrnames[]; > > diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c > index babd862..ab034e5 100644 > --- a/security/integrity/evm/evm_crypto.c > +++ b/security/integrity/evm/evm_crypto.c > @@ -112,7 +112,7 @@ static void hmac_add_misc(struct shash_desc *desc, struct inode *inode, > hmac_misc.gid = from_kgid(&init_user_ns, inode->i_gid); > hmac_misc.mode = inode->i_mode; > crypto_shash_update(desc, (const u8 *)&hmac_misc, sizeof(hmac_misc)); > - if (evm_hmac_version > 1) > + if (evm_hmac_attrs & EVM_HMAC_ATTR_FSUUID) > crypto_shash_update(desc, inode->i_sb->s_uuid, > sizeof(inode->i_sb->s_uuid)); > crypto_shash_final(desc, digest); > diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c > index 996092f..9c05929 100644 > --- a/security/integrity/evm/evm_main.c > +++ b/security/integrity/evm/evm_main.c > @@ -32,7 +32,7 @@ static char *integrity_status_msg[] = { > }; > char *evm_hmac = "hmac(sha1)"; > char *evm_hash = "sha1"; > -int evm_hmac_version = CONFIG_EVM_HMAC_VERSION; > +int evm_hmac_attrs; > > char *evm_config_xattrnames[] = { > #ifdef CONFIG_SECURITY_SELINUX > @@ -57,6 +57,19 @@ static int __init evm_set_fixmode(char *str) > } > __setup("evm=", evm_set_fixmode); > > +static int __init evm_init_config(void) > +{ > + char *attrs = CONFIG_EVM_HMAC_ATTRS; > + char *p; > + > + while ((p = strsep(&attrs, ", \t"))) { > + if (!strcmp(p, "fsuuid")) > + evm_hmac_attrs |= EVM_HMAC_ATTR_FSUUID; > + } > + pr_info("HMAC attrs: 0x%x\n", evm_hmac_attrs); > + return 0; > +} > + > static int evm_find_protected_xattrs(struct dentry *dentry) > { > struct inode *inode = dentry->d_inode; > @@ -432,6 +445,12 @@ static int __init init_evm(void) > { > int error; > > + error = evm_init_config(); > + if (error < 0) { > + pr_info("Error parsing config lists\n"); > + goto err; > + } > + > error = evm_init_secfs(); > if (error < 0) { > pr_info("Error registering secfs\n"); ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 7/8] evm: introduce EVM hmac attribute list 2014-03-04 2:09 ` Mimi Zohar @ 2014-03-04 14:20 ` Dmitry Kasatkin 0 siblings, 0 replies; 31+ messages in thread From: Dmitry Kasatkin @ 2014-03-04 14:20 UTC (permalink / raw) To: Mimi Zohar Cc: Dmitry Kasatkin, linux-security-module, James Morris, linux-kernel@vger.kernel.org, casey.schaufler On Tue, Mar 4, 2014 at 4:09 AM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote: > On Fri, 2014-02-28 at 16:59 +0200, Dmitry Kasatkin wrote: >> This patch replaces using of hmac version configuration parameter >> with attribute list. It allows to build kernels which works with >> previously labeled filesystems. >> >> Currently supported attribute is 'fsuuid' which is equivalent of >> former version 2. >> >> Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com> > > Please include the new boot command line option in > Documentation/kernel-parameters.txt. > There is no kernel parameter, but configuration parameter.. Again for flexibility to add more parameters and be able to use new kernel on existing labeled filesystems. Kernel command line option can be added on the future. - Dmitry > Mimi > >> --- >> security/integrity/evm/Kconfig | 19 ++++++++++--------- >> security/integrity/evm/evm.h | 4 +++- >> security/integrity/evm/evm_crypto.c | 2 +- >> security/integrity/evm/evm_main.c | 21 ++++++++++++++++++++- >> 4 files changed, 34 insertions(+), 12 deletions(-) >> >> diff --git a/security/integrity/evm/Kconfig b/security/integrity/evm/Kconfig >> index d35b491..2be51fa 100644 >> --- a/security/integrity/evm/Kconfig >> +++ b/security/integrity/evm/Kconfig >> @@ -12,15 +12,16 @@ config EVM >> >> If you are unsure how to answer this question, answer N. >> >> -config EVM_HMAC_VERSION >> - int "EVM HMAC version" >> - depends on EVM >> - default 2 >> - help >> - This options adds EVM HMAC version support. >> - 1 - original version >> - 2 - add per filesystem unique identifier (UUID) (default) >> +config EVM_HMAC_ATTRS >> + string "HMAC attributes" >> + default "fsuuid" >> + help >> + This options allows to specify list of optional attributes included into HMAC >> + calculation. It makes it possible easily upgrade to newer kernels. >> + >> + Default value is 'fsuuid', which is former version 2. >> + if blank, it is equivalent of version 1 >> >> WARNING: changing the HMAC calculation method or adding >> additional info to the calculation, requires existing EVM >> - labeled file systems to be relabeled. >> + labeled file systems to be relabeled. >> diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h >> index 37c88dd..c8fa0aa 100644 >> --- a/security/integrity/evm/evm.h >> +++ b/security/integrity/evm/evm.h >> @@ -24,11 +24,13 @@ >> extern int evm_initialized; >> extern char *evm_hmac; >> extern char *evm_hash; >> -extern int evm_hmac_version; >> +extern int evm_hmac_attrs; >> >> extern struct crypto_shash *hmac_tfm; >> extern struct crypto_shash *hash_tfm; >> >> +#define EVM_HMAC_ATTR_FSUUID 0x0001 >> + >> /* List of EVM protected security xattrs */ >> extern char *evm_config_xattrnames[]; >> >> diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c >> index babd862..ab034e5 100644 >> --- a/security/integrity/evm/evm_crypto.c >> +++ b/security/integrity/evm/evm_crypto.c >> @@ -112,7 +112,7 @@ static void hmac_add_misc(struct shash_desc *desc, struct inode *inode, >> hmac_misc.gid = from_kgid(&init_user_ns, inode->i_gid); >> hmac_misc.mode = inode->i_mode; >> crypto_shash_update(desc, (const u8 *)&hmac_misc, sizeof(hmac_misc)); >> - if (evm_hmac_version > 1) >> + if (evm_hmac_attrs & EVM_HMAC_ATTR_FSUUID) >> crypto_shash_update(desc, inode->i_sb->s_uuid, >> sizeof(inode->i_sb->s_uuid)); >> crypto_shash_final(desc, digest); >> diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c >> index 996092f..9c05929 100644 >> --- a/security/integrity/evm/evm_main.c >> +++ b/security/integrity/evm/evm_main.c >> @@ -32,7 +32,7 @@ static char *integrity_status_msg[] = { >> }; >> char *evm_hmac = "hmac(sha1)"; >> char *evm_hash = "sha1"; >> -int evm_hmac_version = CONFIG_EVM_HMAC_VERSION; >> +int evm_hmac_attrs; >> >> char *evm_config_xattrnames[] = { >> #ifdef CONFIG_SECURITY_SELINUX >> @@ -57,6 +57,19 @@ static int __init evm_set_fixmode(char *str) >> } >> __setup("evm=", evm_set_fixmode); >> >> +static int __init evm_init_config(void) >> +{ >> + char *attrs = CONFIG_EVM_HMAC_ATTRS; >> + char *p; >> + >> + while ((p = strsep(&attrs, ", \t"))) { >> + if (!strcmp(p, "fsuuid")) >> + evm_hmac_attrs |= EVM_HMAC_ATTR_FSUUID; >> + } >> + pr_info("HMAC attrs: 0x%x\n", evm_hmac_attrs); >> + return 0; >> +} >> + >> static int evm_find_protected_xattrs(struct dentry *dentry) >> { >> struct inode *inode = dentry->d_inode; >> @@ -432,6 +445,12 @@ static int __init init_evm(void) >> { >> int error; >> >> + error = evm_init_config(); >> + if (error < 0) { >> + pr_info("Error parsing config lists\n"); >> + goto err; >> + } >> + >> error = evm_init_secfs(); >> if (error < 0) { >> pr_info("Error registering secfs\n"); > > -- Thanks, Dmitry ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 8/8] evm: introduce EVM hmac xattr list 2014-02-28 14:59 [PATCH 0/8] integrity: fixes and new features Dmitry Kasatkin ` (6 preceding siblings ...) 2014-02-28 14:59 ` [PATCH 7/8] evm: introduce EVM hmac attribute list Dmitry Kasatkin @ 2014-02-28 14:59 ` Dmitry Kasatkin 2014-03-04 2:39 ` Mimi Zohar 7 siblings, 1 reply; 31+ messages in thread From: Dmitry Kasatkin @ 2014-02-28 14:59 UTC (permalink / raw) To: linux-security-module, zohar Cc: jmorris, linux-kernel, casey.schaufler, dmitry.kasatkin, Dmitry Kasatkin EVM currently uses source hard coded list of xattrs which needs to be included into the HMAC calculation. This is very unflexible. Adding new attributes requires modifcation of the source code and prevents building the kernel which works with previously labeled filesystems. Early versions of Smack used only one xattr security.SMACK64, which is protected by EVM. Now Smack has a few more attributes and they are not protected. Adding support to the source code makes it impossible to use new kernel with previousely labeled filesystems. This patch replaces hardcoded xattr array with dynamic list which is initialized from CONFIG_EVM_HMAC_XATTRS variable. It allows to build kernel with with support of older and newer EVM HMAC formats. Possible future extension will be to read xattr list from the kernel command line or from securityfs entry. Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com> --- security/integrity/evm/Kconfig | 10 ++++++ security/integrity/evm/evm.h | 7 +++- security/integrity/evm/evm_crypto.c | 8 ++--- security/integrity/evm/evm_main.c | 69 +++++++++++++++++++------------------ 4 files changed, 55 insertions(+), 39 deletions(-) diff --git a/security/integrity/evm/Kconfig b/security/integrity/evm/Kconfig index 2be51fa..06237b8 100644 --- a/security/integrity/evm/Kconfig +++ b/security/integrity/evm/Kconfig @@ -25,3 +25,13 @@ config EVM_HMAC_ATTRS WARNING: changing the HMAC calculation method or adding additional info to the calculation, requires existing EVM labeled file systems to be relabeled. + +config EVM_HMAC_XATTRS + string "HMAC xattrs" + default "security.selinux security.SMACK64 security.ima security.capability" + help + This options allows to specify list of extended attributes included into HMAC + calculation. It makes it possible easily upgrade to newer kernels. + + Default value: + security.selinux, security.SMACK64, security.ima, security.capability diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h index c8fa0aa..4d1c51e 100644 --- a/security/integrity/evm/evm.h +++ b/security/integrity/evm/evm.h @@ -31,8 +31,13 @@ extern struct crypto_shash *hash_tfm; #define EVM_HMAC_ATTR_FSUUID 0x0001 +struct evm_hmac_xattr { + struct list_head list; + char *xattr; +}; + /* List of EVM protected security xattrs */ -extern char *evm_config_xattrnames[]; +extern struct list_head evm_hmac_xattrs; int evm_init_key(void); int evm_update_evmxattr(struct dentry *dentry, diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c index ab034e5..7e5bfb7 100644 --- a/security/integrity/evm/evm_crypto.c +++ b/security/integrity/evm/evm_crypto.c @@ -133,7 +133,7 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry, { struct inode *inode = dentry->d_inode; struct shash_desc *desc; - char **xattrname; + struct evm_hmac_xattr *entry; size_t xattr_size = 0; char *xattr_value = NULL; int error; @@ -146,15 +146,15 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry, return PTR_ERR(desc); error = -ENODATA; - for (xattrname = evm_config_xattrnames; *xattrname != NULL; xattrname++) { + list_for_each_entry(entry, &evm_hmac_xattrs, list) { if ((req_xattr_name && req_xattr_value) - && !strcmp(*xattrname, req_xattr_name)) { + && !strcmp(entry->xattr, req_xattr_name)) { error = 0; crypto_shash_update(desc, (const u8 *)req_xattr_value, req_xattr_value_len); continue; } - size = vfs_getxattr_alloc(dentry, *xattrname, + size = vfs_getxattr_alloc(dentry, entry->xattr, &xattr_value, xattr_size, GFP_NOFS); if (size == -ENOMEM) { error = -ENOMEM; diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c index 9c05929..13e03ad 100644 --- a/security/integrity/evm/evm_main.c +++ b/security/integrity/evm/evm_main.c @@ -34,19 +34,7 @@ char *evm_hmac = "hmac(sha1)"; char *evm_hash = "sha1"; int evm_hmac_attrs; -char *evm_config_xattrnames[] = { -#ifdef CONFIG_SECURITY_SELINUX - XATTR_NAME_SELINUX, -#endif -#ifdef CONFIG_SECURITY_SMACK - XATTR_NAME_SMACK, -#endif -#ifdef CONFIG_IMA_APPRAISE - XATTR_NAME_IMA, -#endif - XATTR_NAME_CAPS, - NULL -}; +LIST_HEAD(evm_hmac_xattrs); static int evm_fixmode; static int __init evm_set_fixmode(char *str) @@ -61,27 +49,53 @@ static int __init evm_init_config(void) { char *attrs = CONFIG_EVM_HMAC_ATTRS; char *p; + char *xattrs = CONFIG_EVM_HMAC_XATTRS; + struct evm_hmac_xattr *entry; while ((p = strsep(&attrs, ", \t"))) { if (!strcmp(p, "fsuuid")) evm_hmac_attrs |= EVM_HMAC_ATTR_FSUUID; } pr_info("HMAC attrs: 0x%x\n", evm_hmac_attrs); + while ((p = strsep(&xattrs, ", \t"))) { +#ifndef CONFIG_SECURITY_SELINUX + if (!strcmp(p, XATTR_NAME_SELINUX)) + continue; +#endif +#ifndef CONFIG_SECURITY_SMACK + if (strstr(p, "SMACK64")) + continue; +#endif +#ifndef CONFIG_IMA_APPRAISE + if (!strcmp(p, XATTR_NAME_IMA)) + continue; +#endif + entry = kmalloc(sizeof(*entry), GFP_KERNEL); + if (!entry) + return -ENOMEM; + INIT_LIST_HEAD(&entry->list); + entry->xattr = kstrdup(p, GFP_KERNEL); + if (!entry->xattr) + return -ENOMEM; + list_add_tail(&entry->list, &evm_hmac_xattrs); + } + list_for_each_entry(entry, &evm_hmac_xattrs, list) + pr_info("%s\n", entry->xattr); return 0; } static int evm_find_protected_xattrs(struct dentry *dentry) { struct inode *inode = dentry->d_inode; - char **xattr; + struct evm_hmac_xattr *entry; int error; int count = 0; if (!inode->i_op || !inode->i_op->getxattr) return -EOPNOTSUPP; - for (xattr = evm_config_xattrnames; *xattr != NULL; xattr++) { - error = inode->i_op->getxattr(dentry, *xattr, NULL, 0); + list_for_each_entry(entry, &evm_hmac_xattrs, list) { + error = inode->i_op->getxattr(dentry, entry->xattr, NULL, 0); if (error < 0) { if (error == -ENODATA) continue; @@ -183,19 +197,19 @@ out: static int evm_protected_xattr(const char *req_xattr_name) { - char **xattrname; + struct evm_hmac_xattr *entry; int namelen; int found = 0; namelen = strlen(req_xattr_name); - for (xattrname = evm_config_xattrnames; *xattrname != NULL; xattrname++) { - if ((strlen(*xattrname) == namelen) - && (strncmp(req_xattr_name, *xattrname, namelen) == 0)) { + list_for_each_entry(entry, &evm_hmac_xattrs, list) { + if ((strlen(entry->xattr) == namelen) + && (strncmp(req_xattr_name, entry->xattr, namelen) == 0)) { found = 1; break; } if (strncmp(req_xattr_name, - *xattrname + XATTR_SECURITY_PREFIX_LEN, + entry->xattr + XATTR_SECURITY_PREFIX_LEN, strlen(req_xattr_name)) == 0) { found = 1; break; @@ -462,19 +476,6 @@ err: return error; } -/* - * evm_display_config - list the EVM protected security extended attributes - */ -static int __init evm_display_config(void) -{ - char **xattrname; - - for (xattrname = evm_config_xattrnames; *xattrname != NULL; xattrname++) - pr_info("%s\n", *xattrname); - return 0; -} - -pure_initcall(evm_display_config); late_initcall(init_evm); MODULE_DESCRIPTION("Extended Verification Module"); -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH 8/8] evm: introduce EVM hmac xattr list 2014-02-28 14:59 ` [PATCH 8/8] evm: introduce EVM hmac xattr list Dmitry Kasatkin @ 2014-03-04 2:39 ` Mimi Zohar 2014-03-04 3:00 ` Casey Schaufler 0 siblings, 1 reply; 31+ messages in thread From: Mimi Zohar @ 2014-03-04 2:39 UTC (permalink / raw) To: Dmitry Kasatkin Cc: linux-security-module, jmorris, linux-kernel, casey.schaufler, dmitry.kasatkin On Fri, 2014-02-28 at 16:59 +0200, Dmitry Kasatkin wrote: > EVM currently uses source hard coded list of xattrs which needs to be > included into the HMAC calculation. This is very unflexible. > Adding new attributes requires modifcation of the source code and > prevents building the kernel which works with previously labeled > filesystems. > > Early versions of Smack used only one xattr security.SMACK64, > which is protected by EVM. Now Smack has a few more attributes and > they are not protected. Adding support to the source code makes it > impossible to use new kernel with previousely labeled filesystems. I think this patch will break any existing filesystems labeled with 'security.smack64'. Details inline. > This patch replaces hardcoded xattr array with dynamic list which is > initialized from CONFIG_EVM_HMAC_XATTRS variable. It allows to build > kernel with with support of older and newer EVM HMAC formats. > > Possible future extension will be to read xattr list from the kernel > command line or from securityfs entry. > > Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com> > --- > security/integrity/evm/Kconfig | 10 ++++++ > security/integrity/evm/evm.h | 7 +++- > security/integrity/evm/evm_crypto.c | 8 ++--- > security/integrity/evm/evm_main.c | 69 +++++++++++++++++++------------------ > 4 files changed, 55 insertions(+), 39 deletions(-) > > diff --git a/security/integrity/evm/Kconfig b/security/integrity/evm/Kconfig > index 2be51fa..06237b8 100644 > --- a/security/integrity/evm/Kconfig > +++ b/security/integrity/evm/Kconfig > @@ -25,3 +25,13 @@ config EVM_HMAC_ATTRS > WARNING: changing the HMAC calculation method or adding > additional info to the calculation, requires existing EVM > labeled file systems to be relabeled. > + > +config EVM_HMAC_XATTRS > + string "HMAC xattrs" > + default "security.selinux security.SMACK64 security.ima security.capability" > + help > + This options allows to specify list of extended attributes included into HMAC > + calculation. It makes it possible easily upgrade to newer kernels. > + > + Default value: > + security.selinux, security.SMACK64, security.ima, security.capability > diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h > index c8fa0aa..4d1c51e 100644 > --- a/security/integrity/evm/evm.h > +++ b/security/integrity/evm/evm.h > @@ -31,8 +31,13 @@ extern struct crypto_shash *hash_tfm; > > #define EVM_HMAC_ATTR_FSUUID 0x0001 > > +struct evm_hmac_xattr { > + struct list_head list; > + char *xattr; > +}; > + > /* List of EVM protected security xattrs */ > -extern char *evm_config_xattrnames[]; > +extern struct list_head evm_hmac_xattrs; > > int evm_init_key(void); > int evm_update_evmxattr(struct dentry *dentry, > diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c > index ab034e5..7e5bfb7 100644 > --- a/security/integrity/evm/evm_crypto.c > +++ b/security/integrity/evm/evm_crypto.c > @@ -133,7 +133,7 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry, > { > struct inode *inode = dentry->d_inode; > struct shash_desc *desc; > - char **xattrname; > + struct evm_hmac_xattr *entry; > size_t xattr_size = 0; > char *xattr_value = NULL; > int error; > @@ -146,15 +146,15 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry, > return PTR_ERR(desc); > > error = -ENODATA; > - for (xattrname = evm_config_xattrnames; *xattrname != NULL; xattrname++) { > + list_for_each_entry(entry, &evm_hmac_xattrs, list) { > if ((req_xattr_name && req_xattr_value) > - && !strcmp(*xattrname, req_xattr_name)) { > + && !strcmp(entry->xattr, req_xattr_name)) { > error = 0; > crypto_shash_update(desc, (const u8 *)req_xattr_value, > req_xattr_value_len); > continue; > } > - size = vfs_getxattr_alloc(dentry, *xattrname, > + size = vfs_getxattr_alloc(dentry, entry->xattr, > &xattr_value, xattr_size, GFP_NOFS); > if (size == -ENOMEM) { > error = -ENOMEM; > diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c > index 9c05929..13e03ad 100644 > --- a/security/integrity/evm/evm_main.c > +++ b/security/integrity/evm/evm_main.c > @@ -34,19 +34,7 @@ char *evm_hmac = "hmac(sha1)"; > char *evm_hash = "sha1"; > int evm_hmac_attrs; > > -char *evm_config_xattrnames[] = { > -#ifdef CONFIG_SECURITY_SELINUX > - XATTR_NAME_SELINUX, > -#endif > -#ifdef CONFIG_SECURITY_SMACK > - XATTR_NAME_SMACK, > -#endif > -#ifdef CONFIG_IMA_APPRAISE > - XATTR_NAME_IMA, > -#endif > - XATTR_NAME_CAPS, > - NULL > -}; > +LIST_HEAD(evm_hmac_xattrs); > > static int evm_fixmode; > static int __init evm_set_fixmode(char *str) > @@ -61,27 +49,53 @@ static int __init evm_init_config(void) > { > char *attrs = CONFIG_EVM_HMAC_ATTRS; > char *p; > + char *xattrs = CONFIG_EVM_HMAC_XATTRS; > + struct evm_hmac_xattr *entry; > > while ((p = strsep(&attrs, ", \t"))) { > if (!strcmp(p, "fsuuid")) > evm_hmac_attrs |= EVM_HMAC_ATTR_FSUUID; > } > pr_info("HMAC attrs: 0x%x\n", evm_hmac_attrs); > + while ((p = strsep(&xattrs, ", \t"))) { > +#ifndef CONFIG_SECURITY_SELINUX > + if (!strcmp(p, XATTR_NAME_SELINUX)) > + continue; > +#endif > +#ifndef CONFIG_SECURITY_SMACK > + if (strstr(p, "SMACK64")) > + continue; > +#endif As you mentioned, filesystems previously only included 'security.smack64' in the HMAC calculation. This patch includes all xattrs prefixed with smack64. All previously labeled filesystems would need to be relabeled. Mimi > +#ifndef CONFIG_IMA_APPRAISE > + if (!strcmp(p, XATTR_NAME_IMA)) > + continue; > +#endif > + entry = kmalloc(sizeof(*entry), GFP_KERNEL); > + if (!entry) > + return -ENOMEM; > + INIT_LIST_HEAD(&entry->list); > + entry->xattr = kstrdup(p, GFP_KERNEL); > + if (!entry->xattr) > + return -ENOMEM; > + list_add_tail(&entry->list, &evm_hmac_xattrs); > + } > + list_for_each_entry(entry, &evm_hmac_xattrs, list) > + pr_info("%s\n", entry->xattr); > return 0; > } > > static int evm_find_protected_xattrs(struct dentry *dentry) > { > struct inode *inode = dentry->d_inode; > - char **xattr; > + struct evm_hmac_xattr *entry; > int error; > int count = 0; > > if (!inode->i_op || !inode->i_op->getxattr) > return -EOPNOTSUPP; > > - for (xattr = evm_config_xattrnames; *xattr != NULL; xattr++) { > - error = inode->i_op->getxattr(dentry, *xattr, NULL, 0); > + list_for_each_entry(entry, &evm_hmac_xattrs, list) { > + error = inode->i_op->getxattr(dentry, entry->xattr, NULL, 0); > if (error < 0) { > if (error == -ENODATA) > continue; > @@ -183,19 +197,19 @@ out: > > static int evm_protected_xattr(const char *req_xattr_name) > { > - char **xattrname; > + struct evm_hmac_xattr *entry; > int namelen; > int found = 0; > > namelen = strlen(req_xattr_name); > - for (xattrname = evm_config_xattrnames; *xattrname != NULL; xattrname++) { > - if ((strlen(*xattrname) == namelen) > - && (strncmp(req_xattr_name, *xattrname, namelen) == 0)) { > + list_for_each_entry(entry, &evm_hmac_xattrs, list) { > + if ((strlen(entry->xattr) == namelen) > + && (strncmp(req_xattr_name, entry->xattr, namelen) == 0)) { > found = 1; > break; > } > if (strncmp(req_xattr_name, > - *xattrname + XATTR_SECURITY_PREFIX_LEN, > + entry->xattr + XATTR_SECURITY_PREFIX_LEN, > strlen(req_xattr_name)) == 0) { > found = 1; > break; > @@ -462,19 +476,6 @@ err: > return error; > } > > -/* > - * evm_display_config - list the EVM protected security extended attributes > - */ > -static int __init evm_display_config(void) > -{ > - char **xattrname; > - > - for (xattrname = evm_config_xattrnames; *xattrname != NULL; xattrname++) > - pr_info("%s\n", *xattrname); > - return 0; > -} > - > -pure_initcall(evm_display_config); > late_initcall(init_evm); > > MODULE_DESCRIPTION("Extended Verification Module"); ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 8/8] evm: introduce EVM hmac xattr list 2014-03-04 2:39 ` Mimi Zohar @ 2014-03-04 3:00 ` Casey Schaufler 2014-03-04 3:21 ` Mimi Zohar 0 siblings, 1 reply; 31+ messages in thread From: Casey Schaufler @ 2014-03-04 3:00 UTC (permalink / raw) To: Mimi Zohar, Dmitry Kasatkin Cc: linux-security-module, jmorris, linux-kernel, Casey Schaufler, dmitry.kasatkin On 3/3/2014 6:39 PM, Mimi Zohar wrote: > On Fri, 2014-02-28 at 16:59 +0200, Dmitry Kasatkin wrote: >> EVM currently uses source hard coded list of xattrs which needs to be >> included into the HMAC calculation. This is very unflexible. >> Adding new attributes requires modifcation of the source code and >> prevents building the kernel which works with previously labeled >> filesystems. >> >> Early versions of Smack used only one xattr security.SMACK64, >> which is protected by EVM. Now Smack has a few more attributes and >> they are not protected. Adding support to the source code makes it >> impossible to use new kernel with previousely labeled filesystems. > I think this patch will break any existing filesystems labeled with > 'security.smack64'. Details inline. > >> This patch replaces hardcoded xattr array with dynamic list which is >> initialized from CONFIG_EVM_HMAC_XATTRS variable. It allows to build >> kernel with with support of older and newer EVM HMAC formats. >> >> Possible future extension will be to read xattr list from the kernel >> command line or from securityfs entry. >> >> Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com> >> --- >> security/integrity/evm/Kconfig | 10 ++++++ >> security/integrity/evm/evm.h | 7 +++- >> security/integrity/evm/evm_crypto.c | 8 ++--- >> security/integrity/evm/evm_main.c | 69 +++++++++++++++++++------------------ >> 4 files changed, 55 insertions(+), 39 deletions(-) >> >> diff --git a/security/integrity/evm/Kconfig b/security/integrity/evm/Kconfig >> index 2be51fa..06237b8 100644 >> --- a/security/integrity/evm/Kconfig >> +++ b/security/integrity/evm/Kconfig >> @@ -25,3 +25,13 @@ config EVM_HMAC_ATTRS >> WARNING: changing the HMAC calculation method or adding >> additional info to the calculation, requires existing EVM >> labeled file systems to be relabeled. >> + >> +config EVM_HMAC_XATTRS >> + string "HMAC xattrs" >> + default "security.selinux security.SMACK64 security.ima security.capability" >> + help >> + This options allows to specify list of extended attributes included into HMAC >> + calculation. It makes it possible easily upgrade to newer kernels. >> + >> + Default value: >> + security.selinux, security.SMACK64, security.ima, security.capability >> diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h >> index c8fa0aa..4d1c51e 100644 >> --- a/security/integrity/evm/evm.h >> +++ b/security/integrity/evm/evm.h >> @@ -31,8 +31,13 @@ extern struct crypto_shash *hash_tfm; >> >> #define EVM_HMAC_ATTR_FSUUID 0x0001 >> >> +struct evm_hmac_xattr { >> + struct list_head list; >> + char *xattr; >> +}; >> + >> /* List of EVM protected security xattrs */ >> -extern char *evm_config_xattrnames[]; >> +extern struct list_head evm_hmac_xattrs; >> >> int evm_init_key(void); >> int evm_update_evmxattr(struct dentry *dentry, >> diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c >> index ab034e5..7e5bfb7 100644 >> --- a/security/integrity/evm/evm_crypto.c >> +++ b/security/integrity/evm/evm_crypto.c >> @@ -133,7 +133,7 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry, >> { >> struct inode *inode = dentry->d_inode; >> struct shash_desc *desc; >> - char **xattrname; >> + struct evm_hmac_xattr *entry; >> size_t xattr_size = 0; >> char *xattr_value = NULL; >> int error; >> @@ -146,15 +146,15 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry, >> return PTR_ERR(desc); >> >> error = -ENODATA; >> - for (xattrname = evm_config_xattrnames; *xattrname != NULL; xattrname++) { >> + list_for_each_entry(entry, &evm_hmac_xattrs, list) { >> if ((req_xattr_name && req_xattr_value) >> - && !strcmp(*xattrname, req_xattr_name)) { >> + && !strcmp(entry->xattr, req_xattr_name)) { >> error = 0; >> crypto_shash_update(desc, (const u8 *)req_xattr_value, >> req_xattr_value_len); >> continue; >> } >> - size = vfs_getxattr_alloc(dentry, *xattrname, >> + size = vfs_getxattr_alloc(dentry, entry->xattr, >> &xattr_value, xattr_size, GFP_NOFS); >> if (size == -ENOMEM) { >> error = -ENOMEM; >> diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c >> index 9c05929..13e03ad 100644 >> --- a/security/integrity/evm/evm_main.c >> +++ b/security/integrity/evm/evm_main.c >> @@ -34,19 +34,7 @@ char *evm_hmac = "hmac(sha1)"; >> char *evm_hash = "sha1"; >> int evm_hmac_attrs; >> >> -char *evm_config_xattrnames[] = { >> -#ifdef CONFIG_SECURITY_SELINUX >> - XATTR_NAME_SELINUX, >> -#endif >> -#ifdef CONFIG_SECURITY_SMACK >> - XATTR_NAME_SMACK, >> -#endif >> -#ifdef CONFIG_IMA_APPRAISE >> - XATTR_NAME_IMA, >> -#endif >> - XATTR_NAME_CAPS, >> - NULL >> -}; >> +LIST_HEAD(evm_hmac_xattrs); >> >> static int evm_fixmode; >> static int __init evm_set_fixmode(char *str) >> @@ -61,27 +49,53 @@ static int __init evm_init_config(void) >> { >> char *attrs = CONFIG_EVM_HMAC_ATTRS; >> char *p; >> + char *xattrs = CONFIG_EVM_HMAC_XATTRS; >> + struct evm_hmac_xattr *entry; >> >> while ((p = strsep(&attrs, ", \t"))) { >> if (!strcmp(p, "fsuuid")) >> evm_hmac_attrs |= EVM_HMAC_ATTR_FSUUID; >> } >> pr_info("HMAC attrs: 0x%x\n", evm_hmac_attrs); >> + while ((p = strsep(&xattrs, ", \t"))) { >> +#ifndef CONFIG_SECURITY_SELINUX >> + if (!strcmp(p, XATTR_NAME_SELINUX)) >> + continue; >> +#endif >> +#ifndef CONFIG_SECURITY_SMACK >> + if (strstr(p, "SMACK64")) >> + continue; >> +#endif > As you mentioned, filesystems previously only included > 'security.smack64' in the HMAC calculation. This patch includes all > xattrs prefixed with smack64. All previously labeled filesystems would > need to be relabeled. > > Mimi Only the SMACK64 attribute is assigned to all files. The SMACK64EXEC and SMACK64TRANSMUTE attributes are optional. You do not want these attributes in most cases. Few files should actually have them. They should only be used if they are there, and ignored otherwise. If that is not possible it is better to ignore them completely. > >> +#ifndef CONFIG_IMA_APPRAISE >> + if (!strcmp(p, XATTR_NAME_IMA)) >> + continue; >> +#endif >> + entry = kmalloc(sizeof(*entry), GFP_KERNEL); >> + if (!entry) >> + return -ENOMEM; >> + INIT_LIST_HEAD(&entry->list); >> + entry->xattr = kstrdup(p, GFP_KERNEL); >> + if (!entry->xattr) >> + return -ENOMEM; >> + list_add_tail(&entry->list, &evm_hmac_xattrs); >> + } >> + list_for_each_entry(entry, &evm_hmac_xattrs, list) >> + pr_info("%s\n", entry->xattr); >> return 0; >> } >> >> static int evm_find_protected_xattrs(struct dentry *dentry) >> { >> struct inode *inode = dentry->d_inode; >> - char **xattr; >> + struct evm_hmac_xattr *entry; >> int error; >> int count = 0; >> >> if (!inode->i_op || !inode->i_op->getxattr) >> return -EOPNOTSUPP; >> >> - for (xattr = evm_config_xattrnames; *xattr != NULL; xattr++) { >> - error = inode->i_op->getxattr(dentry, *xattr, NULL, 0); >> + list_for_each_entry(entry, &evm_hmac_xattrs, list) { >> + error = inode->i_op->getxattr(dentry, entry->xattr, NULL, 0); >> if (error < 0) { >> if (error == -ENODATA) >> continue; >> @@ -183,19 +197,19 @@ out: >> >> static int evm_protected_xattr(const char *req_xattr_name) >> { >> - char **xattrname; >> + struct evm_hmac_xattr *entry; >> int namelen; >> int found = 0; >> >> namelen = strlen(req_xattr_name); >> - for (xattrname = evm_config_xattrnames; *xattrname != NULL; xattrname++) { >> - if ((strlen(*xattrname) == namelen) >> - && (strncmp(req_xattr_name, *xattrname, namelen) == 0)) { >> + list_for_each_entry(entry, &evm_hmac_xattrs, list) { >> + if ((strlen(entry->xattr) == namelen) >> + && (strncmp(req_xattr_name, entry->xattr, namelen) == 0)) { >> found = 1; >> break; >> } >> if (strncmp(req_xattr_name, >> - *xattrname + XATTR_SECURITY_PREFIX_LEN, >> + entry->xattr + XATTR_SECURITY_PREFIX_LEN, >> strlen(req_xattr_name)) == 0) { >> found = 1; >> break; >> @@ -462,19 +476,6 @@ err: >> return error; >> } >> >> -/* >> - * evm_display_config - list the EVM protected security extended attributes >> - */ >> -static int __init evm_display_config(void) >> -{ >> - char **xattrname; >> - >> - for (xattrname = evm_config_xattrnames; *xattrname != NULL; xattrname++) >> - pr_info("%s\n", *xattrname); >> - return 0; >> -} >> - >> -pure_initcall(evm_display_config); >> late_initcall(init_evm); >> >> MODULE_DESCRIPTION("Extended Verification Module"); > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 8/8] evm: introduce EVM hmac xattr list 2014-03-04 3:00 ` Casey Schaufler @ 2014-03-04 3:21 ` Mimi Zohar 2014-03-04 14:18 ` Dmitry Kasatkin 0 siblings, 1 reply; 31+ messages in thread From: Mimi Zohar @ 2014-03-04 3:21 UTC (permalink / raw) To: Casey Schaufler Cc: Dmitry Kasatkin, linux-security-module, jmorris, linux-kernel, dmitry.kasatkin On Mon, 2014-03-03 at 19:00 -0800, Casey Schaufler wrote: > On 3/3/2014 6:39 PM, Mimi Zohar wrote: > > On Fri, 2014-02-28 at 16:59 +0200, Dmitry Kasatkin wrote: > >> EVM currently uses source hard coded list of xattrs which needs to be > >> included into the HMAC calculation. This is very unflexible. > >> Adding new attributes requires modifcation of the source code and > >> prevents building the kernel which works with previously labeled > >> filesystems. > >> > >> Early versions of Smack used only one xattr security.SMACK64, > >> which is protected by EVM. Now Smack has a few more attributes and > >> they are not protected. Adding support to the source code makes it > >> impossible to use new kernel with previousely labeled filesystems. > > I think this patch will break any existing filesystems labeled with > > 'security.smack64'. Details inline. > > > >> This patch replaces hardcoded xattr array with dynamic list which is > >> initialized from CONFIG_EVM_HMAC_XATTRS variable. It allows to build > >> kernel with with support of older and newer EVM HMAC formats. > >> > >> Possible future extension will be to read xattr list from the kernel > >> command line or from securityfs entry. > >> > >> Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com> > >> --- > >> security/integrity/evm/Kconfig | 10 ++++++ > >> security/integrity/evm/evm.h | 7 +++- > >> security/integrity/evm/evm_crypto.c | 8 ++--- > >> security/integrity/evm/evm_main.c | 69 +++++++++++++++++++------------------ > >> 4 files changed, 55 insertions(+), 39 deletions(-) > >> > >> diff --git a/security/integrity/evm/Kconfig b/security/integrity/evm/Kconfig > >> index 2be51fa..06237b8 100644 > >> --- a/security/integrity/evm/Kconfig > >> +++ b/security/integrity/evm/Kconfig > >> @@ -25,3 +25,13 @@ config EVM_HMAC_ATTRS > >> WARNING: changing the HMAC calculation method or adding > >> additional info to the calculation, requires existing EVM > >> labeled file systems to be relabeled. > >> + > >> +config EVM_HMAC_XATTRS > >> + string "HMAC xattrs" > >> + default "security.selinux security.SMACK64 security.ima security.capability" > >> + help > >> + This options allows to specify list of extended attributes included into HMAC > >> + calculation. It makes it possible easily upgrade to newer kernels. > >> + > >> + Default value: > >> + security.selinux, security.SMACK64, security.ima, security.capability > >> diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h > >> index c8fa0aa..4d1c51e 100644 > >> --- a/security/integrity/evm/evm.h > >> +++ b/security/integrity/evm/evm.h > >> @@ -31,8 +31,13 @@ extern struct crypto_shash *hash_tfm; > >> > >> #define EVM_HMAC_ATTR_FSUUID 0x0001 > >> > >> +struct evm_hmac_xattr { > >> + struct list_head list; > >> + char *xattr; > >> +}; > >> + > >> /* List of EVM protected security xattrs */ > >> -extern char *evm_config_xattrnames[]; > >> +extern struct list_head evm_hmac_xattrs; > >> > >> int evm_init_key(void); > >> int evm_update_evmxattr(struct dentry *dentry, > >> diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c > >> index ab034e5..7e5bfb7 100644 > >> --- a/security/integrity/evm/evm_crypto.c > >> +++ b/security/integrity/evm/evm_crypto.c > >> @@ -133,7 +133,7 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry, > >> { > >> struct inode *inode = dentry->d_inode; > >> struct shash_desc *desc; > >> - char **xattrname; > >> + struct evm_hmac_xattr *entry; > >> size_t xattr_size = 0; > >> char *xattr_value = NULL; > >> int error; > >> @@ -146,15 +146,15 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry, > >> return PTR_ERR(desc); > >> > >> error = -ENODATA; > >> - for (xattrname = evm_config_xattrnames; *xattrname != NULL; xattrname++) { > >> + list_for_each_entry(entry, &evm_hmac_xattrs, list) { > >> if ((req_xattr_name && req_xattr_value) > >> - && !strcmp(*xattrname, req_xattr_name)) { > >> + && !strcmp(entry->xattr, req_xattr_name)) { > >> error = 0; > >> crypto_shash_update(desc, (const u8 *)req_xattr_value, > >> req_xattr_value_len); > >> continue; > >> } > >> - size = vfs_getxattr_alloc(dentry, *xattrname, > >> + size = vfs_getxattr_alloc(dentry, entry->xattr, > >> &xattr_value, xattr_size, GFP_NOFS); > >> if (size == -ENOMEM) { > >> error = -ENOMEM; > >> diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c > >> index 9c05929..13e03ad 100644 > >> --- a/security/integrity/evm/evm_main.c > >> +++ b/security/integrity/evm/evm_main.c > >> @@ -34,19 +34,7 @@ char *evm_hmac = "hmac(sha1)"; > >> char *evm_hash = "sha1"; > >> int evm_hmac_attrs; > >> > >> -char *evm_config_xattrnames[] = { > >> -#ifdef CONFIG_SECURITY_SELINUX > >> - XATTR_NAME_SELINUX, > >> -#endif > >> -#ifdef CONFIG_SECURITY_SMACK > >> - XATTR_NAME_SMACK, > >> -#endif > >> -#ifdef CONFIG_IMA_APPRAISE > >> - XATTR_NAME_IMA, > >> -#endif > >> - XATTR_NAME_CAPS, > >> - NULL > >> -}; > >> +LIST_HEAD(evm_hmac_xattrs); > >> > >> static int evm_fixmode; > >> static int __init evm_set_fixmode(char *str) > >> @@ -61,27 +49,53 @@ static int __init evm_init_config(void) > >> { > >> char *attrs = CONFIG_EVM_HMAC_ATTRS; > >> char *p; > >> + char *xattrs = CONFIG_EVM_HMAC_XATTRS; > >> + struct evm_hmac_xattr *entry; > >> > >> while ((p = strsep(&attrs, ", \t"))) { > >> if (!strcmp(p, "fsuuid")) > >> evm_hmac_attrs |= EVM_HMAC_ATTR_FSUUID; > >> } > >> pr_info("HMAC attrs: 0x%x\n", evm_hmac_attrs); > >> + while ((p = strsep(&xattrs, ", \t"))) { > >> +#ifndef CONFIG_SECURITY_SELINUX > >> + if (!strcmp(p, XATTR_NAME_SELINUX)) > >> + continue; > >> +#endif > >> +#ifndef CONFIG_SECURITY_SMACK > >> + if (strstr(p, "SMACK64")) > >> + continue; > >> +#endif > > As you mentioned, filesystems previously only included > > 'security.smack64' in the HMAC calculation. This patch includes all > > xattrs prefixed with smack64. All previously labeled filesystems would > > need to be relabeled. > > > > Mimi > > Only the SMACK64 attribute is assigned to all files. The SMACK64EXEC > and SMACK64TRANSMUTE attributes are optional. You do not want these > attributes in most cases. Few files should actually have them. They > should only be used if they are there, and ignored otherwise. If that > is not possible it is better to ignore them completely. Going forward the code would work just fine. All smack64 prefixed labels would be included in the HMAC calculation. My concern is for existing labeled filesystems, which only included the SMACK64 label in the HMAC calculation, but have other SMACK64 labels. The HMAC verification would fail for these files. Mimi > > > >> +#ifndef CONFIG_IMA_APPRAISE > >> + if (!strcmp(p, XATTR_NAME_IMA)) > >> + continue; > >> +#endif > >> + entry = kmalloc(sizeof(*entry), GFP_KERNEL); > >> + if (!entry) > >> + return -ENOMEM; > >> + INIT_LIST_HEAD(&entry->list); > >> + entry->xattr = kstrdup(p, GFP_KERNEL); > >> + if (!entry->xattr) > >> + return -ENOMEM; > >> + list_add_tail(&entry->list, &evm_hmac_xattrs); > >> + } > >> + list_for_each_entry(entry, &evm_hmac_xattrs, list) > >> + pr_info("%s\n", entry->xattr); > >> return 0; > >> } > >> > >> static int evm_find_protected_xattrs(struct dentry *dentry) > >> { > >> struct inode *inode = dentry->d_inode; > >> - char **xattr; > >> + struct evm_hmac_xattr *entry; > >> int error; > >> int count = 0; > >> > >> if (!inode->i_op || !inode->i_op->getxattr) > >> return -EOPNOTSUPP; > >> > >> - for (xattr = evm_config_xattrnames; *xattr != NULL; xattr++) { > >> - error = inode->i_op->getxattr(dentry, *xattr, NULL, 0); > >> + list_for_each_entry(entry, &evm_hmac_xattrs, list) { > >> + error = inode->i_op->getxattr(dentry, entry->xattr, NULL, 0); > >> if (error < 0) { > >> if (error == -ENODATA) > >> continue; > >> @@ -183,19 +197,19 @@ out: > >> > >> static int evm_protected_xattr(const char *req_xattr_name) > >> { > >> - char **xattrname; > >> + struct evm_hmac_xattr *entry; > >> int namelen; > >> int found = 0; > >> > >> namelen = strlen(req_xattr_name); > >> - for (xattrname = evm_config_xattrnames; *xattrname != NULL; xattrname++) { > >> - if ((strlen(*xattrname) == namelen) > >> - && (strncmp(req_xattr_name, *xattrname, namelen) == 0)) { > >> + list_for_each_entry(entry, &evm_hmac_xattrs, list) { > >> + if ((strlen(entry->xattr) == namelen) > >> + && (strncmp(req_xattr_name, entry->xattr, namelen) == 0)) { > >> found = 1; > >> break; > >> } > >> if (strncmp(req_xattr_name, > >> - *xattrname + XATTR_SECURITY_PREFIX_LEN, > >> + entry->xattr + XATTR_SECURITY_PREFIX_LEN, > >> strlen(req_xattr_name)) == 0) { > >> found = 1; > >> break; > >> @@ -462,19 +476,6 @@ err: > >> return error; > >> } > >> > >> -/* > >> - * evm_display_config - list the EVM protected security extended attributes > >> - */ > >> -static int __init evm_display_config(void) > >> -{ > >> - char **xattrname; > >> - > >> - for (xattrname = evm_config_xattrnames; *xattrname != NULL; xattrname++) > >> - pr_info("%s\n", *xattrname); > >> - return 0; > >> -} > >> - > >> -pure_initcall(evm_display_config); > >> late_initcall(init_evm); > >> > >> MODULE_DESCRIPTION("Extended Verification Module"); > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > Please read the FAQ at http://www.tux.org/lkml/ > > > ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 8/8] evm: introduce EVM hmac xattr list 2014-03-04 3:21 ` Mimi Zohar @ 2014-03-04 14:18 ` Dmitry Kasatkin 2014-03-04 20:36 ` Mimi Zohar 0 siblings, 1 reply; 31+ messages in thread From: Dmitry Kasatkin @ 2014-03-04 14:18 UTC (permalink / raw) To: Mimi Zohar Cc: Casey Schaufler, Dmitry Kasatkin, linux-security-module, James Morris, linux-kernel@vger.kernel.org On Tue, Mar 4, 2014 at 5:21 AM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote: > On Mon, 2014-03-03 at 19:00 -0800, Casey Schaufler wrote: >> On 3/3/2014 6:39 PM, Mimi Zohar wrote: >> > On Fri, 2014-02-28 at 16:59 +0200, Dmitry Kasatkin wrote: >> >> EVM currently uses source hard coded list of xattrs which needs to be >> >> included into the HMAC calculation. This is very unflexible. >> >> Adding new attributes requires modifcation of the source code and >> >> prevents building the kernel which works with previously labeled >> >> filesystems. >> >> >> >> Early versions of Smack used only one xattr security.SMACK64, >> >> which is protected by EVM. Now Smack has a few more attributes and >> >> they are not protected. Adding support to the source code makes it >> >> impossible to use new kernel with previousely labeled filesystems. >> > I think this patch will break any existing filesystems labeled with >> > 'security.smack64'. Details inline. >> > >> >> This patch replaces hardcoded xattr array with dynamic list which is >> >> initialized from CONFIG_EVM_HMAC_XATTRS variable. It allows to build >> >> kernel with with support of older and newer EVM HMAC formats. >> >> >> >> Possible future extension will be to read xattr list from the kernel >> >> command line or from securityfs entry. >> >> >> >> Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com> >> >> --- >> >> security/integrity/evm/Kconfig | 10 ++++++ >> >> security/integrity/evm/evm.h | 7 +++- >> >> security/integrity/evm/evm_crypto.c | 8 ++--- >> >> security/integrity/evm/evm_main.c | 69 +++++++++++++++++++------------------ >> >> 4 files changed, 55 insertions(+), 39 deletions(-) >> >> >> >> diff --git a/security/integrity/evm/Kconfig b/security/integrity/evm/Kconfig >> >> index 2be51fa..06237b8 100644 >> >> --- a/security/integrity/evm/Kconfig >> >> +++ b/security/integrity/evm/Kconfig >> >> @@ -25,3 +25,13 @@ config EVM_HMAC_ATTRS >> >> WARNING: changing the HMAC calculation method or adding >> >> additional info to the calculation, requires existing EVM >> >> labeled file systems to be relabeled. >> >> + >> >> +config EVM_HMAC_XATTRS >> >> + string "HMAC xattrs" >> >> + default "security.selinux security.SMACK64 security.ima security.capability" >> >> + help >> >> + This options allows to specify list of extended attributes included into HMAC >> >> + calculation. It makes it possible easily upgrade to newer kernels. >> >> + >> >> + Default value: >> >> + security.selinux, security.SMACK64, security.ima, security.capability >> >> diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h >> >> index c8fa0aa..4d1c51e 100644 >> >> --- a/security/integrity/evm/evm.h >> >> +++ b/security/integrity/evm/evm.h >> >> @@ -31,8 +31,13 @@ extern struct crypto_shash *hash_tfm; >> >> >> >> #define EVM_HMAC_ATTR_FSUUID 0x0001 >> >> >> >> +struct evm_hmac_xattr { >> >> + struct list_head list; >> >> + char *xattr; >> >> +}; >> >> + >> >> /* List of EVM protected security xattrs */ >> >> -extern char *evm_config_xattrnames[]; >> >> +extern struct list_head evm_hmac_xattrs; >> >> >> >> int evm_init_key(void); >> >> int evm_update_evmxattr(struct dentry *dentry, >> >> diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c >> >> index ab034e5..7e5bfb7 100644 >> >> --- a/security/integrity/evm/evm_crypto.c >> >> +++ b/security/integrity/evm/evm_crypto.c >> >> @@ -133,7 +133,7 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry, >> >> { >> >> struct inode *inode = dentry->d_inode; >> >> struct shash_desc *desc; >> >> - char **xattrname; >> >> + struct evm_hmac_xattr *entry; >> >> size_t xattr_size = 0; >> >> char *xattr_value = NULL; >> >> int error; >> >> @@ -146,15 +146,15 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry, >> >> return PTR_ERR(desc); >> >> >> >> error = -ENODATA; >> >> - for (xattrname = evm_config_xattrnames; *xattrname != NULL; xattrname++) { >> >> + list_for_each_entry(entry, &evm_hmac_xattrs, list) { >> >> if ((req_xattr_name && req_xattr_value) >> >> - && !strcmp(*xattrname, req_xattr_name)) { >> >> + && !strcmp(entry->xattr, req_xattr_name)) { >> >> error = 0; >> >> crypto_shash_update(desc, (const u8 *)req_xattr_value, >> >> req_xattr_value_len); >> >> continue; >> >> } >> >> - size = vfs_getxattr_alloc(dentry, *xattrname, >> >> + size = vfs_getxattr_alloc(dentry, entry->xattr, >> >> &xattr_value, xattr_size, GFP_NOFS); >> >> if (size == -ENOMEM) { >> >> error = -ENOMEM; >> >> diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c >> >> index 9c05929..13e03ad 100644 >> >> --- a/security/integrity/evm/evm_main.c >> >> +++ b/security/integrity/evm/evm_main.c >> >> @@ -34,19 +34,7 @@ char *evm_hmac = "hmac(sha1)"; >> >> char *evm_hash = "sha1"; >> >> int evm_hmac_attrs; >> >> >> >> -char *evm_config_xattrnames[] = { >> >> -#ifdef CONFIG_SECURITY_SELINUX >> >> - XATTR_NAME_SELINUX, >> >> -#endif >> >> -#ifdef CONFIG_SECURITY_SMACK >> >> - XATTR_NAME_SMACK, >> >> -#endif >> >> -#ifdef CONFIG_IMA_APPRAISE >> >> - XATTR_NAME_IMA, >> >> -#endif >> >> - XATTR_NAME_CAPS, >> >> - NULL >> >> -}; >> >> +LIST_HEAD(evm_hmac_xattrs); >> >> >> >> static int evm_fixmode; >> >> static int __init evm_set_fixmode(char *str) >> >> @@ -61,27 +49,53 @@ static int __init evm_init_config(void) >> >> { >> >> char *attrs = CONFIG_EVM_HMAC_ATTRS; >> >> char *p; >> >> + char *xattrs = CONFIG_EVM_HMAC_XATTRS; >> >> + struct evm_hmac_xattr *entry; >> >> >> >> while ((p = strsep(&attrs, ", \t"))) { >> >> if (!strcmp(p, "fsuuid")) >> >> evm_hmac_attrs |= EVM_HMAC_ATTR_FSUUID; >> >> } >> >> pr_info("HMAC attrs: 0x%x\n", evm_hmac_attrs); >> >> + while ((p = strsep(&xattrs, ", \t"))) { >> >> +#ifndef CONFIG_SECURITY_SELINUX >> >> + if (!strcmp(p, XATTR_NAME_SELINUX)) >> >> + continue; >> >> +#endif >> >> +#ifndef CONFIG_SECURITY_SMACK >> >> + if (strstr(p, "SMACK64")) >> >> + continue; >> >> +#endif >> > As you mentioned, filesystems previously only included >> > 'security.smack64' in the HMAC calculation. This patch includes all >> > xattrs prefixed with smack64. All previously labeled filesystems would >> > need to be relabeled. >> > >> > Mimi >> >> Only the SMACK64 attribute is assigned to all files. The SMACK64EXEC >> and SMACK64TRANSMUTE attributes are optional. You do not want these >> attributes in most cases. Few files should actually have them. They >> should only be used if they are there, and ignored otherwise. If that >> is not possible it is better to ignore them completely. > > Going forward the code would work just fine. All smack64 prefixed > labels would be included in the HMAC calculation. My concern is for > existing labeled filesystems, which only included the SMACK64 label in > the HMAC calculation, but have other SMACK64 labels. The HMAC > verification would fail for these files. > Hello, There is no problem for existing labeled filesystems. Patch does not work with SMACK64 prefix, but with exact name. Only xattrs listed in the configuration options are included By default "security.selinux, security.SMACK64, security.ima, security.capability" which correspond to EVM current functionality... Existing systems using new kernel may stick to original EVM xattr set. But new systems might prefer to add "more" xattrs.. This patch makes it flexible... - Dmitry > Mimi > >> > >> >> +#ifndef CONFIG_IMA_APPRAISE >> >> + if (!strcmp(p, XATTR_NAME_IMA)) >> >> + continue; >> >> +#endif >> >> + entry = kmalloc(sizeof(*entry), GFP_KERNEL); >> >> + if (!entry) >> >> + return -ENOMEM; >> >> + INIT_LIST_HEAD(&entry->list); >> >> + entry->xattr = kstrdup(p, GFP_KERNEL); >> >> + if (!entry->xattr) >> >> + return -ENOMEM; >> >> + list_add_tail(&entry->list, &evm_hmac_xattrs); >> >> + } >> >> + list_for_each_entry(entry, &evm_hmac_xattrs, list) >> >> + pr_info("%s\n", entry->xattr); >> >> return 0; >> >> } >> >> >> >> static int evm_find_protected_xattrs(struct dentry *dentry) >> >> { >> >> struct inode *inode = dentry->d_inode; >> >> - char **xattr; >> >> + struct evm_hmac_xattr *entry; >> >> int error; >> >> int count = 0; >> >> >> >> if (!inode->i_op || !inode->i_op->getxattr) >> >> return -EOPNOTSUPP; >> >> >> >> - for (xattr = evm_config_xattrnames; *xattr != NULL; xattr++) { >> >> - error = inode->i_op->getxattr(dentry, *xattr, NULL, 0); >> >> + list_for_each_entry(entry, &evm_hmac_xattrs, list) { >> >> + error = inode->i_op->getxattr(dentry, entry->xattr, NULL, 0); >> >> if (error < 0) { >> >> if (error == -ENODATA) >> >> continue; >> >> @@ -183,19 +197,19 @@ out: >> >> >> >> static int evm_protected_xattr(const char *req_xattr_name) >> >> { >> >> - char **xattrname; >> >> + struct evm_hmac_xattr *entry; >> >> int namelen; >> >> int found = 0; >> >> >> >> namelen = strlen(req_xattr_name); >> >> - for (xattrname = evm_config_xattrnames; *xattrname != NULL; xattrname++) { >> >> - if ((strlen(*xattrname) == namelen) >> >> - && (strncmp(req_xattr_name, *xattrname, namelen) == 0)) { >> >> + list_for_each_entry(entry, &evm_hmac_xattrs, list) { >> >> + if ((strlen(entry->xattr) == namelen) >> >> + && (strncmp(req_xattr_name, entry->xattr, namelen) == 0)) { >> >> found = 1; >> >> break; >> >> } >> >> if (strncmp(req_xattr_name, >> >> - *xattrname + XATTR_SECURITY_PREFIX_LEN, >> >> + entry->xattr + XATTR_SECURITY_PREFIX_LEN, >> >> strlen(req_xattr_name)) == 0) { >> >> found = 1; >> >> break; >> >> @@ -462,19 +476,6 @@ err: >> >> return error; >> >> } >> >> >> >> -/* >> >> - * evm_display_config - list the EVM protected security extended attributes >> >> - */ >> >> -static int __init evm_display_config(void) >> >> -{ >> >> - char **xattrname; >> >> - >> >> - for (xattrname = evm_config_xattrnames; *xattrname != NULL; xattrname++) >> >> - pr_info("%s\n", *xattrname); >> >> - return 0; >> >> -} >> >> - >> >> -pure_initcall(evm_display_config); >> >> late_initcall(init_evm); >> >> >> >> MODULE_DESCRIPTION("Extended Verification Module"); >> > >> > -- >> > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in >> > the body of a message to majordomo@vger.kernel.org >> > More majordomo info at http://vger.kernel.org/majordomo-info.html >> > Please read the FAQ at http://www.tux.org/lkml/ >> > >> > > -- Thanks, Dmitry ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 8/8] evm: introduce EVM hmac xattr list 2014-03-04 14:18 ` Dmitry Kasatkin @ 2014-03-04 20:36 ` Mimi Zohar 2014-03-05 9:26 ` Dmitry Kasatkin 0 siblings, 1 reply; 31+ messages in thread From: Mimi Zohar @ 2014-03-04 20:36 UTC (permalink / raw) To: Dmitry Kasatkin Cc: Casey Schaufler, Dmitry Kasatkin, linux-security-module, James Morris, linux-kernel@vger.kernel.org On Tue, 2014-03-04 at 16:18 +0200, Dmitry Kasatkin wrote: > On Tue, Mar 4, 2014 at 5:21 AM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote: > > On Mon, 2014-03-03 at 19:00 -0800, Casey Schaufler wrote: > >> On 3/3/2014 6:39 PM, Mimi Zohar wrote: > >> > On Fri, 2014-02-28 at 16:59 +0200, Dmitry Kasatkin wrote: > >> >> EVM currently uses source hard coded list of xattrs which needs to be > >> >> included into the HMAC calculation. This is very unflexible. > >> >> Adding new attributes requires modifcation of the source code and > >> >> prevents building the kernel which works with previously labeled > >> >> filesystems. > >> >> > >> >> Early versions of Smack used only one xattr security.SMACK64, > >> >> which is protected by EVM. Now Smack has a few more attributes and > >> >> they are not protected. Adding support to the source code makes it > >> >> impossible to use new kernel with previousely labeled filesystems. > >> > I think this patch will break any existing filesystems labeled with > >> > 'security.smack64'. Details inline. > >> > > >> >> This patch replaces hardcoded xattr array with dynamic list which is > >> >> initialized from CONFIG_EVM_HMAC_XATTRS variable. It allows to build > >> >> kernel with with support of older and newer EVM HMAC formats. So instead of having a single kernel, this allows you to build different kernels with different xattr labels included in the HMAC. Wouldn't you want a migration mode, similar to 'fix' mode, that only updates the HMAC, if the existing HMAC verified based on the prior set of xattrs? > >> >> > >> >> Possible future extension will be to read xattr list from the kernel > >> >> command line or from securityfs entry. > >> >> > >> >> Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com> > >> >> --- > >> >> security/integrity/evm/Kconfig | 10 ++++++ > >> >> security/integrity/evm/evm.h | 7 +++- > >> >> security/integrity/evm/evm_crypto.c | 8 ++--- > >> >> security/integrity/evm/evm_main.c | 69 +++++++++++++++++++------------------ > >> >> 4 files changed, 55 insertions(+), 39 deletions(-) > >> >> > >> >> diff --git a/security/integrity/evm/Kconfig b/security/integrity/evm/Kconfig > >> >> index 2be51fa..06237b8 100644 > >> >> --- a/security/integrity/evm/Kconfig > >> >> +++ b/security/integrity/evm/Kconfig > >> >> @@ -25,3 +25,13 @@ config EVM_HMAC_ATTRS > >> >> WARNING: changing the HMAC calculation method or adding > >> >> additional info to the calculation, requires existing EVM > >> >> labeled file systems to be relabeled. > >> >> + > >> >> +config EVM_HMAC_XATTRS > >> >> + string "HMAC xattrs" > >> >> + default "security.selinux security.SMACK64 security.ima security.capability" > >> >> + help > >> >> + This options allows to specify list of extended attributes included into HMAC > >> >> + calculation. It makes it possible easily upgrade to newer kernels. > >> >> + > >> >> + Default value: > >> >> + security.selinux, security.SMACK64, security.ima, security.capability > >> >> diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h > >> >> index c8fa0aa..4d1c51e 100644 > >> >> --- a/security/integrity/evm/evm.h > >> >> +++ b/security/integrity/evm/evm.h > >> >> @@ -31,8 +31,13 @@ extern struct crypto_shash *hash_tfm; > >> >> > >> >> #define EVM_HMAC_ATTR_FSUUID 0x0001 > >> >> > >> >> +struct evm_hmac_xattr { > >> >> + struct list_head list; > >> >> + char *xattr; > >> >> +}; > >> >> + > >> >> /* List of EVM protected security xattrs */ > >> >> -extern char *evm_config_xattrnames[]; > >> >> +extern struct list_head evm_hmac_xattrs; > >> >> > >> >> int evm_init_key(void); > >> >> int evm_update_evmxattr(struct dentry *dentry, > >> >> diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c > >> >> index ab034e5..7e5bfb7 100644 > >> >> --- a/security/integrity/evm/evm_crypto.c > >> >> +++ b/security/integrity/evm/evm_crypto.c > >> >> @@ -133,7 +133,7 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry, > >> >> { > >> >> struct inode *inode = dentry->d_inode; > >> >> struct shash_desc *desc; > >> >> - char **xattrname; > >> >> + struct evm_hmac_xattr *entry; > >> >> size_t xattr_size = 0; > >> >> char *xattr_value = NULL; > >> >> int error; > >> >> @@ -146,15 +146,15 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry, > >> >> return PTR_ERR(desc); > >> >> > >> >> error = -ENODATA; > >> >> - for (xattrname = evm_config_xattrnames; *xattrname != NULL; xattrname++) { > >> >> + list_for_each_entry(entry, &evm_hmac_xattrs, list) { > >> >> if ((req_xattr_name && req_xattr_value) > >> >> - && !strcmp(*xattrname, req_xattr_name)) { > >> >> + && !strcmp(entry->xattr, req_xattr_name)) { > >> >> error = 0; > >> >> crypto_shash_update(desc, (const u8 *)req_xattr_value, > >> >> req_xattr_value_len); > >> >> continue; > >> >> } > >> >> - size = vfs_getxattr_alloc(dentry, *xattrname, > >> >> + size = vfs_getxattr_alloc(dentry, entry->xattr, > >> >> &xattr_value, xattr_size, GFP_NOFS); > >> >> if (size == -ENOMEM) { > >> >> error = -ENOMEM; > >> >> diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c > >> >> index 9c05929..13e03ad 100644 > >> >> --- a/security/integrity/evm/evm_main.c > >> >> +++ b/security/integrity/evm/evm_main.c > >> >> @@ -34,19 +34,7 @@ char *evm_hmac = "hmac(sha1)"; > >> >> char *evm_hash = "sha1"; > >> >> int evm_hmac_attrs; > >> >> > >> >> -char *evm_config_xattrnames[] = { > >> >> -#ifdef CONFIG_SECURITY_SELINUX > >> >> - XATTR_NAME_SELINUX, > >> >> -#endif > >> >> -#ifdef CONFIG_SECURITY_SMACK > >> >> - XATTR_NAME_SMACK, > >> >> -#endif > >> >> -#ifdef CONFIG_IMA_APPRAISE > >> >> - XATTR_NAME_IMA, > >> >> -#endif > >> >> - XATTR_NAME_CAPS, > >> >> - NULL > >> >> -}; > >> >> +LIST_HEAD(evm_hmac_xattrs); > >> >> > >> >> static int evm_fixmode; > >> >> static int __init evm_set_fixmode(char *str) > >> >> @@ -61,27 +49,53 @@ static int __init evm_init_config(void) > >> >> { > >> >> char *attrs = CONFIG_EVM_HMAC_ATTRS; > >> >> char *p; > >> >> + char *xattrs = CONFIG_EVM_HMAC_XATTRS; > >> >> + struct evm_hmac_xattr *entry; > >> >> > >> >> while ((p = strsep(&attrs, ", \t"))) { > >> >> if (!strcmp(p, "fsuuid")) > >> >> evm_hmac_attrs |= EVM_HMAC_ATTR_FSUUID; > >> >> } > >> >> pr_info("HMAC attrs: 0x%x\n", evm_hmac_attrs); > >> >> + while ((p = strsep(&xattrs, ", \t"))) { > >> >> +#ifndef CONFIG_SECURITY_SELINUX > >> >> + if (!strcmp(p, XATTR_NAME_SELINUX)) > >> >> + continue; > >> >> +#endif > >> >> +#ifndef CONFIG_SECURITY_SMACK > >> >> + if (strstr(p, "SMACK64")) > >> >> + continue; > >> >> +#endif > >> > As you mentioned, filesystems previously only included > >> > 'security.smack64' in the HMAC calculation. This patch includes all > >> > xattrs prefixed with smack64. All previously labeled filesystems would > >> > need to be relabeled. > >> > > >> > Mimi > >> > >> Only the SMACK64 attribute is assigned to all files. The SMACK64EXEC > >> and SMACK64TRANSMUTE attributes are optional. You do not want these > >> attributes in most cases. Few files should actually have them. They > >> should only be used if they are there, and ignored otherwise. If that > >> is not possible it is better to ignore them completely. > > > > Going forward the code would work just fine. All smack64 prefixed > > labels would be included in the HMAC calculation. My concern is for > > existing labeled filesystems, which only included the SMACK64 label in > > the HMAC calculation, but have other SMACK64 labels. The HMAC > > verification would fail for these files. > > > > Hello, > > There is no problem for existing labeled filesystems. > > Patch does not work with SMACK64 prefix, but with exact name. > > Only xattrs listed in the configuration options are included > By default "security.selinux, security.SMACK64, security.ima, > security.capability" > which correspond to EVM current functionality... > > Existing systems using new kernel may stick to original EVM xattr set. > > But new systems might prefer to add "more" xattrs.. > > This patch makes it flexible... Ok, the list of xattrs included in the HMAC calculation is build time configurable and runtime verified. thanks, Mimi ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 8/8] evm: introduce EVM hmac xattr list 2014-03-04 20:36 ` Mimi Zohar @ 2014-03-05 9:26 ` Dmitry Kasatkin 2014-03-05 16:04 ` Mimi Zohar 0 siblings, 1 reply; 31+ messages in thread From: Dmitry Kasatkin @ 2014-03-05 9:26 UTC (permalink / raw) To: Mimi Zohar Cc: Casey Schaufler, Dmitry Kasatkin, linux-security-module, James Morris, linux-kernel@vger.kernel.org On Tue, Mar 4, 2014 at 10:36 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote: > On Tue, 2014-03-04 at 16:18 +0200, Dmitry Kasatkin wrote: >> On Tue, Mar 4, 2014 at 5:21 AM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote: >> > On Mon, 2014-03-03 at 19:00 -0800, Casey Schaufler wrote: >> >> On 3/3/2014 6:39 PM, Mimi Zohar wrote: >> >> > On Fri, 2014-02-28 at 16:59 +0200, Dmitry Kasatkin wrote: >> >> >> EVM currently uses source hard coded list of xattrs which needs to be >> >> >> included into the HMAC calculation. This is very unflexible. >> >> >> Adding new attributes requires modifcation of the source code and >> >> >> prevents building the kernel which works with previously labeled >> >> >> filesystems. >> >> >> >> >> >> Early versions of Smack used only one xattr security.SMACK64, >> >> >> which is protected by EVM. Now Smack has a few more attributes and >> >> >> they are not protected. Adding support to the source code makes it >> >> >> impossible to use new kernel with previousely labeled filesystems. >> >> > I think this patch will break any existing filesystems labeled with >> >> > 'security.smack64'. Details inline. >> >> > >> >> >> This patch replaces hardcoded xattr array with dynamic list which is >> >> >> initialized from CONFIG_EVM_HMAC_XATTRS variable. It allows to build >> >> >> kernel with with support of older and newer EVM HMAC formats. > > So instead of having a single kernel, this allows you to build different > kernels with different xattr labels included in the HMAC. Wouldn't you > want a migration mode, similar to 'fix' mode, that only updates the > HMAC, if the existing HMAC verified based on the prior set of xattrs? > It would require to maintain 2 sets of xattrs: "old" and "new". What if "new" becomes "old" again, while there were still not updated previous "old" :) I think it would bring unnecessary complexity. System designers define what xattrs they want to protect and stick with that. Filesystems stays anyway, but allows to upgrade to new kernels. If someone really wants to upgrade the system, just use "evm=fix" and run recursive fix.., e.g. 'evmctl -r ima_fix /' - Dmitry >> >> >> >> >> >> Possible future extension will be to read xattr list from the kernel >> >> >> command line or from securityfs entry. >> >> >> >> >> >> Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com> >> >> >> --- >> >> >> security/integrity/evm/Kconfig | 10 ++++++ >> >> >> security/integrity/evm/evm.h | 7 +++- >> >> >> security/integrity/evm/evm_crypto.c | 8 ++--- >> >> >> security/integrity/evm/evm_main.c | 69 +++++++++++++++++++------------------ >> >> >> 4 files changed, 55 insertions(+), 39 deletions(-) >> >> >> >> >> >> diff --git a/security/integrity/evm/Kconfig b/security/integrity/evm/Kconfig >> >> >> index 2be51fa..06237b8 100644 >> >> >> --- a/security/integrity/evm/Kconfig >> >> >> +++ b/security/integrity/evm/Kconfig >> >> >> @@ -25,3 +25,13 @@ config EVM_HMAC_ATTRS >> >> >> WARNING: changing the HMAC calculation method or adding >> >> >> additional info to the calculation, requires existing EVM >> >> >> labeled file systems to be relabeled. >> >> >> + >> >> >> +config EVM_HMAC_XATTRS >> >> >> + string "HMAC xattrs" >> >> >> + default "security.selinux security.SMACK64 security.ima security.capability" >> >> >> + help >> >> >> + This options allows to specify list of extended attributes included into HMAC >> >> >> + calculation. It makes it possible easily upgrade to newer kernels. >> >> >> + >> >> >> + Default value: >> >> >> + security.selinux, security.SMACK64, security.ima, security.capability >> >> >> diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h >> >> >> index c8fa0aa..4d1c51e 100644 >> >> >> --- a/security/integrity/evm/evm.h >> >> >> +++ b/security/integrity/evm/evm.h >> >> >> @@ -31,8 +31,13 @@ extern struct crypto_shash *hash_tfm; >> >> >> >> >> >> #define EVM_HMAC_ATTR_FSUUID 0x0001 >> >> >> >> >> >> +struct evm_hmac_xattr { >> >> >> + struct list_head list; >> >> >> + char *xattr; >> >> >> +}; >> >> >> + >> >> >> /* List of EVM protected security xattrs */ >> >> >> -extern char *evm_config_xattrnames[]; >> >> >> +extern struct list_head evm_hmac_xattrs; >> >> >> >> >> >> int evm_init_key(void); >> >> >> int evm_update_evmxattr(struct dentry *dentry, >> >> >> diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c >> >> >> index ab034e5..7e5bfb7 100644 >> >> >> --- a/security/integrity/evm/evm_crypto.c >> >> >> +++ b/security/integrity/evm/evm_crypto.c >> >> >> @@ -133,7 +133,7 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry, >> >> >> { >> >> >> struct inode *inode = dentry->d_inode; >> >> >> struct shash_desc *desc; >> >> >> - char **xattrname; >> >> >> + struct evm_hmac_xattr *entry; >> >> >> size_t xattr_size = 0; >> >> >> char *xattr_value = NULL; >> >> >> int error; >> >> >> @@ -146,15 +146,15 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry, >> >> >> return PTR_ERR(desc); >> >> >> >> >> >> error = -ENODATA; >> >> >> - for (xattrname = evm_config_xattrnames; *xattrname != NULL; xattrname++) { >> >> >> + list_for_each_entry(entry, &evm_hmac_xattrs, list) { >> >> >> if ((req_xattr_name && req_xattr_value) >> >> >> - && !strcmp(*xattrname, req_xattr_name)) { >> >> >> + && !strcmp(entry->xattr, req_xattr_name)) { >> >> >> error = 0; >> >> >> crypto_shash_update(desc, (const u8 *)req_xattr_value, >> >> >> req_xattr_value_len); >> >> >> continue; >> >> >> } >> >> >> - size = vfs_getxattr_alloc(dentry, *xattrname, >> >> >> + size = vfs_getxattr_alloc(dentry, entry->xattr, >> >> >> &xattr_value, xattr_size, GFP_NOFS); >> >> >> if (size == -ENOMEM) { >> >> >> error = -ENOMEM; >> >> >> diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c >> >> >> index 9c05929..13e03ad 100644 >> >> >> --- a/security/integrity/evm/evm_main.c >> >> >> +++ b/security/integrity/evm/evm_main.c >> >> >> @@ -34,19 +34,7 @@ char *evm_hmac = "hmac(sha1)"; >> >> >> char *evm_hash = "sha1"; >> >> >> int evm_hmac_attrs; >> >> >> >> >> >> -char *evm_config_xattrnames[] = { >> >> >> -#ifdef CONFIG_SECURITY_SELINUX >> >> >> - XATTR_NAME_SELINUX, >> >> >> -#endif >> >> >> -#ifdef CONFIG_SECURITY_SMACK >> >> >> - XATTR_NAME_SMACK, >> >> >> -#endif >> >> >> -#ifdef CONFIG_IMA_APPRAISE >> >> >> - XATTR_NAME_IMA, >> >> >> -#endif >> >> >> - XATTR_NAME_CAPS, >> >> >> - NULL >> >> >> -}; >> >> >> +LIST_HEAD(evm_hmac_xattrs); >> >> >> >> >> >> static int evm_fixmode; >> >> >> static int __init evm_set_fixmode(char *str) >> >> >> @@ -61,27 +49,53 @@ static int __init evm_init_config(void) >> >> >> { >> >> >> char *attrs = CONFIG_EVM_HMAC_ATTRS; >> >> >> char *p; >> >> >> + char *xattrs = CONFIG_EVM_HMAC_XATTRS; >> >> >> + struct evm_hmac_xattr *entry; >> >> >> >> >> >> while ((p = strsep(&attrs, ", \t"))) { >> >> >> if (!strcmp(p, "fsuuid")) >> >> >> evm_hmac_attrs |= EVM_HMAC_ATTR_FSUUID; >> >> >> } >> >> >> pr_info("HMAC attrs: 0x%x\n", evm_hmac_attrs); >> >> >> + while ((p = strsep(&xattrs, ", \t"))) { >> >> >> +#ifndef CONFIG_SECURITY_SELINUX >> >> >> + if (!strcmp(p, XATTR_NAME_SELINUX)) >> >> >> + continue; >> >> >> +#endif >> >> >> +#ifndef CONFIG_SECURITY_SMACK >> >> >> + if (strstr(p, "SMACK64")) >> >> >> + continue; >> >> >> +#endif >> >> > As you mentioned, filesystems previously only included >> >> > 'security.smack64' in the HMAC calculation. This patch includes all >> >> > xattrs prefixed with smack64. All previously labeled filesystems would >> >> > need to be relabeled. >> >> > >> >> > Mimi >> >> >> >> Only the SMACK64 attribute is assigned to all files. The SMACK64EXEC >> >> and SMACK64TRANSMUTE attributes are optional. You do not want these >> >> attributes in most cases. Few files should actually have them. They >> >> should only be used if they are there, and ignored otherwise. If that >> >> is not possible it is better to ignore them completely. >> > >> > Going forward the code would work just fine. All smack64 prefixed >> > labels would be included in the HMAC calculation. My concern is for >> > existing labeled filesystems, which only included the SMACK64 label in >> > the HMAC calculation, but have other SMACK64 labels. The HMAC >> > verification would fail for these files. >> > >> >> Hello, >> >> There is no problem for existing labeled filesystems. >> >> Patch does not work with SMACK64 prefix, but with exact name. >> >> Only xattrs listed in the configuration options are included >> By default "security.selinux, security.SMACK64, security.ima, >> security.capability" >> which correspond to EVM current functionality... >> >> Existing systems using new kernel may stick to original EVM xattr set. >> >> But new systems might prefer to add "more" xattrs.. >> >> This patch makes it flexible... > > Ok, the list of xattrs included in the HMAC calculation is build time > configurable and runtime verified. > yes. > thanks, > > Mimi > > -- Thanks, Dmitry ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 8/8] evm: introduce EVM hmac xattr list 2014-03-05 9:26 ` Dmitry Kasatkin @ 2014-03-05 16:04 ` Mimi Zohar 2014-03-05 22:20 ` Dmitry Kasatkin 0 siblings, 1 reply; 31+ messages in thread From: Mimi Zohar @ 2014-03-05 16:04 UTC (permalink / raw) To: Dmitry Kasatkin Cc: Casey Schaufler, Dmitry Kasatkin, linux-security-module, James Morris, linux-kernel@vger.kernel.org On Wed, 2014-03-05 at 11:26 +0200, Dmitry Kasatkin wrote: > On Tue, Mar 4, 2014 at 10:36 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote: > > On Tue, 2014-03-04 at 16:18 +0200, Dmitry Kasatkin wrote: > >> On Tue, Mar 4, 2014 at 5:21 AM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote: > >> > On Mon, 2014-03-03 at 19:00 -0800, Casey Schaufler wrote: > >> >> On 3/3/2014 6:39 PM, Mimi Zohar wrote: > >> >> > On Fri, 2014-02-28 at 16:59 +0200, Dmitry Kasatkin wrote: > >> >> >> EVM currently uses source hard coded list of xattrs which needs to be > >> >> >> included into the HMAC calculation. This is very unflexible. > >> >> >> Adding new attributes requires modifcation of the source code and > >> >> >> prevents building the kernel which works with previously labeled > >> >> >> filesystems. > >> >> >> > >> >> >> Early versions of Smack used only one xattr security.SMACK64, > >> >> >> which is protected by EVM. Now Smack has a few more attributes and > >> >> >> they are not protected. Adding support to the source code makes it > >> >> >> impossible to use new kernel with previousely labeled filesystems. > >> >> > I think this patch will break any existing filesystems labeled with > >> >> > 'security.smack64'. Details inline. > >> >> > > >> >> >> This patch replaces hardcoded xattr array with dynamic list which is > >> >> >> initialized from CONFIG_EVM_HMAC_XATTRS variable. It allows to build > >> >> >> kernel with with support of older and newer EVM HMAC formats. > > > > So instead of having a single kernel, this allows you to build different > > kernels with different xattr labels included in the HMAC. Wouldn't you > > want a migration mode, similar to 'fix' mode, that only updates the > > HMAC, if the existing HMAC verified based on the prior set of xattrs? > > > > It would require to maintain 2 sets of xattrs: "old" and "new". > What if "new" becomes "old" again, while there were still not updated > previous "old" :) > > I think it would bring unnecessary complexity. > System designers define what xattrs they want to protect and stick with that. > Filesystems stays anyway, but allows to upgrade to new kernels. > > If someone really wants to upgrade the system, just use "evm=fix" and run > recursive fix.., e.g. 'evmctl -r ima_fix /' Right, but instead of fixing everything, we would want a "safe" fix. Verify the existing HMAC, before updating the HMAC based on the new set of xattrs. With this design, only an "old" and "new" set of xattrs would be needed. Mimi ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 8/8] evm: introduce EVM hmac xattr list 2014-03-05 16:04 ` Mimi Zohar @ 2014-03-05 22:20 ` Dmitry Kasatkin 0 siblings, 0 replies; 31+ messages in thread From: Dmitry Kasatkin @ 2014-03-05 22:20 UTC (permalink / raw) To: Mimi Zohar Cc: Casey Schaufler, Dmitry Kasatkin, linux-security-module, James Morris, linux-kernel@vger.kernel.org On Wed, Mar 5, 2014 at 6:04 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote: > On Wed, 2014-03-05 at 11:26 +0200, Dmitry Kasatkin wrote: >> On Tue, Mar 4, 2014 at 10:36 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote: >> > On Tue, 2014-03-04 at 16:18 +0200, Dmitry Kasatkin wrote: >> >> On Tue, Mar 4, 2014 at 5:21 AM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote: >> >> > On Mon, 2014-03-03 at 19:00 -0800, Casey Schaufler wrote: >> >> >> On 3/3/2014 6:39 PM, Mimi Zohar wrote: >> >> >> > On Fri, 2014-02-28 at 16:59 +0200, Dmitry Kasatkin wrote: >> >> >> >> EVM currently uses source hard coded list of xattrs which needs to be >> >> >> >> included into the HMAC calculation. This is very unflexible. >> >> >> >> Adding new attributes requires modifcation of the source code and >> >> >> >> prevents building the kernel which works with previously labeled >> >> >> >> filesystems. >> >> >> >> >> >> >> >> Early versions of Smack used only one xattr security.SMACK64, >> >> >> >> which is protected by EVM. Now Smack has a few more attributes and >> >> >> >> they are not protected. Adding support to the source code makes it >> >> >> >> impossible to use new kernel with previousely labeled filesystems. >> >> >> > I think this patch will break any existing filesystems labeled with >> >> >> > 'security.smack64'. Details inline. >> >> >> > >> >> >> >> This patch replaces hardcoded xattr array with dynamic list which is >> >> >> >> initialized from CONFIG_EVM_HMAC_XATTRS variable. It allows to build >> >> >> >> kernel with with support of older and newer EVM HMAC formats. >> > >> > So instead of having a single kernel, this allows you to build different >> > kernels with different xattr labels included in the HMAC. Wouldn't you >> > want a migration mode, similar to 'fix' mode, that only updates the >> > HMAC, if the existing HMAC verified based on the prior set of xattrs? >> > >> >> It would require to maintain 2 sets of xattrs: "old" and "new". >> What if "new" becomes "old" again, while there were still not updated >> previous "old" :) >> >> I think it would bring unnecessary complexity. >> System designers define what xattrs they want to protect and stick with that. >> Filesystems stays anyway, but allows to upgrade to new kernels. >> >> If someone really wants to upgrade the system, just use "evm=fix" and run >> recursive fix.., e.g. 'evmctl -r ima_fix /' > > Right, but instead of fixing everything, we would want a "safe" fix. > Verify the existing HMAC, before updating the HMAC based on the new set > of xattrs. With this design, only an "old" and "new" set of xattrs > would be needed. > This can be the next step further. Now we need to support new xattrs in the kernel, without breaking existing labeled filesystem. Dmitry > Mimi > -- Thanks, Dmitry ^ permalink raw reply [flat|nested] 31+ messages in thread
end of thread, other threads:[~2014-03-05 22:20 UTC | newest] Thread overview: 31+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-02-28 14:59 [PATCH 0/8] integrity: fixes and new features Dmitry Kasatkin 2014-02-28 14:59 ` [PATCH 1/8] ima: fix erronous removal of security.ima xattr Dmitry Kasatkin 2014-03-03 13:43 ` Mimi Zohar 2014-02-28 14:59 ` [PATCH 2/8] integrity: fix checkpatch errors Dmitry Kasatkin 2014-02-28 15:16 ` Dmitry Kasatkin 2014-03-03 13:41 ` Mimi Zohar 2014-03-03 14:01 ` Dmitry Kasatkin 2014-03-03 14:23 ` Mimi Zohar 2014-03-04 2:02 ` Mimi Zohar 2014-02-28 14:59 ` [PATCH 3/8] ima: return d_name.name if d_path fails Dmitry Kasatkin 2014-03-03 13:45 ` Mimi Zohar 2014-02-28 14:59 ` [PATCH 4/8] evm: EVM does not use MD5 Dmitry Kasatkin 2014-03-04 2:02 ` Mimi Zohar 2014-02-28 14:59 ` [PATCH 5/8] ima: skip memory allocation for empty files Dmitry Kasatkin 2014-03-04 2:03 ` Mimi Zohar 2014-02-28 14:59 ` [PATCH 6/8] evm: enable key retention service automatically Dmitry Kasatkin 2014-03-04 2:02 ` Mimi Zohar 2014-03-04 14:10 ` Dmitry Kasatkin 2014-03-04 19:25 ` Mimi Zohar 2014-02-28 14:59 ` [PATCH 7/8] evm: introduce EVM hmac attribute list Dmitry Kasatkin 2014-03-04 2:09 ` Mimi Zohar 2014-03-04 14:20 ` Dmitry Kasatkin 2014-02-28 14:59 ` [PATCH 8/8] evm: introduce EVM hmac xattr list Dmitry Kasatkin 2014-03-04 2:39 ` Mimi Zohar 2014-03-04 3:00 ` Casey Schaufler 2014-03-04 3:21 ` Mimi Zohar 2014-03-04 14:18 ` Dmitry Kasatkin 2014-03-04 20:36 ` Mimi Zohar 2014-03-05 9:26 ` Dmitry Kasatkin 2014-03-05 16:04 ` Mimi Zohar 2014-03-05 22:20 ` Dmitry Kasatkin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox