mbox series

[0/9] integrity: Move hooks into LSM

Message ID 20221013222702.never.990-kees@kernel.org (mailing list archive)
Headers show
Series integrity: Move hooks into LSM | expand

Message

Kees Cook Oct. 13, 2022, 10:36 p.m. UTC
Hi,

It's been over 4 years since LSM stack was introduced. The integrity
subsystem is long overdue for moving to this infrastructure. Here's my
first pass at converting integrity and ima (and some of evm) into LSM
hooks. This should be enough of an example to finish evm, and introduce
the missing hooks for both. For example, after this, it looks like ima
only has a couple places it's still doing things outside of the LSM. At
least these stood out:

fs/namei.c:     ima_post_create_tmpfile(mnt_userns, inode);
fs/namei.c:                             ima_post_path_mknod(mnt_userns, dentry);

Mimi, can you please take this series and finish the conversion for
what's missing in ima and evm?

I would also call attention to "175 insertions(+), 240 deletions(-)" --
as expected, this is a net reduction in code.

Thanks!

-Kees

Kees Cook (9):
  integrity: Prepare for having "ima" and "evm" available in "integrity"
    LSM
  security: Move trivial IMA hooks into LSM
  ima: Move xattr hooks into LSM
  ima: Move ima_file_free() into LSM
  LSM: Introduce inode_post_setattr hook
  fs: Introduce file_to_perms() helper
  ima: Move ima_file_check() into LSM
  integrity: Move trivial hooks into LSM
  integrity: Move integrity_inode_get() out of global header

 fs/attr.c                             |  3 +-
 fs/file_table.c                       |  1 -
 fs/namei.c                            |  2 -
 fs/nfsd/vfs.c                         |  6 --
 include/linux/evm.h                   |  6 --
 include/linux/fs.h                    | 22 +++++++
 include/linux/ima.h                   | 87 ---------------------------
 include/linux/integrity.h             | 30 +--------
 include/linux/lsm_hook_defs.h         |  3 +
 security/Kconfig                      | 10 +--
 security/apparmor/include/file.h      | 18 ++----
 security/integrity/evm/evm_main.c     | 14 ++++-
 security/integrity/iint.c             | 28 +++++++--
 security/integrity/ima/ima.h          | 12 ++++
 security/integrity/ima/ima_appraise.c | 21 +++++--
 security/integrity/ima/ima_main.c     | 66 ++++++++++++++------
 security/integrity/integrity.h        |  8 +++
 security/security.c                   | 78 ++++++------------------
 18 files changed, 175 insertions(+), 240 deletions(-)

Comments

Paul Moore Oct. 13, 2022, 10:47 p.m. UTC | #1
On Thu, Oct 13, 2022 at 6:36 PM Kees Cook <keescook@chromium.org> wrote:
>
> Hi,
>
> It's been over 4 years since LSM stack was introduced. The integrity
> subsystem is long overdue for moving to this infrastructure. Here's my
> first pass at converting integrity and ima (and some of evm) into LSM
> hooks. This should be enough of an example to finish evm, and introduce
> the missing hooks for both. For example, after this, it looks like ima
> only has a couple places it's still doing things outside of the LSM. At
> least these stood out:
>
> fs/namei.c:     ima_post_create_tmpfile(mnt_userns, inode);
> fs/namei.c:                             ima_post_path_mknod(mnt_userns, dentry);
>
> Mimi, can you please take this series and finish the conversion for
> what's missing in ima and evm?
>
> I would also call attention to "175 insertions(+), 240 deletions(-)" --
> as expected, this is a net reduction in code.
>
> Thanks!

Without looking at any of the code, I just want to say this 100% gets
my vote; this is something we need to make happen at some point.

Thanks Kees!

