Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

storage: Checking and fixing system config for NBDE #17796

Merged
merged 1 commit into from
Jan 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
243 changes: 230 additions & 13 deletions pkg/storaged/crypto-keyslots.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,21 +25,24 @@ import {
Checkbox, ClipboardCopy,
Form, FormGroup,
DataListItem, DataListItemRow, DataListItemCells, DataListCell, DataList,
Text, TextVariants, TextInput as TextInputPF, Stack,
TextContent, Text, TextVariants, TextList, TextListItem, TextInput as TextInputPF, Stack,
} from "@patternfly/react-core";
import { EditIcon, MinusIcon, PlusIcon } from "@patternfly/react-icons";

import sha1 from "js-sha1";
import sha256 from "js-sha256";
import stable_stringify from "json-stable-stringify-without-jsonify";
import { check_missing_packages, install_missing_packages, Enum as PkEnum } from "packagekit";

import {
dialog_open,
SelectOneRadio, TextInput, PassInput, Skip
} from "./dialog.jsx";
import { array_find, decode_filename, block_name } from "./utils.js";
import { array_find, decode_filename, encode_filename, block_name, for_each_async } from "./utils.js";
import { fmt_to_fragments } from "utils.jsx";
import { StorageButton } from "./storage-controls.jsx";
import { parse_options, unparse_options } from "./format-dialog.jsx";
import { edit_config } from "./crypto-tab.jsx";

import clevis_luks_passphrase_sh from "raw-loader!./clevis-luks-passphrase.sh";

Expand Down Expand Up @@ -253,6 +256,197 @@ export function init_existing_passphrase(block, just_type, callback) {
};
}

/* Getting the system ready for NBDE on the root filesystem.

We need the clevis module in the initrd. If it is not there, the
clevis-dracut package should be installed and the initrd needs to
be regenerated. We do this only after the user has agreed to it.

The kernel command line needs to have rd.neednet=1 in it. We just
do this unconditionally because it's so fast.
*/

function ensure_package_installed(steps, progress, package_name) {
function status_callback(progress) {
return p => {
let text = null;
if (p.waiting) {
text = _("Waiting for other software management operations to finish");
} else if (p.package) {
Comment on lines +273 to +275
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These 3 added lines are not executed by any test. Details

let fmt;
if (p.info == PkEnum.INFO_DOWNLOADING)
fmt = _("Downloading $0");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This added line is not executed by any test. Details

else if (p.info == PkEnum.INFO_REMOVING)
fmt = _("Removing $0");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This added line is not executed by any test. Details

else
fmt = _("Installing $0");
text = cockpit.format(fmt, p.package);
}
progress(text, p.cancel);
};
}

progress(cockpit.format(_("Checking for $0 package"), package_name), null);
return check_missing_packages([package_name], null)
.then(data => {
progress(null, null);
if (data.missing_names.length + data.unavailable_names.length > 0)
steps.push({
title: cockpit.format(_("The $0 package must be installed."), package_name),
func: progress => {
if (data.remove_names.length > 0)
return Promise.reject(cockpit.format(_("Installing $0 would remove $1."), name, data.remove_names[0]));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This added line is not executed by any test. Details

else if (data.unavailable_names.length > 0)
return Promise.reject(cockpit.format(_("The $0 package is not available from any repository."), name));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This added line is not executed by any test. Details

else
return install_missing_packages(data, status_callback(progress));
}
});
});
}

function ensure_initrd_clevis_support(steps, progress, package_name) {
const task = cockpit.spawn(["lsinitrd", "-m"], { superuser: true, err: "message" });
progress(_("Checking for NBDE support in the initrd"), () => task.close());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This added line is not executed by any test. Details

return task.then(data => {
progress(null, null);
if (data.indexOf("clevis") < 0) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels a little brittle, as also lsinitrd -m also prints early CPIO image and stuff. But there doesn't seem to be a better way.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, I understand that. But it also prints other stuff. It was more a wish that more tools would have parseable output with for example --json.

return ensure_package_installed(steps, progress, package_name)
.then(() => {
steps.push({
title: _("The initrd must be regenerated."),
func: progress => {
// dracut doesn't react to SIGINT, so let's not enable our Cancel button
progress(_("Regenerating initrd"), null);
return cockpit.spawn(["dracut", "--force", "--regenerate-all"],
{ superuser: true, err: "message" });
}
});
});
}
});
}

