Re: [RFC PATCH] kern/dl: Add module version check

[Zhang Boyang <zhangboyang.id@gmail.com>]

Hi,

(sorry for duplicate emails because I forgot to add CCs)

On 2022/12/21 06:58, Robbie Harwood wrote:
> Zhang Boyang <zhangboyang.id@gmail.com> writes:
> 
>> This patch add version information to GRUB modules. Specifically,
>> PACKAGE_VERSION is embedded as null-terminated string in .modver
>> section. This string is checked at module loading time. That module will
>> be rejected if mismatch is found. This will prevent loading modules from
>> different versions of GRUB.
>>
>> It should be pointed out that the version string is only consisted of
>> PACKAGE_VERSION. Any difference not reflected in PACKAGE_VERSION is not
>> noticed by version check (e.g. configure options).
> 
> Right now, this only affects non-secureboot scenarios (because we don't
> have external signed module support).  I would want to see a resolution
> to the external module signing question first so that we don't paint
> ourselves into a corner with something like this.
> 

This feature is a prerequisite for external signed module support (and 
it is mainly designed for this purpose). SBAT itself is not sufficient 
to protect version mismatch attacks. Consider this scenario:

1) GRUB 2.XX is free of vulnerabilities

2) GRUB 2.YY is also free of vulnerabilities

3) So GRUB 2.XX shares same SBAT numbers with GRUB 2.YY

4) So if there is no version check, it's possible to load GRUB 2.YY 
modules into GRUB 2.XX (and vice versa)

5) However, due to some changes in API or ABI, there is possibility that 
there are vulnerabilities when using GRUB 2.YY modules with GRUB 2.XX.

> I share Glenn's confusion about what real-world problems this addresses:
> my understanding is that grub modules mostly register callbacks, so the
> possibility of disaster is low (unless the callback interfaces change of
> course, but that generally has not happened).
> 

Please see my reply to Glenn for a example that mismatched modules 
crashes GRUB.

> The combination of those two things leads me to suspect this is not the
> right approach.  It seems likely that if we want to down the road of
> versionlocking, something like the kernel's ephemeral key approach would
> be better suited - and if we want external modules (which I don't think
> we do), full SBAT support.
> 

Sorry but I'm not familiar with the kernel's ephemeral key approach. 
Could you please give me some links to this?

By the way, I figured out why shim doesn't enforce SBAT checks for files 
if they are not directly loaded by shim AND have no SBAT information. 
According to 
https://github.com/rhboot/shim/commit/a2da05fcb8972628bec08e4adfc13abbafc319ad 
  , it's vendor's responsibility to revoke all files without SBAT 
information and only sign files with SBAT information at all time.

My grub-wrap patch 
(https://lists.gnu.org/archive/html/grub-devel/2022-12/msg00070.html ) 
already has support for embedding SBAT information to PE wrappers, 
although it not enforced. With this patch, it should be easy to 
implement external modules if wanted (yes, with full SBAT support, if 
vendor always add SBAT information to modules).

Best Regards,
Zhang Boyang

> Be well,
> --Robbie