[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2163E0AB-77B5-46EA-9987-6C9CC4937E0B@oracle.com>
Date: Mon, 11 Mar 2024 21:29:59 +0000
From: Eric Snowberg <eric.snowberg@...cle.com>
To: Jarkko Sakkinen <jarkko@...nel.org>
CC: "open list:SECURITY SUBSYSTEM" <linux-security-module@...r.kernel.org>,
David Howells <dhowells@...hat.com>,
David Woodhouse <dwmw2@...radead.org>,
"herbert@...dor.apana.org.au" <herbert@...dor.apana.org.au>,
"davem@...emloft.net" <davem@...emloft.net>,
Ard Biesheuvel
<ardb@...nel.org>, Paul Moore <paul@...l-moore.com>,
James Morris
<jmorris@...ei.org>,
"Serge E. Hallyn" <serge@...lyn.com>,
Mimi Zohar
<zohar@...ux.ibm.com>,
Roberto Sassu <roberto.sassu@...wei.com>,
Dmitry
Kasatkin <dmitry.kasatkin@...il.com>,
Mickaël Salaün
<mic@...ikod.net>,
"casey@...aufler-ca.com" <casey@...aufler-ca.com>,
Stefan
Berger <stefanb@...ux.ibm.com>,
open list <linux-kernel@...r.kernel.org>,
"keyrings@...r.kernel.org" <keyrings@...r.kernel.org>,
"linux-crypto@...r.kernel.org" <linux-crypto@...r.kernel.org>,
"linux-efi@...r.kernel.org" <linux-efi@...r.kernel.org>,
"linux-integrity@...r.kernel.org" <linux-integrity@...r.kernel.org>
Subject: Re: [PATCH RFC 1/8] certs: Introduce ability to link to a system key
> On Mar 11, 2024, at 1:16 PM, Jarkko Sakkinen <jarkko@...nel.org> wrote:
>
> On Mon Mar 11, 2024 at 6:11 PM EET, Eric Snowberg wrote:
>> Introduce a new function to allow a keyring to link to a key contained
>> within one of the system keyrings (builtin, secondary, or platform).
>> Depending on how the kernel is built, if the machine keyring is
>> available, it will be checked as well, since it is linked to the secondary
>> keyring. If the asymmetric key id matches a key within one of these
>> system keyrings, the matching key is linked into the passed in
>> keyring.
>>
>> Signed-off-by: Eric Snowberg <eric.snowberg@...cle.com>
>> ---
>> certs/system_keyring.c | 29 +++++++++++++++++++++++++++++
>> include/keys/system_keyring.h | 7 ++++++-
>> 2 files changed, 35 insertions(+), 1 deletion(-)
>>
>> diff --git a/certs/system_keyring.c b/certs/system_keyring.c
>> index 9de610bf1f4b..b647be49f6e0 100644
>> --- a/certs/system_keyring.c
>> +++ b/certs/system_keyring.c
>> @@ -426,3 +426,32 @@ void __init set_platform_trusted_keys(struct key *keyring)
>> platform_trusted_keys = keyring;
>> }
>> #endif
>> +
>> +/**
>> + * system_key_link - Link to a system key
>> + * @keyring: The keyring to link into
>> + * @id: The asymmetric key id to look for in the system keyring
>> + */
>> +int system_key_link(struct key *keyring, struct asymmetric_key_id *id)
>> +{
>> + struct key *tkey;
>
> I'd suggest to replace this with just 'tkey'. Single obscure character
> does not bring any readability value.
I assume you mean replace it with "key"? If so, yes, I'll change that in the next round.
>> +
>> +#ifdef CONFIG_SECONDARY_TRUSTED_KEYRING
>> + tkey = find_asymmetric_key(secondary_trusted_keys, id, NULL, NULL, false);
>> +#else
>> + tkey = find_asymmetric_key(builtin_trusted_keys, id, NULL, NULL, false);
>> +#endif
>
> I'd have just single call site here and inside ifdef-shenanigans i'd
> just set helper "keyring" to point to the appropriate keyring.
I'll also make this change
>> + if (!IS_ERR(tkey))
>> + goto found;
>> +
>> + tkey = find_asymmetric_key(platform_trusted_keys, id, NULL, NULL, false);
>> +
>
> Please remove this empty line as the check below is associated with it.
and remove the empty line. Thanks.
Powered by blists - more mailing lists