mbox series

[v5,0/3] Support for hardware-wrapped inline encryption keys

Message ID 20220228070520.74082-1-ebiggers@kernel.org
Headers show
Series Support for hardware-wrapped inline encryption keys | expand

Message

Eric Biggers Feb. 28, 2022, 7:05 a.m. UTC
[This patchset is based on v5.17-rc6.  It can also be retrieved from tag
"wrapped-keys-v5" of https://git.kernel.org/pub/scm/fs/fscrypt/fscrypt.git]

This patchset adds block and fscrypt support for hardware-wrapped keys
when the inline encryption hardware supports them.

Hardware-wrapped keys are inline encryption keys that are wrapped
(encrypted) by a key internal to the hardware.  The wrapping key is an
ephemeral per-boot key, except at initial unlocking time.
Hardware-wrapped keys can only be unwrapped (decrypted) by the hardware,
e.g. when a key is programmed into a keyslot.  They are never visible to
software in raw form, except optionally during key generation.  The
hardware supports importing keys as well as generating keys itself.

This feature protects encryption keys from read-only compromises of
kernel memory, such as that which can occur during a cold boot attack.
It does this without limiting the number of keys that can be used, as
would be the case with solutions that didn't use key wrapping.

Hardware supporting this feature is present on recent Qualcomm SoCs
(SM8350 and later) as well as on the Google Tensor SoC.

This patchset is organized as follows:

- Patch 1 adds the block support and documentation, excluding the ioctls
  needed to get a key ready to be used in the first place.

- Patch 2 adds new block device ioctls for importing, generating, and
  preparing hardware-wrapped keys.

- Patch 3 adds the fscrypt support and documentation.

For full details, see the individual patches, especially the detailed
documentation they add to Documentation/block/inline-encryption.rst and
Documentation/filesystems/fscrypt.rst.

This feature also requires UFS driver changes.  For Qualcomm SoCs, these
are being worked on at
https://lore.kernel.org/linux-scsi/20211206225725.77512-1-quic_gaurkash@quicinc.com/T/#u.
I've verified that this feature works end-to-end on the SM8350 HDK with
the upstream kernel with these two patchsets applied.  (One caveat is
that a custom TrustZone image needs to be flashed to the board too; I
understand that Qualcomm is working on addressing that.)

I've also written xfstests which verify that this feature encrypts files
correctly.  These tests can currently be found at
https://git.kernel.org/pub/scm/linux/kernel/git/ebiggers/xfstests-dev.git/log/?h=wip-wrapped-keys.
I'll be sending these tests in a separate patchset.

Changed v4 => v5:
    - Dropped the RFC tag, now that these patches are actually testable.
    - Split the BLKCRYPTOCREATEKEY ioctl into BLKCRYPTOIMPORTKEY and
      BLKCRYPTOGENERATEKEY.  (I'm thinking that these operations are
      distinct enough that two separate ioctls would be best.)
    - Added some warning messages in fscrypt_derive_sw_secret().
    - Rebased onto v5.17-rc6.

Changed v3 => v4:
    - Rebased onto v5.16-rc1 and dropped a few bits that were upstreamed.
    - Updated cover letter to link to Gaurav's UFS driver patchset.

Changed v2 => v3:
    - Dropped some fscrypt cleanups that were applied.
    - Rebased on top of the latest linux-block and fscrypt branches.
    - Minor cleanups.

Changed v1 => v2:
    - Added new ioctls for creating and preparing hardware-wrapped keys.
    - Rebased onto my patchset which renames blk_keyslot_manager to
      blk_crypto_profile.

Eric Biggers (3):
  block: add basic hardware-wrapped key support
  block: add ioctls to create and prepare hardware-wrapped keys
  fscrypt: add support for hardware-wrapped keys

 Documentation/block/inline-encryption.rst | 241 +++++++++++++++++++++-
 Documentation/filesystems/fscrypt.rst     | 154 ++++++++++++--
 block/blk-crypto-fallback.c               |   5 +-
 block/blk-crypto-internal.h               |  10 +
 block/blk-crypto-profile.c                |  97 +++++++++
 block/blk-crypto.c                        | 190 ++++++++++++++++-
 block/ioctl.c                             |   5 +
 drivers/md/dm-table.c                     |   1 +
 drivers/mmc/host/cqhci-crypto.c           |   2 +
 drivers/scsi/ufs/ufshcd-crypto.c          |   1 +
 fs/crypto/fscrypt_private.h               |  72 ++++++-
 fs/crypto/hkdf.c                          |   4 +-
 fs/crypto/inline_crypt.c                  |  75 ++++++-
 fs/crypto/keyring.c                       | 119 ++++++++---
 fs/crypto/keysetup.c                      |  71 ++++++-
 fs/crypto/keysetup_v1.c                   |   5 +-
 fs/crypto/policy.c                        |  11 +-
 include/linux/blk-crypto-profile.h        |  80 +++++++
 include/linux/blk-crypto.h                |  70 ++++++-
 include/uapi/linux/fs.h                   |  26 +++
 include/uapi/linux/fscrypt.h              |   7 +-
 21 files changed, 1153 insertions(+), 93 deletions(-)


base-commit: 7e57714cd0ad2d5bb90e50b5096a0e671dec1ef3

Comments

Bart Van Assche March 4, 2022, 12:53 a.m. UTC | #1
On 2/27/22 23:05, Eric Biggers wrote:
> -static u8 blank_key[BLK_CRYPTO_MAX_KEY_SIZE];
> +static u8 blank_key[BLK_CRYPTO_MAX_STANDARD_KEY_SIZE];
>   
>   static void blk_crypto_fallback_evict_keyslot(unsigned int slot)
>   {
> @@ -539,7 +539,7 @@ static int blk_crypto_fallback_init(void)
>   	if (blk_crypto_fallback_inited)
>   		return 0;
>   
> -	prandom_bytes(blank_key, BLK_CRYPTO_MAX_KEY_SIZE);
> +	prandom_bytes(blank_key, BLK_CRYPTO_MAX_STANDARD_KEY_SIZE);

Please use sizeof(blank_key) to make it easier for readers to verify that the 
length argument is correct.

> +int blk_crypto_derive_sw_secret(struct blk_crypto_profile *profile,
> +				const u8 *wrapped_key,
> +				unsigned int wrapped_key_size,
> +				u8 sw_secret[BLK_CRYPTO_SW_SECRET_SIZE])
> +{
> +	int err = -EOPNOTSUPP;
> +
> +	if (profile &&
> +	    (profile->key_types_supported & BLK_CRYPTO_KEY_TYPE_HW_WRAPPED) &&
> +	    profile->ll_ops.derive_sw_secret) {
> +		blk_crypto_hw_enter(profile);
> +		err = profile->ll_ops.derive_sw_secret(profile, wrapped_key,
> +						       wrapped_key_size,
> +						       sw_secret);
> +		blk_crypto_hw_exit(profile);
> +	}
> +	return err;
> +}

Please use the common kernel style: return early if the preconditions have not 
been met. That helps to keep the indentation level low.

> @@ -68,7 +71,10 @@ static int __init bio_crypt_ctx_init(void)
>   
>   	/* Sanity check that no algorithm exceeds the defined limits. */
>   	for (i = 0; i < BLK_ENCRYPTION_MODE_MAX; i++) {
> -		BUG_ON(blk_crypto_modes[i].keysize > BLK_CRYPTO_MAX_KEY_SIZE);
> +		BUG_ON(blk_crypto_modes[i].keysize >
> +		       BLK_CRYPTO_MAX_STANDARD_KEY_SIZE);
> +		BUG_ON(blk_crypto_modes[i].security_strength >
> +		       blk_crypto_modes[i].keysize);
>   		BUG_ON(blk_crypto_modes[i].ivsize > BLK_CRYPTO_MAX_IV_SIZE);
>   	}

Does the following advice from Linus Torvalds apply to the above code: "because 
there is NO EXCUSE to knowingly kill the kernel"? See also 
https://lkml.org/lkml/2016/10/4/1.

Thanks,

Bart.