Re: [PATCH RFC v11 8/19] uapi|audit|ipe: add ipe auditing support
From: Fan Wu
Date: Thu Nov 02 2023 - 18:55:20 EST
On 10/23/2023 8:52 PM, Paul Moore wrote:
On Oct 4, 2023 Fan Wu <wufan@xxxxxxxxxxxxxxxxxxx> wrote:
Users of IPE require a way to identify when and why an operation fails,
allowing them to both respond to violations of policy and be notified
of potentially malicious actions on their systems with respect to IPE
itself.
This patch introduces 3 new audit events.
AUDIT_IPE_ACCESS(1420) indicates the result of an IPE policy evaluation
of a resource.
AUDIT_IPE_CONFIG_CHANGE(1421) indicates the current active IPE policy
has been changed to another loaded policy.
AUDIT_IPE_POLICY_LOAD(1422) indicates a new IPE policy has been loaded
into the kernel.
This patch also adds support for success auditing, allowing users to
identify why an allow decision was made for a resource. However, it is
recommended to use this option with caution, as it is quite noisy.
Here are some examples of the new audit record types:
AUDIT_IPE_ACCESS(1420):
audit: AUDIT1420 path="/root/vol/bin/hello" dev="sda"
ino=3897 rule="op=EXECUTE boot_verified=TRUE action=ALLOW"
audit: AUDIT1420 path="/mnt/ipe/bin/hello" dev="dm-0"
ino=2 rule="DEFAULT action=DENY"
audit: AUDIT1420 path="/tmp/tmpdp2h1lub/deny/bin/hello" dev="tmpfs"
ino=131 rule="DEFAULT action=DENY"
The above three records were generated when the active IPE policy only
allows binaries from the initial booted drive(sda) to run. The three
identical `hello` binary were placed at different locations, only the
first hello from sda was allowed.
Field path followed by the file's path name.
Field dev followed by the device name as found in /dev where the file is
from.
Note that for device mappers it will use the name `dm-X` instead of
the name in /dev/mapper.
For a file in a temp file system, which is not from a device, it will use
`tmpfs` for the field.
The implementation of this part is following another existing use case
LSM_AUDIT_DATA_INODE in security/lsm_audit.c
Field ino followed by the file's inode number.
Field rule followed by the IPE rule made the access decision. The whole
rule must be audited because the decision is based on the combination of
all property conditions in the rule.
Along with the syscall audit event, user can know why a blocked
happened. For example:
audit: AUDIT1420 path="/mnt/ipe/bin/hello" dev="dm-0"
ino=2 rule="DEFAULT action=DENY"
audit[1956]: SYSCALL arch=c000003e syscall=59
success=no exit=-13 a0=556790138df0 a1=556790135390 a2=5567901338b0
a3=ab2a41a67f4f1f4e items=1 ppid=147 pid=1956 auid=4294967295 uid=0
gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=pts0
ses=4294967295 comm="bash" exe="/usr/bin/bash" key=(null)
The above two records showed bash used execve to run "hello" and got
blocked by IPE. Note that the IPE records are always prior to a SYSCALL
record.
AUDIT_IPE_CONFIG_CHANGE(1421):
audit: AUDIT1421
old_active_pol_name="Allow_All" old_active_pol_version=0.0.0
old_policy_digest=sha256:E3B0C44298FC1C149AFBF4C8996FB92427AE41E4649
new_active_pol_name="boot_verified" new_active_pol_version=0.0.0
new_policy_digest=sha256:820EEA5B40CA42B51F68962354BA083122A20BB846F
auid=4294967295 ses=4294967295 lsm=ipe res=1
The above record showed the current IPE active policy switch from
`Allow_All` to `boot_verified` along with the version and the hash
digest of the two policies. Note IPE can only have one policy active
at a time, all access decision evaluation is based on the current active
policy.
The normal procedure to deploy a policy is loading the policy to deploy
into the kernel first, then switch the active policy to it.
AUDIT_IPE_POLICY_LOAD(1422):
audit: AUDIT1422 policy_name="boot_verified" policy_version=0.0.0
policy_digest=sha256:820EEA5B40CA42B51F68962354BA083122A20BB846F26765076DD
auid=4294967295 ses=4294967295 lsm=ipe res=1
The above record showed a new policy has been loaded into the kernel
with the policy name, policy version and policy hash.
Signed-off-by: Deven Bowers <deven.desai@xxxxxxxxxxxxxxxxxxx>
Signed-off-by: Fan Wu <wufan@xxxxxxxxxxxxxxxxxxx>
...
---
include/uapi/linux/audit.h | 3 +
security/ipe/Kconfig | 2 +-
security/ipe/Makefile | 1 +
security/ipe/audit.c | 195 +++++++++++++++++++++++++++++++++++++
security/ipe/audit.h | 18 ++++
security/ipe/eval.c | 32 ++++--
security/ipe/eval.h | 8 ++
security/ipe/fs.c | 70 +++++++++++++
security/ipe/policy.c | 5 +
9 files changed, 327 insertions(+), 7 deletions(-)
create mode 100644 security/ipe/audit.c
create mode 100644 security/ipe/audit.h
...
diff --git a/security/ipe/audit.c b/security/ipe/audit.c
new file mode 100644
index 000000000000..e123701d5e3b
--- /dev/null
+++ b/security/ipe/audit.c
@@ -0,0 +1,195 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) Microsoft Corporation. All rights reserved.
+ */
+
+#include <linux/slab.h>
+#include <linux/audit.h>
+#include <linux/types.h>
+#include <crypto/hash.h>
+
+#include "ipe.h"
+#include "eval.h"
+#include "hooks.h"
+#include "policy.h"
+#include "audit.h"
+
+#define ACTSTR(x) ((x) == IPE_ACTION_ALLOW ? "ALLOW" : "DENY")
+
+#define IPE_AUDIT_HASH_ALG "sha256"
+
+#define AUDIT_POLICY_LOAD_FMT "policy_name=\"%s\" policy_version=%hu.%hu.%hu "\
+ "policy_digest=" IPE_AUDIT_HASH_ALG ":"
+#define AUDIT_OLD_ACTIVE_POLICY_FMT "old_active_pol_name=\"%s\" "\
+ "old_active_pol_version=%hu.%hu.%hu "\
+ "old_policy_digest=" IPE_AUDIT_HASH_ALG ":"
+#define AUDIT_NEW_ACTIVE_POLICY_FMT "new_active_pol_name=\"%s\" "\
+ "new_active_pol_version=%hu.%hu.%hu "\
+ "new_policy_digest=" IPE_AUDIT_HASH_ALG ":"
+
+static const char *const audit_op_names[__IPE_OP_MAX] = {
+ "EXECUTE",
+ "FIRMWARE",
+ "KMODULE",
+ "KEXEC_IMAGE",
+ "KEXEC_INITRAMFS",
+ "IMA_POLICY",
+ "IMA_X509_CERT",
+};
+
+static const char *const audit_prop_names[__IPE_PROP_MAX] = {
+ "boot_verified=FALSE",
+ "boot_verified=TRUE",
+};
I would suggest taking the same approach for both @audit_op_names and
@audit_prop_names; either include the field name in the string array
for both or leave it out of both.
Yes sure, I will move the "op=" into audit_op_names.
+/**
+ * audit_rule - audit an IPE policy rule approximation.
+ * @ab: Supplies a pointer to the audit_buffer to append to.
+ * @r: Supplies a pointer to the ipe_rule to approximate a string form for.
+ */
+static void audit_rule(struct audit_buffer *ab, const struct ipe_rule *r)
+{
+ const struct ipe_prop *ptr;
+
+ audit_log_format(ab, "rule=\"op=%s ", audit_op_names[r->op]);
+
+ list_for_each_entry(ptr, &r->props, next)
+ audit_log_format(ab, "%s ", audit_prop_names[ptr->type]);
+
+ audit_log_format(ab, "action=%s\"", ACTSTR(r->action));
+}
+
+/**
+ * ipe_audit_match - audit a match for IPE policy.
+ * @ctx: Supplies a pointer to the evaluation context that was used in the
+ * evaluation.
+ * @match_type: Supplies the scope of the match: rule, operation default,
+ * global default.
+ * @act: Supplies the IPE's evaluation decision, deny or allow.
+ * @r: Supplies a pointer to the rule that was matched, if possible.
+ * @enforce: Supplies the enforcement/permissive state at the point
+ * the enforcement decision was made.
+ */
Does it make sense to move @match_type into the ipe_eval_ctx struct?
I feel the @match_type is part of the evaluation result information,
which is the result of the context against the active policy. So I
prefer keeping it as a local variable in the evaluation loop.
-Fan
+void ipe_audit_match(const struct ipe_eval_ctx *const ctx,
+ enum ipe_match match_type,
+ enum ipe_action_type act, const struct ipe_rule *const r)
+{
+ struct inode *inode;
+ struct audit_buffer *ab;
+ const char *op = audit_op_names[ctx->op];
+
+ if (act != IPE_ACTION_DENY && !READ_ONCE(success_audit))
+ return;
+
+ ab = audit_log_start(audit_context(), GFP_KERNEL, AUDIT_IPE_ACCESS);
+ if (!ab)
+ return;
+
+ if (ctx->file) {
+ audit_log_d_path(ab, "path=", &ctx->file->f_path);
+ inode = file_inode(ctx->file);
+ if (inode) {
+ audit_log_format(ab, " dev=");
+ audit_log_untrustedstring(ab, inode->i_sb->s_id);
+ audit_log_format(ab, " ino=%lu ", inode->i_ino);
+ }
+ }
+
+ if (match_type == IPE_MATCH_RULE)
+ audit_rule(ab, r);
+ else if (match_type == IPE_MATCH_TABLE)
+ audit_log_format(ab, "rule=\"DEFAULT op=%s action=%s\"", op,
+ ACTSTR(act));
+ else
+ audit_log_format(ab, "rule=\"DEFAULT action=%s\"",
+ ACTSTR(act));
+
+ audit_log_end(ab);
+}
--
paul-moore.com