3:44 p.m.
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(a)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
3:44 p.m.
New subject: [PATCH 2/2] integrity: Add errno field in audit message
Error code is not included in the audit messages logged by
the integrity subsystem. Add "errno" field in the audit messages
logged by the integrity subsystem and set the value to the error code
passed to integrity_audit_msg() in the "result" parameter.
Sample audit messages:
[ 6.284329] audit: type=1804 audit(1591756723.627:2): pid=1 uid=0 auid=4294967295
ses=4294967295 subj=kernel op=add_boot_aggregate cause=alloc_entry
comm="swapper/0" name="boot_aggregate" res=0 errno=-12
[ 8.085456] audit: type=1802 audit(1592005947.297:9): pid=1 uid=0 auid=4294967295
ses=4294967295 subj=system_u:system_r:init_t:s0 op=policy_update cause=completed
comm="systemd" res=1 errno=0
Signed-off-by: Lakshmi Ramasubramanian <nramas(a)linux.microsoft.com>
Suggested-by: Steve Grubb <sgrubb(a)redhat.com>
---
security/integrity/integrity_audit.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/security/integrity/integrity_audit.c b/security/integrity/integrity_audit.c
index 5109173839cc..a265024f82f3 100644
--- a/security/integrity/integrity_audit.c
+++ b/security/integrity/integrity_audit.c
@@ -53,6 +53,6 @@ void integrity_audit_msg(int audit_msgno, struct inode *inode,
audit_log_untrustedstring(ab, inode->i_sb->s_id);
audit_log_format(ab, " ino=%lu", inode->i_ino);
}
- audit_log_format(ab, " res=%d", !result);
+ audit_log_format(ab, " res=%d errno=%d", !result, result);
audit_log_end(ab);
}
--
2.27.0
4:15 p.m.
New subject: [PATCH 2/2] integrity: Add errno field in audit message
On Wed, Jun 17, 2020 at 4:44 PM Lakshmi Ramasubramanian
<nramas(a)linux.microsoft.com> wrote:
Error code is not included in the audit messages logged by
the integrity subsystem. Add "errno" field in the audit messages
logged by the integrity subsystem and set the value to the error code
passed to integrity_audit_msg() in the "result" parameter.
Sample audit messages:
[ 6.284329] audit: type=1804 audit(1591756723.627:2): pid=1 uid=0 auid=4294967295
ses=4294967295 subj=kernel op=add_boot_aggregate cause=alloc_entry
comm="swapper/0" name="boot_aggregate" res=0 errno=-12
[ 8.085456] audit: type=1802 audit(1592005947.297:9): pid=1 uid=0 auid=4294967295
ses=4294967295 subj=system_u:system_r:init_t:s0 op=policy_update cause=completed
comm="systemd" res=1 errno=0
Signed-off-by: Lakshmi Ramasubramanian <nramas(a)linux.microsoft.com>
Suggested-by: Steve Grubb <sgrubb(a)redhat.com>
---
security/integrity/integrity_audit.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Acked-by: Paul Moore <paul(a)paul-moore.com>
diff --git a/security/integrity/integrity_audit.c
b/security/integrity/integrity_audit.c
index 5109173839cc..a265024f82f3 100644
--- a/security/integrity/integrity_audit.c
+++ b/security/integrity/integrity_audit.c
@@ -53,6 +53,6 @@ void integrity_audit_msg(int audit_msgno, struct inode *inode,
audit_log_untrustedstring(ab, inode->i_sb->s_id);
audit_log_format(ab, " ino=%lu", inode->i_ino);
}
- audit_log_format(ab, " res=%d", !result);
+ audit_log_format(ab, " res=%d errno=%d", !result, result);
audit_log_end(ab);
}
--
2.27.0
--
paul moore
www.paul-moore.com
5:36 p.m.
New subject: [PATCH 2/2] integrity: Add errno field in audit message
On Wednesday, June 17, 2020 4:44:36 PM EDT Lakshmi Ramasubramanian wrote:
Error code is not included in the audit messages logged by
the integrity subsystem. Add "errno" field in the audit messages
logged by the integrity subsystem and set the value to the error code
passed to integrity_audit_msg() in the "result" parameter.
Sample audit messages:
[ 6.284329] audit: type=1804 audit(1591756723.627:2): pid=1 uid=0
auid=4294967295 ses=4294967295 subj=kernel op=add_boot_aggregate
cause=alloc_entry comm="swapper/0" name="boot_aggregate" res=0
errno=-12
[ 8.085456] audit: type=1802 audit(1592005947.297:9): pid=1 uid=0
auid=4294967295 ses=4294967295 subj=system_u:system_r:init_t:s0
op=policy_update cause=completed comm="systemd" res=1 errno=0
Signed-off-by: Lakshmi Ramasubramanian <nramas(a)linux.microsoft.com>
Suggested-by: Steve Grubb <sgrubb(a)redhat.com>
Acked-by: Steve Grubb <sgrubb(a)redhat.com>
---
security/integrity/integrity_audit.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/security/integrity/integrity_audit.c
b/security/integrity/integrity_audit.c index 5109173839cc..a265024f82f3
100644
--- a/security/integrity/integrity_audit.c
+++ b/security/integrity/integrity_audit.c
@@ -53,6 +53,6 @@ void integrity_audit_msg(int audit_msgno, struct inode
*inode, audit_log_untrustedstring(ab, inode->i_sb->s_id);
audit_log_format(ab, " ino=%lu", inode->i_ino);
}
- audit_log_format(ab, " res=%d", !result);
+ audit_log_format(ab, " res=%d errno=%d", !result, result);
audit_log_end(ab);
}
12:41 p.m.
New subject: [PATCH 2/2] integrity: Add errno field in audit message
On Wed, 2020-06-17 at 13:44 -0700, Lakshmi Ramasubramanian wrote:
Error code is not included in the audit messages logged by
the integrity subsystem. Add "errno" field in the audit messages
logged by the integrity subsystem and set the value to the error code
passed to integrity_audit_msg() in the "result" parameter.
Sample audit messages:
[ 6.284329] audit: type=1804 audit(1591756723.627:2): pid=1 uid=0 auid=4294967295
ses=4294967295 subj=kernel op=add_boot_aggregate cause=alloc_entry
comm="swapper/0" name="boot_aggregate" res=0 errno=-12
[ 8.085456] audit: type=1802 audit(1592005947.297:9): pid=1 uid=0 auid=4294967295
ses=4294967295 subj=system_u:system_r:init_t:s0 op=policy_update cause=completed
comm="systemd" res=1 errno=0
Signed-off-by: Lakshmi Ramasubramanian <nramas(a)linux.microsoft.com>
Suggested-by: Steve Grubb <sgrubb(a)redhat.com>
---
security/integrity/integrity_audit.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/security/integrity/integrity_audit.c b/security/integrity/integrity_audit.c
index 5109173839cc..a265024f82f3 100644
--- a/security/integrity/integrity_audit.c
+++ b/security/integrity/integrity_audit.c
@@ -53,6 +53,6 @@ void integrity_audit_msg(int audit_msgno, struct inode *inode,
audit_log_untrustedstring(ab, inode->i_sb->s_id);
audit_log_format(ab, " ino=%lu", inode->i_ino);
}
- audit_log_format(ab, " res=%d", !result);
+ audit_log_format(ab, " res=%d errno=%d", !result, result);
audit_log_end(ab);
}
For the reasons that I mentioned previously, unless others are willing
to add their Reviewed-by tag not for the audit aspect in particular,
but IMA itself, I'm not comfortable making this change all at once.
Previously I suggested making the existing integrity_audit_msg() a
wrapper for a new function with errno. Steve said, "We normally do
not like to have fields that swing in and out ...", but said setting
errno to 0 is fine. The original integrity_audit_msg() function would
call the new function with errno set to 0.
Mimi
1:05 p.m.
New subject: [PATCH 2/2] integrity: Add errno field in audit message
On 6/18/20 10:41 AM, Mimi Zohar wrote:
For the reasons that I mentioned previously, unless others are willing
to add their Reviewed-by tag not for the audit aspect in particular,
but IMA itself, I'm not comfortable making this change all at once.
Previously I suggested making the existing integrity_audit_msg() a
wrapper for a new function with errno. Steve said, "We normally do
not like to have fields that swing in and out ...", but said setting
errno to 0 is fine. The original integrity_audit_msg() function would
call the new function with errno set to 0.
If the original integrity_audit_msg() always calls the new function with
errno set to 0, there would be audit messages where "res" field is set
to "0" (fail) because "result" was non-zero, but errno set to
"0"
(success). Wouldn't this be confusing?
In PATCH 1/2 I've made changes to make the "result" parameter to
integrity_audit_msg() consistent - i.e., it is always an error code (0
for success and a negative value for error). Would that address your
concerns?
thanks,
-lakshmi
1:10 p.m.
New subject: [PATCH 2/2] integrity: Add errno field in audit message
On Thu, 2020-06-18 at 11:05 -0700, Lakshmi Ramasubramanian wrote:
On 6/18/20 10:41 AM, Mimi Zohar wrote:
>
> For the reasons that I mentioned previously, unless others are willing
> to add their Reviewed-by tag not for the audit aspect in particular,
> but IMA itself, I'm not comfortable making this change all at once.
>
> Previously I suggested making the existing integrity_audit_msg() a
> wrapper for a new function with errno. Steve said, "We normally do
> not like to have fields that swing in and out ...", but said setting
> errno to 0 is fine. The original integrity_audit_msg() function would
> call the new function with errno set to 0.
If the original integrity_audit_msg() always calls the new function with
errno set to 0, there would be audit messages where "res" field is set
to "0" (fail) because "result" was non-zero, but errno set to
"0"
(success). Wouldn't this be confusing?
In PATCH 1/2 I've made changes to make the "result" parameter to
integrity_audit_msg() consistent - i.e., it is always an error code (0
for success and a negative value for error). Would that address your
concerns?
You're overloading "res" to imply errno. Define a new parameter
specifically for errno.
Mimi
1596
days inactive
1597
days old
linux-audit@lists.linux-audit.osci.io
6 comments
4 participants
participants (4)
-
Lakshmi Ramasubramanian -
Mimi Zohar -
Paul Moore -
Steve Grubb