Re: [PATCH v3] certs: Add option to disallow non-CA certificates in secondary trusted keying

[Mimi Zohar]

On Mon, 2023-10-09 at 10:10 -0400, Mimi Zohar wrote:
> On Tue, 2023-10-03 at 02:49 +0300, Jarkko Sakkinen wrote:
> > On Mon Oct 2, 2023 at 1:46 PM EEST, Denis Glazkov wrote:
> > > The Linux kernel has an IMA (Integrity Measurement Architecture)
> > > subsystem to check the integrity of the file system based on digital
> > > signatures. IMA uses certificates in `.ima` keying to check integrity.
> > >
> > > Only certificates issued by one of the trusted CA (Certificate Authority)
> > > certificates can be added to the `.ima` keying.
> > >
> > > The Linux kernel now has a secondary trusted keying to which trusted
> > > certificates from user space can be added if you have superuser
> > > privileges. Previously, all trusted certificates were in the built-in
> > > trusted keying, which could not be modified from user space.
> > > Trusted certificates were placed in the built-in trusted keying at
> > > kernel compile time.
> > >
> > > The secondary trusted keying is designed so that any certificates that
> > > are signed by one of the trusted CA certificates in the built-in or
> > > secondary trusted keyring can be added to it.
> > >
> > > Let's imagine that we have the following certificate trust chain:
> > >
> > >              ┌───────────────────────────┬─────────────────────┐
> > >              │                           │     ┌───────┐       │
> > >              │                           │     │       │       │
> > > ┌────────────▼────────┐    ┌─────────────▼─────▼────┐  │ ┌─────┴─────┐
> > > │.builtin_trusted_keys│◄───┤.secondary_trusted_keys ├──┘ │   .ima    │
> > > ├─────────────────────┤    ├────────────────────────┤    ├───────────┤
> > > │     Root CA Cert    │-----► Intermediate CA Cert  │-----► IMA Cert │
> > > └─────────────────────┘    └────────────────────────┘    └───────────┘
> > >
> > >                 Issues                  Restricted by
> > >             -------------►             ──────────────►
> > >
> > > Since the IMA certificate is signed by a CA certificate from a secondary
> > > trusted keying, an attacker with superuser privileges will be able to
> > > add the IMA certificate to the secondary trusted keying. That is, the IMA
> > > certificate will become trusted.
> > >
> > > Since, with `CONFIG_MODULE_SIG` option enabled, modules can only be
> > > loaded into kernel space if they are signed with one of the trusted
> > > certificates, an attacker could sign untrusted kernel modules with
> > > the private key corresponding to the IMA certificate and successfully
> > > load the untrusted modules into kernel space.
> > >
> > > This patch was created not to solve only the problem of loading
> > > untrusted kernel modules, but to make it possible to use a secondary
> > > trusted keying only as a part of a chain of trust containing only
> > > CA certificates with no digital signature capability. This will
> > > help avoid similar problems when new features appear in the linux
> > > kernel that are similar to kernel modules in terms of their impact
> > > on system security, which will also use trusted certificates for
> > > signature verification.
> > >
> > > This patch adds the configuration that once enabled, only
> > > certificates that meet the following requirements can be added
> > > to the secondary trusted keying:
> > >
> > > 1. The certificate is a CA (Certificate Authority)
> > > 2. The certificate must be used for verifying a CA's signatures
> > > 3. The certificate must not be used for digital signatures
> > >
> > > Signed-off-by: Denis Glazkov <d.glazkov@xxxxxx>
> > > ---
> > > v1 -> v2:
> > >  - Rebase the patch from `linux-next` to the main `linux` repo master branch
> > >  - Make the commit message more detailed
> > >  - Move the variable declaration to the `if` block
> > >  - Replace `#ifdef` with `IS_ENABLED` macro
> > >
> > > v2 -> v3:
> > >  - Add the purpose and goal of the patch to the commit message
> > > ---
> > >  certs/Kconfig          |  9 +++++++++
> > >  certs/system_keyring.c | 16 ++++++++++++++++
> > >  2 files changed, 25 insertions(+)
> > >
> > > diff --git a/certs/Kconfig b/certs/Kconfig
> > > index 1f109b070877..4a4dc8aab892 100644
> > > --- a/certs/Kconfig
> > > +++ b/certs/Kconfig
> > > @@ -90,6 +90,15 @@ config SECONDARY_TRUSTED_KEYRING
> > >  	  those keys are not blacklisted and are vouched for by a key built
> > >  	  into the kernel or already in the secondary trusted keyring.
> > >  
> > > +config SECONDARY_TRUSTED_KEYRING_FOR_CA_CERTIFICATES_ONLY
> > > +	bool "Allow only CA certificates to be added to the secondary trusted keyring"
> > > +	depends on SECONDARY_TRUSTED_KEYRING
> > > +	help
> > > +	  If set, only CA certificates can be added to the secondary trusted keyring.
> > > +	  An acceptable CA certificate must include the `keyCertSign` value in
> > > +	  the `keyUsage` field. CA certificates that include the `digitalSignature`
> > > +	  value in the `keyUsage` field will not be accepted.
> > > +
> > >  config SYSTEM_BLACKLIST_KEYRING
> > >  	bool "Provide system-wide ring of blacklisted keys"
> > >  	depends on KEYS
> > > diff --git a/certs/system_keyring.c b/certs/system_keyring.c
> > > index 9de610bf1f4b..ee14447374e7 100644
> > > --- a/certs/system_keyring.c
> > > +++ b/certs/system_keyring.c
> > > @@ -99,6 +99,22 @@ int restrict_link_by_builtin_and_secondary_trusted(
> > >  		/* Allow the builtin keyring to be added to the secondary */
> > >  		return 0;
> > >  
> > > +	if (IS_ENABLED(CONFIG_SECONDARY_TRUSTED_KEYRING_FOR_CA_CERTIFICATES_ONLY) &&
> > > +	    dest_keyring == secondary_trusted_keys) {
> > > +		const struct public_key *pub = payload->data[asym_crypto];
> > > +
> > > +		if (type != &key_type_asymmetric)
> > > +			return -EOPNOTSUPP;
> > > +		if (!pub)
> > > +			return -ENOPKG;
> > > +		if (!test_bit(KEY_EFLAG_CA, &pub->key_eflags))
> > > +			return -EPERM;
> > > +		if (!test_bit(KEY_EFLAG_KEYCERTSIGN, &pub->key_eflags))
> > > +			return -EPERM;
> > > +		if (test_bit(KEY_EFLAG_DIGITALSIG, &pub->key_eflags))
> > > +			return -EPERM;
> > > +	}
> > > +
> > >  	return restrict_link_by_signature(dest_keyring, type, payload,
> > >  					  secondary_trusted_keys);
> > >  }
> > > -- 
> > > 2.34.1
> > 
> > I don't think this does any harm. What do you think Mimi?
> 
> I really like the idea of only allowing CA keys to be loaded onto the
> secondary trusted keyring.  However, the secondary trusted keyring has
> been around a long time with the ability of loading non CA keys.  Is
> the real concern here about loading non CA keys signed by keys on the
> builtin keyring or the new machine keyring?
> 
> It would make sense for the new Kconfig to somehow require
> INTEGRITY_CA_MACHINE_KEYRING_MAX, if INTEGRITY_MACHINE_KEYRING is
> configured.

This patch allows CA certificates signed by any key either linked to or
on the secondary keyring to be loaded onto the secondary keyring.

I just posted  "[RFC PATCH] certs: Only allow certs signed by keys on
the builtin keyring" as an alternative.  It only allows loading
certificates onto the secondary keyring signed by a key on the builtin
keyring.

-- 
thanks,

Mimi