> Kees Cook (9):
>   integrity: Prepare for having "ima" and "evm" available in "integrity"
>     LSM
>   security: Move trivial IMA hooks into LSM
>   ima: Move xattr hooks into LSM
>   ima: Move ima_file_free() into LSM
>   LSM: Introduce inode_post_setattr hook
>   fs: Introduce file_to_perms() helper
>   ima: Move ima_file_check() into LSM
>   integrity: Move trivial hooks into LSM
>   integrity: Move integrity_inode_get() out of global header
>
>  fs/attr.c                             |  3 +-
>  fs/file_table.c                       |  1 -
>  fs/namei.c                            |  2 -
>  fs/nfsd/vfs.c                         |  6 --
>  include/linux/evm.h                   |  6 --
>  include/linux/fs.h                    | 22 +++++++
>  include/linux/ima.h                   | 87 ---------------------------
>  include/linux/integrity.h             | 30 +--------
>  include/linux/lsm_hook_defs.h         |  3 +
>  security/Kconfig                      | 10 +--
>  security/apparmor/include/file.h      | 18 ++----
>  security/integrity/evm/evm_main.c     | 14 ++++-
>  security/integrity/iint.c             | 28 +++++++--
>  security/integrity/ima/ima.h          | 12 ++++
>  security/integrity/ima/ima_appraise.c | 21 +++++--
>  security/integrity/ima/ima_main.c     | 66 ++++++++++++++------
>  security/integrity/integrity.h        |  8 +++
>  security/security.c                   | 78 ++++++------------------
>  18 files changed, 175 insertions(+), 240 deletions(-)
Mimi Zohar Oct. 14, 2022, 1:16 a.m. UTC | #2
On Thu, 2022-10-13 at 18:47 -0400, Paul Moore wrote:
> On Thu, Oct 13, 2022 at 6:36 PM Kees Cook <keescook@chromium.org> wrote:
> >
> > Hi,
> >
> > It's been over 4 years since LSM stack was introduced. The integrity
> > subsystem is long overdue for moving to this infrastructure. Here's my
> > first pass at converting integrity and ima (and some of evm) into LSM
> > hooks. This should be enough of an example to finish evm, and introduce
> > the missing hooks for both. For example, after this, it looks like ima
> > only has a couple places it's still doing things outside of the LSM. At
> > least these stood out:
> >
> > fs/namei.c:     ima_post_create_tmpfile(mnt_userns, inode);
> > fs/namei.c:                             ima_post_path_mknod(mnt_userns, dentry);
> >
> > Mimi, can you please take this series and finish the conversion for
> > what's missing in ima and evm?
> >
> > I would also call attention to "175 insertions(+), 240 deletions(-)" --
> > as expected, this is a net reduction in code.
> >
> > Thanks!
> 
> Without looking at any of the code, I just want to say this 100% gets
> my vote; this is something we need to make happen at some point.
> 
> Thanks Kees!

Sorry I'm on vacation this week and the beginning of next week, but
will look at it when I get back.

Mimi
Mickaël Salaün Oct. 18, 2022, 3:31 p.m. UTC | #3
There is a complementary patch series that didn't received review: 
https://lore.kernel.org/all/20210427113732.471066-1-roberto.sassu@huawei.com/

