mbox series

[v1,0/3] KEYS: trusted: Introduce support for NXP CAAM-based trusted keys

Message ID cover.56fff82362af6228372ea82e6bd7e586e23f0966.1615914058.git-series.a.fatoum@pengutronix.de (mailing list archive)
Headers show
Series KEYS: trusted: Introduce support for NXP CAAM-based trusted keys | expand

Message

Ahmad Fatoum March 16, 2021, 5:01 p.m. UTC
The Cryptographic Acceleration and Assurance Module (CAAM) is an IP core
built into many newer i.MX and QorIQ SoCs by NXP.

Its blob mechanism can AES encrypt/decrypt user data using a unique
never-disclosed device-specific key. There has been multiple
discussions on how to represent this within the kernel:

 - [RFC] crypto: caam - add red blobifier
   Steffen implemented[1] a PoC sysfs driver to start a discussion on how to
   best integrate the blob mechanism.
   Mimi suggested that it could be used to implement trusted keys.
   Trusted keys back then were a TPM-only feature.

 - security/keys/secure_key: Adds the secure key support based on CAAM.
   Udit added[2] a new "secure" key type with the CAAM as backend. The key
   material stays within the kernel only.
   Mimi and James agreed that this needs a generic interface, not specific
   to CAAM. Mimi suggested trusted keys. Jan noted that this could serve as
   basis for TEE-backed keys.

 - [RFC] drivers: crypto: caam: key: Add caam_tk key type
   Franck added[3] a new "caam_tk" key type based on Udit's work. The key
   material stays within the kernel only, but can optionally be user-set
   instead of coming from RNG. James voiced the opinion that there should
   be just one user-facing generic wrap/unwrap key type with multiple
   possible handlers. David suggested trusted keys.

 - Introduce TEE based Trusted Keys support
   Sumit reworked[4] trusted keys to support multiple possible backends with
   one chosen at boot time and added a new TEE backend along with TPM.
   This now sits in Jarkko's master branch to be sent out for v5.13

This patch series builds on top of Sumit's rework to have the CAAM as yet another
trusted key backend.

The CAAM bits are based on Steffen's initial patch from 2015. His work had been
used in the field for some years now, so I preferred not to deviate too much from it.

This series has been tested with dmcrypt[5] on an i.MX6DL.

Looking forward to your feedback.

Cheers,
Ahmad

 [1]: https://lore.kernel.org/linux-crypto/1447082306-19946-2-git-send-email-s.trumtrar@pengutronix.de/
 [2]: https://lore.kernel.org/linux-integrity/20180723111432.26830-1-udit.agarwal@nxp.com/
 [3]: https://lore.kernel.org/lkml/1551456599-10603-2-git-send-email-franck.lenormand@nxp.com/
 [4]: https://lore.kernel.org/lkml/1604419306-26105-1-git-send-email-sumit.garg@linaro.org/
 [5]: https://lore.kernel.org/linux-integrity/20210122084321.24012-2-a.fatoum@pengutronix.de/

---
To: Jarkko Sakkinen <jarkko@kernel.org>
To: "Horia Geantă" <horia.geanta@nxp.com>
To: Mimi Zohar <zohar@linux.ibm.com>
To: Aymen Sghaier <aymen.sghaier@nxp.com>
To: Herbert Xu <herbert@gondor.apana.org.au>
To: "David S. Miller" <davem@davemloft.net>
To: James Bottomley <jejb@linux.ibm.com>
Cc: David Howells <dhowells@redhat.com>
Cc: James Morris <jmorris@namei.org>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: Steffen Trumtrar <s.trumtrar@pengutronix.de>
Cc: Udit Agarwal <udit.agarwal@nxp.com>
Cc: Jan Luebbe <j.luebbe@penutronix.de>
Cc: David Gstir <david@sigma-star.at>
Cc: Franck LENORMAND <franck.lenormand@nxp.com>
Cc: Sumit Garg <sumit.garg@linaro.org>
Cc: linux-integrity@vger.kernel.org
Cc: keyrings@vger.kernel.org
Cc: linux-crypto@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-security-module@vger.kernel.org

Ahmad Fatoum (3):
  crypto: caam - add in-kernel interface for blob generator
  KEYS: trusted: implement fallback to kernel RNG
  KEYS: trusted: Introduce support for NXP CAAM-based trusted keys

 Documentation/admin-guide/kernel-parameters.txt |   1 +-
 drivers/crypto/caam/Kconfig                     |   4 +-
 drivers/crypto/caam/Makefile                    |   1 +-
 drivers/crypto/caam/blob_gen.c                  | 230 +++++++++++++++++-
 include/keys/trusted-type.h                     |   2 +-
 include/keys/trusted_caam.h                     |  11 +-
 include/soc/fsl/caam-blob.h                     |  54 ++++-
 security/keys/trusted-keys/Makefile             |   1 +-
 security/keys/trusted-keys/trusted_caam.c       |  74 +++++-
 security/keys/trusted-keys/trusted_core.c       |  17 +-
 10 files changed, 392 insertions(+), 3 deletions(-)
 create mode 100644 drivers/crypto/caam/blob_gen.c
 create mode 100644 include/keys/trusted_caam.h
 create mode 100644 include/soc/fsl/caam-blob.h
 create mode 100644 security/keys/trusted-keys/trusted_caam.c

base-commit: 8a3fa8ade8a35d8f7c178f5680f07f223da41b87

Comments

Richard Weinberger March 16, 2021, 11:10 p.m. UTC | #1
Ahmad,

