From 77ace98fc1ada47a1a41921818b6cfcfb72d7636 Mon Sep 17 00:00:00 2001 From: Marius Vollmer Date: Mon, 25 Mar 2024 09:45:44 +0200 Subject: [PATCH] storage: Improve handling of LUKS backed btrfs Single-device btrfs volumes now behave like other filesystems: Unmounting all subvolumes will automatically lock the LUKS device, and mounting the first subvolume will automatically unlock it. Options in /etc/crypttab are now maintained correctly for btrfs volumes with multiple subvolumes. For example, the LUKS device is only "noauto" when all of the subvolumes are also "noauto", and it is readonly exactly when all of the subvolumes are readonly. Cockpit can't unfortunately always know whether a locked LUKS device is part of a single-device or multi-device btrfs volume. (Because UDisks2 only tracks one parent of a fstab entry, we would need to fix that.) We assume that they are single-device volumes (because they are more common, probably) and let the user mount their subvolumes directly. If the assumption turns out to have been wrong, the mount operation is cleanly aborted. --- pkg/storaged/block/create-pages.jsx | 27 ++- pkg/storaged/btrfs/filesystem.jsx | 27 ++- pkg/storaged/btrfs/subvolume.jsx | 180 ++++++++-------- pkg/storaged/btrfs/utils.jsx | 40 ++++ pkg/storaged/filesystem/mismounting.jsx | 2 +- pkg/storaged/filesystem/mounting-dialog.jsx | 153 +++++++++----- pkg/storaged/filesystem/utils.jsx | 8 +- pkg/storaged/utils.js | 43 +++- test/verify/check-storage-anaconda | 11 +- test/verify/check-storage-btrfs | 214 ++++++++++++++++++-- 10 files changed, 523 insertions(+), 182 deletions(-) diff --git a/pkg/storaged/block/create-pages.jsx b/pkg/storaged/block/create-pages.jsx index 3054c5ef01a..8cd51fa93ca 100644 --- a/pkg/storaged/block/create-pages.jsx +++ b/pkg/storaged/block/create-pages.jsx @@ -35,7 +35,8 @@ import { make_swap_card } from "../swap/swap.jsx"; import { make_encryption_card } from "../crypto/encryption.jsx"; import { make_btrfs_device_card } from "../btrfs/device.jsx"; import { make_btrfs_filesystem_card } from "../btrfs/filesystem.jsx"; -import { make_btrfs_subvolume_pages } from "../btrfs/subvolume.jsx"; +import { make_btrfs_subvolume_pages, make_btrfs_subvolume_pages_from_child_config } from "../btrfs/subvolume.jsx"; +import { find_btrfs_volume } from "../btrfs/utils.jsx"; import { new_page } from "../pages.jsx"; @@ -58,8 +59,8 @@ export function make_block_page(parent, block, card) { const is_btrfs = (fstab_config.length > 0 && (fstab_config[2].indexOf("subvol=") >= 0 || fstab_config[2].indexOf("subvolid=") >= 0)); - const block_btrfs_blockdev = content_block && client.blocks_fsys_btrfs[content_block.path]; - const single_device_volume = block_btrfs_blockdev && block_btrfs_blockdev.data.num_devices === 1; + const btrfs_vol = find_btrfs_volume(block); + const single_device_volume = !btrfs_vol || btrfs_vol.data.num_devices <= 1; if (client.blocks_ptable[block.path]) { make_partition_table_page(parent, block, card); @@ -85,8 +86,14 @@ export function make_block_page(parent, block, card) { // can not happen unless there is a bug in the code above. console.error("Assertion failure: is_crypto == false"); } - if (fstab_config.length > 0 && !is_btrfs) { - card = make_filesystem_card(card, block, null, fstab_config); + if (fstab_config.length > 0) { + if (is_btrfs) { + if (single_device_volume) + card = make_btrfs_filesystem_card(card, block, null); + else + card = make_locked_encrypted_data_card(card, block); + } else + card = make_filesystem_card(card, block, null, fstab_config); } else { card = make_locked_encrypted_data_card(card, block); } @@ -95,11 +102,11 @@ export function make_block_page(parent, block, card) { const block_pvol = client.blocks_pvol[content_block.path]; const block_swap = client.blocks_swap[content_block.path]; - if (block_btrfs_blockdev) { + if (btrfs_vol) { if (single_device_volume) card = make_btrfs_filesystem_card(card, block, content_block); else - card = make_btrfs_device_card(card, block, content_block, block_btrfs_blockdev); + card = make_btrfs_device_card(card, block, content_block, btrfs_vol); } else if (is_filesystem) { card = make_filesystem_card(card, block, content_block, fstab_config); } else if ((content_block.IdUsage == "raid" && content_block.IdType == "LVM2_member") || @@ -122,8 +129,10 @@ export function make_block_page(parent, block, card) { if (card) { const page = new_page(parent, card); - if (block_btrfs_blockdev && single_device_volume) - make_btrfs_subvolume_pages(page, block_btrfs_blockdev); + if (btrfs_vol && single_device_volume) + make_btrfs_subvolume_pages(page, btrfs_vol); + else if (!content_block && is_btrfs && single_device_volume) + make_btrfs_subvolume_pages_from_child_config(page, block); return page; } } diff --git a/pkg/storaged/btrfs/filesystem.jsx b/pkg/storaged/btrfs/filesystem.jsx index d062d7a3815..3e5844aa8bf 100644 --- a/pkg/storaged/btrfs/filesystem.jsx +++ b/pkg/storaged/btrfs/filesystem.jsx @@ -27,9 +27,9 @@ import { DescriptionList } from "@patternfly/react-core/dist/esm/components/Desc import { new_card, ChildrenTable, StorageCard, StorageDescription } from "../pages.jsx"; +import { format_dialog } from "../block/format-dialog.jsx"; import { StorageUsageBar, StorageLink } from "../storage-controls.jsx"; import { btrfs_device_usage, btrfs_is_volume_mounted } from "./utils.jsx"; -import { btrfs_device_actions } from "./device.jsx"; import { rename_dialog } from "./volume.jsx"; const _ = cockpit.gettext; @@ -43,17 +43,18 @@ export function make_btrfs_filesystem_card(next, backing_block, content_block) { return new_card({ title: _("btrfs filesystem"), next, - actions: btrfs_device_actions(backing_block, content_block), + actions: [ + { title: _("Format"), action: () => format_dialog(client, backing_block.path), danger: true }, + ], component: BtrfsFilesystemCard, props: { backing_block, content_block }, }); } const BtrfsFilesystemCard = ({ card, backing_block, content_block }) => { - const block_btrfs = client.blocks_fsys_btrfs[content_block.path]; + const block_btrfs = content_block && client.blocks_fsys_btrfs[content_block.path]; const uuid = block_btrfs && block_btrfs.data.uuid; const label = block_btrfs && block_btrfs.data.label; - const use = btrfs_device_usage(client, uuid, block_btrfs.path); // Changing the label is only supported when the device is not mounted // otherwise we will get btrfs filesystem error ERROR: device /dev/vda5 is @@ -66,18 +67,22 @@ const BtrfsFilesystemCard = ({ card, backing_block, content_block }) => { + { block_btrfs && rename_dialog(block_btrfs, label)} - excuse={is_mounted ? _("Btrfs volume is mounted") : null}> - {_("edit")} - } + value={label} + action={ + rename_dialog(block_btrfs, label)} + excuse={is_mounted ? _("Btrfs volume is mounted") : null}> + {_("edit")} + } /> + } + { content_block && + } { block_btrfs && - + } diff --git a/pkg/storaged/btrfs/subvolume.jsx b/pkg/storaged/btrfs/subvolume.jsx index b776dae9390..8387226577b 100644 --- a/pkg/storaged/btrfs/subvolume.jsx +++ b/pkg/storaged/btrfs/subvolume.jsx @@ -29,10 +29,13 @@ import { import { StorageUsageBar } from "../storage-controls.jsx"; import { encode_filename, decode_filename, - get_fstab_config_with_client, reload_systemd, extract_option, parse_options, + reload_systemd, extract_option, parse_options, flatten, teardown_active_usage, + maybe_update_crypto_options, } from "../utils.js"; -import { btrfs_usage, validate_subvolume_name, parse_subvol_from_options } from "./utils.jsx"; +import { + btrfs_usage, validate_subvolume_name, parse_subvol_from_options, is_probably_single_device_btrfs_volume +} from "./utils.jsx"; import { at_boot_input, update_at_boot_input, mounting_dialog, mount_options } from "../filesystem/mounting-dialog.jsx"; import { dialog_open, TextInput, @@ -40,20 +43,19 @@ import { } from "../dialog.jsx"; import { check_mismounted_fsys, MismountAlert } from "../filesystem/mismounting.jsx"; import { - is_mounted, is_valid_mount_point, mount_point_text, MountPoint, edit_mount_point + get_fstab_config, is_mounted, is_valid_mount_point, mount_point_text, MountPoint, edit_mount_point } from "../filesystem/utils.jsx"; + import client, { btrfs_poll } from "../client.js"; const _ = cockpit.gettext; -function subvolume_unmount(volume, subvol, forced_options) { - const block = client.blocks[volume.path]; - mounting_dialog(client, block, "unmount", forced_options, subvol); +function subvolume_unmount(block, subvol, forced_options, subvols) { + mounting_dialog(client, block, "unmount", forced_options, subvol, subvols); } -function subvolume_mount(volume, subvol, forced_options) { - const block = client.blocks[volume.path]; - mounting_dialog(client, block, "mount", forced_options, subvol); +function subvolume_mount(block, subvol, forced_options, subvols) { + mounting_dialog(client, block, "mount", forced_options, subvol, subvols); } function get_mount_point_in_parent(volume, subvol) { @@ -67,7 +69,7 @@ function get_mount_point_in_parent(volume, subvol) { (subvol.pathname.substring(0, p.pathname.length) == p.pathname && subvol.pathname[p.pathname.length] == "/"); if (has_parent_subvol && is_mounted(client, block, p)) { - const [, pmp, opts] = get_fstab_config_with_client(client, block, false, p); + const [, pmp, opts] = get_fstab_config(block, false, p); const opt_ro = extract_option(parse_options(opts), "ro"); if (!opt_ro) { if (p.pathname == "/") @@ -80,7 +82,7 @@ function get_mount_point_in_parent(volume, subvol) { return null; } -function set_mount_options(subvol, block, vals) { +async function set_mount_options(subvol, block, vals) { const mount_options = []; const mount_now = vals.variant != "nomount"; @@ -118,14 +120,12 @@ function set_mount_options(subvol, block, vals) { } ]; - return block.AddConfigurationItem(config, {}) - .then(reload_systemd) - .then(() => { - if (mount_now) { - return client.mount_at(block, mount_point); - } else - return Promise.resolve(); - }); + await block.AddConfigurationItem(config, {}); + await reload_systemd(); + if (mount_now) + await client.mount_at(block, mount_point); + if (is_probably_single_device_btrfs_volume(block)) + await maybe_update_crypto_options(client, block); } function subvolume_create(volume, subvol, parent_dir) { @@ -205,7 +205,7 @@ function subvolume_delete(volume, subvol, mount_point_in_parent, card) { const usage = []; for (const sv of all_subvols) { - const [config, mount_point] = get_fstab_config_with_client(client, block, false, sv); + const [config, mount_point] = get_fstab_config(block, false, sv); const fs_is_mounted = is_mounted(client, block, sv); usage.push({ @@ -254,94 +254,108 @@ function dirname(path) { return path.substr(0, i); } -export function make_btrfs_subvolume_pages(parent, volume) { - let subvols = client.uuids_btrfs_subvols[volume.data.uuid]; - if (!subvols) { - const block = client.blocks[volume.path]; - /* - * Try to show subvolumes based on fstab entries. We collect - * all subvolumes that are mentioned in fstab entries so that - * the user can at least mount those. - * - * The real subvolume data structure has "id" fields and - * "parent" fields that refer to the ids to form a tree. We - * want to do the same here, and we give fake ids to our fake - * subvolumes for this reason. We don't store these fake ids - * in the "id" field since we don't want them to be taken - * seriously by the rest of the code. - */ - let fake_id = 5; - subvols = [{ pathname: "/", id: 5, fake_id: fake_id++ }]; - const subvols_by_pathname = { }; - for (const config of block.Configuration) { - if (config[0] == "fstab") { - const opts = config[1].opts; - if (!opts) - continue; - - const fstab_subvol = parse_subvol_from_options(decode_filename(opts.v)); - - if (fstab_subvol && fstab_subvol.pathname && fstab_subvol.pathname !== "/") { - fstab_subvol.fake_id = fake_id++; - subvols_by_pathname[fstab_subvol.pathname] = fstab_subvol; - subvols.push(fstab_subvol); - } +function get_subvols_from_fstab(configuration) { + /* + * Try to show subvolumes based on fstab entries. We collect + * all subvolumes that are mentioned in fstab entries so that + * the user can at least mount those. + * + * The real subvolume data structure has "id" fields and + * "parent" fields that refer to the ids to form a tree. We + * want to do the same here, and we give fake ids to our fake + * subvolumes for this reason. We don't store these fake ids + * in the "id" field since we don't want them to be taken + * seriously by the rest of the code. + */ + let fake_id = 5; + const subvols = [{ pathname: "/", id: 5, fake_id: fake_id++ }]; + const subvols_by_pathname = { }; + for (const config of configuration) { + if (config[0] == "fstab") { + const opts = config[1].opts; + if (!opts) + continue; + + const fstab_subvol = parse_subvol_from_options(decode_filename(opts.v)); + + if (fstab_subvol && fstab_subvol.pathname && fstab_subvol.pathname !== "/") { + fstab_subvol.fake_id = fake_id++; + subvols_by_pathname[fstab_subvol.pathname] = fstab_subvol; + subvols.push(fstab_subvol); } } + } - // Find parents - for (const pn in subvols_by_pathname) { - let dn = pn; - while (true) { - dn = dirname(dn); - if (!dn) { - subvols_by_pathname[pn].parent = 5; - break; - } else if (subvols_by_pathname[dn]) { - subvols_by_pathname[pn].parent = subvols_by_pathname[dn].fake_id; - break; - } + // Find parents + for (const pn in subvols_by_pathname) { + let dn = pn; + while (true) { + dn = dirname(dn); + if (!dn) { + subvols_by_pathname[pn].parent = 5; + break; + } else if (subvols_by_pathname[dn]) { + subvols_by_pathname[pn].parent = subvols_by_pathname[dn].fake_id; + break; } } } + return subvols; +} + +export function make_btrfs_subvolume_pages(parent, volume) { + let subvols = client.uuids_btrfs_subvols[volume.data.uuid]; + if (!subvols) { + const block = client.blocks[volume.path]; + subvols = get_subvols_from_fstab(block.Configuration); + } + const root = subvols.find(s => s.id == 5); if (root) - make_btrfs_subvolume_page(parent, volume, root, "", subvols); + make_btrfs_subvolume_page(parent, client.blocks[volume.path], volume, root, "", subvols); } -function make_btrfs_subvolume_page(parent, volume, subvol, path_prefix, subvols) { +export function make_btrfs_subvolume_pages_from_child_config(parent, backing_block) { + const block_crypto = client.blocks_crypto[backing_block.path]; + const subvols = get_subvols_from_fstab(block_crypto.ChildConfiguration); + + const root = subvols.find(s => s.id == 5); + if (root) + make_btrfs_subvolume_page(parent, backing_block, null, root, "", subvols); +} + +function make_btrfs_subvolume_page(parent, backing_block, volume, subvol, path_prefix, subvols) { const actions = []; - const use = btrfs_usage(client, volume); - const block = client.blocks[volume.path]; - const fstab_config = get_fstab_config_with_client(client, block, false, subvol); + const content_block = volume && client.blocks[volume.path]; + const fstab_config = get_fstab_config(backing_block, true, subvol); const [, mount_point, opts] = fstab_config; const opt_ro = extract_option(parse_options(opts), "ro"); - const mismount_warning = check_mismounted_fsys(block, block, fstab_config, subvol); - const mounted = is_mounted(client, block, subvol); + const mismount_warning = check_mismounted_fsys(backing_block, content_block, fstab_config, subvol); + const mounted = content_block && is_mounted(client, content_block, subvol); const mp_text = mount_point_text(mount_point, mounted); if (mp_text == null) return null; const forced_options = [`subvol=${subvol.pathname}`]; - const mount_point_in_parent = get_mount_point_in_parent(volume, subvol); + const mount_point_in_parent = volume && get_mount_point_in_parent(volume, subvol); if (client.in_anaconda_mode()) { actions.push({ title: _("Edit mount point"), - action: () => edit_mount_point(block, forced_options, subvol), + action: () => edit_mount_point(content_block || backing_block, forced_options, subvol, subvols), }); } if (mounted) { actions.push({ title: _("Unmount"), - action: () => subvolume_unmount(volume, subvol, forced_options), + action: () => subvolume_unmount(content_block, subvol, forced_options, subvols), }); } else { actions.push({ title: _("Mount"), - action: () => subvolume_mount(volume, subvol, forced_options), + action: () => subvolume_mount(content_block || backing_block, subvol, forced_options, subvols), }); } @@ -385,37 +399,37 @@ function make_btrfs_subvolume_page(parent, volume, subvol, path_prefix, subvols) const card = new_card({ title: _("btrfs subvolume"), next: null, - page_location: ["btrfs", volume.data.uuid, subvol.pathname], + page_location: ["btrfs", volume ? volume.data.uuid : "XXX", subvol.pathname], page_name: strip_prefix(subvol.pathname, path_prefix), - page_size: mounted && , + page_size: mounted && , location: mp_text, component: BtrfsSubvolumeCard, has_warning: !!mismount_warning, - props: { subvol, mount_point, mismount_warning, block, fstab_config, forced_options }, + props: { subvol, subvols, mount_point, mismount_warning, backing_block, content_block, fstab_config, forced_options }, actions, }); const page = new_page(parent, card); for (const sv of subvols) { if (sv.parent && (sv.parent === subvol.id || sv.parent === subvol.fake_id)) { - make_btrfs_subvolume_page(page, volume, sv, subvol.pathname + "/", subvols); + make_btrfs_subvolume_page(page, backing_block, volume, sv, subvol.pathname + "/", subvols); } } } -const BtrfsSubvolumeCard = ({ card, subvol, mismount_warning, block, fstab_config, forced_options }) => { +const BtrfsSubvolumeCard = ({ card, subvol, subvols, mismount_warning, content_block, backing_block, fstab_config, forced_options }) => { return ( }> + backing_block={backing_block} content_block={content_block} subvol={subvol} />}> + backing_block={backing_block} content_block={content_block} + forced_options={forced_options} subvol={subvol} subvols={subvols} /> diff --git a/pkg/storaged/btrfs/utils.jsx b/pkg/storaged/btrfs/utils.jsx index 402ded0fef9..577b281723e 100644 --- a/pkg/storaged/btrfs/utils.jsx +++ b/pkg/storaged/btrfs/utils.jsx @@ -17,6 +17,7 @@ * along with Cockpit; If not, see . */ import cockpit from "cockpit"; +import client from "../client.js"; import { decode_filename } from "../utils.js"; @@ -88,3 +89,42 @@ export function validate_subvolume_name(name) { if (name.includes('/')) return cockpit.format(_("Name cannot contain the character '/'.")); } + +// Find the o.fd.UDisks2.Filesystem.Btrfs proxy for a block device, if +// we can. +// +// This might also work for locked encrypted devices. + +export function find_btrfs_volume(block) { + if (client.blocks_cleartext[block.path]) + block = client.blocks_cleartext[block.path]; + + const block_btrfs = client.blocks_fsys_btrfs[block.path]; + if (block_btrfs) + return block_btrfs; + + // Might be a locked LUKS device. Try to figure out the uuid or + // label from its child fstab entries. + const block_crypto = client.blocks_crypto[block.path]; + if (!block_crypto) + return null; + + for (const c of block_crypto.ChildConfiguration) { + if (c[0] == "fstab") { + const fsname = decode_filename(c[1].fsname.v); + const uuid_match = fsname.match(/^UUID=(?[A-Fa-f0-9-]+)/); + if (uuid_match) { + const btrfs = client.uuids_btrfs_volume[uuid_match.groups.uuid]; + if (btrfs) + return btrfs; + } + } + } + + return null; +} + +export function is_probably_single_device_btrfs_volume(block) { + const btrfs_vol = find_btrfs_volume(block); + return !btrfs_vol || btrfs_vol.data.num_devices <= 1; +} diff --git a/pkg/storaged/filesystem/mismounting.jsx b/pkg/storaged/filesystem/mismounting.jsx index 0fb5fb7756c..511eb4dd9a3 100644 --- a/pkg/storaged/filesystem/mismounting.jsx +++ b/pkg/storaged/filesystem/mismounting.jsx @@ -26,7 +26,7 @@ import { Alert } from "@patternfly/react-core/dist/esm/components/Alert/index.js import { encode_filename, parse_options, unparse_options, extract_option, reload_systemd, - set_crypto_auto_option, get_mount_points, + get_mount_points, set_crypto_auto_option, } from "../utils.js"; import { StorageButton } from "../storage-controls.jsx"; diff --git a/pkg/storaged/filesystem/mounting-dialog.jsx b/pkg/storaged/filesystem/mounting-dialog.jsx index bb21343a43d..89ca8c39e8a 100644 --- a/pkg/storaged/filesystem/mounting-dialog.jsx +++ b/pkg/storaged/filesystem/mounting-dialog.jsx @@ -18,9 +18,9 @@ */ import cockpit from "cockpit"; +import React from "react"; import client from "../client.js"; -import React from "react"; import { FormHelperText } from "@patternfly/react-core/dist/esm/components/Form/index.js"; import { HelperText, HelperTextItem, } from "@patternfly/react-core/dist/esm/components/HelperText/index.js"; import { ExclamationTriangleIcon, InfoCircleIcon } from "@patternfly/react-icons"; @@ -28,8 +28,10 @@ import { ExclamationTriangleIcon, InfoCircleIcon } from "@patternfly/react-icons import { encode_filename, parse_options, unparse_options, extract_option, reload_systemd, - set_crypto_options, is_mounted_synch, + is_mounted_synch, get_active_usage, teardown_active_usage, + set_crypto_auto_option, + maybe_update_crypto_options, } from "../utils.js"; import { @@ -40,6 +42,7 @@ import { } from "../dialog.jsx"; import { init_existing_passphrase, unlock_with_type } from "../crypto/keyslots.jsx"; import { initial_tab_options } from "../block/format-dialog.jsx"; +import { is_probably_single_device_btrfs_volume } from "../btrfs/utils.jsx"; import { is_mounted, get_fstab_config, @@ -48,18 +51,20 @@ import { const _ = cockpit.gettext; -export const mount_options = (opt_ro, extra_options, is_visible) => { +export const mount_options = (opt_ro, extra_options, is_visible, force_ro) => { return CheckBoxes("mount_options", _("Mount options"), { visible: vals => !client.in_anaconda_mode() && (!is_visible || is_visible(vals)), value: { - ro: opt_ro, + ro: opt_ro || force_ro, extra: extra_options || false }, fields: [ { title: _("Mount read only"), tag: "ro", + disabled: force_ro, + tooltip: force_ro && _("This subvolume can only be mounted read-only right now. To mount it read-write, unmount all other subvolumes first.") }, { title: _("Custom mount options"), tag: "extra", type: "checkboxWithInput" }, ] @@ -149,7 +154,7 @@ export function update_at_boot_input(dlg, vals, trigger) { dlg.set_options("at_boot", { explanation: mount_explanation[vals.at_boot] }); } -export function mounting_dialog(client, block, mode, forced_options, subvol) { +export function mounting_dialog(client, block, mode, forced_options, subvol, subvols) { const block_fsys = client.blocks_fsys[block.path]; const [old_config, old_dir, old_opts, old_parents] = get_fstab_config(block, true, subvol); const options = old_config ? old_opts : initial_tab_options(client, block, true); @@ -262,7 +267,33 @@ export function mounting_dialog(client, block, mode, forced_options, subvol) { try { await unlock_with_type(client, client.blocks[crypto.path], passphrase, passphrase_type, crypto_unlock_readonly); - return await client.wait_for(() => client.blocks_cleartext[crypto.path]); + // Check whether we have just opened a + // multi-device btrfs volume. If so, we need to + // give up. + // + // Ideally, Cockpit would always know when + // something is part of a multi-device btrfs + // volume, even for locked LUKS devices, and would + // never let people mount subvolumes of such a + // volume unless all devices are available. + // + // But that knowledge is based on the "x-parent" + // options in /etc/fstab, and those are + // unreliable. + // + // Thus, while we should try to not let the user + // run into this situation here as much as + // possible, we will probably not be able to rule + // it out completely. + const cleartext = await client.wait_for(() => client.blocks_cleartext[crypto.path]); + if (cleartext.IdType == "btrfs" && client.features.btrfs) { + const btrfs = await client.wait_for(() => client.blocks_fsys_btrfs[cleartext.path]); + if (btrfs.data.num_devices > 1) { + await set_crypto_auto_option(block, true); + throw new Error("unexpected-multi-device-btrfs"); + } + } + return cleartext; } catch (error) { passphrase_type = null; dlg.set_values({ needs_explicit_passphrase: true }); @@ -274,17 +305,6 @@ export function mounting_dialog(client, block, mode, forced_options, subvol) { return block; } - function maybe_lock() { - if (mode == "unmount" && !subvol && !client.in_anaconda_mode()) { - const crypto_backing = client.blocks[block.CryptoBackingDevice]; - const crypto_backing_crypto = crypto_backing && client.blocks_crypto[crypto_backing.path]; - if (crypto_backing_crypto) { - return crypto_backing_crypto.Lock({}); - } else - return Promise.resolve(); - } - } - // We need to reload systemd twice: Once at the beginning so // that it is up to date with whatever is currently in fstab, // and once at the end to make it see our changes. Otherwise @@ -306,7 +326,6 @@ export function mounting_dialog(client, block, mode, forced_options, subvol) { else if (new_config && !is_mounted(client, block)) return maybe_mount(); }) - .then(maybe_lock) .then(reload_systemd)); } @@ -320,6 +339,28 @@ export function mounting_dialog(client, block, mode, forced_options, subvol) { else at_boot = "local"; + let need_rw_backing = false; + let backing_is_busy = false; + if (subvol) { + for (const sv of subvols) { + if (sv.pathname != subvol.pathname) { + const [, , opts] = get_fstab_config(block, false, sv); + if (opts) { + const opt_ro = extract_option(parse_options(opts), "ro"); + if (!opt_ro) + need_rw_backing = true; + } + if (is_mounted(client, block, sv)) + backing_is_busy = true; + } + } + } + + // If LUKS is open as read-only and is kept busy by other mounts, + // we can't change it to read-write. + + const force_ro = client.blocks[block.CryptoBackingDevice] && block.ReadOnly && backing_is_busy; + let fields = null; if (mode == "mount" || mode == "update") { fields = [ @@ -333,7 +374,7 @@ export function mounting_dialog(client, block, mode, forced_options, subvol) { mode == "update", subvol) }), - mount_options(opt_ro, extra_options, null), + mount_options(opt_ro, extra_options, null, force_ro), at_boot_input(at_boot), ]; @@ -358,7 +399,17 @@ export function mounting_dialog(client, block, mode, forced_options, subvol) { update: _("Save") }; - function do_unmount() { + function maybe_lock() { + const block_fsys = client.blocks_fsys[block.path]; + const crypto_backing = client.blocks[block.CryptoBackingDevice]; + const crypto_backing_crypto = crypto_backing && client.blocks_crypto[crypto_backing.path]; + if (crypto_backing_crypto && block_fsys && block_fsys.MountPoints.length == 0 && !client.in_anaconda_mode()) { + return crypto_backing_crypto.Lock({}); + } else + return Promise.resolve(); + } + + async function do_unmount() { let opts = []; opts.push("noauto"); if (opt_ro) @@ -373,29 +424,32 @@ export function mounting_dialog(client, block, mode, forced_options, subvol) { opts = opts.concat(forced_options); if (extra_options) opts = opts.concat(extra_options); - return (maybe_set_crypto_options(null, false, null, null) - .then(() => maybe_update_config(old_dir, unparse_options(opts)))); + + await maybe_update_config(old_dir, unparse_options(opts)); + if (is_probably_single_device_btrfs_volume(block)) { + await maybe_update_crypto_options(client, block); + await maybe_lock(); + } } let passphrase_type; - function maybe_set_crypto_options(readonly, auto, nofail, netdev) { - if (client.blocks_crypto[block.path]) { - return set_crypto_options(block, readonly, auto, nofail, netdev); - } else if (client.blocks_crypto[block.CryptoBackingDevice]) { - return set_crypto_options(client.blocks[block.CryptoBackingDevice], readonly, auto, nofail, netdev); - } else - return Promise.resolve(); - } - const usage = get_active_usage(client, block.path, null, null, false, subvol); + function desired_crypto_readonly(vals_ro) { + // If LUKS is busy, we can't Lock and Unlock it, so don't try + // to make it read-only, even if that would be the correct + // thing to do. + if (client.blocks[block.CryptoBackingDevice] && !block.ReadOnly && backing_is_busy) + return false; + return !need_rw_backing && vals_ro; + } + function update_explicit_passphrase(vals_ro) { const backing = client.blocks[block.CryptoBackingDevice]; let need_passphrase = (block.IdUsage == "crypto" && mode == "mount"); if (backing) { - // XXX - take subvols into account. - if (block.ReadOnly != vals_ro) + if (block.ReadOnly != desired_crypto_readonly(vals_ro)) need_passphrase = true; } dlg.set_values({ needs_explicit_passphrase: need_passphrase && !passphrase_type }); @@ -413,9 +467,9 @@ export function mounting_dialog(client, block, mode, forced_options, subvol) { Action: { Title: mode_action[mode], disable_on_error: usage.Teardown, - action: function (vals) { + action: async function (vals) { if (mode == "unmount") { - return do_unmount(); + await do_unmount(); } else if (mode == "mount" || mode == "update") { let opts = []; if ((mode == "update" && !is_filesystem_mounted) || vals.at_boot == "never") @@ -432,17 +486,24 @@ export function mounting_dialog(client, block, mode, forced_options, subvol) { opts = opts.concat(forced_options); if (vals.mount_options?.extra) opts = opts.concat(parse_options(vals.mount_options.extra)); - // XXX - take subvols into account. - const crypto_unlock_readonly = vals.mount_options?.ro ?? opt_ro; - return (maybe_update_config(client.add_mount_point_prefix(vals.mount_point), - unparse_options(opts), - vals.passphrase, - passphrase_type, - crypto_unlock_readonly) - .then(() => maybe_set_crypto_options(vals.mount_options?.ro, - opts.indexOf("noauto") == -1, - vals.at_boot == "nofail", - vals.at_boot == "netdev"))); + const crypto_unlock_readonly = desired_crypto_readonly(vals.mount_options?.ro ?? opt_ro); + try { + await maybe_update_config(client.add_mount_point_prefix(vals.mount_point), + unparse_options(opts), + vals.passphrase, + passphrase_type, + crypto_unlock_readonly); + if (is_probably_single_device_btrfs_volume(block)) + await maybe_update_crypto_options(client, block); + } catch (error) { + if (error.message == "unexpected-multi-device-btrfs") { + dialog_open({ + Title: _("Multi-device btrfs volume detected"), + Body:

{_("This device is only part of a btrfs volume. Please make all devices available in order to mount it.")}

+ }); + } else + throw error; + } } } }, diff --git a/pkg/storaged/filesystem/utils.jsx b/pkg/storaged/filesystem/utils.jsx index cc3ca0cf9bf..6a39360eeb7 100644 --- a/pkg/storaged/filesystem/utils.jsx +++ b/pkg/storaged/filesystem/utils.jsx @@ -148,11 +148,11 @@ export function get_cryptobacking_noauto(client, block) { return crypto_options.indexOf("noauto") >= 0; } -export function edit_mount_point(block, forced_options, subvol) { - mounting_dialog(client, block, "update", forced_options, subvol); +export function edit_mount_point(block, forced_options, subvol, subvols) { + mounting_dialog(client, block, "update", forced_options, subvol, subvols); } -export const MountPoint = ({ fstab_config, forced_options, backing_block, content_block, subvol }) => { +export const MountPoint = ({ fstab_config, forced_options, backing_block, content_block, subvol, subvols }) => { const is_filesystem_mounted = content_block && is_mounted(client, content_block, subvol); const [, old_dir, old_opts] = fstab_config; const split_options = parse_options(old_opts); @@ -217,7 +217,7 @@ export const MountPoint = ({ fstab_config, forced_options, backing_block, conten } edit_mount_point(content_block || backing_block, - forced_options, subvol)}> + forced_options, subvol, subvols)}> {_("edit")} diff --git a/pkg/storaged/utils.js b/pkg/storaged/utils.js index f09a444f257..f249e7b1def 100644 --- a/pkg/storaged/utils.js +++ b/pkg/storaged/utils.js @@ -97,7 +97,7 @@ export function edit_crypto_config(block, modify) { }); } -export function set_crypto_options(block, readonly, auto, nofail, netdev) { +function set_crypto_options(block, readonly, auto, nofail, netdev) { return edit_crypto_config(block, (config, commit) => { const opts = config.options ? parse_options(decode_filename(config.options.v)) : []; if (readonly !== null) { @@ -130,6 +130,41 @@ export function set_crypto_auto_option(block, flag) { return set_crypto_options(block, null, flag, null, null); } +async function update_crypto_options_for_children(client, block) { + // This sets the readonly, noauto, nofail, and _netdev options as + // required by the content of this block device. + + const block_crypto = client.blocks_crypto[block.path]; + + let readonly = true; + let noauto = true; + let nofail = true; + let netdev = true; + + for (const c of block_crypto.ChildConfiguration) { + if (c[0] == "fstab") { + const opts = parse_options(decode_filename(c[1].opts.v)); + if (opts.indexOf("ro") < 0) + readonly = false; + if (opts.indexOf("noauto") < 0) + noauto = false; + if (opts.indexOf("nofail") < 0) + nofail = false; + if (opts.indexOf("_netdev") < 0) + netdev = false; + } + } + + await set_crypto_options(block, readonly, !noauto, nofail, netdev); +} + +export async function maybe_update_crypto_options(client, block) { + if (client.blocks_crypto[block.path]) + await update_crypto_options_for_children(client, block); + else if (client.blocks_crypto[block.CryptoBackingDevice]) + await update_crypto_options_for_children(client, client.blocks[block.CryptoBackingDevice]); +} + export let hostnamed = cockpit.dbus("org.freedesktop.hostname1").proxy(); // for unit tests @@ -808,8 +843,8 @@ export function get_fstab_config_with_client(client, block, also_child_config, s // btrfs mounted without subvol argument. const btrfs_volume = client.blocks_fsys_btrfs[block.path]; - const default_subvolid = client.uuids_btrfs_default_subvol[btrfs_volume.data.uuid]; - if (default_subvolid === subvol.id && !opts.find(o => o.indexOf("subvol=") >= 0 || o.indexOf("subvolid=") >= 0)) + const default_subvolid = btrfs_volume && client.uuids_btrfs_default_subvol[btrfs_volume.data.uuid]; + if (default_subvolid && default_subvolid === subvol.id && !opts.find(o => o.indexOf("subvol=") >= 0 || o.indexOf("subvolid=") >= 0)) return true; return false; @@ -820,7 +855,7 @@ export function get_fstab_config_with_client(client, block, also_child_config, s let config = block.Configuration.find(match); if (!config && also_child_config && client.blocks_crypto[block.path]) - config = client.blocks_crypto[block.path]?.ChildConfiguration.find(c => c[0] == "fstab"); + config = client.blocks_crypto[block.path]?.ChildConfiguration.find(match); if (config && decode_filename(config[1].type.v) != "swap") { const mnt_opts = get_block_mntopts(config[1]).split(","); diff --git a/test/verify/check-storage-anaconda b/test/verify/check-storage-anaconda index bd63558b144..ab7338c0ca3 100755 --- a/test/verify/check-storage-anaconda +++ b/test/verify/check-storage-anaconda @@ -311,16 +311,17 @@ class TestStorageAnaconda(storagelib.StorageCase): self.dialog_apply() self.dialog_wait_close() - # Unmount and lock, mount point exporting should still work - + # Unmount, mount point exporting should still work. self.click_dropdown(self.card_row("Storage", location="/mnt/butter"), "Unmount") - self.dialog({}) - self.click_dropdown(self.card_row("Storage", name=disk), "Lock") - b.wait_text(self.card_row_col("Storage", 1, 3), "Locked data (encrypted)") + self.confirm() + + self.click_card_row("Storage", name=disk) + cleartext = b.text(self.card_desc("Encryption", "Cleartext device")) self.expectExportedDevice(disk, { "type": "crypto", + "cleartext_device": cleartext, "content": { "type": "filesystem", "subvolumes": { diff --git a/test/verify/check-storage-btrfs b/test/verify/check-storage-btrfs index c5ca64b94f1..68c46806246 100755 --- a/test/verify/check-storage-btrfs +++ b/test/verify/check-storage-btrfs @@ -522,32 +522,208 @@ class TestStorageBtrfs(storagelib.StorageCase): mount_point = "/run/butter" passphrase = "einszweidrei" - m.execute(f""" - echo {passphrase} | cryptsetup luksFormat --pbkdf-memory 32768 {disk} - echo {passphrase} | cryptsetup luksOpen {disk} btrfs-test - mkfs.btrfs -L {label} /dev/mapper/btrfs-test - """) + self.login_and_go("/storage") + + self.addCleanup(self.machine.execute, f"umount {mount_point} || true") + self.addCleanup(self.machine.execute, f"umount /run/cake || true") + + self.click_dropdown(self.card_row("Storage", name=disk), "Format") + self.dialog_wait_open() + self.dialog_set_val("name", label) + self.dialog_set_val("type", "btrfs") + self.dialog_set_val("mount_point", mount_point) + self.dialog_set_val("crypto", "luks1") + self.dialog_set_val("passphrase", passphrase) + self.dialog_set_val("passphrase2", passphrase) + self.dialog_apply() + # creation of btrfs partition can take a while on TF. + with b.wait_timeout(30): + self.dialog_wait_close() + + b.wait_in_text(self.card_row("Storage", name=disk), "btrfs filesystem (encrypted)") + self.click_card_row("Storage", name=disk) + + # LUKS should be open, "nofail", but not "noauto" or "readonly" + b.wait_text_not(self.card_desc("Encryption", "Cleartext device"), "") + self.assert_in_configuration(disk, "crypttab", "options", "nofail") + self.assert_not_in_configuration(disk, "crypttab", "options", "noauto") + self.assert_not_in_configuration(disk, "crypttab", "options", "readonly") + + # Make a second subvolume that is readonly and not nofail + self.click_dropdown(self.card_row("btrfs filesystem", name="/"), "Create subvolume") + self.dialog_wait_open() + self.dialog_set_val("name", "cake") + self.dialog_set_val("mount_point", "/run/cake") + self.dialog_set_val("mount_options.ro", True) + self.dialog_set_val("at_boot", "local") + self.dialog_apply() + self.dialog_wait_close() + + # LUKS should be open, and none of "nofail", "noauto", or "readonly" + b.wait_text_not(self.card_desc("Encryption", "Cleartext device"), "-") + self.assert_not_in_configuration(disk, "crypttab", "options", "nofail") + self.assert_not_in_configuration(disk, "crypttab", "options", "noauto") + self.assert_not_in_configuration(disk, "crypttab", "options", "readonly") + + # Check that the clear text device is writeable + uuid = m.execute(f"cryptsetup luksUUID {disk}").strip() + cleartext_dev = "/dev/mapper/luks-" + uuid + self.assertEqual(m.execute(f"lsblk -no RO {cleartext_dev}").strip(), "0") + + # Unmount first subvol + self.click_dropdown(self.card_row("btrfs filesystem", name="/"), "Unmount") + self.confirm() + + # LUKS should still be open, with the same options + b.wait_text_not(self.card_desc("Encryption", "Cleartext device"), "-") + self.assert_not_in_configuration(disk, "crypttab", "options", "nofail") + self.assert_not_in_configuration(disk, "crypttab", "options", "noauto") + self.assert_not_in_configuration(disk, "crypttab", "options", "readonly") + + # Unmount second subvol + self.click_dropdown(self.card_row("btrfs filesystem", name="cake"), "Unmount") + self.confirm() + + # Only now should LUKS be closed and have "noauto" option. + b.wait_text(self.card_desc("Encryption", "Cleartext device"), "-") + self.assert_not_in_configuration(disk, "crypttab", "options", "nofail") + self.assert_in_configuration(disk, "crypttab", "options", "noauto") + self.assert_not_in_configuration(disk, "crypttab", "options", "readonly") + + # Mount first subvol readonly, this unlocks LUKS and requires + # the passphrase + self.click_dropdown(self.card_row("btrfs filesystem", name="/"), "Mount") + self.dialog_wait_open() + self.dialog_set_val("mount_options.ro", True) + self.dialog_set_val("passphrase", passphrase) + self.dialog_apply() + self.dialog_wait_close() + + # Now LUKS is open again, and "readonly" + b.wait_text_not(self.card_desc("Encryption", "Cleartext device"), "-") + self.assert_not_in_configuration(disk, "crypttab", "options", "nofail") + self.assert_not_in_configuration(disk, "crypttab", "options", "noauto") + self.assert_in_configuration(disk, "crypttab", "options", "readonly") + + # Check that the clear text device is actually readonly + self.assertEqual(m.execute(f"lsblk -no RO {cleartext_dev}").strip(), "1") + + # Mount second subvol as "nofail", this makes LUKS "nofail" as well. + self.click_dropdown(self.card_row("btrfs filesystem", name="cake"), "Mount") + self.dialog_wait_open() + self.dialog_set_val("at_boot", "nofail") + self.dialog_apply() + self.dialog_wait_close() + + b.wait_text_not(self.card_desc("Encryption", "Cleartext device"), "-") + self.assert_in_configuration(disk, "crypttab", "options", "nofail") + self.assert_not_in_configuration(disk, "crypttab", "options", "noauto") + self.assert_in_configuration(disk, "crypttab", "options", "readonly") + + def testMultiDeviceLuks(self): + m = self.machine + b = self.browser + + label = "butter" + mount_point = "/run/butter" + passphrase = "einszweidrei" + + disk1 = self.add_ram_disk(size=140) + disk2 = self.add_loopback_disk(size=140) + + self.addCleanup(self.machine.execute, f"umount {mount_point} || true; for l in /dev/mapper/luks-*; do cryptsetup close $l; done") self.login_and_go("/storage") + + # Create single-device encrypted btrfs first + self.click_dropdown(self.card_row("Storage", name=disk1), "Format") + self.dialog_wait_open() + self.dialog_set_val("name", label) + self.dialog_set_val("type", "btrfs") + self.dialog_set_val("mount_point", mount_point) + self.dialog_set_val("crypto", "luks1") + self.dialog_set_val("passphrase", passphrase) + self.dialog_set_val("passphrase2", passphrase) + self.dialog_apply() # creation of btrfs partition can take a while on TF. with b.wait_timeout(30): - b.wait_visible(self.card_row("Storage", name="sda")) - b.wait_in_text(self.card_row("Storage", name="sda"), "btrfs filesystem (encrypted)") - self.click_dropdown(self.card_row("Storage", name="sda") + " + tr", "Mount") - self.dialog({"mount_point": mount_point}) + self.dialog_wait_close() - m.execute(f""" - umount {mount_point} - cryptsetup luksClose /dev/mapper/btrfs-test - """) - b.wait_in_text(self.card_row("Storage", name="sda"), "Locked data (encrypted)") - self.click_dropdown(self.card_row("Storage", name="sda"), "Unlock") - self.dialog({"passphrase": "einszweidrei"}) - b.wait_in_text(self.card_row("Storage", name="sda"), "btrfs filesystem (encrypted)") + # LUKS should not be "noauto". + self.assert_not_in_configuration(disk1, "crypttab", "options", "noauto") + + # Add a second encrypted device + self.click_dropdown(self.card_row("Storage", name=disk2), "Format") + self.dialog_wait_open() + self.dialog_set_val("type", "empty") + self.dialog_set_val("crypto", "luks1") + self.dialog_set_val("passphrase", passphrase) + self.dialog_set_val("passphrase2", passphrase) + self.dialog_apply() + # creation of luks can take a while. + with b.wait_timeout(30): + self.dialog_wait_close() - self.click_dropdown(self.card_row("Storage", name="sda") + " + tr", "Mount") + self.assert_not_in_configuration(disk2, "crypttab", "options", "noauto") + + self.click_card_row("Storage", name=disk2) + cleartext = b.text(self.card_desc("Encryption", "Cleartext device")) + + m.execute(f"btrfs device add {cleartext} {mount_point}; udevadm trigger") + + # Navigate to volume + b.click(self.card_desc("btrfs device", "btrfs volume") + " button") + + # Unmount + self.click_dropdown(self.card_row("btrfs subvolumes", name="/"), "Unmount") self.confirm() - b.wait_in_text(self.card_row("Storage", location=mount_point), "btrfs subvolume") + + # Both disks are still "auto" + self.assert_not_in_configuration(disk1, "crypttab", "options", "noauto") + self.assert_not_in_configuration(disk2, "crypttab", "options", "noauto") + + # Lock both LUKS + b.click(self.card_parent_link()) + self.click_dropdown(self.card_row("Storage", name=disk1), "Lock") + self.click_dropdown(self.card_row("Storage", name=disk2), "Lock") + b.wait_text(self.card_row_col("Storage", row_name=disk1, col_index=3), "btrfs filesystem (encrypted)") + b.wait_text(self.card_row_col("Storage", row_name=disk2, col_index=3), "Locked data (encrypted)") + + # Check both are now "noauto" + self.assert_in_configuration(disk1, "crypttab", "options", "noauto") + self.assert_in_configuration(disk2, "crypttab", "options", "noauto") + + # Try to mount via first disk. Cockpit thinks that this might + # be a single-device btrfs so it let's us mount it, but will + # figure it all out once the device is open. + self.click_dropdown(self.card_row("Storage", location=mount_point + " (not mounted)"), "Mount") + self.dialog_wait_open() + self.dialog_set_val("passphrase", passphrase) + self.dialog_apply() + b.wait_in_text("#dialog", "This device is only part of a btrfs volume.") + self.dialog_cancel() + self.dialog_wait_close() + + # Cockpit now knows it's a multi-device volume + b.wait_text(self.card_row_col("Storage", row_name=label, col_index=3), "btrfs subvolumes") + + # LUKS for disk1 is now "auto" + self.assert_not_in_configuration(disk1, "crypttab", "options", "noauto") + + # Unlock second disk + self.click_dropdown(self.card_row("Storage", name=disk2), "Unlock") + self.dialog_wait_open() + self.dialog_set_val("passphrase", passphrase) + self.dialog_apply() + self.dialog_wait_close() + + # LUKS for disk2 is now "auto" as well + self.assert_not_in_configuration(disk2, "crypttab", "options", "noauto") + + # The btrfs volume is now functional, so we can finally mount it + self.click_dropdown(self.card_row("Storage", location=mount_point + " (not mounted)"), "Mount") + self.confirm() + b.wait_visible(self.card_row("Storage", location=mount_point)) def testNoSubvolMount(self): m = self.machine