From: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
To: zohar@linux.ibm.com, bauerman@linux.ibm.com, nayna@linux.ibm.com,
sgrubb@redhat.com, paul@paul-moore.com
Cc: rgb@redhat.com, linux-integrity@vger.kernel.org,
linux-audit@redhat.com, linux-kernel@vger.kernel.org
Subject: [PATCH 1/2] IMA: pass error code in result parameter to integrity_audit_msg()
Date: Wed, 17 Jun 2020 13:44:35 -0700 [thread overview]
Message-ID: <20200617204436.2226-1-nramas@linux.microsoft.com> (raw)
The value passed in "result" parameter to integrity_audit_msg() is
not an error code in some instances. Update these instances so that
"result" parameter always contains an error code.
Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
---
security/integrity/ima/ima_appraise.c | 20 ++++++++++++--------
security/integrity/ima/ima_fs.c | 8 +++++---
2 files changed, 17 insertions(+), 11 deletions(-)
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index a9649b04b9f1..253dcb331249 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -226,7 +226,7 @@ static int xattr_verify(enum ima_hooks func, struct integrity_iint_cache *iint,
}
clear_bit(IMA_DIGSIG, &iint->atomic_flags);
if (xattr_len - sizeof(xattr_value->type) - hash_start >=
- iint->ima_hash->length)
+ iint->ima_hash->length) {
/*
* xattr length may be longer. md5 hash in previous
* version occupied 20 bytes in xattr, instead of 16
@@ -234,6 +234,9 @@ static int xattr_verify(enum ima_hooks func, struct integrity_iint_cache *iint,
rc = memcmp(&xattr_value->data[hash_start],
iint->ima_hash->digest,
iint->ima_hash->length);
+ if (rc)
+ rc = -EINVAL;
+ }
else
rc = -EINVAL;
if (rc) {
@@ -355,7 +358,7 @@ int ima_appraise_measurement(enum ima_hooks func,
struct dentry *dentry = file_dentry(file);
struct inode *inode = d_backing_inode(dentry);
enum integrity_status status = INTEGRITY_UNKNOWN;
- int rc = xattr_len;
+ int rc = -EACCES;
bool try_modsig = iint->flags & IMA_MODSIG_ALLOWED && modsig;
/* If not appraising a modsig, we need an xattr. */
@@ -363,10 +366,7 @@ int ima_appraise_measurement(enum ima_hooks func,
return INTEGRITY_UNKNOWN;
/* If reading the xattr failed and there's no modsig, error out. */
- if (rc <= 0 && !try_modsig) {
- if (rc && rc != -ENODATA)
- goto out;
-
+ if (xattr_len <= 0 && !try_modsig) {
cause = iint->flags & IMA_DIGSIG_REQUIRED ?
"IMA-signature-required" : "missing-hash";
status = INTEGRITY_NOLABEL;
@@ -379,7 +379,8 @@ int ima_appraise_measurement(enum ima_hooks func,
goto out;
}
- status = evm_verifyxattr(dentry, XATTR_NAME_IMA, xattr_value, rc, iint);
+ status = evm_verifyxattr(dentry, XATTR_NAME_IMA,
+ xattr_value, xattr_len, iint);
switch (status) {
case INTEGRITY_PASS:
case INTEGRITY_PASS_IMMUTABLE:
@@ -432,14 +433,17 @@ int ima_appraise_measurement(enum ima_hooks func,
if ((ima_appraise & IMA_APPRAISE_FIX) && !try_modsig &&
(!xattr_value ||
xattr_value->type != EVM_IMA_XATTR_DIGSIG)) {
- if (!ima_fix_xattr(dentry, iint))
+ if (!ima_fix_xattr(dentry, iint)) {
status = INTEGRITY_PASS;
+ rc = 0;
+ }
}
/* Permit new files with file signatures, but without data. */
if (inode->i_size == 0 && iint->flags & IMA_NEW_FILE &&
xattr_value && xattr_value->type == EVM_IMA_XATTR_DIGSIG) {
status = INTEGRITY_PASS;
+ rc = 0;
}
integrity_audit_msg(AUDIT_INTEGRITY_DATA, inode, filename,
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index e3fcad871861..a3a270cff94f 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -335,10 +335,10 @@ static ssize_t ima_write_policy(struct file *file, const char __user *buf,
result = ima_read_policy(data);
} else if (ima_appraise & IMA_APPRAISE_POLICY) {
pr_err("signed policy file (specified as an absolute pathname) required\n");
+ result = -EACCES;
integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL, NULL,
"policy_update", "signed policy required",
- 1, 0);
- result = -EACCES;
+ result, 0);
} else {
result = ima_parse_add_rule(data);
}
@@ -406,6 +406,7 @@ static int ima_open_policy(struct inode *inode, struct file *filp)
static int ima_release_policy(struct inode *inode, struct file *file)
{
const char *cause = valid_policy ? "completed" : "failed";
+ int result = 0;
if ((file->f_flags & O_ACCMODE) == O_RDONLY)
return seq_release(inode, file);
@@ -413,11 +414,12 @@ static int ima_release_policy(struct inode *inode, struct file *file)
if (valid_policy && ima_check_policy() < 0) {
cause = "failed";
valid_policy = 0;
+ result = -EINVAL;
}
pr_info("policy update %s\n", cause);
integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL, NULL,
- "policy_update", cause, !valid_policy, 0);
+ "policy_update", cause, result, 0);
if (!valid_policy) {
ima_delete_rules();
--
2.27.0
--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
WARNING: multiple messages have this Message-ID (diff)
From: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
To: zohar@linux.ibm.com, bauerman@linux.ibm.com, nayna@linux.ibm.com,
sgrubb@redhat.com, paul@paul-moore.com
Cc: rgb@redhat.com, linux-integrity@vger.kernel.org,
linux-audit@redhat.com, linux-kernel@vger.kernel.org
Subject: [PATCH 1/2] IMA: pass error code in result parameter to integrity_audit_msg()
Date: Wed, 17 Jun 2020 13:44:35 -0700 [thread overview]
Message-ID: <20200617204436.2226-1-nramas@linux.microsoft.com> (raw)
The value passed in "result" parameter to integrity_audit_msg() is
not an error code in some instances. Update these instances so that
"result" parameter always contains an error code.
Signed-off-by: Lakshmi Ramasubramanian <nramas@linux.microsoft.com>
---
security/integrity/ima/ima_appraise.c | 20 ++++++++++++--------
security/integrity/ima/ima_fs.c | 8 +++++---
2 files changed, 17 insertions(+), 11 deletions(-)
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index a9649b04b9f1..253dcb331249 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -226,7 +226,7 @@ static int xattr_verify(enum ima_hooks func, struct integrity_iint_cache *iint,
}
clear_bit(IMA_DIGSIG, &iint->atomic_flags);
if (xattr_len - sizeof(xattr_value->type) - hash_start >=
- iint->ima_hash->length)
+ iint->ima_hash->length) {
/*
* xattr length may be longer. md5 hash in previous
* version occupied 20 bytes in xattr, instead of 16
@@ -234,6 +234,9 @@ static int xattr_verify(enum ima_hooks func, struct integrity_iint_cache *iint,
rc = memcmp(&xattr_value->data[hash_start],
iint->ima_hash->digest,
iint->ima_hash->length);
+ if (rc)
+ rc = -EINVAL;
+ }
else
rc = -EINVAL;
if (rc) {
@@ -355,7 +358,7 @@ int ima_appraise_measurement(enum ima_hooks func,
struct dentry *dentry = file_dentry(file);
struct inode *inode = d_backing_inode(dentry);
enum integrity_status status = INTEGRITY_UNKNOWN;
- int rc = xattr_len;
+ int rc = -EACCES;
bool try_modsig = iint->flags & IMA_MODSIG_ALLOWED && modsig;
/* If not appraising a modsig, we need an xattr. */
@@ -363,10 +366,7 @@ int ima_appraise_measurement(enum ima_hooks func,
return INTEGRITY_UNKNOWN;
/* If reading the xattr failed and there's no modsig, error out. */
- if (rc <= 0 && !try_modsig) {
- if (rc && rc != -ENODATA)
- goto out;
-
+ if (xattr_len <= 0 && !try_modsig) {
cause = iint->flags & IMA_DIGSIG_REQUIRED ?
"IMA-signature-required" : "missing-hash";
status = INTEGRITY_NOLABEL;
@@ -379,7 +379,8 @@ int ima_appraise_measurement(enum ima_hooks func,
goto out;
}
- status = evm_verifyxattr(dentry, XATTR_NAME_IMA, xattr_value, rc, iint);
+ status = evm_verifyxattr(dentry, XATTR_NAME_IMA,
+ xattr_value, xattr_len, iint);
switch (status) {
case INTEGRITY_PASS:
case INTEGRITY_PASS_IMMUTABLE:
@@ -432,14 +433,17 @@ int ima_appraise_measurement(enum ima_hooks func,
if ((ima_appraise & IMA_APPRAISE_FIX) && !try_modsig &&
(!xattr_value ||
xattr_value->type != EVM_IMA_XATTR_DIGSIG)) {
- if (!ima_fix_xattr(dentry, iint))
+ if (!ima_fix_xattr(dentry, iint)) {
status = INTEGRITY_PASS;
+ rc = 0;
+ }
}
/* Permit new files with file signatures, but without data. */
if (inode->i_size == 0 && iint->flags & IMA_NEW_FILE &&
xattr_value && xattr_value->type == EVM_IMA_XATTR_DIGSIG) {
status = INTEGRITY_PASS;
+ rc = 0;
}
integrity_audit_msg(AUDIT_INTEGRITY_DATA, inode, filename,
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index e3fcad871861..a3a270cff94f 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -335,10 +335,10 @@ static ssize_t ima_write_policy(struct file *file, const char __user *buf,
result = ima_read_policy(data);
} else if (ima_appraise & IMA_APPRAISE_POLICY) {
pr_err("signed policy file (specified as an absolute pathname) required\n");
+ result = -EACCES;
integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL, NULL,
"policy_update", "signed policy required",
- 1, 0);
- result = -EACCES;
+ result, 0);
} else {
result = ima_parse_add_rule(data);
}
@@ -406,6 +406,7 @@ static int ima_open_policy(struct inode *inode, struct file *filp)
static int ima_release_policy(struct inode *inode, struct file *file)
{
const char *cause = valid_policy ? "completed" : "failed";
+ int result = 0;
if ((file->f_flags & O_ACCMODE) == O_RDONLY)
return seq_release(inode, file);
@@ -413,11 +414,12 @@ static int ima_release_policy(struct inode *inode, struct file *file)
if (valid_policy && ima_check_policy() < 0) {
cause = "failed";
valid_policy = 0;
+ result = -EINVAL;
}
pr_info("policy update %s\n", cause);
integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL, NULL,
- "policy_update", cause, !valid_policy, 0);
+ "policy_update", cause, result, 0);
if (!valid_policy) {
ima_delete_rules();
--
2.27.0
next reply other threads:[~2020-06-17 20:56 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-17 20:44 Lakshmi Ramasubramanian [this message]
2020-06-17 20:44 ` [PATCH 1/2] IMA: pass error code in result parameter to integrity_audit_msg() Lakshmi Ramasubramanian
2020-06-17 20:44 ` [PATCH 2/2] integrity: Add errno field in audit message Lakshmi Ramasubramanian
2020-06-17 20:44 ` Lakshmi Ramasubramanian
2020-06-17 21:15 ` Paul Moore
2020-06-17 21:15 ` Paul Moore
2020-06-17 22:36 ` Steve Grubb
2020-06-17 22:36 ` Steve Grubb
2020-06-18 17:41 ` Mimi Zohar
2020-06-18 17:41 ` Mimi Zohar
2020-06-18 18:05 ` Lakshmi Ramasubramanian
2020-06-18 18:05 ` Lakshmi Ramasubramanian
2020-06-18 18:10 ` Mimi Zohar
2020-06-18 18:10 ` Mimi Zohar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200617204436.2226-1-nramas@linux.microsoft.com \
--to=nramas@linux.microsoft.com \
--cc=bauerman@linux.ibm.com \
--cc=linux-audit@redhat.com \
--cc=linux-integrity@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=nayna@linux.ibm.com \
--cc=paul@paul-moore.com \
--cc=rgb@redhat.com \
--cc=sgrubb@redhat.com \
--cc=zohar@linux.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.