Message-ID: <c490397315c2704e9ef65c8ad3fefedb239f1997.camel@linux.ibm.com>
Date: Mon, 23 Dec 2024 07:09:56 -0500
From: Mimi Zohar <zohar@...ux.ibm.com>
To: Eric Snowberg <eric.snowberg@...cle.com>,
        linux-security-module@...r.kernel.org
Cc: dhowells@...hat.com, dwmw2@...radead.org, herbert@...dor.apana.org.au,
        davem@...emloft.net, ardb@...nel.org, jarkko@...nel.org,
        paul@...l-moore.com, jmorris@...ei.org, serge@...lyn.com,
        roberto.sassu@...wei.com, dmitry.kasatkin@...il.com, mic@...ikod.net,
        casey@...aufler-ca.com, stefanb@...ux.ibm.com, ebiggers@...nel.org,
        rdunlap@...radead.org, linux-kernel@...r.kernel.org,
        keyrings@...r.kernel.org, linux-crypto@...r.kernel.org,
        linux-efi@...r.kernel.org, linux-integrity@...r.kernel.org
Subject: Re: [RFC PATCH v3 00/13] Clavis LSM

On Thu, 2024-10-17 at 09:55 -0600, Eric Snowberg wrote:
> Motivation:
> 
> Each end-user has their own security threat model. What is important to one
> end-user may not be important to another. There is not a right or wrong threat
> model.
> 
> A common request made when adding new kernel changes that could impact the
> threat model around system kernel keys is to add additional Kconfig options.
> As kernel developers, it is challenging to both add and keep track of all the
> Kconfig options around security features that may limit or restrict
> system key usage.  It is also difficult for a general purpose distro to take
> advantage of some of these features, since it may prevent some users from
> executing their workload.
> 
> It is the author's belief that it is better left up to the end-user on how
> kernel keys should be used within their system.
> 
> Throughout the Linux kernel, key usage is tracked when doing signature
> verification with keys contained within one of the system keyrings;  however,
> there isn't a way for the end-user to enforce this usage.  This series gives the
> end-user the ability to configure key usage based on their threat model.
> Having the ability to enforce key usage also improves security by reducing the
> attack surface should a system key be compromised. It allows new features to be
> added without the need for additional Kconfig options for fear of changing the
> end-user's threat model. It also allows a distro to build a kernel that suits
> various end-user's needs without resorting to selecting Kconfig options with
> the least restrictive security options.

The motivation for this patch set is convincing and addresses limiting the usage
of keys loaded directly or indirectly onto the system trusted keyrings - 
.builtin, .machine, and .secondary_trusted_keys keyrings.  Pre-loading the build
time ephemeral kernel module signing key is a nice improvement from the previous
versions.  My main concern is not with Clavis per-se, but that the LSM
infrastructure allows configuring all the LSMs, but enabling at build time and
modifying at runtime a subset of them.  Without Clavis enabled, nothing changes
- any key on the system trusted keyrings remains usable for any purpose.  With
the current LSM design, the end user security threat model cannot be guaranteed.

