storage: Support "noauto" crypto backing devices better (cockpit-project#14470)

A filesystem that has a crypto backing device with the "noauto" option
will now always have the "noauto" mount option, even when mounted.

If such a filesystem doesn't have the "noauto" option, Cockpit will
warn about it and offer to add it.

Fixes cockpit-project#14467

Author: mvollmer

pkg/storaged/format-dialog.jsx

@@ -26,7 +26,7 @@ import {
     BlockingMessage, TeardownMessage
 } from "./dialog.jsx";
 
-import { get_fstab_config, is_valid_mount_point } from "./fsys-tab.jsx";
+import { get_fstab_config, is_valid_mount_point, get_cryptobacking_noauto } from "./fsys-tab.jsx";
 
 const _ = cockpit.gettext;
 
@@ -250,12 +250,8 @@ export function format_dialog(client, path, start, size, enable_dos_extended) {
             ),
         ],
         update: function (dlg, vals, trigger) {
-            if (trigger == "crypto_options" && vals.crypto_options.auto == false)
-                dlg.set_nested_values("mount_options", { auto: false });
             if (trigger == "crypto_options" && vals.crypto_options.ro == true)
                 dlg.set_nested_values("mount_options", { ro: true });
-            if (trigger == "mount_options" && vals.mount_options.auto == true)
-                dlg.set_nested_values("crypto_options", { auto: true });
             if (trigger == "mount_options" && vals.mount_options.ro == false)
                 dlg.set_nested_values("crypto_options", { ro: false });
         },