On Tue, Mar 16, 2021 at 6:24 PM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>
> The Cryptographic Acceleration and Assurance Module (CAAM) is an IP core
> built into many newer i.MX and QorIQ SoCs by NXP.
>
> Its blob mechanism can AES encrypt/decrypt user data using a unique
> never-disclosed device-specific key. There has been multiple
> discussions on how to represent this within the kernel:
>
>  - [RFC] crypto: caam - add red blobifier
>    Steffen implemented[1] a PoC sysfs driver to start a discussion on how to
>    best integrate the blob mechanism.
>    Mimi suggested that it could be used to implement trusted keys.
>    Trusted keys back then were a TPM-only feature.
>
>  - security/keys/secure_key: Adds the secure key support based on CAAM.
>    Udit added[2] a new "secure" key type with the CAAM as backend. The key
>    material stays within the kernel only.
>    Mimi and James agreed that this needs a generic interface, not specific
>    to CAAM. Mimi suggested trusted keys. Jan noted that this could serve as
>    basis for TEE-backed keys.
>
>  - [RFC] drivers: crypto: caam: key: Add caam_tk key type
>    Franck added[3] a new "caam_tk" key type based on Udit's work. The key
>    material stays within the kernel only, but can optionally be user-set
>    instead of coming from RNG. James voiced the opinion that there should
>    be just one user-facing generic wrap/unwrap key type with multiple
>    possible handlers. David suggested trusted keys.
>
>  - Introduce TEE based Trusted Keys support
>    Sumit reworked[4] trusted keys to support multiple possible backends with
>    one chosen at boot time and added a new TEE backend along with TPM.
>    This now sits in Jarkko's master branch to be sent out for v5.13
>
> This patch series builds on top of Sumit's rework to have the CAAM as yet another
> trusted key backend.
>
> The CAAM bits are based on Steffen's initial patch from 2015. His work had been
> used in the field for some years now, so I preferred not to deviate too much from it.
>
> This series has been tested with dmcrypt[5] on an i.MX6DL.

Do have this series also in a git repo to pull from?
I'd like to give it a test on various systems.

> Looking forward to your feedback.

Thanks for working on this! David and I will have a closer look these days.
Ahmad Fatoum March 17, 2021, 2:08 p.m. UTC | #2
Hello Richard,

On 17.03.21 00:10, Richard Weinberger wrote:
> On Tue, Mar 16, 2021 at 6:24 PM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>> This series has been tested with dmcrypt[5] on an i.MX6DL.
> 
> Do have this series also in a git repo to pull from?
> I'd like to give it a test on various systems.

Yes, please pull git://git.pengutronix.de/afa/linux
Branch v5.12/topic/trusted-source-caam

It includes these three patches on top of Jarkko's linux-tpmdd/master.

>> Looking forward to your feedback.
> 
> Thanks for working on this! David and I will have a closer look these days.

Great. Here is a simple testing regiment that could help you getting started:

# First boot
    DEV=/dev/loop0
    ALGO=aes-cbc-essiv:sha256
    KEYNAME=kmk
    BLOCKS=20

    mount -o remount,rw /
    fallocate -l $((BLOCKS*512)) ~/loop0.img
    losetup -P $DEV ~/loop0.img
    KEY="$(keyctl add trusted $KEYNAME 'new 32' @s)"
    keyctl pipe $KEY >~/kmk.blob

    TABLE="0 $BLOCKS crypt $ALGO :32:trusted:$KEYNAME 0 $DEV 0 1 allow_discards"
    echo $TABLE | dmsetup create mydev
    echo $TABLE | dmsetup load mydev
    dd if=/dev/zero of=/dev/mapper/mydev || true
    echo "It works!" 1<> /dev/mapper/mydev
    cryptsetup close mydev

# Second boot
    DEV=/dev/loop0
    ALGO=aes-cbc-essiv:sha256
    KEYNAME=kmk
    BLOCKS=20

    losetup -P $DEV ~/loop0.img
    keyctl add trusted $KEYNAME "load $(cat ~/kmk.blob)" @s
    TABLE="0 $BLOCKS crypt $ALGO :32:trusted:$KEYNAME 0 $DEV 0 1 allow_discards"
    echo $TABLE | dmsetup create mydev
    echo $TABLE | dmsetup load mydev

# Should print that It works!
    hexdump -C /dev/mapper/mydev
Horia Geantă March 21, 2021, 8:01 p.m. UTC | #3
On 3/16/2021 7:02 PM, Ahmad Fatoum wrote:
> The Cryptographic Acceleration and Assurance Module (CAAM) is an IP core
> built into many newer i.MX and QorIQ SoCs by NXP.
> 
> Its blob mechanism can AES encrypt/decrypt user data using a unique
> never-disclosed device-specific key. There has been multiple
> discussions on how to represent this within the kernel:
> 
>  - [RFC] crypto: caam - add red blobifier
>    Steffen implemented[1] a PoC sysfs driver to start a discussion on how to
>    best integrate the blob mechanism.
>    Mimi suggested that it could be used to implement trusted keys.
>    Trusted keys back then were a TPM-only feature.
> 
>  - security/keys/secure_key: Adds the secure key support based on CAAM.
>    Udit added[2] a new "secure" key type with the CAAM as backend. The key
>    material stays within the kernel only.
>    Mimi and James agreed that this needs a generic interface, not specific
>    to CAAM. Mimi suggested trusted keys. Jan noted that this could serve as
>    basis for TEE-backed keys.
> 
>  - [RFC] drivers: crypto: caam: key: Add caam_tk key type
>    Franck added[3] a new "caam_tk" key type based on Udit's work. The key
>    material stays within the kernel only, but can optionally be user-set
>    instead of coming from RNG. James voiced the opinion that there should
>    be just one user-facing generic wrap/unwrap key type with multiple
>    possible handlers. David suggested trusted keys.
> 
The whole point was to use caam "black blobs", with the main advantage of
keys being kept encrypted in memory after "unsealing" the blobs.
(Keys in blobs are encrypted with a persistent BKEK - blob KEK, derived from
fuse-based OTPMK. OTOH black keys are keys encrypted with an ephemeral, random
KEK that is stored in an internal caam register. When a black blob is unsealed,
the key is practically rekeyed, the random key replacing the BKEK; key is never
exposed in plaintext, rekeying happens in caam).

