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 90D98C54E94 for ; Tue, 24 Jan 2023 13:20:24 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id C8A20855EB; Tue, 24 Jan 2023 14:20:21 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=linaro.org 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=linaro.org header.i=@linaro.org header.b="cxqgM+G2"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 6ECDD854D2; Tue, 24 Jan 2023 14:20:19 +0100 (CET) Received: from mail-ej1-x62e.google.com (mail-ej1-x62e.google.com [IPv6:2a00:1450:4864:20::62e]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 55DC285608 for ; Tue, 24 Jan 2023 14:20:14 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=ilias.apalodimas@linaro.org Received: by mail-ej1-x62e.google.com with SMTP id tz11so39024829ejc.0 for ; Tue, 24 Jan 2023 05:20:14 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=A/WpskcVMe6g8pG+KS6LC+DDawj3YGT2n3t7EeHbdVU=; b=cxqgM+G2gpPOVeUgV0Oda/Ab1XhfVDBOB9yO1MOujZbOYdB8vmMHgTKPXzK9ZB5i1q ofWiRkoJ3Tx3KXfj0TxjKRAZ30NK4Dcx+rhthqVBtvpFKxqCndiV9yppcFkhX4jIDT9v +duXBa0TAQF94Oi39YK2O/yIikq1tpZdOzN64x/01TWNt2Y3uqodVfvz4YkBGYbn0nUS wozLqsYULLjoPXDiQjIMoHIxWH0ROC2yH4KWsw4zZ2WCM96U5dyW+MHg7POyc+FignAJ HQadgDsTt4+WdeT5nj+KkrrOYegHswQAikixfABWFiMp6vSUSXqTAhcfkYXBod1x7RQB 4fOw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=A/WpskcVMe6g8pG+KS6LC+DDawj3YGT2n3t7EeHbdVU=; b=2T1b81k85CXqkEoded6LBB6/GT+DVqKWY41ySdRn1OLYi9+rjrj6Cvh/Pw6TwSLadq CcbrqJqHwXnKOnPocGWj6f+LTi/Ky9BUQxesdnynv62c6Dd4QsCn/aVw9yk2SaVgfpHr UShRIAKpfjzmn6lq/V66GqFEzlogFurL8OGN45QDIjomloLw638q5vqugswJh67gOoEM ApYWXNGFQa0aJ/NREKJe0CkEcVtM/y+ubSgHKPv5mDAXo83z3qsFJ5QYQvIOj+HnWG4m QL46C699pm8KXUT5PJHfjT4TG+RwnTy0nTMARuVuGWdz0lIwtKaq2uQVAC8JjR456yd3 o+QQ== X-Gm-Message-State: AFqh2koB2misowlgjBjHriScztF6fqH5mHSZr30jtBC35iUEpb27o2if D8/bc4WTYzwq8GHUiimvQzMcWQ== X-Google-Smtp-Source: AMrXdXtU0J5YEkeUv0QLwiVzE390gFbf41ArztxLKTW0idgQP95OTaxMRMp39GdnRWFIt7EvDk4hLA== X-Received: by 2002:a17:907:80c7:b0:84c:eca0:5f67 with SMTP id io7-20020a17090780c700b0084ceca05f67mr31021984ejc.54.1674566413836; Tue, 24 Jan 2023 05:20:13 -0800 (PST) Received: from hera (ppp079167090036.access.hol.gr. [79.167.90.36]) by smtp.gmail.com with ESMTPSA id l12-20020a170906078c00b0079800b8173asm882954ejc.158.2023.01.24.05.20.12 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 24 Jan 2023 05:20:13 -0800 (PST) Date: Tue, 24 Jan 2023 15:20:11 +0200 From: Ilias Apalodimas To: Eddie James Cc: u-boot@lists.denx.de, sjg@chromium.org, xypron.glpk@gmx.de Subject: Re: [PATCH v3 2/6] tpm: Support boot measurements Message-ID: References: <20230112161607.282165-1-eajames@linux.ibm.com> <20230112161607.282165-3-eajames@linux.ibm.com> <6193164e-b7f3-ea80-b843-614e1004507c@linux.ibm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <6193164e-b7f3-ea80-b843-614e1004507c@linux.ibm.com> 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 Hi Eddie, On Mon, Jan 23, 2023 at 02:15:18PM -0600, Eddie James wrote: > > 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? Sure that's fine. I just prefer a single implementation of getting sizes and masks > > > > > + > > > +#define tpm2_algorithm_to_mask(a) (1 << (a)) > > > + > > > /* NV index attributes */ > > > + [...] > > > + 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. > It's been a while since I wrote this but from memory you should 1. move the code from efi_init_event_log() and replace the memory allocation calls from efi -> calloc 2. get rid of that create_final_event() since it's part of the EFI TCG protocol. 3. Pass the eventlog to the EFI code eventually, allocate EFI memory and copy it and create the final events table. The main problem I can see is handling of the EFI Final Events table but I think it's doable. > > > > > > + 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. I think the only difference is the final events log table which is described in the EFI TCG spec. Rename it on the next version, I'll have a quick look in case we can unify these two as well Thanks /Ilias > > > 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