[Skiboot] [RFC PATCH RESEND 00/10] Initial Implementation of Secure Boot Key Management support

[Stewart Smith]

Sorry for the delay in looking closely at this patchset.

Eric Richter <erichte at linux.ibm.com> writes:
> Resending the whole set, since I apparently goofed up my list
> membership and only a few patches got through. Apologies for the noise.
>
> This patch set includes a very drafty implementation of the structures
> and runtime services for the management of the secure boot keys, used
> for OS secure boot. These features have seen numerous redesigns and
> reimplementations, and so we are posting them now to hopefully gather
> more feedback, suggestions, and recommendations as we continue to
> develop secure boot support for POWER.
>
> We are specifically looking for feedback on the general design of the
> API itself, as well as some of the higher level implementation details,
> such as allocated memory usage. Given that this is a draft, there are
> some hacky bits of code that are still slated for a rewrite, suggestions
> for cleaner reimplementations would also be appreciated.
>
> As it currently stands, the skiroot kernel will be handling most of the
> secure boot logic and enforcement, leaving the implementation in skiboot
> to be little more than a secure variable storage. Thus, this set only
> handles the operations necessary to load and store the variables in
> PNOR. For now, the secboot partition is not validated in any form, but
> future patch sets will require a hash of the partition to match that of
> a hash stored in TPM NV space. The TPM NV index will be locked after
> early skiroot, therefore the PNOR can not be modified without breaking
> this hash.

>From other discussions, I think the direction has (since this patchset)
changed a bit to now do more processing inside skiboot itself?

There needs to be a decent addition to doc/ on the model of
what's implemented and *why* it's implemented that way.

> The secboot partition is split into three major sections, two variable
> storage sections (one active and one back up), and a section for
> queueing variable updates, referred to as the "update queue". Since we
> cannot update the TPM hash until the next boot, we must process variable
> changes on boot.

This bit of code is going to need a good test suite to ensure
correctness, *especially* in the face of various forms of failure.

One point that Daniel raised when talking through this kind of design is
that with an A/B approach there is possibly an avenue for a downgrade
attack. We should clearly document the behaviour in all failure
scanarios.

Also, it'd be good to see documentation on the physical format of the
partition. Considering this is effectively a format that will live
"forever", we should get it right.


Additional comments on some of the other patches

-- 
Stewart Smith
OPAL Architect, IBM.