Thread (9 messages) 9 messages, 4 authors, 2021-07-29

Re: [RFC PATCH v1] fscrypt: support encrypted and trusted keys

From: Eric Biggers <ebiggers@kernel.org>
Date: 2021-07-27 16:38:29
Also in: keyrings, linux-crypto, linux-fscrypt, linux-security-module, lkml

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.
quoted hunk ↗ jump to hunk
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
[ref]
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 <redacted>
Cc: Mimi Zohar <zohar@linux.ibm.com>
Cc: Sumit Garg <redacted>
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help