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 0FB30C05027 for ; Thu, 2 Feb 2023 16:25:01 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id BD03B85CE1; Thu, 2 Feb 2023 17:24:58 +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="G+Eo3lWX"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 67D8685CF5; Thu, 2 Feb 2023 17:24:57 +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 C271A85CDF for ; Thu, 2 Feb 2023 17:24:53 +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 (m0098409.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id 312G6W2I004987; Thu, 2 Feb 2023 16:24:49 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h=message-id : date : subject : to : cc : references : from : in-reply-to : content-type : content-transfer-encoding : mime-version; s=pp1; bh=m/gGQcRDWdRwQ6DDHxcvR6PWQwxs+5aEw3G/sZUVNC4=; b=G+Eo3lWXwh0pfsNZI7oHv2jMzfbRQVKljasVlKC78Unqo1A48td6DIHEpd2944ZFd+d9 N49wF+JvGbXfNGmBw2T9kVHgmYu8XGEKOFhfpZRDeVFVJwjKYF21FL2RQOOzF/2zlFNy 5oOSVSYnkhg0+okB/fTpP8Ji6AeWR2fvNyvU/euEx1DOgegUZ2rwtC98tCtRrK/lVLmG +RNPWWBt+3FZc0m/9PvVo9ZJ1FWO66kilO78OQg/huKbqbnAG/WwpOoSsI3yul9k8zTg Sg7BUUfrEYj5E/7OJv1nkPbBXC0cezkqDYEuC3wuage0+rFUuCrFqhMMz7NXNWD55P6A IQ== Received: from pps.reinject (localhost [127.0.0.1]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3ng92mcj6b-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 02 Feb 2023 16:24:48 +0000 Received: from m0098409.ppops.net (m0098409.ppops.net [127.0.0.1]) by pps.reinject (8.17.1.5/8.17.1.5) with ESMTP id 312G6j37006829; Thu, 2 Feb 2023 16:24:48 GMT Received: from ppma04wdc.us.ibm.com (1a.90.2fa9.ip4.static.sl-reverse.com [169.47.144.26]) by mx0a-001b2d01.pphosted.com (PPS) with ESMTPS id 3ng92mcj5u-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 02 Feb 2023 16:24:48 +0000 Received: from pps.filterd (ppma04wdc.us.ibm.com [127.0.0.1]) by ppma04wdc.us.ibm.com (8.17.1.19/8.17.1.19) with ESMTP id 312FJxYM028526; Thu, 2 Feb 2023 16:24:46 GMT Received: from smtprelay07.wdc07v.mail.ibm.com ([9.208.129.116]) by ppma04wdc.us.ibm.com (PPS) with ESMTPS id 3ncvuyvd7y-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Thu, 02 Feb 2023 16:24:46 +0000 Received: from smtpav04.dal12v.mail.ibm.com (smtpav04.dal12v.mail.ibm.com [10.241.53.103]) by smtprelay07.wdc07v.mail.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 312GOj167013096 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Thu, 2 Feb 2023 16:24:45 GMT Received: from smtpav04.dal12v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 760D45805A; Thu, 2 Feb 2023 16:24:45 +0000 (GMT) Received: from smtpav04.dal12v.mail.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id E0CD75804E; Thu, 2 Feb 2023 16:24:44 +0000 (GMT) Received: from [9.65.214.66] (unknown [9.65.214.66]) by smtpav04.dal12v.mail.ibm.com (Postfix) with ESMTP; Thu, 2 Feb 2023 16:24:44 +0000 (GMT) Message-ID: Date: Thu, 2 Feb 2023 10:24:44 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.6.0 Subject: Re: [PATCH v4 2/6] tpm: Support boot measurements To: Ilias Apalodimas Cc: u-boot@lists.denx.de, sjg@chromium.org, xypron.glpk@gmx.de References: <20230125171810.3724530-1-eajames@linux.ibm.com> <20230125171810.3724530-3-eajames@linux.ibm.com> Content-Language: en-US From: Eddie James In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed X-TM-AS-GCONF: 00 X-Proofpoint-GUID: 9HSM6XgHgG6AcbBEGqEP5OgkAJeyfd-W X-Proofpoint-ORIG-GUID: WrKg14JbNQMwNzzIDdDwsr5OY57aETig Content-Transfer-Encoding: 7bit X-Proofpoint-UnRewURL: 0 URL was un-rewritten MIME-Version: 1.0 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-02-02_10,2023-02-02_01,2022-06-22_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 mlxscore=0 spamscore=0 adultscore=0 bulkscore=0 mlxlogscore=999 suspectscore=0 impostorscore=0 priorityscore=1501 malwarescore=0 lowpriorityscore=0 clxscore=1015 phishscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2212070000 definitions=main-2302020143 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/26/23 01:51, Ilias Apalodimas wrote: > Hi Eddie, > > Thanks for the cleanup! Unfortunately this doesn't compile with EFI > selected, but in general it looks pretty good. Thanks, yes I forgot to remove tcg2_pcr_read > > On Wed, Jan 25, 2023 at 11:18:06AM -0600, Eddie James wrote: >> Add TPM2 functions to support boot measurement. This includes >> starting up the TPM, initializing/appending the event log, and >> measuring the U-Boot version. Much of the code was used in the >> EFI subsystem, so remove it there and use the common functions. >> >> Signed-off-by: Eddie James >> --- >> include/efi_tcg2.h | 44 -- >> include/tpm-v2.h | 254 ++++++++++ >> lib/efi_loader/efi_tcg2.c | 975 +++----------------------------------- >> lib/tpm-v2.c | 799 +++++++++++++++++++++++++++++++ >> 4 files changed, 1129 insertions(+), 943 deletions(-) >> >> HR_NV_INDEX = TPM_HT_NV_INDEX << HR_SHIFT, >> }; >> > [...] > >> } >> >> +} >> + >> +int tcg2_measure_data(struct udevice *dev, struct tcg2_event_log *elog, >> + u32 pcr_index, u32 size, const u8 *data, u32 event_type, >> + u32 event_size, const u8 *event) >> +{ >> + struct tpml_digest_values digest_list; >> + int rc; >> + >> + rc = tcg2_create_digest(dev, data, 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, >> + event_size, event); >> +} >> + >> +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); >> +} > The only difference between these 2 is the measured data. Can't we make > one function? If data = NULL we could just measure the event Ok, yes that should work. I'll probably add a macro for tcg2_measure_event that fills in NULL and 0 for data and size for convenience. > >> + >> +int tcg2_init_log(struct udevice *dev, struct tcg2_event_log *elog) >> +{ >> + int rc; >> + >> + rc = tcg2_platform_get_log(dev, (void **)&elog->log, &elog->log_size); >> + if (rc) >> + return rc; >> + >> + elog->log_position = 0; >> + elog->found = false; >> + rc = tcg2_log_parse(dev, elog); >> + if (rc) >> + return rc; >> + >> + if (!elog->found) { >> + rc = tcg2_log_init(dev, elog); >> + if (rc) >> + return rc; > You can get rid of this if and just return rc on the function Ack. > >> + } >> + >> + return 0; >> +} >> + >> +int tcg2_measurement_init(struct udevice **dev, struct tcg2_event_log *elog) >> +{ >> + int rc; >> + >> + rc = tcg2_platform_get_tpm2(dev); >> + if (rc) >> + return rc; >> + >> + rc = tpm_init(*dev); >> + if (rc) >> + return rc; >> + >> + rc = tpm2_startup(*dev, TPM2_SU_CLEAR); >> + if (rc) { >> + tcg2_platform_startup_error(*dev, rc); >> + return rc; >> + } >> + >> + rc = tpm2_self_test(*dev, TPMI_YES); >> + if (rc) >> + printf("%s: self test error, continuing.\n", __func__); > This is the correct init sequence of the TPM. I was trying to solve > something similar a few days ago [0]. I haven't merged that patch yet, > but I will send a new version that also calls tpm_init() and doesn't exit > on -EBUSY. Can you use that patch instead of open coding the init > sequence? Yes I'd be happy to use it. I'll just base my series on yours. > Also we should be bailing out on selftest errors?' Maybe so, yes. Whatever behavior your auto startup function implements will be fine, I'm sure. > > The problem here is that with U-Boot's lazy binding we might need to init > the TPM in other subsystems and at the very least we can have a function > doing that for us reliably. > >> + >> + rc = tcg2_init_log(*dev, elog); >> + if (rc) { >> + tcg2_measurement_term(*dev, elog, true); >> + return rc; >> + } >> + >> + rc = tcg2_measure_event(*dev, elog, 0, EV_S_CRTM_VERSION, >> + strlen(version_string) + 1, >> + (u8 *)version_string); >> + if (rc) { >> + tcg2_measurement_term(*dev, elog, true); >> + return rc; >> + } >> + >> + return 0; >> +} >> + >> +void tcg2_measurement_term(struct udevice *dev, struct tcg2_event_log *elog, >> + bool error) >> +{ >> + u32 event = error ? 0x1 : 0xffffffff; >> + int i; >> + >> + for (i = 0; i < 8; ++i) >> + tcg2_measure_event(dev, elog, i, EV_SEPARATOR, sizeof(event), >> + (const u8 *)&event); > Is the separator event at the end of the eventlog described in the spec? Yes. https://trustedcomputinggroup.org/wp-content/uploads/TCG_ServerManagDomainFWProfile_r1p00_pub.pdf See section 7.1.1 > >> + >> + if (elog->log) >> + unmap_physmem(elog->log, MAP_NOCACHE); >> +} >> + >> +__weak int tcg2_platform_get_log(struct udevice *dev, void **addr, u32 *size) >> +{ >> + const __be32 *addr_prop; >> + const __be32 *size_prop; >> + int asize; >> + int ssize; >> + >> + *addr = NULL; >> + *size = 0; >> + >> + addr_prop = dev_read_prop(dev, "linux,sml-base", &asize); >> + if (!addr_prop) >> + addr_prop = dev_read_prop(dev, "tpm_event_log_addr", &asize); >> + size_prop = dev_read_prop(dev, "linux,sml-size", &ssize); >> + if (!size_prop) >> + size_prop = dev_read_prop(dev, "tpm_event_log_size", &ssize); >> + if (addr_prop && size_prop) { >> + u64 a = of_read_number(addr_prop, asize / sizeof(__be32)); >> + u64 s = of_read_number(size_prop, ssize / sizeof(__be32)); >> + >> + *addr = map_physmem(a, s, MAP_NOCACHE); >> + *size = (u32)s; >> + } else { >> + struct ofnode_phandle_args args; >> + phys_addr_t a; >> + phys_size_t s; >> + >> + if (dev_read_phandle_with_args(dev, "memory-region", NULL, 0, >> + 0, &args)) >> + return -ENODEV; >> + >> + a = ofnode_get_addr_size(args.node, "reg", &s); >> + if (a == FDT_ADDR_T_NONE) >> + return -ENOMEM; >> + >> + *addr = map_physmem(a, s, MAP_NOCACHE); >> + *size = (u32)s; >> + } >> + >> + return 0; >> +} >> + >> +__weak int tcg2_platform_get_tpm2(struct udevice **dev) >> +{ >> + for_each_tpm_device(*dev) { >> + if (tpm_get_version(*dev) == TPM_V2) >> + return 0; >> + } >> + >> + return -ENODEV; >> +} >> + >> +__weak void tcg2_platform_startup_error(struct udevice *dev, int rc) {} >> + >> u32 tpm2_startup(struct udevice *dev, enum tpm2_startup_types mode) >> { >> const u8 command_v2[12] = { >> @@ -342,6 +1016,131 @@ u32 tpm2_get_capability(struct udevice *dev, u32 capability, u32 property, >> return 0; >> } >> >> +static int tpm2_get_num_pcr(struct udevice *dev, u32 *num_pcr) >> +{ >> + u8 response[(sizeof(struct tpms_capability_data) - >> + offsetof(struct tpms_capability_data, data))]; >> + u32 properties_offset = >> + offsetof(struct tpml_tagged_tpm_property, tpm_property) + >> + offsetof(struct tpms_tagged_property, value); >> + u32 ret; >> + >> + memset(response, 0, sizeof(response)); >> + ret = tpm2_get_capability(dev, TPM2_CAP_TPM_PROPERTIES, >> + TPM2_PT_PCR_COUNT, response, 1); >> + if (ret) >> + return ret; >> + >> + *num_pcr = get_unaligned_be32(response + properties_offset); >> + if (*num_pcr > TPM2_MAX_PCRS) { >> + printf("%s: too many pcrs: %u\n", __func__, *num_pcr); >> + return -E2BIG; >> + } >> + >> + return 0; >> +} >> + > [...] > > [0] https://lore.kernel.org/u-boot/20230125144851.532154-1-ilias.apalodimas@linaro.org/ > > Thanks > /Ilias