[GIT PULL] TPM DEVICE DRIVER changes for v5.14

[GIT PULL] TPM DEVICE DRIVER changes for v5.14

[Jarkko Sakkinen]

Hi,

Contains bug fixes for TPM, and support for signing modules using elliptic
curve keys, which I promised to pick up to my tree.

/Jarkko

The following changes since commit 0c18f29aae7ce3dadd26d8ee3505d07cc982df75:

  module: limit enabling module.sig_enforce (2021-06-22 11:13:19 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git/ tags/tpmdd-next-v5.14-rc1

for you to fetch changes up to 87e9688481163dee836c7f86e02f9aaf3240af2e:

  certs: Add support for using elliptic curve keys for signing modules (2021-06-23 16:51:04 +0300)

----------------------------------------------------------------
tpmdd updates for Linux v5.14-rc1

----------------------------------------------------------------
Amir Mizinski (1):
      tpm: add longer timeout for TPM2_CC_VERIFY_SIGNATURE

Jarkko Sakkinen (1):
      tpm: Replace WARN_ONCE() with dev_err_once() in tpm_tis_status()

Javier Martinez Canillas (1):
      tpm_tis_spi: add missing SPI device ID entries

Liguang Zhang (1):
      tpm_tis_spi: set default probe function if device id not match

Stefan Berger (2):
      certs: Trigger creation of RSA module signing key if it's not an RSA key
      certs: Add support for using elliptic curve keys for signing modules

Tian Tao (2):
      tpm_crb: Use IOMEM_ERR_PTR when function returns iomem
      char: tpm: move to use request_irq by IRQF_NO_AUTOEN flag

Yang Yingliang (1):
      tpm: fix some doc warnings in tpm1-cmd.c

Zhen Lei (1):
      tpm_tis: Use DEFINE_RES_MEM() to simplify code

 certs/Kconfig                         | 26 ++++++++++++++++++++++++++
 certs/Makefile                        | 21 +++++++++++++++++++++
 crypto/asymmetric_keys/pkcs7_parser.c |  8 ++++++++
 drivers/char/tpm/tpm1-cmd.c           |  4 ++--
 drivers/char/tpm/tpm2-cmd.c           |  2 +-
 drivers/char/tpm/tpm_crb.c            |  2 +-
 drivers/char/tpm/tpm_tis.c            |  6 +-----
 drivers/char/tpm/tpm_tis_core.c       | 25 ++++++++++++++++++-------
 drivers/char/tpm/tpm_tis_core.h       |  3 ++-
 drivers/char/tpm/tpm_tis_i2c_cr50.c   |  4 ++--
 drivers/char/tpm/tpm_tis_spi_main.c   | 14 ++++++++++----
 11 files changed, 92 insertions(+), 23 deletions(-)

Re: [GIT PULL] TPM DEVICE DRIVER changes for v5.14

[Linus Torvalds]

On Wed, Jun 23, 2021 at 6:56 AM Jarkko Sakkinen <jarkko@kernel.org> wrote:
>
> Contains bug fixes for TPM, and support for signing modules using elliptic
> curve keys, which I promised to pick up to my tree.

I pulled this, but then I looked at the key type changes, and that
made me so scared that I unpulled it again.

In particular, that code will do

    shell rm -f $(CONFIG_MODULE_SIG_KEY)

from the Makefile if some config options have changed.

That just seems too broken for words. Maybe I misunderstand this, but
this really seems like an easy mistake might cause the kernel build to
actively start removing some random user key files that the user
pointed at previously.

                  Linus

Re: [GIT PULL] TPM DEVICE DRIVER changes for v5.14

[Stefan Berger]

On 6/28/21 1:34 PM, Linus Torvalds wrote:
> On Wed, Jun 23, 2021 at 6:56 AM Jarkko Sakkinen <jarkko@kernel.org> wrote:
>> Contains bug fixes for TPM, and support for signing modules using elliptic
>> curve keys, which I promised to pick up to my tree.
> I pulled this, but then I looked at the key type changes, and that
> made me so scared that I unpulled it again.
>
> In particular, that code will do
>
>      shell rm -f $(CONFIG_MODULE_SIG_KEY)
>
> from the Makefile if some config options have changed.

I suppose it is from this part here.

+# Support user changing key type
+ifdef CONFIG_MODULE_SIG_KEY_TYPE_ECDSA
+keytype_openssl = -newkey ec -pkeyopt ec_paramgen_curve:secp384r1
+ifeq ($(openssl_available),yes)
+$(if $(findstring id-ecPublicKey,$(X509TEXT)),,$(shell rm -f 
$(CONFIG_MODULE_SIG_KEY)))
+endif
+endif # CONFIG_MODULE_SIG_KEY_TYPE_ECDSA
+
+ifdef CONFIG_MODULE_SIG_KEY_TYPE_RSA
+ifeq ($(openssl_available),yes)
  $(if $(findstring rsaEncryption,$(X509TEXT)),,$(shell rm -f 
$(CONFIG_MODULE_SIG_KEY)))
  endif
+endif # CONFIG_MODULE_SIG_KEY_TYPE_RSA


If the user changed the build option from an ECDSA module signing key to 
an RSA signing key or vice versa then this code deletes the current 
signing key and subsequent code in the Makefile will create an RSA or 
ECDSA signing key following the user's choice. I suppose this is 
expected behavior that when a user chooses an RSA signing key it will 
use an RSA signing key. Maybe we should make a backup copy of the 
previous key, if that helps.


>
> That just seems too broken for words. Maybe I misunderstand this, but
> this really seems like an easy mistake might cause the kernel build to
> actively start removing some random user key files that the user
> pointed at previously.

The removal is triggered by the user changing the type of key from what 
is in the keyfile.

   Stefan



>
>                    Linus

Re: [GIT PULL] TPM DEVICE DRIVER changes for v5.14

[Linus Torvalds]

On Mon, Jun 28, 2021 at 11:33 AM Stefan Berger <stefanb@linux.ibm.com> wrote:
>
> The removal is triggered by the user changing the type of key from what
> is in the keyfile.

I understand.

But if I earlier pointed the kernel config to one my RSA keys, and
then I change some key type config option to something else, I sure as
hell don't want to perhaps lose my key as a result.

Yes, one common situation is that the key is some automatically
generated one. That's what I use personally - I want a temporary key
that is thrown away and never exists except for validating that "yup,
I built these modules for this kernel". Removing that temporary key is
fine.

But if I pointed MODULE_SIG_KEY to something outside the kernel build,
I sure as hell don't want the kernel build deleting it. Ever. In fact,
it should never write to it. It should extract the key information
from it, and nothing else.

So no. No backups either. Because there is not a single valid
situation where you'd want a backup - because the kernel build should
never EVER modify the original.

Maybe I misunderstand what is going on, but I think the whole thing is
completely wrongly designed. The _only_ key that the kernel build
should touchn is the auto-generated throw-away one (ie
"certs/signing_key.pem"), not CONFIG_MODULE_SIG_KEY in general.

                 Linus

Re: [GIT PULL] TPM DEVICE DRIVER changes for v5.14

[Stefan Berger]

On 6/28/21 3:11 PM, Linus Torvalds wrote:
> On Mon, Jun 28, 2021 at 11:33 AM Stefan Berger <stefanb@linux.ibm.com> wrote:
>> The removal is triggered by the user changing the type of key from what
>> is in the keyfile.
>
>
> So no. No backups either. Because there is not a single valid
> situation where you'd want a backup - because the kernel build should
> never EVER modify the original.
>
> Maybe I misunderstand what is going on, but I think the whole thing is
> completely wrongly designed. The _only_ key that the kernel build
> should touchn is the auto-generated throw-away one (ie
> "certs/signing_key.pem"), not CONFIG_MODULE_SIG_KEY in general.

Correct, and the code (certs/Makefile) is surrounded by the check for 
this particular file here, so it won't touch anything else:

[...]

ifeq ($(CONFIG_MODULE_SIG_KEY),"certs/signing_key.pem")

ifeq ($(openssl_available),yes)
X509TEXT=$(shell openssl x509 -in $(CONFIG_MODULE_SIG_KEY) -text)
endif

# Support user changing key type
ifdef CONFIG_MODULE_SIG_KEY_TYPE_ECDSA
keytype_openssl = -newkey ec -pkeyopt ec_paramgen_curve:secp384r1
ifeq ($(openssl_available),yes)
$(if $(findstring id-ecPublicKey,$(X509TEXT)),,$(shell rm -f 
$(CONFIG_MODULE_SIG_KEY)))
endif
endif # CONFIG_MODULE_SIG_KEY_TYPE_ECDSA

ifdef CONFIG_MODULE_SIG_KEY_TYPE_RSA
ifeq ($(openssl_available),yes)
$(if $(findstring rsaEncryption,$(X509TEXT)),,$(shell rm -f 
$(CONFIG_MODULE_SIG_KEY)))
endif
endif # CONFIG_MODULE_SIG_KEY_TYPE_RSA

[...]


There's one dent in this patch series that requires suppressing an error 
output:  https://lkml.org/lkml/2021/6/25/452


   Stefan


>
>                   Linus

Re: [GIT PULL] TPM DEVICE DRIVER changes for v5.14

[Linus Torvalds]

On Mon, Jun 28, 2021 at 12:21 PM Stefan Berger <stefanb@linux.ibm.com> wrote:
>
> Correct, and the code (certs/Makefile) is surrounded by the check for
> this particular file here, so it won't touch anything else:

Ahh, I missed that part.

Can we just make it really really obvious, and not use
CONFIG_MODULE_SIG_KEY at all, then?

IOW, make these literally be about "certs/signing_key.pem" and nothing
else, so that when people grep for this, or look at the Makefile, they
don't fall into that trap I fell into?

That also would make it obvious that there are no pathname quoting issues etc.

             Linus

Re: [GIT PULL] TPM DEVICE DRIVER changes for v5.14

[Stefan Berger]

On 6/28/21 3:27 PM, Linus Torvalds wrote:
> On Mon, Jun 28, 2021 at 12:21 PM Stefan Berger <stefanb@linux.ibm.com> wrote:
>> Correct, and the code (certs/Makefile) is surrounded by the check for
>> this particular file here, so it won't touch anything else:
> Ahh, I missed that part.
>
> Can we just make it really really obvious, and not use
> CONFIG_MODULE_SIG_KEY at all, then?
>
> IOW, make these literally be about "certs/signing_key.pem" and nothing
> else, so that when people grep for this, or look at the Makefile, they
> don't fall into that trap I fell into?

Yes, sir.

    Stefan


>
> That also would make it obvious that there are no pathname quoting issues etc.
>
>               Linus

Re: [GIT PULL] TPM DEVICE DRIVER changes for v5.14

[Jarkko Sakkinen]

On Mon, Jun 28, 2021 at 10:34:26AM -0700, Linus Torvalds wrote:
> On Wed, Jun 23, 2021 at 6:56 AM Jarkko Sakkinen <jarkko@kernel.org> wrote:
> >
> > Contains bug fixes for TPM, and support for signing modules using elliptic
> > curve keys, which I promised to pick up to my tree.
> 
> I pulled this, but then I looked at the key type changes, and that
> made me so scared that I unpulled it again.
> 
> In particular, that code will do
> 
>     shell rm -f $(CONFIG_MODULE_SIG_KEY)
> 
> from the Makefile if some config options have changed.
> 
> That just seems too broken for words. Maybe I misunderstand this, but
> this really seems like an easy mistake might cause the kernel build to
> actively start removing some random user key files that the user
> pointed at previously.
> 
>                   Linus

Since there was still a new fix for the series [*], I'd rather refine
the pull request without these patches, and not risk them being blocker
for the rest of the commits.

[*] https://lore.kernel.org/linux-integrity/20210629201257.dr77kemy66mxpox5@kernel.org/

/Jarkko

Re: [GIT PULL] TPM DEVICE DRIVER changes for v5.14

[Linus Torvalds]

On Tue, Jun 29, 2021 at 1:20 PM Jarkko Sakkinen <jarkko@kernel.org> wrote:
>
> Since there was still a new fix for the series [*], I'd rather refine
> the pull request without these patches, and not risk them being blocker
> for the rest of the commits.

They seemed to be just the last two commits at the end of the series,
so I could take everything up to 0178f9d0f60b ("tpm: Replace
WARN_ONCE() with dev_err_once() in tpm_tis_status()") perhaps?

I can do that even without a new pull request (I've done that kind of
thing before where I decide to pull everything but the last few
commits). But admittedly I'd prefer to see a new pull request just so
that I get a signed tag (which I wouldn't get if I just merged that
top commit).

                     Linus

Re: [GIT PULL] TPM DEVICE DRIVER changes for v5.14

[Jarkko Sakkinen]

On Tue, Jun 29, 2021 at 02:08:53PM -0700, Linus Torvalds wrote:
> On Tue, Jun 29, 2021 at 1:20 PM Jarkko Sakkinen <jarkko@kernel.org> wrote:
> >
> > Since there was still a new fix for the series [*], I'd rather refine
> > the pull request without these patches, and not risk them being blocker
> > for the rest of the commits.
> 
> They seemed to be just the last two commits at the end of the series,
> so I could take everything up to 0178f9d0f60b ("tpm: Replace
> WARN_ONCE() with dev_err_once() in tpm_tis_status()") perhaps?
> 
> I can do that even without a new pull request (I've done that kind of
> thing before where I decide to pull everything but the last few
> commits). But admittedly I'd prefer to see a new pull request just so
> that I get a signed tag (which I wouldn't get if I just merged that
> top commit).
> 
>                      Linus

OK, that would be great! Please, do that. Thank you.

/Jarkko