Handle case where we try to use a partially used luksmeta slot
In some situations, especially with older versions of clevis, we can end up with a partially used luksmeta slot. We can identify such slots because they will be marked as inactive, yet they will contain the clevis UUID, "cb6e8904-81ff-40da-a84a-07ab9ab5715e". When this situation happens, we have cryptsetup and luksmeta slots "out of sync", and since we currently have cryptsetup choose the slot, we may end up trying to use such a partially used slot, which in turn will fail because luksmeta will not be able to save data to it. We handle this case by wiping the partially used slot, if we identify the situation will arise. Tests also added to verify this case is handled properly. Fixes: #70
This commit is contained in:
Sergio Correia
parent
745ee46295
commit
e9acb551d3
202
0003-Handle-case-where-we-try-to-use-a-partially-used-luk.patch
Normal file
202
0003-Handle-case-where-we-try-to-use-a-partially-used-luk.patch
Normal file
@ -0,0 +1,202 @@
|
||||
From bc4c6374e8bbe49992a545d4b51d2b4f020bae06 Mon Sep 17 00:00:00 2001
|
||||
From: Sergio Correia <scorreia@redhat.com>
|
||||
Date: Thu, 19 Dec 2019 09:27:52 -0300
|
||||
Subject: [PATCH] Handle case where we try to use a partially used luksmeta
|
||||
slot #142
|
||||
|
||||
In some situations, especially with older versions of clevis, we can end
|
||||
up with a partially used luksmeta slot.
|
||||
|
||||
We can identify such slots because they will be marked as inactive, yet
|
||||
they will contain the clevis UUID, "cb6e8904-81ff-40da-a84a-07ab9ab5715e".
|
||||
|
||||
When this situation happens, we have cryptsetup and luksmeta slots "out
|
||||
of sync", and since we currently have cryptsetup choose the slot, we may
|
||||
end up trying to use such a partially used slot, which in turn will fail
|
||||
because luksmeta will not be able to save data to it.
|
||||
|
||||
We handle this case by wiping the partially used slot, if we identify
|
||||
the situation will arise.
|
||||
|
||||
Tests also added to verify this case is handled properly.
|
||||
|
||||
Fixes: #70
|
||||
---
|
||||
src/luks/clevis-luks-bind | 30 ++++++
|
||||
.../tests/bind-already-used-luksmeta-slot | 102 ++++++++++++++++++
|
||||
src/luks/tests/meson.build | 2 +
|
||||
3 files changed, 134 insertions(+)
|
||||
create mode 100755 src/luks/tests/bind-already-used-luksmeta-slot
|
||||
|
||||
diff --git a/src/luks/clevis-luks-bind b/src/luks/clevis-luks-bind
|
||||
index 6edfccd..7468ed9 100755
|
||||
--- a/src/luks/clevis-luks-bind
|
||||
+++ b/src/luks/clevis-luks-bind
|
||||
@@ -75,6 +75,18 @@ if ! CFG=${@:$((OPTIND++)):1} || [ -z "$CFG" ]; then
|
||||
usage
|
||||
fi
|
||||
|
||||
+if cryptsetup isLuks --type luks1 "$DEV"; then
|
||||
+ # The first free slot, as per cryptsetup. In connection to bug #70, we may
|
||||
+ # have to wipe out the LUKSMeta slot priot to adding the new key.
|
||||
+ first_free_cs_slot=$(cryptsetup luksDump "${DEV}" \
|
||||
+ | sed -rn 's|^Key Slot ([0-7]): DISABLED$|\1|p' \
|
||||
+ | head -n 1)
|
||||
+ if [ -z "${first_free_cs_slot}" ]; then
|
||||
+ echo "There are no more free slots in ${DEV}!" >&2
|
||||
+ exit 1
|
||||
+ fi
|
||||
+fi
|
||||
+
|
||||
if [ -n "$KEY" ]; then
|
||||
if [ "$KEY" == "-" ]; then
|
||||
if cryptsetup isLuks --type luks1 "$DEV"; then
|
||||
@@ -114,6 +126,24 @@ case "$KEY" in
|
||||
*) ! IFS= read -rd '' existing_key < "$KEY";;
|
||||
esac
|
||||
|
||||
+# Check if the key is valid.
|
||||
+if ! cryptsetup luksOpen --test-passphrase "${DEV}" \
|
||||
+ --key-file <(echo -n "${existing_key}"); then
|
||||
+ exit 1
|
||||
+fi
|
||||
+
|
||||
+if cryptsetup isLuks --type luks1 "${DEV}"; then
|
||||
+ # In certain circumstances, we may have LUKSMeta slots "not in sync" with
|
||||
+ # cryptsetup, which means we will try to save LUKSMeta metadata over an
|
||||
+ # already used or partially used slot -- github issue #70.
|
||||
+ # If that is the case, let's wipe the LUKSMeta slot here prior to saving.
|
||||
+ if read -r _ state uuid < <(luksmeta show -d "${DEV}" \
|
||||
+ | grep "^${first_free_cs_slot} *"); then
|
||||
+ if [ "${state}" = "inactive" ] && [ "${uuid}" = "${UUID}" ]; then
|
||||
+ luksmeta wipe -f -d "${DEV}" -s "${first_free_cs_slot}"
|
||||
+ fi
|
||||
+ fi
|
||||
+fi
|
||||
|
||||
#Add the new key
|
||||
if [ -n "$SLT" ]; then
|
||||
diff --git a/src/luks/tests/bind-already-used-luksmeta-slot b/src/luks/tests/bind-already-used-luksmeta-slot
|
||||
new file mode 100755
|
||||
index 0000000..eea3c80
|
||||
--- /dev/null
|
||||
+++ b/src/luks/tests/bind-already-used-luksmeta-slot
|
||||
@@ -0,0 +1,102 @@
|
||||
+#!/bin/bash -ex
|
||||
+# vim: set tabstop=8 shiftwidth=4 softtabstop=4 expandtab smarttab colorcolumn=80:
|
||||
+#
|
||||
+# Copyright (c) 2019 Red Hat, Inc.
|
||||
+# Author: Sergio Correia <scorreia@redhat.com>
|
||||
+#
|
||||
+# This program is free software: you can redistribute it and/or modify
|
||||
+# it under the terms of the GNU General Public License as published by
|
||||
+# the Free Software Foundation, either version 3 of the License, or
|
||||
+# (at your option) any later version.
|
||||
+#
|
||||
+# This program is distributed in the hope that it will be useful,
|
||||
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
|
||||
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
|
||||
+# GNU General Public License for more details.
|
||||
+#
|
||||
+# You should have received a copy of the GNU General Public License
|
||||
+# along with this program. If not, see <http://www.gnu.org/licenses/>.
|
||||
+#
|
||||
+
|
||||
+TEST="${0}"
|
||||
+. tests-common-functions
|
||||
+
|
||||
+on_exit() {
|
||||
+ [ -d "${TMP}" ] && rm -rf "${TMP}"
|
||||
+}
|
||||
+
|
||||
+trap 'on_exit' EXIT
|
||||
+trap 'exit' ERR
|
||||
+
|
||||
+TMP="$(mktemp -d)"
|
||||
+
|
||||
+ADV="${TMP}/adv.jws"
|
||||
+create_tang_adv "${ADV}"
|
||||
+CFG="$(printf '{"url":"foobar","adv":"%s"}' "$ADV")"
|
||||
+
|
||||
+# LUKS1.
|
||||
+DEV="${TMP}/luks1-device"
|
||||
+UUID="cb6e8904-81ff-40da-a84a-07ab9ab5715e"
|
||||
+
|
||||
+# We can have a "partially" used if it is an inactive slot that has an UUID
|
||||
+# already:
|
||||
+# 1 inactive cb6e8904-81ff-40da-a84a-07ab9ab5715e
|
||||
+# We end up in this situation if the cryptsetup step adding the key failed,
|
||||
+# for instance because we provided a wrong pass phrase, and luksmeta saved
|
||||
+# data anyway. We used to have an issue with clevis luks bind script, in which
|
||||
+# we would still run luksmeta save even if the cryptsetup step failed.
|
||||
+
|
||||
+bind_and_verify() {
|
||||
+ local DEV="${1}"
|
||||
+ local PASS="${2}"
|
||||
+ local SLT="${3}"
|
||||
+
|
||||
+ if ! clevis luks bind -f -d "${DEV}" tang "${CFG}" <<< "${PASS}"; then
|
||||
+ error "${TEST}: Binding is expected to succeed when given a correct (${PASS}) password." >&2
|
||||
+ fi
|
||||
+
|
||||
+ if ! read -r _ state uuid < <(luksmeta show -d "${DEV}" | grep "^${SLT} *"); then
|
||||
+ error "${TEST}: Error reading LUKSmeta info for slot ${SLT} of ${DEV}." >&2
|
||||
+ fi
|
||||
+
|
||||
+ if [ "${state}" != "active" ]; then
|
||||
+ error "${TEST}: state (${state}) is expected to be 'active'." >&2
|
||||
+ fi
|
||||
+
|
||||
+ if [ "${uuid}" != "${UUID}" ]; then
|
||||
+ error "${TEST}: UUID ($uuid) is expected to be '${UUID}'." >&2
|
||||
+ fi
|
||||
+}
|
||||
+
|
||||
+SLT=1
|
||||
+NEW_PASS="new-pass"
|
||||
+PASS="${DEFAULT_PASS}"
|
||||
+WRONG_PASS="wrong-password-here"
|
||||
+
|
||||
+new_device "luks1" "${DEV}"
|
||||
+luksmeta init -f -d "${DEV}"
|
||||
+if cryptsetup luksAddKey "${DEV}" < <(echo "${WRONG_PASS}"; echo -n "${NEW_PASS}"); then
|
||||
+ error "${TEST}: cryptsetup should not succeed in adding key when given a wrong passphrase." >&2
|
||||
+fi
|
||||
+
|
||||
+# Ok, the cryptsetup step failed, since we gave a wrong password. That means
|
||||
+# that right now the luksmeta slot is inactive. Let's simulate the bad
|
||||
+# condition by saving the UUID there anyway.
|
||||
+echo "foo" | luksmeta save -d "${DEV}" -u "${UUID}"
|
||||
+
|
||||
+# Verify we have slot 1 like this:
|
||||
+# # 1 inactive cb6e8904-81ff-40da-a84a-07ab9ab5715e
|
||||
+if ! read -r _ state uuid < <(luksmeta show -d "${DEV}" | grep "^${SLT} *"); then
|
||||
+ error "${TEST}: Error reading LUKSmeta info for slot ${SLT} of ${DEV}." >&2
|
||||
+fi
|
||||
+
|
||||
+if [ "${state}" != "inactive" ]; then
|
||||
+ error "${TEST}: state (${state}) is expected to be 'inactive', in case #1." >&2
|
||||
+fi
|
||||
+
|
||||
+if [ "${uuid}" != "${UUID}" ]; then
|
||||
+ error "${TEST}: UUID ($uuid) is expected to be '${UUID}', in case #1." >&2
|
||||
+fi
|
||||
+
|
||||
+# Verify if can bind correctly in this situation.
|
||||
+bind_and_verify "${DEV}" "${PASS}" "1"
|
||||
diff --git a/src/luks/tests/meson.build b/src/luks/tests/meson.build
|
||||
index 5059625..2245a46 100644
|
||||
--- a/src/luks/tests/meson.build
|
||||
+++ b/src/luks/tests/meson.build
|
||||
@@ -18,6 +18,8 @@ test('unbind-luks1', find_program('unbind-luks1'), env: env)
|
||||
test('bind-key-file-non-interactive', find_program('bind-key-file-non-interactive-luks1'), env: env)
|
||||
test('bind-pass-with-newline', find_program('bind-pass-with-newline-luks1'), env: env)
|
||||
test('bind-pass-with-newline-keyfile', find_program('bind-pass-with-newline-keyfile-luks1'), env: env)
|
||||
+# Bug #70.
|
||||
+test('bind-already-used-luksmeta-slot', find_program('bind-already-used-luksmeta-slot'), env: env, timeout: 60)
|
||||
|
||||
# LUKS2 tests go here.
|
||||
# Binding LUKS2 takes longer, so timeout is increased for a few tests.
|
||||
--
|
||||
2.23.0
|
||||
|
||||
@ -3,7 +3,7 @@
|
||||
|
||||
Name: clevis
|
||||
Version: 11
|
||||
Release: 10%{?dist}
|
||||
Release: 11%{?dist}
|
||||
Summary: Automated decryption framework
|
||||
|
||||
License: GPLv3+
|
||||
@ -21,6 +21,7 @@ Patch6: pins-tpm2-add-support-for-tpm2-tools-4.X.patch
|
||||
# Backport of some fixes and also adding tests in the build.
|
||||
Patch7: 0001-Backport-upstream-tests-and-fixes.patch
|
||||
Patch8: 0002-Disabling-LUKS2-tests-for-now.patch
|
||||
Patch9: 0003-Handle-case-where-we-try-to-use-a-partially-used-luk.patch
|
||||
|
||||
BuildRequires: gcc
|
||||
BuildRequires: meson
|
||||
@ -171,6 +172,11 @@ exit 0
|
||||
%attr(4755, root, root) %{_libexecdir}/%{name}-luks-udisks2
|
||||
|
||||
%changelog
|
||||
* Thu Dec 19 2019 Sergio Correia <scorreia@redhat.com> - 11-11
|
||||
- Backport upstream PR#70 - Handle case where we try to use a partially
|
||||
used luksmeta slot
|
||||
Resolves: rhbz#1672371
|
||||
|
||||
* Thu Dec 05 2019 Sergio Correia <scorreia@redhat.com> - 11-10
|
||||
- Disable LUKS2 tests for now, since they fail randomly in Koji
|
||||
builders, killing the build
|
||||
|
||||
Loading…
Reference in New Issue
Block a user