mbox series

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

Message ID 20180801234042.6740-1-erichte@linux.ibm.com
Headers show
Series Initial Implementation of Secure Boot Key Management support | expand

Message

Eric Richter Aug. 1, 2018, 11:40 p.m. UTC
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.

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.

Patches 1-3 contain simple background fixes and dependencies for the
following commits. Patches 4,5 contain the basic implementation of the
structures and initialization process. Patches 6-10 each implement a
single runtime service, each including the basic idea and usage in the
patch description.

Claudio Carvalho (3):
  libstb/container.h: add stdbool.h and short_types.h headers
  core/flash.c: add SECBOOT read and write support
  libstb: initialize and format pnor secboot partition

Eric Richter (7):
  opal-api: add values for secboot keystore management
  keystore: initialize the base keystore structure
  keystore: add opal_get_variable runtime service
  keystore: add opal_set_variable runtime service
  keystore: add opal_get_next_variable runtime service
  keystore: add opal_secboot_commit runtime service
  keystore: add experimental opal_lock_variables runtime service

 core/flash.c          | 118 ++++++++++++++++
 include/opal-api.h    |   7 +-
 include/platform.h    |   4 +
 libstb/Makefile.inc   |   2 +-
 libstb/container.h    |   2 +
 libstb/keystore.c     | 296 +++++++++++++++++++++++++++++++++++++++
 libstb/keystore.h     |  57 ++++++++
 libstb/secboot_part.c | 373 ++++++++++++++++++++++++++++++++++++++++++++++++++
 libstb/secboot_part.h |  24 ++++
 libstb/secureboot.c   |   4 +
 10 files changed, 885 insertions(+), 2 deletions(-)
 create mode 100644 libstb/keystore.c
 create mode 100644 libstb/keystore.h
 create mode 100644 libstb/secboot_part.c
 create mode 100644 libstb/secboot_part.h

Comments

Stewart Smith March 4, 2019, 4:52 a.m. UTC | #1
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