Re: Firmware signing -- Re: [PATCH 00/27] security, efi: Add kernel lockdown

[AKASHI, Takahiro]

Mimi,

On Wed, Nov 08, 2017 at 09:17:37PM -0500, Mimi Zohar wrote:
> > > IMHO that should just fail then, ie, a "locked down" kernel should not want to
> > > *pass* a firmware signature if such thing could not be done.
> > > 
> > > Its no different than trying to verify a signed module on a "locked down" for
> > > which it has no signature.
> > > 
> > > But perhaps I'm not understanding the issue well, let me know.
> > 
> > My point is quite simple:
> > my_deviceA_init() {
> >         err = request_firmware(&fw, "deviceA"); <--- (a)
> >         if (err)
> >                 goto err_request;
> > 
> >         err = verify_firmware(fw);  <--- (b)
> >         if (err)
> >                 goto err_verify;
> > 
> >         load_fw_to_deviceA(fw);     <--- (c)
> >         ...
> > }
> > 
> > As legacy device drivers does not have (b), there is no chance to
> > prevent loading a firmware at (c) for locked-down kernel.
> > 
> > If you allow me to bring in yet another function, say
> > request_firmware_signable(), which should be used in place of (a)
> > for all verification-aware drivers, that would be fine.
> 
> I really don't understand why you need a new function.ÂÂThe
> request_firmware() eventually calls kernel_read_file_from_path(),
> which already calls the pre and post LSM hooks.

My assumption here is:
1) there are some less important and so security-insensitive firmwares,
   by which I mean that such firmwares won't be expected to be signed in
   terms of vulnerability or integrity.
   (I can't give you examples though.)
2) firmware's signature will be presented separately from the firmware
   blob itself. Say, "firmware.bin.p7s" for "firmware.bin"

I don't think that the current security_kernel(_post)_read_file() scheme
fit with this assumption very well.

Thanks,
-Takahiro AKASHI


> IMA-appraisal is already on these hooks verifying the requested
> firmware's signature. ÂFor systems with "lockdown" enabled, but
> without IMA-appraisal enabled, define a small, builtin LSM that sits
> on these LSM hooks and denies the unsigned firmware requests.
> 
> Mimi
> 
> > In this case, all the invocation of request_firmware() in legacy code
> > could be forced to fail in locked-down kernel.
> > 
> > But I think that "signable" should be allowed to be combined with other
> > features of request_firmware variants like _(no)wait or _direct.
> > 
> > -Takahiro AKASHI
>