Re: [PATCH RFC v10 1/17] security: add ipe lsm

[Fan Wu]

On Sat, Jul 08, 2023 at 12:22:59AM -0400, Paul Moore wrote:
> On Jun 28, 2023 Fan Wu <wufan@xxxxxxxxxxxxxxxxxxx> wrote:
> > 
> > Integrity Policy Enforcement (IPE) is an LSM that provides an
> > complimentary approach to Mandatory Access Control than existing LSMs
> > today.
> > 
> > Existing LSMs have centered around the concept of access to a resource
> > should be controlled by the current user's credentials. IPE's approach,
> > is that access to a resource should be controlled by the system's trust
> > of a current resource.
> > 
> > The basis of this approach is defining a global policy to specify which
> > resource can be trusted.
> > 
> > Signed-off-by: Deven Bowers <deven.desai@xxxxxxxxxxxxxxxxxxx>
> > Signed-off-by: Fan Wu <wufan@xxxxxxxxxxxxxxxxxxx>
> > ---
> >  MAINTAINERS           |  7 +++++++
> >  security/Kconfig      | 11 ++++++-----
> >  security/Makefile     |  1 +
> >  security/ipe/Kconfig  | 17 +++++++++++++++++
> >  security/ipe/Makefile | 10 ++++++++++
> >  security/ipe/ipe.c    | 37 +++++++++++++++++++++++++++++++++++++
> >  security/ipe/ipe.h    | 16 ++++++++++++++++
> >  7 files changed, 94 insertions(+), 5 deletions(-)
> >  create mode 100644 security/ipe/Kconfig
> >  create mode 100644 security/ipe/Makefile
> >  create mode 100644 security/ipe/ipe.c
> >  create mode 100644 security/ipe/ipe.h
> 
> ...
> 
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index a82795114ad4..ad00887d38ea 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -10278,6 +10278,13 @@ T:	git git://git.kernel.org/pub/scm/linux/kernel/git/zohar/linux-integrity.git
> >  F:	security/integrity/
> >  F:	security/integrity/ima/
> >  
> > +INTEGRITY POLICY ENFORCEMENT (IPE)
> > +M:	Fan Wu <wufan@xxxxxxxxxxxxxxxxxxx>
> > +L:	linux-security-module@xxxxxxxxxxxxxxx
> > +S:	Supported
> > +T:	git git://github.com/microsoft/ipe.git
> 
> Using the raw git protocol doesn't seem to work with GH, I think you
> need to refernce the git/https URL:
> 
>  https://github.com/microsoft/ipe.git
> 
Sure I can change it.

> > +F:	security/ipe/
> > +
> >  INTEL 810/815 FRAMEBUFFER DRIVER
> >  M:	Antonino Daplas <adaplas@xxxxxxxxx>
> >  L:	linux-fbdev@xxxxxxxxxxxxxxx
> > diff --git a/security/Kconfig b/security/Kconfig
> > index 97abeb9b9a19..daa4626ea99c 100644
> > --- a/security/Kconfig
> > +++ b/security/Kconfig
> > @@ -202,6 +202,7 @@ source "security/yama/Kconfig"
> >  source "security/safesetid/Kconfig"
> >  source "security/lockdown/Kconfig"
> >  source "security/landlock/Kconfig"
> > +source "security/ipe/Kconfig"
> >  
> >  source "security/integrity/Kconfig"
> >  
> > @@ -241,11 +242,11 @@ endchoice
> >  
> >  config LSM
> >  	string "Ordered list of enabled LSMs"
> > -	default "landlock,lockdown,yama,loadpin,safesetid,smack,selinux,tomoyo,apparmor,bpf" if DEFAULT_SECURITY_SMACK
> > -	default "landlock,lockdown,yama,loadpin,safesetid,apparmor,selinux,smack,tomoyo,bpf" if DEFAULT_SECURITY_APPARMOR
> > -	default "landlock,lockdown,yama,loadpin,safesetid,tomoyo,bpf" if DEFAULT_SECURITY_TOMOYO
> > -	default "landlock,lockdown,yama,loadpin,safesetid,bpf" if DEFAULT_SECURITY_DAC
> > -	default "landlock,lockdown,yama,loadpin,safesetid,selinux,smack,tomoyo,apparmor,bpf"
> > +	default "landlock,lockdown,yama,loadpin,safesetid,smack,selinux,tomoyo,apparmor,bpf,ipe" if DEFAULT_SECURITY_SMACK
> > +	default "landlock,lockdown,yama,loadpin,safesetid,apparmor,selinux,smack,tomoyo,bpf,ipe" if DEFAULT_SECURITY_APPARMOR
> > +	default "landlock,lockdown,yama,loadpin,safesetid,tomoyo,bpf,ipe" if DEFAULT_SECURITY_TOMOYO
> > +	default "landlock,lockdown,yama,loadpin,safesetid,bpf,ipe" if DEFAULT_SECURITY_DAC
> > +	default "landlock,lockdown,yama,loadpin,safesetid,selinux,smack,tomoyo,apparmor,bpf,ipe"
> 
> Generally speaking the BPF LSM should be the last entry in the LSM
> list to help prevent issues caused by a BPF LSM returning an improper
> error and shortcutting a LSM after it.
> 
Thanks for the insight, I will update this part.

> >  	help
> >  	  A comma-separated list of LSMs, in initialization order.
> >  	  Any LSMs left off this list, except for those with order
> 
> ...
> 
> > diff --git a/security/ipe/Makefile b/security/ipe/Makefile
> > new file mode 100644
> > index 000000000000..571648579991
> > --- /dev/null
> > +++ b/security/ipe/Makefile
> > @@ -0,0 +1,10 @@
> > +# SPDX-License-Identifier: GPL-2.0
> > +#
> > +# Copyright (C) Microsoft Corporation. All rights reserved.
> > +#
> > +# Makefile for building the IPE module as part of the kernel tree.
> > +#
> > +
> > +obj-$(CONFIG_SECURITY_IPE) += \
> > +	hooks.o \
> > +	ipe.o \
> 
> It doesn't look like security/ipe/hook.c is included in this patch.
> 
> It is important to ensure that each patch compiles after it is
> applied.
Sorry this was accidentally added during a rebase, I will try to avoid such a mistake in the future.

-Fan

> 
> --
> paul-moore.com