All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] sed-opal: if key is available from IOC_OPAL_SAVE use it when locking
@ 2022-12-02  0:36 luca.boccassi
  2022-12-02  8:48 ` Christian Brauner
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: luca.boccassi @ 2022-12-02  0:36 UTC (permalink / raw)
  To: linux-block; +Cc: jonathan.derrick, gmazyland, axboe, brauner, stepan.horacek

From: Luca Boccassi <bluca@debian.org>

Usually when closing a crypto device (eg: dm-crypt with LUKS) the
volume key is not required, as it requires root privileges anyway, and
root can deny access to a disk in many ways regardless. Requiring the
volume key to lock the device is a peculiarity of the OPAL
specification.

Given we might already have saved the key if the user requested it via
the 'IOC_OPAL_SAVE' ioctl, we can use that key to lock the device if no
key was provided here and the locking range matches. This allows
integrating OPAL with tools and libraries that are used to the common
behaviour and do not ask for the volume key when closing a device.

If the caller provides a key on the other hand it will still be used as
before, no changes in that case.

Suggested-by: Štěpán Horáček <stepan.horacek@gmail.com>
Signed-off-by: Luca Boccassi <bluca@debian.org>
---
 block/sed-opal.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/block/sed-opal.c b/block/sed-opal.c
index 9bdb833e5817..b54bb76e4484 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -2470,6 +2470,35 @@ static int opal_lock_unlock(struct opal_dev *dev,
 		return -EINVAL;
 
 	mutex_lock(&dev->dev_lock);
+
+	/*
+	 * Usually when closing a crypto device (eg: dm-crypt with LUKS) the volume key
+	 * is not required, as it requires root privileges anyway, and root can deny
+	 * access to a disk in many ways regardless. Requiring the volume key to lock
+	 * the device is a peculiarity of the OPAL specification.
+	 * Given we might already have saved the key if the user requested it via the
+	 * 'IOC_OPAL_SAVE' ioctl, we can use that key to lock the device if no key was
+	 * provided here and the locking range matches. This allows integrating OPAL
+	 * with tools and libraries that are used to the common behaviour and do not
+	 * ask for the volume key when closing a device.
+	 */
+	if (lk_unlk->l_state == OPAL_LK && lk_unlk->session.opal_key.key_len == 0) {
+		struct opal_suspend_data *iter;
+
+		setup_opal_dev(dev);
+		list_for_each_entry(iter, &dev->unlk_lst, node) {
+			if (iter->lr == lk_unlk->session.opal_key.lr &&
+					iter->unlk.session.opal_key.key_len > 0) {
+				lk_unlk->session.opal_key.key_len =
+					iter->unlk.session.opal_key.key_len;
+				memcpy(lk_unlk->session.opal_key.key,
+					iter->unlk.session.opal_key.key,
+					iter->unlk.session.opal_key.key_len);
+				break;
+			}
+		}
+	}
+
 	ret = __opal_lock_unlock(dev, lk_unlk);
 	mutex_unlock(&dev->dev_lock);
 
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH] sed-opal: if key is available from IOC_OPAL_SAVE use it when locking
  2022-12-02  0:36 [PATCH] sed-opal: if key is available from IOC_OPAL_SAVE use it when locking luca.boccassi
@ 2022-12-02  8:48 ` Christian Brauner
  2022-12-02  9:11   ` Christoph Hellwig
  2022-12-02 10:28   ` Luca Boccassi
  2022-12-03  0:12 ` [PATCH v2] sed-opal: allow using IOC_OPAL_SAVE for locking too luca.boccassi
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 14+ messages in thread
From: Christian Brauner @ 2022-12-02  8:48 UTC (permalink / raw)
  To: luca.boccassi
  Cc: linux-block, jonathan.derrick, gmazyland, axboe, stepan.horacek

On Fri, Dec 02, 2022 at 12:36:10AM +0000, luca.boccassi@gmail.com wrote:
> From: Luca Boccassi <bluca@debian.org>
> 
> Usually when closing a crypto device (eg: dm-crypt with LUKS) the
> volume key is not required, as it requires root privileges anyway, and
> root can deny access to a disk in many ways regardless. Requiring the
> volume key to lock the device is a peculiarity of the OPAL
> specification.
> 
> Given we might already have saved the key if the user requested it via
> the 'IOC_OPAL_SAVE' ioctl, we can use that key to lock the device if no
> key was provided here and the locking range matches. This allows
> integrating OPAL with tools and libraries that are used to the common
> behaviour and do not ask for the volume key when closing a device.
> 
> If the caller provides a key on the other hand it will still be used as
> before, no changes in that case.
> 
> Suggested-by: Štěpán Horáček <stepan.horacek@gmail.com>
> Signed-off-by: Luca Boccassi <bluca@debian.org>
> ---

But it would be quite the change in behavior for existing users, no?

It might be better to add an ioctl that would allow to set an option on
the opal device after it was opened which marks it as closable without
providing the key?

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] sed-opal: if key is available from IOC_OPAL_SAVE use it when locking
  2022-12-02  8:48 ` Christian Brauner
