Messages in this thread | | | Date | Thu, 2 Nov 2023 15:55:11 -0700 | Subject | Re: [PATCH RFC v11 8/19] uapi|audit|ipe: add ipe auditing support | From | Fan Wu <> |
| |
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
| |