From: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com>
To: Dionna Amalie Glaze <dionnaglaze@google.com>,
Ard Biesheuvel <ardb@kernel.org>
Cc: Qinkun Bao <qinkun@google.com>,
linux-efi@vger.kernel.org,
Ilias Apalodimas <ilias.apalodimas@linaro.org>
Subject: Re: [PATCH 3/4] efi/libstub: Measure into CC protocol if TCG2 protocol is absent
Date: Tue, 5 Mar 2024 10:33:05 -0800 [thread overview]
Message-ID: <2f2416e0-265b-4f03-abf5-fb29c1a0e61b@linux.intel.com> (raw)
In-Reply-To: <CAAH4kHb7ZgfrQQjVr-JiutQthwkbrVGR2DkUDE=E0ZoWGO34eA@mail.gmail.com>
On 3/5/24 10:00 AM, Dionna Amalie Glaze wrote:
> On Tue, Mar 5, 2024 at 9:47 AM Ard Biesheuvel <ardb@kernel.org> wrote:
>> On Tue, 5 Mar 2024 at 18:34, Dionna Amalie Glaze <dionnaglaze@google.com> wrote:
>>> On Mon, Mar 4, 2024 at 2:44 AM Ard Biesheuvel <ardb+git@google.com> wrote:
>>>> From: Ard Biesheuvel <ardb@kernel.org>
>>>>
>>>> To accommodate confidential compute VMs that expose the simplified CC
>>>> measurement protocol instead of the full-blown TCG2 one, fall back to
>>>> the former if the latter does not exist.
>>>>
>>>> The CC protocol was designed to be used in this manner, which is why the
>>>> types and prototypes have been kept the same where possible. So reuse
>>>> the existing code, and only deviate from the TCG2 code path where
>>>> needed.
>>>>
>>>> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
>>>> ---
>>>> drivers/firmware/efi/libstub/efi-stub-helper.c | 67 +++++++++++++++-----
>>>> drivers/firmware/efi/libstub/efistub.h | 3 +
>>>> 2 files changed, 53 insertions(+), 17 deletions(-)
>>>>
>>>> diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c
>>>> index 0dbc9d3f4abd..21f4567324f6 100644
>>>> --- a/drivers/firmware/efi/libstub/efi-stub-helper.c
>>>> +++ b/drivers/firmware/efi/libstub/efi-stub-helper.c
>>>> @@ -223,44 +223,77 @@ static efi_status_t efi_measure_tagged_event(unsigned long load_addr,
>>>> unsigned long load_size,
>>>> enum efistub_event_type event)
>>>> {
>>>> + union {
>>>> + efi_status_t
>>>> + (__efiapi *hash_log_extend_event)(void *, u64, efi_physical_addr_t,
>>>> + u64, const union efistub_event *);
>>>> + struct { u32 hash_log_extend_event; } mixed_mode;
>>>> + } method;
>>>> struct efistub_measured_event *evt;
>>>> int size = struct_size(evt, tagged_event_data,
>>>> events[event].event_data_len);
>>>> efi_guid_t tcg2_guid = EFI_TCG2_PROTOCOL_GUID;
>>>> efi_tcg2_protocol_t *tcg2 = NULL;
>>>> + union efistub_event ev;
>>>> efi_status_t status;
>>>> + void *protocol;
>>>>
>>>> efi_bs_call(locate_protocol, &tcg2_guid, NULL, (void **)&tcg2);
>>>> if (tcg2) {
>>>> - status = efi_bs_call(allocate_pool, EFI_LOADER_DATA, size,
>>>> - (void **)&evt);
>>>> - if (status != EFI_SUCCESS)
>>>> - goto fail;
>>>> -
>>>> - evt->event_data.tcg2_data = (struct efi_tcg2_event){
>>>> + ev.tcg2_data = (struct efi_tcg2_event){
>>>> .event_size = size,
>>>> - .event_header.header_size = sizeof(evt->event_data.tcg2_data.event_header),
>>>> + .event_header.header_size = sizeof(ev.tcg2_data.event_header),
>>>> .event_header.header_version = EFI_TCG2_EVENT_HEADER_VERSION,
>>>> .event_header.pcr_index = events[event].pcr_index,
>>>> .event_header.event_type = EV_EVENT_TAG,
>>>> };
>>>> + protocol = tcg2;
>>>> + method.hash_log_extend_event =
>>>> + (void *)efi_table_attr(tcg2, hash_log_extend_event);
>>>> + } else {
>>> +Qinkun Bao
>>> Given that the exclusive or between CC_MEASUREMENT_PROTOCOL and the
>>> TCG protocol breaks backwards compatibility, it'd be preferable to
>>> measure into all the measurement protocols that are present.
>> How so? Older kernels will use TCG2 if it exists, and so will new
>> kernels. The only difference is that on new kernels, the CC protocol
>> will be used in case TCG2 is not implemented.
>>
>> So the only affected scenario here is a system that today implements
>> TCG but not CC, and intends to implement CC later and receive
>> measurements into both protocols. Does that really qualify as backward
>> compatibility? I'd rather not accommodate future systems that
>> implement something that the UEFI spec says they should not.
>>
>>> The UEFI
>>> v2.10 standard says that firmware "should not" provide both, but it is
>>> not MUST NOT. Given this and our desire to provide service continuity,
>>> I ask that you remove the "else" guard.
>>>
>> Ignoring the newer protocol if the established one exists is an
>> excellent way of making sure this does not happen.
>>
> The problem is that the protocols are not equivalent, and we disagree
> with the standard's claim of "should not" to the point that we're
> taking it to the USWG. The "should not" advisement is predicated on
> not trusting boot layers to use both protocols when they're both
> present, such that you could accidentally miss measuring a
> security-critical event. That's a strawman though, since you already
> need to develop trust in those boot layers. We have software supply
> chain endorsements for tracking that kind of property for use in
> attestation verification.
>
> The CC protocol is useful for hardware-rooted boot measurement, but it
> does nothing about the rest of TPM 2.0. There are plenty of users that
> want to use a vTPM that's hosted by the VMM but also get an extra
> integrity assurance that measurements into TDX RTMRs and attested by
> an Intel-rooted key pass an extra level of scrutiny.
>
If you check the EDK2 part of this support, it also uses if else model.
It does not measure both. If there a complete vTPM support, why
can't user trust measurements from it? I think the CC vendors will
ensure their vTPM implementation is protected from attack from the
host (like implementing it part of firmware or launching it as service in
a separate VM).
>>>> + efi_guid_t cc_guid = EFI_CC_MEASUREMENT_PROTOCOL_GUID;
>>>> + efi_cc_protocol_t *cc = NULL;
>>>>
>>>> - evt->tagged_event_id = events[event].event_id;
>>>> - evt->tagged_event_data_size = events[event].event_data_len;
>>>> -
>>>> - memcpy(evt->tagged_event_data, events[event].event_data,
>>>> - events[event].event_data_len);
>>>> + efi_bs_call(locate_protocol, &cc_guid, NULL, (void **)&cc);
>>>> + if (!cc)
>>>> + return EFI_UNSUPPORTED;
>>>>
>>>> - status = efi_call_proto(tcg2, hash_log_extend_event, 0,
>>>> - load_addr, load_size, &evt->event_data.tcg2_data);
>>>> - efi_bs_call(free_pool, evt);
>>>> + ev.cc_data = (struct efi_cc_event){
>>>> + .event_size = size,
>>>> + .event_header.header_size = sizeof(ev.cc_data.event_header),
>>>> + .event_header.header_version = EFI_CC_EVENT_HEADER_VERSION,
>>>> + .event_header.event_type = EV_EVENT_TAG,
>>>> + };
>>>>
>>>> + status = efi_call_proto(cc, map_pcr_to_mr_index,
>>>> + events[event].pcr_index,
>>>> + &ev.cc_data.event_header.mr_index);
>>>> if (status != EFI_SUCCESS)
>>>> goto fail;
>>>> - return EFI_SUCCESS;
>>>> +
>>>> + protocol = cc;
>>>> + method.hash_log_extend_event =
>>>> + (void *)efi_table_attr(cc, hash_log_extend_event);
>>>> }
>>>>
>>>> - return EFI_UNSUPPORTED;
>>>> + status = efi_bs_call(allocate_pool, EFI_LOADER_DATA, size, (void **)&evt);
>>>> + if (status != EFI_SUCCESS)
>>>> + goto fail;
>>>> +
>>>> + evt->event_data = ev;
>>>> + evt->tagged_event_id = events[event].event_id;
>>>> + evt->tagged_event_data_size = events[event].event_data_len;
>>>> +
>>>> + memcpy(evt->tagged_event_data, events[event].event_data,
>>>> + events[event].event_data_len);
>>>> +
>>>> + status = efi_fn_call(&method, hash_log_extend_event, protocol, 0,
>>>> + load_addr, load_size, &evt->event_data);
>>>> + efi_bs_call(free_pool, evt);
>>>> +
>>>> + if (status == EFI_SUCCESS)
>>>> + return EFI_SUCCESS;
>>>> +
>>>> fail:
>>>> efi_warn("Failed to measure data for event %d: 0x%lx\n", event, status);
>>>> return status;
>>>> diff --git a/drivers/firmware/efi/libstub/efistub.h b/drivers/firmware/efi/libstub/efistub.h
>>>> index d621bfb719c4..4bf9a76796b7 100644
>>>> --- a/drivers/firmware/efi/libstub/efistub.h
>>>> +++ b/drivers/firmware/efi/libstub/efistub.h
>>>> @@ -954,8 +954,11 @@ union efi_cc_protocol {
>>>> } mixed_mode;
>>>> };
>>>>
>>>> +static_assert(sizeof(efi_tcg2_event_t) == sizeof(efi_cc_event_t));
>>>> +
>>>> union efistub_event {
>>>> efi_tcg2_event_t tcg2_data;
>>>> + efi_cc_event_t cc_data;
>>>> };
>>>>
>>>> struct efistub_measured_event {
>>>> --
>>>> 2.44.0.278.ge034bb2e1d-goog
>>>>
>>>>
>>>
>>> --
>>> -Dionna Glaze, PhD (she/her)
>
>
--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer
next prev parent reply other threads:[~2024-03-05 18:33 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-04 10:44 [PATCH 0/4] efi/libstub: Fall back to CC proto for measurement Ard Biesheuvel
2024-03-04 10:44 ` [PATCH 1/4] efi/libstub: Fold efi_tcg2_tagged_event into efi_measured_event Ard Biesheuvel
2024-03-05 4:30 ` Kuppuswamy Sathyanarayanan
2024-03-05 8:21 ` Ard Biesheuvel
2024-03-05 19:19 ` Kuppuswamy Sathyanarayanan
2024-03-04 10:44 ` [PATCH 2/4] efi/libstub: Add Confidential Computing (CC) measurement typedefs Ard Biesheuvel
2024-03-05 18:00 ` Ilias Apalodimas
2024-03-05 19:27 ` Kuppuswamy Sathyanarayanan
2024-03-05 19:59 ` Ilias Apalodimas
2024-03-04 10:44 ` [PATCH 3/4] efi/libstub: Measure into CC protocol if TCG2 protocol is absent Ard Biesheuvel
2024-03-05 17:34 ` Dionna Amalie Glaze
2024-03-05 17:47 ` Ard Biesheuvel
2024-03-05 17:55 ` Ilias Apalodimas
2024-03-05 18:00 ` Dionna Amalie Glaze
2024-03-05 18:33 ` Kuppuswamy Sathyanarayanan [this message]
2024-03-05 18:46 ` Dionna Amalie Glaze
2024-03-05 19:36 ` Kuppuswamy Sathyanarayanan
2024-03-05 21:28 ` Dionna Amalie Glaze
2024-03-05 21:28 ` Ard Biesheuvel
2024-03-05 21:39 ` Kuppuswamy Sathyanarayanan
2024-03-04 10:44 ` [PATCH 4/4] efi/libstub: Add get_event_log() support for CC platforms Ard Biesheuvel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2f2416e0-265b-4f03-abf5-fb29c1a0e61b@linux.intel.com \
--to=sathyanarayanan.kuppuswamy@linux.intel.com \
--cc=ardb@kernel.org \
--cc=dionnaglaze@google.com \
--cc=ilias.apalodimas@linaro.org \
--cc=linux-efi@vger.kernel.org \
--cc=qinkun@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).