storage: Correctly maintain actual LUKS device readonly-ness

Previously, Cockpit would set the crypttab "readonly" option, but
would not actually unlock crypto devices with the correct
readonly-ness. One needs to pass this explicitly in the "Unlock"
UDisks2 method, as it turns out.

Also, Cockpit needs to reopen the crypt devices whenever readonly-ness
changes, such as during formatting, and configuration changes.

(And we need to workaround a unfortunate limitation in
clevis-luks-unlock.)

pkg/storaged/block/format-dialog.jsx

@@ -176,7 +176,7 @@ function format_dialog_internal(client, path, start, size, enable_dos_extended,
     const content_block = block.IdUsage == "crypto" ? client.blocks_cleartext[path] : block;
 
     const offer_keep_keys = block.IdUsage == "crypto";
-    const unlock_before_format = offer_keep_keys && !content_block;
+    const unlock_before_format = offer_keep_keys && (!content_block || content_block.ReadOnly);
 
     const create_partition = (start !== undefined);
 
@@ -494,6 +494,8 @@ function format_dialog_internal(client, path, start, size, enable_dos_extended,
                 if (is_encrypted(vals)) {
                     let opts = [];
                     if (is_filesystem(vals)) {
+                        if (vals.mount_options?.ro)
+                            opts.push("readonly");
                         if (!mount_now || vals.at_boot == "never")
                             opts.push("noauto");
                         if (vals.at_boot == "nofail")
@@ -572,17 +574,25 @@ function format_dialog_internal(client, path, start, size, enable_dos_extended,
                 if (config_items.length > 0)
                     options["config-items"] = { t: 'a(sa{sv})', v: config_items };
 
-                function maybe_unlock() {
+                async function maybe_unlock() {
                     const content_block = client.blocks_cleartext[path];
-                    if (content_block)
+                    if (content_block) {
+                        if (content_block.ReadOnly) {
+                            console.log("REOPEN read/write");
+                            const block_crypto = client.blocks_crypto[path];
+                            await block_crypto.Lock({});
+                            await unlock_with_type(client, block, vals.old_passphrase, existing_passphrase_type, false);
+                        }
                         return content_block;
+                    }
 
-                    return (unlock_with_type(client, block, vals.old_passphrase, existing_passphrase_type)
-                            .catch(error => {
-                                dlg.set_values({ needs_explicit_passphrase: true });
-                                return Promise.reject(error);
-                            })
-                            .then(() => client.blocks_cleartext[path]));
+                    try {
+                        await unlock_with_type(client, block, vals.old_passphrase, existing_passphrase_type, false);
+                        return client.blocks_cleartext[path];
+                    } catch (error) {
+                        dlg.set_values({ needs_explicit_passphrase: true });
+                        throw error;
+                    }
                 }
 
                 function format() {
@@ -644,6 +654,16 @@ function format_dialog_internal(client, path, start, size, enable_dos_extended,
                     if (is_encrypted(vals))
                         remember_passphrase(new_block, vals.passphrase);
 
+                    if (is_encrypted(vals) && is_filesystem(vals) && vals.mount_options?.ro) {
+                        console.log("REOPEN readonly")
+                        const block_crypto = await client.wait_for(() => block_crypto_for_block(path));
+                        await block_crypto.Lock({});
+                        if (vals.passphrase)
+                            await block_crypto.Unlock(vals.passphrase, { "read-only": { t: "b", v: true } });
+                        else
+                            await unlock_with_type(client, block, vals.old_passphrase, existing_passphrase_type, true);
+                    }
+
                     if (is_filesystem(vals) && mount_now) {
                         const block_fsys = await client.wait_for(() => block_fsys_for_block(path));
                         await client.mount_at(client.blocks[block_fsys.path], mount_point);

pkg/storaged/crypto/actions.jsx

@@ -23,7 +23,6 @@ import client from "../client";
 import { get_existing_passphrase, unlock_with_type } from "./keyslots.jsx";
 import { set_crypto_auto_option } from "../utils.js";
 import { dialog_open, PassInput } from "../dialog.jsx";
-import { remember_passphrase } from "../anaconda.jsx";
 
 const _ = cockpit.gettext;
 
@@ -45,8 +44,7 @@ export function unlock(block) {
             Action: {
                 Title: _("Unlock"),
                 action: async function (vals) {
-                    await crypto.Unlock(vals.passphrase, {});
-                    remember_passphrase(block, vals.passphrase);
+                    await unlock_with_type(client, block, vals.passphrase, null);
                     await set_crypto_auto_option(block, true);
                 }
             }

pkg/storaged/crypto/keyslots.jsx

@@ -38,7 +38,10 @@ import {
     dialog_open,
     SelectOneRadio, TextInput, PassInput, Skip
 } from "../dialog.jsx";
-import { decode_filename, encode_filename, get_block_mntopts, block_name, for_each_async, get_children, parse_options, unparse_options, edit_crypto_config } from "../utils.js";
+import {
+    decode_filename, encode_filename, get_block_mntopts, block_name, for_each_async, get_children,
+    parse_options, extract_option, unparse_options, edit_crypto_config
+} from "../utils.js";
 import { StorageButton } from "../storage-controls.jsx";
 
 import clevis_luks_passphrase_sh from "./clevis-luks-passphrase.sh";
@@ -72,22 +75,51 @@ export function clevis_recover_passphrase(block, just_type) {
             .then(output => output.trim());
 }
 
-function clevis_unlock(block) {
+async function clevis_unlock(client, block, luksname, readonly) {
     const dev = decode_filename(block.Device);
-    const clear_dev = "luks-" + block.IdUUID;
-    return cockpit.spawn(["clevis", "luks", "unlock", "-d", dev, "-n", clear_dev],
-                         { superuser: true });
+    const clear_dev = luksname || "luks-" + block.IdUUID;
+
+    if (readonly) {
+        // HACK - clevis-luks-unlock can not unlock things readonly.
+        // But see https://github.com/latchset/clevis/pull/317 (merged
+        // Feb 2023, unreleased as of Feb 2024).
+        const passphrase = await clevis_recover_passphrase(block, false);
+        const crypto = client.blocks_crypto[block.path];
+        const unlock_options = { "read-only": { t: "b", v: readonly } };
+        await crypto.Unlock(passphrase, unlock_options);
+        return;
+    }
+
+    await cockpit.spawn(["clevis", "luks", "unlock", "-d", dev, "-n", clear_dev],
+                        { superuser: true });
 }
 
-export async function unlock_with_type(client, block, passphrase, passphrase_type) {
+export async function unlock_with_type(client, block, passphrase, passphrase_type, override_readonly) {
     const crypto = client.blocks_crypto[block.path];
+    let readonly = false;
+    let luksname = null;
+
+    for (const c of block.Configuration) {
+        if (c[0] == "crypttab") {
+            const options = parse_options(decode_filename(c[1].options.v));
+            readonly = extract_option(options, "readonly") || extract_option(options, "read-only");
+            luksname = decode_filename(c[1].name.v);
+            break;
+        }
+    }
+
+    if (override_readonly !== null && override_readonly !== undefined)
+        readonly = override_readonly;
+
+    const unlock_options = { "read-only": { t: "b", v: readonly } };
+
     if (passphrase) {
-        await crypto.Unlock(passphrase, {});
+        await crypto.Unlock(passphrase, unlock_options);
         remember_passphrase(block, passphrase);
     } else if (passphrase_type == "stored") {
-        await crypto.Unlock("", {});
+        await crypto.Unlock("", unlock_options);
     } else if (passphrase_type == "clevis") {
-        await clevis_unlock(block);
+        await clevis_unlock(client, block, luksname, readonly);
     } else {
         // This should always be caught and should never show up in the UI
         throw new Error("No passphrase");

pkg/storaged/dialog.jsx

@@ -863,11 +863,12 @@ export const SelectSpace = (tag, title, options) => {
     };
 };
 
-const CheckBoxComponent = ({ tag, val, title, tooltip, update_function }) => {
+const CheckBoxComponent = ({ tag, val, title, tooltip, disabled, update_function }) => {
     return (
         <Checkbox data-field={tag} data-field-type="checkbox"
                   id={tag}
                   isChecked={val}
+                  isDisabled={disabled}
                   label={
                       <>
                           {title}
@@ -905,6 +906,7 @@ export const CheckBoxes = (tag, title, options) => {
                                               tag={ftag}
                                               val={fval}
                                               title={field.title}
+                                              disabled={field.disabled}
                                               tooltip={field.tooltip}
                                               options={options}
                                               update_function={fchange} />;

pkg/storaged/filesystem/mounting-dialog.jsx

@@ -52,7 +52,10 @@ export const mount_options = (opt_ro, extra_options, is_visible) => {
                               extra: extra_options || false
                           },
                           fields: [
-                              { title: _("Mount read only"), tag: "ro" },
+                              {
+                                  title: _("Mount read only"),
+                                  tag: "ro",
+                              },
                               { title: _("Custom mount options"), tag: "extra", type: "checkboxWithInput" },
                           ]
                       });
@@ -107,7 +110,7 @@ export function mounting_dialog(client, block, mode, forced_options, subvol) {
 
     const is_filesystem_mounted = is_mounted(client, block, subvol);
 
-    function maybe_update_config(new_dir, new_opts, passphrase, passphrase_type) {
+    function maybe_update_config(new_dir, new_opts, passphrase, passphrase_type, crypto_unlock_readonly) {
         let new_config = null;
         let all_new_opts;
 
@@ -183,17 +186,31 @@ export function mounting_dialog(client, block, mode, forced_options, subvol) {
         }
 
         async function maybe_unlock() {
-            const crypto = client.blocks_crypto[block.path];
-            if (mode == "mount" && crypto) {
-                try {
-                    await unlock_with_type(client, block, passphrase, passphrase_type);
-                    return await client.wait_for(() => client.blocks_cleartext[block.path]);
-                } catch (error) {
-                    dlg.set_values({ needs_explicit_passphrase: true });
-                    throw error;
+            if (mode == "mount" || mode == "update") {
+                let crypto = client.blocks_crypto[block.path];
+                const backing = client.blocks[block.CryptoBackingDevice];
+
+                if (backing && block.ReadOnly != crypto_unlock_readonly) {
+                    // We are working on a open crypto device, but it
+                    // has the wrong readonly-ness. Close it so that we can reopen it below.
+                    crypto = client.blocks_crypto[backing.path];
+                    await crypto.Lock({});
                 }
-            } else
-                return block;
+
+                if (crypto) {
+                    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]);
+                    } catch (error) {
+                        passphrase_type = null;
+                        dlg.set_values({ needs_explicit_passphrase: true });
+                        throw error;
+                    }
+                }
+            }
+
+            return block;
         }
 
         function maybe_lock() {
@@ -255,18 +272,17 @@ export function mounting_dialog(client, block, mode, forced_options, subvol) {
                                                                 mode == "update",
                                                                 subvol)
                       }),
-            mount_options(opt_ro, extra_options),
+            mount_options(opt_ro, extra_options, null),
             at_boot_input(at_boot),
         ];
 
-        if (block.IdUsage == "crypto" && mode == "mount")
-            fields = fields.concat([
-                PassInput("passphrase", _("Passphrase"),
-                          {
-                              visible: vals => vals.needs_explicit_passphrase,
-                              validate: val => !val.length && _("Passphrase cannot be empty"),
-                          })
-            ]);
+        fields = fields.concat([
+            PassInput("passphrase", _("Passphrase"),
+                      {
+                          visible: vals => vals.needs_explicit_passphrase,
+                          validate: val => !val.length && _("Passphrase cannot be empty"),
+                      })
+        ]);
     }
 
     const mode_title = {
@@ -313,13 +329,26 @@ export function mounting_dialog(client, block, mode, forced_options, subvol) {
 
     const usage = get_active_usage(client, block.path, null, null, false, subvol);
 
+    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)
+                need_passphrase = true;
+        }
+        dlg.set_values({ needs_explicit_passphrase: need_passphrase && !passphrase_type });
+    }
+
     const dlg = dialog_open({
         Title: cockpit.format(mode_title[mode], old_dir_for_display),
         Fields: fields,
         Teardown: TeardownMessage(usage, old_dir),
         update: function (dlg, vals, trigger) {
             if (trigger == "at_boot")
                 dlg.set_options("at_boot", { explanation: mount_explanation[vals.at_boot] });
+            if (trigger == "mount_options")
+                update_explicit_passphrase(vals.mount_options.ro);
         },
         Action: {
             Title: mode_action[mode],
@@ -343,10 +372,13 @@ 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;
                     return (maybe_update_config(client.add_mount_point_prefix(vals.mount_point),
                                                 unparse_options(opts),
                                                 vals.passphrase,
-                                                passphrase_type)
+                                                passphrase_type,
+                                                crypto_unlock_readonly)
                             .then(() => maybe_set_crypto_options(vals.mount_options?.ro,
                                                                  opts.indexOf("noauto") == -1,
                                                                  vals.at_boot == "nofail",
@@ -356,9 +388,10 @@ export function mounting_dialog(client, block, mode, forced_options, subvol) {
         },
         Inits: [
             init_active_usage_processes(client, usage, old_dir),
-            (block.IdUsage == "crypto" && mode == "mount")
-                ? init_existing_passphrase(block, true, type => { passphrase_type = type })
-                : null
+            init_existing_passphrase(block, true, type => {
+                passphrase_type = type;
+                update_explicit_passphrase(dlg.get_value("mount_options")?.ro ?? opt_ro);
+            }),
         ]
     });
 }

test/verify/check-storage-luks

@@ -166,6 +166,17 @@ class TestStorageLuks(storagelib.StorageCase):
         self.assert_in_configuration(dev, "crypttab", "options", "readonly")
         self.assert_in_configuration(cleartext_dev, "fstab", "opts", "ro")
 
+        # Check that the clear text device is actually readonly
+        self.assertEqual(m.execute(f"lsblk -no RO {cleartext_dev}").strip(), "1")
+
+        # Try to change mount options. This should not be possible
+        # while LUKS is open.
+        b.click(self.card_desc("ext4 filesystem", "Mount point") + " button")
+        self.dialog_wait_open()
+        b.wait_visible(self.dialog_field("mount_options.ro") + ":disabled")
+        self.dialog_cancel()
+        self.dialog_wait_close()
+
         # Delete the partition.
         self.click_card_dropdown("Partition", "Delete")
         self.confirm()
@@ -223,12 +234,10 @@ class TestStorageLuks(storagelib.StorageCase):
         b.click(self.card_button("Locked data", "Unlock"))
         self.dialog({"passphrase": "vainu-reku-toma-rolle-kaja"})
 
-        # Try to format it, just for kicks
-        self.click_card_dropdown("Unformatted data", "Format")
-        self.dialog({"type": "ext4",
-                     "mount_point": "/run/foo"})
-        b.wait_visible(self.card("ext4 filesystem"))
-        self.wait_mounted("ext4 filesystem")
+        # Should be unlocked readonly
+        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(), "1")
 
         # Now create a empty, encrypted partition
         self.click_card_dropdown("Solid State Drive", "Create partition table")
@@ -551,6 +560,24 @@ class TestStorageNBDE(storagelib.StorageCase, packagelib.PackageCase):
         self.dialog_wait_close()
         self.wait_mounted("ext4 filesystem")
 
+        # LUKS should be read/write
+        uuid = m.execute(f"cryptsetup luksUUID {dev}").strip()
+        cleartext_dev = "/dev/mapper/luks-" + uuid
+        self.assertEqual(m.execute(f"lsblk -no RO {cleartext_dev}").strip(), "0")
+
+        # Remount "readonly", LUKS should be readonly
+        b.click(self.card_button("ext4 filesystem", "Unmount"))
+        self.confirm()
+        b.click(self.card_button("Filesystem", "Mount"))
+        self.dialog_wait_open()
+        self.dialog_set_val("mount_options.ro", True)
+        self.dialog_apply()
+        self.dialog_wait_close()
+        self.wait_mounted("ext4 filesystem")
+
+        # Now LUKS should be readonly
+        self.assertEqual(m.execute(f"lsblk -no RO {cleartext_dev}").strip(), "1")
+
         # Edit the key, without providing an existing passphrase
         #
         b.click(panel + "ul li:nth-child(2) [aria-label='Edit']")