Mimi
> 
> Solution:
> 
> This series introduces a new LSM called Clavis (Latin word meaning key).
> This LSM leaves it up to the end-user to determine what system keys they want
> to use and for what purpose.
> 
> The Clavis LSM adds the ability to do access control for all system keys.  When
> enabled, until an ACL entry is added for a specific key, none of the system keys
> may be used for any type of verification purpose.  When the kernel is built,
> typically kernel modules are signed with an ephemeral key, an ACL entry for the
> ephemeral key is pre-loaded, allowing the kernel modules to load during boot. At
> build time other ACL entries may also be included.
> 
> The Clavis LSM requires the end-user to have their own public key infrastructure
> (PKI).  In order for a Clavis ACL entry to be added, the ACL must be signed by
> what is being called the Clavis key.  The Clavis key is owned by the end-user.
> The Clavis public key can be contained within the machine keyring, or it can be
> added after the machine boots.
> 
> Not only is there a new Clavis key being introduced, but there is also a new
> .clavis keyring.  The .clavis keyring contains a single Clavis key. It also
> contains any number of ACL entries that are signed by the Clavis key.
> 
> It is believed that the most common setup would be to have the Clavis key
> contained within the machine keyring. Enabling the Clavis LSM during boot is
> accomplished by passing in the asymmetric key id for the Clavis key within a
> new "clavis=" boot param.  The asymmetric key id must match one already
> contained within any of the system keyrings.  If a match is found, a link is
> created into the new .clavis keyring.  This Clavis key shall be used as the
> root of trust for any keyring ACL updates afterwards.
> 
> On UEFI systems the "clavis" boot param is mirrored into a new UEFI variable
> within the EFI stub code. This variable will persist until the next reboot.
> This same type of functionality is done within shim. Since this variable is
> created before ExitBootServices (EBS) it will not have the NVRAM bit set,
> signifying it was created during the Boot Services phase. This is being used
> so the "clavis" boot param can not be changed via kexec, thereby preventing a
> pivot of the root of trust.
> 
> As mentioned earlier, this LSM introduces a new .clavis keyring.  Following
> boot, no new keys can be added to this keyring and only the key designated via
> the initial boot param may be used. If the clavis boot param was not used, the
> LSM can be enabled afterwards using the keyctl command.  The end-user may add
> their Clavis key into the .clavis keyring and the Clavis LSM shall be enabled.
> 
> The .clavis keyring also holds the access control list for system keys. A new
> key type called clavis_key_acl is being introduced. This contains the usage
> followed by the asymmetric key id. To be added to the clavis keyring, the
> clavis_key_acl must be S/MIME signed by the Clavis key. New ACL additions to
> the .clavis keyring may be added at any time.
> 
> Currently this LSM does not require new changes or modifications to any user
> space tools.  It also does not have a securityfs interface.  Everything is
> done using the existing keyctl tool through the new .clavis keyring. The
> S/MIME signing can be done with a simple OpenSSL command. If additions or
> updates need to be added in the future, new ACL key types could be created.
> With this approach, maintainability should not be an issue in the future
> if missing items are identified.
> 
> Clavis must be configured at build time with CONFIG_SECURITY_CLAVIS=y. The list
> of security modules enabled by default is set with CONFIG_LSM.  The kernel
> configuration must contain CONFIG_LSM=[...],clavis with [...] as the list of
> other security modules for the running system.
> 
> For setup and usage instructions, a clavis admin-guide has been included
> in Documentation/admin-guide/LSM/clavis.rst.
> 
> Future enhancements to this LSM could include:
> 
> 1. Subsystems that currently use system keys with
>    VERIFYING_UNSPECIFIED_SIGNATURE could be updated with their specific usage
>    type.  For example, a usage type for IMA, BPF, etc could be added.
> 
> 2. Having the ability to allow platform keys to be on par with all other
>    system keys when using this LSM. This would be useful for a user that
>    controls their entire UEFI SB DB key chain and doesn't want to use MOK keys.
>    This could also potentially remove the need for the machine keyring all
>    together.
> 
> 3. Some of the Kconfig options around key usage and types could be deprecated.
> 
> I would appreciate any feedback on this approach. Thanks.
> 
> Changes in v3:
>   Rebased to 6.12-rc3
>   Added Kunit test code
>   Preload an ACL in the clavis keyring with the ephemeral module signing key
>   Preload user defined ACL data into the clavis keyring with build time data
>   Changes to the second patch recommended by Jarkko
>   Reordered patches recommended by Mimi
>   Documentation improvements recommended by Randy
> 
> Changes in v2:
>   Rebased to 6.10-rc1
>   Various cleanup in the first patch recommended by Jarkko
>   Documentation improvements recommended by Randy
>   Fixed lint warnings
>   Other cleanup
> 
> Eric Snowberg (13):
>   certs: Remove CONFIG_INTEGRITY_PLATFORM_KEYRING check
>   certs: Introduce ability to link to a system key
>   clavis: Introduce a new system keyring called clavis
>   keys: Add new verification type (VERIFYING_CLAVIS_SIGNATURE)
>   clavis: Introduce a new key type called clavis_key_acl
>   clavis: Populate clavis keyring acl with kernel module signature
>   keys: Add ability to track intended usage of the public key
>   clavis: Introduce new LSM called clavis
>   clavis: Allow user to define acl at build time
>   efi: Make clavis boot param persist across kexec
>   clavis: Prevent boot param change during kexec
>   clavis: Add function redirection for Kunit support
>   clavis: Kunit support
> 
>  Documentation/admin-guide/LSM/clavis.rst      | 191 ++++++
>  .../admin-guide/kernel-parameters.txt         |   6 +
>  MAINTAINERS                                   |   7 +
>  certs/.gitignore                              |   1 +
>  certs/Makefile                                |  20 +
>  certs/blacklist.c                             |   3 +
>  certs/clavis_module_acl.c                     |   7 +
>  certs/system_keyring.c                        |  36 +-
>  crypto/asymmetric_keys/asymmetric_type.c      |   1 +
>  crypto/asymmetric_keys/pkcs7_trust.c          |  20 +
>  crypto/asymmetric_keys/pkcs7_verify.c         |   5 +
>  crypto/asymmetric_keys/signature.c            |   4 +
>  drivers/firmware/efi/Kconfig                  |  12 +
>  drivers/firmware/efi/libstub/Makefile         |   1 +
>  drivers/firmware/efi/libstub/clavis.c         |  33 +
>  .../firmware/efi/libstub/efi-stub-helper.c    |   2 +
>  drivers/firmware/efi/libstub/efi-stub.c       |   2 +
>  drivers/firmware/efi/libstub/efistub.h        |   8 +
>  drivers/firmware/efi/libstub/x86-stub.c       |   2 +
>  include/crypto/pkcs7.h                        |   3 +
>  include/crypto/public_key.h                   |   4 +
>  include/keys/system_keyring.h                 |   7 +-
>  include/linux/efi.h                           |   1 +
>  include/linux/integrity.h                     |   8 +
>  include/linux/lsm_count.h                     |   8 +-
>  include/linux/lsm_hook_defs.h                 |   2 +
>  include/linux/security.h                      |   7 +
>  include/linux/verification.h                  |   2 +
>  include/uapi/linux/lsm.h                      |   1 +
>  security/Kconfig                              |  11 +-
>  security/Makefile                             |   1 +
>  security/clavis/.gitignore                    |   2 +
>  security/clavis/.kunitconfig                  |   4 +
>  security/clavis/Kconfig                       |  37 ++
>  security/clavis/Makefile                      | 156 +++++
>  security/clavis/clavis.c                      |  26 +
>  security/clavis/clavis.h                      |  62 ++
>  security/clavis/clavis_builtin_acl.c          |   7 +
>  security/clavis/clavis_efi.c                  |  50 ++
>  security/clavis/clavis_keyring.c              | 426 +++++++++++++
>  security/clavis/clavis_test.c                 | 566 ++++++++++++++++++
>  security/integrity/iint.c                     |   2 +
>  security/security.c                           |  13 +
>  .../selftests/lsm/lsm_list_modules_test.c     |   3 +
>  44 files changed, 1757 insertions(+), 13 deletions(-)
>  create mode 100644 Documentation/admin-guide/LSM/clavis.rst
>  create mode 100644 certs/clavis_module_acl.c
>  create mode 100644 drivers/firmware/efi/libstub/clavis.c
>  create mode 100644 security/clavis/.gitignore
>  create mode 100644 security/clavis/.kunitconfig
>  create mode 100644 security/clavis/Kconfig
>  create mode 100644 security/clavis/Makefile
>  create mode 100644 security/clavis/clavis.c
>  create mode 100644 security/clavis/clavis.h
>  create mode 100644 security/clavis/clavis_builtin_acl.c
>  create mode 100644 security/clavis/clavis_efi.c
>  create mode 100644 security/clavis/clavis_keyring.c
>  create mode 100644 security/clavis/clavis_test.c
> 
> 
> base-commit: 8e929cb546ee42c9a61d24fae60605e9e3192354

Powered by blists - more mailing lists