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/print-cert-tbs-hash.sh . >>>> >>>> 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: https://lore.kernel.org/r/20210312171232.2681989-6-mic@xxxxxxxxxxx >>> >>> 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.
