Skip to content

Commit

Permalink
storage: Correctly maintain actual LUKS device readonly-ness
Browse files Browse the repository at this point in the history 
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.)
  • Loading branch information
@mvollmer
mvollmer committed Mar 19, 2024
1 parent 294db9e commit 3f7f1af
Show file tree
Hide file tree
Showing 6 changed files with 219 additions and 54 deletions.
38 changes: 29 additions & 9 deletions pkg/storaged/block/format-dialog.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -285,6 +285,8 @@ function format_dialog_internal(client, path, start, size, enable_dos_extended,
extract_option(crypto_split_options, "noauto");
extract_option(crypto_split_options, "nofail");
extract_option(crypto_split_options, "_netdev");
extract_option(crypto_split_options, "readonly");
extract_option(crypto_split_options, "read-only");
const crypto_extra_options = unparse_options(crypto_split_options);

let [, old_dir, old_opts] = get_fstab_config(block, true,
Expand Down Expand Up @@ -494,6 +496,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")
Expand Down Expand Up @@ -572,17 +576,24 @@ 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) {
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() {
Expand Down Expand Up @@ -644,6 +655,15 @@ 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) {
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);
Expand Down
4 changes: 1 addition & 3 deletions pkg/storaged/crypto/actions.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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);
}
}
Expand Down
54 changes: 44 additions & 10 deletions pkg/storaged/crypto/keyslots.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import cockpit from "cockpit";
import React from "react";
import client from "../client.js";

import { CardBody, CardHeader, CardTitle } from '@patternfly/react-core/dist/esm/components/Card/index.js';
import { Checkbox } from "@patternfly/react-core/dist/esm/components/Checkbox/index.js";
Expand All @@ -38,7 +39,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";
Expand Down Expand Up @@ -72,22 +76,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");
Expand Down Expand Up @@ -187,7 +220,8 @@ export function init_existing_passphrase(block, just_type, callback) {
return {
title: _("Unlocking disk"),
func: dlg => {
return get_existing_passphrase(block, just_type).then(passphrase => {
const backing = client.blocks[block.CryptoBackingDevice];
return get_existing_passphrase(backing || block, just_type).then(passphrase => {
if (!passphrase)
dlg.set_values({ needs_explicit_passphrase: true });
if (callback)
Expand Down
4 changes: 3 additions & 1 deletion pkg/storaged/dialog.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down Expand Up @@ -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} />;
Expand Down
83 changes: 58 additions & 25 deletions pkg/storaged/filesystem/mounting-dialog.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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" },
]
});
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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" && is_filesystem_mounted)) {
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() {
Expand Down Expand Up @@ -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 = {
Expand Down Expand Up @@ -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],
Expand All @@ -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 ?? opt_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",
Expand All @@ -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);
}),
]
});
}
Loading

0 comments on commit 3f7f1af

Please sign in to comment.