Current implementation uses "red blobs", which means keys are left unprotected
in memory after blobs are unsealed.

>  - Introduce TEE based Trusted Keys support
>    Sumit reworked[4] trusted keys to support multiple possible backends with
>    one chosen at boot time and added a new TEE backend along with TPM.
>    This now sits in Jarkko's master branch to be sent out for v5.13
> 
> This patch series builds on top of Sumit's rework to have the CAAM as yet another
> trusted key backend.
> 
Shouldn't the description under TRUSTED_KEYS (in security/keys/Kconfig)
be updated to reflect the availability of multiple backends?

Thanks,
Horia
Ahmad Fatoum March 23, 2021, 4:34 p.m. UTC | #4
Hello Horia,

On 21.03.21 21:01, Horia Geantă wrote:
> On 3/16/2021 7:02 PM, Ahmad Fatoum wrote:
>> This patch series builds on top of Sumit's rework to have the CAAM as yet another
>> trusted key backend.
>>
> Shouldn't the description under TRUSTED_KEYS (in security/keys/Kconfig)
> be updated to reflect the availability of multiple backends?

This is indeed no longer correct. It also depends on TCG_TPM, which AFAIU
is not really needed for the new TEE backend.

@Sumit, can you confirm?
Ahmad Fatoum March 23, 2021, 4:37 p.m. UTC | #5
Hello Horia,

On 21.03.21 21:01, Horia Geantă wrote:
>>  - [RFC] drivers: crypto: caam: key: Add caam_tk key type
>>    Franck added[3] a new "caam_tk" key type based on Udit's work. The key
>>    material stays within the kernel only, but can optionally be user-set
>>    instead of coming from RNG. James voiced the opinion that there should
>>    be just one user-facing generic wrap/unwrap key type with multiple
>>    possible handlers. David suggested trusted keys.
>>
> The whole point was to use caam "black blobs", with the main advantage of
> keys being kept encrypted in memory after "unsealing" the blobs.
> (Keys in blobs are encrypted with a persistent BKEK - blob KEK, derived from
> fuse-based OTPMK. OTOH black keys are keys encrypted with an ephemeral, random
> KEK that is stored in an internal caam register. When a black blob is unsealed,
> the key is practically rekeyed, the random key replacing the BKEK; key is never
> exposed in plaintext, rekeying happens in caam).
> 
> Current implementation uses "red blobs", which means keys are left unprotected
> in memory after blobs are unsealed.

Oh. I will reread the series when sending the v2 cover letter. Thanks for spotting.

(Sorry for the noise, missed this question first time)

>>  - Introduce TEE based Trusted Keys support
>>    Sumit reworked[4] trusted keys to support multiple possible backends with
>>    one chosen at boot time and added a new TEE backend along with TPM.
>>    This now sits in Jarkko's master branch to be sent out for v5.13
>>
>> This patch series builds on top of Sumit's rework to have the CAAM as yet another
>> trusted key backend.
>>
> Shouldn't the description under TRUSTED_KEYS (in security/keys/Kconfig)
> be updated to reflect the availability of multiple backends?
> 
> Thanks,
> Horia
>
Sumit Garg March 24, 2021, 6:23 a.m. UTC | #6
On Tue, 23 Mar 2021 at 22:04, Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>
> Hello Horia,
>
> On 21.03.21 21:01, Horia Geantă wrote:
> > On 3/16/2021 7:02 PM, Ahmad Fatoum wrote:
> >> This patch series builds on top of Sumit's rework to have the CAAM as yet another
> >> trusted key backend.
> >>
> > Shouldn't the description under TRUSTED_KEYS (in security/keys/Kconfig)
> > be updated to reflect the availability of multiple backends?
>
> This is indeed no longer correct. It also depends on TCG_TPM, which AFAIU
> is not really needed for the new TEE backend.
>
> @Sumit, can you confirm?
>

Yes, that's correct. Let me share a separate patch to fix that.

-Sumit

> --
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
Richard Weinberger March 30, 2021, 9:50 p.m. UTC | #7
Ahmad,

On Wed, Mar 17, 2021 at 3:08 PM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:

>     TABLE="0 $BLOCKS crypt $ALGO :32:trusted:$KEYNAME 0 $DEV 0 1 allow_discards"
>     echo $TABLE | dmsetup create mydev
>     echo $TABLE | dmsetup load mydev

Do you also plan to add support for this to cryptsetup?

David and I have added (rough) support for our CAAM/DCP based keyrings
to cryptsetup:
https://github.com/sigma-star/cryptsetup/tree/rw/plain

I'm pretty sure with minimal changes it will work with your recent approach too.
Richard Weinberger March 30, 2021, 10:04 p.m. UTC | #8
Ahmad,

On Wed, Mar 17, 2021 at 3:08 PM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>     keyctl add trusted $KEYNAME "load $(cat ~/kmk.blob)" @s

