Re: [PATCH v6 0/6] Add CA enforcement keyring restrictions

[Jarkko Sakkinen]

On Wed, Mar 22, 2023 at 12:16:28PM -0400, Eric Snowberg wrote:
> Prior to the introduction of the machine keyring, most distros simply 
> allowed all keys contained within the platform keyring to be used
> for both kernel and module verification.  This was done by an out of
> tree patch.  Some distros took it even further and loaded all these keys
> into the secondary trusted keyring.  This also allowed the system owner 
> to add their own key for IMA usage.
> 
> Each distro contains similar documentation on how to sign kernel modules
> and enroll the key into the MOK.  The process is fairly straightforward.
> With the introduction of the machine keyring, the process remains
> basically the same, without the need for any out of tree patches.
> 
> The machine keyring allowed distros to eliminate the out of tree patches
> for kernel module signing.  However, it falls short in allowing the end 
> user to add their own keys for IMA. Currently, the machine keyring can not 
> be used as another trust anchor for adding keys to the ima keyring, since 
> CA enforcement does not currently exist.  This would expand the current 
> integrity gap. The IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY 
> Kconfig states that keys may be added to the ima keyrings if the key is 
> validly signed by a CA cert in the system built-in or secondary trusted 
> keyring.  Currently, there is not code that enforces the contents of a
> CA cert.
> 
> This series introduces a way to do CA enforcement with the machine
> keyring.  It introduces three different ways to configure the machine
> keyring.  New Kconfig options are added to control the types of keys
> that may be added to it.  The default option allows all MOK keys into the
> machine keyring.  When CONFIG_INTEGRITY_CA_MACHINE_KEYRING is selected,
> the X.509 CA bit must be true and the key usage must contain keyCertSign; 
> any other usage field may also be set.  When
> CONFIG_INTEGRITY_CA_MACHINE_KEYRING_MAX is also selected, the X.509 CA
> bit must be true and the key usage must contain keyCertSign. With this
> option digitialSignature usage may not be set.  If a key doesn't pass 
> the CA restriction check, instead of going into the machine keyring, it 
> is added to the platform keyring.  With the ability to configure the
> machine keyring with CA restrictions, code that prevented the machine
> keyring from being enabled with
> IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY has been removed.
> 
> Changelog:
> v6:
> - No new code changes
> - Added Reviewed-by and ACKs
> - Formatting change requested by Jarkko
> 
> v5:
> - Removed the Kconfig _MIN Kconfig option and split it into different
>   entries.
> - Added requested commit message changes
> 
> v4:
> - Removed all code that validated the certificate chain back to the root
>   CA. Now the only restriction is what is initially placed in the
>   machine keyring.
> - Check and store if the X.509 usage contains digitalSignature
> - New Kconfig menu item with none, min and max CA restriction on the 
>   machine keyring
> 
> v3:
> - Allow Intermediate CA certs to be enrolled through the MOK. The
>   Intermediate CA cert must contain keyCertSign key usage and have the 
>   CA bit set to true. This was done by removing the self signed
>   requirement.
> 
> Eric Snowberg (6):
>   KEYS: Create static version of public_key_verify_signature
>   KEYS: Add missing function documentation
>   KEYS: X.509: Parse Basic Constraints for CA
>   KEYS: X.509: Parse Key Usage
>   KEYS: CA link restriction
>   integrity: machine keyring CA configuration
> 
>  certs/system_keyring.c                    | 14 +++++--
>  crypto/asymmetric_keys/restrict.c         | 45 ++++++++++++++++++++
>  crypto/asymmetric_keys/x509_cert_parser.c | 50 +++++++++++++++++++++++
>  include/crypto/public_key.h               | 28 +++++++++++++
>  security/integrity/Kconfig                | 23 ++++++++++-
>  security/integrity/digsig.c               |  8 +++-
>  6 files changed, 162 insertions(+), 6 deletions(-)
> 
> 
> base-commit: e8d018dd0257f744ca50a729e3d042cf2ec9da65
> -- 
> 2.27.0
> 

I can pick this, and I guess I can add Mimi's tested-by's to all of the
patches?

BR, Jarkko