Hi Vivek

Please see my comment in-lined…

Alexei

From: Vivek Prasad <vivek.prasad@live.com>
Sent: 05 May 2020 05:37
To: Stuart Yoder <Stuart.Yoder@arm.com>; Alexei Fedorov <Alexei.Fedorov@arm.com>; tf-a <tf-a@lists.trustedfirmware.org>; Raghu Krishnamurthy <raghu.ncstate@icloud.com>
Cc: Vivek Kumar <vivek@amperecomputing.com>; Matteo Carlini <Matteo.Carlini@arm.com>; Samer El-Haj-Mahmoud <Samer.El-Haj-Mahmoud@arm.com>; Loc Ho <loc.ho@amperecomputing.com>; Charles Garcia-Tobin <Charles.Garcia-Tobin@arm.com>; Dong Wei <Dong.Wei@arm.com>; Mohamad Ammar <moe@amperecomputing.com>; bchaffin <bchaffin@amperecomputing.com>; Ard Biesheuvel <Ard.Biesheuvel@arm.com>; Joanna Farley <Joanna.Farley@arm.com>; Harb Abdulhamid (harb@amperecomputing.com) <harb@amperecomputing.com>; Vivek Prasad OS <vivek.prasad@os.amperecomputing.com>; Tai Nguyen <ttnguyen@amperecomputing.com>
Subject: Re: [TF-A] Proposal for Measured Boot Implementation

Hi Stuart,

Please see inline...

-Vivek

On 4/15/20 11:49 PM, Vivek Prasad wrote:
> Hello Stuart, Alexei,
>
>
> Chiming-in here on Ampere's behalf...
>
>
> We analysed this proposal internally. And we see a number issues with this, some of which was already raised by Raghu in the previous threads.
>
> Here is a summary of the main issues that we see.
>
>   * Only supporting mbedtls, and this is fixed config at compile time.
>       o We propose that there should be a variable for the algorithm to be
> used, which can be setup at initialization time.

My understanding is that TF-A supports a pluggable crypto
library design where other libraries besides mbedtls can
be used.

Regarding initialization time selection of the algorithm, I don't
know if there are existing examples of the boot ROM (BL1)
reading variable configuration data from flash.  But, if this can
be done sanely seems good.

[Vivek]>> Ageed. TF-A supports pluggable Crypto. However, the current design limits the usage of any other digest for measurement than what is being used for images. TPM measurement algorithm and image hash authentications are two separate activities and may use different digests. The current design limits this.

>   * This solution relies on taking the hash directly from the digest as
> the measurement, instead of the computed hash.  This is not safe,
> especially considering measured boot may use a different hash bank,
> so digest hash may not be correct/valid.

I'm not 100% I understand, but the hash of BL2 is computed.  This was added
in a separate patch (see 0ab496458b).  There is a function arm_bl1_set_bl2_hash()
where BL1 computes the BL2 hash.  It is then passed to BL2 via tb_fw_config.

>   * Only measuring the BL2 image, per the ARM SBSG we must be measuring and logging
>   **all** images/boot phases
>       o BL31
>       o BL32 (all secure partitions)
>       o BL33 (UEFI or any other non-secure boot loader)
>           + Once we ERET into BL33, the measure boot flow continues and is owned> by that boot loader

The BL2 code does measure all other loaded images.  See
fvp_bl2_plat_handle_post_image_load() in fvp_bl2_setup.c.

[Vivek] >> I don’t see BL2 measuring images as part of this merge. https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/3555
Authentication is not same as measurement, we need to perform a hash-log-extend every time a new hash is being computed.  Where is the actual measurement to the PCR0 happening?

[Alexei]>> BL2 measurements are part of https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/3806 patch.

>   * Only see support for PCR0, any/all unsigned config data must be logged to PCR1.

Yes, agree need to support PCR1 as well.

>   * Passing PCRs to non-secure software before logging is not compliant with TCG
>   Static-Root-of-Trust Measurement (SRTM) requirements
>       o It was discussed before in separate conversations… especially in
>         systems where you are talked about two different signing domains where BL33
>         is a different trust/signing domain.
>       o BL33 should only do hash-log-extend… there is no need for BL33 to
>         be aware of the current PCR value (beyond what is provided in the boot event
>         log).

The current patches pass the event log to BL33, but that's it.  They currently
assume that the secure world will write the TPM PCRs.

>   * Based on comments on the mail thread, there seem to be bad
>     assumptions/expectations around TPM accessibility from non-secure world.
>       o Expecting SPI/I2C TPMs to be directly accessed from non-secure
>         world instead of abstracting hardware details via the TCG CRB interface>         (which has been already standardized as the defacto mechanism for ARM on
>         past mobile, client, and server solutions).
>       o CRB will "just work" for Aptio/EDK2/Linux/Windows/Hyper-V/VMWare
>       o NOTE: This goes back to what is a “productizable” TPM solution.
>         We want it to be turn-key solution for customers without having to
>         support/develop proprietary drivers.

