| Message ID | 20220304175403.20092-1-nayna@linux.ibm.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | integrity: support including firmware ".platform" keys at build time | expand |
On Fri, 2022-03-04 at 12:54 -0500, Nayna Jain wrote: > Some firmware support secure boot 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 the ".platform" > keyring, preventing the kernel from verifying the kexec kernel image > signature. > > This patchset exports load_certificate_list() and defines a new function > load_builtin_platform_cert() to load compiled in certificates onto the > ".platform" keyring. > > Changelog: > v9: > * Rebased on tpmdd master branch repo - > git://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git > > v8: > * Includes Jarkko's feedback on patch description and removed Reported-by > for Patch 1. > > v7: > * Incldues Jarkko's feedback on patch description for Patch 1 and 3. > > v6: > * Includes Jarkko's feedback: > * Split Patch 2 into two. > * Update Patch description. > > v5: > * Renamed load_builtin_platform_cert() to load_platform_certificate_list() > and config INTEGRITY_PLATFORM_BUILTIN_KEYS to INTEGRITY_PLATFORM_KEYS, as > suggested by Mimi Zohar. > > v4: > * Split into two patches as per Mimi Zohar and Dimitri John Ledkov > recommendation. > > v3: > * Included Jarkko's feedback > ** updated patch description to include approach. > ** removed extern for function declaration in the .h file. > * Included load_certificate_list() within #ifdef CONFIG_KEYS condition. > > v2: > * Fixed the error reported by kernel test robot > * Updated patch description based on Jarkko's feedback. > > Nayna Jain (3): > certs: export load_certificate_list() to be used outside certs/ > integrity: make integrity_keyring_from_id() non-static > integrity: support including firmware ".platform" keys at build time > > certs/Makefile | 5 ++-- > certs/blacklist.c | 1 - > certs/common.c | 2 +- > certs/common.h | 9 ------- > certs/system_keyring.c | 1 - > include/keys/system_keyring.h | 6 +++++ > security/integrity/Kconfig | 10 +++++++ > security/integrity/Makefile | 15 ++++++++++- > 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, 90 insertions(+), 16 deletions(-) > delete mode 100644 certs/common.h > create mode 100644 security/integrity/platform_certs/platform_cert.S > > > base-commit: c9e54f38976a1c0ec69c0a6208b3fd55fceb01d1 Thanks for the trouble! I'll pick these. BR, Jarkko
On Fri, Mar 04, 2022 at 12:54:00PM -0500, Nayna Jain wrote: > Some firmware support secure boot 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 the ".platform" > keyring, preventing the kernel from verifying the kexec kernel image > signature. > > This patchset exports load_certificate_list() and defines a new function > load_builtin_platform_cert() to load compiled in certificates onto the > ".platform" keyring. > > Changelog: > v9: > * Rebased on tpmdd master branch repo - > git://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git > > v8: > * Includes Jarkko's feedback on patch description and removed Reported-by > for Patch 1. > > v7: > * Incldues Jarkko's feedback on patch description for Patch 1 and 3. > > v6: > * Includes Jarkko's feedback: > * Split Patch 2 into two. > * Update Patch description. > > v5: > * Renamed load_builtin_platform_cert() to load_platform_certificate_list() > and config INTEGRITY_PLATFORM_BUILTIN_KEYS to INTEGRITY_PLATFORM_KEYS, as > suggested by Mimi Zohar. > > v4: > * Split into two patches as per Mimi Zohar and Dimitri John Ledkov > recommendation. > > v3: > * Included Jarkko's feedback > ** updated patch description to include approach. > ** removed extern for function declaration in the .h file. > * Included load_certificate_list() within #ifdef CONFIG_KEYS condition. > > v2: > * Fixed the error reported by kernel test robot > * Updated patch description based on Jarkko's feedback. > > Nayna Jain (3): > certs: export load_certificate_list() to be used outside certs/ > integrity: make integrity_keyring_from_id() non-static > integrity: support including firmware ".platform" keys at build time > > certs/Makefile | 5 ++-- > certs/blacklist.c | 1 - > certs/common.c | 2 +- > certs/common.h | 9 ------- > certs/system_keyring.c | 1 - > include/keys/system_keyring.h | 6 +++++ > security/integrity/Kconfig | 10 +++++++ > security/integrity/Makefile | 15 ++++++++++- > 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, 90 insertions(+), 16 deletions(-) > delete mode 100644 certs/common.h > create mode 100644 security/integrity/platform_certs/platform_cert.S > > > base-commit: c9e54f38976a1c0ec69c0a6208b3fd55fceb01d1 > -- > 2.27.0 Thank you, applied. BR, Jarkko
On Sat, Mar 05, 2022 at 07:37:18AM +0200, Jarkko Sakkinen wrote: > On Fri, Mar 04, 2022 at 12:54:00PM -0500, Nayna Jain wrote: > > Some firmware support secure boot 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 the ".platform" > > keyring, preventing the kernel from verifying the kexec kernel image > > signature. > > > > This patchset exports load_certificate_list() and defines a new function > > load_builtin_platform_cert() to load compiled in certificates onto the > > ".platform" keyring. > > > > Changelog: > > v9: > > * Rebased on tpmdd master branch repo - > > git://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git > > > > v8: > > * Includes Jarkko's feedback on patch description and removed Reported-by > > for Patch 1. > > > > v7: > > * Incldues Jarkko's feedback on patch description for Patch 1 and 3. > > > > v6: > > * Includes Jarkko's feedback: > > * Split Patch 2 into two. > > * Update Patch description. > > > > v5: > > * Renamed load_builtin_platform_cert() to load_platform_certificate_list() > > and config INTEGRITY_PLATFORM_BUILTIN_KEYS to INTEGRITY_PLATFORM_KEYS, as > > suggested by Mimi Zohar. > > > > v4: > > * Split into two patches as per Mimi Zohar and Dimitri John Ledkov > > recommendation. > > > > v3: > > * Included Jarkko's feedback > > ** updated patch description to include approach. > > ** removed extern for function declaration in the .h file. > > * Included load_certificate_list() within #ifdef CONFIG_KEYS condition. > > > > v2: > > * Fixed the error reported by kernel test robot > > * Updated patch description based on Jarkko's feedback. > > > > Nayna Jain (3): > > certs: export load_certificate_list() to be used outside certs/ > > integrity: make integrity_keyring_from_id() non-static > > integrity: support including firmware ".platform" keys at build time > > > > certs/Makefile | 5 ++-- > > certs/blacklist.c | 1 - > > certs/common.c | 2 +- > > certs/common.h | 9 ------- > > certs/system_keyring.c | 1 - > > include/keys/system_keyring.h | 6 +++++ > > security/integrity/Kconfig | 10 +++++++ > > security/integrity/Makefile | 15 ++++++++++- > > 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, 90 insertions(+), 16 deletions(-) > > delete mode 100644 certs/common.h > > create mode 100644 security/integrity/platform_certs/platform_cert.S > > > > > > base-commit: c9e54f38976a1c0ec69c0a6208b3fd55fceb01d1 > > -- > > 2.27.0 > > Thank you, applied. > > BR, Jarkko You need to fix this: WARNING: externs should be avoided in .c files #129: FILE: security/integrity/platform_certs/platform_keyring.c:19: +extern __initconst const unsigned long platform_certificate_list_size; I.e. remove extern's from font. Please send one more version. BR, Jarkko
On 3/5/22 00:49, Jarkko Sakkinen wrote: > On Sat, Mar 05, 2022 at 07:37:18AM +0200, Jarkko Sakkinen wrote: >> On Fri, Mar 04, 2022 at 12:54:00PM -0500, Nayna Jain wrote: >>> Some firmware support secure boot 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 the ".platform" >>> keyring, preventing the kernel from verifying the kexec kernel image >>> signature. >>> >>> This patchset exports load_certificate_list() and defines a new function >>> load_builtin_platform_cert() to load compiled in certificates onto the >>> ".platform" keyring. >>> >>> Changelog: >>> v9: >>> * Rebased on tpmdd master branch repo - >>> git://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git >>> >>> v8: >>> * Includes Jarkko's feedback on patch description and removed Reported-by >>> for Patch 1. >>> >>> v7: >>> * Incldues Jarkko's feedback on patch description for Patch 1 and 3. >>> >>> v6: >>> * Includes Jarkko's feedback: >>> * Split Patch 2 into two. >>> * Update Patch description. >>> >>> v5: >>> * Renamed load_builtin_platform_cert() to load_platform_certificate_list() >>> and config INTEGRITY_PLATFORM_BUILTIN_KEYS to INTEGRITY_PLATFORM_KEYS, as >>> suggested by Mimi Zohar. >>> >>> v4: >>> * Split into two patches as per Mimi Zohar and Dimitri John Ledkov >>> recommendation. >>> >>> v3: >>> * Included Jarkko's feedback >>> ** updated patch description to include approach. >>> ** removed extern for function declaration in the .h file. >>> * Included load_certificate_list() within #ifdef CONFIG_KEYS condition. >>> >>> v2: >>> * Fixed the error reported by kernel test robot >>> * Updated patch description based on Jarkko's feedback. >>> >>> Nayna Jain (3): >>> certs: export load_certificate_list() to be used outside certs/ >>> integrity: make integrity_keyring_from_id() non-static >>> integrity: support including firmware ".platform" keys at build time >>> >>> certs/Makefile | 5 ++-- >>> certs/blacklist.c | 1 - >>> certs/common.c | 2 +- >>> certs/common.h | 9 ------- >>> certs/system_keyring.c | 1 - >>> include/keys/system_keyring.h | 6 +++++ >>> security/integrity/Kconfig | 10 +++++++ >>> security/integrity/Makefile | 15 ++++++++++- >>> 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, 90 insertions(+), 16 deletions(-) >>> delete mode 100644 certs/common.h >>> create mode 100644 security/integrity/platform_certs/platform_cert.S >>> >>> >>> base-commit: c9e54f38976a1c0ec69c0a6208b3fd55fceb01d1 >>> -- >>> 2.27.0 >> Thank you, applied. >> >> BR, Jarkko > You need to fix this: > > WARNING: externs should be avoided in .c files > #129: FILE: security/integrity/platform_certs/platform_keyring.c:19: > +extern __initconst const unsigned long platform_certificate_list_size; Yes, because I followed the same convention as used for system keyring certs. Following externs are defined in system_keyring.c for referencing variables defined in .S file. extern __initconst const u8 system_certificate_list[]; extern __initconst const unsigned long system_certificate_list_size; extern __initconst const unsigned long module_cert_size; Thanks & Regards, - Nayna
On Sat, Mar 05, 2022 at 08:32:43PM -0500, Nayna wrote: > > On 3/5/22 00:49, Jarkko Sakkinen wrote: > > On Sat, Mar 05, 2022 at 07:37:18AM +0200, Jarkko Sakkinen wrote: > > > On Fri, Mar 04, 2022 at 12:54:00PM -0500, Nayna Jain wrote: > > > > Some firmware support secure boot 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 the ".platform" > > > > keyring, preventing the kernel from verifying the kexec kernel image > > > > signature. > > > > > > > > This patchset exports load_certificate_list() and defines a new function > > > > load_builtin_platform_cert() to load compiled in certificates onto the > > > > ".platform" keyring. > > > > > > > > Changelog: > > > > v9: > > > > * Rebased on tpmdd master branch repo - > > > > git://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git > > > > > > > > v8: > > > > * Includes Jarkko's feedback on patch description and removed Reported-by > > > > for Patch 1. > > > > > > > > v7: > > > > * Incldues Jarkko's feedback on patch description for Patch 1 and 3. > > > > > > > > v6: > > > > * Includes Jarkko's feedback: > > > > * Split Patch 2 into two. > > > > * Update Patch description. > > > > > > > > v5: > > > > * Renamed load_builtin_platform_cert() to load_platform_certificate_list() > > > > and config INTEGRITY_PLATFORM_BUILTIN_KEYS to INTEGRITY_PLATFORM_KEYS, as > > > > suggested by Mimi Zohar. > > > > > > > > v4: > > > > * Split into two patches as per Mimi Zohar and Dimitri John Ledkov > > > > recommendation. > > > > > > > > v3: > > > > * Included Jarkko's feedback > > > > ** updated patch description to include approach. > > > > ** removed extern for function declaration in the .h file. > > > > * Included load_certificate_list() within #ifdef CONFIG_KEYS condition. > > > > > > > > v2: > > > > * Fixed the error reported by kernel test robot > > > > * Updated patch description based on Jarkko's feedback. > > > > > > > > Nayna Jain (3): > > > > certs: export load_certificate_list() to be used outside certs/ > > > > integrity: make integrity_keyring_from_id() non-static > > > > integrity: support including firmware ".platform" keys at build time > > > > > > > > certs/Makefile | 5 ++-- > > > > certs/blacklist.c | 1 - > > > > certs/common.c | 2 +- > > > > certs/common.h | 9 ------- > > > > certs/system_keyring.c | 1 - > > > > include/keys/system_keyring.h | 6 +++++ > > > > security/integrity/Kconfig | 10 +++++++ > > > > security/integrity/Makefile | 15 ++++++++++- > > > > 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, 90 insertions(+), 16 deletions(-) > > > > delete mode 100644 certs/common.h > > > > create mode 100644 security/integrity/platform_certs/platform_cert.S > > > > > > > > > > > > base-commit: c9e54f38976a1c0ec69c0a6208b3fd55fceb01d1 > > > > -- > > > > 2.27.0 > > > Thank you, applied. > > > > > > BR, Jarkko > > You need to fix this: > > > > WARNING: externs should be avoided in .c files > > #129: FILE: security/integrity/platform_certs/platform_keyring.c:19: > > +extern __initconst const unsigned long platform_certificate_list_size; > > Yes, because I followed the same convention as used for system keyring > certs. Following externs are defined in system_keyring.c for referencing > variables defined in .S file. > > extern __initconst const u8 system_certificate_list[]; > extern __initconst const unsigned long system_certificate_list_size; > extern __initconst const unsigned long module_cert_size; So why don't you do as the warning says and place them to integrity.h for instance? BR, Jarkko