On Mon, May 05, 2025 at 06:15:01PM +0200, Mikulas Patocka wrote:
>
>
> On Thu, 1 May 2025, Eric Biggers wrote:
>
> > From: Eric Biggers <ebigg...@google.com>
> >
> > Make the device-mapper layer pass through the derive_sw_secret,
> > import_key, generate_key, and prepare_key blk-crypto operations when all
> > underlying devices support hardware-wrapped inline crypto keys and are
> > passing through inline crypto support.
> >
> > Commit ebc4176551cd ("blk-crypto: add basic hardware-wrapped key
> > support") already made BLK_CRYPTO_KEY_TYPE_HW_WRAPPED be passed through
> > in the same way that the other crypto capabilities are. But the wrapped
> > key support also includes additional operations in blk_crypto_ll_ops,
> > and the dm layer needs to implement those to pass them through.
> > derive_sw_secret is needed by fscrypt, while the other operations are
> > needed for the new blk-crypto ioctls to work on device-mapper devices
> > and not just the raw partitions.
> >
> > Signed-off-by: Eric Biggers <ebigg...@google.com>
> > ---
> > drivers/md/dm-table.c | 177 ++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 177 insertions(+)
> >
> > diff --git a/drivers/md/dm-table.c b/drivers/md/dm-table.c
> > index a937e1e12482e..0a71bedff81c5 100644
> > --- a/drivers/md/dm-table.c
> > +++ b/drivers/md/dm-table.c
> > +
> > +static int dm_exec_wrappedkey_op(struct blk_crypto_profile *profile,
> > + struct dm_wrappedkey_op_args *args)
> > +{
> > + struct mapped_device *md =
> > + container_of(profile, struct dm_crypto_profile, profile)->md;
> > + struct dm_target *ti;
> > + struct dm_table *t;
> > + int srcu_idx;
> > + int i;
> > +
> > + args->err = -EOPNOTSUPP;
> > +
> > + t = dm_get_live_table(md, &srcu_idx);
> > + if (!t)
> > + goto out;
> > +
> > + /*
> > + * blk-crypto currently has no support for multiple incompatible
> > + * implementations of wrapped inline crypto keys on a single system.
> > + * It was already checked earlier that support for wrapped keys was
> > + * declared on all underlying devices. Thus, all the underlying devices
> > + * should support all wrapped key operations and they should behave
> > + * identically, i.e. work with the same keys. So, just executing the
> > + * operation on the first device on which it works suffices for now.
> > + */
> > + for (i = 0; i < t->num_targets; i++) {
> > + ti = dm_table_get_target(t, i);
> > + if (!ti->type->iterate_devices)
> > + continue;
> > + ti->type->iterate_devices(ti, dm_wrappedkey_op_callback, args);
> > + if (!args->err)
> > + break;
> > + }
>
> I have a dumb question - if it doesn't matter through which block device
> do you set up the keys, why do you set them up through a block device at
> all?
>
> What about making functions that set up the keys without taking block
> device as an argument, calling these functions directly and bypassing
> device mapper entirely?
Userspace needs to direct the key setup operations, so we'd need a UAPI for it
to do so. We could add a custom syscall, or some hacked-up extension of
add_key(), and add a custom registration mechanism to allow a single
implementation of wrapped keys (e.g. from ufs-qcom) to register itself as the
system's wrapped key provider which the syscall would then use.
But it seemed cleaner to instead use block device ioctls and take advantage of
the existing blk-crypto-profile. That already handles registering and
unregistering the implementation, and it also already handles things like
locking, and resuming the UFS controller if it's in suspend.
It also keeps the door open to supporting the case where different
wrapped-key-capable block devices don't necessarily accept the same keys, even
if that isn't the case currently.
- Eric
