Skip to content

Commit

Permalink
storage: Improved passphrase handling
Browse files Browse the repository at this point in the history 
Whenever a passphrase is needed, we now also try the stored passphrase
in addition to one recovered from Clevis.

But while a passphrase recovered from Clevis can be assumed to work, a
stored passphrase can easily be wrong.  Therefore, if a operation that
requires a passphrase fails, we now give the use the opportunity to
provide a different one.

This also fixes the flow when adding/changing Clevis keys.  Earlier,
this was done with two dialogs where the first asks for the passphrase
and the second will show the error if that passphrase was wrong,
without giving the user a opportunity to correct the passphrase.  Now
this is still done in two steps, but the second one will show a
passphrase input if the operation failed because of a wrong
passphrase.

Unlocking a disk is one of the operations that the above applies to.
It is however a bit special: It is possible to unlock a disk with a
stored passphrase or a Clevis passphrase without having the passphrase
travel through the Browser.  So we do this.

Besides improving passphrase handling for the existing operations,
this also lays the groundwork for a upcoming combined Unlock-and-Mount
operation.  That operation needs to automatically use a stored
passphrase, not let the passphrase travel through the Browser, fall
back to asking for a passphrase if the recovered one fails, and open a
dialog without doing any opportunistic unlocking just to see what will
work.

And if we anyway need all these features, why not use them in all
places that deal with passphrases.  Hence this change.
  • Loading branch information
@mvollmer
mvollmer committed May 25, 2021
1 parent fd02f13 commit 29d6c20
Show file tree
Hide file tree
Showing 5 changed files with 145 additions and 67 deletions.
33 changes: 29 additions & 4 deletions pkg/storaged/clevis-luks-passphrase.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,25 @@

set -eu

# clevis-luks-passphrase [--type] DEV
#
# Try to recover a passphrase via clevis that will open DEV, and
# output it on stdout. Such a passphrase can be used for things like
# resizing of LUKSv2 containers, or adding/changing keys.
#
# If "--type" is given, this tool writes just "clevis" to stdout when
# a passphrase is found, instead of the actual passphrase. This is
# useful to limit exposure of the passphrase when all you want to know
# is whether "clevis luks unlock" can be expected to succeed.

