[PATCH AliOS 4.19 v3 11/15] KEYS: trusted: allow module init if TPM is inactive or deactivated

[PATCH AliOS 4.19 v3 11/15] KEYS: trusted: allow module init if TPM is inactive or deactivated

[Shan]

From: Roberto Sassu <roberto.sassu@huawei.com>

commit 2d6c25215ab26bb009de3575faab7b685f138e92 upstream.

Commit c78719203fc6 ("KEYS: trusted: allow trusted.ko to initialize w/o a
TPM") allows the trusted module to be loaded even if a TPM is not found, to
avoid module dependency problems.

However, trusted module initialization can still fail if the TPM is
inactive or deactivated. tpm_get_random() returns an error.

This patch removes the call to tpm_get_random() and instead extends the PCR
specified by the user with zeros. The security of this alternative is
equivalent to the previous one, as either option prevents with a PCR update
unsealing and misuse of sealed data by a user space process.

Even if a PCR is extended with zeros, instead of random data, it is still
computationally infeasible to find a value as input for a new PCR extend
operation, to obtain again the PCR value that would allow unsealing.

Cc: stable@vger.kernel.org
Fixes: 240730437deb ("KEYS: trusted: explicitly use tpm_chip structure...")
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
Reviewed-by: Tyler Hicks <tyhicks@canonical.com>
Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

Signed-off-by: mayuanchen <mayuanchen@hygon.cn>
Change-Id: Iada0e052c2ab4a0fbc2db4ac2690da3115d985c6
Signed-off-by: Shan <chenshan@hygon.cn>
---
 security/keys/trusted.c | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/security/keys/trusted.c b/security/keys/trusted.c
index 5e983eb9a..b03525d0f 100644
--- a/security/keys/trusted.c
+++ b/security/keys/trusted.c
@@ -1216,24 +1216,11 @@ static int __init trusted_shash_alloc(void)
 
 static int __init init_digests(void)
 {
-	u8 digest[TPM_MAX_DIGEST_SIZE];
-	int ret;
-	int i;
-
-	ret = tpm_get_random(chip, digest, TPM_MAX_DIGEST_SIZE);
-	if (ret < 0)
-		return ret;
-	if (ret < TPM_MAX_DIGEST_SIZE)
-		return -EFAULT;
-
 	digests = kcalloc(chip->nr_allocated_banks, sizeof(*digests),
 					  GFP_KERNEL);
 	if (!digests)
 		return -ENOMEM;
 
-	for (i = 0; i < chip->nr_allocated_banks; i++)
-		memcpy(digests[i].digest, digest, TPM_MAX_DIGEST_SIZE);
-
 	return 0;
 }
 
-- 
2.17.1

Re: [PATCH AliOS 4.19 v3 11/15] KEYS: trusted: allow module init if TPM is inactive or deactivated

[Greg KH]

On Tue, Dec 15, 2020 at 04:29:18PM +0800, Shan wrote:
> From: Roberto Sassu <roberto.sassu@huawei.com>
> 
> commit 2d6c25215ab26bb009de3575faab7b685f138e92 upstream.
> 
> Commit c78719203fc6 ("KEYS: trusted: allow trusted.ko to initialize w/o a
> TPM") allows the trusted module to be loaded even if a TPM is not found, to
> avoid module dependency problems.
> 
> However, trusted module initialization can still fail if the TPM is
> inactive or deactivated. tpm_get_random() returns an error.
> 
> This patch removes the call to tpm_get_random() and instead extends the PCR
> specified by the user with zeros. The security of this alternative is
> equivalent to the previous one, as either option prevents with a PCR update
> unsealing and misuse of sealed data by a user space process.
> 
> Even if a PCR is extended with zeros, instead of random data, it is still
> computationally infeasible to find a value as input for a new PCR extend
> operation, to obtain again the PCR value that would allow unsealing.
> 
> Cc: stable@vger.kernel.org
> Fixes: 240730437deb ("KEYS: trusted: explicitly use tpm_chip structure...")
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> Reviewed-by: Tyler Hicks <tyhicks@canonical.com>
> Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> 
> Signed-off-by: mayuanchen <mayuanchen@hygon.cn>
> Change-Id: Iada0e052c2ab4a0fbc2db4ac2690da3115d985c6
> Signed-off-by: Shan <chenshan@hygon.cn>
> ---
>  security/keys/trusted.c | 13 -------------
>  1 file changed, 13 deletions(-)

Why is this being sent to the stable list?  Do you want this backported
to 4.19.y?  If so, why, and what is the change-id stuff in there for?

confused,

greg k-h

答复: [PATCH AliOS 4.19 v3 11/15] KEYS: trusted: allow module init if TPM is inactive or deactivated

[Shan Chen]

-Shan

> -----邮件原件-----
> 发件人: Greg KH [mailto:gregkh@linuxfoundation.org]
> 发送时间: 2020年12月15日 17:24
> 收件人: Shan Chen <chenshan@hygon.cn>
> 抄送: alikernel-developer@linux.alibaba.com; Roberto Sassu
> <roberto.sassu@huawei.com>; Yuanchen Ma <mayuanchen@hygon.cn>; Hao
> Feng <fenghao@hygon.cn>; Zhiwei Ying <yingzhiwei@hygon.cn>;
> stable@vger.kernel.org; Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> 主题: Re: [PATCH AliOS 4.19 v3 11/15] KEYS: trusted: allow module init if TPM
> is inactive or deactivated
> 
> On Tue, Dec 15, 2020 at 04:29:18PM +0800, Shan wrote:
> > From: Roberto Sassu <roberto.sassu@huawei.com>
> >
> > commit 2d6c25215ab26bb009de3575faab7b685f138e92 upstream.
> >
> > Commit c78719203fc6 ("KEYS: trusted: allow trusted.ko to initialize
> > w/o a
> > TPM") allows the trusted module to be loaded even if a TPM is not
> > found, to avoid module dependency problems.
> >
> > However, trusted module initialization can still fail if the TPM is
> > inactive or deactivated. tpm_get_random() returns an error.
> >
> > This patch removes the call to tpm_get_random() and instead extends
> > the PCR specified by the user with zeros. The security of this
> > alternative is equivalent to the previous one, as either option
> > prevents with a PCR update unsealing and misuse of sealed data by a user
> space process.
> >
> > Even if a PCR is extended with zeros, instead of random data, it is
> > still computationally infeasible to find a value as input for a new
> > PCR extend operation, to obtain again the PCR value that would allow
> unsealing.
> >
> > Cc: stable@vger.kernel.org
> > Fixes: 240730437deb ("KEYS: trusted: explicitly use tpm_chip
> > structure...")
> > Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
> > Reviewed-by: Tyler Hicks <tyhicks@canonical.com>
> > Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
> > Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> >
> > Signed-off-by: mayuanchen <mayuanchen@hygon.cn>
> > Change-Id: Iada0e052c2ab4a0fbc2db4ac2690da3115d985c6
> > Signed-off-by: Shan <chenshan@hygon.cn>
> > ---
> >  security/keys/trusted.c | 13 -------------
> >  1 file changed, 13 deletions(-)
> 
> Why is this being sent to the stable list?  Do you want this backported to
> 4.19.y?  If so, why, and what is the change-id stuff in there for?
> 
> confused,
> 
> greg k-h

Sorry for the disturbing, it's not meant for the kernel community. We're backporting this commit for some private usage, and carelessly sent out this mail as git send-email automatically cc'ed the sob listed addresses. Have had the cc suppressed. pls ignore. Thanks!

Shan

[PATCH AliOS 4.19 v3 12/15] KEYS: trusted: correctly initialize digests and fix locking issue

[Shan]

From: Roberto Sassu <roberto.sassu@huawei.com>

commit 9f75c82246313d4c2a6bc77e947b45655b3b5ad5 upstream.

Commit 0b6cf6b97b7e ("tpm: pass an array of tpm_extend_digest structures to
tpm_pcr_extend()") modifies tpm_pcr_extend() to accept a digest for each
PCR bank. After modification, tpm_pcr_extend() expects that digests are
passed in the same order as the algorithms set in chip->allocated_banks.

This patch fixes two issues introduced in the last iterations of the patch
set: missing initialization of the TPM algorithm ID in the tpm_digest
structures passed to tpm_pcr_extend() by the trusted key module, and
unreleased locks in the TPM driver due to returning from tpm_pcr_extend()
without calling tpm_put_ops().

Cc: stable@vger.kernel.org
Fixes: 0b6cf6b97b7e ("tpm: pass an array of tpm_extend_digest structures to tpm_pcr_extend()")
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
Suggested-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Reviewed-by: Jerry Snitselaar <jsnitsel@redhat.com>
Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>

Signed-off-by: mayuanchen <mayuanchen@hygon.cn>
Change-Id: If1f7d414bcf2d8189d07623fea04d0b5db7060d8
Signed-off-by: Shan <chenshan@hygon.cn>
---
 drivers/char/tpm/tpm-interface.c | 14 +++++++++-----
 security/keys/trusted.c          |  5 +++++
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 46e0882d6..d0303d298 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -1057,18 +1057,22 @@ int tpm_pcr_extend(struct tpm_chip *chip, u32 pcr_idx,
 	if (!chip)
 		return -ENODEV;
 
-	for (i = 0; i < chip->nr_allocated_banks; i++)
-		if (digests[i].alg_id != chip->allocated_banks[i].alg_id)
-			return -EINVAL;
+	for (i = 0; i < chip->nr_allocated_banks; i++) {
+		if (digests[i].alg_id != chip->allocated_banks[i].alg_id) {
+			rc = EINVAL;
+			goto out;
+		}
+	}
 
 	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
 		rc = tpm2_pcr_extend(chip, pcr_idx, digests);
-		tpm_put_ops(chip);
-		return rc;
+		goto out;
 	}
 
 	rc = tpm1_pcr_extend(chip, pcr_idx, digests[0].digest,
 			     "attempting extend a PCR value");
+
+out:
 	tpm_put_ops(chip);
 	return rc;
 }
diff --git a/security/keys/trusted.c b/security/keys/trusted.c
index b03525d0f..536970168 100644
--- a/security/keys/trusted.c
+++ b/security/keys/trusted.c
@@ -1216,11 +1216,16 @@ static int __init trusted_shash_alloc(void)
 
 static int __init init_digests(void)
 {
+	int i;
+
 	digests = kcalloc(chip->nr_allocated_banks, sizeof(*digests),
 					  GFP_KERNEL);
 	if (!digests)
 		return -ENOMEM;
 
+	for (i = 0; i < chip->nr_allocated_banks; i++)
+		digests[i].alg_id = chip->allocated_banks[i].alg_id;
+
 	return 0;
 }
 
-- 
2.17.1