Re: [edk2-devel] [PATCH V4 3/3] SecurityPkg: Support CcMeasurementProtocol in DxeTpmMeasurementLib

["Sami Mujawar" <sami.mujawar@arm.com>]

Hi Min,

Please find my response inline marked [SAMI].

Regards,

Sami Mujawar


On 04/11/2021 01:49 PM, Xu, Min M wrote:
> On November 4, 2021 9:35 PM, Xu Min wrote:
>> On November 4, 2021 4:21 PM, Gerd Hoffmann wrote:
>>>    Hi,
>>>
>>>> [SAMI] Apologies, I missed this in my previous review. I think the
>>>> behaviour if both the TCG2 and CC measurement protocols are
>>>> installed would be inconsistent between DxeTpmMeasurementLib and
>>>> DxeTpm2MeasureBootLib. The main difference being in the later, the
>>>> TCG2 protocol takes precedence for extending the measurement.
>>> Yes, we should have consistent behavior in both cases.
>> In DxeTpmMeasurementLib, Cc measurement protocol is used as the first try. If
>> it fails, then it try to measure with TCG2 / TCG protocol in turn.
>> In DxeTpm2MeasureBootLib, TCG2 protocol is used the as the first try. If it fails,
>> CC measurement protocol is tried in turn.
>> Yes, this is inconsistent. I will update DxeTpm2MeasureBootLib to try Cc
>> measurement protocol first, then try TCG2 protocol if Cc measurement protocol
>> fails. In this way, only one protocol will be called to do the measurement. But
>> TCG2 protocol is the first try, CC measurement protocol is the second try.
>>
>>>> I think it would be good to modify DxeTpm2MeasureBootLib so that the
>>>> CC measurement protocol is used if both protocols are installed.
>>>> What do you think?
>>> Does it makes sense to use both protocols?
>> Agree with Gerd. I don't think we should use both protocols to do the
>> measurement.
>> My suggestion is that, first try CC protocol, if it fails, then try TCG2 protocol. Just
>> as I explained above.
> Another option will be that:
> In DxeTpmMeasurementLib the pseudo would look like:
> If (CC Protocol is installed) {
>    Status = CcMeasureAndLogData (...)
> } else {  // below is the original code
>   Status = Tpm20MeasureAndLogData (...)
>   If (EFI_ERROR (Status)) {
>      Status = Tpm12MeasureAndLogData (...)
>   }
> }
>
> In DxeTpm2MeasureBootLib, the pseudo would look like:
> If (CC Protocol is installed) {
>      Status = DoCcMeasureBoot(...)
> } else if (TCG2 protocol is installed) {
>      Status = DoTcg2MeasureBoot(...)
> }
[SAMI] Your pseudo code looks good to me. It makes the measurement logic 
much clearer.
  Also, I am not aware if there is a use-case for both the CC Protocol 
and the TCG2 protocols to be installed at the same time.
[/SAMI]
> Sami & Gerd
> What's your thougth?
>
> Thanks
> Min