Re: [PATCH RFC v11 8/19] uapi|audit|ipe: add ipe auditing support

Fan Wu
2023-11-02T22:55:17Z

On 10/23/2023 8:52 PM, Paul Moore wrote:
> On Oct 4, 2023 Fan Wu <wufan@linux.microsoft.com> 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@linux.microsoft.com>
>> Signed-off-by: Fan Wu <wufan@linux.microsoft.com>
...
>> ---
>> 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