Re: [PATCH] audit: optionally print warning after waiting to enqueue record
On Wed, Jun 17, 2020 at 6:54 PM Max Englander wrote: > On Wed, Jun 17, 2020 at 02:47:19PM -0400, Paul Moore wrote: > > On Tue, Jun 16, 2020 at 12:58 AM Max Englander > > wrote: > > > > > > In environments where security is prioritized, users may set > > > --backlog_wait_time to a high value in order to reduce the likelihood > > > that any audit event is lost, even though doing so may result in > > > unpredictable performance if the kernel schedules a timeout when the > > > backlog limit is exceeded. For these users, the next best thing to > > > predictable performance is the ability to quickly detect and react to > > > degraded performance. This patch proposes to aid the detection of kernel > > > audit subsystem pauses through the following changes: > > > > > > Add a variable named audit_backlog_warn_time. Enforce the value of this > > > variable to be no less than zero, and no more than the value of > > > audit_backlog_wait_time. > > > > > > If audit_backlog_warn_time is greater than zero and if the total time > > > spent waiting to enqueue an audit record is greater than or equal to > > > audit_backlog_warn_time, then print a warning with the total time > > > spent waiting. > > > > > > An example configuration: > > > > > > auditctl --backlog_warn_time 50 > > > > > > An example warning message: > > > > > > audit: sleep_time=52 >= audit_backlog_warn_time=50 > > > > > > Tested on Ubuntu 18.04.04 using complementary changes to the audit > > > userspace: https://github.com/linux-audit/audit-userspace/pull/131. > > > > > > Signed-off-by: Max Englander > > > --- > > > include/uapi/linux/audit.h | 7 ++- > > > kernel/audit.c | 35 +++ > > > 2 files changed, 41 insertions(+), 1 deletion(-) > > > > If an admin is prioritizing security, aka don't loose any audit > > records, and there is a concern over variable system latency due to an > > audit queue backlog, why not simply disable the backlog limit? > > > > -- > > paul moore > > www.paul-moore.com > > That’s good in some cases, but in other cases unbounded growth of the > backlog could result in memory issues. If the kernel runs out of memory > it would drop the audit event or possibly have other problems. It could > also also consume memory in a way that starves user workloads or causes > them to be killed by the OOMKiller. > > To refine my motivating use case a bit, if a Kubernetes admin wants to > prioritize security, and also avoid unbounded growth of the audit > backlog, they may set -b and --backlog_wait_time in a way that limits > kernel memory usage and reduces the likelihood that any audit event is > lost. Occasional performance degradation may be acceptable to the admin, > but they would like a way to be alerted to prolonged kernel pauses, so > that they can investigate and take corrective action (increase backlog, > increase server capacity, move some workloads to other servers, etc.). > > To state another way. The kernel currently can be configured to print a > message when the backlog limit is exceeded and it must discard the audit > event. This is a useful message for admins, which they can address with > corrective action. I think a message similar to the one proposed by this > patch would be equally useful when the backlog limit is exceeded and the > kernel is configured to wait for the backlog to drain. Admins could > address that message in the same way, but without the cost of lost audit > events. I'm still struggling to understand how this is any better than disabling the backlog limit, or setting it very high, and simply monitoring the audit size of the audit backlog. This way the admin doesn't have to worry about the latency issues of a full backlog, while still being able to trigger actions based on the state of the backlog. The userspace tooling/scripting to watch the backlog size would be trivial, and would arguably provide much better visibility into the backlog state than a single warning threshold in the kernel. -- paul moore www.paul-moore.com -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit
Re: [PATCH] audit: optionally print warning after waiting to enqueue record
On Wed, Jun 17, 2020 at 02:47:19PM -0400, Paul Moore wrote: > On Tue, Jun 16, 2020 at 12:58 AM Max Englander > wrote: > > > > In environments where security is prioritized, users may set > > --backlog_wait_time to a high value in order to reduce the likelihood > > that any audit event is lost, even though doing so may result in > > unpredictable performance if the kernel schedules a timeout when the > > backlog limit is exceeded. For these users, the next best thing to > > predictable performance is the ability to quickly detect and react to > > degraded performance. This patch proposes to aid the detection of kernel > > audit subsystem pauses through the following changes: > > > > Add a variable named audit_backlog_warn_time. Enforce the value of this > > variable to be no less than zero, and no more than the value of > > audit_backlog_wait_time. > > > > If audit_backlog_warn_time is greater than zero and if the total time > > spent waiting to enqueue an audit record is greater than or equal to > > audit_backlog_warn_time, then print a warning with the total time > > spent waiting. > > > > An example configuration: > > > > auditctl --backlog_warn_time 50 > > > > An example warning message: > > > > audit: sleep_time=52 >= audit_backlog_warn_time=50 > > > > Tested on Ubuntu 18.04.04 using complementary changes to the audit > > userspace: https://github.com/linux-audit/audit-userspace/pull/131. > > > > Signed-off-by: Max Englander > > --- > > include/uapi/linux/audit.h | 7 ++- > > kernel/audit.c | 35 +++ > > 2 files changed, 41 insertions(+), 1 deletion(-) > > If an admin is prioritizing security, aka don't loose any audit > records, and there is a concern over variable system latency due to an > audit queue backlog, why not simply disable the backlog limit? > > -- > paul moore > www.paul-moore.com That’s good in some cases, but in other cases unbounded growth of the backlog could result in memory issues. If the kernel runs out of memory it would drop the audit event or possibly have other problems. It could also also consume memory in a way that starves user workloads or causes them to be killed by the OOMKiller. To refine my motivating use case a bit, if a Kubernetes admin wants to prioritize security, and also avoid unbounded growth of the audit backlog, they may set -b and --backlog_wait_time in a way that limits kernel memory usage and reduces the likelihood that any audit event is lost. Occasional performance degradation may be acceptable to the admin, but they would like a way to be alerted to prolonged kernel pauses, so that they can investigate and take corrective action (increase backlog, increase server capacity, move some workloads to other servers, etc.). To state another way. The kernel currently can be configured to print a message when the backlog limit is exceeded and it must discard the audit event. This is a useful message for admins, which they can address with corrective action. I think a message similar to the one proposed by this patch would be equally useful when the backlog limit is exceeded and the kernel is configured to wait for the backlog to drain. Admins could address that message in the same way, but without the cost of lost audit events. -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit
Re: [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 > Suggested-by: Steve Grubb Acked-by: Steve Grubb > --- > 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); > } -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit
Re: [PATCH ghak90 V8 07/16] audit: add contid support for signalling the audit daemon
On Mon, Jun 8, 2020 at 2:04 PM Richard Guy Briggs wrote: > On 2020-04-22 13:24, Paul Moore wrote: > > On Fri, Apr 17, 2020 at 6:26 PM Eric W. Biederman > > wrote: > > > Paul Moore writes: > > > > On Thu, Apr 16, 2020 at 4:36 PM Eric W. Biederman > > > > wrote: > > > >> Paul Moore writes: > > > >> > On Mon, Mar 30, 2020 at 1:49 PM Richard Guy Briggs > > > >> > wrote: > > > >> >> On 2020-03-30 13:34, Paul Moore wrote: > > > >> >> > On Mon, Mar 30, 2020 at 12:22 PM Richard Guy Briggs > > > >> >> > wrote: > > > >> >> > > On 2020-03-30 10:26, Paul Moore wrote: > > > >> >> > > > On Mon, Mar 30, 2020 at 9:47 AM Richard Guy Briggs > > > >> >> > > > wrote: > > > >> >> > > > > On 2020-03-28 23:11, Paul Moore wrote: > > > >> >> > > > > > On Tue, Mar 24, 2020 at 5:02 PM Richard Guy Briggs > > > >> >> > > > > > wrote: > > > >> >> > > > > > > On 2020-03-23 20:16, Paul Moore wrote: > > > >> >> > > > > > > > On Thu, Mar 19, 2020 at 6:03 PM Richard Guy Briggs > > > >> >> > > > > > > > wrote: > > > >> >> > > > > > > > > On 2020-03-18 18:06, Paul Moore wrote: > > > >> > > > > >> > ... > > > >> > > > > >> >> > > Well, every time a record gets generated, *any* record gets > > > >> >> > > generated, > > > >> >> > > we'll need to check for which audit daemons this record is in > > > >> >> > > scope and > > > >> >> > > generate a different one for each depending on the content and > > > >> >> > > whether > > > >> >> > > or not the content is influenced by the scope. > > > >> >> > > > > >> >> > That's the problem right there - we don't want to have to > > > >> >> > generate a > > > >> >> > unique record for *each* auditd on *every* record. That is a > > > >> >> > recipe > > > >> >> > for disaster. > > > >> >> > > > > >> >> > Solving this for all of the known audit records is not something > > > >> >> > we > > > >> >> > need to worry about in depth at the moment (although giving it > > > >> >> > some > > > >> >> > casual thought is not a bad thing), but solving this for the audit > > > >> >> > container ID information *is* something we need to worry about > > > >> >> > right > > > >> >> > now. > > > >> >> > > > >> >> If you think that a different nested contid value string per daemon > > > >> >> is > > > >> >> not acceptable, then we are back to issuing a record that has only > > > >> >> *one* > > > >> >> contid listed without any nesting information. This brings us back > > > >> >> to > > > >> >> the original problem of keeping *all* audit log history since the > > > >> >> boot > > > >> >> of the machine to be able to track the nesting of any particular > > > >> >> contid. > > > >> > > > > >> > I'm not ruling anything out, except for the "let's just completely > > > >> > regenerate every record for each auditd instance". > > > >> > > > >> Paul I am a bit confused about what you are referring to when you say > > > >> regenerate every record. > > > >> > > > >> Are you saying that you don't want to repeat the sequence: > > > >> audit_log_start(...); > > > >> audit_log_format(...); > > > >> audit_log_end(...); > > > >> for every nested audit daemon? > > > > > > > > If it can be avoided yes. Audit performance is already not-awesome, > > > > this would make it even worse. > > > > > > As far as I can see not repeating sequences like that is fundamental > > > for making this work at all. Just because only the audit subsystem > > > should know about one or multiple audit daemons. Nothing else should > > > care. > > > > Yes, exactly, this has been mentioned in the past. Both the > > performance hit and the code complication in the caller are things we > > must avoid. > > > > > >> Or are you saying that you would like to literraly want to send the > > > >> same > > > >> skb to each of the nested audit daemons? > > > > > > > > Ideally we would reuse the generated audit messages as much as > > > > possible. Less work is better. That's really my main concern here, > > > > let's make sure we aren't going to totally tank performance when we > > > > have a bunch of nested audit daemons. > > > > > > So I think there are two parts of this answer. Assuming we are talking > > > about nesting audit daemons in containers we will have different > > > rulesets and I expect most of the events for a nested audit daemon won't > > > be of interest to the outer audit daemon. > > > > Yes, this is another thing that Richard and I have discussed in the > > past. We will basically need to create per-daemon queues, rules, > > tracking state, etc.; that is easy enough. What will be slightly more > > tricky is the part where we apply the filters to the individual > > records and decide if that record is valid/desired for a given daemon. > > I think it can be done without too much pain, and any changes to the > > callers, but it will require a bit of work to make sure it is done > > well and that records are needlessly duplicated in the kernel. > > > > > Beyond that it should be very straight forward to
Re: [PATCH 2/2] integrity: Add errno field in audit message
On Wed, Jun 17, 2020 at 4:44 PM 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 > Suggested-by: Steve Grubb > --- > security/integrity/integrity_audit.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Acked-by: Paul Moore > 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 -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit
[PATCH 1/2] IMA: pass error code in result parameter to integrity_audit_msg()
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
---
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 ? "com
[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 Suggested-by: Steve Grubb --- 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 -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit
[PATCH] IMA: Add audit log for failure conditions
process_buffer_measurement() and ima_alloc_key_entry() functions need to
log an audit message for auditing integrity measurement failures.
Add audit message in these two functions. Remove "pr_devel" log message
in process_buffer_measurement().
Sample audit messages:
[6.415374] audit: type=1804 audit(1592005945.627:2): pid=1 uid=0
auid=4294967295 ses=4294967295 subj=kernel op=measuring_kexec_cmdline
cause=alloc_entry comm="swapper/0" name="kexec-cmdline" res=0
[8.128004] audit: type=1804 audit(1592005947.341:11): pid=1 uid=0
auid=4294967295 ses=4294967295 subj=system_u:system_r:init_t:s0
op=measuring_key cause=hashing_error comm="systemd"
name=".builtin_trusted_keys" res=0
Signed-off-by: Lakshmi Ramasubramanian
Suggested-by: Mimi Zohar
---
security/integrity/ima/ima.h| 48 -
security/integrity/ima/ima_main.c | 18 +++---
security/integrity/ima/ima_policy.c | 2 +-
security/integrity/ima/ima_queue_keys.c | 5 +++
4 files changed, 51 insertions(+), 22 deletions(-)
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index df93ac258e01..c3a32e181b48 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -186,27 +186,43 @@ static inline unsigned int ima_hash_key(u8 *digest)
return (digest[0] | digest[1] << 8) % IMA_MEASURE_HTABLE_SIZE;
}
-#define __ima_hooks(hook) \
- hook(NONE) \
- hook(FILE_CHECK)\
- hook(MMAP_CHECK)\
- hook(BPRM_CHECK)\
- hook(CREDS_CHECK) \
- hook(POST_SETATTR) \
- hook(MODULE_CHECK) \
- hook(FIRMWARE_CHECK)\
- hook(KEXEC_KERNEL_CHECK)\
- hook(KEXEC_INITRAMFS_CHECK) \
- hook(POLICY_CHECK) \
- hook(KEXEC_CMDLINE) \
- hook(KEY_CHECK) \
- hook(MAX_CHECK)
-#define __ima_hook_enumify(ENUM) ENUM,
+#define __ima_hooks(hook) \
+ hook(NONE, none)\
+ hook(FILE_CHECK, file) \
+ hook(MMAP_CHECK, mmap) \
+ hook(BPRM_CHECK, bprm) \
+ hook(CREDS_CHECK, creds)\
+ hook(POST_SETATTR, post_setattr)\
+ hook(MODULE_CHECK, module) \
+ hook(FIRMWARE_CHECK, firmware) \
+ hook(KEXEC_KERNEL_CHECK, kexec_kernel) \
+ hook(KEXEC_INITRAMFS_CHECK, kexec_initramfs)\
+ hook(POLICY_CHECK, policy) \
+ hook(KEXEC_CMDLINE, kexec_cmdline) \
+ hook(KEY_CHECK, key)\
+ hook(MAX_CHECK, none)
+
+#define __ima_hook_enumify(ENUM, str) ENUM,
+#define __ima_stringify(arg) (#arg)
+#define __ima_hook_measuring_stringify(ENUM, str) \
+ (__ima_stringify(measuring_ ##str)),
enum ima_hooks {
__ima_hooks(__ima_hook_enumify)
};
+static const char * const ima_hooks_measure_str[] = {
+ __ima_hooks(__ima_hook_measuring_stringify)
+};
+
+static inline const char *func_measure_str(enum ima_hooks func)
+{
+ if (func >= MAX_CHECK)
+ return ima_hooks_measure_str[NONE];
+
+ return ima_hooks_measure_str[func];
+}
+
extern const char *const func_tokens[];
struct modsig;
diff --git a/security/integrity/ima/ima_main.c
b/security/integrity/ima/ima_main.c
index c1583d98c5e5..8a001aa8e592 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -740,6 +740,7 @@ void process_buffer_measurement(const void *buf, int size,
int pcr, const char *keyring)
{
int ret = 0;
+ const char *audit_cause = "ENOMEM";
struct ima_template_entry *entry = NULL;
struct integrity_iint_cache iint = {};
struct ima_event_data event_data = {.iint = &iint,
@@ -794,21 +795,28 @@ void process_buffer_measurement(const void *buf, int size,
iint.ima_hash->length = hash_digest_size[ima_hash_algo];
ret = ima_calc_buffer_hash(buf, size, iint.ima_hash);
- if (ret < 0)
+ if (ret < 0) {
+ audit_cause = "hashing_error";
goto out;
+ }
ret = ima_alloc_init_template(&event_data, &entry, template);
- if (ret < 0)
+ if (ret < 0) {
+ audit_cause = "alloc_entry";
goto out;
+ }
ret = ima_store_template(entry, violation, NULL, buf, pcr);
-
- if (ret < 0)
+ if (ret < 0) {
+ audit_cause = "store_entry";
ima_free_template_entry(entry);
+ }
out:
if (ret < 0)
- pr_devel("%s: failed, result: %d\n", __func__, ret);
+ integrity_audit_msg(AUDIT_INTEGRITY_PCR, NULL, eve
Re: [PATCH] audit: Use struct_size() helper in alloc_chunk
On Mon, Jun 1, 2020 at 11:36 AM Paul Moore wrote:
> On Sun, May 24, 2020 at 4:47 PM Gustavo A. R. Silva
> wrote:
> > One of the more common cases of allocation size calculations is finding
> > the size of a structure that has a zero-sized array at the end, along
> > with memory for some number of elements for that array. For example:
> >
> > struct audit_chunk {
> > ...
> > struct node {
> > struct list_head list;
> > struct audit_tree *owner;
> > unsigned index; /* index; upper bit indicates 'will
> > prune' */
> > } owners[];
> > };
> >
> > Make use of the struct_size() helper instead of an open-coded version
> > in order to avoid any potential type mistakes.
> >
> > So, replace the following form:
> >
> > offsetof(struct audit_chunk, owners) + count * sizeof(struct node);
> >
> > with:
> >
> > struct_size(chunk, owners, count)
> >
> > This code was detected with the help of Coccinelle.
> >
> > Signed-off-by: Gustavo A. R. Silva
> > ---
> > kernel/audit_tree.c | 4 +---
> > 1 file changed, 1 insertion(+), 3 deletions(-)
>
> Thanks, this looks reasonable to me, but it came in too late for the
> v5.8 merge window (I dislike taking changes past -rc5/6 unless
> critical). Once the merge window closes I'll merge this into
> audit/next.
FYI, I just merged this into audit/next. Thanks!
--
paul moore
www.paul-moore.com
--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
Re: [PATCH] audit: optionally print warning after waiting to enqueue record
On Tue, Jun 16, 2020 at 12:58 AM Max Englander wrote: > > In environments where security is prioritized, users may set > --backlog_wait_time to a high value in order to reduce the likelihood > that any audit event is lost, even though doing so may result in > unpredictable performance if the kernel schedules a timeout when the > backlog limit is exceeded. For these users, the next best thing to > predictable performance is the ability to quickly detect and react to > degraded performance. This patch proposes to aid the detection of kernel > audit subsystem pauses through the following changes: > > Add a variable named audit_backlog_warn_time. Enforce the value of this > variable to be no less than zero, and no more than the value of > audit_backlog_wait_time. > > If audit_backlog_warn_time is greater than zero and if the total time > spent waiting to enqueue an audit record is greater than or equal to > audit_backlog_warn_time, then print a warning with the total time > spent waiting. > > An example configuration: > > auditctl --backlog_warn_time 50 > > An example warning message: > > audit: sleep_time=52 >= audit_backlog_warn_time=50 > > Tested on Ubuntu 18.04.04 using complementary changes to the audit > userspace: https://github.com/linux-audit/audit-userspace/pull/131. > > Signed-off-by: Max Englander > --- > include/uapi/linux/audit.h | 7 ++- > kernel/audit.c | 35 +++ > 2 files changed, 41 insertions(+), 1 deletion(-) If an admin is prioritizing security, aka don't loose any audit records, and there is a concern over variable system latency due to an audit queue backlog, why not simply disable the backlog limit? -- paul moore www.paul-moore.com -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit
