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 X-Spam-Level: X-Spam-Status: No, score=-1.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D4895C169C4 for ; Wed, 6 Feb 2019 16:07:59 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 99FD92186A for ; Wed, 6 Feb 2019 16:07:59 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727212AbfBFQH7 (ORCPT ); Wed, 6 Feb 2019 11:07:59 -0500 Received: from lhrrgout.huawei.com ([185.176.76.210]:32876 "EHLO huawei.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1726270AbfBFQH7 (ORCPT ); Wed, 6 Feb 2019 11:07:59 -0500 Received: from LHREML713-CAH.china.huawei.com (unknown [172.18.7.107]) by Forcepoint Email with ESMTP id 2CBC3381FA0F0E930B1B; Wed, 6 Feb 2019 16:07:57 +0000 (GMT) Received: from [10.204.65.155] (10.204.65.155) by smtpsuk.huawei.com (10.201.108.36) with Microsoft SMTP Server (TLS) id 14.3.408.0; Wed, 6 Feb 2019 16:07:49 +0000 Subject: Re: [PATCH v10 0/6] tpm: retrieve digest size of unknown algorithms from TPM To: Jarkko Sakkinen CC: , , , , , , , , References: <20190206105724.14495-1-roberto.sassu@huawei.com> <20190206125725.GA9257@linux.intel.com> <1fff7b54-cc9a-7705-4c26-819bdd1286f2@huawei.com> <20190206135450.GA20061@linux.intel.com> <2dea31e3-0796-e21f-5625-1ddc78485c71@huawei.com> <20190206153445.GA23970@linux.intel.com> From: Roberto Sassu Message-ID: <3cd14c24-9de2-d347-b40c-22ca87d9df87@huawei.com> Date: Wed, 6 Feb 2019 17:07:49 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101 Thunderbird/60.3.0 MIME-Version: 1.0 In-Reply-To: <20190206153445.GA23970@linux.intel.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Originating-IP: [10.204.65.155] X-CFilter-Loop: Reflected Sender: linux-integrity-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-integrity@vger.kernel.org On 2/6/2019 4:34 PM, Jarkko Sakkinen wrote: > On Wed, Feb 06, 2019 at 03:25:48PM +0100, Roberto Sassu wrote: >> On 2/6/2019 2:54 PM, Jarkko Sakkinen wrote: >>> On Wed, Feb 06, 2019 at 02:07:04PM +0100, Roberto Sassu wrote: >>>> On 2/6/2019 1:57 PM, Jarkko Sakkinen wrote: >>>>> On Wed, Feb 06, 2019 at 11:57:18AM +0100, Roberto Sassu wrote: >>>>>> Update >>>>>> >>>>>> This version of the patch set includes three additional patches (5-7/7) >>>>>> that allow users of the TPM driver to provide a digest for each PCR bank to >>>>>> tpm_pcr_extend(). The new patches have been included to facilitate the >>>>>> review of all the changes to support TPM 2.0 crypto agility for reading and >>>>>> extending PCRs. >>>>>> >>>>>> >>>>>> Original patch set description >>>>>> >>>>>> The TPM driver currently relies on the crypto subsystem to determine the >>>>>> digest size of supported TPM algorithms. In the future, TPM vendors might >>>>>> implement new algorithms in their chips, and those algorithms might not >>>>>> be supported by the crypto subsystem. >>>>>> >>>>>> Usually, vendors provide patches for the new hardware, and likely >>>>>> the crypto subsystem will be updated before the new algorithm is >>>>>> introduced. However, old kernels might be updated later, after patches >>>>>> are included in the mainline kernel. This would leave the opportunity >>>>>> for attackers to misuse PCRs, as PCR banks with an unknown algorithm >>>>>> are not extended. >>>>>> >>>>>> This patch set provides a long term solution for this issue. If a TPM >>>>>> algorithm is not known by the crypto subsystem, the TPM driver retrieves >>>>>> the digest size from the TPM with a PCR read. All the PCR banks are >>>>>> extended, even if the algorithm is not yet supported by the crypto >>>>>> subsystem. >>>>>> >>>>>> PCR bank information (TPM algorithm ID, digest size, crypto subsystem ID) >>>>>> is stored in the tpm_chip structure and available for users of the TPM >>>>>> driver. >>>>>> >>>>>> Changelog >>>>>> >>>>>> v9: >>>>>> - add comment for ima_load_kexec_buffer() in ima_init() >>>>>> - move 'digests' and ima_init_digests() to ima_queue.c >>>>>> - remove comment for TPM_RETRY >>>>>> >>>>>> v8: >>>>>> - set digest array length in tpm_digest to new constant TPM_MAX_DIGEST_SIZE >>>>>> - replace enum tpm_const with #define >>>>>> - rename labels in init_trusted() >>>>>> - allocate tpm_digest array after retrieving random data in init_digests() >>>>>> >>>>>> v7: >>>>>> - use memchr_inv() in tpm2_get_pcr_allocation() >>>>>> - add patch to move tpm_chip to include/linux/tpm.h >>>>>> - add patch to set the tpm_chip argument in trusted.c to a pointer from >>>>>> tpm_default_chip() >>>>>> - remove definition of tpm_extend_digest >>>>>> - remove code in tpm2_pcr_extend() to extend unused PCR banks with the >>>>>> first digest passed by the caller of tpm_pcr_extend() >>>>>> - remove count parameter from tpm_pcr_extend() and tpm2_pcr_extend() >>>>>> - remove padding of SHA1 digest in tpm_pcr_extend() >>>>>> - pre-allocate and initialize array of tpm_digest structures in >>>>>> security/keys/trusted.c and security/integrity/ima/ima_init.c >>>>>> >>>>>> v6: >>>>>> - squash patches 4-6 >>>>>> - rename tpm_bank_list to tpm_extend_digest, extend_size and extend_data >>>>>> members to size and data >>>>>> - add comment in tpm2_init_bank_info() >>>>>> >>>>>> v5: >>>>>> - rename digest_struct variable to digest >>>>>> - add _head suffix to tcg_efi_specid_event and tcg_pcr_event2 >>>>>> - rename digest_size member of tpm_bank_list to extend_size >>>>>> - change type of alg_id member of tpm_bank_list from u8 to u16 >>>>>> - add missing semi-colon in pcrlock() >>>>>> >>>>>> v4: >>>>>> - rename active_banks to allocated_banks >>>>>> - replace kmalloc_array() with kcalloc() >>>>>> - increment nr_allocated_banks if at least one PCR in the bank is selected >>>>>> - pass multiple digests to tpm_pcr_extend() >>>>>> >>>>>> v3: >>>>>> - remove end marker change >>>>>> - replace active_banks static array with pointer to dynamic array >>>>>> - remove TPM2_ACTIVE_PCR_BANKS >>>>>> >>>>>> v2: >>>>>> - change the end marker of the active_banks array >>>>>> - check digest size from output of PCR read command >>>>>> - remove count parameter from tpm_pcr_read() and tpm2_pcr_read() >>>>>> >>>>>> v1: >>>>>> - modify definition of tpm_pcr_read() >>>>>> - move hash algorithms and definition of tpm2_digest to include/linux/tpm.h >>>>>> >>>>>> >>>>>> Roberto Sassu (6): >>>>>> tpm: dynamically allocate the allocated_banks array >>>>>> tpm: rename and export tpm2_digest and tpm2_algorithms >>>>>> tpm: retrieve digest size of unknown algorithms with PCR read >>>>>> tpm: move tpm_chip definition to include/linux/tpm.h >>>>>> KEYS: trusted: explicitly use tpm_chip structure from >>>>>> tpm_default_chip() >>>>>> tpm: pass an array of tpm_extend_digest structures to tpm_pcr_extend() >>>>>> >>>>>> drivers/char/tpm/tpm-chip.c | 1 + >>>>>> drivers/char/tpm/tpm-interface.c | 38 ++++---- >>>>>> drivers/char/tpm/tpm.h | 117 ++--------------------- >>>>>> drivers/char/tpm/tpm1-cmd.c | 12 +++ >>>>>> drivers/char/tpm/tpm2-cmd.c | 139 ++++++++++++++++++++-------- >>>>>> include/linux/tpm.h | 126 ++++++++++++++++++++++++- >>>>>> include/linux/tpm_eventlog.h | 9 +- >>>>>> security/integrity/ima/ima.h | 1 + >>>>>> security/integrity/ima/ima_crypto.c | 10 +- >>>>>> security/integrity/ima/ima_init.c | 4 + >>>>>> security/integrity/ima/ima_queue.c | 27 +++++- >>>>>> security/keys/trusted.c | 73 +++++++++++---- >>>>>> 12 files changed, 349 insertions(+), 208 deletions(-) >>>>>> >>>>>> -- >>>>>> 2.17.1 >>>>>> >>>>> >>>>> Can you do two things: >>>>> >>>>> 1. Check the v3: https://patchwork.kernel.org/patch/10797181/. Needs >>>>> to be applied first. >>>>> 2. Check that this does not cause merge conflicts with the current >>>>> imaster? If it does, fix them, and use --subject-prefix="RESEND,PATCH v10". >>>>> >>>>> Thanks. >>>> >>>> I already applied the patch set on top of linux-tpmdd/next >>>> (41ac27d19b07) and your patch. >>>> >>>> Also, there are no conflicts with the current master. >>>> >>>> Roberto >>> >>> Before getting your new version, I applied: >>> >>> https://patchwork.kernel.org/project/linux-integrity/list/?series=76223 >>> >>> This causes some merge conflicts: >>> >>> error: Failed to merge in the changes. >>> Patch failed at 0003 tpm: retrieve digest size of unknown algorithms with PCR read >> >> I fixed it and patch 4/6. But, I think there is a problem in your patch >> 'tpm: move tpm_validate_commmand() to tpm2-space.c': >> >> --- >> -int tpm2_prepare_space(struct tpm_chip *chip, struct tpm_space *space, u32 >> cc, >> - u8 *cmd) >> +static int tpm_validate_command(struct tpm_chip *chip, struct tpm_space >> *space, >> + const void *cmd, size_t len) >> +{ >> + const struct tpm_header *header = (const void *)cmd; >> + int i; >> + u32 cc; >> + u32 attrs; >> + unsigned int nr_handles; >> + >> + if (len < TPM_HEADER_SIZE) >> + return -EINVAL; >> + >> + if (chip->nr_commands) { >> >> [...] >> >> + } >> + >> + return cc; >> + >> --- >> >> 'cc' may be not initialized before returning the value. > > Ah. Thank you for catching that one. Can you quickly check the attached > commit? I'll squash it to the corresponding commit. Seems ok to me. I'll send patch 3/6 and 4/6. Roberto > /Jarkko > -- HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063 Managing Director: Bo PENG, Jian LI, Yanli SHI