by Mimi Zohar

[permalink] [raw]

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

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 <[email protected]>
> > > ---
> > > 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