@ 2022-12-02  9:11   ` Christoph Hellwig
  2022-12-02 10:28   ` Luca Boccassi
  1 sibling, 0 replies; 14+ messages in thread
From: Christoph Hellwig @ 2022-12-02  9:11 UTC (permalink / raw)
  To: Christian Brauner
  Cc: luca.boccassi, linux-block, jonathan.derrick, gmazyland, axboe,
	stepan.horacek

On Fri, Dec 02, 2022 at 09:48:45AM +0100, Christian Brauner wrote:
> It might be better to add an ioctl that would allow to set an option on
> the opal device after it was opened which marks it as closable without
> providing the key?

Yes.

And while we're at it: please fix the overly long lines in the block
comment that make it completely unreadable.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] sed-opal: if key is available from IOC_OPAL_SAVE use it when locking
  2022-12-02  8:48 ` Christian Brauner
  2022-12-02  9:11   ` Christoph Hellwig
@ 2022-12-02 10:28   ` Luca Boccassi
  2022-12-02 10:37     ` Christian Brauner
  1 sibling, 1 reply; 14+ messages in thread
From: Luca Boccassi @ 2022-12-02 10:28 UTC (permalink / raw)
  To: Christian Brauner
  Cc: linux-block, jonathan.derrick, gmazyland, axboe, stepan.horacek

On Fri, 2 Dec 2022 at 08:48, Christian Brauner <brauner@kernel.org> wrote:
>
> On Fri, Dec 02, 2022 at 12:36:10AM +0000, luca.boccassi@gmail.com wrote:
> > From: Luca Boccassi <bluca@debian.org>
> >
> > Usually when closing a crypto device (eg: dm-crypt with LUKS) the
> > volume key is not required, as it requires root privileges anyway, and
> > root can deny access to a disk in many ways regardless. Requiring the
> > volume key to lock the device is a peculiarity of the OPAL
> > specification.
> >
> > Given we might already have saved the key if the user requested it via
> > the 'IOC_OPAL_SAVE' ioctl, we can use that key to lock the device if no
> > key was provided here and the locking range matches. This allows
> > integrating OPAL with tools and libraries that are used to the common
> > behaviour and do not ask for the volume key when closing a device.
> >
> > If the caller provides a key on the other hand it will still be used as
> > before, no changes in that case.
> >
> > Suggested-by: Štěpán Horáček <stepan.horacek@gmail.com>
> > Signed-off-by: Luca Boccassi <bluca@debian.org>
> > ---
>
> But it would be quite the change in behavior for existing users, no?
>
> It might be better to add an ioctl that would allow to set an option on
> the opal device after it was opened which marks it as closable without
> providing the key?

Closing with a zero-length key could not work before and would fail
with eperm, so I can't imagine anyone using it as such, so I didn't
bother.
But I don't mind either way, so I will add a new ioctl for v2.

Kind regards,
Luca Boccassi

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] sed-opal: if key is available from IOC_OPAL_SAVE use it when locking
  2022-12-02 10:28   ` Luca Boccassi
@ 2022-12-02 10:37     ` Christian Brauner
  0 siblings, 0 replies; 14+ messages in thread
From: Christian Brauner @ 2022-12-02 10:37 UTC (permalink / raw)
  To: Luca Boccassi
  Cc: linux-block, jonathan.derrick, gmazyland, axboe, stepan.horacek

