[RFC,v9,06/16] ipe: add LSM hooks on execution and kernel read

Commit Message

Fan Wu Jan. 30, 2023, 10:57 p.m. UTC

From: Deven Bowers <[e-mail]>

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 <[e-mail]>
Signed-off-by: Fan Wu <[e-mail]>
---
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
---
 security/ipe/hooks.c | 169 +++++++++++++++++++++++++++++++++++++++++++
 security/ipe/hooks.h |  13 ++++
 security/ipe/ipe.c   |   6 ++
 3 files changed, 188 insertions(+)

Comments

Roberto Sassu Jan. 31, 2023, 12:51 p.m. UTC | #1

On Mon, 2023-01-30 at 14:57 -0800, Fan Wu wrote:
> From: Deven Bowers <[e-mail]>
> 
> 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 <[e-mail]>
> Signed-off-by: Fan Wu <[e-mail]>
> ---
> 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
> ---
>  security/ipe/hooks.c | 169 +++++++++++++++++++++++++++++++++++++++++++
>  security/ipe/hooks.h |  13 ++++
>  security/ipe/ipe.c   |   6 ++
>  3 files changed, 188 insertions(+)
> 
> diff --git a/security/ipe/hooks.c b/security/ipe/hooks.c
> index 335b773c7ae1..fd5109e29c76 100644
> --- a/security/ipe/hooks.c
> +++ b/security/ipe/hooks.c
> @@ -23,3 +23,172 @@ void ipe_sb_free_security(struct super_block *mnt_sb)
>  {
>  	ipe_invalidate_pinned_sb(mnt_sb);
>  }
> +
> +/**
> + * ipe_bprm_check_security - ipe security hook function for bprm check.
> + * @bprm: Supplies a pointer to a linux_binprm structure to source the file
> + *	  being evaluated.
> + *
> + * This LSM hook is called when a binary is loaded through the exec
> + * family of system calls.
> + * Return:
> + * *0	- OK
> + * *!0	- Error
> + */
> +int ipe_bprm_check_security(struct linux_binprm *bprm)
> +{
> +	struct ipe_eval_ctx ctx = { 0 };
> +
> +	build_eval_ctx(&ctx, bprm->file, ipe_op_exec);
> +	return ipe_evaluate_event(&ctx);
> +}
> +
> +/**
> + * ipe_mmap_file - ipe security hook function for mmap check.
> + * @f: File being mmap'd. Can be NULL in the case of anonymous memory.
> + * @reqprot: The requested protection on the mmap, passed from usermode.
> + * @prot: The effective protection on the mmap, resolved from reqprot and
> + *	  system configuration.
> + * @flags: Unused.
> + *
> + * This hook is called when a file is loaded through the mmap
> + * family of system calls.
> + *
> + * Return:
> + * * 0	- OK
> + * * !0	- Error
> + */
> +int ipe_mmap_file(struct file *f, unsigned long reqprot, unsigned long prot,
> +		  unsigned long flags)
> +{
> +	struct ipe_eval_ctx ctx = { 0 };
> +
> +	if (prot & PROT_EXEC || reqprot & PROT_EXEC) {

Since the kernel only adds flags and doesn't clear them, isn't safe to
just consider prot? Oh, you mentioned it in the changelog, maybe just
for ipe_file_mprotect().

> +		build_eval_ctx(&ctx, f, ipe_op_exec);
> +		return ipe_evaluate_event(&ctx);
> +	}

Uhm, I think some considerations that IMA does for mmap() are relevant
also for IPE.

For example, look at mmap_violation_check(). It checks if there are
writable mappings, and if yes, it denies the access.

Similarly for mprotect(), is adding PROT_EXEC safe?

> +
> +	return 0;
> +}
> +
> +/**
> + * ipe_file_mprotect - ipe security hook function for mprotect check.
> + * @vma: Existing virtual memory area created by mmap or similar.
> + * @reqprot: The requested protection on the mmap, passed from usermode.
> + * @prot: The effective protection on the mmap, resolved from reqprot and
> + *	  system configuration.
> + *
> + * This LSM hook is called when a mmap'd region of memory is changing
> + * its protections via mprotect.
> + *
> + * Return:
> + * * 0	- OK
> + * * !0	- Error
> + */
> +int ipe_file_mprotect(struct vm_area_struct *vma, unsigned long reqprot,
> +		      unsigned long prot)
> +{
> +	struct ipe_eval_ctx ctx = { 0 };
> +
> +	/* Already Executable */
> +	if (vma->vm_flags & VM_EXEC)
> +		return 0;
> +
> +	if (prot & PROT_EXEC) {
> +		build_eval_ctx(&ctx, vma->vm_file, ipe_op_exec);
> +		return ipe_evaluate_event(&ctx);
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * 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;
> +
> +	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_max;
> +		WARN(op == ipe_op_max, "no rule setup for enum %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 = { 0 };
> +
> +	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_max;
> +		WARN(op == ipe_op_max, "no rule setup for enum %d", id);
> +	}
> +
> +	build_eval_ctx(&ctx, NULL, op);
> +	return ipe_evaluate_event(&ctx);
> +}
> diff --git a/security/ipe/hooks.h b/security/ipe/hooks.h
> index 30fe455389bf..857cae69678c 100644
> --- a/security/ipe/hooks.h
> +++ b/security/ipe/hooks.h
> @@ -11,4 +11,17 @@
>  
>  void ipe_sb_free_security(struct super_block *mnt_sb);
>  
> +int ipe_bprm_check_security(struct linux_binprm *bprm);
> +
> +int ipe_mmap_file(struct file *f, unsigned long reqprot, unsigned long prot,
> +		  unsigned long flags);
> +
> +int ipe_file_mprotect(struct vm_area_struct *vma, unsigned long reqprot,
> +		      unsigned long prot);
> +
> +int ipe_kernel_read_file(struct file *file, enum kernel_read_file_id id,
> +			 bool contents);
> +
> +int ipe_kernel_load_data(enum kernel_load_data_id id, bool contents);
> +
>  #endif /* IPE_HOOKS_H */
> diff --git a/security/ipe/ipe.c b/security/ipe/ipe.c
> index bef923026b50..7af2f942decd 100644
> --- a/security/ipe/ipe.c
> +++ b/security/ipe/ipe.c
> @@ -4,6 +4,7 @@
>   */
>  
>  #include "ipe.h"
> +#include "hooks.h"
>  
>  bool ipe_enabled;
>  
> @@ -12,6 +13,11 @@ static struct lsm_blob_sizes ipe_blobs __lsm_ro_after_init = {
>  
>  static struct security_hook_list ipe_hooks[] __lsm_ro_after_init = {
>  	LSM_HOOK_INIT(sb_free_security, ipe_sb_free_security),
> +	LSM_HOOK_INIT(bprm_check_security, ipe_bprm_check_security),
> +	LSM_HOOK_INIT(mmap_file, ipe_mmap_file),
> +	LSM_HOOK_INIT(file_mprotect, ipe_file_mprotect),
> +	LSM_HOOK_INIT(kernel_read_file, ipe_kernel_read_file),
> +	LSM_HOOK_INIT(kernel_load_data, ipe_kernel_load_data),
>  };

Uhm, maybe I would incorporate patch 1 with this.

Roberto

Fan Wu Feb. 9, 2023, 10:42 p.m. UTC | #2

On Tue, Jan 31, 2023 at 01:51:39PM +0100, Roberto Sassu wrote:
> On Mon, 2023-01-30 at 14:57 -0800, Fan Wu wrote:
> > +
> > +/**
> > + * ipe_mmap_file - ipe security hook function for mmap check.
> > + * @f: File being mmap'd. Can be NULL in the case of anonymous memory.
> > + * @reqprot: The requested protection on the mmap, passed from usermode.
> > + * @prot: The effective protection on the mmap, resolved from reqprot and
> > + *	  system configuration.
> > + * @flags: Unused.
> > + *
> > + * This hook is called when a file is loaded through the mmap
> > + * family of system calls.
> > + *
> > + * Return:
> > + * * 0	- OK
> > + * * !0	- Error
> > + */
> > +int ipe_mmap_file(struct file *f, unsigned long reqprot, unsigned long prot,
> > +		  unsigned long flags)
> > +{
> > +	struct ipe_eval_ctx ctx = { 0 };
> > +
> > +	if (prot & PROT_EXEC || reqprot & PROT_EXEC) {
> 
> Since the kernel only adds flags and doesn't clear them, isn't safe to
> just consider prot? Oh, you mentioned it in the changelog, maybe just
> for ipe_file_mprotect().
> 

Thanks for pointing that out, yes reqprot it indeed unnecessary, I will remove
this part in the next version. 

> > +		build_eval_ctx(&ctx, f, ipe_op_exec);
> > +		return ipe_evaluate_event(&ctx);
> > +	}
> 
> Uhm, I think some considerations that IMA does for mmap() are relevant
> also for IPE.
> 
> For example, look at mmap_violation_check(). It checks if there are
> writable mappings, and if yes, it denies the access.
> 
> Similarly for mprotect(), is adding PROT_EXEC safe?
> 

Yes, writable mapping might need to treat differently. But for the current version
I think it is safe because currently we only support dmverity and fsverity,
they are inherently read-only.

But if in the future if there is a feature can support writable mapping, IPE might
better provide user the flexibility to allow or deny execute writable mappings,
for example, adding a new property like file_writable=TRUE. Then user can deploy
a rule like op=EXECUTE file_writable=TRUE action=DENY to deny execute a writable
mapping.

> >  
> > @@ -12,6 +13,11 @@ static struct lsm_blob_sizes ipe_blobs __lsm_ro_after_init = {
> >  
> >  static struct security_hook_list ipe_hooks[] __lsm_ro_after_init = {
> >  	LSM_HOOK_INIT(sb_free_security, ipe_sb_free_security),
> > +	LSM_HOOK_INIT(bprm_check_security, ipe_bprm_check_security),
> > +	LSM_HOOK_INIT(mmap_file, ipe_mmap_file),
> > +	LSM_HOOK_INIT(file_mprotect, ipe_file_mprotect),
> > +	LSM_HOOK_INIT(kernel_read_file, ipe_kernel_read_file),
> > +	LSM_HOOK_INIT(kernel_load_data, ipe_kernel_load_data),
> >  };
> 
> Uhm, maybe I would incorporate patch 1 with this.
> 
> Roberto

This might not be possible because this patch has some dependencies on the previous patches.
-Fan

Paul Moore March 2, 2023, 7:05 p.m. UTC | #3

On Mon, Jan 30, 2023 at 5:59 PM Fan Wu <[e-mail]> wrote:
>
> From: Deven Bowers <[e-mail]>
>
> 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 <[e-mail]>
> Signed-off-by: Fan Wu <[e-mail]>

...

> ---
>  security/ipe/hooks.c | 169 +++++++++++++++++++++++++++++++++++++++++++
>  security/ipe/hooks.h |  13 ++++
>  security/ipe/ipe.c   |   6 ++
>  3 files changed, 188 insertions(+)
>
> diff --git a/security/ipe/hooks.c b/security/ipe/hooks.c
> index 335b773c7ae1..fd5109e29c76 100644
> --- a/security/ipe/hooks.c
> +++ b/security/ipe/hooks.c
> @@ -23,3 +23,172 @@ void ipe_sb_free_security(struct super_block *mnt_sb)
>  {
>         ipe_invalidate_pinned_sb(mnt_sb);
>  }
> +
> +/**
> + * ipe_bprm_check_security - ipe security hook function for bprm check.
> + * @bprm: Supplies a pointer to a linux_binprm structure to source the file
> + *       being evaluated.
> + *
> + * This LSM hook is called when a binary is loaded through the exec
> + * family of system calls.
> + * Return:
> + * *0  - OK
> + * *!0 - Error
> + */
> +int ipe_bprm_check_security(struct linux_binprm *bprm)
> +{
> +       struct ipe_eval_ctx ctx = { 0 };
> +
> +       build_eval_ctx(&ctx, bprm->file, ipe_op_exec);
> +       return ipe_evaluate_event(&ctx);
> +}
> +
> +/**
> + * ipe_mmap_file - ipe security hook function for mmap check.
> + * @f: File being mmap'd. Can be NULL in the case of anonymous memory.
> + * @reqprot: The requested protection on the mmap, passed from usermode.
> + * @prot: The effective protection on the mmap, resolved from reqprot and
> + *       system configuration.
> + * @flags: Unused.
> + *
> + * This hook is called when a file is loaded through the mmap
> + * family of system calls.
> + *
> + * Return:
> + * * 0 - OK
> + * * !0        - Error
> + */
> +int ipe_mmap_file(struct file *f, unsigned long reqprot, unsigned long prot,
> +                 unsigned long flags)
> +{
> +       struct ipe_eval_ctx ctx = { 0 };
> +
> +       if (prot & PROT_EXEC || reqprot & PROT_EXEC) {

Is there a reason why you care about @reqprot?  It seems like IPE
would only be interested in the protection flags that the kernel is
actually using.

I notice that in the `ipe_file_mprotect()` hook you ignore @reqprot,
which I believe is the right thing to do.

> +               build_eval_ctx(&ctx, f, ipe_op_exec);
> +               return ipe_evaluate_event(&ctx);
> +       }
> +
> +       return 0;
> +}
> +
> +/**
> + * ipe_file_mprotect - ipe security hook function for mprotect check.
> + * @vma: Existing virtual memory area created by mmap or similar.
> + * @reqprot: The requested protection on the mmap, passed from usermode.
> + * @prot: The effective protection on the mmap, resolved from reqprot and
> + *       system configuration.
> + *
> + * This LSM hook is called when a mmap'd region of memory is changing
> + * its protections via mprotect.
> + *
> + * Return:
> + * * 0 - OK
> + * * !0        - Error
> + */
> +int ipe_file_mprotect(struct vm_area_struct *vma, unsigned long reqprot,
> +                     unsigned long prot)
> +{
> +       struct ipe_eval_ctx ctx = { 0 };
> +
> +       /* Already Executable */
> +       if (vma->vm_flags & VM_EXEC)
> +               return 0;
> +
> +       if (prot & PROT_EXEC) {
> +               build_eval_ctx(&ctx, vma->vm_file, ipe_op_exec);
> +               return ipe_evaluate_event(&ctx);
> +       }
> +
> +       return 0;
> +}
> +
> +/**
> + * 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;
> +
> +       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_max;
> +               WARN(op == ipe_op_max, "no rule setup for enum %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 = { 0 };
> +
> +       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_max;
> +               WARN(op == ipe_op_max, "no rule setup for enum %d", id);
> +       }
> +
> +       build_eval_ctx(&ctx, NULL, op);
> +       return ipe_evaluate_event(&ctx);
> +}
> diff --git a/security/ipe/hooks.h b/security/ipe/hooks.h
> index 30fe455389bf..857cae69678c 100644
> --- a/security/ipe/hooks.h
> +++ b/security/ipe/hooks.h
> @@ -11,4 +11,17 @@
>
>  void ipe_sb_free_security(struct super_block *mnt_sb);
>
> +int ipe_bprm_check_security(struct linux_binprm *bprm);
> +
> +int ipe_mmap_file(struct file *f, unsigned long reqprot, unsigned long prot,
> +                 unsigned long flags);
> +
> +int ipe_file_mprotect(struct vm_area_struct *vma, unsigned long reqprot,
> +                     unsigned long prot);
> +
> +int ipe_kernel_read_file(struct file *file, enum kernel_read_file_id id,
> +                        bool contents);
> +
> +int ipe_kernel_load_data(enum kernel_load_data_id id, bool contents);
> +
>  #endif /* IPE_HOOKS_H */
> diff --git a/security/ipe/ipe.c b/security/ipe/ipe.c
> index bef923026b50..7af2f942decd 100644
> --- a/security/ipe/ipe.c
> +++ b/security/ipe/ipe.c
> @@ -4,6 +4,7 @@
>   */
>
>  #include "ipe.h"
> +#include "hooks.h"
>
>  bool ipe_enabled;
>
> @@ -12,6 +13,11 @@ static struct lsm_blob_sizes ipe_blobs __lsm_ro_after_init = {
>
>  static struct security_hook_list ipe_hooks[] __lsm_ro_after_init = {
>         LSM_HOOK_INIT(sb_free_security, ipe_sb_free_security),
> +       LSM_HOOK_INIT(bprm_check_security, ipe_bprm_check_security),
> +       LSM_HOOK_INIT(mmap_file, ipe_mmap_file),
> +       LSM_HOOK_INIT(file_mprotect, ipe_file_mprotect),
> +       LSM_HOOK_INIT(kernel_read_file, ipe_kernel_read_file),
> +       LSM_HOOK_INIT(kernel_load_data, ipe_kernel_load_data),
>  };
>
>  /**
> --
> 2.39.0

--
paul-moore.com

Fan Wu April 10, 2023, 9:22 p.m. UTC | #4

On Thu, Mar 02, 2023 at 02:05:20PM -0500, Paul Moore wrote:
> On Mon, Jan 30, 2023 at 5:59???PM Fan Wu <[e-mail]> wrote:
> >
> > From: Deven Bowers <[e-mail]>
> >
> > 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 <[e-mail]>
> > Signed-off-by: Fan Wu <[e-mail]>
> 
> ...
> 
> > ---
> >  security/ipe/hooks.c | 169 +++++++++++++++++++++++++++++++++++++++++++
> >  security/ipe/hooks.h |  13 ++++
> >  security/ipe/ipe.c   |   6 ++
> >  3 files changed, 188 insertions(+)
> >
> > diff --git a/security/ipe/hooks.c b/security/ipe/hooks.c
> > index 335b773c7ae1..fd5109e29c76 100644
> > --- a/security/ipe/hooks.c
> > +++ b/security/ipe/hooks.c
> > @@ -23,3 +23,172 @@ void ipe_sb_free_security(struct super_block *mnt_sb)
> >  {
> >         ipe_invalidate_pinned_sb(mnt_sb);
> >  }
> > +
> > +/**
> > + * ipe_bprm_check_security - ipe security hook function for bprm check.
> > + * @bprm: Supplies a pointer to a linux_binprm structure to source the file
> > + *       being evaluated.
> > + *
> > + * This LSM hook is called when a binary is loaded through the exec
> > + * family of system calls.
> > + * Return:
> > + * *0  - OK
> > + * *!0 - Error
> > + */
> > +int ipe_bprm_check_security(struct linux_binprm *bprm)
> > +{
> > +       struct ipe_eval_ctx ctx = { 0 };
> > +
> > +       build_eval_ctx(&ctx, bprm->file, ipe_op_exec);
> > +       return ipe_evaluate_event(&ctx);
> > +}
> > +
> > +/**
> > + * ipe_mmap_file - ipe security hook function for mmap check.
> > + * @f: File being mmap'd. Can be NULL in the case of anonymous memory.
> > + * @reqprot: The requested protection on the mmap, passed from usermode.
> > + * @prot: The effective protection on the mmap, resolved from reqprot and
> > + *       system configuration.
> > + * @flags: Unused.
> > + *
> > + * This hook is called when a file is loaded through the mmap
> > + * family of system calls.
> > + *
> > + * Return:
> > + * * 0 - OK
> > + * * !0        - Error
> > + */
> > +int ipe_mmap_file(struct file *f, unsigned long reqprot, unsigned long prot,
> > +                 unsigned long flags)
> > +{
> > +       struct ipe_eval_ctx ctx = { 0 };
> > +
> > +       if (prot & PROT_EXEC || reqprot & PROT_EXEC) {
> 
> Is there a reason why you care about @reqprot?  It seems like IPE
> would only be interested in the protection flags that the kernel is
> actually using.
> 
> I notice that in the `ipe_file_mprotect()` hook you ignore @reqprot,
> which I believe is the right thing to do.
> 

Yes I double checked and found that's not necessary, I will remove that.

-Fan

> > +               build_eval_ctx(&ctx, f, ipe_op_exec);
> > +               return ipe_evaluate_event(&ctx);
> > +       }
> > +
> > +       return 0;
> > +}
> > +
> > +/**
> > + * ipe_file_mprotect - ipe security hook function for mprotect check.
> > + * @vma: Existing virtual memory area created by mmap or similar.
> > + * @reqprot: The requested protection on the mmap, passed from usermode.
> > + * @prot: The effective protection on the mmap, resolved from reqprot and
> > + *       system configuration.
> > + *
> > + * This LSM hook is called when a mmap'd region of memory is changing
> > + * its protections via mprotect.
> > + *
> > + * Return:
> > + * * 0 - OK
> > + * * !0        - Error
> > + */
> > +int ipe_file_mprotect(struct vm_area_struct *vma, unsigned long reqprot,
> > +                     unsigned long prot)
> > +{
> > +       struct ipe_eval_ctx ctx = { 0 };
> > +
> > +       /* Already Executable */
> > +       if (vma->vm_flags & VM_EXEC)
> > +               return 0;
> > +
> > +       if (prot & PROT_EXEC) {
> > +               build_eval_ctx(&ctx, vma->vm_file, ipe_op_exec);
> > +               return ipe_evaluate_event(&ctx);
> > +       }
> > +
> > +       return 0;
> > +}
> > +
> > +/**
> > + * 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;
> > +
> > +       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_max;
> > +               WARN(op == ipe_op_max, "no rule setup for enum %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 = { 0 };
> > +
> > +       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_max;
> > +               WARN(op == ipe_op_max, "no rule setup for enum %d", id);
> > +       }
> > +
> > +       build_eval_ctx(&ctx, NULL, op);
> > +       return ipe_evaluate_event(&ctx);
> > +}
> > diff --git a/security/ipe/hooks.h b/security/ipe/hooks.h
> > index 30fe455389bf..857cae69678c 100644
> > --- a/security/ipe/hooks.h
> > +++ b/security/ipe/hooks.h
> > @@ -11,4 +11,17 @@
> >
> >  void ipe_sb_free_security(struct super_block *mnt_sb);
> >
> > +int ipe_bprm_check_security(struct linux_binprm *bprm);
> > +
> > +int ipe_mmap_file(struct file *f, unsigned long reqprot, unsigned long prot,
> > +                 unsigned long flags);
> > +
> > +int ipe_file_mprotect(struct vm_area_struct *vma, unsigned long reqprot,
> > +                     unsigned long prot);
> > +
> > +int ipe_kernel_read_file(struct file *file, enum kernel_read_file_id id,
> > +                        bool contents);
> > +
> > +int ipe_kernel_load_data(enum kernel_load_data_id id, bool contents);
> > +
> >  #endif /* IPE_HOOKS_H */
> > diff --git a/security/ipe/ipe.c b/security/ipe/ipe.c
> > index bef923026b50..7af2f942decd 100644
> > --- a/security/ipe/ipe.c
> > +++ b/security/ipe/ipe.c
> > @@ -4,6 +4,7 @@
> >   */
> >
> >  #include "ipe.h"
> > +#include "hooks.h"
> >
> >  bool ipe_enabled;
> >
> > @@ -12,6 +13,11 @@ static struct lsm_blob_sizes ipe_blobs __lsm_ro_after_init = {
> >
> >  static struct security_hook_list ipe_hooks[] __lsm_ro_after_init = {
> >         LSM_HOOK_INIT(sb_free_security, ipe_sb_free_security),
> > +       LSM_HOOK_INIT(bprm_check_security, ipe_bprm_check_security),
> > +       LSM_HOOK_INIT(mmap_file, ipe_mmap_file),
> > +       LSM_HOOK_INIT(file_mprotect, ipe_file_mprotect),
> > +       LSM_HOOK_INIT(kernel_read_file, ipe_kernel_read_file),
> > +       LSM_HOOK_INIT(kernel_load_data, ipe_kernel_load_data),
> >  };
> >
> >  /**
> > --
> > 2.39.0
> 
> --
> paul-moore.com

Patch

diff --git a/security/ipe/hooks.c b/security/ipe/hooks.c
index 335b773c7ae1..fd5109e29c76 100644
--- a/security/ipe/hooks.c
+++ b/security/ipe/hooks.c
@@ -23,3 +23,172 @@  void ipe_sb_free_security(struct super_block *mnt_sb)
 {
 	ipe_invalidate_pinned_sb(mnt_sb);
 }
+
+/**
+ * ipe_bprm_check_security - ipe security hook function for bprm check.
+ * @bprm: Supplies a pointer to a linux_binprm structure to source the file
+ *	  being evaluated.
+ *
+ * This LSM hook is called when a binary is loaded through the exec
+ * family of system calls.
+ * Return:
+ * *0	- OK
+ * *!0	- Error
+ */
+int ipe_bprm_check_security(struct linux_binprm *bprm)
+{
+	struct ipe_eval_ctx ctx = { 0 };
+
+	build_eval_ctx(&ctx, bprm->file, ipe_op_exec);
+	return ipe_evaluate_event(&ctx);
+}
+
+/**
+ * ipe_mmap_file - ipe security hook function for mmap check.
+ * @f: File being mmap'd. Can be NULL in the case of anonymous memory.
+ * @reqprot: The requested protection on the mmap, passed from usermode.
+ * @prot: The effective protection on the mmap, resolved from reqprot and
+ *	  system configuration.
+ * @flags: Unused.
+ *
+ * This hook is called when a file is loaded through the mmap
+ * family of system calls.
+ *
+ * Return:
+ * * 0	- OK
+ * * !0	- Error
+ */
+int ipe_mmap_file(struct file *f, unsigned long reqprot, unsigned long prot,
+		  unsigned long flags)
+{
+	struct ipe_eval_ctx ctx = { 0 };
+
+	if (prot & PROT_EXEC || reqprot & PROT_EXEC) {
+		build_eval_ctx(&ctx, f, ipe_op_exec);
+		return ipe_evaluate_event(&ctx);
+	}
+
+	return 0;
+}
+
+/**
+ * ipe_file_mprotect - ipe security hook function for mprotect check.
+ * @vma: Existing virtual memory area created by mmap or similar.
+ * @reqprot: The requested protection on the mmap, passed from usermode.
+ * @prot: The effective protection on the mmap, resolved from reqprot and
+ *	  system configuration.
+ *
+ * This LSM hook is called when a mmap'd region of memory is changing
+ * its protections via mprotect.
+ *
+ * Return:
+ * * 0	- OK
+ * * !0	- Error
+ */
+int ipe_file_mprotect(struct vm_area_struct *vma, unsigned long reqprot,
+		      unsigned long prot)
+{
+	struct ipe_eval_ctx ctx = { 0 };
+
+	/* Already Executable */
+	if (vma->vm_flags & VM_EXEC)
+		return 0;
+
+	if (prot & PROT_EXEC) {
+		build_eval_ctx(&ctx, vma->vm_file, ipe_op_exec);
+		return ipe_evaluate_event(&ctx);
+	}
+
+	return 0;
+}
+
+/**
+ * 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;
+
+	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_max;
+		WARN(op == ipe_op_max, "no rule setup for enum %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 = { 0 };
+
+	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_max;
+		WARN(op == ipe_op_max, "no rule setup for enum %d", id);
+	}
+
+	build_eval_ctx(&ctx, NULL, op);
+	return ipe_evaluate_event(&ctx);
+}
diff --git a/security/ipe/hooks.h b/security/ipe/hooks.h
index 30fe455389bf..857cae69678c 100644
--- a/security/ipe/hooks.h
+++ b/security/ipe/hooks.h
@@ -11,4 +11,17 @@ 
 
 void ipe_sb_free_security(struct super_block *mnt_sb);
 
+int ipe_bprm_check_security(struct linux_binprm *bprm);
+
+int ipe_mmap_file(struct file *f, unsigned long reqprot, unsigned long prot,
+		  unsigned long flags);
+
+int ipe_file_mprotect(struct vm_area_struct *vma, unsigned long reqprot,
+		      unsigned long prot);
+
+int ipe_kernel_read_file(struct file *file, enum kernel_read_file_id id,
+			 bool contents);
+
+int ipe_kernel_load_data(enum kernel_load_data_id id, bool contents);
+
 #endif /* IPE_HOOKS_H */
diff --git a/security/ipe/ipe.c b/security/ipe/ipe.c
index bef923026b50..7af2f942decd 100644
--- a/security/ipe/ipe.c
+++ b/security/ipe/ipe.c
@@ -4,6 +4,7 @@ 
  */
 
 #include "ipe.h"
+#include "hooks.h"
 
 bool ipe_enabled;
 
@@ -12,6 +13,11 @@  static struct lsm_blob_sizes ipe_blobs __lsm_ro_after_init = {
 
 static struct security_hook_list ipe_hooks[] __lsm_ro_after_init = {
 	LSM_HOOK_INIT(sb_free_security, ipe_sb_free_security),
+	LSM_HOOK_INIT(bprm_check_security, ipe_bprm_check_security),
+	LSM_HOOK_INIT(mmap_file, ipe_mmap_file),
+	LSM_HOOK_INIT(file_mprotect, ipe_file_mprotect),
+	LSM_HOOK_INIT(kernel_read_file, ipe_kernel_read_file),
+	LSM_HOOK_INIT(kernel_load_data, ipe_kernel_load_data),
 };
 
 /**