Is there a reason why we can't pass the desired backend name in the
trusted key parameters?
e.g.
keyctl add trusted $KEYNAME "backendtype caam load $(cat ~/kmk.blob)" @s
James Bottomley March 30, 2021, 10:16 p.m. UTC | #9
On Wed, 2021-03-31 at 00:04 +0200, Richard Weinberger wrote:
> Ahmad,
> 
> On Wed, Mar 17, 2021 at 3:08 PM Ahmad Fatoum <a.fatoum@pengutronix.de
> > wrote:
> >     keyctl add trusted $KEYNAME "load $(cat ~/kmk.blob)" @s
> 
> Is there a reason why we can't pass the desired backend name in the
> trusted key parameters?
> e.g.
> keyctl add trusted $KEYNAME "backendtype caam load $(cat ~/kmk.blob)"
> @s

Why would you want to in the load?  The blob should be type specific,
so a TPM key shouldn't load as a CAAM key and vice versa ... and if
they're not they need to be made so before the patches go upstream.

I could possibly see that you might want to be type specific in the
create, but once you're simply loading an already created key, the
trusted key subsystem should be able to figure what to do on its own.

James
Richard Weinberger March 31, 2021, 6:36 p.m. UTC | #10
James,

----- Ursprüngliche Mail -----
> Von: "James Bottomley" <jejb@linux.ibm.com>
>> On Wed, Mar 17, 2021 at 3:08 PM Ahmad Fatoum <a.fatoum@pengutronix.de
>> > wrote:
>> >     keyctl add trusted $KEYNAME "load $(cat ~/kmk.blob)" @s
>> 
>> Is there a reason why we can't pass the desired backend name in the
>> trusted key parameters?
>> e.g.
>> keyctl add trusted $KEYNAME "backendtype caam load $(cat ~/kmk.blob)"
>> @s
> 
> Why would you want to in the load?  The blob should be type specific,
> so a TPM key shouldn't load as a CAAM key and vice versa ... and if
> they're not they need to be made so before the patches go upstream.

I fear right now there is no good way to detect whether a blob is desired
for CAAM or TPM.

> I could possibly see that you might want to be type specific in the
> create, but once you're simply loading an already created key, the
> trusted key subsystem should be able to figure what to do on its own.

So you have some kind of container format in mind which denotes the
type of the blob?

Thanks,
//richard
James Bottomley March 31, 2021, 6:49 p.m. UTC | #11
On Wed, 2021-03-31 at 20:36 +0200, Richard Weinberger wrote:
> James,
> 
> ----- Ursprüngliche Mail -----
> > Von: "James Bottomley" <jejb@linux.ibm.com>
> > > On Wed, Mar 17, 2021 at 3:08 PM Ahmad Fatoum <
> > > a.fatoum@pengutronix.de wrote:
> > > >     keyctl add trusted $KEYNAME "load $(cat ~/kmk.blob)" @s
> > > 
> > > Is there a reason why we can't pass the desired backend name in
> > > the
> > > trusted key parameters?
> > > e.g.
> > > keyctl add trusted $KEYNAME "backendtype caam load $(cat
> > > ~/kmk.blob)"
> > > @s
> > 
> > Why would you want to in the load?  The blob should be type
> > specific, so a TPM key shouldn't load as a CAAM key and vice versa
> > ... and if they're not they need to be made so before the patches
> > go upstream.
> 
> I fear right now there is no good way to detect whether a blob is
> desired for CAAM or TPM.

At least for the TPM the old format is two TPM2B structures, and the
new one is ASN.1 so either should be completely distinguishable over
what CAAM does.

> > I could possibly see that you might want to be type specific in the
> > create, but once you're simply loading an already created key, the
> > trusted key subsystem should be able to figure what to do on its
> > own.
> 
> So you have some kind of container format in mind which denotes the
> type of the blob?

Well, yes.  For the TPM, there's a defined ASN.1 format for the keys:

https://git.kernel.org/pub/scm/linux/kernel/git/jejb/openssl_tpm2_engine.git/tree/tpm2-asn.h

and part of the design of the file is that it's distinguishable either
in DER or PEM (by the guards) format so any crypto application can know
it's dealing with a TPM key simply by inspecting the file.  I think you
need the same thing for CAAM and any other format.

We're encouraging new ASN.1 formats to be of the form

SEQUENCE {
    type   OBJECT IDENTIFIER
    ... key specific fields ...
}

Where you choose a defined OID to represent the key and that means
every key even in DER form begins with a unique binary signature.

James
Richard Weinberger March 31, 2021, 7:36 p.m. UTC | #12
James,

----- Ursprüngliche Mail -----
> Von: "James Bottomley" <jejb@linux.ibm.com>
> Well, yes.  For the TPM, there's a defined ASN.1 format for the keys:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/jejb/openssl_tpm2_engine.git/tree/tpm2-asn.h
> 
> and part of the design of the file is that it's distinguishable either
> in DER or PEM (by the guards) format so any crypto application can know
> it's dealing with a TPM key simply by inspecting the file.  I think you
> need the same thing for CAAM and any other format.
> 
> We're encouraging new ASN.1 formats to be of the form
> 
> SEQUENCE {
>    type   OBJECT IDENTIFIER
>    ... key specific fields ...
> }
> 
> Where you choose a defined OID to represent the key and that means
> every key even in DER form begins with a unique binary signature.

I like this idea.
Ahmad, what do you think?

That way we could also get rid off the kernel parameter and all the fall back logic,
given that we find a way to reliable detect TEE blobs too...

