| Message ID | 20221202003610.100024-1-luca.boccassi@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | sed-opal: if key is available from IOC_OPAL_SAVE use it when locking | expand |
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?
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.
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
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.
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);