[Skiboot] [PATCH 15/15] Add libstb

[Stewart Smith]

Claudio Carvalho <cclaudio at linux.vnet.ibm.com> writes:
> On 09/20/2016 05:39 AM, Stewart Smith wrote:
>> Claudio Carvalho <cclaudio at linux.vnet.ibm.com> writes:
>>> +/**
>>> + * This maps a PCR for each resource we can measure. The PCR number is
>>> + * mapped according to the TCG specs.
>>> + * Only resources included in this whitelist can be measured.
>>> + */
>>> +static struct {
>>> +
>>> +	/* PNOR partition id */
>>> +	enum resource_id id;
>>> +
>>> +	/* PCR mapping for the resource id */
>>> +	TPM_Pcr pcr;
>>> +
>>> +	/* Resource name */
>>> +	const char name[MAX_RESOURCE_NAME+1];
>>> +
>>> +} resource_map[] = {
>>> +	{ RESOURCE_ID_KERNEL, PCR_4, "BOOTKERNEL" },
>>> +	{ RESOURCE_ID_CAPP,   PCR_2, "CAPP"},
>>> +};
>> 
>> I wonder if this should be up in the platform code?
>
> A similar array, part_name_map[], is defined in core/flash.c. Maybe we
> can merge both structures and create an array somewhere that can be
> consumed by anyone just by adding an "extern array" in include/platform.h.
>
> Concern #1: in the current design, new records can be added only if we
> know to what TPM PCR the resource must be extended. Implications: for
> example, if we need to add a new record specific for FSP platforms in
> the array, the development of the FSP feature would need to count in
> some time to define the PCR, but secure and trusted boot are not
> supported by FSP platforms yet.

True, which is why I'd err on the side of assert() if we attempt to and
we're loading something where we don't have a known PCR.

> We can always do some refactoring in the structure and in the libstb
> code to resolve that (e.g. adding a field that indicates whether or not
> the resource should be verified/measured).

I can't think of a situation where we'd want to *not* measure something
we're loading.

> Concern #2: AFAIK, currently skiboot downloads external code only from
> PNOR. If we decide to download code from somewhere else should we create
> a new array for the new resources or can we use the same array?

Either using FSP mailbox commands or by directly accessing
flash (ignoring the mambo case where we do funky things with flash that
mikey has been playing with a bunch for p9).

> If we keep resource_map[] in stb.c, it seems that we will have more
> control on the array and the development of other skiboot parts will be
> less affected by it.

As long as the default action if the resource isn't mapped to a PCR is
to abort boot, I think I'm okay with it.

>> RESOURCE_ID_INITRAMFS, currently only used on FSP platforms.
>> 
>> Any thoughts on what would happen on a FSP platform with the additional
>> LID(s) we need loading? (vpd and hostservices).
>> 
>
> Libstb is platform independent, but platforms need to initialize it and
> properly call the libstb API to verify and measure all resources
> consumed by skiboot at boot time.
>
> Currently, my patches add support only for habanero, but we can easily
> change them to support all BMC platforms. We just need to initialize
> libstb for all BMC platforms instead, since all of them use core/flash.c
> to download resources from PNOR.
>
> Secure and trusted boot support for FSP platforms would be a separate
> patch set, assuming that it doesn't use core/flash.c to download
> resources from PNOR. Additionally, the PNOR partitions consumed by FSP
> platforms would have to be added to resource_map[] (assuming that the
> partition doesn't have dynamic content such as NVRAM which has only
> variable:value records).

If the core code could do teh measurement rather than teh core/flash.c
or FSP code, that would be better.


-- 
Stewart Smith
OPAL Architect, IBM.