Thanks,
//richard
Ahmad Fatoum April 1, 2021, 10:04 a.m. UTC | #13
Hello Richard,

On 30.03.21 23:50, Richard Weinberger wrote:
> Ahmad,
> 
> On Wed, Mar 17, 2021 at 3:08 PM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
> 
>>     TABLE="0 $BLOCKS crypt $ALGO :32:trusted:$KEYNAME 0 $DEV 0 1 allow_discards"
>>     echo $TABLE | dmsetup create mydev
>>     echo $TABLE | dmsetup load mydev
> 
> Do you also plan to add support for this to cryptsetup?
> 
> David and I have added (rough) support for our CAAM/DCP based keyrings
> to cryptsetup:
> https://github.com/sigma-star/cryptsetup/tree/rw/plain
> 
> I'm pretty sure with minimal changes it will work with your recent approach too.

I am using dmsetup directly in my project. I am not familiar with cryptsetup
plain. What benefits do you see with this over direct dmsetup?

What I'd like to see eventually is support for this with LUKS.
There is a RFE on trusted keys and cryptsetup on the project's repository[1].

The behavior I'd want it that the LUKS header would point at the trusted key
blob to use and load it into the kernel. This of course means that
you won't be able to have multiple keys for the encrypted partition.

[1]: https://gitlab.com/cryptsetup/cryptsetup/-/issues/443

Cheers,
Ahmad
Ahmad Fatoum April 1, 2021, 10:06 a.m. UTC | #14
Hello Richard,

On 31.03.21 21:36, Richard Weinberger wrote:
> James,
> 
> ----- Ursprüngliche Mail -----
>> Von: "James Bottomley" <jejb@linux.ibm.com>
>> Well, yes.  For the TPM, there's a defined ASN.1 format for the keys:
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/jejb/openssl_tpm2_engine.git/tree/tpm2-asn.h
>>
>> and part of the design of the file is that it's distinguishable either
>> in DER or PEM (by the guards) format so any crypto application can know
>> it's dealing with a TPM key simply by inspecting the file.  I think you
>> need the same thing for CAAM and any other format.
>>
>> We're encouraging new ASN.1 formats to be of the form
>>
>> SEQUENCE {
>>    type   OBJECT IDENTIFIER
>>    ... key specific fields ...
>> }
>>
>> Where you choose a defined OID to represent the key and that means
>> every key even in DER form begins with a unique binary signature.
> 
> I like this idea.
> Ahmad, what do you think?
> 
> That way we could also get rid off the kernel parameter and all the fall back logic,
> given that we find a way to reliable detect TEE blobs too...

Sounds good to me. Sumit, your thoughts on doing this for TEE as well?

> 
> Thanks,
> //richard
>
Richard Weinberger April 1, 2021, 10:20 a.m. UTC | #15
Ahmad,

----- Ursprüngliche Mail -----
> Von: "Ahmad Fatoum" <a.fatoum@pengutronix.de>
>> I'm pretty sure with minimal changes it will work with your recent approach too.
> 
> I am using dmsetup directly in my project. I am not familiar with cryptsetup
> plain. What benefits do you see with this over direct dmsetup?

cryptsetup is the de-facto standard to setup encrypted block devices.
There is a lot of existing tooling around cryptsetup already (systemd, etc..),
so being able to use CAAM keys for dm-crypt with cryptsetup seems natural to me.
Plain mode allows setting up dm-crypt without LUKS.

Thanks,
//richard
Ahmad Fatoum April 1, 2021, 10:28 a.m. UTC | #16
Hello,

On 01.04.21 12:20, Richard Weinberger wrote:
> Ahmad,
> 
> ----- Ursprüngliche Mail -----
>> Von: "Ahmad Fatoum" <a.fatoum@pengutronix.de>
>>> I'm pretty sure with minimal changes it will work with your recent approach too.
>>
>> I am using dmsetup directly in my project. I am not familiar with cryptsetup
>> plain. What benefits do you see with this over direct dmsetup?
> 
> cryptsetup is the de-facto standard to setup encrypted block devices.
> There is a lot of existing tooling around cryptsetup already (systemd, etc..),

Do you mean systemd-cryptsetup? It looks to me like it's just a way to supply
the keyphrase. With trusted keys and a keyphrase unknown to userspace, this
won't work.

> so being able to use CAAM keys for dm-crypt with cryptsetup seems natural to me.
> Plain mode allows setting up dm-crypt without LUKS.

I don't (yet) see the utility of it without LUKS. Perhaps a command dump on how
to do the same I did with dmsetup, but with cryptsetup plain instead could
help me to see the benefits?

Cheers,
Ahmad

> 
> Thanks,
> //richard
>
Richard Weinberger April 1, 2021, 10:53 a.m. UTC | #17
Ahmad,

----- Ursprüngliche Mail -----
> Do you mean systemd-cryptsetup? It looks to me like it's just a way to supply
> the keyphrase. With trusted keys and a keyphrase unknown to userspace, this
> won't work.

Nah, I meant existing scripts/service Files.

> I don't (yet) see the utility of it without LUKS. Perhaps a command dump on how
> to do the same I did with dmsetup, but with cryptsetup plain instead could
> help me to see the benefits?

My reasoning is simple, why do I need a different tool when there is already one
that could do the task too?
Usually the systems I get my hands on use already dm-crypt with cryptsetup in some way.
So I have the tooling already in my initramfs, etc.. and need to adopt the callers of cryptsetup a little.

If I need all of a sudden different/additional tooling, it means more work, more docs to write,
more hassle with crypto/system reviewers, etc...

