[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <f5a1d856-0482-a2c3-0e62-3ca911ce3dd2@omp.ru>
Date: Wed, 6 Sep 2023 15:58:58 +0300
From: Sergey Shtylyov <s.shtylyov@....ru>
To: Denis Glazkov <d.glazkov@....ru>
CC: David Howells <dhowells@...hat.com>,
David Woodhouse <dwmw2@...radead.org>,
"keyrings@...r.kernel.org" <keyrings@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] certs: Add the ability to add only CA certificates to the
secondary trusted keyring
On 9/6/23 2:32 PM, Denis Glazkov wrote:
> When building a chain of trust for IMA certificates issued from
It's a shame I forgot what IMA stands for... and even Google doesn't
give any suitable value...
> intermediate certificates using a secondary trusted keying, there
> is no way to restrict the addition of IMA certificates to trusted
> certificates, since any certificate signed by an built-in or
> secondary trusted certificate can be added to the secondary
> trusted keying.
>
> With root privileges, an attacker can load a certificate intended
> for IMA into the trusted certificates and sign the kernel modules
> with the corresponding private key. This allows an attacker to
> load untrusted modules into kernel space.
>
> 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.
Oh, and I also forgot what CA stands for... :-/
> 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@....ru>
[...]
> diff --git a/certs/system_keyring.c b/certs/system_keyring.c
> index 9de610bf1f4b..8d45c19ba92e 100644
> --- a/certs/system_keyring.c
> +++ b/certs/system_keyring.c
> @@ -90,6 +90,10 @@ int restrict_link_by_builtin_and_secondary_trusted(
> const union key_payload *payload,
> struct key *restrict_key)
> {
> +#ifdef CONFIG_SECONDARY_TRUSTED_KEYRING_FOR_CA_CERTIFICATES_ONLY
> + struct public_key *pub;
> +#endif
Mhm, why this is not a part of the *if* block in the next hunk?
You don't use this variable outside that block...
[...]
> @@ -99,6 +103,23 @@ int restrict_link_by_builtin_and_secondary_trusted(
> /* Allow the builtin keyring to be added to the secondary */
> return 0;
>
> +#ifdef CONFIG_SECONDARY_TRUSTED_KEYRING_FOR_CA_CERTIFICATES_ONLY
> + if (dest_keyring == secondary_trusted_keys) {
> + if (type != &key_type_asymmetric)
> + return -EOPNOTSUPP;
> +
> + pub = payload->data[asym_crypto];
I'm not seeing this index declared in Linus' repo...
> + 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;
> + }
> +#endif
> +
> return restrict_link_by_signature(dest_keyring, type, payload,
> secondary_trusted_keys);
> }
MBR, Sergey
Powered by blists - more mailing lists