Message ID | 20180801234042.6740-1-erichte@linux.ibm.com |
---|---|
Headers | show |
Series | Initial Implementation of Secure Boot Key Management support | expand |
Sorry for the delay in looking closely at this patchset. Eric Richter <erichte@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