I don't want you to force to use cryptsetup.
The only goal was pointing out that it can be done with cryptsetup and that there
is already code such that no work is done twice.
One the kernel side it does not matter.

Thanks,
//richard
Ahmad Fatoum April 1, 2021, 10:57 a.m. UTC | #18
Hello Richard,

On 01.04.21 12:53, Richard Weinberger wrote:
> Ahmad,
> 
> ----- Ursprüngliche Mail -----
>> Do you mean systemd-cryptsetup? It looks to me like it's just a way to supply
>> the keyphrase. With trusted keys and a keyphrase unknown to userspace, this
>> won't work.
> 
> Nah, I meant existing scripts/service Files.
> 
>> I don't (yet) see the utility of it without LUKS. Perhaps a command dump on how
>> to do the same I did with dmsetup, but with cryptsetup plain instead could
>> help me to see the benefits?
> 
> My reasoning is simple, why do I need a different tool when there is already one
> that could do the task too?
> Usually the systems I get my hands on use already dm-crypt with cryptsetup in some way.
> So I have the tooling already in my initramfs, etc.. and need to adopt the callers of cryptsetup a little.
> 
> If I need all of a sudden different/additional tooling, it means more work, more docs to write,
> more hassle with crypto/system reviewers, etc...
> 
> I don't want you to force to use cryptsetup.

I'd love to use cryptsetup with LUKS and trusted keys eventually. I'll take
a look and see if cryptsetup plain maybe a suitable stop-gap solution for us.

> The only goal was pointing out that it can be done with cryptsetup and that there
> is already code such that no work is done twice.
> One the kernel side it does not matter.

Thanks for the pointer,
Ahmad

> 
> Thanks,
> //richard
>
Richard Weinberger April 1, 2021, 11:05 a.m. UTC | #19
Ahmad,

----- Ursprüngliche Mail -----
> Von: "Ahmad Fatoum" <a.fatoum@pengutronix.de>
>> I don't want you to force to use cryptsetup.
> 
> I'd love to use cryptsetup with LUKS and trusted keys eventually. I'll take

But using LUKS would mean that cryptsetup has access to the plain disc encryption key material?
This would be a no-go for many systems out there, key material must not accessible to userspace.
I know, distrusting userspace root is not easy, but doable. :)

Thanks,
//richard
David Howells April 1, 2021, 11:11 a.m. UTC | #20
Richard Weinberger <richard.weinberger@gmail.com> wrote:

> On Wed, Mar 17, 2021 at 3:08 PM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
> >     keyctl add trusted $KEYNAME "load $(cat ~/kmk.blob)" @s
>
> Is there a reason why we can't pass the desired backend name in the
> trusted key parameters?
> e.g.
> keyctl add trusted $KEYNAME "backendtype caam load $(cat ~/kmk.blob)" @s

I wonder...  Does it make sense to add a new variant of the add_key() and
keyctl_instantiate() syscalls that takes an additional parameter string,
separate from the payload blob?

       key_serial_t add_key2(const char *type, const char *description,
			     const char *params,
                             const void *payload, size_t plen,
                             key_serial_t keyring);

which could then by used, say:

	keyctl add --payload=~/kmk.blob trusted $KEYNAME "backendtype caam load" @s

This would then appear in

	struct key_preparsed_payload {
		const char	*orig_description;
		char		*description;
		char		*params;	<---
		union key_payload payload;
		const void	*data;
		size_t		datalen;
		size_t		quotalen;
		time64_t	expiry;
	};

params would then be NULL for add_key().

If add_key2() is not available, the --payload param gets concatenated to the
parameters string.

Might be too complicated, I guess.  Though it might make sense just to do the
concatenation inside the keyctl program.

David
Ahmad Fatoum April 1, 2021, 11:13 a.m. UTC | #21
Hello Richard,

On 01.04.21 13:05, Richard Weinberger wrote:
> Ahmad,
> 
> ----- Ursprüngliche Mail -----
>> Von: "Ahmad Fatoum" <a.fatoum@pengutronix.de>
>>> I don't want you to force to use cryptsetup.
>>
>> I'd love to use cryptsetup with LUKS and trusted keys eventually. I'll take
> 
> But using LUKS would mean that cryptsetup has access to the plain disc encryption key material?
> This would be a no-go for many systems out there, key material must not accessible to userspace.
> I know, distrusting userspace root is not easy, but doable. :)

The LUKS2 format supports tokens. I see no reason why the encrypted blob
couldn't be stored there along with the usual metadata. cryptsetup would
then load it as kernel trusted key and use it for dmcrypt decryption.

This will mean we have to part ways with features such as having multiple
keys, but I think it's worth it to have a plug and play solution for
trusted keys.

Of course, someone needs to implement this first ^^.

Cheers,
Ahmad

> 
> Thanks,
> //richard
>
Richard Weinberger April 1, 2021, 11:16 a.m. UTC | #22
Ahmad,

----- Ursprüngliche Mail -----
> Von: "Ahmad Fatoum" <a.fatoum@pengutronix.de>
>> But using LUKS would mean that cryptsetup has access to the plain disc
>> encryption key material?
>> This would be a no-go for many systems out there, key material must not
>> accessible to userspace.
>> I know, distrusting userspace root is not easy, but doable. :)
> 
> The LUKS2 format supports tokens. I see no reason why the encrypted blob
> couldn't be stored there along with the usual metadata. cryptsetup would
> then load it as kernel trusted key and use it for dmcrypt decryption.
> 
> This will mean we have to part ways with features such as having multiple
> keys, but I think it's worth it to have a plug and play solution for
> trusted keys.