On Fri, Dec 02, 2022 at 10:28:10AM +0000, Luca Boccassi wrote:
> On Fri, 2 Dec 2022 at 08:48, Christian Brauner <brauner@kernel.org> wrote:
> >
> > On Fri, Dec 02, 2022 at 12:36:10AM +0000, luca.boccassi@gmail.com wrote:
> > > From: Luca Boccassi <bluca@debian.org>
> > >
> > > Usually when closing a crypto device (eg: dm-crypt with LUKS) the
> > > volume key is not required, as it requires root privileges anyway, and
> > > root can deny access to a disk in many ways regardless. Requiring the
> > > volume key to lock the device is a peculiarity of the OPAL
> > > specification.
> > >
> > > Given we might already have saved the key if the user requested it via
> > > the 'IOC_OPAL_SAVE' ioctl, we can use that key to lock the device if no
> > > key was provided here and the locking range matches. This allows
> > > integrating OPAL with tools and libraries that are used to the common
> > > behaviour and do not ask for the volume key when closing a device.
> > >
> > > If the caller provides a key on the other hand it will still be used as
> > > before, no changes in that case.
> > >
> > > Suggested-by: Štěpán Horáček <stepan.horacek@gmail.com>
> > > Signed-off-by: Luca Boccassi <bluca@debian.org>
> > > ---
> >
> > But it would be quite the change in behavior for existing users, no?
> >
> > It might be better to add an ioctl that would allow to set an option on
> > the opal device after it was opened which marks it as closable without
> > providing the key?
> 
> Closing with a zero-length key could not work before and would fail
> with eperm, so I can't imagine anyone using it as such, so I didn't
> bother.

That's not the point though. Afaict, with your change users that rely on
the device only being closable by users that have access to the key
couldn't rely on his anymore. At least that's what the change
description seems to imply.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2] sed-opal: allow using IOC_OPAL_SAVE for locking too
  2022-12-02  0:36 [PATCH] sed-opal: if key is available from IOC_OPAL_SAVE use it when locking luca.boccassi
  2022-12-02  8:48 ` Christian Brauner
@ 2022-12-03  0:12 ` luca.boccassi
  2022-12-05  7:09   ` Christoph Hellwig
  2022-12-06  0:03 ` [PATCH v3] " luca.boccassi
  2022-12-06  9:29 ` [PATCH v4] " luca.boccassi
  3 siblings, 1 reply; 14+ messages in thread
From: luca.boccassi @ 2022-12-03  0:12 UTC (permalink / raw)
  To: linux-block; +Cc: jonathan.derrick, gmazyland, axboe, brauner, stepan.horacek

From: Luca Boccassi <bluca@debian.org>

Usually when closing a crypto device (eg: dm-crypt with LUKS) the
volume key is not required, as it requires root privileges anyway, and
root can deny access to a disk in many ways regardless. Requiring the
volume key to lock the device is a peculiarity of the OPAL
specification.

Given we might already have saved the key if the user requested it via
the 'IOC_OPAL_SAVE' ioctl, we can use that key to lock the device if no
key was provided here and the locking range matches, and the user sets
the appropriate flag with 'IOC_OPAL_SAVE'. This allows integrating OPAL
with tools and libraries that are used to the common behaviour and do
not ask for the volume key when closing a device.

Callers can always pass a non-zero key and it will be used regardless,
as before.

Suggested-by: Štěpán Horáček <stepan.horacek@gmail.com>
Signed-off-by: Luca Boccassi <bluca@debian.org>
---
v2: break on 80chr for optimal rendering on 1970s green monochrome monitors
    make the new functionality dependent on a new flag that has to be
    passed to IOC_OPAL_SAVE, using reserved bits in its ioctl struct

 block/sed-opal.c              | 32 ++++++++++++++++++++++++++++++++
 include/uapi/linux/sed-opal.h |  3 ++-
 2 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/block/sed-opal.c b/block/sed-opal.c
index 9bdb833e5817..3a81754a0fdf 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -2470,6 +2470,38 @@ static int opal_lock_unlock(struct opal_dev *dev,
 		return -EINVAL;
 
 	mutex_lock(&dev->dev_lock);
+
+	/*
+	 * Usually when closing a crypto device (eg: dm-crypt with LUKS) the volume
+	 * key is not required, as it requires root privileges anyway, and root can
+	 * deny access to a disk in many ways regardless. Requiring the volume key
+	 * to lock the device is a peculiarity of the OPAL specification.
+	 * Given we might already have saved the key if the user requested it via
+	 * the 'IOC_OPAL_SAVE' ioctl, we can use that key to lock the device if no
+	 * key was provided here, the locking range matches and the appropriate
+	 * flag was passed with 'IOC_OPAL_SAVE'. This allows integrating OPAL with
+	 * tools and libraries that are used to the common behaviour and do not ask
+	 * for the volume key when closing a device.
+	 */
+	if (lk_unlk->l_state == OPAL_LK &&
+			lk_unlk->session.opal_key.key_len == 0) {
+		struct opal_suspend_data *iter;
+
+		setup_opal_dev(dev);
+		list_for_each_entry(iter, &dev->unlk_lst, node) {
+			if (iter->unlk.save_for_lock &&
+					iter->lr == lk_unlk->session.opal_key.lr &&
+					iter->unlk.session.opal_key.key_len > 0) {
+				lk_unlk->session.opal_key.key_len =
+					iter->unlk.session.opal_key.key_len;
+				memcpy(lk_unlk->session.opal_key.key,
+					iter->unlk.session.opal_key.key,
+					iter->unlk.session.opal_key.key_len);
+				break;
+			}
+		}
+	}
+
 	ret = __opal_lock_unlock(dev, lk_unlk);
 	mutex_unlock(&dev->dev_lock);
 
diff --git a/include/uapi/linux/sed-opal.h b/include/uapi/linux/sed-opal.h
index 2573772e2fb3..fa604fb07f50 100644
--- a/include/uapi/linux/sed-opal.h
+++ b/include/uapi/linux/sed-opal.h
@@ -76,7 +76,8 @@ struct opal_user_lr_setup {
 struct opal_lock_unlock {
 	struct opal_session_info session;
 	__u32 l_state;
-	__u8 __align[4];
+	__u8 save_for_lock:1; /* if in IOC_OPAL_SAVE will also use key to lock */
+	__u8 __align[3];
 };
 
 struct opal_new_pw {
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] sed-opal: allow using IOC_OPAL_SAVE for locking too
  2022-12-03  0:12 ` [PATCH v2] sed-opal: allow using IOC_OPAL_SAVE for locking too luca.boccassi
