Re: [PATCH 8/8] ima: Differentiate auditing policy rules from "audit" actions

From: Stefan Berger
Date: Fri Jun 01 2018 - 16:50:24 EST

On 06/01/2018 04:13 PM, Paul Moore wrote:
On Fri, Jun 1, 2018 at 4:00 PM, Stefan Berger
<stefanb@xxxxxxxxxxxxxxxxxx> wrote:
On 05/30/2018 07:34 PM, Richard Guy Briggs wrote:
On 2018-05-30 17:38, Stefan Berger wrote:
On 05/30/2018 05:22 PM, Paul Moore wrote:
On Wed, May 30, 2018 at 9:08 AM, Stefan Berger
<stefanb@xxxxxxxxxxxxxxxxxx> wrote:
On 05/30/2018 08:49 AM, Richard Guy Briggs wrote:
On 2018-05-24 16:11, Stefan Berger wrote:
The AUDIT_INTEGRITY_RULE is used for auditing IMA policy rules and
the IMA "audit" policy action. This patch defines
AUDIT_INTEGRITY_POLICY_RULE to reflect the IMA policy rules.

With this change we now call integrity_audit_msg_common() to get
common integrity auditing fields. This now produces the following
record when parsing an IMA policy rule:

type=UNKNOWN[1806] msg=audit(1527004216.690:311): action=dont_measure
\
fsmagic=0x9fa0 pid=1613 uid=0 auid=0 ses=2 \
subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 \
op=policy_update cause=parse_rule comm="echo"
exe="/usr/bin/echo" \
tty=tty2 res=1

Signed-off-by: Stefan Berger <stefanb@xxxxxxxxxxxxxxxxxx>
---
include/uapi/linux/audit.h | 3 ++-
security/integrity/ima/ima_policy.c | 5 +++--
2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index 4e61a9e05132..776e0abd35cf 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -146,7 +146,8 @@
#define AUDIT_INTEGRITY_STATUS 1802 /* Integrity
enable
status */
#define AUDIT_INTEGRITY_HASH 1803 /* Integrity HASH type */
#define AUDIT_INTEGRITY_PCR 1804 /* PCR invalidation msgs
*/
-#define AUDIT_INTEGRITY_RULE 1805 /* policy rule */
+#define AUDIT_INTEGRITY_RULE 1805 /* IMA "audit" action policy
msgs */
+#define AUDIT_INTEGRITY_POLICY_RULE 1806 /* IMA policy rules */
#define AUDIT_KERNEL 2000 /* Asynchronous
audit
record. NOT A REQUEST. */
diff --git a/security/integrity/ima/ima_policy.c
b/security/integrity/ima/ima_policy.c
index 3aed25a7178a..a8ae47a386b4 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -634,7 +634,7 @@ static int ima_parse_rule(char *rule, struct
ima_rule_entry *entry)
int result = 0;
ab = integrity_audit_log_start(NULL, GFP_KERNEL,
- AUDIT_INTEGRITY_RULE);
+ AUDIT_INTEGRITY_POLICY_RULE);
Is it possible to connect this record to a syscall by replacing the
first parameter (NULL) by current->context?
We're likely going to need to "associate" this record (audit speak for
making the first parameter non-NULL) with others for the audit
container ID work. If you do it now, Richard's patches will likely
get a few lines smaller and that will surely make him a bit happier :)
Richard is also introducing a local context that we can then create and
use
instead of the NULL. Can we not use that then?
That is for records for which there is no syscall or user associated.

In fact there is another recent change that would be better to use than
current->audit_context, which is the function audit_context().
See commit cdfb6b3 ("audit: use inline function to get audit context").

Steven seems to say: "We don't want to add syscall records to everything.
That messes up schemas and existing code. The integrity events are 1
record
in size and should stay that way. This saves disk space and improves
readability."

We would have to fix current->context in this case since it is NULL. We
get
to this location by root cat'ing a policy or writing a policy filename
into
/sys/kernel/security/ima/policy.
Perhaps I'm missing something, but current in this case should point
to the process which is writing to the policy file, yes?
Yes, but current->context is NULL for some reason.
Is it always this way? If it isn't, which it should not be, we should
find out why. Well, we should find out why this is NULL anyways, since
it shouldn't be.

When someone writes a policy for IMA into securityfs, it's always NULL.
There's another location where IMA uses the current->audit_context, and
that's here:

https://elixir.bootlin.com/linux/latest/source/security/integrity/ima/ima_api.c#L323

At this location we sometimes see a (background) process with an
audit_context but in the majority of cases it's current->audit_context is
NULL. Starting a process as root or also non-root user, with the appropriate
IMA audit policy rules set, we always see a NULL audit_context here.
What does your audit configuration look like?

Depending on your configuration a NULL audit_context can be expected,
see audit_dummy_context(). I believe the default Fedora audit config
will leave you with a NULL audit_context for all processes. I also
believe that unless you explicitly set "audit=1" on the kernel command
line the init/systemd process will have a NULL audit_context (there
was actually a range of kernels where even setting "audit=1" wouldn't
be sufficient due to a bug we fixed a little while ago).

Look at the audit_alloc() function, it is called when a new process is
fork'd and is responsible for allocating a new audit_context. If the
currently loaded audit config dictates that auditing is to be disabled
for this new process (state == AUDIT_DISABLED) then an audit_context
is not allocated and current->context remains NULL.

I found that out also. The background process that had the audit context was created when a different audit policy was active and therefore still has the audit_context and creates the associated syscall messages. The new processes don't get it because of -a task,never rule.