Re: [PATCH v2 06/16] x86/efi: Generating random HMAC key for siging hibernate image

[Matt Fleming]

On Thu, 27 Aug, at 05:04:52PM, joeyli wrote:
> 
> The purpose of checking attribute of hibernation key variable is 
> in case someone created a key variable on runtime environment _before_
> this kernel create boot service variable. That causes EFI stub may load
> a key that from non-secure environment.
> 
> That's why delete non-boot service variable and create new one here.
 
I think bailing is more appropriate in that case, not deleting the
variable. The environment is not what we expected it to be, so we
shouldn't tamper with it.

But I don't feel super strongly about this point. I just wanted to
raise the question of whether it actually makes sense to delete the
variable that we obviously didn't create or whether it makes more
sense to refuse to verify hibernation images.

> > > diff --git a/arch/x86/include/asm/suspend.h b/arch/x86/include/asm/suspend.h
> > > index 2fab6c2..ab463c4 100644
> > > --- a/arch/x86/include/asm/suspend.h
> > > +++ b/arch/x86/include/asm/suspend.h
> > > @@ -3,3 +3,12 @@
> > >  #else
> > >  # include <asm/suspend_64.h>
> > >  #endif
> > > +
> > > +#ifdef CONFIG_HIBERNATE_VERIFICATION
> > > +#include <linux/suspend.h>
> > > +
> > > +struct hibernation_keys {
> > > +	unsigned long hkey_status;
> > > +	u8 hibernation_key[HIBERNATION_DIGEST_SIZE];
> > > +};
> > > +#endif
> > 
> > Have you given any thought to how things are going to work if we
> > change the hash function in the future, or provide a choice? That
> > information doesn't appear anywhere in the above struct.
> > 
> 
> Do you mean the hash function of signing hibernation image changed, so the
> hibernation key also need to re-generate for new length?
 
Yeah, that kind of thing.

> I will add a field in struct to forward the length of hibernation key variable.
> In the future kernel can check the length to handle the change.

Would it also make sense to explicitly record the hash function used
as well as the length?

-- 
Matt Fleming, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/