I don't think the patches assume TPM accessibility from normal world.  They
pass the measurements to the secure world (BL32) via tb_fw_config so
they can be replayed into the TPM.

Any kind of secure world TPM service should be based on CRB.

But, we don't want to preclude the normal world owning the TPM for
some use cases.

Thanks,
Stuart

---

From: Stuart Yoder <stuart.yoder@arm.com>
Sent: Friday, April 17, 2020 12:15 AM
To: Vivek Prasad <vivek.prasad@live.com>; Alexei Fedorov <Alexei.Fedorov@arm.com>; tf-a <tf-a@lists.trustedfirmware.org>; Raghu Krishnamurthy <raghu.ncstate@icloud.com>
Cc: Vivek Kumar <vivek@amperecomputing.com>; Matteo Carlini <Matteo.Carlini@arm.com>; Samer El-Haj-Mahmoud <samer.el-haj-mahmoud@arm.com>; Loc Ho <loc.ho@amperecomputing.com>; Charles Garcia-Tobin <Charles.Garcia-Tobin@arm.com>; Dong Wei <Dong.Wei@arm.com>; Mohamad Ammar <moe@amperecomputing.com>; Benjamin Chaffin <bchaffin@amperecomputing.com>; Ard Biesheuvel <Ard.Biesheuvel@arm.com>; Joanna Farley <Joanna.Farley@arm.com>
Subject: Re: [TF-A] Proposal for Measured Boot Implementation

On 4/15/20 11:49 PM, Vivek Prasad wrote:
> Hello Stuart, Alexei,
>
>
> Chiming-in here on Ampere's behalf...
>
>
> We analysed this proposal internally. And we see a number issues with this, some of which was already raised by Raghu in the previous threads.
>
> Here is a summary of the main issues that we see.
>
>   * Only supporting mbedtls, and this is fixed config at compile time.
>       o We propose that there should be a variable for the algorithm to be
> used, which can be setup at initialization time.

My understanding is that TF-A supports a pluggable crypto
library design where other libraries besides mbedtls can
be used.

Regarding initialization time selection of the algorithm, I don't
know if there are existing examples of the boot ROM (BL1)
reading variable configuration data from flash.  But, if this can
be done sanely seems good.

>   * This solution relies on taking the hash directly from the digest as
> the measurement, instead of the computed hash.  This is not safe,
> especially considering measured boot may use a different hash bank,
> so digest hash may not be correct/valid.

I'm not 100% I understand, but the hash of BL2 is computed.  This was added
in a separate patch (see 0ab496458b).  There is a function arm_bl1_set_bl2_hash()
where BL1 computes the BL2 hash.  It is then passed to BL2 via tb_fw_config.

>   * Only measuring the BL2 image, per the ARM SBSG we must be measuring and logging
>   **all** images/boot phases
>       o BL31
>       o BL32 (all secure partitions)
>       o BL33 (UEFI or any other non-secure boot loader)
>           + Once we ERET into BL33, the measure boot flow continues and is owned> by that boot loader

The BL2 code does measure all other loaded images.  See
fvp_bl2_plat_handle_post_image_load() in fvp_bl2_setup.c.


>   * Only see support for PCR0, any/all unsigned config data must be logged to PCR1.

Yes, agree need to support PCR1 as well.

>   * Passing PCRs to non-secure software before logging is not compliant with TCG
>   Static-Root-of-Trust Measurement (SRTM) requirements
>       o It was discussed before in separate conversations… especially in
>         systems where you are talked about two different signing domains where BL33
>         is a different trust/signing domain.
>       o BL33 should only do hash-log-extend… there is no need for BL33 to
>         be aware of the current PCR value (beyond what is provided in the boot event
>         log).

The current patches pass the event log to BL33, but that's it.  They currently
assume that the secure world will write the TPM PCRs.

>   * Based on comments on the mail thread, there seem to be bad
>     assumptions/expectations around TPM accessibility from non-secure world.
>       o Expecting SPI/I2C TPMs to be directly accessed from non-secure
>         world instead of abstracting hardware details via the TCG CRB interface>         (which has been already standardized as the defacto mechanism for ARM on
>         past mobile, client, and server solutions).
>       o CRB will "just work" for Aptio/EDK2/Linux/Windows/Hyper-V/VMWare
>       o NOTE: This goes back to what is a “productizable” TPM solution.
>         We want it to be turn-key solution for customers without having to
>         support/develop proprietary drivers.

I don't think the patches assume TPM accessibility from normal world.  They
pass the measurements to the secure world (BL32) via tb_fw_config so
they can be replayed into the TPM.

Any kind of secure world TPM service should be based on CRB.

But, we don't want to preclude the normal world owning the TPM for
some use cases.

Thanks,
Stuart