Ah, now I can follow your thoughts!
Yes, that would be nice to have. :)

I kind of assumed you want to use LUKS with passphrases and CAAM blobs.

Thanks,
//richard
Sumit Garg April 1, 2021, 12:55 p.m. UTC | #23
Hi Richard,

On Wed, 31 Mar 2021 at 03:34, Richard Weinberger
<richard.weinberger@gmail.com> wrote:
>
> Ahmad,
>
> On Wed, Mar 17, 2021 at 3:08 PM Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
> >     keyctl add trusted $KEYNAME "load $(cat ~/kmk.blob)" @s
>
> Is there a reason why we can't pass the desired backend name in the
> trusted key parameters?
> e.g.
> keyctl add trusted $KEYNAME "backendtype caam load $(cat ~/kmk.blob)" @s
>

IIUC, this would require support for multiple trusted keys backends at
runtime but currently the trusted keys subsystem only supports a
single backend which is selected via kernel module parameter during
boot.

So the trusted keys framework needs to evolve to support multiple
trust sources at runtime but I would like to understand the use-cases
first. IMO, selecting the best trust source available on a platform
for trusted keys should be a one time operation, so why do we need to
have other backends available at runtime as well?

-Sumit

> --
> Thanks,
> //richard
Richard Weinberger April 1, 2021, 1:17 p.m. UTC | #24
Sumit,

----- Ursprüngliche Mail -----
> Von: "Sumit Garg" <sumit.garg@linaro.org>
> IIUC, this would require support for multiple trusted keys backends at
> runtime but currently the trusted keys subsystem only supports a
> single backend which is selected via kernel module parameter during
> boot.
> 
> So the trusted keys framework needs to evolve to support multiple
> trust sources at runtime but I would like to understand the use-cases
> first. IMO, selecting the best trust source available on a platform
> for trusted keys should be a one time operation, so why do we need to
> have other backends available at runtime as well?

I thought about devices with a TPM-Chip and CAAM.
IMHO allowing only one backend at the same time is a little over simplified. 

Thanks,
//richard
Sumit Garg April 1, 2021, 1:20 p.m. UTC | #25
On Thu, 1 Apr 2021 at 15:36, Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>
> Hello Richard,
>
> On 31.03.21 21:36, Richard Weinberger wrote:
> > James,
> >
> > ----- Ursprüngliche Mail -----
> >> Von: "James Bottomley" <jejb@linux.ibm.com>
> >> Well, yes.  For the TPM, there's a defined ASN.1 format for the keys:
> >>
> >> https://git.kernel.org/pub/scm/linux/kernel/git/jejb/openssl_tpm2_engine.git/tree/tpm2-asn.h
> >>
> >> and part of the design of the file is that it's distinguishable either
> >> in DER or PEM (by the guards) format so any crypto application can know
> >> it's dealing with a TPM key simply by inspecting the file.  I think you
> >> need the same thing for CAAM and any other format.
> >>
> >> We're encouraging new ASN.1 formats to be of the form
> >>
> >> SEQUENCE {
> >>    type   OBJECT IDENTIFIER
> >>    ... key specific fields ...
> >> }
> >>
> >> Where you choose a defined OID to represent the key and that means
> >> every key even in DER form begins with a unique binary signature.
> >
> > I like this idea.
> > Ahmad, what do you think?
> >
> > That way we could also get rid off the kernel parameter and all the fall back logic,
> > given that we find a way to reliable detect TEE blobs too...
>
> Sounds good to me. Sumit, your thoughts on doing this for TEE as well?
>

AFAIU, ASN.1 formating should be independent of trusted keys backends
which could be abstracted to trusted keys core layer so that every
backend could be plugged in seamlessly.

James,

Would it be possible to achieve this?

-Sumit

> >
> > Thanks,
> > //richard
> >
>
> --
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
Ahmad Fatoum April 1, 2021, 1:30 p.m. UTC | #26
Hello Richard, Sumit,

On 01.04.21 15:17, Richard Weinberger wrote:
> Sumit,
> 
> ----- Ursprüngliche Mail -----
>> Von: "Sumit Garg" <sumit.garg@linaro.org>
>> IIUC, this would require support for multiple trusted keys backends at
>> runtime but currently the trusted keys subsystem only supports a
>> single backend which is selected via kernel module parameter during
>> boot.
>>
>> So the trusted keys framework needs to evolve to support multiple
>> trust sources at runtime but I would like to understand the use-cases
>> first. IMO, selecting the best trust source available on a platform
>> for trusted keys should be a one time operation, so why do we need to
>> have other backends available at runtime as well?
> 
> I thought about devices with a TPM-Chip and CAAM.
> IMHO allowing only one backend at the same time is a little over simplified. 

It is, but I'd rather leave this until it's actually needed.
What can be done now is adopting a format for the exported keys that would
make this extension seamless in future.

Cheers,
Ahmad
Sumit Garg April 1, 2021, 1:52 p.m. UTC | #27
On Thu, 1 Apr 2021 at 19:00, Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
>
> Hello Richard, Sumit,
>
> On 01.04.21 15:17, Richard Weinberger wrote:
> > Sumit,
> >
> > ----- Ursprüngliche Mail -----
> >> Von: "Sumit Garg" <sumit.garg@linaro.org>
> >> IIUC, this would require support for multiple trusted keys backends at
> >> runtime but currently the trusted keys subsystem only supports a
> >> single backend which is selected via kernel module parameter during
> >> boot.
> >>
> >> So the trusted keys framework needs to evolve to support multiple
> >> trust sources at runtime but I would like to understand the use-cases
> >> first. IMO, selecting the best trust source available on a platform
> >> for trusted keys should be a one time operation, so why do we need to
> >> have other backends available at runtime as well?
> >
> > I thought about devices with a TPM-Chip and CAAM.

