Re: [PATCH] security/keys/trusted: Allow operation without hardware TPM

From: Roberto Sassu
Date: Thu Mar 21 2019 - 13:46:08 EST


On 3/21/2019 5:30 PM, Dan Williams wrote:
On Thu, Mar 21, 2019 at 7:27 AM Roberto Sassu <roberto.sassu@xxxxxxxxxx> wrote:

On 3/21/2019 2:54 PM, Jarkko Sakkinen wrote:
On Mon, Mar 18, 2019 at 04:45:13PM -0700, Dan Williams wrote:
Rather than fail initialization of the trusted.ko module, arrange for
the module to load, but rely on trusted_instantiate() to fail
trusted-key operations.

Fixes: 240730437deb ("KEYS: trusted: explicitly use tpm_chip structure...")
Cc: Roberto Sassu <roberto.sassu@xxxxxxxxxx>
Cc: Jarkko Sakkinen <jarkko.sakkinen@xxxxxxxxxxxxxxx>
Cc: James Bottomley <jejb@xxxxxxxxxxxxx>
Cc: Jarkko Sakkinen <jarkko.sakkinen@xxxxxxxxxxxxxxx>
Cc: Mimi Zohar <zohar@xxxxxxxxxxxxx>
Cc: David Howells <dhowells@xxxxxxxxxx>
Signed-off-by: Dan Williams <dan.j.williams@xxxxxxxxx>

It should check for chip in each function that uses TPM now that
the code does not rely on default chip. Otherwise, the semantics
are kind of inconsistent.

If no other TPM function can be used before a successful key
instantiate, checking for a chip only in trusted_instantiate() seems
sufficient. Then, the same chip will be used by all TPM functions until
module unloading, since we incremented the reference count.

I would suggest to move the tpm_default_chip() and init_digests() calls
to trusted_instantiate() to restore the old behavior of init_trusted().

trusted_instantiate() should look like:
---
if (!chip) {
chip = tpm_default_chip();
if (!chip)
return -ENODEV;
}

if (!digests) {
ret = init_digests();
if (ret < 0)
return ret;
}

This patch already achieves that because tpm_find_get_ops() will fail
and cause tpm_is_tpm2() to return NULL.

In addition, the changes I proposed would allow users to create trusted
keys if a TPM is added later. CONFIG_TRUSTED_KEYS=y and
CONFIG_TCG_TPM=m is a valid configuration.

Jarkko, Dan's patch seems sufficient to fix the issue. He could include
the changes I proposed in his patch. What is your opinion?

Thanks

Roberto

--
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Bo PENG, Jian LI, Yanli SHI