On 1/16/23 06:00, Ilias Apalodimas wrote:
Hi Eddie
+static inline u16 tpm2_algorithm_to_len(enum tpm2_algorithms a)
+{
+ switch (a) {
+ case TPM2_ALG_SHA1:
+ return TPM2_SHA1_DIGEST_SIZE;
+ case TPM2_ALG_SHA256:
+ return TPM2_SHA256_DIGEST_SIZE;
+ case TPM2_ALG_SHA384:
+ return TPM2_SHA384_DIGEST_SIZE;
+ case TPM2_ALG_SHA512:
+ return TPM2_SHA512_DIGEST_SIZE;
+ default:
+ return 0;
+ }
+}
Any reason we can't move the static 'const struct digest_info
hash_algo_list' from the efi_tcg.c here? We can then move the
functions defined in there alg_to_mask and alg_to_len.
And since alg_to_mask is really just a bitshift maybe replace that?
Hi,
It seems more efficient to keep the switch statement rather than iterate
through the structure array looking for the matching hash algorithm? I
could remove the 'static const struct digest_info hash_algo_list' in
efi_tcg2.c and instead use the tpm2_algorithm_to_len and
tpm2_algorithm_to_mask in efi_tcg2.c. What do you think?
+
+#define tpm2_algorithm_to_mask(a) (1 << (a))
+
/* NV index attributes */
enum tpm_index_attrs {
TPMA_NV_PPWRITE = 1UL << 0,
@@ -419,6 +481,142 @@ enum {
HR_NV_INDEX = TPM_HT_NV_INDEX << HR_SHIFT,
};
+/**
+ * struct tcg2_event_log - Container for managing the platform event log
+ *
+ * @log: Address of the log
+ * @log_position: Current entry position
+ * @log_size: Log space available
+ */
+struct tcg2_event_log {
+ u8 *log;
+ u32 log_position;
<snip>
- }
- }
-
- *pcr_banks = pcrs.count;
-
- return 0;
-out:
- return -1;
-}
-
/**
* __get_active_pcr_banks() - returns the currently active PCR banks
*
@@ -638,7 +378,7 @@ static efi_status_t __get_active_pcr_banks(u32
*active_pcr_banks)
efi_status_t ret;
int err;
- ret = platform_get_tpm2_device(&dev);
+ ret = tcg2_platform_get_tpm2(&dev);
if (ret != EFI_SUCCESS)
goto out;
We can get rid of this entirely and just define the
efi_tcg2_get_active_pcr_banks in the efi_tcg.c now.
__get_active_pcr_banks == tcg2_get_active_pcr_banks with the only
diffence being the udevice which is now an argument
Ack.
@@ -654,70 +394,6 @@ out:
return ret;
}
* efi_tcg2_get_capability() - protocol capability information and state
information
*
@@ -759,7 +435,7 @@ efi_tcg2_get_capability(struct efi_tcg2_protocol *this,
capability->protocol_version.major = 1;
capability->protocol_version.minor = 1;
<snip>
+
+static int tcg2_log_append_check(struct tcg2_event_log *elog, u32 pcr_index,
+ u32 event_type,
+ struct tpml_digest_values *digest_list,
+ u32 size, const u8 *event)
+{
+ u32 event_size;
+ u8 *log;
+
+ event_size = size + tcg2_event_get_size(digest_list);
+ if (elog->log_position + event_size > elog->log_size) {
+ printf("%s: log too large: %u + %u > %u\n", __func__,
+ elog->log_position, event_size, elog->log_size);
+ return -ENOBUFS;
+ }
+
+ log = elog->log + elog->log_position;
+ elog->log_position += event_size;
+
+ tcg2_log_append(pcr_index, event_type, digest_list, size, event, log);
+
+ return 0;
+}
+
+static int tcg2_log_init(struct udevice *dev, struct tcg2_event_log *elog)
+{
I think we need to re-use efi_init_event_log here. The reason is that on
Arm devices TF-A is capable of constructing an eventlog and passing it
along in memory. That code takes that into account and tries to reuse the
existing EventLog passed from previous boot stages.
The main difference between the EFI function and this one
is the allocated memory of the EventLog itself. But even in this case, it
would be better to tweak the EFI code and do
create log -> Allocate EFI memory -> copy log and then use that for EFI
OK... I'll try and get that to work. I see some potential issues like
the fact that EFI finds the platform event log differently.
+ struct tcg_efi_spec_id_event *ev;
+ struct tcg_pcr_event *log;
+ u32 event_size;
+ u32 count = 0;
+ u32 log_size;
+ u32 active;
+ u32 mask;
+ size_t i;
+ u16 len;
+ int rc;
+
+ rc = tcg2_get_active_pcr_banks(dev, &active);
+ if (rc)
+ return rc;
+
+ event_size = offsetof(struct tcg_efi_spec_id_event, digest_sizes);
+ for (i = 0; i < ARRAY_SIZE(tcg2algos); ++i) {
+ mask = tpm2_algorithm_to_mask(tcg2algos[i]);
+
+ if (!(active & mask))
+ continue;
+
+ switch (tcg2algos[i]) {
+ case TPM2_ALG_SHA1:
+ case TPM2_ALG_SHA256:
+ case TPM2_ALG_SHA384:
+ case TPM2_ALG_SHA512:
+ count++;
+ break;
+ default:
+ continue;
+ }
+ }
+
+ event_size += 1 +
+ (sizeof(struct tcg_efi_spec_id_event_algorithm_size) * count);
+ log_size = offsetof(struct tcg_pcr_event, event) + event_size;
+
+ if (log_size > elog->log_size) {
+ printf("%s: log too large: %u > %u\n", __func__, log_size,
+ elog->log_size);
+ return -ENOBUFS;
+ }
+
<snip>
+
+int tcg2_measure_event(struct udevice *dev, struct tcg2_event_log *elog,
+ u32 pcr_index, u32 event_type, u32 size,
+ const u8 *event)
+{
+ struct tpml_digest_values digest_list;
+ int rc;
+
+ rc = tcg2_create_digest(dev, event, size, &digest_list);
+ if (rc)
+ return rc;
+
+ rc = tcg2_pcr_extend(dev, pcr_index, &digest_list);
+ if (rc)
+ return rc;
+
+ return tcg2_log_append_check(elog, pcr_index, event_type, &digest_list,
+ size, event);
+}
There's a static efi_status_t tcg2_measure_event(...) left in efi_tcg.c
which breaks compilation. WE should just use the one you added in tpm-v2.c
Hmm. The EFI version does a slightly different way of event logging, so
I'm not sure it's that simple. There is EFI specific stuff
(ExitBootServices?) so I'm not sure it can be common... I can change the
name in efi_tcg2.c to compile correctly.
Thanks,
Eddie
+
+ return 0;
+}
+
u32 tpm2_dam_reset(struct udevice *dev, const char *pw, const ssize_t pw_sz)
{
u8 command_v2[COMMAND_BUFFER_SIZE] = {
--
2.31.1
Regards
/Ilias
