efi/libstub: Fall back to CC proto for measurement
cover efi/libstub: Fall back to CC proto for measurement
From: Ard Biesheuvel <[email protected]>
This is a follow-up to Kuppuswamy's series [0] to add TDX based measurement of the initrd and command line to the EFI stub.
In view of time wrt the merge window, I've taken the liberty of applying my review feedback to the code directly, rather than going back and forth with the author to converge on something we can both agree on.
[0] https://lore.kernel.org/all/20240215030002.281456-1-sathyanarayanan.kuppuswamy@linux.intel.com/
Ard Biesheuvel (2):
efi/libstub: Fold efi_tcg2_tagged_event into efi_measured_event efi/libstub: Measure into CC protocol if TCG2 protocol is absent
Kuppuswamy Sathyanarayanan (2):
efi/libstub: Add Confidential Computing (CC) measurement typedefs efi/libstub: Add get_event_log() support for CC platforms
drivers/firmware/efi/libstub/efi-stub-helper.c | 83 ++++++++++------ drivers/firmware/efi/libstub/efi-stub.c | 2 +- drivers/firmware/efi/libstub/efistub.h | 104 ++++++++++++++++++-- drivers/firmware/efi/libstub/tpm.c | 74 +++++++++----- drivers/firmware/efi/libstub/x86-stub.c | 2 +- include/linux/efi.h | 4 + 6 files changed, 206 insertions(+), 63 deletions(-)
Signature · 1 line
2.44.0.278.ge034bb2e1d-goog
PATCH 1/4 efi/libstub: Fold efi_tcg2_tagged_event into efi_measured_event
From: Ard Biesheuvel <[email protected]>
In spite of the efi_ prefix, struct efi_tcg2_tagged_event is specific to the EFI stub, and so we can tweak it to our liking if needed, e.g., to accommodate the TDX variant of the TCG2 measurement protocol.
In preparation for that, get rid of it entirely, and combine it with the efi_measured_event struct used by the measurement code.
--- drivers/firmware/efi/libstub/efi-stub-helper.c | 26 ++++++++------------ drivers/firmware/efi/libstub/efistub.h | 18 ++++++++------ 2 files changed, 21 insertions(+), 23 deletions(-)
drivers/firmware/efi/libstub/efi-stub-helper.c +10 -16
@@ -193,7 +193,7 @@ void efi_apply_loadoptions_quirk(const void **load_options, u32 *load_options_si *load_options_size = load_option_unpacked.optional_data_size; } -enum efistub_event { +enum efistub_event_type {EFISTUB_EVT_INITRD ,EFISTUB_EVT_LOAD_OPTIONS ,EFISTUB_EVT_COUNT ,
@@ -221,44 +221,38 @@ static const struct {static efi_status_t efi_measure_tagged_event (unsigned long load_addr,unsigned long load_size, -enum efistub_event event) +enum efistub_event_type event) { +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 ;efi_status_t status;efi_bs_call (locate_protocol, &tcg2_guid,NULL , (void **)&tcg2);if (tcg2) { -struct efi_measured_event { -efi_tcg2_event_t event_data; -efi_tcg2_tagged_event_t tagged_event; -u8 tagged_event_data[]; - } *evt; -int size =sizeof (*evt) + events[event].event_data_len; - status =efi_bs_call (allocate_pool,EFI_LOADER_DATA , size, (void **)&evt);if (status !=EFI_SUCCESS ) goto fail; - evt->event_data = (struct efi_tcg2_event ){ + evt->event_data.tcg2_data = (struct efi_tcg2_event ){ .event_size = size, - .event_header.header_size =sizeof (evt->event_data.event_header), + .event_header.header_size =sizeof (evt->event_data.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 , }; - evt->tagged_event = (struct efi_tcg2_tagged_event ){ - .tagged_event_id = events[event].event_id, - .tagged_event_data_size = events[event].event_data_len, - }; + 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_call_proto (tcg2, hash_log_extend_event,0 , - load_addr, load_size, &evt->event_data); + load_addr, load_size, &evt->event_data.tcg2_data);efi_bs_call (free_pool, evt);if (status !=EFI_SUCCESS )
drivers/firmware/efi/libstub/efistub.h +11 -7
@@ -843,14 +843,7 @@ struct efi_tcg2_event {/* u8[] event follows here */ } __packed; -struct efi_tcg2_tagged_event { -u32 tagged_event_id; -u32 tagged_event_data_size; -/* u8 tagged event data follows here */ -} __packed; -typedef struct efi_tcg2_event efi_tcg2_event_t ; -typedef struct efi_tcg2_tagged_event efi_tcg2_tagged_event_t ;typedef union efi_tcg2_protocol efi_tcg2_protocol_t ;union efi_tcg2_protocol {
@@ -882,6 +875,17 @@ union efi_tcg2_protocol { } mixed_mode; }; +union efistub_event { +efi_tcg2_event_t tcg2_data; +}; + +struct efistub_measured_event { +union efistub_event event_data; +u32 tagged_event_id; +u32 tagged_event_data_size; +u8 tagged_event_data[]; +} __packed; +struct riscv_efi_boot_protocol {u64 revision;
Signature · 1 line
2.44.0.278.ge034bb2e1d-goog
Re: [PATCH 1/4] efi/libstub: Fold efi_tcg2_tagged_event into efi_measured_event
Quoted 29 lines · Ard Biesheuvel
From: Ard Biesheuvel <[email protected]>
In spite of the
efi_prefix, structefi_tcg2_tagged_eventis specific to the EFI stub, and so we can tweak it to our liking if needed, e.g., to accommodate the TDX variant of the TCG2 measurement protocol.In preparation for that, get rid of it entirely, and combine it with the
efi_measured_eventstruct used by the measurement code.Signed-off-by Ard Biesheuvel <[email protected]>--- drivers/firmware/efi/libstub/efi-stub-helper.c | 26 ++++++++------------ drivers/firmware/efi/libstub/efistub.h | 18 ++++++++------ 2 files changed, 21 insertions(+), 23 deletions(-)drivers/firmware/efi/libstub/efi-stub-helper.c +1
-1@@ -193,7 +193,7 @@ void efi_apply_loadoptions_quirk(const void **load_options, u32 *load_options_si *load_options_size = load_option_unpacked.optional_data_size; } -enum efistub_event { +enum efistub_event_type {EFISTUB_EVT_INITRD ,EFISTUB_EVT_LOAD_OPTIONS ,EFISTUB_EVT_COUNT ,
drivers/firmware/efi/libstub/efi-stub-helper.c static const struct { · line 228
Quoted 9 lines
drivers/firmware/efi/libstub/efi-stub-helper.c +4
-1@@ -221,44 +221,38 @@ static const struct {static efi_status_t efi_measure_tagged_event (unsigned long load_addr,unsigned long load_size, -enum efistub_event event) +enum efistub_event_type event) { +struct efistub_measured_event *evt; +int size =struct_size (evt, tagged_event_data, + events[event].event_data_len);
Include linux/overflow.h explicitly?
Quoted 5 lines
drivers/firmware/efi/libstub/efi-stub-helper.c +0
-0efi_guid_t tcg2_guid =EFI_TCG2_PROTOCOL_GUID ;efi_tcg2_protocol_t *tcg2 =NULL ;efi_status_t status;efi_bs_call (locate_protocol, &tcg2_guid,NULL , (void **)&tcg2);
drivers/firmware/efi/libstub/efi-stub-helper.c static const struct { · line 236
Quoted 10 lines
drivers/firmware/efi/libstub/efi-stub-helper.c +0
-7if (tcg2) { -struct efi_measured_event { -efi_tcg2_event_t event_data; -efi_tcg2_tagged_event_t tagged_event; -u8 tagged_event_data[]; - } *evt; -int size =sizeof (*evt) + events[event].event_data_len; - status =efi_bs_call (allocate_pool,EFI_LOADER_DATA , size, (void **)&evt);
It looks like in patch 3 you have converted evt as stack variable. Since that change is not specific to CC fallback, can it be moved here?
Quoted 53 lines
drivers/firmware/efi/libstub/efi-stub-helper.c +5
-7if (status !=EFI_SUCCESS ) goto fail; - evt->event_data = (struct efi_tcg2_event ){ + evt->event_data.tcg2_data = (struct efi_tcg2_event ){ .event_size = size, - .event_header.header_size =sizeof (evt->event_data.event_header), + .event_header.header_size =sizeof (evt->event_data.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 , }; - evt->tagged_event = (struct efi_tcg2_tagged_event ){ - .tagged_event_id = events[event].event_id, - .tagged_event_data_size = events[event].event_data_len, - }; + 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_call_proto (tcg2, hash_log_extend_event,0 , - load_addr, load_size, &evt->event_data); + load_addr, load_size, &evt->event_data.tcg2_data);efi_bs_call (free_pool, evt);if (status !=EFI_SUCCESS )drivers/firmware/efi/libstub/efistub.h +1
-7@@ -843,14 +843,7 @@ struct efi_tcg2_event {/* u8[] event follows here */ } __packed; -struct efi_tcg2_tagged_event { -u32 tagged_event_id; -u32 tagged_event_data_size; -/* u8 tagged event data follows here */ -} __packed; -typedef struct efi_tcg2_event efi_tcg2_event_t ; -typedef struct efi_tcg2_tagged_event efi_tcg2_tagged_event_t ;typedef union efi_tcg2_protocol efi_tcg2_protocol_t ;union efi_tcg2_protocol {@@ -882,6 +875,17 @@ union efi_tcg2_protocol { } mixed_mode; }; +union efistub_event {
drivers/firmware/efi/libstub/efistub.h union efi_tcg2_protocol { · line 888
Quoted 10 lines
drivers/firmware/efi/libstub/efistub.h +10
-0+efi_tcg2_event_t tcg2_data; +}; + +struct efistub_measured_event { +union efistub_event event_data; +u32 tagged_event_id; +u32 tagged_event_data_size; +u8 tagged_event_data[]; +} __packed; +
Since efistub_measured_event is only used efi-stub-helper.c, why not leave it there?
Quoted 3 lines
drivers/firmware/efi/libstub/efistub.h +0
-0struct riscv_efi_boot_protocol {u64 revision;
Signature · 2 lines
Sathyanarayanan Kuppuswamy Linux Kernel Developer
Ard Biesheuvel
maintainerRe: [PATCH 1/4] efi/libstub: Fold efi_tcg2_tagged_event into efi_measured_event
Quoted 33 lines · Kuppuswamy Sathyanarayanan
Quoted 30 lines · Ard Biesheuvel
From: Ard Biesheuvel <[email protected]>
In spite of the
efi_prefix, structefi_tcg2_tagged_eventis specific to the EFI stub, and so we can tweak it to our liking if needed, e.g., to accommodate the TDX variant of the TCG2 measurement protocol.In preparation for that, get rid of it entirely, and combine it with the
efi_measured_eventstruct used by the measurement code.Signed-off-by Ard Biesheuvel <[email protected]>--- drivers/firmware/efi/libstub/efi-stub-helper.c | 26 ++++++++------------ drivers/firmware/efi/libstub/efistub.h | 18 ++++++++------ 2 files changed, 21 insertions(+), 23 deletions(-)drivers/firmware/efi/libstub/efi-stub-helper.c +1
-1@@ -193,7 +193,7 @@ void efi_apply_loadoptions_quirk(const void **load_options, u32 *load_options_si *load_options_size = load_option_unpacked.optional_data_size; } -enum efistub_event { +enum efistub_event_type {EFISTUB_EVT_INITRD ,EFISTUB_EVT_LOAD_OPTIONS ,EFISTUB_EVT_COUNT ,@@ -221,44 +221,38 @@ static const struct {
Quoted 11 lines
drivers/firmware/efi/libstub/efi-stub-helper.c static const struct { · line 228
Quoted 8 lines
drivers/firmware/efi/libstub/efi-stub-helper.c +4
-1static efi_status_t efi_measure_tagged_event (unsigned long load_addr,unsigned long load_size, -enum efistub_event event) +enum efistub_event_type event) { +struct efistub_measured_event *evt; +int size =struct_size (evt, tagged_event_data, + events[event].event_data_len);Include
linux/overflow.hexplicitly?
Yes, good point.
Quoted 16 lines
Quoted 5 lines
drivers/firmware/efi/libstub/efi-stub-helper.c +0
-0efi_guid_t tcg2_guid =EFI_TCG2_PROTOCOL_GUID ;efi_tcg2_protocol_t *tcg2 =NULL ;efi_status_t status;efi_bs_call (locate_protocol, &tcg2_guid,NULL , (void **)&tcg2);drivers/firmware/efi/libstub/efi-stub-helper.c static const struct { · line 236
Quoted 10 lines
drivers/firmware/efi/libstub/efi-stub-helper.c +0
-7if (tcg2) { -struct efi_measured_event { -efi_tcg2_event_t event_data; -efi_tcg2_tagged_event_t tagged_event; -u8 tagged_event_data[]; - } *evt; -int size =sizeof (*evt) + events[event].event_data_len; - status =efi_bs_call (allocate_pool,EFI_LOADER_DATA , size, (void **)&evt);
Quoted 3 lines
It looks like in patch 3 you have converted evt as stack variable. Since that change is not specific to CC fallback, can it be moved here?
Not sure what you mean here. evt is still there after parch #3
Quoted 52 lines
Quoted 52 lines
drivers/firmware/efi/libstub/efi-stub-helper.c +5
-7if (status !=EFI_SUCCESS ) goto fail; - evt->event_data = (struct efi_tcg2_event ){ + evt->event_data.tcg2_data = (struct efi_tcg2_event ){ .event_size = size, - .event_header.header_size =sizeof (evt->event_data.event_header), + .event_header.header_size =sizeof (evt->event_data.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 , }; - evt->tagged_event = (struct efi_tcg2_tagged_event ){ - .tagged_event_id = events[event].event_id, - .tagged_event_data_size = events[event].event_data_len, - }; + 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_call_proto (tcg2, hash_log_extend_event,0 , - load_addr, load_size, &evt->event_data); + load_addr, load_size, &evt->event_data.tcg2_data);efi_bs_call (free_pool, evt);if (status !=EFI_SUCCESS )drivers/firmware/efi/libstub/efistub.h +0
-7@@ -843,14 +843,7 @@ struct efi_tcg2_event {/* u8[] event follows here */ } __packed; -struct efi_tcg2_tagged_event { -u32 tagged_event_id; -u32 tagged_event_data_size; -/* u8 tagged event data follows here */ -} __packed; -typedef struct efi_tcg2_event efi_tcg2_event_t ; -typedef struct efi_tcg2_tagged_event efi_tcg2_tagged_event_t ;typedef union efi_tcg2_protocol efi_tcg2_protocol_t ;union efi_tcg2_protocol {@@ -882,6 +875,17 @@ union efi_tcg2_protocol { } mixed_mode; };
Quoted 15 lines
drivers/firmware/efi/libstub/efistub.h union efi_tcg2_protocol { · line 888
Quoted 11 lines
drivers/firmware/efi/libstub/efistub.h +11
-0+union efistub_event { +efi_tcg2_event_t tcg2_data; +}; + +struct efistub_measured_event { +union efistub_event event_data; +u32 tagged_event_id; +u32 tagged_event_data_size; +u8 tagged_event_data[]; +} __packed; +Since
efistub_measured_eventis only usedefi-stub-helper.c, why not leave it there?
Indeed. I will move it back.
Kuppuswamy Sathyanarayanan
Reviewed-byRe: [PATCH 1/4] efi/libstub: Fold efi_tcg2_tagged_event into efi_measured_event
Quoted 10 lines · Ard Biesheuvel
Quoted 8 lines · Kuppuswamy Sathyanarayanan
Quoted 6 lines · Ard Biesheuvel
From: Ard Biesheuvel <[email protected]>
In spite of the
efi_prefix, structefi_tcg2_tagged_eventis specific to the EFI stub, and so we can tweak it to our liking if needed, e.g., to accommodate the TDX variant of the TCG2 measurement protocol.
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines
In preparation for that, get rid of it entirely, and combine it with the
efi_measured_eventstruct used by the measurement code.Signed-off-by Ard Biesheuvel <[email protected]>---
With nits fixed,
Quoted 46 lines
Quoted 19 lines
Quoted 19 lines
drivers/firmware/efi/libstub/efi-stub-helper.c | 26 ++++++++------------ drivers/firmware/efi/libstub/efistub.h | 18 ++++++++------ 2 files changed, 21 insertions(+), 23 deletions(-)drivers/firmware/efi/libstub/efi-stub-helper.c +1
-1@@ -193,7 +193,7 @@ void efi_apply_loadoptions_quirk(const void **load_options, u32 *load_options_si *load_options_size = load_option_unpacked.optional_data_size; } -enum efistub_event { +enum efistub_event_type {EFISTUB_EVT_INITRD ,EFISTUB_EVT_LOAD_OPTIONS ,EFISTUB_EVT_COUNT ,@@ -221,44 +221,38 @@ static const struct {Quoted 10 lines
drivers/firmware/efi/libstub/efi-stub-helper.c static const struct { · line 228
Quoted 8 lines
drivers/firmware/efi/libstub/efi-stub-helper.c +4
-1static efi_status_t efi_measure_tagged_event (unsigned long load_addr,unsigned long load_size, -enum efistub_event event) +enum efistub_event_type event) { +struct efistub_measured_event *evt; +int size =struct_size (evt, tagged_event_data, + events[event].event_data_len);Include
linux/overflow.hexplicitly?Yes, good point.
Quoted 15 lines
Quoted 15 lines
drivers/firmware/efi/libstub/efi-stub-helper.c +0
-7efi_guid_t tcg2_guid =EFI_TCG2_PROTOCOL_GUID ;efi_tcg2_protocol_t *tcg2 =NULL ;efi_status_t status;efi_bs_call (locate_protocol, &tcg2_guid,NULL , (void **)&tcg2);if (tcg2) { -struct efi_measured_event { -efi_tcg2_event_t event_data; -efi_tcg2_tagged_event_t tagged_event; -u8 tagged_event_data[]; - } *evt; -int size =sizeof (*evt) + events[event].event_data_len; - status =efi_bs_call (allocate_pool,EFI_LOADER_DATA , size, (void **)&evt);
Quoted 4 lines
Quoted 3 lines
It looks like in patch 3 you have converted evt as stack variable. Since that change is not specific to CC fallback, can it be moved here?
Not sure what you mean here. evt is still there after parch #3
Sorry, it looks like I misread the patch # 3. Please ignore this comment.
Quoted 68 lines
Quoted 53 lines
Quoted 53 lines
drivers/firmware/efi/libstub/efi-stub-helper.c +5
-7if (status !=EFI_SUCCESS ) goto fail; - evt->event_data = (struct efi_tcg2_event ){ + evt->event_data.tcg2_data = (struct efi_tcg2_event ){ .event_size = size, - .event_header.header_size =sizeof (evt->event_data.event_header), + .event_header.header_size =sizeof (evt->event_data.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 , }; - evt->tagged_event = (struct efi_tcg2_tagged_event ){ - .tagged_event_id = events[event].event_id, - .tagged_event_data_size = events[event].event_data_len, - }; + 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_call_proto (tcg2, hash_log_extend_event,0 , - load_addr, load_size, &evt->event_data); + load_addr, load_size, &evt->event_data.tcg2_data);efi_bs_call (free_pool, evt);if (status !=EFI_SUCCESS )drivers/firmware/efi/libstub/efistub.h +0
-7@@ -843,14 +843,7 @@ struct efi_tcg2_event {/* u8[] event follows here */ } __packed; -struct efi_tcg2_tagged_event { -u32 tagged_event_id; -u32 tagged_event_data_size; -/* u8 tagged event data follows here */ -} __packed; -typedef struct efi_tcg2_event efi_tcg2_event_t ; -typedef struct efi_tcg2_tagged_event efi_tcg2_tagged_event_t ;typedef union efi_tcg2_protocol efi_tcg2_protocol_t ;union efi_tcg2_protocol {@@ -882,6 +875,17 @@ union efi_tcg2_protocol { } mixed_mode; };Quoted 14 lines
drivers/firmware/efi/libstub/efistub.h union efi_tcg2_protocol { · line 888
Quoted 11 lines
drivers/firmware/efi/libstub/efistub.h +11
-0+union efistub_event { +efi_tcg2_event_t tcg2_data; +}; + +struct efistub_measured_event { +union efistub_event event_data; +u32 tagged_event_id; +u32 tagged_event_data_size; +u8 tagged_event_data[]; +} __packed; +Since
efistub_measured_eventis only usedefi-stub-helper.c, why not leave it there?Indeed. I will move it back.
Signature · 2 lines
Sathyanarayanan Kuppuswamy Linux Kernel Developer
PATCH 2/4 efi/libstub: Add Confidential Computing (CC) measurement typedefs
In reply to Ard BiesheuvelFrom: Kuppuswamy Sathyanarayanan <[email protected]>
If the virtual firmware implements TPM support, TCG2 protocol will be used for kernel measurements and event logging support. But in CC environment, not all platforms support or enable the TPM feature. UEFI specification [1] exposes protocol and interfaces used for kernel measurements in CC platforms without TPM support.
More details about the EFI CC measurements and logging can be found in [1].
--- drivers/firmware/efi/libstub/efistub.h | 79 ++++++++++++++++++++ include/linux/efi.h | 1 + 2 files changed, 80 insertions(+)
drivers/firmware/efi/libstub/efistub.h +79 -0
@@ -875,6 +875,85 @@ union efi_tcg2_protocol { } mixed_mode; }; +typedef struct { +u8 major; +u8 minor; +}efi_cc_version_t ; + +typedef struct { +u8 type; +u8 sub_type; +}efi_cc_type_t ; + +/* EFI CC type/subtype defines */ +#define EFI_CC_TYPE_NONE 0 +#define EFI_CC_TYPE_AMD_SEV 1 +#define EFI_CC_TYPE_INTEL_TDX 2 + +typedef u32 efi_cc_mr_index_t ; + +struct efi_cc_event { +u32 event_size; +struct { +u32 header_size; +u16 header_version; +u32 mr_index; +u32 event_type; + } __packed event_header; +u8 event_data[0 ]; +} __packed; + +typedef struct efi_cc_event efi_cc_event_t ; + +typedef u32 efi_cc_event_log_bitmap_t ; +typedef u32 efi_cc_event_log_format_t ; +typedef u32 efi_cc_event_algorithm_bitmap_t ; + +typedef struct { +u8 size; +efi_cc_version_t structure_version; +efi_cc_version_t protocol_version; +efi_cc_event_algorithm_bitmap_t hash_algorithm_bitmap; +efi_cc_event_log_bitmap_t supported_event_logs; +efi_cc_type_t cc_type; +}efi_cc_boot_service_cap_t ; + +#define EFI_CC_EVENT_HEADER_VERSION 1 + +#define EFI_CC_BOOT_HASH_ALG_SHA384 0x00000004 + +typedef union efi_cc_protocol efi_cc_protocol_t ; + +union efi_cc_protocol { +struct { +efi_status_t + (__efiapi *get_capability)(efi_cc_protocol_t *, + efi_cc_boot_service_cap_t *); + +efi_status_t + (__efiapi *get_event_log)(efi_cc_protocol_t *, + efi_cc_event_log_format_t, + efi_physical_addr_t *, + efi_physical_addr_t *, + efi_bool_t *); + +efi_status_t + (__efiapi *hash_log_extend_event)(efi_cc_protocol_t *, u64, + efi_physical_addr_t, u64, + const efi_cc_event_t *); + + efi_status_t + (__efiapi *map_pcr_to_mr_index)(efi_cc_protocol_t *,u32 , +efi_cc_mr_index_t *); + }; +struct { +u32 get_capability; +u32 get_event_log; +u32 hash_log_extend_event; +u32 map_pcr_to_mr_index; + } mixed_mode; +}; +union efistub_event {efi_tcg2_event_t tcg2_data; };
include/linux/efi.h +1 -0
@@ -400,6 +400,7 @@ void efi_native_runtime_setup(void);#define EFI_CERT_X509_GUID EFI_GUID(0xa5c059a1, 0x94e4, 0x4aa7, 0x87, 0xb5, 0xab, 0x15, 0x5c, 0x2b, 0xf0, 0x72)#define EFI_CERT_X509_SHA256_GUID EFI_GUID(0x3bd2a492, 0x96c0, 0x4079, 0xb4, 0x20, 0xfc, 0xf9, 0x8e, 0xf1, 0x03, 0xed)#define EFI_CC_BLOB_GUID EFI_GUID(0x067b1f5f, 0xcf26, 0x44c5, 0x85, 0x54, 0x93, 0xd7, 0x77, 0x91, 0x2d, 0x42) +#define EFI_CC_MEASUREMENT_PROTOCOL_GUID EFI_GUID(0x96751a3d, 0x72f4, 0x41a6, 0xa7, 0x94, 0xed, 0x5d, 0x0e, 0x67, 0xae, 0x6b)/* * This GUID is used to pass to the kernel proper the struct screen_info
Signature · 1 line
2.44.0.278.ge034bb2e1d-goog
Ilias Apalodimas
reviewerRe: [PATCH 2/4] efi/libstub: Add Confidential Computing (CC) measurement typedefs
Hi Ard,
Quoted 21 lines · Ard Biesheuvel
From: Kuppuswamy Sathyanarayanan <[email protected]>
If the virtual firmware implements TPM support, TCG2 protocol will be used for kernel measurements and event logging support. But in CC environment, not all platforms support or enable the TPM feature. UEFI specification [1] exposes protocol and interfaces used for kernel measurements in CC platforms without TPM support.
More details about the EFI CC measurements and logging can be found in [1].
Link https://uefi.org/specs/UEFI/2.10/38_Confidential_Computing.html#efi-cc-measurement-protocol [1]Signed-off-by Kuppuswamy Sathyanarayanan <[email protected]>[ardb: Drop code changes, keep typedefs and #define's only]Signed-off-by Ard Biesheuvel <[email protected]>--- drivers/firmware/efi/libstub/efistub.h | 79 ++++++++++++++++++++ include/linux/efi.h | 1 + 2 files changed, 80 insertions(+)
[...]
drivers/firmware/efi/libstub/efistub.h union efi_tcg2_protocol { · line 903
Quoted 10 lines
drivers/firmware/efi/libstub/efistub.h +10
-0+ +struct efi_cc_event { +u32 event_size; +struct { +u32 header_size; +u16 header_version; +u32 mr_index; +u32 event_type; + } __packed event_header; +u8 event_data[0 ];
We should define this as a flexible array member instead of a zero-length array? The spec is funny and defines this as event_data[1]. I think we aren't using the sizeof(struct efi_cc_event) anywhere, so if this struct is not used as a member of another struct or an array we can omit it entirely.
[...]
Cheers
/Ilias
Re: [PATCH 2/4] efi/libstub: Add Confidential Computing (CC) measurement typedefs
Quoted 25 lines · Ilias Apalodimas
Hi Ard,
Quoted 20 lines · Ard Biesheuvel
From: Kuppuswamy Sathyanarayanan <[email protected]>
If the virtual firmware implements TPM support, TCG2 protocol will be used for kernel measurements and event logging support. But in CC environment, not all platforms support or enable the TPM feature. UEFI specification [1] exposes protocol and interfaces used for kernel measurements in CC platforms without TPM support.
More details about the EFI CC measurements and logging can be found in [1].
Link https://uefi.org/specs/UEFI/2.10/38_Confidential_Computing.html#efi-cc-measurement-protocol [1]Signed-off-by Kuppuswamy Sathyanarayanan <[email protected]>[ardb: Drop code changes, keep typedefs and #define's only]Signed-off-by Ard Biesheuvel <[email protected]>--- drivers/firmware/efi/libstub/efistub.h | 79 ++++++++++++++++++++ include/linux/efi.h | 1 + 2 files changed, 80 insertions(+)[...]
Quoted 15 lines
drivers/firmware/efi/libstub/efistub.h union efi_tcg2_protocol { · line 903
Quoted 10 lines
drivers/firmware/efi/libstub/efistub.h +10
-0+ +struct efi_cc_event { +u32 event_size; +struct { +u32 header_size; +u16 header_version; +u32 mr_index; +u32 event_type; + } __packed event_header; +u8 event_data[0 ];We should define this as a flexible array member instead of a zero-length array? The spec is funny and defines this as event_data[1]. I think we aren't using the
sizeof(struct efi_cc_event)anywhere, so if this struct is not used as a member of another struct or an array we can omit it entirely.
Flexible array is also fine or we can just add a comment like "u8[] event follows here" like in struct efi_tcg2_event .
Quoted 5 lines
[...]
Cheers
/Ilias
Signature · 2 lines
Sathyanarayanan Kuppuswamy Linux Kernel Developer
Ilias Apalodimas
reviewerRe: [PATCH 2/4] efi/libstub: Add Confidential Computing (CC) measurement typedefs
Quoted 44 lines · Kuppuswamy Sathyanarayanan
Quoted 25 lines · Ilias Apalodimas
Hi Ard,
Quoted 20 lines · Ard Biesheuvel
From: Kuppuswamy Sathyanarayanan <[email protected]>
If the virtual firmware implements TPM support, TCG2 protocol will be used for kernel measurements and event logging support. But in CC environment, not all platforms support or enable the TPM feature. UEFI specification [1] exposes protocol and interfaces used for kernel measurements in CC platforms without TPM support.
More details about the EFI CC measurements and logging can be found in [1].
Link https://uefi.org/specs/UEFI/2.10/38_Confidential_Computing.html#efi-cc-measurement-protocol [1]Signed-off-by Kuppuswamy Sathyanarayanan <[email protected]>[ardb: Drop code changes, keep typedefs and #define's only]Signed-off-by Ard Biesheuvel <[email protected]>--- drivers/firmware/efi/libstub/efistub.h | 79 ++++++++++++++++++++ include/linux/efi.h | 1 + 2 files changed, 80 insertions(+)[...]
Quoted 15 lines
drivers/firmware/efi/libstub/efistub.h union efi_tcg2_protocol { · line 903
Quoted 10 lines
drivers/firmware/efi/libstub/efistub.h +10
-0+ +struct efi_cc_event { +u32 event_size; +struct { +u32 header_size; +u16 header_version; +u32 mr_index; +u32 event_type; + } __packed event_header; +u8 event_data[0 ];We should define this as a flexible array member instead of a zero-length array? The spec is funny and defines this as event_data[1]. I think we aren't using the
sizeof(struct efi_cc_event)anywhere, so if this struct is not used as a member of another struct or an array we can omit it entirely.
Quoted 2 lines
Flexible array is also fine or we can just add a comment like "u8[] event follows here" like in struct
efi_tcg2_event.
Yes. I just noticed I had a typo above. I meant to write "if this struct *is* used as a member of another struct or an array we can omit it entirely."
Quoted 11 lines
Quoted 6 lines
[...]
Cheers
/Ilias--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer
PATCH 3/4 efi/libstub: Measure into CC protocol if TCG2 protocol is absent
In reply to Ard BiesheuvelFrom: Ard Biesheuvel <[email protected]>
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.
--- drivers/firmware/efi/libstub/efi-stub-helper.c | 67 +++++++++++++++----- drivers/firmware/efi/libstub/efistub.h | 3 + 2 files changed, 53 insertions(+), 17 deletions(-)
drivers/firmware/efi/libstub/efi-stub-helper.c +50 -17
@@ -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 { +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;
drivers/firmware/efi/libstub/efistub.h +3 -0
@@ -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 {
Signature · 1 line
2.44.0.278.ge034bb2e1d-goog
Re: [PATCH 3/4] efi/libstub: Measure into CC protocol if TCG2 protocol is absent
Quoted 51 lines · Ard Biesheuvel
From: Ard Biesheuvel <[email protected]>
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 <[email protected]>--- drivers/firmware/efi/libstub/efi-stub-helper.c | 67 +++++++++++++++----- drivers/firmware/efi/libstub/efistub.h | 3 + 2 files changed, 53 insertions(+), 17 deletions(-)drivers/firmware/efi/libstub/efi-stub-helper.c +9
-6@@ -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,
drivers/firmware/efi/libstub/efi-stub-helper.c static efi_status_t efi_measure_tagged_event(unsigned long load_addr, · line 253
Quoted 10 lines
drivers/firmware/efi/libstub/efi-stub-helper.c +5
-1- .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. 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.
Quoted 76 lines
drivers/firmware/efi/libstub/efi-stub-helper.c +36
-10+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;drivers/firmware/efi/libstub/efistub.h +3
-1@@ -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
Signature · 1 line
-Dionna Glaze, PhD (she/her)
Ard Biesheuvel
maintainerRe: [PATCH 3/4] efi/libstub: Measure into CC protocol if TCG2 protocol is absent
Quoted 60 lines · Dionna Amalie Glaze
Quoted 58 lines · Ard Biesheuvel
From: Ard Biesheuvel <[email protected]>
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 <[email protected]>--- drivers/firmware/efi/libstub/efi-stub-helper.c | 67 +++++++++++++++----- drivers/firmware/efi/libstub/efistub.h | 3 + 2 files changed, 53 insertions(+), 17 deletions(-)drivers/firmware/efi/libstub/efi-stub-helper.c +11
-7@@ -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;
Quoted 8 lines
drivers/firmware/efi/libstub/efi-stub-helper.c static efi_status_t efi_measure_tagged_event(unsigned long load_addr, · line 253
Quoted 3 lines
drivers/firmware/efi/libstub/efi-stub-helper.c +3
-0+ method.hash_log_extend_event = + (void *)efi_table_attr (tcg2, hash_log_extend_event); + }else {+Qinkun Bao
Given that the exclusive or betweenCC_MEASUREMENT_PROTOCOLand 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.
Quoted 5 lines
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.
Quoted 80 lines
Quoted 73 lines
drivers/firmware/efi/libstub/efi-stub-helper.c +36
-10+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;drivers/firmware/efi/libstub/efistub.h +3
-1@@ -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 { --Quoted 3 lines
2.44.0.278.ge034bb2e1d-goog
--
-Dionna Glaze, PhD (she/her)
Ilias Apalodimas
reviewerRe: [PATCH 3/4] efi/libstub: Measure into CC protocol if TCG2 protocol is absent
Quoted 81 lines · Ard Biesheuvel
Quoted 60 lines · Dionna Amalie Glaze
Quoted 58 lines · Ard Biesheuvel
From: Ard Biesheuvel <[email protected]>
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 <[email protected]>--- drivers/firmware/efi/libstub/efi-stub-helper.c | 67 +++++++++++++++----- drivers/firmware/efi/libstub/efistub.h | 3 + 2 files changed, 53 insertions(+), 17 deletions(-)drivers/firmware/efi/libstub/efi-stub-helper.c +11
-7@@ -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;Quoted 8 lines
drivers/firmware/efi/libstub/efi-stub-helper.c static efi_status_t efi_measure_tagged_event(unsigned long load_addr, · line 253
Quoted 3 lines
drivers/firmware/efi/libstub/efi-stub-helper.c +3
-0+ method.hash_log_extend_event = + (void *)efi_table_attr (tcg2, hash_log_extend_event); + }else {+Qinkun Bao
Given that the exclusive or betweenCC_MEASUREMENT_PROTOCOLand 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.
Quoted 8 lines
Quoted 5 lines
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.
Apart from what the EFI spec says, why would anyone want to use both protocols? Won't we end up measuring things 2x ?
Thanks
/Ilias
Quoted 83 lines
Quoted 82 lines
Quoted 75 lines
drivers/firmware/efi/libstub/efi-stub-helper.c +36
-10+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;drivers/firmware/efi/libstub/efistub.h +3
-1@@ -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 { --Quoted 3 lines
2.44.0.278.ge034bb2e1d-goog
--
-Dionna Glaze, PhD (she/her)
Re: [PATCH 3/4] efi/libstub: Measure into CC protocol if TCG2 protocol is absent
In reply to Ard BiesheuvelQuoted 81 lines · Ard Biesheuvel
Quoted 60 lines · Dionna Amalie Glaze
Quoted 58 lines · Ard Biesheuvel
From: Ard Biesheuvel <[email protected]>
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 <[email protected]>--- drivers/firmware/efi/libstub/efi-stub-helper.c | 67 +++++++++++++++----- drivers/firmware/efi/libstub/efistub.h | 3 + 2 files changed, 53 insertions(+), 17 deletions(-)drivers/firmware/efi/libstub/efi-stub-helper.c +11
-7@@ -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;Quoted 8 lines
drivers/firmware/efi/libstub/efi-stub-helper.c static efi_status_t efi_measure_tagged_event(unsigned long load_addr, · line 253
Quoted 3 lines
drivers/firmware/efi/libstub/efi-stub-helper.c +3
-0+ method.hash_log_extend_event = + (void *)efi_table_attr (tcg2, hash_log_extend_event); + }else {+Qinkun Bao
Given that the exclusive or betweenCC_MEASUREMENT_PROTOCOLand 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.
Quoted 9 lines
Quoted 5 lines
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.
Quoted 81 lines
Quoted 81 lines
Quoted 74 lines
drivers/firmware/efi/libstub/efi-stub-helper.c +36
-10+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;drivers/firmware/efi/libstub/efistub.h +3
-1@@ -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 { --Quoted 3 lines
2.44.0.278.ge034bb2e1d-goog
--
-Dionna Glaze, PhD (she/her)
Signature · 1 line
-Dionna Glaze, PhD (she/her)
Re: [PATCH 3/4] efi/libstub: Measure into CC protocol if TCG2 protocol is absent
Quoted 95 lines · Dionna Amalie Glaze
Quoted 76 lines · Ard Biesheuvel
Quoted 57 lines · Dionna Amalie Glaze
Quoted 56 lines · Ard Biesheuvel
From: Ard Biesheuvel <[email protected]>
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 <[email protected]>--- drivers/firmware/efi/libstub/efi-stub-helper.c | 67 +++++++++++++++----- drivers/firmware/efi/libstub/efistub.h | 3 + 2 files changed, 53 insertions(+), 17 deletions(-)drivers/firmware/efi/libstub/efi-stub-helper.c +10
-7@@ -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 , };Quoted 8 lines
drivers/firmware/efi/libstub/efi-stub-helper.c static efi_status_t efi_measure_tagged_event(unsigned long load_addr, · line 253
Quoted 4 lines
drivers/firmware/efi/libstub/efi-stub-helper.c +4
-0+ protocol = tcg2; + method.hash_log_extend_event = + (void *)efi_table_attr (tcg2, hash_log_extend_event); + }else {+Qinkun Bao
Given that the exclusive or betweenCC_MEASUREMENT_PROTOCOLand 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.
Quoted 8 lines
Quoted 5 lines
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.
Quoted 6 lines
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).
Quoted 81 lines
Quoted 79 lines
Quoted 79 lines
Quoted 73 lines
drivers/firmware/efi/libstub/efi-stub-helper.c +36
-10+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;drivers/firmware/efi/libstub/efistub.h +3
-1@@ -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 { --Quoted 3 lines
2.44.0.278.ge034bb2e1d-goog
--
-Dionna Glaze, PhD (she/her)
Signature · 2 lines
Sathyanarayanan Kuppuswamy Linux Kernel Developer
Re: [PATCH 3/4] efi/libstub: Measure into CC protocol if TCG2 protocol is absent
Quoted 98 lines · Kuppuswamy Sathyanarayanan
Quoted 95 lines · Dionna Amalie Glaze
Quoted 76 lines · Ard Biesheuvel
Quoted 57 lines · Dionna Amalie Glaze
Quoted 56 lines · Ard Biesheuvel
From: Ard Biesheuvel <[email protected]>
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 <[email protected]>--- drivers/firmware/efi/libstub/efi-stub-helper.c | 67 +++++++++++++++----- drivers/firmware/efi/libstub/efistub.h | 3 + 2 files changed, 53 insertions(+), 17 deletions(-)drivers/firmware/efi/libstub/efi-stub-helper.c +10
-7@@ -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 , };Quoted 8 lines
drivers/firmware/efi/libstub/efi-stub-helper.c static efi_status_t efi_measure_tagged_event(unsigned long load_addr, · line 253
Quoted 4 lines
drivers/firmware/efi/libstub/efi-stub-helper.c +4
-0+ protocol = tcg2; + method.hash_log_extend_event = + (void *)efi_table_attr (tcg2, hash_log_extend_event); + }else {+Qinkun Bao
Given that the exclusive or betweenCC_MEASUREMENT_PROTOCOLand 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.
Quoted 8 lines
Quoted 5 lines
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.
Quoted 8 lines
Quoted 6 lines
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.
Yes, we've been discussing this with Intel and they agreed to allow a default false build option to measure into both.
Quoted 2 lines
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
There are folks who want to do a double-check with TEE quotes, but yes I agree in general this is not the best situation. It's a stepping stones model rather than scaling Everest in one bound. Ideally you'd have a measured and protected TPM implementation with adequate security for persistent data so that the CC_MEASUREMENT_PROTOCOL is fully redundant and therefore not needed.
But anyway, the standard is what it is for now, so I wouldn't block this patch based on this request. When there's more alignment from the UEFI standards working group and an accepted patch into EDK2, then we can revisit this in the different boot layers.
Quoted 89 lines
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).
Quoted 71 lines
Quoted 71 lines
Quoted 71 lines
Quoted 71 lines
drivers/firmware/efi/libstub/efi-stub-helper.c +36
-10+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;drivers/firmware/efi/libstub/efistub.h +3
-0@@ -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; };Quoted 10 lines
Quoted 8 lines
Quoted 8 lines
Quoted 5 lines
drivers/firmware/efi/libstub/efistub.h +0
-1struct efistub_measured_event { --2.44.0.278.ge034bb2e1d-goog
--
-Dionna Glaze, PhD (she/her)--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer
Signature · 1 line
-Dionna Glaze, PhD (she/her)
Re: [PATCH 3/4] efi/libstub: Measure into CC protocol if TCG2 protocol is absent
Quoted 105 lines · Dionna Amalie Glaze
Quoted 103 lines · Kuppuswamy Sathyanarayanan
Quoted 101 lines · Dionna Amalie Glaze
Quoted 76 lines · Ard Biesheuvel
Quoted 57 lines · Dionna Amalie Glaze
Quoted 56 lines · Ard Biesheuvel
From: Ard Biesheuvel <[email protected]>
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 <[email protected]>--- drivers/firmware/efi/libstub/efi-stub-helper.c | 67 +++++++++++++++----- drivers/firmware/efi/libstub/efistub.h | 3 + 2 files changed, 53 insertions(+), 17 deletions(-)drivers/firmware/efi/libstub/efi-stub-helper.c +10
-7@@ -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 , };Quoted 8 lines
drivers/firmware/efi/libstub/efi-stub-helper.c static efi_status_t efi_measure_tagged_event(unsigned long load_addr, · line 253
Quoted 4 lines
drivers/firmware/efi/libstub/efi-stub-helper.c +4
-0+ protocol = tcg2; + method.hash_log_extend_event = + (void *)efi_table_attr (tcg2, hash_log_extend_event); + }else {+Qinkun Bao
Given that the exclusive or betweenCC_MEASUREMENT_PROTOCOLand 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.
Quoted 8 lines
Quoted 5 lines
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.
Quoted 3 lines
Quoted 1 line
If you check the EDK2 part of this support, it also uses if else model.
Yes, we've been discussing this with Intel and they agreed to allow a default false build option to measure into both.
To make it clear, any plans to update the spec with this requirement?
Quoted 10 lines
Quoted 3 lines
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
There are folks who want to do a double-check with TEE quotes, but yes I agree in general this is not the best situation. It's a stepping stones model rather than scaling Everest in one bound. Ideally you'd have a measured and protected TPM implementation with adequate security for persistent data so that the
CC_MEASUREMENT_PROTOCOLis fully redundant and therefore not needed.
Quoted 4 lines
But anyway, the standard is what it is for now, so I wouldn't block this patch based on this request. When there's more alignment from the UEFI standards working group and an accepted patch into EDK2, then we can revisit this in the different boot layers.
Got it. Personally I think we can consider it, once it is adapted in firmware and updated in spec (to make it consistent with firmware support). Anyway, since doing it is harmless, I will leave it to the maintainers choice.
Quoted 89 lines
Quoted 88 lines
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).
Quoted 71 lines
Quoted 71 lines
Quoted 71 lines
Quoted 71 lines
drivers/firmware/efi/libstub/efi-stub-helper.c +36
-10+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;drivers/firmware/efi/libstub/efistub.h +3
-0@@ -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; };Quoted 8 lines
Quoted 7 lines
Quoted 7 lines
Quoted 5 lines
drivers/firmware/efi/libstub/efistub.h +0
-1struct efistub_measured_event { --2.44.0.278.ge034bb2e1d-goog
--
-Dionna Glaze, PhD (she/her)--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer
Signature · 2 lines
Sathyanarayanan Kuppuswamy Linux Kernel Developer
Re: [PATCH 3/4] efi/libstub: Measure into CC protocol if TCG2 protocol is absent
Quoted 108 lines · Kuppuswamy Sathyanarayanan
Quoted 105 lines · Dionna Amalie Glaze
Quoted 103 lines · Kuppuswamy Sathyanarayanan
Quoted 101 lines · Dionna Amalie Glaze
Quoted 76 lines · Ard Biesheuvel
Quoted 57 lines · Dionna Amalie Glaze
Quoted 56 lines · Ard Biesheuvel
From: Ard Biesheuvel <[email protected]>
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 <[email protected]>--- drivers/firmware/efi/libstub/efi-stub-helper.c | 67 +++++++++++++++----- drivers/firmware/efi/libstub/efistub.h | 3 + 2 files changed, 53 insertions(+), 17 deletions(-)drivers/firmware/efi/libstub/efi-stub-helper.c +10
-7@@ -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 , };Quoted 8 lines
drivers/firmware/efi/libstub/efi-stub-helper.c static efi_status_t efi_measure_tagged_event(unsigned long load_addr, · line 253
Quoted 4 lines
drivers/firmware/efi/libstub/efi-stub-helper.c +4
-0+ protocol = tcg2; + method.hash_log_extend_event = + (void *)efi_table_attr (tcg2, hash_log_extend_event); + }else {+Qinkun Bao
Given that the exclusive or betweenCC_MEASUREMENT_PROTOCOLand 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.
Quoted 8 lines
Quoted 5 lines
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.
Quoted 6 lines
Quoted 3 lines
Quoted 1 line
If you check the EDK2 part of this support, it also uses if else model.
Yes, we've been discussing this with Intel and they agreed to allow a default false build option to measure into both.
To make it clear, any plans to update the spec with this requirement?
I don't think so. I do agree generally with the "should not" wording as a long term end goal.
But given both interfaces anyway, I think they should both be used. If that clarification is required in the spec, I can take it to the USWG. I will note that the "else" interpretation is not universal, as is the case in rhboot/shim and grub2.
Quoted 113 lines
Quoted 10 lines
Quoted 3 lines
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
There are folks who want to do a double-check with TEE quotes, but yes I agree in general this is not the best situation. It's a stepping stones model rather than scaling Everest in one bound. Ideally you'd have a measured and protected TPM implementation with adequate security for persistent data so that the
CC_MEASUREMENT_PROTOCOLis fully redundant and therefore not needed.Quoted 4 lines
But anyway, the standard is what it is for now, so I wouldn't block this patch based on this request. When there's more alignment from the UEFI standards working group and an accepted patch into EDK2, then we can revisit this in the different boot layers.
Got it. Personally I think we can consider it, once it is adapted in firmware and updated in spec (to make it consistent with firmware support). Anyway, since doing it is harmless, I will leave it to the maintainers choice.
Quoted 84 lines
Quoted 84 lines
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).
Quoted 79 lines
Quoted 78 lines
Quoted 78 lines
Quoted 73 lines
drivers/firmware/efi/libstub/efi-stub-helper.c +36
-10+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;drivers/firmware/efi/libstub/efistub.h +3
-1@@ -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 { --Quoted 3 lines
2.44.0.278.ge034bb2e1d-goog
--
-Dionna Glaze, PhD (she/her)Quoted 5 lines
Quoted 4 lines
--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer
Signature · 1 line
-Dionna Glaze, PhD (she/her)
Ard Biesheuvel
maintainerRe: [PATCH 3/4] efi/libstub: Measure into CC protocol if TCG2 protocol is absent
In reply to Dionna Amalie GlazeQuoted 123 lines · Dionna Amalie Glaze
Quoted 98 lines · Kuppuswamy Sathyanarayanan
Quoted 95 lines · Dionna Amalie Glaze
Quoted 76 lines · Ard Biesheuvel
Quoted 57 lines · Dionna Amalie Glaze
Quoted 56 lines · Ard Biesheuvel
From: Ard Biesheuvel <[email protected]>
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 <[email protected]>--- drivers/firmware/efi/libstub/efi-stub-helper.c | 67 +++++++++++++++----- drivers/firmware/efi/libstub/efistub.h | 3 + 2 files changed, 53 insertions(+), 17 deletions(-)drivers/firmware/efi/libstub/efi-stub-helper.c +10
-7@@ -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 , };Quoted 8 lines
drivers/firmware/efi/libstub/efi-stub-helper.c static efi_status_t efi_measure_tagged_event(unsigned long load_addr, · line 253
Quoted 4 lines
drivers/firmware/efi/libstub/efi-stub-helper.c +4
-0+ protocol = tcg2; + method.hash_log_extend_event = + (void *)efi_table_attr (tcg2, hash_log_extend_event); + }else {+Qinkun Bao
Given that the exclusive or betweenCC_MEASUREMENT_PROTOCOLand 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.
Quoted 8 lines
Quoted 5 lines
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.
Quoted 8 lines
Quoted 6 lines
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.
Yes, we've been discussing this with Intel and they agreed to allow a default false build option to measure into both.
Quoted 2 lines
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
There are folks who want to do a double-check with TEE quotes, but yes I agree in general this is not the best situation. It's a stepping stones model rather than scaling Everest in one bound. Ideally you'd have a measured and protected TPM implementation with adequate security for persistent data so that the
CC_MEASUREMENT_PROTOCOLis fully redundant and therefore not needed.
Quoted 5 lines
But anyway, the standard is what it is for now, so I wouldn't block this patch based on this request. When there's more alignment from the UEFI standards working group and an accepted patch into EDK2, then we can revisit this in the different boot layers.
Yeah some spec guidance on when having both protocols makes sense and why would help here. If you trust the VMM to operate the vTPM for you, what is the point of the CC protocol?
For now, I'll go with the changes as proposed, also because we're close to the merge window. We can always revisit this and backport if needed.
Re: [PATCH 3/4] efi/libstub: Measure into CC protocol if TCG2 protocol is absent
In reply to Ard BiesheuvelQuoted 6 lines · Ard Biesheuvel
From: Ard Biesheuvel <[email protected]>
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.
Quoted 7 lines
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 <[email protected]>---
Looks fine to me.
Quoted 119 lines
drivers/firmware/efi/libstub/efi-stub-helper.c | 67 +++++++++++++++----- drivers/firmware/efi/libstub/efistub.h | 3 + 2 files changed, 53 insertions(+), 17 deletions(-)drivers/firmware/efi/libstub/efi-stub-helper.c +50
-17@@ -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 { +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;drivers/firmware/efi/libstub/efistub.h +3
-0@@ -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 {
Signature · 2 lines
Sathyanarayanan Kuppuswamy Linux Kernel Developer
PATCH 4/4 efi/libstub: Add get_event_log() support for CC platforms
In reply to Ard BiesheuvelFrom: Kuppuswamy Sathyanarayanan <[email protected]>
To allow event log info access after boot, EFI boot stub extracts the event log information and installs it in an EFI configuration table. Currently, EFI boot stub only supports installation of event log only for TPM 1.2 and TPM 2.0 protocols. Extend the same support for CC protocol. Since CC platform also uses TCG2 format, reuse TPM2 support code as much as possible.
--- drivers/firmware/efi/libstub/efi-stub.c | 2 +- drivers/firmware/efi/libstub/efistub.h | 4 +- drivers/firmware/efi/libstub/tpm.c | 74 +++++++++++++------- drivers/firmware/efi/libstub/x86-stub.c | 2 +- include/linux/efi.h | 3 + 5 files changed, 57 insertions(+), 28 deletions(-)
drivers/firmware/efi/libstub/efi-stub.c +1 -1
@@ -167,7 +167,7 @@ efi_status_t efi_stub_common(efi_handle_t handle, si =setup_graphics (); -efi_retrieve_tpm2_eventlog (); +efi_retrieve_eventlog ();/* Ask the firmware to clear memory on unclean shutdown */ efi_enable_reset_attack_mitigation ();
drivers/firmware/efi/libstub/efistub.h +3 -1
@@ -922,6 +922,8 @@ typedef struct {#define EFI_CC_BOOT_HASH_ALG_SHA384 0x00000004 +#define EFI_CC_EVENT_LOG_FORMAT_TCG_2 0x00000002 +typedef union efi_cc_protocol efi_cc_protocol_t ;union efi_cc_protocol {
@@ -1147,7 +1149,7 @@ static inline voidefi_enable_reset_attack_mitigation (void) { }#endif -void efi_retrieve_tpm2_eventlog (void ); +void efi_retrieve_eventlog (void );struct screen_info *alloc_screen_info (void );struct screen_info *__alloc_screen_info (void );
drivers/firmware/efi/libstub/tpm.c +49 -25
105 lines not on this page · raw message
drivers/firmware/efi/libstub/x86-stub.c +1 -1
@@ -923,7 +923,7 @@ void __noreturn efi_stub_entry(efi_handle_t handle,efi_random_get_seed (); -efi_retrieve_tpm2_eventlog (); +efi_retrieve_eventlog ();setup_graphics (boot_params);
include/linux/efi.h +3 -0
@@ -440,6 +440,9 @@ void efi_native_runtime_setup(void);/* OVMF protocol GUIDs */ #define OVMF_SEV_MEMORY_ACCEPTANCE_PROTOCOL_GUID EFI_GUID(0xc5a010fe, 0x38a7, 0x4531, 0x8a, 0x4a, 0x05, 0x00, 0xd2, 0xfd, 0x16, 0x49) +/* CC GUIDs */ +#define LINUX_EFI_CC_FINAL_LOG_GUID EFI_GUID(0xdd4a4648, 0x2de7, 0x4665, 0x96, 0x4d, 0x21, 0xd9, 0xef, 0x5f, 0xb4, 0x46) + typedef struct { efi_guid_t guid; u64 table;
Signature · 1 line
2.44.0.278.ge034bb2e1d-goog