function ensure_root_nbde_support(steps, progress) {
progress(_("Adding rd.neednet=1 to kernel command line"), null);
return cockpit.spawn(["grubby", "--update-kernel=ALL", "--args=rd.neednet=1"],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this might be a silly question, but doesn't grubby also re-generate initramfs. So in theory we could first install clevis-dracut. But that's a real nitpick I feel :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

grubby does not regenerate the initrd. At least it never did for me.

{ superuser: true, err: "message" })
.then(() => ensure_initrd_clevis_support(steps, progress, "clevis-dracut"));
}

function ensure_fstab_option(steps, progress, client, block, option) {
const cleartext = client.blocks_cleartext[block.path];
const crypto = client.blocks_crypto[block.path];
const fsys_config = (cleartext
? array_find(cleartext.Configuration, function (c) { return c[0] == "fstab" })
: array_find(crypto.ChildConfiguration, function (c) { return c[0] == "fstab" }));
const fsys_options = fsys_config && parse_options(decode_filename(fsys_config[1].opts.v));

if (!fsys_options || fsys_options.indexOf(option) >= 0)
return Promise.resolve();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This added line is not executed by any test. Details


const new_fsys_options = fsys_options.concat([option]);
const new_fsys_config = [
"fstab",
Object.assign({ }, fsys_config[1],
{
opts: {
t: 'ay',
v: encode_filename(unparse_options(new_fsys_options))
}
})
];
progress(cockpit.format(_("Adding \"$0\" to filesystem options"), option), null);
return block.UpdateConfigurationItem(fsys_config, new_fsys_config, { });
}

function ensure_crypto_option(steps, progress, client, block, option) {
const crypto_config = array_find(block.Configuration, function (c) { return c[0] == "crypttab" });
const crypto_options = crypto_config && parse_options(decode_filename(crypto_config[1].options.v));
if (!crypto_options || crypto_options.indexOf(option) >= 0)
return Promise.resolve();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This added line is not executed by any test. Details


const new_crypto_options = crypto_options.concat([option]);
progress(cockpit.format(_("Adding \"$0\" to encryption options"), option), null);
return edit_config(block, (config, commit) => {
config.options = { t: 'ay', v: encode_filename(unparse_options(new_crypto_options)) };
return commit();
});
}

function ensure_systemd_unit_enabled(steps, progress, name, package_name) {
progress(cockpit.format(_("Enabling $0"), name));
return cockpit.spawn(["systemctl", "is-enabled", name], { err: "message" })
.catch((err, output) => {
if (err && output == "" && package_name) {
// We assume that installing the package will enable the unit.
return ensure_package_installed(steps, progress, package_name);
} else
return cockpit.spawn(["systemctl", "enable", name],
{ superuser: true, err: "message" });
});
}

function ensure_non_root_nbde_support(steps, progress, client, block) {
return ensure_systemd_unit_enabled(steps, progress, "remote-cryptsetup.target")
.then(() => ensure_systemd_unit_enabled(steps, progress, "clevis-luks-askpass.path", "clevis-systemd"))
.then(() => ensure_fstab_option(steps, progress, client, block, "_netdev"))
.then(() => ensure_crypto_option(steps, progress, client, block, "_netdev"));
}

function ensure_nbde_support(steps, progress, client, block) {
const cleartext = client.blocks_cleartext[block.path];
const crypto = client.blocks_crypto[block.path];
const fsys_config = (cleartext
? array_find(cleartext.Configuration, function (c) { return c[0] == "fstab" })
: array_find(crypto.ChildConfiguration, function (c) { return c[0] == "fstab" }));
const dir = decode_filename(fsys_config[1].dir.v);

if (dir == "/") {
if (client.get_config("nbde_root_help", false)) {
steps.is_root = true;
return ensure_root_nbde_support(steps, progress);
} else
return Promise.resolve();
} else
Comment on lines +409 to +411
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These 3 added lines are not executed by any test. Details

return ensure_non_root_nbde_support(steps, progress, client, block);
}