opt_type=no
if [ $# -gt 1 ] && [ "$1" = "--type" ]; then
opt_type=yes
shift
fi

if [ $# -ne 1 ]; then
echo "usage: $0 DEV" >&2
echo "usage: $0 [--type] DEV" >&2
exit 1
fi

Expand All @@ -21,7 +38,11 @@ if cryptsetup isLuks --type luks1 "$DEV"; then
luksmeta show -d "$DEV" | while read slot state uuid; do
if [ "$state" = "active" -a "$uuid" = "$CLEVIS_UUID" ]; then
if pp=$(luksmeta load -d "$DEV" -s "$slot" | clevis decrypt); then
printf '%s\n' "$pp"
if [ "$opt_type" = yes ]; then
echo clevis
else
printf '%s\n' "$pp"
fi
break
fi
fi
Expand All @@ -32,8 +53,12 @@ elif cryptsetup isLuks --type luks2 "$DEV"; then
tok=`cryptsetup token export --token-id "$id" "$DEV"`
jwe=`printf '%s\n' "$tok" | jose fmt -j- -Og jwe -o- | jose jwe fmt -i- -c`

if pt=`echo -n "$jwe" | clevis decrypt`; then
printf '%s\n' "$pt"
if pt=`printf '%s' "$jwe" | clevis decrypt`; then
if [ "$opt_type" = yes ]; then
echo clevis
else
printf '%s\n' "$pt"
fi
break
fi
done
Expand Down
58 changes: 24 additions & 34 deletions pkg/storaged/content-views.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import { job_progress_wrapper } from "./jobs-panel.jsx";

import { FilesystemTab, is_mounted, mounting_dialog } from "./fsys-tab.jsx";
import { CryptoTab } from "./crypto-tab.jsx";
import { get_existing_passphrase } from "./crypto-keyslots.jsx";
import { BlockVolTab, PoolVolTab } from "./lvol-tabs.jsx";
import { PVolTab, MDRaidMemberTab, VDOBackingTab } from "./pvol-tabs.jsx";
import { PartitionTab } from "./part-tab.jsx";
Expand Down Expand Up @@ -199,49 +200,38 @@ function create_tabs(client, target, is_partition) {
}

function unlock() {
if (!client.features.clevis)
return unlock_with_passphrase();
else {
return clevis_unlock()
.then(null,
function () {
return unlock_with_passphrase();
});
}
var crypto = client.blocks_crypto[block.path];
if (!crypto)
return;

return get_existing_passphrase(block, true).then(type => {
if (type == "stored") {
return (crypto.Unlock("", {})
.catch(() => unlock_with_passphrase()));
} else if (type == "clevis") {
return (clevis_unlock()
.catch(() => unlock_with_passphrase()));
} else
unlock_with_passphrase();
});
}

function unlock_with_passphrase() {
var crypto = client.blocks_crypto[block.path];
if (!crypto)
return;

/* If there is a stored passphrase, the Unlock method will
* use it unconditionally. So we don't ask for one in
* that case.
*
* http://storaged.org/doc/udisks2-api/latest/gdbus-org.freedesktop.UDisks2.Block.html#gdbus-method-org-freedesktop-UDisks2-Block.GetSecretConfiguration
*/
return block.GetSecretConfiguration({}).then(function (items) {
for (var i = 0; i < items.length; i++) {
if (items[i][0] == 'crypttab' &&
items[i][1]['passphrase-contents'] &&
utils.decode_filename(items[i][1]['passphrase-contents'].v)) {
return crypto.Unlock("", { });
}
}

dialog_open({
dialog_open({
Title: _("Unlock"),
Fields: [
PassInput("passphrase", _("Passphrase"), {})
],
Action: {
Title: _("Unlock"),
Fields: [
PassInput("passphrase", _("Passphrase"), {})
],
Action: {
Title: _("Unlock"),
action: function (vals) {
return crypto.Unlock(vals.passphrase, {});
}
action: function (vals) {
return crypto.Unlock(vals.passphrase, {});
}
});
}
});
}

Expand Down
78 changes: 62 additions & 16 deletions pkg/storaged/crypto-keyslots.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ import {
dialog_open,
SelectOneRadio, TextInput, PassInput, Skip
} from "./dialog.jsx";
import { decode_filename, block_name } from "./utils.js";
import { array_find, decode_filename, block_name } from "./utils.js";
import { fmt_to_fragments } from "./utilsx.jsx";
import { StorageButton } from "./storage-controls.jsx";

Expand Down Expand Up @@ -119,9 +119,13 @@ function clevis_remove(block, key) {
{ superuser: true, pty: true, err: "message" });
}

export function clevis_recover_passphrase(block) {
export function clevis_recover_passphrase(block, just_type) {
var dev = decode_filename(block.Device);
return cockpit.script(clevis_luks_passphrase_sh, [dev],
var args = [];
if (just_type)
args.push("--type");
args.push(dev);
return cockpit.script(clevis_luks_passphrase_sh, args,
{ superuser: true, err: "message" })
.then(output => output.trim());
}
Expand Down Expand Up @@ -157,6 +161,14 @@ function slot_remove(block, slot, passphrase) {
return spawn;
}

function passphrase_test(block, passphrase) {
var dev = decode_filename(block.Device);
return (cockpit.spawn(["cryptsetup", "luksOpen", "--test-passphrase", dev],
{ superuser: true, err: "message" }).input(passphrase)
.then(() => true)
.catch(() => false));
}

/* Dialogs
*/

Expand All @@ -172,20 +184,52 @@ export function existing_passphrase_fields(explanation) {
];
}

export function get_existing_passphrase(dlg, block) {
const prom = clevis_recover_passphrase(block).then(passphrase => {
if (passphrase == "") {
function get_stored_passphrase(block, just_type) {
const pub_config = array_find(block.Configuration, function (c) { return c[0] == "crypttab" });
if (pub_config && pub_config[1]["passphrase-path"] && decode_filename(pub_config[1]["passphrase-path"].v) != "") {
if (just_type)
return Promise.resolve("stored");
return block.GetSecretConfiguration({}).then(function (items) {
for (var i = 0; i < items.length; i++) {
if (items[i][0] == 'crypttab' && items[i][1]['passphrase-contents'])
return decode_filename(items[i][1]['passphrase-contents'].v);
}
return "";
});
}
}

export function get_existing_passphrase(block, just_type) {
return clevis_recover_passphrase(block, just_type).then(passphrase => {
return passphrase || get_stored_passphrase(block, just_type);
});
}

export function get_existing_passphrase_for_dialog(dlg, block, just_type) {
const prom = get_existing_passphrase(block, just_type).then(passphrase => {
if (!passphrase)
dlg.set_values({ needs_explicit_passphrase: true });
return null;
} else {
return passphrase;
}
return passphrase;
});

dlg.run(_("Unlocking disk..."), prom);
return prom;
}

export function request_passphrase_on_error_handler(dlg, vals, recovered_passphrase, block) {
return function (error) {
if (vals.passphrase === undefined) {
return (passphrase_test(block, recovered_passphrase)
.then(good => {
if (!good)
dlg.set_values({ needs_explicit_passphrase: true });
return Promise.reject(error);
}));
} else
return Promise.reject(error);
};
}

function parse_url(url) {
// clevis-encrypt-tang defaults to "http://" (via curl), so we do the same here.
if (!RegExp("^[a-zA-Z]+://").test(url))
Expand Down Expand Up @@ -258,7 +302,7 @@ function add_dialog(client, block) {
}
});

get_existing_passphrase(dlg, block).then(pp => { recovered_passphrase = pp });
get_existing_passphrase_for_dialog(dlg, block).then(pp => { recovered_passphrase = pp });
}

function edit_passphrase_dialog(block, key) {
Expand Down Expand Up @@ -303,7 +347,7 @@ function edit_clevis_dialog(client, block, key) {
}
});

get_existing_passphrase(dlg, block).then(pp => { recovered_passphrase = pp });
get_existing_passphrase_for_dialog(dlg, block).then(pp => { recovered_passphrase = pp });
}

function edit_tang_adv(client, block, key, url, adv, passphrase) {
Expand All @@ -312,7 +356,7 @@ function edit_tang_adv(client, block, key, url, adv, passphrase) {

var sigkey_thps = compute_sigkey_thps(tang_adv_payload(adv));

dialog_open({
const dlg = dialog_open({
Title: _("Verify key"),
Body: (
<div>
Expand All @@ -327,13 +371,15 @@ function edit_tang_adv(client, block, key, url, adv, passphrase) {
<div>{_("Manually check with SSH: ")}<pre className="inline-pre">{cmd}</pre></div>
</div>
),
Fields: existing_passphrase_fields(_("Saving a new passphrase requires unlocking the disk. Please provide a current disk passphrase.")),
Action: {
Title: _("Trust key"),
action: function () {
return clevis_add(block, "tang", { url: url, adv: adv }, passphrase).then(() => {
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));
}
}
});
Expand Down
27 changes: 16 additions & 11 deletions pkg/storaged/lvol-tabs.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,10 @@ import {
DescriptionListDescription
} from "@patternfly/react-core";
import { StorageButton, StorageLink } from "./storage-controls.jsx";
import { existing_passphrase_fields, get_existing_passphrase } from "./crypto-keyslots.jsx";
import {
existing_passphrase_fields, get_existing_passphrase_for_dialog,
request_passphrase_on_error_handler
} from "./crypto-keyslots.jsx";
import { dialog_open, TextInput, SizeSlider, BlockingMessage, TeardownMessage } from "./dialog.jsx";

const _ = cockpit.gettext;
Expand Down Expand Up @@ -247,17 +250,18 @@ function lvol_grow(client, lvol, info, to_fit) {
action: function (vals) {
return utils.teardown_active_usage(client, usage)
.then(function () {
return lvol_and_fsys_resize(client, lvol,
to_fit ? grow_size : vals.size,
info.grow_needs_unmount,
vals.passphrase || recovered_passphrase);
return (lvol_and_fsys_resize(client, lvol,
to_fit ? grow_size : vals.size,
info.grow_needs_unmount,
vals.passphrase || recovered_passphrase)
.catch(request_passphrase_on_error_handler(dlg, vals, recovered_passphrase, block)));
});
}
}
});

if (passphrase_fields.length)
get_existing_passphrase(dlg, block).then(pp => { recovered_passphrase = pp });
get_existing_passphrase_for_dialog(dlg, block).then(pp => { recovered_passphrase = pp });
}

function lvol_shrink(client, lvol, info, to_fit) {
Expand Down Expand Up @@ -332,17 +336,18 @@ function lvol_shrink(client, lvol, info, to_fit) {
action: function (vals) {
return utils.teardown_active_usage(client, usage)
.then(function () {
return lvol_and_fsys_resize(client, lvol,
to_fit ? shrink_size : vals.size,
to_fit ? false : info.shrink_needs_unmount,
vals.passphrase || recovered_passphrase);
return (lvol_and_fsys_resize(client, lvol,
to_fit ? shrink_size : vals.size,
to_fit ? false : info.shrink_needs_unmount,
vals.passphrase || recovered_passphrase)
.catch(request_passphrase_on_error_handler(dlg, vals, recovered_passphrase, block)));
});
}
}
});

if (passphrase_fields.length)
get_existing_passphrase(dlg, block).then(pp => { recovered_passphrase = pp });
get_existing_passphrase_for_dialog(dlg, block).then(pp => { recovered_passphrase = pp });
}

export class BlockVolTab extends React.Component {
Expand Down
16 changes: 14 additions & 2 deletions test/verify/check-storage-luks
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,14 +97,23 @@ class TestStorageLuks(StorageCase):
self.assertNotEqual(m.execute("grep 'weird,options' /etc/crypttab"), "")
self.wait_in_storaged_configuration("weird,options")

# Change passphrase
# Change stored passphrase
edit_button = self.content_tab_info_label(1, 2, "Stored passphrase") + " + dd button"
self.dialog_with_retry(trigger=lambda: b.click(edit_button),
expect={"passphrase": "vainu-reku-toma-rolle-kaja"},
values={"passphrase": "wrong-passphrase"})

self.assertEqual(m.execute("cat /etc/luks-keys/*"), "wrong-passphrase")

# Lock it
self.content_dropdown_action(1, "Lock")
b.wait_not_in_text("#detail-content", "ext4 file system")

# Unlock, this tries the wrong passphrase but eventually prompts.
self.content_head_action(1, "Unlock")
self.dialog({"passphrase": "vainu-reku-toma-rolle-kaja"})
self.content_row_wait_in_col(2, 1, "ext4 file system")

# Remove passphrase
edit_button = self.content_tab_info_label(1, 2, "Stored passphrase") + " + dd button"
self.dialog_with_retry(trigger=lambda: b.click(edit_button),
Expand Down Expand Up @@ -186,11 +195,14 @@ class TestStorageLuks(StorageCase):
self.dialog_wait_apply_enabled()
self.dialog_set_val("type", "tang")
self.dialog_set_val("tang_url", "127.0.0.1")
self.dialog_set_val("passphrase", "vainu-reku-toma-rolle-kaja")
self.dialog_set_val("passphrase", "wrong-passphrase")
self.dialog_apply()
b.wait_in_text("#dialog", "Make sure the key hash from the Tang server matches")
b.wait_in_text("#dialog", m.execute("tang-show-keys").strip())
self.dialog_apply()
b.wait_in_text("#dialog", "No key available with this passphrase.")
self.dialog_set_val("passphrase", "vainu-reku-toma-rolle-kaja")
self.dialog_apply()
self.dialog_wait_close()
b.wait_visible(panel + "ul li:nth-child(2)")
b.wait_in_text(panel + "ul li:nth-child(2)", "127.0.0.1")
Expand Down

0 comments on commit 29d6c20

Please sign in to comment.