@ 2022-12-05  7:09   ` Christoph Hellwig
  0 siblings, 0 replies; 14+ messages in thread
From: Christoph Hellwig @ 2022-12-05  7:09 UTC (permalink / raw)
  To: luca.boccassi
  Cc: linux-block, jonathan.derrick, gmazyland, axboe, brauner, stepan.horacek

On Sat, Dec 03, 2022 at 12:12:43AM +0000, luca.boccassi@gmail.com wrote:
> +	 * Usually when closing a crypto device (eg: dm-crypt with LUKS) the volume

As said last time, please correctly format your block comments so that
they don't spill out of 80 characters for every single line and become
completely unreadable.

> +	if (lk_unlk->l_state == OPAL_LK &&
> +			lk_unlk->session.opal_key.key_len == 0) {
> +		struct opal_suspend_data *iter;
> +
> +		setup_opal_dev(dev);
> +		list_for_each_entry(iter, &dev->unlk_lst, node) {
> +			if (iter->unlk.save_for_lock &&
> +					iter->lr == lk_unlk->session.opal_key.lr &&
> +					iter->unlk.session.opal_key.key_len > 0) {
> +				lk_unlk->session.opal_key.key_len =
> +					iter->unlk.session.opal_key.key_len;
> +				memcpy(lk_unlk->session.opal_key.key,
> +					iter->unlk.session.opal_key.key,
> +					iter->unlk.session.opal_key.key_len);
> +				break;
> +			}
> +		}

And please split this logic into a helper.

>  	__u32 l_state;
> -	__u8 __align[4];
> +	__u8 save_for_lock:1; /* if in IOC_OPAL_SAVE will also use key to lock */

Please never use bitfields in ABIs, the psABI rules for them are just
a mess to start with, and not filling all of them leads to leaks of
stack contents as well.  Just turn the entire 4 bytes into a flags
field and then just bitmasks.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3] sed-opal: allow using IOC_OPAL_SAVE for locking too
  2022-12-02  0:36 [PATCH] sed-opal: if key is available from IOC_OPAL_SAVE use it when locking luca.boccassi
  2022-12-02  8:48 ` Christian Brauner
  2022-12-03  0:12 ` [PATCH v2] sed-opal: allow using IOC_OPAL_SAVE for locking too luca.boccassi
@ 2022-12-06  0:03 ` luca.boccassi
  2022-12-06  8:30   ` Christoph Hellwig
  2022-12-06  9:23   ` Christian Brauner
  2022-12-06  9:29 ` [PATCH v4] " luca.boccassi
  3 siblings, 2 replies; 14+ messages in thread
From: luca.boccassi @ 2022-12-06  0:03 UTC (permalink / raw)
  To: linux-block
  Cc: jonathan.derrick, gmazyland, axboe, brauner, stepan.horacek, hch

From: Luca Boccassi <bluca@debian.org>

Usually when closing a crypto device (eg: dm-crypt with LUKS) the
volume key is not required, as it requires root privileges anyway, and
root can deny access to a disk in many ways regardless. Requiring the
volume key to lock the device is a peculiarity of the OPAL
specification.

Given we might already have saved the key if the user requested it via
the 'IOC_OPAL_SAVE' ioctl, we can use that key to lock the device if no
key was provided here and the locking range matches, and the user sets
the appropriate flag with 'IOC_OPAL_SAVE'. This allows integrating OPAL
with tools and libraries that are used to the common behaviour and do
not ask for the volume key when closing a device.

Callers can always pass a non-zero key and it will be used regardless,
as before.

Suggested-by: Štěpán Horáček <stepan.horacek@gmail.com>
Signed-off-by: Luca Boccassi <bluca@debian.org>
---
v3: split out static helper, switch from bitfield to flags field (leaving
    two bytes for future use, u16 should be enough for flags for this ioctl)

 block/sed-opal.c              | 39 +++++++++++++++++++++++++++++++++++
 include/uapi/linux/sed-opal.h |  8 ++++++-
 2 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/block/sed-opal.c b/block/sed-opal.c
index 9bdb833e5817..463873f61e01 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -2461,6 +2461,44 @@ static int __opal_set_mbr_done(struct opal_dev *dev, struct opal_key *key)
 	return execute_steps(dev, mbrdone_step, ARRAY_SIZE(mbrdone_step));
 }
 
+static void opal_lock_check_for_saved_key(struct opal_dev *dev,
+			    struct opal_lock_unlock *lk_unlk)
+{
+	struct opal_suspend_data *iter;
+
+	if (lk_unlk->l_state != OPAL_LK ||
+			lk_unlk->session.opal_key.key_len > 0)
+		return;
+
+	/*
+	 * Usually when closing a crypto device (eg: dm-crypt with LUKS) the
+	 * volume key is not required, as it requires root privileges anyway,
+	 * and root can deny access to a disk in many ways regardless.
+	 * Requiring the volume key to lock the device is a peculiarity of the
+	 * OPAL specification. Given we might already have saved the key if
+	 * the user requested it via the 'IOC_OPAL_SAVE' ioctl, we can use
+	 * that key to lock the device if no key was provided here, the
+	 * locking range matches and the appropriate flag was passed with
+	 * 'IOC_OPAL_SAVE'.
+	 * This allows integrating OPAL with tools and libraries that are used
+	 * to the common behaviour and do not ask for the volume key when
+	 * closing a device.
+	 */
+	setup_opal_dev(dev);
+	list_for_each_entry(iter, &dev->unlk_lst, node) {
+		if ((iter->unlk.flags & OPAL_SAVE_FOR_LOCK) &&
+				iter->lr == lk_unlk->session.opal_key.lr &&
+				iter->unlk.session.opal_key.key_len > 0) {
+			lk_unlk->session.opal_key.key_len =
+				iter->unlk.session.opal_key.key_len;
+			memcpy(lk_unlk->session.opal_key.key,
+				iter->unlk.session.opal_key.key,
+				iter->unlk.session.opal_key.key_len);
+			break;
+		}
+	}
+}
+
 static int opal_lock_unlock(struct opal_dev *dev,
 			    struct opal_lock_unlock *lk_unlk)
 {
@@ -2470,6 +2508,7 @@ static int opal_lock_unlock(struct opal_dev *dev,
 		return -EINVAL;
 
 	mutex_lock(&dev->dev_lock);
+	opal_lock_check_for_saved_key(dev, lk_unlk);
 	ret = __opal_lock_unlock(dev, lk_unlk);
 	mutex_unlock(&dev->dev_lock);
 
diff --git a/include/uapi/linux/sed-opal.h b/include/uapi/linux/sed-opal.h
index 2573772e2fb3..1fed3c9294fc 100644
--- a/include/uapi/linux/sed-opal.h
+++ b/include/uapi/linux/sed-opal.h
@@ -44,6 +44,11 @@ enum opal_lock_state {
 	OPAL_LK = 0x04, /* 0100 */
 };
 
+enum opal_lock_flags {
+	/* IOC_OPAL_SAVE will also store the provided key for locking */
+	OPAL_SAVE_FOR_LOCK = 0x01,
+};
+
 struct opal_key {
 	__u8 lr;
 	__u8 key_len;
@@ -76,7 +81,8 @@ struct opal_user_lr_setup {
 struct opal_lock_unlock {
 	struct opal_session_info session;
 	__u32 l_state;
-	__u8 __align[4];
+	__u16 flags;
+	__u8 __align[2];
 };
 
 struct opal_new_pw {
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v3] sed-opal: allow using IOC_OPAL_SAVE for locking too
  2022-12-06  0:03 ` [PATCH v3] " luca.boccassi