In this case why would one prefer to use CAAM when you have standards
compliant TPM-Chip which additionally offers sealing to specific PCR
(integrity measurement) values.

> > IMHO allowing only one backend at the same time is a little over simplified.
>
> It is, but I'd rather leave this until it's actually needed.
> What can be done now is adopting a format for the exported keys that would
> make this extension seamless in future.
>

+1

-Sumit

> Cheers,
> Ahmad
>
> --
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
Richard Weinberger April 1, 2021, 1:59 p.m. UTC | #28
Sumit,

----- Ursprüngliche Mail -----
> Von: "Sumit Garg" <sumit.garg@linaro.org>
> In this case why would one prefer to use CAAM when you have standards
> compliant TPM-Chip which additionally offers sealing to specific PCR
> (integrity measurement) values.

I don't think we can dictate what good/sane solutions are and which are not.
Both CAAM and TPM have pros and cons, I don't see why supporting both is a bad idea.

>> > IMHO allowing only one backend at the same time is a little over simplified.
>>
>> It is, but I'd rather leave this until it's actually needed.
>> What can be done now is adopting a format for the exported keys that would
>> make this extension seamless in future.
>>
> 
> +1

As long we don't make multiple backends at runtime impossible I'm
fine and will happily add support for it when needed. :-)

Thanks,
//richard
Sumit Garg April 1, 2021, 2:12 p.m. UTC | #29
On Thu, 1 Apr 2021 at 19:29, Richard Weinberger <richard@nod.at> wrote:
>
> Sumit,
>
> ----- Ursprüngliche Mail -----
> > Von: "Sumit Garg" <sumit.garg@linaro.org>
> > In this case why would one prefer to use CAAM when you have standards
> > compliant TPM-Chip which additionally offers sealing to specific PCR
> > (integrity measurement) values.
>
> I don't think we can dictate what good/sane solutions are and which are not.
> Both CAAM and TPM have pros and cons, I don't see why supporting both is a bad idea.

I didn't mean to say that supporting both is a bad idea but rather I
was looking for use-cases where one time selection of the best trust
source (whether it be a TPM or TEE or CAAM etc.) for a platform
wouldn't suffice for user needs.

>
> >> > IMHO allowing only one backend at the same time is a little over simplified.
> >>
> >> It is, but I'd rather leave this until it's actually needed.
> >> What can be done now is adopting a format for the exported keys that would
> >> make this extension seamless in future.
> >>
> >
> > +1
>
> As long we don't make multiple backends at runtime impossible I'm
> fine and will happily add support for it when needed. :-)
>

You are most welcome to add such support. I will be happy to review it.

-Sumit

> Thanks,
> //richard
James Bottomley April 1, 2021, 6:26 p.m. UTC | #30
On Thu, 2021-04-01 at 18:50 +0530, Sumit Garg wrote:
> On Thu, 1 Apr 2021 at 15:36, Ahmad Fatoum <a.fatoum@pengutronix.de>
> wrote:
> > Hello Richard,
> > 
> > On 31.03.21 21:36, Richard Weinberger wrote:
> > > James,
> > > 
> > > ----- Ursprüngliche Mail -----
> > > > Von: "James Bottomley" <jejb@linux.ibm.com>
> > > > Well, yes.  For the TPM, there's a defined ASN.1 format for the
> > > > keys:
> > > > 
> > > > https://git.kernel.org/pub/scm/linux/kernel/git/jejb/openssl_tpm2_engine.git/tree/tpm2-asn.h
> > > > 
> > > > and part of the design of the file is that it's distinguishable
> > > > either
> > > > in DER or PEM (by the guards) format so any crypto application
> > > > can know
> > > > it's dealing with a TPM key simply by inspecting the file.  I
> > > > think you
> > > > need the same thing for CAAM and any other format.
> > > > 
> > > > We're encouraging new ASN.1 formats to be of the form
> > > > 
> > > > SEQUENCE {
> > > >    type   OBJECT IDENTIFIER
> > > >    ... key specific fields ...
> > > > }
> > > > 
> > > > Where you choose a defined OID to represent the key and that
> > > > means
> > > > every key even in DER form begins with a unique binary
> > > > signature.
> > > 
> > > I like this idea.
> > > Ahmad, what do you think?
> > > 
> > > That way we could also get rid off the kernel parameter and all
> > > the fall back logic,
> > > given that we find a way to reliable detect TEE blobs too...
> > 
> > Sounds good to me. Sumit, your thoughts on doing this for TEE as
> > well?
> > 
> 
> AFAIU, ASN.1 formating should be independent of trusted keys backends
> which could be abstracted to trusted keys core layer so that every
> backend could be plugged in seamlessly.
> 
> James,
> 
> Would it be possible to achieve this?

I'm not quite sure what you're asking.  The saved format of the keys is
particular to the underlying hardware.  The ASN.1 wraps this so we have
an identifier, some useful information to help load the key (like the
policy systemements) and then the blobs the hardware expects.

This makes the ASN.1 specific to the backend but having a
distinguishing OID that allows anyone to tell which backend it needs
from the file.

So you can call the ASN.1 format that begins with the type OID, the
"universal" format, but if you mean there should be a standard ASN.1
format for all keys that defines all the fields then that's not
possible because the fields after type are very specific to the
underlying hardware.

James