| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <46922873bd020f8cd443b7fca59ef802.paul@paul-moore.com>
Date: Mon, 23 Oct 2023 23:52:24 -0400
From: Paul Moore <paul@...l-moore.com>
To: Fan Wu <wufan@...ux.microsoft.com>, corbet@....net,
zohar@...ux.ibm.com, jmorris@...ei.org, serge@...lyn.com,
tytso@....edu, ebiggers@...nel.org, axboe@...nel.dk,
agk@...hat.com, snitzer@...nel.org, eparis@...hat.com
Cc: linux-doc@...r.kernel.org, linux-integrity@...r.kernel.org,
linux-security-module@...r.kernel.org,
linux-fscrypt@...r.kernel.org, linux-block@...r.kernel.org,
dm-devel@...hat.com, audit@...r.kernel.org,
roberto.sassu@...wei.com, linux-kernel@...r.kernel.org,
Deven Bowers <deven.desai@...ux.microsoft.com>,
Fan Wu <wufan@...ux.microsoft.com>
Subject: Re: [PATCH RFC v11 4/19] ipe: add LSM hooks on execution and kernel read
On Oct 4, 2023 Fan Wu <wufan@...ux.microsoft.com> wrote:
>
> IPE's initial goal is to control both execution and the loading of
> kernel modules based on the system's definition of trust. It
> accomplishes this by plugging into the security hooks for
> bprm_check_security, file_mprotect, mmap_file, kernel_load_data,
> and kernel_read_data.
>
> Signed-off-by: Deven Bowers <deven.desai@...ux.microsoft.com>
> Signed-off-by: Fan Wu <wufan@...ux.microsoft.com>
> ---
> v2:
> + Split evaluation loop, access control hooks,
> and evaluation loop from policy parser and userspace
> interface to pass mailing list character limit
>
> v3:
> + Move ipe_load_properties to patch 04.
> + Remove useless 0-initializations
> + Prefix extern variables with ipe_
> + Remove kernel module parameters, as these are
> exposed through sysctls.
> + Add more prose to the IPE base config option
> help text.
> + Use GFP_KERNEL for audit_log_start.
> + Remove unnecessary caching system.
> + Remove comments from headers
> + Use rcu_access_pointer for rcu-pointer null check
> + Remove usage of reqprot; use prot only.
> + Move policy load and activation audit event to 03/12
>
> v4:
> + Remove sysctls in favor of securityfs nodes
> + Re-add kernel module parameters, as these are now
> exposed through securityfs.
> + Refactor property audit loop to a separate function.
>
> v5:
> + fix minor grammatical errors
> + do not group rule by curly-brace in audit record,
> reconstruct the exact rule.
>
> v6:
> + No changes
>
> v7:
> + Further split lsm creation, the audit system, the evaluation loop
> and access control hooks into separate commits.
>
> v8:
> + Rename hook functions to follow the lsmname_hook_name convention
> + Remove ipe_hook enumeration, can be derived from correlation with
> syscall audit record.
>
> v9:
> + Minor changes for adapting to the new parser
>
> v10:
> + Remove @reqprot part
>
> v11:
> + Fix code style issues
> ---
> security/ipe/Makefile | 1 +
> security/ipe/eval.c | 14 ++++
> security/ipe/eval.h | 3 +
> security/ipe/hooks.c | 183 ++++++++++++++++++++++++++++++++++++++++++
> security/ipe/hooks.h | 25 ++++++
> security/ipe/ipe.c | 6 ++
> 6 files changed, 232 insertions(+)
> create mode 100644 security/ipe/hooks.c
> create mode 100644 security/ipe/hooks.h
...
> diff --git a/security/ipe/hooks.c b/security/ipe/hooks.c
> new file mode 100644
> index 000000000000..6164a9b53361
> --- /dev/null
> +++ b/security/ipe/hooks.c
> @@ -0,0 +1,183 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (C) Microsoft Corporation. All rights reserved.
> + */
...
> +/**
> + * ipe_kernel_read_file - ipe security hook function for kernel read.
> + * @file: Supplies a pointer to the file structure being read in from disk.
> + * @id: Supplies the enumeration identifying the purpose of the read.
> + * @contents: Unused.
> + *
> + * This LSM hook is called when a file is being read in from disk from
> + * the kernel.
> + *
> + * Return:
> + * 0 - OK
> + * !0 - Error
> + */
> +int ipe_kernel_read_file(struct file *file, enum kernel_read_file_id id,
> + bool contents)
> +{
> + enum ipe_op_type op;
> + struct ipe_eval_ctx ctx = IPE_EVAL_CTX_INIT;
> +
> + switch (id) {
> + case READING_FIRMWARE:
> + op = IPE_OP_FIRMWARE;
> + break;
> + case READING_MODULE:
> + op = IPE_OP_KERNEL_MODULE;
> + break;
> + case READING_KEXEC_INITRAMFS:
> + op = IPE_OP_KEXEC_INITRAMFS;
> + break;
> + case READING_KEXEC_IMAGE:
> + op = IPE_OP_KEXEC_IMAGE;
> + break;
> + case READING_POLICY:
> + op = IPE_OP_IMA_POLICY;
> + break;
> + case READING_X509_CERTIFICATE:
> + op = IPE_OP_IMA_X509;
> + break;
> + default:
> + op = IPE_OP_INVALID;
> + WARN(op == IPE_OP_INVALID, "no rule setup for enum %d", id);
I'm not sure you need to test @op above since you set @op on the line
above, just use true/1 to simplify things. It also seems like it
might be helpful to provice some context for the enum above in the
WARN() message. For example:
WARN(1, "no rule setup for kernel_read_file %d", id);
> + }
> +
> + build_eval_ctx(&ctx, file, op);
> + return ipe_evaluate_event(&ctx);
> +}
> +
> +/**
> + * ipe_kernel_load_data - ipe security hook function for kernel load data.
> + * @id: Supplies the enumeration identifying the purpose of the read.
> + * @contents: Unused.
> + *
> + * This LSM hook is called when a buffer is being read in from disk.
> + *
> + * Return:
> + * * 0 - OK
> + * * !0 - Error
> + */
> +int ipe_kernel_load_data(enum kernel_load_data_id id, bool contents)
> +{
> + enum ipe_op_type op;
> + struct ipe_eval_ctx ctx = IPE_EVAL_CTX_INIT;
> +
> + switch (id) {
> + case LOADING_FIRMWARE:
> + op = IPE_OP_FIRMWARE;
> + break;
> + case LOADING_MODULE:
> + op = IPE_OP_KERNEL_MODULE;
> + break;
> + case LOADING_KEXEC_INITRAMFS:
> + op = IPE_OP_KEXEC_INITRAMFS;
> + break;
> + case LOADING_KEXEC_IMAGE:
> + op = IPE_OP_KEXEC_IMAGE;
> + break;
> + case LOADING_POLICY:
> + op = IPE_OP_IMA_POLICY;
> + break;
> + case LOADING_X509_CERTIFICATE:
> + op = IPE_OP_IMA_X509;
> + break;
> + default:
> + op = IPE_OP_INVALID;
> + WARN(op == IPE_OP_INVALID, "no rule setup for enum %d", id);
See my comments in ipe_kernel_read_file(), they also apply here.
> + }
> +
> + build_eval_ctx(&ctx, NULL, op);
> + return ipe_evaluate_event(&ctx);
> +}
--
paul-moore.com
Powered by blists - more mailing lists