lkml.org 
[lkml]   [2021]   [Sep]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v2] integrity: support including firmware ".platform" keys at build time
From
Date

On 9/16/21 10:46 AM, Jarkko Sakkinen wrote:
> On Thu, 2021-09-16 at 08:57 -0400, Nayna Jain wrote:
>> Some firmware support secureboot by embedding static keys to verify the
>> Linux kernel during boot. However, these firmware do not expose an
>> interface for the kernel to load firmware keys onto ".platform" keyring.
>> This would prevent kernel signature verification on kexec. For those
>> environments, allow firmware keys to be compiled into the kernel and
>> loaded onto the ".platform" keyring.
> "allow" means absolutely nothing. Just tell what your patch does,
> and approach taken. Already the patch description should roughly
> give idea what and why of code changes. There's nothing here.
>
>> Reported-by: kernel test robot <lkp@intel.com>
> I don't get this reported-by here.
>
>> Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
>> ---
>>
>> v2:
>> * Fixed the error reported by kernel test robot
>> * Updated patch description based on Jarkko's feedback.
>>
>> certs/Makefile | 3 ++-
>> certs/blacklist.c | 1 -
>> certs/common.c | 2 +-
>> certs/common.h | 9 -------
>> certs/system_keyring.c | 1 -
>> include/keys/system_keyring.h | 3 +++
>> security/integrity/Kconfig | 10 +++++++
>> security/integrity/Makefile | 17 +++++++++++-
>> security/integrity/digsig.c | 2 +-
>> security/integrity/integrity.h | 6 +++++
>> .../integrity/platform_certs/platform_cert.S | 23 ++++++++++++++++
>> .../platform_certs/platform_keyring.c | 26 +++++++++++++++++++
>> 12 files changed, 88 insertions(+), 15 deletions(-)
>> delete mode 100644 certs/common.h
>> create mode 100644 security/integrity/platform_certs/platform_cert.S
>>
>> diff --git a/certs/Makefile b/certs/Makefile
>> index 279433783b10..64ee37f38b85 100644
>> --- a/certs/Makefile
>> +++ b/certs/Makefile
>> @@ -3,7 +3,8 @@
>> # Makefile for the linux kernel signature checking certificates.
>> #
>>
>> -obj-$(CONFIG_SYSTEM_TRUSTED_KEYRING) += system_keyring.o system_certificates.o common.o
>> +obj-$(CONFIG_KEYS) += common.o
>> +obj-$(CONFIG_SYSTEM_TRUSTED_KEYRING) += system_keyring.o system_certificates.o
>> obj-$(CONFIG_SYSTEM_BLACKLIST_KEYRING) += blacklist.o common.o
>> obj-$(CONFIG_SYSTEM_REVOCATION_LIST) += revocation_certificates.o
>> ifneq ($(CONFIG_SYSTEM_BLACKLIST_HASH_LIST),"")
>> diff --git a/certs/blacklist.c b/certs/blacklist.c
>> index c9a435b15af4..b95e9b19c42f 100644
>> --- a/certs/blacklist.c
>> +++ b/certs/blacklist.c
>> @@ -17,7 +17,6 @@
>> #include <linux/uidgid.h>
>> #include <keys/system_keyring.h>
>> #include "blacklist.h"
>> -#include "common.h"
>>
>> static struct key *blacklist_keyring;
>>
>> diff --git a/certs/common.c b/certs/common.c
>> index 16a220887a53..41f763415a00 100644
>> --- a/certs/common.c
>> +++ b/certs/common.c
>> @@ -2,7 +2,7 @@
>>
>> #include <linux/kernel.h>
>> #include <linux/key.h>
>> -#include "common.h"
> Why this include is removed?
>
> You should include to your commit message *also* the approach
> you are taking. If you export a function, you should mention
> it explicitly.

Thanks Jarrko for the review.

Do you think it would be better to split this patch into two ?

Patch 1: Export load_certificate_list() to be called outside certs/

Patch 2: Add and load compiled-in certificates in ".platform" keyring.

Thanks & Regards,

    - Nayna


\
 
 \ /
  Last update: 2021-09-18 21:38    [W:0.512 / U:0.020 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site