| Message ID | 20210727144349.11215-1-a.fatoum@pengutronix.de (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | [RFC,v1] fscrypt: support encrypted and trusted keys | expand |
On Tue, Jul 27, 2021 at 04:43:49PM +0200, Ahmad Fatoum wrote: > For both v1 and v2 key setup mechanisms, userspace supplies the raw key > material to the kernel after which it is never again disclosed to > userspace. > > Use of encrypted and trusted keys offers stronger guarantees: > The key material is generated within the kernel and is never disclosed to > userspace in clear text and, in the case of trusted keys, can be > directly rooted to a trust source like a TPM chip. Please include a proper justification for this feature and update the relevant sections of Documentation/filesystems/fscrypt.rst to explain why someone would want to use this feature and what it accomplishes. As-is, this feature doesn't seem to have a very strong justification. Please also see previous threads where this feature was discussed/requested: https://lkml.kernel.org/linux-fscrypt/20180110124418.24385-1-git@andred.net/T/#u, https://lkml.kernel.org/linux-fscrypt/20180118131359.8365-1-git@andred.net/T/#u, https://lkml.kernel.org/linux-fscrypt/20200116193228.GA266386@vader/T/#u Note that there are several design flaws with the encrypted and trusted key types: - By default, trusted keys are generated using the TPM's RNG rather than the kernel's RNG, which places all trust in an unauditable black box. - trusted and encrypted keys aren't restricted to specific uses in the kernel (like the fscrypt-provisioning key type is) but rather are general-purpose. Hence, it may be possible to leak their contents to userspace by requesting their use for certain algorithms/features, e.g. to encrypt a dm-crypt target using a weak cipher that is vulnerable to key recovery attacks. - "encrypted" keys that use a master key of type "user" are supported, despite these being easily obtainable in the clear by userspace providing their own master key. This violates one of the main design goals of "encrypted" keys. Also, using the "trusted" key type isn't necessary to achieve TPM-bound encryption, as TPM binding can be handled in userspace instead. So I really would like to see a proper justification for this feature, and have it be properly documented. One comment on the UAPI below. > > Add support for trusted and encrypted keys by repurposing > fscrypt_add_key_arg::raw to hold the key description when the new > FSCRYPT_KEY_ARG_TYPE_DESC flag is supplied. The location of the flag > was previously reserved and enforced by ioctl code to be zero, so this > change won't break backwards compatibility. > > Corresponding userspace patches are available for fscryptctl: > https://github.com/google/fscryptctl/pull/23 > > Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de> > --- > key_extract_material used by this patch is added in > <cover.b2fdd70b830d12853b12a12e32ceb0c8162c1346.1626945419.git-series.a.fatoum@pengutronix.de> > which still awaits feedback. > > Sending this RFC out anyway to get some feedback from the fscrypt > developers whether this is the correct way to go about it. > > To: "Theodore Y. Ts'o" <tytso@mit.edu> > To: Jaegeuk Kim <jaegeuk@kernel.org> > To: Eric Biggers <ebiggers@kernel.org> > Cc: Jarkko Sakkinen <jarkko@kernel.org> > Cc: James Morris <jmorris@namei.org> > Cc: "Serge E. Hallyn" <serge@hallyn.com> > Cc: James Bottomley <jejb@linux.ibm.com> > Cc: Mimi Zohar <zohar@linux.ibm.com> > Cc: Sumit Garg <sumit.garg@linaro.org> > Cc: David Howells <dhowells@redhat.com> > Cc: linux-fscrypt@vger.kernel.org > Cc: linux-crypto@vger.kernel.org > Cc: linux-integrity@vger.kernel.org > Cc: linux-security-module@vger.kernel.org > Cc: keyrings@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > --- > Documentation/filesystems/fscrypt.rst | 24 ++++++++--- > fs/crypto/keyring.c | 59 ++++++++++++++++++++++++--- > include/uapi/linux/fscrypt.h | 16 +++++++- > 3 files changed, 87 insertions(+), 12 deletions(-) > > diff --git a/Documentation/filesystems/fscrypt.rst b/Documentation/filesystems/fscrypt.rst > index 44b67ebd6e40..83738af2afa3 100644 > --- a/Documentation/filesystems/fscrypt.rst > +++ b/Documentation/filesystems/fscrypt.rst > @@ -681,11 +681,15 @@ It can be executed on any file or directory on the target filesystem, > but using the filesystem's root directory is recommended. It takes in > a pointer to struct fscrypt_add_key_arg, defined as follows:: > > + #define FSCRYPT_KEY_ADD_RAW_ASIS 0 > + #define FSCRYPT_KEY_ADD_RAW_DESC 1 > + > struct fscrypt_add_key_arg { > struct fscrypt_key_specifier key_spec; > __u32 raw_size; > __u32 key_id; > - __u32 __reserved[8]; > + __u32 raw_flags; /* one of FSCRYPT_KEY_ADD_RAW_* */ > + __u32 __reserved[7]; > __u8 raw[]; > }; > > @@ -732,8 +736,11 @@ as follows: > Alternatively, if ``key_id`` is nonzero, this field must be 0, since > in that case the size is implied by the specified Linux keyring key. > > -- ``key_id`` is 0 if the raw key is given directly in the ``raw`` > - field. Otherwise ``key_id`` is the ID of a Linux keyring key of > +- If ``key_id`` is 0, the raw key is given directly in the ``raw`` > + field if ``raw_flags == FSCRYPT_KEY_ADD_RAW_ASIS``. With > + ``raw_flags == FSCRYPT_KEY_ADD_RAW_DESC``, ``raw`` is instead > + interpreted as the description of an encrypted or trusted key. > + Otherwise ``key_id`` is the ID of a Linux keyring key of > type "fscrypt-provisioning" whose payload is > struct fscrypt_provisioning_key_payload whose ``raw`` field contains > the raw key and whose ``type`` field matches ``key_spec.type``. > @@ -748,8 +755,15 @@ as follows: > without having to store the raw keys in userspace memory. Why not just allow the key_id field to specify a "trusted" or "encrypted" key? Why is it necessary for FS_IOC_ADD_ENCRYPTION_KEY to support two different ways of looking up keyring keys -- by ID and by description? Looking up by ID works fine for "fscrypt-provisioning" keys; why are "trusted" and "encrypted" keys different in this regard? - Eric
Hello Eric, On 27.07.21 18:38, Eric Biggers wrote: > On Tue, Jul 27, 2021 at 04:43:49PM +0200, Ahmad Fatoum wrote: >> For both v1 and v2 key setup mechanisms, userspace supplies the raw key >> material to the kernel after which it is never again disclosed to >> userspace. >> >> Use of encrypted and trusted keys offers stronger guarantees: >> The key material is generated within the kernel and is never disclosed to >> userspace in clear text and, in the case of trusted keys, can be >> directly rooted to a trust source like a TPM chip. > > Please include a proper justification for this feature I've patches pending for extending trusted keys to wrap the key sealing functionality of the CAAM IP on NXP SoCs[1]. I want the kernel to generate key material in the factory, have the CAAM encrypt it using its undisclosed unique key and pass it to userspace as encrypted blob that is persisted to an unencrypted volume. The intention is to thwart offline decryption of an encrypted file system in an embedded system, where a passphrase can't be supplied by an end user. Employing TPM and TEE trusted keys with this is already possible with dm-crypt, but I'd like this to be possible out-of-the-box with ubifs + fscrypt as well. > and update the relevant > sections of Documentation/filesystems/fscrypt.rst to explain why someone would > want to use this feature and what it accomplishes. How about: - type "fscrypt-provisioning" whose payload is + type "fscrypt-provisioning" or "trusted": + "fscrypt-provisioning" keys have a payload of struct fscrypt_provisioning_key_payload whose ``raw`` field contains the raw key and whose ``type`` field matches ``key_spec.type``. Since ``raw`` is variable-length, the total size of this key's payload must be ``sizeof(struct fscrypt_provisioning_key_payload)`` - plus the raw key size. The process must have Search permission on - this key. + plus the raw key size. + For "trusted" keys, the payload is directly taken as the raw key. + The process must have Search permission on this key. - Most users should leave this 0 and specify the raw key directly. + Most users leave this 0 and specify the raw key directly. - The support for specifying a Linux keyring key is intended mainly to - allow re-adding keys after a filesystem is unmounted and re-mounted, + "trusted" keys are useful to leverage kernel support for sealing and + unsealing key material. Sealed keys can be persisted to unencrypted + storage and later used to decrypt the file system without requiring + userspace to know the raw key material. + "fscrypt-provisioning" key support is intended mainly to allow + re-adding keys after a filesystem is unmounted and re-mounted, > As-is, this feature doesn't seem to have a very strong justification. Please > also see previous threads where this feature was discussed/requested: > https://lkml.kernel.org/linux-fscrypt/20180110124418.24385-1-git@andred.net/T/#u, > https://lkml.kernel.org/linux-fscrypt/20180118131359.8365-1-git@andred.net/T/#u, > https://lkml.kernel.org/linux-fscrypt/20200116193228.GA266386@vader/T/#u Thanks. I wasn't aware of the last one. I (re-)read them now. I hope this mail manages to address the concerns. (Also added original authors of these mail threads to CC) > Note that there are several design flaws with the encrypted and trusted key > types: > > - By default, trusted keys are generated using the TPM's RNG rather than the > kernel's RNG, which places all trust in an unauditable black box. Patch to fix that awaits feedback on linux-integrity[2]. > - trusted and encrypted keys aren't restricted to specific uses in the kernel > (like the fscrypt-provisioning key type is) but rather are general-purpose. > Hence, it may be possible to leak their contents to userspace by requesting > their use for certain algorithms/features, e.g. to encrypt a dm-crypt target > using a weak cipher that is vulnerable to key recovery attacks. The footgun is already there by allowing users to specify their own raw key. Users can already use $keyid for dm-crypt and then do $ keyctl pipe $keyid | fscryptctl add_key /mnt The responsibility to not reuse key material already lies with the users, regardless if they handle the raw key material directly or indirectly via a trusted key description/ID. > - "encrypted" keys that use a master key of type "user" are supported, despite > these being easily obtainable in the clear by userspace providing their own > master key. This violates one of the main design goals of "encrypted" keys. I care for trusted keys foremost, so I've no problems dropping the encrypted key support. > Also, using the "trusted" key type isn't necessary to achieve TPM-bound > encryption, as TPM binding can be handled in userspace instead. Trusted keys support TEE and hopefully CAAM soon as well. I don't want my userspace directly poking a DMA master. > So I really would like to see a proper justification for this feature, and have > it be properly documented. In light of the extended justification above, do you want me to respin with the proposed changes? > One comment on the UAPI below. > Why not just allow the key_id field to specify a "trusted" or "encrypted" key? > Why is it necessary for FS_IOC_ADD_ENCRYPTION_KEY to support two different ways > of looking up keyring keys -- by ID and by description? Looking up by ID works > fine for "fscrypt-provisioning" keys; why are "trusted" and "encrypted" keys > different in this regard? Mixture of reading emails predating key_id and misunderstanding the API. key_id would be much cleaner indeed. I can change this for v2. Thanks for your review. [1]: https://lore.kernel.org/linux-integrity/655aab117f922320e2123815afb5bf3daeb7b8b3.1626885907.git-series.a.fatoum@pengutronix.de/ [2]: https://lore.kernel.org/linux-integrity/cover.9fc9298fd9d63553491871d043a18affc2dbc8a8.1626885907.git-series.a.fatoum@pengutronix.de/T/#meaefcdc9ac091944ddadaebe0410c2325af0032e Cheers, Ahmad > > - Eric >
On Wed, Jul 28, 2021 at 10:50:42AM +0200, Ahmad Fatoum wrote: > Hello Eric, > > On 27.07.21 18:38, Eric Biggers wrote: > > On Tue, Jul 27, 2021 at 04:43:49PM +0200, Ahmad Fatoum wrote: > >> For both v1 and v2 key setup mechanisms, userspace supplies the raw key > >> material to the kernel after which it is never again disclosed to > >> userspace. > >> > >> Use of encrypted and trusted keys offers stronger guarantees: > >> The key material is generated within the kernel and is never disclosed to > >> userspace in clear text and, in the case of trusted keys, can be > >> directly rooted to a trust source like a TPM chip. > > > > Please include a proper justification for this feature > > I've patches pending for extending trusted keys to wrap the key sealing > functionality of the CAAM IP on NXP SoCs[1]. I want the kernel to > generate key material in the factory, have the CAAM encrypt it using its > undisclosed unique key and pass it to userspace as encrypted blob that is > persisted to an unencrypted volume. The intention is to thwart offline > decryption of an encrypted file system in an embedded system, where a > passphrase can't be supplied by an end user. > > Employing TPM and TEE trusted keys with this is already possible with > dm-crypt, but I'd like this to be possible out-of-the-box with > ubifs + fscrypt as well. Why not do the key management in userspace, like tpm-tools (https://github.com/tpm2-software/tpm2-tools)? There are a lot of uses for this type of hardware besides in-kernel crypto. See https://wiki.archlinux.org/title/Trusted_Platform_Module for all the things you can do with the TPM on Linux, including LUKS encryption; this is all with userspace key management. Wouldn't the CAAM hardware be useful for similar purposes and thus need a similar design as well, e.g. with functionality exposed through some /dev node for userspace to use? Or are you saying it will only ever be useful for in-kernel crypto? > > Note that there are several design flaws with the encrypted and trusted key > > types: > > > > - By default, trusted keys are generated using the TPM's RNG rather than the > > kernel's RNG, which places all trust in an unauditable black box. > > Patch to fix that awaits feedback on linux-integrity[2]. It does *not* fix it, as your patch only provides an option to use the kernel's RNG whereas the default is still the TPM's RNG. Most people don't change defaults. Essentially your same argument was used for Dual_EC_DRBG; people argued it was okay to standardize because people had the option to choose their own constants if they felt the default constants were backdoored. That didn't really matter, though, since in practice everyone just used the default constants. > > > - trusted and encrypted keys aren't restricted to specific uses in the kernel > > (like the fscrypt-provisioning key type is) but rather are general-purpose. > > Hence, it may be possible to leak their contents to userspace by requesting > > their use for certain algorithms/features, e.g. to encrypt a dm-crypt target > > using a weak cipher that is vulnerable to key recovery attacks. > > The footgun is already there by allowing users to specify their own > > raw key. Users can already use $keyid for dm-crypt and then do > > $ keyctl pipe $keyid | fscryptctl add_key /mnt > > The responsibility to not reuse key material already lies with the users, > regardless if they handle the raw key material directly or indirectly via > a trusted key description/ID. Elsewhere you are claiming that "trusted" keys can never be disclosed to userspace. So you can't rely on userspace cooperating, right? - Eric
On Tue, Jul 27, 2021 at 04:43:49PM +0200, Ahmad Fatoum wrote: > For both v1 and v2 key setup mechanisms, userspace supplies the raw key > material to the kernel after which it is never again disclosed to > userspace. > > Use of encrypted and trusted keys offers stronger guarantees: > The key material is generated within the kernel and is never disclosed to > userspace in clear text and, in the case of trusted keys, can be > directly rooted to a trust source like a TPM chip. > > Add support for trusted and encrypted keys by repurposing > fscrypt_add_key_arg::raw to hold the key description when the new > FSCRYPT_KEY_ARG_TYPE_DESC flag is supplied. The location of the flag > was previously reserved and enforced by ioctl code to be zero, so this > change won't break backwards compatibility. > > Corresponding userspace patches are available for fscryptctl: > https://github.com/google/fscryptctl/pull/23 > > Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de> > --- > key_extract_material used by this patch is added in > <cover.b2fdd70b830d12853b12a12e32ceb0c8162c1346.1626945419.git-series.a.fatoum@pengutronix.de> > which still awaits feedback. > > Sending this RFC out anyway to get some feedback from the fscrypt > developers whether this is the correct way to go about it. > > To: "Theodore Y. Ts'o" <tytso@mit.edu> > To: Jaegeuk Kim <jaegeuk@kernel.org> > To: Eric Biggers <ebiggers@kernel.org> > Cc: Jarkko Sakkinen <jarkko@kernel.org> > Cc: James Morris <jmorris@namei.org> > Cc: "Serge E. Hallyn" <serge@hallyn.com> > Cc: James Bottomley <jejb@linux.ibm.com> > Cc: Mimi Zohar <zohar@linux.ibm.com> > Cc: Sumit Garg <sumit.garg@linaro.org> > Cc: David Howells <dhowells@redhat.com> > Cc: linux-fscrypt@vger.kernel.org > Cc: linux-crypto@vger.kernel.org > Cc: linux-integrity@vger.kernel.org > Cc: linux-security-module@vger.kernel.org > Cc: keyrings@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > --- > Documentation/filesystems/fscrypt.rst | 24 ++++++++--- > fs/crypto/keyring.c | 59 ++++++++++++++++++++++++--- > include/uapi/linux/fscrypt.h | 16 +++++++- > 3 files changed, 87 insertions(+), 12 deletions(-) > > diff --git a/Documentation/filesystems/fscrypt.rst b/Documentation/filesystems/fscrypt.rst > index 44b67ebd6e40..83738af2afa3 100644 > --- a/Documentation/filesystems/fscrypt.rst > +++ b/Documentation/filesystems/fscrypt.rst > @@ -681,11 +681,15 @@ It can be executed on any file or directory on the target filesystem, > but using the filesystem's root directory is recommended. It takes in > a pointer to struct fscrypt_add_key_arg, defined as follows:: > > + #define FSCRYPT_KEY_ADD_RAW_ASIS 0 > + #define FSCRYPT_KEY_ADD_RAW_DESC 1 Would be nice to have these documented. /Jarkko
Hello Jarkko, On 29.07.21 00:22, Jarkko Sakkinen wrote: > On Tue, Jul 27, 2021 at 04:43:49PM +0200, Ahmad Fatoum wrote: >> For both v1 and v2 key setup mechanisms, userspace supplies the raw key >> material to the kernel after which it is never again disclosed to >> userspace. >> >> Use of encrypted and trusted keys offers stronger guarantees: >> The key material is generated within the kernel and is never disclosed to >> userspace in clear text and, in the case of trusted keys, can be >> directly rooted to a trust source like a TPM chip. >> >> Add support for trusted and encrypted keys by repurposing >> fscrypt_add_key_arg::raw to hold the key description when the new >> FSCRYPT_KEY_ARG_TYPE_DESC flag is supplied. The location of the flag >> was previously reserved and enforced by ioctl code to be zero, so this >> change won't break backwards compatibility. >> >> Corresponding userspace patches are available for fscryptctl: >> https://github.com/google/fscryptctl/pull/23 >> >> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de> >> --- >> key_extract_material used by this patch is added in >> <cover.b2fdd70b830d12853b12a12e32ceb0c8162c1346.1626945419.git-series.a.fatoum@pengutronix.de> >> which still awaits feedback. >> >> Sending this RFC out anyway to get some feedback from the fscrypt >> developers whether this is the correct way to go about it. >> >> To: "Theodore Y. Ts'o" <tytso@mit.edu> >> To: Jaegeuk Kim <jaegeuk@kernel.org> >> To: Eric Biggers <ebiggers@kernel.org> >> Cc: Jarkko Sakkinen <jarkko@kernel.org> >> Cc: James Morris <jmorris@namei.org> >> Cc: "Serge E. Hallyn" <serge@hallyn.com> >> Cc: James Bottomley <jejb@linux.ibm.com> >> Cc: Mimi Zohar <zohar@linux.ibm.com> >> Cc: Sumit Garg <sumit.garg@linaro.org> >> Cc: David Howells <dhowells@redhat.com> >> Cc: linux-fscrypt@vger.kernel.org >> Cc: linux-crypto@vger.kernel.org >> Cc: linux-integrity@vger.kernel.org >> Cc: linux-security-module@vger.kernel.org >> Cc: keyrings@vger.kernel.org >> Cc: linux-kernel@vger.kernel.org >> --- >> Documentation/filesystems/fscrypt.rst | 24 ++++++++--- >> fs/crypto/keyring.c | 59 ++++++++++++++++++++++++--- >> include/uapi/linux/fscrypt.h | 16 +++++++- >> 3 files changed, 87 insertions(+), 12 deletions(-) >> >> diff --git a/Documentation/filesystems/fscrypt.rst b/Documentation/filesystems/fscrypt.rst >> index 44b67ebd6e40..83738af2afa3 100644 >> --- a/Documentation/filesystems/fscrypt.rst >> +++ b/Documentation/filesystems/fscrypt.rst >> @@ -681,11 +681,15 @@ It can be executed on any file or directory on the target filesystem, >> but using the filesystem's root directory is recommended. It takes in >> a pointer to struct fscrypt_add_key_arg, defined as follows:: >> >> + #define FSCRYPT_KEY_ADD_RAW_ASIS 0 >> + #define FSCRYPT_KEY_ADD_RAW_DESC 1 > > Would be nice to have these documented. They have explanatory comments in the uAPI header. The Documentation file purposefully omits these comments and describes each field separately after the code block. I am just following suit. FWIW, I intend to drop these flags for v2 anyway. Cheers, Ahmad > > /Jarkko >
Hi Eric, On Wed, 28 Jul 2021 at 21:35, Eric Biggers <ebiggers@kernel.org> wrote: > > On Wed, Jul 28, 2021 at 10:50:42AM +0200, Ahmad Fatoum wrote: > > Hello Eric, > > > > On 27.07.21 18:38, Eric Biggers wrote: > > > On Tue, Jul 27, 2021 at 04:43:49PM +0200, Ahmad Fatoum wrote: > > >> For both v1 and v2 key setup mechanisms, userspace supplies the raw key > > >> material to the kernel after which it is never again disclosed to > > >> userspace. > > >> > > >> Use of encrypted and trusted keys offers stronger guarantees: > > >> The key material is generated within the kernel and is never disclosed to > > >> userspace in clear text and, in the case of trusted keys, can be > > >> directly rooted to a trust source like a TPM chip. > > > > > > Please include a proper justification for this feature > > > > I've patches pending for extending trusted keys to wrap the key sealing > > functionality of the CAAM IP on NXP SoCs[1]. I want the kernel to > > generate key material in the factory, have the CAAM encrypt it using its > > undisclosed unique key and pass it to userspace as encrypted blob that is > > persisted to an unencrypted volume. The intention is to thwart offline > > decryption of an encrypted file system in an embedded system, where a > > passphrase can't be supplied by an end user. > > > > Employing TPM and TEE trusted keys with this is already possible with > > dm-crypt, but I'd like this to be possible out-of-the-box with > > ubifs + fscrypt as well. > > Why not do the key management in userspace, like tpm-tools > (https://github.com/tpm2-software/tpm2-tools)? There are a lot of uses for this > type of hardware besides in-kernel crypto. See > https://wiki.archlinux.org/title/Trusted_Platform_Module for all the things you > can do with the TPM on Linux, including LUKS encryption; this is all with > userspace key management. Wouldn't the CAAM hardware be useful for similar > purposes and thus need a similar design as well, e.g. with functionality exposed > through some /dev node for userspace to use? Or are you saying it will only > ever be useful for in-kernel crypto? AFAIK from my prior experience while working with CAAM engine during my time at NXP, it is generally a crypto engine with additional security properties like one discussed here to protect keys (blob encap and decap) etc. But it doesn't offer user authentication similar to what a TPM (ownership) can offer. Although, one should be able to expose CAAM via /dev node but I am not sure if that would be really useful without user authentication. I think similar should be the case for other crypto engines with additional security properties. With restriction of CAAM's security properties to kernel crypto we could at least ensure a kernel boundary that should offer enough resistance from malicious user space attacks. > > > > Note that there are several design flaws with the encrypted and trusted key > > > types: > > > > > > - By default, trusted keys are generated using the TPM's RNG rather than the > > > kernel's RNG, which places all trust in an unauditable black box. > > With regards to trusted keys generated using the TEE's RNG, the underlying implementation being OP-TEE [1] which is an open source TEE implementation built on top of Arm TrustZone providing the hardware based isolation among the TEE and Linux. So regarding auditability, it should be comparatively easier to audit the TEE components designed with a goal of minimal footprint when compared with Linux kernel. [1] https://github.com/OP-TEE/optee_os > > Patch to fix that awaits feedback on linux-integrity[2]. > > It does *not* fix it, as your patch only provides an option to use the kernel's > RNG whereas the default is still the TPM's RNG. > Yes in case of TPM, default is still TPM's RNG but with Ahmad's patch #2, the trust source backend like CAAM should be able to use kernel's RNG by default. -Sumit > Most people don't change defaults. > > Essentially your same argument was used for Dual_EC_DRBG; people argued it was > okay to standardize because people had the option to choose their own constants > if they felt the default constants were backdoored. That didn't really matter, > though, since in practice everyone just used the default constants. > > > > > > - trusted and encrypted keys aren't restricted to specific uses in the kernel > > > (like the fscrypt-provisioning key type is) but rather are general-purpose. > > > Hence, it may be possible to leak their contents to userspace by requesting > > > their use for certain algorithms/features, e.g. to encrypt a dm-crypt target > > > using a weak cipher that is vulnerable to key recovery attacks. > > > > The footgun is already there by allowing users to specify their own > > > > raw key. Users can already use $keyid for dm-crypt and then do > > > > $ keyctl pipe $keyid | fscryptctl add_key /mnt > > > > The responsibility to not reuse key material already lies with the users, > > regardless if they handle the raw key material directly or indirectly via > > a trusted key description/ID. > > Elsewhere you are claiming that "trusted" keys can never be disclosed to > userspace. So you can't rely on userspace cooperating, right? > > - Eric
On 29.07.21 07:56, Sumit Garg wrote: > Hi Eric, > > On Wed, 28 Jul 2021 at 21:35, Eric Biggers <ebiggers@kernel.org> wrote: >> >> On Wed, Jul 28, 2021 at 10:50:42AM +0200, Ahmad Fatoum wrote: >>> Hello Eric, >>> >>> On 27.07.21 18:38, Eric Biggers wrote: >>>> On Tue, Jul 27, 2021 at 04:43:49PM +0200, Ahmad Fatoum wrote: >>>>> For both v1 and v2 key setup mechanisms, userspace supplies the raw key >>>>> material to the kernel after which it is never again disclosed to >>>>> userspace. >>>>> >>>>> Use of encrypted and trusted keys offers stronger guarantees: >>>>> The key material is generated within the kernel and is never disclosed to >>>>> userspace in clear text and, in the case of trusted keys, can be >>>>> directly rooted to a trust source like a TPM chip. >>>> >>>> Please include a proper justification for this feature >>> >>> I've patches pending for extending trusted keys to wrap the key sealing >>> functionality of the CAAM IP on NXP SoCs[1]. I want the kernel to >>> generate key material in the factory, have the CAAM encrypt it using its >>> undisclosed unique key and pass it to userspace as encrypted blob that is >>> persisted to an unencrypted volume. The intention is to thwart offline >>> decryption of an encrypted file system in an embedded system, where a >>> passphrase can't be supplied by an end user. >>> >>> Employing TPM and TEE trusted keys with this is already possible with >>> dm-crypt, but I'd like this to be possible out-of-the-box with >>> ubifs + fscrypt as well. >> >> Why not do the key management in userspace, like tpm-tools >> (https://github.com/tpm2-software/tpm2-tools)? There are a lot of uses for this >> type of hardware besides in-kernel crypto. See >> https://wiki.archlinux.org/title/Trusted_Platform_Module for all the things you >> can do with the TPM on Linux, including LUKS encryption; this is all with >> userspace key management. Wouldn't the CAAM hardware be useful for similar >> purposes and thus need a similar design as well, e.g. with functionality exposed >> through some /dev node for userspace to use? Or are you saying it will only >> ever be useful for in-kernel crypto? > > AFAIK from my prior experience while working with CAAM engine during > my time at NXP, it is generally a crypto engine with additional > security properties like one discussed here to protect keys (blob > encap and decap) etc. But it doesn't offer user authentication similar > to what a TPM (ownership) can offer. Although, one should be able to > expose CAAM via /dev node but I am not sure if that would be really > useful without user authentication. I think similar should be the case > for other crypto engines with additional security properties. > > With restriction of CAAM's security properties to kernel crypto we > could at least ensure a kernel boundary that should offer enough > resistance from malicious user space attacks. Which are a real possibility given the CAAM's direct memory access. We now have safe interfaces mainline wrapping it for HWRNG, crypto acceleration and hopefully soon key sealing and I don't think throwing existing kernel driver support away and inventing a new CAAM uAPI is a practical alternative. >>>> Note that there are several design flaws with the encrypted and trusted key >>>> types: >>>> >>>> - By default, trusted keys are generated using the TPM's RNG rather than the >>>> kernel's RNG, which places all trust in an unauditable black box. >>> > > With regards to trusted keys generated using the TEE's RNG, the > underlying implementation being OP-TEE [1] which is an open source TEE > implementation built on top of Arm TrustZone providing the hardware > based isolation among the TEE and Linux. So regarding auditability, it > should be comparatively easier to audit the TEE components designed > with a goal of minimal footprint when compared with Linux kernel. > > [1] https://github.com/OP-TEE/optee_os > >>> Patch to fix that awaits feedback on linux-integrity[2]. >> >> It does *not* fix it, as your patch only provides an option to use the kernel's >> RNG whereas the default is still the TPM's RNG. >> > > Yes in case of TPM, default is still TPM's RNG but with Ahmad's patch > #2, the trust source backend like CAAM should be able to use kernel's > RNG by default. Exactly. >> Most people don't change defaults. >> >> Essentially your same argument was used for Dual_EC_DRBG; people argued it was >> okay to standardize because people had the option to choose their own constants >> if they felt the default constants were backdoored. That didn't really matter, >> though, since in practice everyone just used the default constants. I'd appreciate your feedback on my CAAM series if you think the defaults can be improved. Trusted keys are no longer restricted to TPMs, so users of other backends shouldn't be dismissed, because one backend can be used with fscrypt by alternative means. >>>> - trusted and encrypted keys aren't restricted to specific uses in the kernel >>>> (like the fscrypt-provisioning key type is) but rather are general-purpose. >>>> Hence, it may be possible to leak their contents to userspace by requesting >>>> their use for certain algorithms/features, e.g. to encrypt a dm-crypt target >>>> using a weak cipher that is vulnerable to key recovery attacks. >>> >>> The footgun is already there by allowing users to specify their own >>> >>> raw key. Users can already use $keyid for dm-crypt and then do >>> >>> $ keyctl pipe $keyid | fscryptctl add_key /mnt >>> >>> The responsibility to not reuse key material already lies with the users, >>> regardless if they handle the raw key material directly or indirectly via >>> a trusted key description/ID. >> >> Elsewhere you are claiming that "trusted" keys can never be disclosed to >> userspace. So you can't rely on userspace cooperating, right? The users I meant are humans, e.g. system integrators. They need to think about burning fuses, signing bootloaders, verifying kernel and root file systems, encrypting file systems and safekeeping their crypto keys. Ample opportunity for stuff to go wrong. They would benefit from having relevant kernel functionality integrate with each other instead of having to carry downstream patches, which we and many others[1] did for years. We now finally have a chance to drop this technical debt thanks to Sumit's trusted key rework and improve user security along the way. So, Eric, how should we proceed? [1]: See the CAAM series cover letter for a history of CAAM sealing upstreaming efforts Cheers, Ahmad
On Thu, Jul 29, 2021 at 11:07:00AM +0200, Ahmad Fatoum wrote: > >> Most people don't change defaults. > >> > >> Essentially your same argument was used for Dual_EC_DRBG; people argued it was > >> okay to standardize because people had the option to choose their own constants > >> if they felt the default constants were backdoored. That didn't really matter, > >> though, since in practice everyone just used the default constants. > > I'd appreciate your feedback on my CAAM series if you think the defaults > can be improved. Trusted keys are no longer restricted to TPMs, > so users of other backends shouldn't be dismissed, because one backend > can be used with fscrypt by alternative means. I already gave feedback: https://lkml.kernel.org/keyrings/YGOcZtkw3ZM5kvl6@gmail.com https://lkml.kernel.org/keyrings/YGUHBelwhvJDhKoo@gmail.com https://lkml.kernel.org/keyrings/YGViOc3DG+Pjuur6@sol.localdomain > >>>> - trusted and encrypted keys aren't restricted to specific uses in the kernel > >>>> (like the fscrypt-provisioning key type is) but rather are general-purpose. > >>>> Hence, it may be possible to leak their contents to userspace by requesting > >>>> their use for certain algorithms/features, e.g. to encrypt a dm-crypt target > >>>> using a weak cipher that is vulnerable to key recovery attacks. > >>> > >>> The footgun is already there by allowing users to specify their own > >>> > >>> raw key. Users can already use $keyid for dm-crypt and then do > >>> > >>> $ keyctl pipe $keyid | fscryptctl add_key /mnt > >>> > >>> The responsibility to not reuse key material already lies with the users, > >>> regardless if they handle the raw key material directly or indirectly via > >>> a trusted key description/ID. > >> > >> Elsewhere you are claiming that "trusted" keys can never be disclosed to > >> userspace. So you can't rely on userspace cooperating, right? > > The users I meant are humans, e.g. system integrators. They need to think about > > burning fuses, signing bootloaders, verifying kernel and root file systems, > > encrypting file systems and safekeeping their crypto keys. Ample opportunity for > > stuff to go wrong. They would benefit from having relevant kernel functionality > > integrate with each other instead of having to carry downstream patches, which > we and many others[1] did for years. We now finally have a chance to drop this > technical debt thanks to Sumit's trusted key rework and improve user security > along the way. > > So, Eric, how should we proceed? > It is probably inevitable that this be added, but you need to document it properly and not make misleading claims like "The key material is generated within the kernel" (for the TPM "trust" source the default is to use the TPM's RNG, *not* the kernel RNG), and "is never disclosed to userspace in clear text" (that's only guaranteed to be true for non-malicious userspace). Also please properly document that this is mainly intended for accessing key wrapping hardware such as CAAM that can't be accessed from userspace in another way like TPMs can. - Eric
diff --git a/Documentation/filesystems/fscrypt.rst b/Documentation/filesystems/fscrypt.rst index 44b67ebd6e40..83738af2afa3 100644 --- a/Documentation/filesystems/fscrypt.rst +++ b/Documentation/filesystems/fscrypt.rst @@ -681,11 +681,15 @@ It can be executed on any file or directory on the target filesystem, but using the filesystem's root directory is recommended. It takes in a pointer to struct fscrypt_add_key_arg, defined as follows:: + #define FSCRYPT_KEY_ADD_RAW_ASIS 0 + #define FSCRYPT_KEY_ADD_RAW_DESC 1 + struct fscrypt_add_key_arg { struct fscrypt_key_specifier key_spec; __u32 raw_size; __u32 key_id; - __u32 __reserved[8]; + __u32 raw_flags; /* one of FSCRYPT_KEY_ADD_RAW_* */ + __u32 __reserved[7]; __u8 raw[]; }; @@ -732,8 +736,11 @@ as follows: Alternatively, if ``key_id`` is nonzero, this field must be 0, since in that case the size is implied by the specified Linux keyring key. -- ``key_id`` is 0 if the raw key is given directly in the ``raw`` - field. Otherwise ``key_id`` is the ID of a Linux keyring key of +- If ``key_id`` is 0, the raw key is given directly in the ``raw`` + field if ``raw_flags == FSCRYPT_KEY_ADD_RAW_ASIS``. With + ``raw_flags == FSCRYPT_KEY_ADD_RAW_DESC``, ``raw`` is instead + interpreted as the description of an encrypted or trusted key. + Otherwise ``key_id`` is the ID of a Linux keyring key of type "fscrypt-provisioning" whose payload is struct fscrypt_provisioning_key_payload whose ``raw`` field contains the raw key and whose ``type`` field matches ``key_spec.type``. @@ -748,8 +755,15 @@ as follows: without having to store the raw keys in userspace memory. - ``raw`` is a variable-length field which must contain the actual - key, ``raw_size`` bytes long. Alternatively, if ``key_id`` is - nonzero, then this field is unused. + key when ``raw_flags == FSCRYPT_KEY_ADD_RAW_ASIS``, + ``raw_size`` bytes long. Alternatively, if + ``raw_flags == FSCRYPT_KEY_ADD_RAW_DESC``, ``raw`` is interpreted + as the key description of an encrypted or trusted key, in that order. + The material of this key will be used as if it were a raw key + supplied by userspace. + + In both cases, the buffer is ``raw_size`` bytes long. If ````key_id`` + is nonzero, then this field is unused. For v2 policy keys, the kernel keeps track of which user (identified by effective user ID) added the key, and only allows the key to be diff --git a/fs/crypto/keyring.c b/fs/crypto/keyring.c index 0b3ffbb4faf4..484f7c883b17 100644 --- a/fs/crypto/keyring.c +++ b/fs/crypto/keyring.c @@ -20,6 +20,9 @@ #include <crypto/skcipher.h> #include <linux/key-type.h> +#include <linux/key-type.h> +#include <keys/encrypted-type.h> +#include <keys/trusted-type.h> #include <linux/random.h> #include <linux/seq_file.h> @@ -662,13 +665,57 @@ int fscrypt_ioctl_add_key(struct file *filp, void __user *_uarg) if (err) goto out_wipe_secret; } else { - if (arg.raw_size < FSCRYPT_MIN_KEY_SIZE || - arg.raw_size > FSCRYPT_MAX_KEY_SIZE) + struct key *keyring_key = ERR_PTR(-EINVAL); + const void *key_material; + const char *desc; + + switch (arg.raw_flags) { + case FSCRYPT_KEY_ADD_RAW_ASIS: + if (arg.raw_size < FSCRYPT_MIN_KEY_SIZE || + arg.raw_size > FSCRYPT_MAX_KEY_SIZE) + return -EINVAL; + secret.size = arg.raw_size; + err = -EFAULT; + if (copy_from_user(secret.raw, uarg->raw, secret.size)) + goto out_wipe_secret; + break; + case FSCRYPT_KEY_ADD_RAW_DESC: + if (arg.raw_size > 4096) + return -EINVAL; + desc = memdup_user_nul(uarg->raw, arg.raw_size); + if (IS_ERR(desc)) + return PTR_ERR(desc); + + if (IS_REACHABLE(CONFIG_ENCRYPTED_KEYS)) + keyring_key = request_key(&key_type_encrypted, desc, NULL); + if (IS_REACHABLE(CONFIG_TRUSTED_KEYS) && IS_ERR(keyring_key)) + keyring_key = request_key(&key_type_trusted, desc, NULL); + + kfree(desc); + + if (IS_ERR(keyring_key)) + return PTR_ERR(keyring_key); + + down_read(&keyring_key->sem); + + key_material = key_extract_material(keyring_key, &secret.size); + if (!IS_ERR(key_material) && (secret.size < FSCRYPT_MIN_KEY_SIZE || + secret.size > FSCRYPT_MAX_KEY_SIZE)) + key_material = ERR_PTR(-EINVAL); + if (IS_ERR(key_material)) { + up_read(&keyring_key->sem); + key_put(keyring_key); + return PTR_ERR(key_material); + } + + memcpy(secret.raw, key_material, secret.size); + + up_read(&keyring_key->sem); + key_put(keyring_key); + break; + default: return -EINVAL; - secret.size = arg.raw_size; - err = -EFAULT; - if (copy_from_user(secret.raw, uarg->raw, secret.size)) - goto out_wipe_secret; + } } err = add_master_key(sb, &secret, &arg.key_spec); diff --git a/include/uapi/linux/fscrypt.h b/include/uapi/linux/fscrypt.h index 9f4428be3e36..bd498a188cf5 100644 --- a/include/uapi/linux/fscrypt.h +++ b/include/uapi/linux/fscrypt.h @@ -119,12 +119,26 @@ struct fscrypt_provisioning_key_payload { __u8 raw[]; }; +/* + * fscrypt_add_key_arg::raw contains the raw key material directly + * if key_id == 0 + */ +#define FSCRYPT_KEY_ADD_RAW_ASIS 0 + +/* + * fscrypt_add_key_arg::raw is a key descriptor for an already + * existing kernel encrypted or trusted key if key_id == 0. + * The kernel key's material will be used as input for fscrypt. + */ +#define FSCRYPT_KEY_ADD_RAW_DESC 1 + /* Struct passed to FS_IOC_ADD_ENCRYPTION_KEY */ struct fscrypt_add_key_arg { struct fscrypt_key_specifier key_spec; __u32 raw_size; __u32 key_id; - __u32 __reserved[8]; + __u32 raw_flags; /* one of FSCRYPT_KEY_ADD_RAW_* */ + __u32 __reserved[7]; __u8 raw[]; };
For both v1 and v2 key setup mechanisms, userspace supplies the raw key material to the kernel after which it is never again disclosed to userspace. Use of encrypted and trusted keys offers stronger guarantees: The key material is generated within the kernel and is never disclosed to userspace in clear text and, in the case of trusted keys, can be directly rooted to a trust source like a TPM chip. Add support for trusted and encrypted keys by repurposing fscrypt_add_key_arg::raw to hold the key description when the new FSCRYPT_KEY_ARG_TYPE_DESC flag is supplied. The location of the flag was previously reserved and enforced by ioctl code to be zero, so this change won't break backwards compatibility. Corresponding userspace patches are available for fscryptctl: https://github.com/google/fscryptctl/pull/23 Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de> --- key_extract_material used by this patch is added in <cover.b2fdd70b830d12853b12a12e32ceb0c8162c1346.1626945419.git-series.a.fatoum@pengutronix.de> which still awaits feedback. Sending this RFC out anyway to get some feedback from the fscrypt developers whether this is the correct way to go about it. To: "Theodore Y. Ts'o" <tytso@mit.edu> To: Jaegeuk Kim <jaegeuk@kernel.org> To: Eric Biggers <ebiggers@kernel.org> Cc: Jarkko Sakkinen <jarkko@kernel.org> Cc: James Morris <jmorris@namei.org> Cc: "Serge E. Hallyn" <serge@hallyn.com> Cc: James Bottomley <jejb@linux.ibm.com> Cc: Mimi Zohar <zohar@linux.ibm.com> Cc: Sumit Garg <sumit.garg@linaro.org> Cc: David Howells <dhowells@redhat.com> Cc: linux-fscrypt@vger.kernel.org Cc: linux-crypto@vger.kernel.org Cc: linux-integrity@vger.kernel.org Cc: linux-security-module@vger.kernel.org Cc: keyrings@vger.kernel.org Cc: linux-kernel@vger.kernel.org --- Documentation/filesystems/fscrypt.rst | 24 ++++++++--- fs/crypto/keyring.c | 59 ++++++++++++++++++++++++--- include/uapi/linux/fscrypt.h | 16 +++++++- 3 files changed, 87 insertions(+), 12 deletions(-)