> On Mar 11, 2024, at 8:45 PM, Randy Dunlap <rdunlap@xxxxxxxxxxxxx> wrote: > > On 3/11/24 09:11, Eric Snowberg wrote: >> In the future it is envisioned this LSM could be enhanced to provide >> access control for UEFI Secure Boot Advanced Targeting (SBAT). Using >> the same clavis= boot param and storing the additional contents within >> the new RT UEFI var, SBAT restrictions could be maintained across kexec. > > What does "RT" mean here? I will define it in the next round. It stands for Run-Time. >> + >> +Clavis is a Linux Security Module that provides mandatory access control to >> +system kernel keys (i.e. builtin, secondary, machine and platform). These >> +restrictions will prohit keys from being used for validation. Upon boot, the > > prohibit I will fix this spelling error and all the others you identified below. > >> +Clavis LSM is provided a key id as a boot param. This single key is then >> +used as the root of trust for any access control modifications made going >> +forward. Access control updates must be signed and validated by this key. >> + >> +Clavis has its own keyring. All ACL updates are applied through this keyring. >> +The update must be signed by the single root of trust key. >> + >> +When enabled, all system keys are prohibited from being used until an ACL is >> +added for it. There is two exceptions to this rule, builtin keys may be used > > What is "it"? The predecessor seems to be "all system keys" (plural). The word "for" should be "to" above. The sentence should be: When enabled, all system keys are prohibited from being used until an ACL is added to it. >> + >> +The Clavis LSM contains a system keyring call .clavis. It contains a single >> +asymmetric key that is use to validate anything added to it. This key can only >> +be added during boot and must be a preexisting system kernel key. If the >> +clavis= boot param is not used, the keyring does not exist and the feature >> +can not be used until the next power on reset. > > So just a reboot won't cause it to be used? Must be power off/on? A reboot would too. I will rework this sentence. I just want to show that doing a kexec can not be used to make changes to the root of trust. >> diff --git a/security/security.c b/security/security.c >> index 4cb832b00c40..d1da60a1b7a4 100644 >> --- a/security/security.c >> +++ b/security/security.c > >> @@ -5313,6 +5314,19 @@ void security_key_post_create_or_update(struct key *keyring, struct key *key, >> call_void_hook(key_post_create_or_update, keyring, key, payload, >> payload_len, flags, create); >> } >> + >> +/** >> + * security_key_verify_signature - verify signature >> + * @key: key >> + * @public_key_signature: signature > > Above should be "@sig:". I will fix that too. Thanks for your review.