On 14/10/2022 00:36, Kees Cook wrote:
> Hi,
> 
> It's been over 4 years since LSM stack was introduced. The integrity
> subsystem is long overdue for moving to this infrastructure. Here's my
> first pass at converting integrity and ima (and some of evm) into LSM
> hooks. This should be enough of an example to finish evm, and introduce
> the missing hooks for both. For example, after this, it looks like ima
> only has a couple places it's still doing things outside of the LSM. At
> least these stood out:
> 
> fs/namei.c:     ima_post_create_tmpfile(mnt_userns, inode);
> fs/namei.c:                             ima_post_path_mknod(mnt_userns, dentry);
> 
> Mimi, can you please take this series and finish the conversion for
> what's missing in ima and evm?
> 
> I would also call attention to "175 insertions(+), 240 deletions(-)" --
> as expected, this is a net reduction in code.
> 
> Thanks!
> 
> -Kees
> 
> Kees Cook (9):
>    integrity: Prepare for having "ima" and "evm" available in "integrity"
>      LSM
>    security: Move trivial IMA hooks into LSM
>    ima: Move xattr hooks into LSM
>    ima: Move ima_file_free() into LSM
>    LSM: Introduce inode_post_setattr hook
>    fs: Introduce file_to_perms() helper
>    ima: Move ima_file_check() into LSM
>    integrity: Move trivial hooks into LSM
>    integrity: Move integrity_inode_get() out of global header
> 
>   fs/attr.c                             |  3 +-
>   fs/file_table.c                       |  1 -
>   fs/namei.c                            |  2 -
>   fs/nfsd/vfs.c                         |  6 --
>   include/linux/evm.h                   |  6 --
>   include/linux/fs.h                    | 22 +++++++
>   include/linux/ima.h                   | 87 ---------------------------
>   include/linux/integrity.h             | 30 +--------
>   include/linux/lsm_hook_defs.h         |  3 +
>   security/Kconfig                      | 10 +--
>   security/apparmor/include/file.h      | 18 ++----
>   security/integrity/evm/evm_main.c     | 14 ++++-
>   security/integrity/iint.c             | 28 +++++++--
>   security/integrity/ima/ima.h          | 12 ++++
>   security/integrity/ima/ima_appraise.c | 21 +++++--
>   security/integrity/ima/ima_main.c     | 66 ++++++++++++++------
>   security/integrity/integrity.h        |  8 +++
>   security/security.c                   | 78 ++++++------------------
>   18 files changed, 175 insertions(+), 240 deletions(-)
>
Roberto Sassu Oct. 18, 2022, 3:38 p.m. UTC | #4
On Tue, 2022-10-18 at 17:31 +0200, Mickaël Salaün wrote:
> There is a complementary patch series that didn't received review: 
> https://lore.kernel.org/all/20210427113732.471066-1-roberto.sassu@huawei.com/

Thanks Mickael, just pushed to Github the patch set you mentioned + IMA
on LSM infrastructure.

That patch set was a prerequisite: allowing multiple stacked LSMs to
provide an xattr at inode creation time.

Roberto

> On 14/10/2022 00:36, Kees Cook wrote:
> > Hi,
> > 
> > It's been over 4 years since LSM stack was introduced. The
> > integrity
> > subsystem is long overdue for moving to this infrastructure. Here's
> > my
> > first pass at converting integrity and ima (and some of evm) into
> > LSM
> > hooks. This should be enough of an example to finish evm, and
> > introduce
> > the missing hooks for both. For example, after this, it looks like
> > ima
> > only has a couple places it's still doing things outside of the
> > LSM. At
> > least these stood out:
> > 
> > fs/namei.c:     ima_post_create_tmpfile(mnt_userns, inode);
> > fs/namei.c:                             ima_post_path_mknod(mnt_use
> > rns, dentry);
> > 
> > Mimi, can you please take this series and finish the conversion for
> > what's missing in ima and evm?
> > 
> > I would also call attention to "175 insertions(+), 240 deletions(-
> > )" --
> > as expected, this is a net reduction in code.
> > 
> > Thanks!
> > 
> > -Kees
> > 
> > Kees Cook (9):
> >    integrity: Prepare for having "ima" and "evm" available in
> > "integrity"
> >      LSM
> >    security: Move trivial IMA hooks into LSM
> >    ima: Move xattr hooks into LSM
> >    ima: Move ima_file_free() into LSM
> >    LSM: Introduce inode_post_setattr hook
> >    fs: Introduce file_to_perms() helper
> >    ima: Move ima_file_check() into LSM
> >    integrity: Move trivial hooks into LSM
> >    integrity: Move integrity_inode_get() out of global header
> > 
> >   fs/attr.c                             |  3 +-
> >   fs/file_table.c                       |  1 -
> >   fs/namei.c                            |  2 -
> >   fs/nfsd/vfs.c                         |  6 --
> >   include/linux/evm.h                   |  6 --
> >   include/linux/fs.h                    | 22 +++++++
> >   include/linux/ima.h                   | 87 ----------------------
> > -----
> >   include/linux/integrity.h             | 30 +--------
> >   include/linux/lsm_hook_defs.h         |  3 +
> >   security/Kconfig                      | 10 +--
> >   security/apparmor/include/file.h      | 18 ++----
> >   security/integrity/evm/evm_main.c     | 14 ++++-
> >   security/integrity/iint.c             | 28 +++++++--
> >   security/integrity/ima/ima.h          | 12 ++++
> >   security/integrity/ima/ima_appraise.c | 21 +++++--
> >   security/integrity/ima/ima_main.c     | 66 ++++++++++++++------
> >   security/integrity/integrity.h        |  8 +++
> >   security/security.c                   | 78 ++++++--------------
> > ----
> >   18 files changed, 175 insertions(+), 240 deletions(-)
> >
Kees Cook Oct. 18, 2022, 6:31 p.m. UTC | #5
On Tue, Oct 18, 2022 at 05:31:59PM +0200, Mickaël Salaün wrote:
> There is a complementary patch series that didn't received review:
> https://lore.kernel.org/all/20210427113732.471066-1-roberto.sassu@huawei.com/

