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=ham 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 0AEABC169C4 for ; Wed, 6 Feb 2019 14:26:14 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id C94CD20844 for ; Wed, 6 Feb 2019 14:26:13 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726708AbfBFO0N (ORCPT ); Wed, 6 Feb 2019 09:26:13 -0500 Received: from lhrrgout.huawei.com ([185.176.76.210]:32875 "EHLO huawei.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1725928AbfBFO0M (ORCPT ); Wed, 6 Feb 2019 09:26:12 -0500 Received: from LHREML713-CAH.china.huawei.com (unknown [172.18.7.107]) by Forcepoint Email with ESMTP id D1B90AA0F8A33B4598DB; Wed, 6 Feb 2019 14:26:10 +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 14:25: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> From: Roberto Sassu Message-ID: <2dea31e3-0796-e21f-5625-1ddc78485c71@huawei.com> Date: Wed, 6 Feb 2019 15:25:48 +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: <20190206135450.GA20061@linux.intel.com> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Originating-IP: [10.204.65.155] X-CFilter-Loop: Reflected Sender: owner-linux-security-module@vger.kernel.org Precedence: bulk List-ID: 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. Roberto > /Jarkko > -- HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063 Managing Director: Bo PENG, Jian LI, Yanli SHI