Re: [PATCH v7 5/5] certs: Allow root user to append signed hashes to the blacklist keyring

From: Mickaël Salaün
Date: Wed Mar 17 2021 - 11:49:21 EST

On 17/03/2021 15:48, Eric Snowberg wrote:
>> On Mar 15, 2021, at 12:01 PM, Mickaël Salaün <mic@xxxxxxxxxxx> wrote:
>> On 15/03/2021 17:59, Eric Snowberg wrote:
>>>> On Mar 12, 2021, at 10:12 AM, Mickaël Salaün <mic@xxxxxxxxxxx> wrote:
>>>> From: Mickaël Salaün <mic@xxxxxxxxxxxxxxxxxxx>
>>>> Add a kernel option SYSTEM_BLACKLIST_AUTH_UPDATE to enable the root user
>>>> to dynamically add new keys to the blacklist keyring. This enables to
>>>> invalidate new certificates, either from being loaded in a keyring, or
>>>> from being trusted in a PKCS#7 certificate chain. This also enables to
>>>> add new file hashes to be denied by the integrity infrastructure.
>>>> Being able to untrust a certificate which could have normaly been
>>>> trusted is a sensitive operation. This is why adding new hashes to the
>>>> blacklist keyring is only allowed when these hashes are signed and
>>>> vouched by the builtin trusted keyring. A blacklist hash is stored as a
>>>> key description. The PKCS#7 signature of this description must be
>>>> provided as the key payload.
>>>> Marking a certificate as untrusted should be enforced while the system
>>>> is running. It is then forbiden to remove such blacklist keys.
>>>> Update blacklist keyring, blacklist key and revoked certificate access rights:
>>>> * allows the root user to search for a specific blacklisted hash, which
>>>> make sense because the descriptions are already viewable;
>>>> * forbids key update (blacklist and asymmetric ones);
>>>> * restricts kernel rights on the blacklist keyring to align with the
>>>> root user rights.
>>>> See help in tools/certs/ .
>>>> Cc: David Howells <dhowells@xxxxxxxxxx>
>>>> Cc: David Woodhouse <dwmw2@xxxxxxxxxxxxx>
>>>> Cc: Eric Snowberg <eric.snowberg@xxxxxxxxxx>
>>>> Cc: Jarkko Sakkinen <jarkko@xxxxxxxxxx>
>>>> Signed-off-by: Mickaël Salaün <mic@xxxxxxxxxxxxxxxxxxx>
>>>> Link:
>>> I tried testing this, it doesn’t work as I would expect.
>>> Here is my test setup:
>>> Kernel built with two keys compiled into the builtin_trusted_keys keyring
>>> Generated a tbs cert from one of the keys and signed it with the other key
>>> As root, added the tbs cert hash to the blacklist keyring
>>> Verified the tbs hash is in the blacklist keyring
>>> Enabled lockdown to enforce kernel module signature checking
>>> Signed a kernel module with the key I just blacklisted
>>> Load the kernel module
>>> I’m seeing the kernel module load, I would expect this to fail, since the
>>> key is now blacklisted. Or is this change just supposed to prevent new keys
>>> from being added in the future?
>> This is the expected behavior and the way the blacklist keyring is
>> currently used, as explained in the commit message:
>> "This enables to invalidate new certificates, either from being loaded
>> in a keyring, or from being trusted in a PKCS#7 certificate chain."
>> If you want a (trusted root) key to be untrusted, you need to remove it
>> from the keyring, which is not allowed for the builtin trusted keyring.
> Is there a non technical reason why this can not be changed to also apply to
> builtin trusted keys? If a user had the same tbs cert hash in their dbx and
> soon mokx, the hash would show up in the .blacklist keyring and invalidate
> any key in the builtin_trusted_keys keyring. After adding the same hash with
> this series, it shows up in the .blacklist_keyring but the value is ignored
> by operations using the builtin_trusted_keys keyring. It just seems
> incomplete to me, or did I miss an earlier discussion on this topic?

The purpose of the blacklist keyring is to block new keys from being
loaded in the kernel. For builtin and dbx/mokx hashes, they are loaded
in the blacklist keyring before builtin certificates are loaded in the
trusted builtin keyring, which can reject them if there is a match. I
think that being able to load a blocked hash at run time should not
change the semantic of the blacklist keyring, which is to block the
loading of certificates. If someone wants to change this semantic, I
think it should be configurable. Indeed, we should keep in mind the
temporal dimension and the hierarchy of trust: dbx/mokx -> builtin
hashes -> run time hashes.