@ 2022-12-06  8:30   ` Christoph Hellwig
  2022-12-06  9:23   ` Christian Brauner
  1 sibling, 0 replies; 14+ messages in thread
From: Christoph Hellwig @ 2022-12-06  8:30 UTC (permalink / raw)
  To: luca.boccassi
  Cc: linux-block, jonathan.derrick, gmazyland, axboe, brauner,
	stepan.horacek, hch

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3] sed-opal: allow using IOC_OPAL_SAVE for locking too
  2022-12-06  0:03 ` [PATCH v3] " luca.boccassi
  2022-12-06  8:30   ` Christoph Hellwig
@ 2022-12-06  9:23   ` Christian Brauner
  1 sibling, 0 replies; 14+ messages in thread
From: Christian Brauner @ 2022-12-06  9:23 UTC (permalink / raw)
  To: luca.boccassi
  Cc: linux-block, jonathan.derrick, gmazyland, axboe, stepan.horacek, hch

On Tue, Dec 06, 2022 at 12:03:46AM +0000, luca.boccassi@gmail.com wrote:
> From: Luca Boccassi <bluca@debian.org>
> 
> Usually when closing a crypto device (eg: dm-crypt with LUKS) the
> volume key is not required, as it requires root privileges anyway, and
> root can deny access to a disk in many ways regardless. Requiring the
> volume key to lock the device is a peculiarity of the OPAL
> specification.
> 
> Given we might already have saved the key if the user requested it via
> the 'IOC_OPAL_SAVE' ioctl, we can use that key to lock the device if no
> key was provided here and the locking range matches, and the user sets
> the appropriate flag with 'IOC_OPAL_SAVE'. This allows integrating OPAL
> with tools and libraries that are used to the common behaviour and do
> not ask for the volume key when closing a device.
> 
> Callers can always pass a non-zero key and it will be used regardless,
> as before.
> 
> Suggested-by: Štěpán Horáček <stepan.horacek@gmail.com>
> Signed-off-by: Luca Boccassi <bluca@debian.org>
> ---

Looks good to me,
Reviewed-by: Christian Brauner <brauner@kernel.org>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v4] sed-opal: allow using IOC_OPAL_SAVE for locking too
  2022-12-02  0:36 [PATCH] sed-opal: if key is available from IOC_OPAL_SAVE use it when locking luca.boccassi
                   ` (2 preceding siblings ...)
  2022-12-06  0:03 ` [PATCH v3] " luca.boccassi
@ 2022-12-06  9:29 ` luca.boccassi
  2022-12-08 16:18   ` Jens Axboe
  2022-12-08 16:20   ` Jens Axboe
  3 siblings, 2 replies; 14+ messages in thread
