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
Make integrity_keyring_from_id() non-static so that it is accessible
by other files in security/integrity.
Reviewed-by: Mimi Zohar <[email protected]>
Signed-off-by: Nayna Jain <[email protected]>
---
security/integrity/digsig.c | 2 +-
security/integrity/integrity.h | 6 ++++++
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index c8c8a4a4e7a0..9c3165c07935 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -39,7 +39,7 @@ static const char * const keyring_name[INTEGRITY_KEYRING_MAX] = {
#define restrict_link_to_ima restrict_link_by_builtin_trusted
#endif
-static struct key *integrity_keyring_from_id(const unsigned int id)
+struct key *integrity_keyring_from_id(const unsigned int id)
{
if (id >= INTEGRITY_KEYRING_MAX)
return ERR_PTR(-EINVAL);
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 2e214c761158..76e9a9515f99 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -168,6 +168,7 @@ int __init integrity_init_keyring(const unsigned int id);
int __init integrity_load_x509(const unsigned int id, const char *path);
int __init integrity_load_cert(const unsigned int id, const char *source,
const void *data, size_t len, key_perm_t perm);
+struct key *integrity_keyring_from_id(const unsigned int id);
#else
static inline int integrity_digsig_verify(const unsigned int id,
@@ -195,6 +196,11 @@ static inline int __init integrity_load_cert(const unsigned int id,
{
return 0;
}
+
+static inline struct key *integrity_keyring_from_id(const unsigned int id)
+{
+ return ERR_PTR(-EOPNOTSUPP);
+}
#endif /* CONFIG_INTEGRITY_SIGNATURE */
#ifdef CONFIG_INTEGRITY_ASYMMETRIC_KEYS
--
2.27.0
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