From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 637F0C25B50 for ; Mon, 23 Jan 2023 20:15:33 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 3C17285774; Mon, 23 Jan 2023 21:15:31 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=linux.ibm.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=ibm.com header.i=@ibm.com header.b="rMN4SrI8"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 62132856E3; Mon, 23 Jan 2023 21:15:30 +0100 (CET) Received: from mx0a-001b2d01.pphosted.com (mx0a-001b2d01.pphosted.com [148.163.156.1]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 0238A8577B for ; Mon, 23 Jan 2023 21:15:26 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=linux.ibm.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=eajames@linux.ibm.com Received: from pps.filterd (m0187473.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 30NJt6nu014604; Mon, 23 Jan 2023 20:15:23 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h=message-id : date : mime-version : from : subject : to : cc : references : in-reply-to : content-type : content-transfer-encoding; s=pp1; bh=rGicBz8O9flu2xe8vEDXoqbfmABfLfHRcCc4dTkGovw=; b=rMN4SrI8UXdyrSfmR1Br1HEJcIUYcZ41lVNuPEo+fMCiZ0hTU4G3gFLNimAutt/afIKt /5ZmMLJ90GDBuP3ihj3luJwDSUimOglGuKvNjY9/Bak0RqAKlyN2EltpxIA4+2dsn6sW c5TEsGA8YV8gMaRgVM9XHNmPbYeWfnK/OqSPs7qG3o412MZzyXXgxZ8TpR0axMRcXUTh 83mudEWCl1mrg4GP9+vOBDO9IOjpSjvG1NjHELZ6UjTvroW7e3qi4HJgzDyPqzB7v7MQ i7GQRyixRYAQDSnJKMwAjxSwqlP0rZwTrrODglfueq+o4a8atqFLWUn8NA6Y586WQT9t Dg== Received: from pps.reinject (localhost [127.0.0.1]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3na0tjrhfn-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 23 Jan 2023 20:15:22 +0000 Received: from m0187473.ppops.net (m0187473.ppops.net [127.0.0.1]) by pps.reinject (8.17.1.5/8.17.1.5) with ESMTP id 30NJuASZ016714; Mon, 23 Jan 2023 20:15:22 GMT Received: from ppma02wdc.us.ibm.com (aa.5b.37a9.ip4.static.sl-reverse.com [169.55.91.170]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3na0tjrher-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 23 Jan 2023 20:15:22 +0000 Received: from pps.filterd (ppma02wdc.us.ibm.com [127.0.0.1]) by ppma02wdc.us.ibm.com (8.17.1.19/8.17.1.19) with ESMTP id 30NGpjGW012727; Mon, 23 Jan 2023 20:15:21 GMT Received: from smtprelay04.wdc07v.mail.ibm.com ([9.208.129.114]) by ppma02wdc.us.ibm.com (PPS) with ESMTPS id 3n87p7e986-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 23 Jan 2023 20:15:21 +0000 Received: from smtpav02.wdc07v.mail.ibm.com (smtpav02.wdc07v.mail.ibm.com [10.39.53.229]) by smtprelay04.wdc07v.mail.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 30NKFKKi20513384 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Mon, 23 Jan 2023 20:15:20 GMT Received: from smtpav02.wdc07v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 0BC7C58061; Mon, 23 Jan 2023 20:15:20 +0000 (GMT) Received: from smtpav02.wdc07v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 6523C58060; Mon, 23 Jan 2023 20:15:19 +0000 (GMT) Received: from [9.160.44.244] (unknown [9.160.44.244]) by smtpav02.wdc07v.mail.ibm.com (Postfix) with ESMTP; Mon, 23 Jan 2023 20:15:19 +0000 (GMT) Message-ID: <6193164e-b7f3-ea80-b843-614e1004507c@linux.ibm.com> Date: Mon, 23 Jan 2023 14:15:18 -0600 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.6.0 From: Eddie James Subject: Re: [PATCH v3 2/6] tpm: Support boot measurements To: Ilias Apalodimas Cc: u-boot@lists.denx.de, sjg@chromium.org, xypron.glpk@gmx.de References: <20230112161607.282165-1-eajames@linux.ibm.com> <20230112161607.282165-3-eajames@linux.ibm.com> Content-Language: en-US In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-TM-AS-GCONF: 00 X-Proofpoint-GUID: OTm04TE9zpqan2dlcOuOj7781Buy8G2u X-Proofpoint-ORIG-GUID: uKXgJkVb9eyOa_aUuPHTL9eGndoRU8oq X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.219,Aquarius:18.0.930,Hydra:6.0.562,FMLib:17.11.122.1 definitions=2023-01-23_12,2023-01-23_01,2022-06-22_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 lowpriorityscore=0 phishscore=0 mlxscore=0 clxscore=1015 impostorscore=0 priorityscore=1501 bulkscore=0 mlxlogscore=999 adultscore=0 malwarescore=0 suspectscore=0 spamscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2212070000 definitions=main-2301230193 X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.6 at phobos.denx.de X-Virus-Status: Clean 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; >> - } >> - } >> - >> - *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; >> + >> +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; >> + } >> + >> + >> +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