@@ -297,8 +293,11 @@ export function format_dialog(client, path, start, size, enable_dos_extended) {
 
                 if (is_filesystem(vals)) {
                     var mount_options = [];
-                    if (!vals.mount_options.auto)
+                    if (!vals.mount_options.auto ||
+                        (is_encrypted(vals) && !vals.crypto_options.auto) ||
+                        get_cryptobacking_noauto(client, block)) {
                         mount_options.push("noauto");
+                    }
                     if (vals.mount_options.ro)
                         mount_options.push("ro");
                     if (vals.mount_options.extra)

pkg/storaged/fsys-tab.jsx

@@ -88,6 +88,19 @@ export function is_valid_mount_point(client, block, val) {
                               other_blocks.map(utils.block_name).join(", "));
 }
 
+export function get_cryptobacking_noauto(client, block) {
+    const crypto_backing = client.blocks[block.CryptoBackingDevice];
+    if (!crypto_backing)
+        return false;
+
+    const crypto_config = utils.array_find(crypto_backing.Configuration, function (c) { return c[0] == "crypttab" });
+    if (!crypto_config)
+        return false;
+
+    const crypto_options = utils.decode_filename(crypto_config[1].options.v).split(",");
+    return crypto_options.map(o => o.trim()).indexOf("noauto") >= 0;
+}
+
 export function check_mismounted_fsys(client, path, enter_warning) {
     const block = client.blocks[path];
     const block_fsys = client.blocks_fsys[path];
@@ -101,6 +114,7 @@ export function check_mismounted_fsys(client, path, enter_warning) {
     const opt_noauto = extract_option(split_options, "noauto");
     const is_mounted = mounted_at.indexOf(dir) >= 0;
     const other_mounts = mounted_at.filter(m => m != dir);
+    const crypto_backing_noauto = get_cryptobacking_noauto(client, block);
 
     let type;
     if (dir) {
@@ -109,9 +123,11 @@ export function check_mismounted_fsys(client, path, enter_warning) {
                 type = "change-mount-on-boot";
             else
                 type = "mounted-no-config";
-        } else if (!is_mounted && !opt_noauto)
+        } else if (crypto_backing_noauto && !opt_noauto)
+            type = "locked-on-boot-mount";
+        else if (!is_mounted && !opt_noauto)
             type = "mount-on-boot";
-        else if (is_mounted && opt_noauto)
+        else if (is_mounted && opt_noauto && !crypto_backing_noauto)
             type = "no-mount-on-boot";
     } else if (other_mounts.length > 0) {
         type = "mounted-no-config";
@@ -125,6 +141,7 @@ export function mounting_dialog(client, block, mode) {
     const block_fsys = client.blocks_fsys[block.path];
     var [old_config, old_dir, old_opts, old_parents] = get_fstab_config(block);
     var options = old_config ? old_opts : initial_tab_options(client, block, true);
+    const crypto_backing_noauto = get_cryptobacking_noauto(client, block);
 
     var split_options = parse_options(options == "defaults" ? "" : options);
     var opt_noauto = extract_option(split_options, "noauto");
@@ -137,14 +154,12 @@ export function mounting_dialog(client, block, mode) {
         var new_config = null;
         var all_new_opts;
 
-        if (old_config) {
-            if (new_opts && old_parents)
-                all_new_opts = new_opts + "," + old_parents;
-            else if (new_opts)
-                all_new_opts = new_opts;
-            else
-                all_new_opts = old_parents;
-        }
+        if (new_opts && old_parents)
+            all_new_opts = new_opts + "," + old_parents;
+        else if (new_opts)
+            all_new_opts = new_opts;
+        else
+            all_new_opts = old_parents;
 
         if (new_dir != "") {
             if (new_dir[0] != "/")
@@ -295,7 +310,7 @@ export function mounting_dialog(client, block, mode) {
                     return do_unmount();
                 } else if (mode == "mount" || mode == "update") {
                     var opts = [];
-                    if (mode == "update" && opt_noauto)
+                    if ((mode == "update" && opt_noauto) || crypto_backing_noauto)
                         opts.push("noauto");
                     if (vals.mount_options.ro)
                         opts.push("ro");
@@ -400,7 +415,7 @@ export class FilesystemTab extends React.Component {
             const { type, other } = mismounted_fsys_warning;
 
             const opts = [];
-            if (type == "mount-on-boot")
+            if (type == "mount-on-boot" || type == "locked-on-boot-mount")
                 opts.push("noauto");
             if (opt_ro)
                 opts.push("ro");
@@ -469,6 +484,10 @@ export class FilesystemTab extends React.Component {
                 text = cockpit.format(_("The filesystem is currently mounted on $0 but will not be mounted after the next boot."), other);
                 fix_config_text = cockpit.format(_("Mount automatically on $0 on boot"), other);
                 fix_mount_text = _("Unmount now");
+            } else if (type == "locked-on-boot-mount") {
+                text = _("The filesystem is configured to be automatically mounted on boot but its encryption container will not be unlocked at that time.");
+                fix_config_text = _("Do not mount automatically on boot");
+                fix_mount_text = null;
             }
 
             mismounted_section = (
@@ -480,7 +499,7 @@ export class FilesystemTab extends React.Component {
                         {text}
                         <div className="storage_alert_action_buttons">
                             <StorageButton onClick={fix_config}>{fix_config_text}</StorageButton>
-                            <StorageButton onClick={fix_mount}>{fix_mount_text}</StorageButton>
+                            { fix_mount_text && <StorageButton onClick={fix_mount}>{fix_mount_text}</StorageButton> }
                         </div>
                     </Alert>
                 </>);

test/verify/check-storage-luks

@@ -377,5 +377,59 @@ class TestStorageLuks(StorageCase):
         self.content_row_wait_in_col(1, 1, "Encrypted data")
         self.content_row_wait_in_col(2, 1, "Unrecognized Data")
 
+    def testNoauto(self):
+        m = self.machine
+        b = self.browser
+
+        self.login_and_go("/storage")
+
+        # Add a disk and format it with luks and a filesystem, but without "Unlock at boot"
+
+        m.add_disk("50M", serial="MYDISK")
+        b.wait_in_text("#drives", "MYDISK")
+        b.click('tr:contains("MYDISK")')
+        b.wait_visible("#storage-detail")
+
+        self.content_head_action(1, "Format")
+        self.dialog({"type": "ext4",
+                     "crypto.on": True,
+                     "crypto_options.auto": False,
+                     "passphrase": "vainu-reku-toma-rolle-kaja",
+                     "passphrase2": "vainu-reku-toma-rolle-kaja",
+                     "mount_options.auto": True,
+                     "mount_point": "/run/foo"})
+        self.content_row_wait_in_col(1, 1, "Encrypted data")
+        self.content_tab_wait_in_info(1, 1, "Options", "noauto")
+        self.content_row_wait_in_col(2, 1, "ext4 File System")
+
+        # The filesystem should be mounted but have the "noauto" option
+        self.wait_mounted(2,1)
+        self.assertIn("noauto", m.execute("grep /run/foo /etc/fstab || true"))
+
+        # Unmounting should keep the noauto option, as always
+        self.content_dropdown_action(2, "Unmount")
+        self.content_tab_wait_in_info(2, 1, "Mount Point", "The filesystem is not mounted")
+        self.assertIn("noauto", m.execute("grep /run/foo /etc/fstab || true"))
+
+        # Mounting should also keep the "noauto", but it should not show up in the extra options
+        self.content_head_action(2, "Mount")
+        self.dialog_check({ "mount_options.extra": False })
+        self.dialog_apply()
+        self.wait_mounted(2,1)
+        self.assertIn("noauto", m.execute("grep /run/foo /etc/fstab || true"))
+
+        # As should updating the mount information
+        self.content_tab_info_action(2, 1, "Mount Point", wrapped=True)
+        self.dialog_check({ "mount_options.extra": False })
+        self.dialog_apply()
+        self.assertIn("noauto", m.execute("grep /run/foo /etc/fstab || true"))
+
+        # Removing "noauto" externally should show a warning
+        m.execute("sed -i -e 's/noauto//' /etc/fstab")
+        fsys_tab = self.content_tab_expand(2, 1)
+        b.wait_in_text(fsys_tab, "The filesystem is configured to be automatically mounted on boot but its encryption container will not be unlocked at that time.")
+        b.click(fsys_tab + " button:contains(Do not mount automatically)")
+        b.wait_not_present(fsys_tab + " button:contains(Do not mount automatically)")
+
 if __name__ == '__main__':
     test_main()

test/verify/check-storage-mounting

@@ -145,18 +145,10 @@ class TestStorageMounting(StorageCase):
         wait_not_checked("crypto_options.ro")
         wait_not_checked("mount_options.ro")
 
-        # Uncheck crypto auto.  This gets propagated to mount auto.
-        self.dialog_set_val("crypto_options.auto", False)
-        wait_not_checked("mount_options.auto")
-
         # Check crypto ro.  This gets propagated to mount ro.
         self.dialog_set_val("crypto_options.ro", True)
         wait_checked("mount_options.ro")
 
-        # Check mount auto.  This gets propagated to crypto auto.
-        self.dialog_set_val("mount_options.auto", True)
-        wait_checked("crypto_options.auto")
-
         # Uncheck mount ro.  This gets propagated to crypto ro.
         self.dialog_set_val("mount_options.ro", False)
         wait_not_checked("crypto_options.ro")
@@ -167,8 +159,8 @@ class TestStorageMounting(StorageCase):
         self.dialog_set_val("crypto_options.ro", True)
 
         # Set extra options.
-        self.dialog_set_val("crypto_options.extra", "foo")
-        self.dialog_set_val("mount_options.extra", "foo")
+        self.dialog_set_val("crypto_options.extra", "x-foo")
+        self.dialog_set_val("mount_options.extra", "x-foo")
 
         # Fill in the erst and do the format
         self.dialog_set_val("passphrase", "vainu-reku-toma-rolle-kaja")
@@ -180,8 +172,10 @@ class TestStorageMounting(StorageCase):
         self.content_row_wait_in_col(1, 1, "Encrypted data")
         self.content_row_wait_in_col(2, 1, "ext4 File System")
         self.wait_in_storaged_configuration("/data")
-        m.execute("grep 'noauto,readonly,foo' /etc/crypttab")
-        m.execute("grep 'noauto,ro,foo' /etc/fstab")
+        self.wait_mounted(2, 1)
+
+        m.execute("grep 'noauto,readonly,x-foo' /etc/crypttab")
+        m.execute("grep 'noauto,ro,x-foo' /etc/fstab")
 
 
     def testMountingHelp(self):
@@ -293,5 +287,25 @@ class TestStorageMounting(StorageCase):
         self.content_row_wait_in_col(1, 1, "ext4 File System")
         self.content_tab_wait_in_info(1, 2, "Mount Point", "/run/data")
 
+    def testFirstMount(self):
+        m = self.machine
+        b = self.browser
+
+        self.login_and_go("/storage")
+
+        m.add_disk("50M", serial="MYDISK")
+        b.click("#drives tr:contains(MYDISK)")
+        b.wait_visible("#storage-detail")
+
+        m.execute("mkfs.ext4 /dev/sda")
+        self.content_row_wait_in_col(1, 1, "ext4 File System")
+
+        m.execute("! grep /run/data /etc/fstab")
+        self.content_head_action(1, "Mount")
+        self.dialog({ "mount_point": "/run/data",
+                      "mount_options.extra": "x-foo" })
+        m.execute("grep /run/data /etc/fstab")
+        m.execute("grep 'x-foo' /etc/fstab")
+
 if __name__ == '__main__':
     test_main()