* [PATCH] Add chainloaded image as shim's verifiable object @ 2021-03-05 12:11 Michael Chang 2021-03-05 12:21 ` Dimitri John Ledkov 2021-03-05 12:32 ` Thomas Frauendorfer 0 siblings, 2 replies; 9+ messages in thread From: Michael Chang @ 2021-03-05 12:11 UTC (permalink / raw) To: The development of GNU GRUB While attempting to dual boot Microsoft Windows with efi chainloader, it failed with below error when secure boot was enabled. error ../../grub-core/kern/verifiers.c:119:verification requested but nobody cares: /EFI/Microsoft/Boot/bootmgfw.efi. It is a regression, as previously it worked without problem. It turns out chainloading image has been locked down introduced by 578c95298 kern: Add lockdown support However we should consider it as verifiable object to shim to allow booting in secure boot enabled mode. The chainloaded image could also have trusted signature signed by vendor with their pubkey cert in db. For that matters it's usage should not be locked down in secure boot, and instead use shim to validate it's signature before running it. Signed-off-by: Michael Chang <mchang@suse.com> --- grub-core/kern/efi/sb.c | 1 + grub-core/kern/lockdown.c | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/grub-core/kern/efi/sb.c b/grub-core/kern/efi/sb.c index 41dadcd14..96d237722 100644 --- a/grub-core/kern/efi/sb.c +++ b/grub-core/kern/efi/sb.c @@ -129,6 +129,7 @@ shim_lock_verifier_init (grub_file_t io __attribute__ ((unused)), case GRUB_FILE_TYPE_BSD_KERNEL: case GRUB_FILE_TYPE_XNU_KERNEL: case GRUB_FILE_TYPE_PLAN9_KERNEL: + case GRUB_FILE_TYPE_EFI_CHAINLOADED_IMAGE: *flags = GRUB_VERIFY_FLAGS_SINGLE_CHUNK; /* Fall through. */ diff --git a/grub-core/kern/lockdown.c b/grub-core/kern/lockdown.c index 0bc70fd42..e1fd1c1e2 100644 --- a/grub-core/kern/lockdown.c +++ b/grub-core/kern/lockdown.c @@ -48,7 +48,6 @@ lockdown_verifier_init (grub_file_t io __attribute__ ((unused)), case GRUB_FILE_TYPE_PXECHAINLOADER: case GRUB_FILE_TYPE_PCCHAINLOADER: case GRUB_FILE_TYPE_COREBOOT_CHAINLOADER: - case GRUB_FILE_TYPE_EFI_CHAINLOADED_IMAGE: case GRUB_FILE_TYPE_ACPI_TABLE: case GRUB_FILE_TYPE_DEVICE_TREE_IMAGE: *flags = GRUB_VERIFY_FLAGS_DEFER_AUTH; -- 2.26.2 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] Add chainloaded image as shim's verifiable object 2021-03-05 12:11 [PATCH] Add chainloaded image as shim's verifiable object Michael Chang @ 2021-03-05 12:21 ` Dimitri John Ledkov 2021-03-05 13:34 ` Michael Chang 2021-03-05 12:32 ` Thomas Frauendorfer 1 sibling, 1 reply; 9+ messages in thread From: Dimitri John Ledkov @ 2021-03-05 12:21 UTC (permalink / raw) To: The development of GNU GRUB; +Cc: Michael Chang This is not an oversight but intentional. Currently there is no chainloader support with SBAT as further development is required to ensure policy is applied correctly. Once SBAT support for chainloading is defined, it will be introduced. And yes it is intended to continue to allow "boot windows" as the menu entry in grub. On Fri, Mar 5, 2021 at 12:12 PM Michael Chang via Grub-devel <grub-devel@gnu.org> wrote: > > While attempting to dual boot Microsoft Windows with efi chainloader, it > failed with below error when secure boot was enabled. > > error ../../grub-core/kern/verifiers.c:119:verification requested but > nobody cares: /EFI/Microsoft/Boot/bootmgfw.efi. > > It is a regression, as previously it worked without problem. > > It turns out chainloading image has been locked down introduced by > > 578c95298 kern: Add lockdown support > > However we should consider it as verifiable object to shim to allow > booting in secure boot enabled mode. The chainloaded image could also > have trusted signature signed by vendor with their pubkey cert in db. > For that matters it's usage should not be locked down in secure boot, > and instead use shim to validate it's signature before running it. > > Signed-off-by: Michael Chang <mchang@suse.com> > --- > grub-core/kern/efi/sb.c | 1 + > grub-core/kern/lockdown.c | 1 - > 2 files changed, 1 insertion(+), 1 deletion(-) > > diff --git a/grub-core/kern/efi/sb.c b/grub-core/kern/efi/sb.c > index 41dadcd14..96d237722 100644 > --- a/grub-core/kern/efi/sb.c > +++ b/grub-core/kern/efi/sb.c > @@ -129,6 +129,7 @@ shim_lock_verifier_init (grub_file_t io __attribute__ ((unused)), > case GRUB_FILE_TYPE_BSD_KERNEL: > case GRUB_FILE_TYPE_XNU_KERNEL: > case GRUB_FILE_TYPE_PLAN9_KERNEL: > + case GRUB_FILE_TYPE_EFI_CHAINLOADED_IMAGE: > *flags = GRUB_VERIFY_FLAGS_SINGLE_CHUNK; > > /* Fall through. */ > diff --git a/grub-core/kern/lockdown.c b/grub-core/kern/lockdown.c > index 0bc70fd42..e1fd1c1e2 100644 > --- a/grub-core/kern/lockdown.c > +++ b/grub-core/kern/lockdown.c > @@ -48,7 +48,6 @@ lockdown_verifier_init (grub_file_t io __attribute__ ((unused)), > case GRUB_FILE_TYPE_PXECHAINLOADER: > case GRUB_FILE_TYPE_PCCHAINLOADER: > case GRUB_FILE_TYPE_COREBOOT_CHAINLOADER: > - case GRUB_FILE_TYPE_EFI_CHAINLOADED_IMAGE: > case GRUB_FILE_TYPE_ACPI_TABLE: > case GRUB_FILE_TYPE_DEVICE_TREE_IMAGE: > *flags = GRUB_VERIFY_FLAGS_DEFER_AUTH; > -- > 2.26.2 > > > _______________________________________________ > Grub-devel mailing list > Grub-devel@gnu.org > https://lists.gnu.org/mailman/listinfo/grub-devel -- Regards, Dimitri. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Add chainloaded image as shim's verifiable object 2021-03-05 12:21 ` Dimitri John Ledkov @ 2021-03-05 13:34 ` Michael Chang 2021-03-05 13:49 ` Dimitri John Ledkov 0 siblings, 1 reply; 9+ messages in thread From: Michael Chang @ 2021-03-05 13:34 UTC (permalink / raw) To: Dimitri John Ledkov; +Cc: The development of GNU GRUB On Fri, Mar 05, 2021 at 12:21:49PM +0000, Dimitri John Ledkov wrote: > This is not an oversight but intentional. > > Currently there is no chainloader support with SBAT as further > development is required to ensure policy is applied correctly. Once > SBAT support for chainloading is defined, it will be introduced. Hm. What I heard is that shim wouldn't enfore SBAT validation for the "indirect" image loaded by grub for now. So we should still able to boot, eg, kernel and other efi image loaded by grub which is still in lacking of SBAT support. > And yes it is intended to continue to allow "boot windows" as the menu > entry in grub. So what is recommended solution in the interim ? We just can't afford to release new grub version that cannot do the chainload ... Thanks, Michael > > On Fri, Mar 5, 2021 at 12:12 PM Michael Chang via Grub-devel > <grub-devel@gnu.org> wrote: > > > > While attempting to dual boot Microsoft Windows with efi chainloader, it > > failed with below error when secure boot was enabled. > > > > error ../../grub-core/kern/verifiers.c:119:verification requested but > > nobody cares: /EFI/Microsoft/Boot/bootmgfw.efi. > > > > It is a regression, as previously it worked without problem. > > > > It turns out chainloading image has been locked down introduced by > > > > 578c95298 kern: Add lockdown support > > > > However we should consider it as verifiable object to shim to allow > > booting in secure boot enabled mode. The chainloaded image could also > > have trusted signature signed by vendor with their pubkey cert in db. > > For that matters it's usage should not be locked down in secure boot, > > and instead use shim to validate it's signature before running it. > > > > Signed-off-by: Michael Chang <mchang@suse.com> > > --- > > grub-core/kern/efi/sb.c | 1 + > > grub-core/kern/lockdown.c | 1 - > > 2 files changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/grub-core/kern/efi/sb.c b/grub-core/kern/efi/sb.c > > index 41dadcd14..96d237722 100644 > > --- a/grub-core/kern/efi/sb.c > > +++ b/grub-core/kern/efi/sb.c > > @@ -129,6 +129,7 @@ shim_lock_verifier_init (grub_file_t io __attribute__ ((unused)), > > case GRUB_FILE_TYPE_BSD_KERNEL: > > case GRUB_FILE_TYPE_XNU_KERNEL: > > case GRUB_FILE_TYPE_PLAN9_KERNEL: > > + case GRUB_FILE_TYPE_EFI_CHAINLOADED_IMAGE: > > *flags = GRUB_VERIFY_FLAGS_SINGLE_CHUNK; > > > > /* Fall through. */ > > diff --git a/grub-core/kern/lockdown.c b/grub-core/kern/lockdown.c > > index 0bc70fd42..e1fd1c1e2 100644 > > --- a/grub-core/kern/lockdown.c > > +++ b/grub-core/kern/lockdown.c > > @@ -48,7 +48,6 @@ lockdown_verifier_init (grub_file_t io __attribute__ ((unused)), > > case GRUB_FILE_TYPE_PXECHAINLOADER: > > case GRUB_FILE_TYPE_PCCHAINLOADER: > > case GRUB_FILE_TYPE_COREBOOT_CHAINLOADER: > > - case GRUB_FILE_TYPE_EFI_CHAINLOADED_IMAGE: > > case GRUB_FILE_TYPE_ACPI_TABLE: > > case GRUB_FILE_TYPE_DEVICE_TREE_IMAGE: > > *flags = GRUB_VERIFY_FLAGS_DEFER_AUTH; > > -- > > 2.26.2 > > > > > > _______________________________________________ > > Grub-devel mailing list > > Grub-devel@gnu.org > > https://lists.gnu.org/mailman/listinfo/grub-devel > > > > -- > Regards, > > Dimitri. > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Add chainloaded image as shim's verifiable object 2021-03-05 13:34 ` Michael Chang @ 2021-03-05 13:49 ` Dimitri John Ledkov 2021-03-08 4:59 ` Michael Chang 0 siblings, 1 reply; 9+ messages in thread From: Dimitri John Ledkov @ 2021-03-05 13:49 UTC (permalink / raw) To: Michael Chang; +Cc: The development of GNU GRUB On Fri, Mar 5, 2021 at 1:34 PM Michael Chang <mchang@suse.com> wrote: > > On Fri, Mar 05, 2021 at 12:21:49PM +0000, Dimitri John Ledkov wrote: > > This is not an oversight but intentional. > > > > Currently there is no chainloader support with SBAT as further > > development is required to ensure policy is applied correctly. Once > > SBAT support for chainloading is defined, it will be introduced. > > Hm. What I heard is that shim wouldn't enfore SBAT validation for the > "indirect" image loaded by grub for now. So we should still able to > boot, eg, kernel and other efi image loaded by grub which is still in > lacking of SBAT support. > There is a difference between chainloading a new bootloader, or booting a linux kernel. When chainloading a different grub that is verified by the vendor-db in shim, sbat should be enforced, if the current grub has sbat. When chainloading windows bootmgr, which is signed by things in db, sbat need not be enforced. It is a delicate balance to strike, as otherwise one can do sbat bypass via downgrading to older grub via chainload. I would recommend to add windowsbootmgr efi entry, and then perform `exit 1` such that this current shim+grub exit, and firmware goes to load the next entry for windows. For `exit 1` support you need https://github.com/rhboot/grub2/commit/ccce3d69ae3eacc7bdc70217304586bd7e74fe1e too. I understand that this is very suboptimal. I kind of wish grub2 could generate and display EFI boot entries, and then let people select them which would set it as BootNext and reset. Cause then one would truly be able to jump to a specific boot menu entry from grub menu. I also need chainloader support elsewhere, thus at the moment we are not shipping 2.06rc1 and instead are shipping 2.04 with backports of patches. > > And yes it is intended to continue to allow "boot windows" as the menu > > entry in grub. > > So what is recommended solution in the interim ? We just can't afford > to release new grub version that cannot do the chainload ... > > Thanks, > Michael > > > > > On Fri, Mar 5, 2021 at 12:12 PM Michael Chang via Grub-devel > > <grub-devel@gnu.org> wrote: > > > > > > While attempting to dual boot Microsoft Windows with efi chainloader, it > > > failed with below error when secure boot was enabled. > > > > > > error ../../grub-core/kern/verifiers.c:119:verification requested but > > > nobody cares: /EFI/Microsoft/Boot/bootmgfw.efi. > > > > > > It is a regression, as previously it worked without problem. > > > > > > It turns out chainloading image has been locked down introduced by > > > > > > 578c95298 kern: Add lockdown support > > > > > > However we should consider it as verifiable object to shim to allow > > > booting in secure boot enabled mode. The chainloaded image could also > > > have trusted signature signed by vendor with their pubkey cert in db. > > > For that matters it's usage should not be locked down in secure boot, > > > and instead use shim to validate it's signature before running it. > > > > > > Signed-off-by: Michael Chang <mchang@suse.com> > > > --- > > > grub-core/kern/efi/sb.c | 1 + > > > grub-core/kern/lockdown.c | 1 - > > > 2 files changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/grub-core/kern/efi/sb.c b/grub-core/kern/efi/sb.c > > > index 41dadcd14..96d237722 100644 > > > --- a/grub-core/kern/efi/sb.c > > > +++ b/grub-core/kern/efi/sb.c > > > @@ -129,6 +129,7 @@ shim_lock_verifier_init (grub_file_t io __attribute__ ((unused)), > > > case GRUB_FILE_TYPE_BSD_KERNEL: > > > case GRUB_FILE_TYPE_XNU_KERNEL: > > > case GRUB_FILE_TYPE_PLAN9_KERNEL: > > > + case GRUB_FILE_TYPE_EFI_CHAINLOADED_IMAGE: > > > *flags = GRUB_VERIFY_FLAGS_SINGLE_CHUNK; > > > > > > /* Fall through. */ > > > diff --git a/grub-core/kern/lockdown.c b/grub-core/kern/lockdown.c > > > index 0bc70fd42..e1fd1c1e2 100644 > > > --- a/grub-core/kern/lockdown.c > > > +++ b/grub-core/kern/lockdown.c > > > @@ -48,7 +48,6 @@ lockdown_verifier_init (grub_file_t io __attribute__ ((unused)), > > > case GRUB_FILE_TYPE_PXECHAINLOADER: > > > case GRUB_FILE_TYPE_PCCHAINLOADER: > > > case GRUB_FILE_TYPE_COREBOOT_CHAINLOADER: > > > - case GRUB_FILE_TYPE_EFI_CHAINLOADED_IMAGE: > > > case GRUB_FILE_TYPE_ACPI_TABLE: > > > case GRUB_FILE_TYPE_DEVICE_TREE_IMAGE: > > > *flags = GRUB_VERIFY_FLAGS_DEFER_AUTH; > > > -- > > > 2.26.2 > > > > > > > > > _______________________________________________ > > > Grub-devel mailing list > > > Grub-devel@gnu.org > > > https://lists.gnu.org/mailman/listinfo/grub-devel > > > > > > > > -- > > Regards, > > > > Dimitri. > > > -- Regards, Dimitri. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Add chainloaded image as shim's verifiable object 2021-03-05 13:49 ` Dimitri John Ledkov @ 2021-03-08 4:59 ` Michael Chang 2021-03-09 18:45 ` Thomas Frauendorfer 0 siblings, 1 reply; 9+ messages in thread From: Michael Chang @ 2021-03-08 4:59 UTC (permalink / raw) To: Dimitri John Ledkov; +Cc: The development of GNU GRUB On Fri, Mar 05, 2021 at 01:49:00PM +0000, Dimitri John Ledkov wrote: > On Fri, Mar 5, 2021 at 1:34 PM Michael Chang <mchang@suse.com> wrote: > > > > On Fri, Mar 05, 2021 at 12:21:49PM +0000, Dimitri John Ledkov wrote: > > > This is not an oversight but intentional. > > > > > > Currently there is no chainloader support with SBAT as further > > > development is required to ensure policy is applied correctly. Once > > > SBAT support for chainloading is defined, it will be introduced. > > > > Hm. What I heard is that shim wouldn't enfore SBAT validation for the > > "indirect" image loaded by grub for now. So we should still able to > > boot, eg, kernel and other efi image loaded by grub which is still in > > lacking of SBAT support. > > > > There is a difference between chainloading a new bootloader, or > booting a linux kernel. > When chainloading a different grub that is verified by the vendor-db > in shim, sbat should be enforced, if the current grub has sbat. > When chainloading windows bootmgr, which is signed by things in db, > sbat need not be enforced. Can't this be done in shim and shim_lock itself ? It should be able to discern that the .sbat is missing and therefore skip vendor-db. For the linux distribution they should only take care rotating out their signing keys for this boothole2 update and use shim_lock to reject degrarded pre-boothole2 grub release. Or did I miss anything here ? > It is a delicate balance to strike, as otherwise one can do sbat > bypass via downgrading to older grub via chainload. > > I would recommend to add windowsbootmgr efi entry, and then perform > `exit 1` such that this current shim+grub exit, and firmware goes to > load the next entry for windows. For `exit 1` support you need > https://github.com/rhboot/grub2/commit/ccce3d69ae3eacc7bdc70217304586bd7e74fe1e > too. The exit approach appears to be very fragile here, as it is very unclear about what the firmware would act upon that. We would expect it to be next boot entry from the boot order list, among them basically can be anything if any boot vars changed in the meanwhile. I even have noticed that at times the boot order list is disregarded after exit performed. The qemu+aavmf would always bring up uiapp to display uefi boot menu in it's reaction to exit. > I understand that this is very suboptimal. I kind of wish grub2 could > generate and display EFI boot entries, and then let people select them > which would set it as BootNext and reset. Cause then one would truly > be able to jump to a specific boot menu entry from grub menu. Why not consider the idea of returning to shim and pointing it the next image to boot ? This way we wouldn't have to touch the existing boot entries, and also avoid the problem that efi boot manager may not be able to verify the image signed by MOK ... > I also need chainloader support elsewhere, thus at the moment we are > not shipping 2.06rc1 and instead are shipping 2.04 with backports of > patches. Please continue the chainloader support, it is really really important feature in many ways. Especially for arm, there's quite a lot of them are shipping with ephemeral boot variables, and thus chainloading from grub is the only way if they have to launch secondary boot images. And I believe there are much more cases that require efi chainloader to work. It is a real ship stopper IMHO if it stopped to work. Thanks, Michael > > > > > And yes it is intended to continue to allow "boot windows" as the menu > > > entry in grub. > > > > So what is recommended solution in the interim ? We just can't afford > > to release new grub version that cannot do the chainload ... > > > > Thanks, > > Michael > > > > > > > > On Fri, Mar 5, 2021 at 12:12 PM Michael Chang via Grub-devel > > > <grub-devel@gnu.org> wrote: > > > > > > > > While attempting to dual boot Microsoft Windows with efi chainloader, it > > > > failed with below error when secure boot was enabled. > > > > > > > > error ../../grub-core/kern/verifiers.c:119:verification requested but > > > > nobody cares: /EFI/Microsoft/Boot/bootmgfw.efi. > > > > > > > > It is a regression, as previously it worked without problem. > > > > > > > > It turns out chainloading image has been locked down introduced by > > > > > > > > 578c95298 kern: Add lockdown support > > > > > > > > However we should consider it as verifiable object to shim to allow > > > > booting in secure boot enabled mode. The chainloaded image could also > > > > have trusted signature signed by vendor with their pubkey cert in db. > > > > For that matters it's usage should not be locked down in secure boot, > > > > and instead use shim to validate it's signature before running it. > > > > > > > > Signed-off-by: Michael Chang <mchang@suse.com> > > > > --- > > > > grub-core/kern/efi/sb.c | 1 + > > > > grub-core/kern/lockdown.c | 1 - > > > > 2 files changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/grub-core/kern/efi/sb.c b/grub-core/kern/efi/sb.c > > > > index 41dadcd14..96d237722 100644 > > > > --- a/grub-core/kern/efi/sb.c > > > > +++ b/grub-core/kern/efi/sb.c > > > > @@ -129,6 +129,7 @@ shim_lock_verifier_init (grub_file_t io __attribute__ ((unused)), > > > > case GRUB_FILE_TYPE_BSD_KERNEL: > > > > case GRUB_FILE_TYPE_XNU_KERNEL: > > > > case GRUB_FILE_TYPE_PLAN9_KERNEL: > > > > + case GRUB_FILE_TYPE_EFI_CHAINLOADED_IMAGE: > > > > *flags = GRUB_VERIFY_FLAGS_SINGLE_CHUNK; > > > > > > > > /* Fall through. */ > > > > diff --git a/grub-core/kern/lockdown.c b/grub-core/kern/lockdown.c > > > > index 0bc70fd42..e1fd1c1e2 100644 > > > > --- a/grub-core/kern/lockdown.c > > > > +++ b/grub-core/kern/lockdown.c > > > > @@ -48,7 +48,6 @@ lockdown_verifier_init (grub_file_t io __attribute__ ((unused)), > > > > case GRUB_FILE_TYPE_PXECHAINLOADER: > > > > case GRUB_FILE_TYPE_PCCHAINLOADER: > > > > case GRUB_FILE_TYPE_COREBOOT_CHAINLOADER: > > > > - case GRUB_FILE_TYPE_EFI_CHAINLOADED_IMAGE: > > > > case GRUB_FILE_TYPE_ACPI_TABLE: > > > > case GRUB_FILE_TYPE_DEVICE_TREE_IMAGE: > > > > *flags = GRUB_VERIFY_FLAGS_DEFER_AUTH; > > > > -- > > > > 2.26.2 > > > > > > > > > > > > _______________________________________________ > > > > Grub-devel mailing list > > > > Grub-devel@gnu.org > > > > https://lists.gnu.org/mailman/listinfo/grub-devel > > > > > > > > > > > > -- > > > Regards, > > > > > > Dimitri. > > > > > > > > -- > Regards, > > Dimitri. > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Add chainloaded image as shim's verifiable object 2021-03-08 4:59 ` Michael Chang @ 2021-03-09 18:45 ` Thomas Frauendorfer 2021-03-10 2:33 ` Michael Chang 0 siblings, 1 reply; 9+ messages in thread From: Thomas Frauendorfer @ 2021-03-09 18:45 UTC (permalink / raw) To: The development of GNU GRUB; +Cc: Dimitri John Ledkov, Michael Chang On Mon, Mar 8, 2021 at 5:59 AM Michael Chang via Grub-devel <grub-devel@gnu.org> wrote: > > On Fri, Mar 05, 2021 at 01:49:00PM +0000, Dimitri John Ledkov wrote: > > On Fri, Mar 5, 2021 at 1:34 PM Michael Chang <mchang@suse.com> wrote: > > > > > > On Fri, Mar 05, 2021 at 12:21:49PM +0000, Dimitri John Ledkov wrote: > > > > This is not an oversight but intentional. > > > > > > > > Currently there is no chainloader support with SBAT as further > > > > development is required to ensure policy is applied correctly. Once > > > > SBAT support for chainloading is defined, it will be introduced. > > > > > > Hm. What I heard is that shim wouldn't enfore SBAT validation for the > > > "indirect" image loaded by grub for now. So we should still able to > > > boot, eg, kernel and other efi image loaded by grub which is still in > > > lacking of SBAT support. > > > > > > > There is a difference between chainloading a new bootloader, or > > booting a linux kernel. > > When chainloading a different grub that is verified by the vendor-db > > in shim, sbat should be enforced, if the current grub has sbat. > > When chainloading windows bootmgr, which is signed by things in db, > > sbat need not be enforced. > > Can't this be done in shim and shim_lock itself ? It should be able to > discern that the .sbat is missing and therefore skip vendor-db. For the > linux distribution they should only take care rotating out their signing > keys for this boothole2 update and use shim_lock to reject degrarded > pre-boothole2 grub release. > > Or did I miss anything here ? As far as I understand it there is one thing to keep in mind when using chainloader with the upcoming shim: If a new vulnerability is discovered in Grub any time in the future, then the SBAT revocation has to be done for the Shim, not Grub. Then a new Shim would need to blacklist all vulnerable Grub versions that were signed with the embedded key. This is because a new, fixed Grub would still load the vulnerable Grub through chainloader and Shim would accept it because it does not check the SBAT section on that verification path. Of course the revocation can be handled on a per distribution / product basis. But this is probably something to discuss on the shim review board. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Add chainloaded image as shim's verifiable object 2021-03-09 18:45 ` Thomas Frauendorfer @ 2021-03-10 2:33 ` Michael Chang 0 siblings, 0 replies; 9+ messages in thread From: Michael Chang @ 2021-03-10 2:33 UTC (permalink / raw) To: Thomas Frauendorfer; +Cc: The development of GNU GRUB, Dimitri John Ledkov On Tue, Mar 09, 2021 at 07:45:55PM +0100, Thomas Frauendorfer wrote: > On Mon, Mar 8, 2021 at 5:59 AM Michael Chang via Grub-devel > <grub-devel@gnu.org> wrote: > > > > On Fri, Mar 05, 2021 at 01:49:00PM +0000, Dimitri John Ledkov wrote: > > > On Fri, Mar 5, 2021 at 1:34 PM Michael Chang <mchang@suse.com> wrote: > > > > > > > > On Fri, Mar 05, 2021 at 12:21:49PM +0000, Dimitri John Ledkov wrote: > > > > > This is not an oversight but intentional. > > > > > > > > > > Currently there is no chainloader support with SBAT as further > > > > > development is required to ensure policy is applied correctly. Once > > > > > SBAT support for chainloading is defined, it will be introduced. > > > > > > > > Hm. What I heard is that shim wouldn't enfore SBAT validation for the > > > > "indirect" image loaded by grub for now. So we should still able to > > > > boot, eg, kernel and other efi image loaded by grub which is still in > > > > lacking of SBAT support. > > > > > > > > > > There is a difference between chainloading a new bootloader, or > > > booting a linux kernel. > > > When chainloading a different grub that is verified by the vendor-db > > > in shim, sbat should be enforced, if the current grub has sbat. > > > When chainloading windows bootmgr, which is signed by things in db, > > > sbat need not be enforced. > > > > Can't this be done in shim and shim_lock itself ? It should be able to > > discern that the .sbat is missing and therefore skip vendor-db. For the > > linux distribution they should only take care rotating out their signing > > keys for this boothole2 update and use shim_lock to reject degrarded > > pre-boothole2 grub release. > > > > Or did I miss anything here ? > > As far as I understand it there is one thing to keep in mind when > using chainloader with the upcoming shim: > > If a new vulnerability is discovered in Grub any time in the future, > then the SBAT revocation has to be done for the Shim, not Grub. > Then a new Shim would need to blacklist all vulnerable Grub versions > that were signed with the embedded key. > > This is because a new, fixed Grub would still load the vulnerable Grub > through chainloader and Shim would accept it because it does not check > the SBAT section on that verification path. > > Of course the revocation can be handled on a per distribution / product basis. > > But this is probably something to discuss on the shim review board. Thanks for your clarification. Yes, granted that chainloader could be a potential vehicle to bypass the the verification in the future sbat revocation is a thing we have to keep in mind and discuss in the future. But given we are still relying on the key revocation mechanism currently for this second round of boothole security update, in that the verification of chainloaded image remain covered by shim lock (vendor_dbx) properly. It is therefore unexpect to see chainloader to be rejected at this moment for this round of security update by the upstream. I hope this also makes my point clear. And last I would like to take the opportunity to thank all who worked on this securiy update. It is tremendous ! Thanks, Michael > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Add chainloaded image as shim's verifiable object 2021-03-05 12:11 [PATCH] Add chainloaded image as shim's verifiable object Michael Chang 2021-03-05 12:21 ` Dimitri John Ledkov @ 2021-03-05 12:32 ` Thomas Frauendorfer 2021-03-05 13:19 ` Michael Chang 1 sibling, 1 reply; 9+ messages in thread From: Thomas Frauendorfer @ 2021-03-05 12:32 UTC (permalink / raw) To: The development of GNU GRUB; +Cc: Michael Chang On Fri, Mar 5, 2021 at 1:12 PM Michael Chang via Grub-devel <grub-devel@gnu.org> wrote: > > While attempting to dual boot Microsoft Windows with efi chainloader, it > failed with below error when secure boot was enabled. > > error ../../grub-core/kern/verifiers.c:119:verification requested but > nobody cares: /EFI/Microsoft/Boot/bootmgfw.efi. > > It is a regression, as previously it worked without problem. > > It turns out chainloading image has been locked down introduced by > > 578c95298 kern: Add lockdown support > > However we should consider it as verifiable object to shim to allow > booting in secure boot enabled mode. The chainloaded image could also > have trusted signature signed by vendor with their pubkey cert in db. > For that matters it's usage should not be locked down in secure boot, > and instead use shim to validate it's signature before running it. > > Signed-off-by: Michael Chang <mchang@suse.com> [cut out] > /* Fall through. */ > diff --git a/grub-core/kern/lockdown.c b/grub-core/kern/lockdown.c > index 0bc70fd42..e1fd1c1e2 100644 > --- a/grub-core/kern/lockdown.c > +++ b/grub-core/kern/lockdown.c > @@ -48,7 +48,6 @@ lockdown_verifier_init (grub_file_t io __attribute__ ((unused)), > case GRUB_FILE_TYPE_PXECHAINLOADER: > case GRUB_FILE_TYPE_PCCHAINLOADER: > case GRUB_FILE_TYPE_COREBOOT_CHAINLOADER: > - case GRUB_FILE_TYPE_EFI_CHAINLOADED_IMAGE: > case GRUB_FILE_TYPE_ACPI_TABLE: > case GRUB_FILE_TYPE_DEVICE_TREE_IMAGE: > *flags = GRUB_VERIFY_FLAGS_DEFER_AUTH; > -- > 2.26.2 The lockdown verifier makes sure that at least one verifer has validated the image. So removing GRUB_FILE_TYPE_EFI_CHAINLOADED_IMAGE from it is a very bad idea. > _______________________________________________ > Grub-devel mailing list > Grub-devel@gnu.orgthe > https://lists.gnu.org/mailman/listinfo/grub-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] Add chainloaded image as shim's verifiable object 2021-03-05 12:32 ` Thomas Frauendorfer @ 2021-03-05 13:19 ` Michael Chang 0 siblings, 0 replies; 9+ messages in thread From: Michael Chang @ 2021-03-05 13:19 UTC (permalink / raw) To: Thomas Frauendorfer; +Cc: The development of GNU GRUB On Fri, Mar 05, 2021 at 01:32:57PM +0100, Thomas Frauendorfer wrote: > On Fri, Mar 5, 2021 at 1:12 PM Michael Chang via Grub-devel > <grub-devel@gnu.org> wrote: > > > > While attempting to dual boot Microsoft Windows with efi chainloader, it > > failed with below error when secure boot was enabled. > > > > error ../../grub-core/kern/verifiers.c:119:verification requested but > > nobody cares: /EFI/Microsoft/Boot/bootmgfw.efi. > > > > It is a regression, as previously it worked without problem. > > > > It turns out chainloading image has been locked down introduced by > > > > 578c95298 kern: Add lockdown support > > > > However we should consider it as verifiable object to shim to allow > > booting in secure boot enabled mode. The chainloaded image could also > > have trusted signature signed by vendor with their pubkey cert in db. > > For that matters it's usage should not be locked down in secure boot, > > and instead use shim to validate it's signature before running it. > > > > Signed-off-by: Michael Chang <mchang@suse.com> > > [cut out] > > > /* Fall through. */ > > diff --git a/grub-core/kern/lockdown.c b/grub-core/kern/lockdown.c > > index 0bc70fd42..e1fd1c1e2 100644 > > --- a/grub-core/kern/lockdown.c > > +++ b/grub-core/kern/lockdown.c > > @@ -48,7 +48,6 @@ lockdown_verifier_init (grub_file_t io __attribute__ ((unused)), > > case GRUB_FILE_TYPE_PXECHAINLOADER: > > case GRUB_FILE_TYPE_PCCHAINLOADER: > > case GRUB_FILE_TYPE_COREBOOT_CHAINLOADER: > > - case GRUB_FILE_TYPE_EFI_CHAINLOADED_IMAGE: > > case GRUB_FILE_TYPE_ACPI_TABLE: > > case GRUB_FILE_TYPE_DEVICE_TREE_IMAGE: > > *flags = GRUB_VERIFY_FLAGS_DEFER_AUTH; > > -- > > 2.26.2 > > The lockdown verifier makes sure that at least one verifer has > validated the image. > So removing GRUB_FILE_TYPE_EFI_CHAINLOADED_IMAGE from it is a very bad idea. Indeed. I will send second patch for not removing it from lockdown. Thanks a lot for your review. Michael > > > _______________________________________________ > > Grub-devel mailing list > > Grub-devel@gnu.orgthe > > https://lists.gnu.org/mailman/listinfo/grub-devel > ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2021-03-10 2:33 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-03-05 12:11 [PATCH] Add chainloaded image as shim's verifiable object Michael Chang 2021-03-05 12:21 ` Dimitri John Ledkov 2021-03-05 13:34 ` Michael Chang 2021-03-05 13:49 ` Dimitri John Ledkov 2021-03-08 4:59 ` Michael Chang 2021-03-09 18:45 ` Thomas Frauendorfer 2021-03-10 2:33 ` Michael Chang 2021-03-05 12:32 ` Thomas Frauendorfer 2021-03-05 13:19 ` Michael Chang
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.