Yeah, this looks interesting! I hope Mimi or other integrity folks can
review these.
Casey Schaufler Oct. 20, 2022, 5:36 p.m. UTC | #6
On 10/13/2022 3:36 PM, Kees Cook wrote:
> Hi,
>
> It's been over 4 years since LSM stack was introduced. The integrity
> subsystem is long overdue for moving to this infrastructure. Here's my
> first pass at converting integrity and ima (and some of evm) into LSM
> hooks. This should be enough of an example to finish evm, and introduce
> the missing hooks for both. For example, after this, it looks like ima
> only has a couple places it's still doing things outside of the LSM. At
> least these stood out:
>
> fs/namei.c:     ima_post_create_tmpfile(mnt_userns, inode);
> fs/namei.c:                             ima_post_path_mknod(mnt_userns, dentry);
>
> Mimi, can you please take this series and finish the conversion for
> what's missing in ima and evm?
>
> I would also call attention to "175 insertions(+), 240 deletions(-)" --
> as expected, this is a net reduction in code.
>
> Thanks!
>
> -Kees

I endorse this effort.

>
> Kees Cook (9):
>   integrity: Prepare for having "ima" and "evm" available in "integrity"
>     LSM
>   security: Move trivial IMA hooks into LSM
>   ima: Move xattr hooks into LSM
>   ima: Move ima_file_free() into LSM
>   LSM: Introduce inode_post_setattr hook
>   fs: Introduce file_to_perms() helper
>   ima: Move ima_file_check() into LSM
>   integrity: Move trivial hooks into LSM
>   integrity: Move integrity_inode_get() out of global header
>
>  fs/attr.c                             |  3 +-
>  fs/file_table.c                       |  1 -
>  fs/namei.c                            |  2 -
>  fs/nfsd/vfs.c                         |  6 --
>  include/linux/evm.h                   |  6 --
>  include/linux/fs.h                    | 22 +++++++
>  include/linux/ima.h                   | 87 ---------------------------
>  include/linux/integrity.h             | 30 +--------
>  include/linux/lsm_hook_defs.h         |  3 +
>  security/Kconfig                      | 10 +--
>  security/apparmor/include/file.h      | 18 ++----
>  security/integrity/evm/evm_main.c     | 14 ++++-
>  security/integrity/iint.c             | 28 +++++++--
>  security/integrity/ima/ima.h          | 12 ++++
>  security/integrity/ima/ima_appraise.c | 21 +++++--
>  security/integrity/ima/ima_main.c     | 66 ++++++++++++++------
>  security/integrity/integrity.h        |  8 +++
>  security/security.c                   | 78 ++++++------------------
>  18 files changed, 175 insertions(+), 240 deletions(-)
>