From: luca.boccassi @ 2022-12-06  9:29 UTC (permalink / raw)
  To: linux-block; +Cc: jonathan.derrick, axboe

From: Luca Boccassi <bluca@debian.org>

Usually when closing a crypto device (eg: dm-crypt with LUKS) the
volume key is not required, as it requires root privileges anyway, and
root can deny access to a disk in many ways regardless. Requiring the
volume key to lock the device is a peculiarity of the OPAL
specification.

Given we might already have saved the key if the user requested it via
the 'IOC_OPAL_SAVE' ioctl, we can use that key to lock the device if no
key was provided here and the locking range matches, and the user sets
the appropriate flag with 'IOC_OPAL_SAVE'. This allows integrating OPAL
with tools and libraries that are used to the common behaviour and do
not ask for the volume key when closing a device.

Callers can always pass a non-zero key and it will be used regardless,
as before.

Suggested-by: Štěpán Horáček <stepan.horacek@gmail.com>
Signed-off-by: Luca Boccassi <bluca@debian.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Christian Brauner <brauner@kernel.org>
---
v4: add reviewed-by tags, no other changes

 block/sed-opal.c              | 39 +++++++++++++++++++++++++++++++++++
 include/uapi/linux/sed-opal.h |  8 ++++++-
 2 files changed, 46 insertions(+), 1 deletion(-)

