diff --git a/0003-Handle-case-where-we-try-to-use-a-partially-used-luk.patch b/0003-Handle-case-where-we-try-to-use-a-partially-used-luk.patch new file mode 100644 index 0000000..1f65ac5 --- /dev/null +++ b/0003-Handle-case-where-we-try-to-use-a-partially-used-luk.patch @@ -0,0 +1,202 @@ +From bc4c6374e8bbe49992a545d4b51d2b4f020bae06 Mon Sep 17 00:00:00 2001 +From: Sergio Correia +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 ++# ++# 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 . ++# ++ ++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 + diff --git a/clevis.spec b/clevis.spec index 700b934..0194005 100644 --- a/clevis.spec +++ b/clevis.spec @@ -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 - 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 - 11-10 - Disable LUKS2 tests for now, since they fail randomly in Koji builders, killing the build