function ensure_nbde_support_dialog(steps, client, block, url, adv, old_key, existing_passphrase) {
const dlg = dialog_open({
Title: _("Add Network Bound Disk Encryption"),
Body: (
<TextContent>
<Text compmonent={TextVariants.p}>
{ steps.is_root
? _("The system does not currently support unlocking the root filesystem with a Tang keyserver.")
: _("The system does not currently support unlocking a filesystem with a Tang keyserver during boot.")
}
</Text>
<Text compmonent={TextVariants.p}>
{_("These additional steps are necessary:")}
</Text>
<TextList>
{ steps.map((s, i) => <TextListItem key={i}>{s.title}</TextListItem>) }
</TextList>
</TextContent>),
Fields: existing_passphrase_fields(_("Saving a new passphrase requires unlocking the disk. Please provide a current disk passphrase.")),
Action: {
Title: _("Fix NBDE support"),
action: (vals, progress) => {
return for_each_async(steps, s => s.func(progress))
.then(() => {
steps = [];
progress(_("Adding key"), null);
return add_or_update_tang(dlg, vals, block,
url, adv, old_key,
vals.passphrase || existing_passphrase);
});
}
}
});
}

function parse_url(url) {
// clevis-encrypt-tang defaults to "http://" (via curl), so we do the same here.
if (!/^[a-zA-Z]+:\/\//.test(url))
Expand Down Expand Up @@ -312,15 +506,16 @@ function add_dialog(client, block) {
].concat(existing_passphrase_fields(_("Saving a new passphrase requires unlocking the disk. Please provide a current disk passphrase."))),
Action: {
Title: _("Add"),
action: function (vals) {
action: function (vals, progress) {
const existing_passphrase = vals.passphrase || recovered_passphrase;
if (!client.features.clevis || vals.type == "luks-passphrase") {
return passphrase_add(block, vals.new_passphrase, existing_passphrase);
} else {
return get_tang_adv(vals.tang_url).then(function (adv) {
edit_tang_adv(client, block, null,
vals.tang_url, adv, existing_passphrase);
});
return get_tang_adv(vals.tang_url)
.then(adv => {
edit_tang_adv(client, block, null,
vals.tang_url, adv, existing_passphrase);
});
}
}
},
Expand Down Expand Up @@ -376,6 +571,14 @@ function edit_clevis_dialog(client, block, key) {
});
}

function add_or_update_tang(dlg, vals, block, url, adv, old_key, passphrase) {
return clevis_add(block, "tang", { url: url, adv: adv }, vals.passphrase || passphrase).then(() => {
if (old_key)
return clevis_remove(block, old_key);
})
.catch(request_passphrase_on_error_handler(dlg, vals, passphrase, block));
}

function edit_tang_adv(client, block, key, url, adv, passphrase) {
const parsed = parse_url(url);
const cmd = cockpit.format("ssh $0 tang-show-keys $1", parsed.hostname, parsed.port);
Expand Down Expand Up @@ -408,12 +611,26 @@ function edit_tang_adv(client, block, key, url, adv, passphrase) {
Fields: existing_passphrase_fields(_("Saving a new passphrase requires unlocking the disk. Please provide a current disk passphrase.")),
Action: {
Title: _("Trust key"),
action: function (vals) {
return clevis_add(block, "tang", { url: url, adv: adv }, vals.passphrase || passphrase).then(() => {
if (key)
return clevis_remove(block, key);
})
.catch(request_passphrase_on_error_handler(dlg, vals, passphrase, block));
action: function (vals, progress) {
if (key) {
return add_or_update_tang(dlg, vals, block,
url, adv, key,
passphrase);
} else {
const steps = [];
return ensure_nbde_support(steps, progress, client, block)
.then(() => {
if (steps.length > 0)
ensure_nbde_support_dialog(steps, client, block, url,
adv, key, passphrase);
else {
progress(null, null);
return add_or_update_tang(dlg, vals, block,
url, adv, key,
passphrase);
}
});
}
}
}
});
Expand Down
6 changes: 5 additions & 1 deletion pkg/storaged/manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,11 @@
"stratis_package": { "fedora": "stratisd",
"centos": "stratisd",
"arch": "stratisd"
}
},
"nbde_root_help": { "fedora": true,
"centos": true,
"rhel": true
}
},
"content-security-policy": "img-src 'self' data:"
}
3 changes: 3 additions & 0 deletions test/common/packagelib.py
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,9 @@ def createYumUpdateInfo(self):
xml += '</updates>\n'
return xml

def addPackageSet(self, name):
self.machine.execute(f"mkdir -p {self.repo_dir}; cp /var/lib/package-sets/{name}/* {self.repo_dir}")

def enableRepo(self):
if self.backend == "apt":
self.createAptChangelogs()
Expand Down
Loading