diff --git a/block/sed-opal.c b/block/sed-opal.c
index 9bdb833e5817..463873f61e01 100644
--- a/block/sed-opal.c
+++ b/block/sed-opal.c
@@ -2461,6 +2461,44 @@ static int __opal_set_mbr_done(struct opal_dev *dev, struct opal_key *key)
 	return execute_steps(dev, mbrdone_step, ARRAY_SIZE(mbrdone_step));
 }
 
+static void opal_lock_check_for_saved_key(struct opal_dev *dev,
+			    struct opal_lock_unlock *lk_unlk)
+{
+	struct opal_suspend_data *iter;
+
+	if (lk_unlk->l_state != OPAL_LK ||
+			lk_unlk->session.opal_key.key_len > 0)
+		return;
+
+	/*
+	 * Usually when closing a crypto device (eg: dm-crypt with LUKS) the
+	 * volume key is not required, as it requires root privileges anyway,
+	 * and root can deny access to a disk in many ways regardless.
+	 * Requiring the volume key to lock the device is a peculiarity of the
+	 * OPAL specification. Given we might already have saved the key if
+	 * the user requested it via the 'IOC_OPAL_SAVE' ioctl, we can use
+	 * that key to lock the device if no key was provided here, the
+	 * locking range matches and the appropriate flag was passed with
+	 * 'IOC_OPAL_SAVE'.
+	 * This allows integrating OPAL with tools and libraries that are used
+	 * to the common behaviour and do not ask for the volume key when
+	 * closing a device.
+	 */
+	setup_opal_dev(dev);
+	list_for_each_entry(iter, &dev->unlk_lst, node) {
+		if ((iter->unlk.flags & OPAL_SAVE_FOR_LOCK) &&
+				iter->lr == lk_unlk->session.opal_key.lr &&
+				iter->unlk.session.opal_key.key_len > 0) {
+			lk_unlk->session.opal_key.key_len =
+				iter->unlk.session.opal_key.key_len;
+			memcpy(lk_unlk->session.opal_key.key,
+				iter->unlk.session.opal_key.key,
+				iter->unlk.session.opal_key.key_len);
+			break;
+		}
+	}
+}
+
 static int opal_lock_unlock(struct opal_dev *dev,
 			    struct opal_lock_unlock *lk_unlk)
 {
@@ -2470,6 +2508,7 @@ static int opal_lock_unlock(struct opal_dev *dev,
 		return -EINVAL;
 
 	mutex_lock(&dev->dev_lock);
+	opal_lock_check_for_saved_key(dev, lk_unlk);
 	ret = __opal_lock_unlock(dev, lk_unlk);
 	mutex_unlock(&dev->dev_lock);
 
diff --git a/include/uapi/linux/sed-opal.h b/include/uapi/linux/sed-opal.h
index 2573772e2fb3..1fed3c9294fc 100644
--- a/include/uapi/linux/sed-opal.h
+++ b/include/uapi/linux/sed-opal.h
@@ -44,6 +44,11 @@ enum opal_lock_state {
 	OPAL_LK = 0x04, /* 0100 */
 };
 
+enum opal_lock_flags {
+	/* IOC_OPAL_SAVE will also store the provided key for locking */
+	OPAL_SAVE_FOR_LOCK = 0x01,
+};
+
 struct opal_key {
 	__u8 lr;
 	__u8 key_len;
@@ -76,7 +81,8 @@ struct opal_user_lr_setup {
 struct opal_lock_unlock {
 	struct opal_session_info session;
 	__u32 l_state;
-	__u8 __align[4];
+	__u16 flags;
+	__u8 __align[2];
 };
 
 struct opal_new_pw {
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v4] sed-opal: allow using IOC_OPAL_SAVE for locking too
  2022-12-06  9:29 ` [PATCH v4] " luca.boccassi
@ 2022-12-08 16:18   ` Jens Axboe
  2022-12-08 16:19     ` Luca Boccassi
  2022-12-08 16:20   ` Jens Axboe
  1 sibling, 1 reply; 14+ messages in thread
From: Jens Axboe @ 2022-12-08 16:18 UTC (permalink / raw)
  To: luca.boccassi, linux-block; +Cc: jonathan.derrick

On 12/6/22 2:29 AM, luca.boccassi@gmail.com wrote:
> v4: add reviewed-by tags, no other changes

Never resend just for that, normal patch tooling picks that up
automatically hence it's just pointless noise.

-- 
Jens Axboe



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4] sed-opal: allow using IOC_OPAL_SAVE for locking too
  2022-12-08 16:18   ` Jens Axboe
@ 2022-12-08 16:19     ` Luca Boccassi
  0 siblings, 0 replies; 14+ messages in thread
From: Luca Boccassi @ 2022-12-08 16:19 UTC (permalink / raw)
  To: Jens Axboe; +Cc: linux-block, jonathan.derrick

On Thu, 8 Dec 2022 at 16:18, Jens Axboe <axboe@kernel.dk> wrote:
>
> On 12/6/22 2:29 AM, luca.boccassi@gmail.com wrote:
> > v4: add reviewed-by tags, no other changes
>
> Never resend just for that, normal patch tooling picks that up
> automatically hence it's just pointless noise.

Sorry, didn't know about that.

Kind Regards,
Luca Boccassi

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v4] sed-opal: allow using IOC_OPAL_SAVE for locking too
  2022-12-06  9:29 ` [PATCH v4] " luca.boccassi
  2022-12-08 16:18   ` Jens Axboe
@ 2022-12-08 16:20   ` Jens Axboe
  1 sibling, 0 replies; 14+ messages in thread
From: Jens Axboe @ 2022-12-08 16:20 UTC (permalink / raw)
  To: linux-block, luca.boccassi; +Cc: jonathan.derrick


On Tue, 06 Dec 2022 09:29:13 +0000, luca.boccassi@gmail.com wrote:
> Usually when closing a crypto device (eg: dm-crypt with LUKS) the
> volume key is not required, as it requires root privileges anyway, and
> root can deny access to a disk in many ways regardless. Requiring the
> volume key to lock the device is a peculiarity of the OPAL
> specification.
> 
> Given we might already have saved the key if the user requested it via
> the 'IOC_OPAL_SAVE' ioctl, we can use that key to lock the device if no
> key was provided here and the locking range matches, and the user sets
> the appropriate flag with 'IOC_OPAL_SAVE'. This allows integrating OPAL
> with tools and libraries that are used to the common behaviour and do
> not ask for the volume key when closing a device.
> 
> [...]

Applied, thanks!

[1/1] sed-opal: allow using IOC_OPAL_SAVE for locking too
      commit: c1f480b2d092960ecf8bb0bd1f27982c33ada42a

Best regards,
-- 
Jens Axboe



^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2022-12-08 16:20 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-02  0:36 [PATCH] sed-opal: if key is available from IOC_OPAL_SAVE use it when locking luca.boccassi
2022-12-02  8:48 ` Christian Brauner
2022-12-02  9:11   ` Christoph Hellwig
2022-12-02 10:28   ` Luca Boccassi
2022-12-02 10:37     ` Christian Brauner
2022-12-03  0:12 ` [PATCH v2] sed-opal: allow using IOC_OPAL_SAVE for locking too luca.boccassi
2022-12-05  7:09   ` Christoph Hellwig
2022-12-06  0:03 ` [PATCH v3] " luca.boccassi
2022-12-06  8:30   ` Christoph Hellwig
2022-12-06  9:23   ` Christian Brauner
2022-12-06  9:29 ` [PATCH v4] " luca.boccassi
2022-12-08 16:18   ` Jens Axboe
2022-12-08 16:19     ` Luca Boccassi
2022-12-08 16:20   ` Jens Axboe

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.