storage: Correctly make LUKS devices read-only during unlocking

Also, refuse to change a filesystems readonly-ness if we can't change
the crypto layer at the same time (since it wont be unlocked during
the operation).

And workaround a unfortunate limitation in clevis-luks-unlock.

pkg/storaged/crypto/actions.jsx

@@ -45,8 +45,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

@@ -43,7 +43,7 @@ import {
 
 const _ = cockpit.gettext;
 
-export const mount_options = (opt_ro, extra_options, is_visible) => {
+export const mount_options = (opt_ro, extra_options, is_visible, ro_disabled) => {
     return CheckBoxes("mount_options", _("Mount options"),
                       {
                           visible: vals => !client.in_anaconda_mode() && (!is_visible || is_visible(vals)),
@@ -52,7 +52,12 @@ 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",
+                                  disabled: ro_disabled,
+                                  tooltip: ro_disabled && _("Filesystem must not be mounted and the encryption layer must be locked in order to change this."),
+                              },
                               { title: _("Custom mount options"), tag: "extra", type: "checkboxWithInput" },
                           ]
                       });
@@ -107,7 +112,16 @@ 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) {
+    let can_change_readonly = true;
+    if (block.CryptoBackingDevice != "/") {
+        // This is a encrypted filesystem and the crypto layer is
+        // open. We can't change the readonly attribute of a open
+        // crypto layer, so we refuse to change readonly-ness in
+        // general.  XXX - take subvols into account.
+        can_change_readonly = false;
+    }
+
+    function maybe_update_config(new_dir, new_opts, passphrase, passphrase_type, crypto_unlock_readonly) {
         let new_config = null;
         let all_new_opts;
 
@@ -186,7 +200,7 @@ export function mounting_dialog(client, block, mode, forced_options, subvol) {
             const crypto = client.blocks_crypto[block.path];
             if (mode == "mount" && crypto) {
                 try {
-                    await unlock_with_type(client, block, passphrase, passphrase_type);
+                    await unlock_with_type(client, block, passphrase, passphrase_type, crypto_unlock_readonly);
                     return await client.wait_for(() => client.blocks_cleartext[block.path]);
                 } catch (error) {
                     dlg.set_values({ needs_explicit_passphrase: true });
@@ -255,7 +269,7 @@ 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, !can_change_readonly),
             at_boot_input(at_boot),
         ];
 
@@ -343,10 +357,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",

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()
@@ -551,6 +562,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']")