| Message ID | 20190116101654.7288-1-kasong@redhat.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | let kexec_file_load use platform keyring to verify the kernel image | expand |
On Wed, 2019-01-16 at 18:16 +0800, Kairui Song wrote: > This patch series adds a .platform_trusted_keys in system_keyring as the > reference to .platform keyring in integrity subsystem, when platform > keyring is being initialized it will be updated. So other component could > use this keyring as well. Remove "other component could use ...". > > This patch series also let kexec_file_load use platform keyring as fall > back if it failed to verify the image against secondary keyring, make it > possible to load kernel signed by third part key if third party key is > imported in the firmware. This is the only reason for these patches. Please remove "also". > > After this patch kexec_file_load will be able to verify a signed PE > bzImage using keys in platform keyring. > > Tested in a VM with locally signed kernel with pesign and imported the > cert to EFI's MokList variable. It's taken so long for me to review/test this patch set due to a regression in sanity_check_segment_list(), introduced somewhere between 4.20 and 5.0.0-rc1. The sgement overlap test - "if ((mend > pstart) && (mstart < pend))" - fails, returning a -EINVAL. Is anyone else seeing this? Mimi > > Kairui Song (2): > integrity, KEYS: add a reference to platform keyring > kexec, KEYS: Make use of platform keyring for signature verify > > Update from V2: > - Use IS_ENABLED in kexec_file_load to judge if platform_trusted_keys > should be used for verifying image as suggested by Mimi Zohar > > Update from V1: > - Make platform_trusted_keys static, and update commit message as suggested > by Mimi Zohar > - Always check if platform keyring is initialized before use it > > Kairui Song (2): > integrity, KEYS: add a reference to platform keyring > kexec, KEYS: Make use of platform keyring for signature verify > > arch/x86/kernel/kexec-bzimage64.c | 13 ++++++++++--- > certs/system_keyring.c | 22 +++++++++++++++++++++- > include/keys/system_keyring.h | 5 +++++ > include/linux/verification.h | 1 + > security/integrity/digsig.c | 6 ++++++ > 5 files changed, 43 insertions(+), 4 deletions(-) >
On 01/17/19 at 08:08pm, Mimi Zohar wrote: > On Wed, 2019-01-16 at 18:16 +0800, Kairui Song wrote: > > This patch series adds a .platform_trusted_keys in system_keyring as the > > reference to .platform keyring in integrity subsystem, when platform > > keyring is being initialized it will be updated. So other component could > > use this keyring as well. > > Remove "other component could use ...". > > > > This patch series also let kexec_file_load use platform keyring as fall > > back if it failed to verify the image against secondary keyring, make it > > possible to load kernel signed by third part key if third party key is > > imported in the firmware. > > This is the only reason for these patches. Please remove "also". > > > > > After this patch kexec_file_load will be able to verify a signed PE > > bzImage using keys in platform keyring. > > > > Tested in a VM with locally signed kernel with pesign and imported the > > cert to EFI's MokList variable. > > It's taken so long for me to review/test this patch set due to a > regression in sanity_check_segment_list(), introduced somewhere > between 4.20 and 5.0.0-rc1. The sgement overlap test - "if ((mend > > pstart) && (mstart < pend))" - fails, returning a -EINVAL. > > Is anyone else seeing this? Mimi, should be this issue? I have sent a fix for that. https://lore.kernel.org/lkml/20181228011247.GA9999@dhcp-128-65.nay.redhat.com/ Thanks Dave
On Fri, 2019-01-18 at 09:35 +0800, Dave Young wrote: > On 01/17/19 at 08:08pm, Mimi Zohar wrote: > > It's taken so long for me to review/test this patch set due to a > > regression in sanity_check_segment_list(), introduced somewhere > > between 4.20 and 5.0.0-rc1. The sgement overlap test - "if ((mend > > > pstart) && (mstart < pend))" - fails, returning a -EINVAL. > > > > Is anyone else seeing this? > > Mimi, should be this issue? I have sent a fix for that. > https://lore.kernel.org/lkml/20181228011247.GA9999@dhcp-128-65.nay.redhat.com/ Thanks, that resolved the regression. Mimi
On 01/18/19 at 09:35am, Dave Young wrote: > On 01/17/19 at 08:08pm, Mimi Zohar wrote: > > On Wed, 2019-01-16 at 18:16 +0800, Kairui Song wrote: > > > This patch series adds a .platform_trusted_keys in system_keyring as the > > > reference to .platform keyring in integrity subsystem, when platform > > > keyring is being initialized it will be updated. So other component could > > > use this keyring as well. > > > > Remove "other component could use ...". > > > > > > This patch series also let kexec_file_load use platform keyring as fall > > > back if it failed to verify the image against secondary keyring, make it > > > possible to load kernel signed by third part key if third party key is > > > imported in the firmware. > > > > This is the only reason for these patches. Please remove "also". > > > > > > > > After this patch kexec_file_load will be able to verify a signed PE > > > bzImage using keys in platform keyring. > > > > > > Tested in a VM with locally signed kernel with pesign and imported the > > > cert to EFI's MokList variable. > > > > It's taken so long for me to review/test this patch set due to a > > regression in sanity_check_segment_list(), introduced somewhere > > between 4.20 and 5.0.0-rc1. The sgement overlap test - "if ((mend > > > pstart) && (mstart < pend))" - fails, returning a -EINVAL. > > > > Is anyone else seeing this? > > Mimi, should be this issue? I have sent a fix for that. > https://lore.kernel.org/lkml/20181228011247.GA9999@dhcp-128-65.nay.redhat.com/ Hi, Kairui, I think you should know this while working on this series, It is good to mention the test dependency in cover letter so that reviewers can save time. BTW, Boris took it in tip already: https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?id=993a110319a4a60aadbd02f6defdebe048f7773b > > Thanks > Dave
On Fri, Jan 18, 2019 at 10:00 AM Dave Young <dyoung@redhat.com> wrote: > > On 01/18/19 at 09:35am, Dave Young wrote: > > On 01/17/19 at 08:08pm, Mimi Zohar wrote: > > > On Wed, 2019-01-16 at 18:16 +0800, Kairui Song wrote: > > > > This patch series adds a .platform_trusted_keys in system_keyring as the > > > > reference to .platform keyring in integrity subsystem, when platform > > > > keyring is being initialized it will be updated. So other component could > > > > use this keyring as well. > > > > > > Remove "other component could use ...". > > > > > > > > This patch series also let kexec_file_load use platform keyring as fall > > > > back if it failed to verify the image against secondary keyring, make it > > > > possible to load kernel signed by third part key if third party key is > > > > imported in the firmware. > > > > > > This is the only reason for these patches. Please remove "also". > > > > > > > > > > > After this patch kexec_file_load will be able to verify a signed PE > > > > bzImage using keys in platform keyring. > > > > > > > > Tested in a VM with locally signed kernel with pesign and imported the > > > > cert to EFI's MokList variable. > > > > > > It's taken so long for me to review/test this patch set due to a > > > regression in sanity_check_segment_list(), introduced somewhere > > > between 4.20 and 5.0.0-rc1. The sgement overlap test - "if ((mend > > > > pstart) && (mstart < pend))" - fails, returning a -EINVAL. > > > > > > Is anyone else seeing this? > > > > Mimi, should be this issue? I have sent a fix for that. > > https://lore.kernel.org/lkml/20181228011247.GA9999@dhcp-128-65.nay.redhat.com/ > > Hi, Kairui, I think you should know this while working on this series, > It is good to mention the test dependency in cover letter so that reviewers > can save time. > > BTW, Boris took it in tip already: > https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git/commit/?id=993a110319a4a60aadbd02f6defdebe048f7773b > Hi, thanks for the suggestion, I did apply your patch to avoid the failure. Will add such info next time. Will send out V4 and update commit message as suggested by Mimi -- Best Regards, Kairui Song
This patch series adds a .platform_trusted_keys in system_keyring as the reference to .platform keyring in integrity subsystem, when platform keyring is being initialized it will be updated. So other component could use this keyring as well. This patch series also let kexec_file_load use platform keyring as fall back if it failed to verify the image against secondary keyring, make it possible to load kernel signed by third part key if third party key is imported in the firmware. After this patch kexec_file_load will be able to verify a signed PE bzImage using keys in platform keyring. Tested in a VM with locally signed kernel with pesign and imported the cert to EFI's MokList variable. Kairui Song (2): integrity, KEYS: add a reference to platform keyring kexec, KEYS: Make use of platform keyring for signature verify Update from V2: - Use IS_ENABLED in kexec_file_load to judge if platform_trusted_keys should be used for verifying image as suggested by Mimi Zohar Update from V1: - Make platform_trusted_keys static, and update commit message as suggested by Mimi Zohar - Always check if platform keyring is initialized before use it Kairui Song (2): integrity, KEYS: add a reference to platform keyring kexec, KEYS: Make use of platform keyring for signature verify arch/x86/kernel/kexec-bzimage64.c | 13 ++++++++++--- certs/system_keyring.c | 22 +++++++++++++++++++++- include/keys/system_keyring.h | 5 +++++ include/linux/verification.h | 1 + security/integrity/digsig.c | 6 ++++++ 5 files changed, 43 insertions(+), 4 deletions(-)