From def3e8b9ee23cb69036910e48ec4e3eff40e04cb Mon Sep 17 00:00:00 2001 From: Dmitry Kasatkin Date: Thu, 20 Sep 2012 22:38:53 +0300 Subject: ima: set appraise status in fix mode only when xattr is fixed When a file system is mounted read-only, setting the xattr value in fix mode fails with an error code -EROFS. The xattr should be fixed after the file system is remounted read-write. This patch verifies that the set xattr succeeds, before setting the appraise status value to INTEGRITY_PASS. Signed-off-by: Dmitry Kasatkin Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_appraise.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) (limited to 'security/integrity/ima/ima_appraise.c') diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index bdc8ba1d1d2..b240c58403e 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -42,12 +42,13 @@ int ima_must_appraise(struct inode *inode, int mask, enum ima_hooks func) return ima_match_policy(inode, func, mask, IMA_APPRAISE); } -static void ima_fix_xattr(struct dentry *dentry, +static int ima_fix_xattr(struct dentry *dentry, struct integrity_iint_cache *iint) { iint->ima_xattr.type = IMA_XATTR_DIGEST; - __vfs_setxattr_noperm(dentry, XATTR_NAME_IMA, (u8 *)&iint->ima_xattr, - sizeof iint->ima_xattr, 0); + return __vfs_setxattr_noperm(dentry, XATTR_NAME_IMA, + (u8 *)&iint->ima_xattr, + sizeof(iint->ima_xattr), 0); } /* @@ -141,8 +142,8 @@ out: if ((ima_appraise & IMA_APPRAISE_FIX) && (!xattr_value || xattr_value->type != EVM_IMA_XATTR_DIGSIG)) { - ima_fix_xattr(dentry, iint); - status = INTEGRITY_PASS; + if (!ima_fix_xattr(dentry, iint)) + status = INTEGRITY_PASS; } integrity_audit_msg(AUDIT_INTEGRITY_DATA, inode, filename, op, cause, rc, 0); -- cgit v1.2.3 From b51524635b73cfa27cc393859b277cee9c042820 Mon Sep 17 00:00:00 2001 From: Dmitry Kasatkin Date: Fri, 21 Sep 2012 01:01:29 +0300 Subject: ima: remove security.ima hexdump Hexdump is not really helping. Audit messages prints error messages. Remove it. Signed-off-by: Dmitry Kasatkin Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_appraise.c | 5 ----- 1 file changed, 5 deletions(-) (limited to 'security/integrity/ima/ima_appraise.c') diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index b240c58403e..fa675c907e0 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -107,11 +107,6 @@ int ima_appraise_measurement(struct integrity_iint_cache *iint, if (rc) { cause = "invalid-hash"; status = INTEGRITY_FAIL; - print_hex_dump_bytes("security.ima: ", DUMP_PREFIX_NONE, - xattr_value, sizeof(*xattr_value)); - print_hex_dump_bytes("collected: ", DUMP_PREFIX_NONE, - (u8 *)&iint->ima_xattr, - sizeof iint->ima_xattr); break; } status = INTEGRITY_PASS; -- cgit v1.2.3 From 0e5a247cb37a97d843ef76d09d5f80deb7893ba3 Mon Sep 17 00:00:00 2001 From: Dmitry Kasatkin Date: Fri, 8 Jun 2012 13:58:49 +0300 Subject: ima: added policy support for 'security.ima' type The 'security.ima' extended attribute may contain either the file data's hash or a digital signature. This patch adds support for requiring a specific extended attribute type. It extends the IMA policy with a new keyword 'appraise_type=imasig'. (Default is hash.) Changelog v2: - Fixed Documentation/ABI/testing/ima_policy option syntax Changelog v1: - Differentiate between 'required' vs. 'actual' extended attribute Signed-off-by: Dmitry Kasatkin Signed-off-by: Mimi Zohar --- security/integrity/ima/ima_appraise.c | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'security/integrity/ima/ima_appraise.c') diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index fa675c907e0..8004332ccb8 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -102,6 +102,11 @@ int ima_appraise_measurement(struct integrity_iint_cache *iint, switch (xattr_value->type) { case IMA_XATTR_DIGEST: + if (iint->flags & IMA_DIGSIG_REQUIRED) { + cause = "IMA signature required"; + status = INTEGRITY_FAIL; + break; + } rc = memcmp(xattr_value->digest, iint->ima_xattr.digest, IMA_DIGEST_SIZE); if (rc) { -- cgit v1.2.3 From d79d72e02485c00b886179538dc8deaffa3be507 Mon Sep 17 00:00:00 2001 From: Mimi Zohar Date: Mon, 3 Dec 2012 17:08:11 -0500 Subject: ima: per hook cache integrity appraisal status With the new IMA policy 'appraise_type=' option, different hooks can require different methods for appraising a file's integrity. For example, the existing 'ima_appraise_tcb' policy defines a generic rule, requiring all root files to be appraised, without specfying the appraisal method. A more specific rule could require all kernel modules, for example, to be signed. appraise fowner=0 func=MODULE_CHECK appraise_type=imasig appraise fowner=0 As a result, the integrity appraisal results for the same inode, but for different hooks, could differ. This patch caches the integrity appraisal results on a per hook basis. Changelog v2: - Rename ima_cache_status() to ima_set_cache_status() - Rename and move get_appraise_status() to ima_get_cache_status() Changelog v0: - include IMA_APPRAISE/APPRAISED_SUBMASK in IMA_DO/DONE_MASK (Dmitry) - Support independent MODULE_CHECK appraise status. - fixed IMA_XXXX_APPRAISE/APPRAISED flags Signed-off-by: Mimi Zohar Signed-off-by: Dmitry Kasatkin --- security/integrity/ima/ima_appraise.c | 71 ++++++++++++++++++++++++++++++----- 1 file changed, 62 insertions(+), 9 deletions(-) (limited to 'security/integrity/ima/ima_appraise.c') diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index 8004332ccb8..2d4becab891 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -51,6 +51,62 @@ static int ima_fix_xattr(struct dentry *dentry, sizeof(iint->ima_xattr), 0); } +/* Return specific func appraised cached result */ +enum integrity_status ima_get_cache_status(struct integrity_iint_cache *iint, + int func) +{ + switch(func) { + case MMAP_CHECK: + return iint->ima_mmap_status; + case BPRM_CHECK: + return iint->ima_bprm_status; + case MODULE_CHECK: + return iint->ima_module_status; + case FILE_CHECK: + default: + return iint->ima_file_status; + } +} + +static void ima_set_cache_status(struct integrity_iint_cache *iint, + int func, enum integrity_status status) +{ + switch(func) { + case MMAP_CHECK: + iint->ima_mmap_status = status; + break; + case BPRM_CHECK: + iint->ima_bprm_status = status; + break; + case MODULE_CHECK: + iint->ima_module_status = status; + break; + case FILE_CHECK: + default: + iint->ima_file_status = status; + break; + } +} + +static void ima_cache_flags(struct integrity_iint_cache *iint, int func) +{ + switch(func) { + case MMAP_CHECK: + iint->flags |= (IMA_MMAP_APPRAISED | IMA_APPRAISED); + break; + case BPRM_CHECK: + iint->flags |= (IMA_BPRM_APPRAISED | IMA_APPRAISED); + break; + case MODULE_CHECK: + iint->flags |= (IMA_MODULE_APPRAISED | IMA_APPRAISED); + break; + case FILE_CHECK: + default: + iint->flags |= (IMA_FILE_APPRAISED | IMA_APPRAISED); + break; + } +} + /* * ima_appraise_measurement - appraise file measurement * @@ -59,7 +115,7 @@ static int ima_fix_xattr(struct dentry *dentry, * * Return 0 on success, error code otherwise */ -int ima_appraise_measurement(struct integrity_iint_cache *iint, +int ima_appraise_measurement(int func, struct integrity_iint_cache *iint, struct file *file, const unsigned char *filename) { struct dentry *dentry = file->f_dentry; @@ -75,9 +131,6 @@ int ima_appraise_measurement(struct integrity_iint_cache *iint, if (!inode->i_op->getxattr) return INTEGRITY_UNKNOWN; - if (iint->flags & IMA_APPRAISED) - return iint->ima_status; - rc = vfs_getxattr_alloc(dentry, XATTR_NAME_IMA, (char **)&xattr_value, 0, GFP_NOFS); if (rc <= 0) { @@ -99,7 +152,6 @@ int ima_appraise_measurement(struct integrity_iint_cache *iint, cause = "invalid-HMAC"; goto out; } - switch (xattr_value->type) { case IMA_XATTR_DIGEST: if (iint->flags & IMA_DIGSIG_REQUIRED) { @@ -148,9 +200,9 @@ out: integrity_audit_msg(AUDIT_INTEGRITY_DATA, inode, filename, op, cause, rc, 0); } else { - iint->flags |= IMA_APPRAISED; + ima_cache_flags(iint, func); } - iint->ima_status = status; + ima_set_cache_status(iint, func, status); kfree(xattr_value); return status; } @@ -196,10 +248,11 @@ void ima_inode_post_setattr(struct dentry *dentry) must_appraise = ima_must_appraise(inode, MAY_ACCESS, POST_SETATTR); iint = integrity_iint_find(inode); if (iint) { + iint->flags &= ~(IMA_APPRAISE | IMA_APPRAISED | + IMA_APPRAISE_SUBMASK | IMA_APPRAISED_SUBMASK | + IMA_ACTION_FLAGS); if (must_appraise) iint->flags |= IMA_APPRAISE; - else - iint->flags &= ~(IMA_APPRAISE | IMA_APPRAISED); } if (!must_appraise) rc = inode->i_op->removexattr(dentry, XATTR_NAME_IMA); -- cgit v1.2.3