Address review for: first set up of TPM DUK and renewal after firmware upg

initrd/bin/kexec-save-default

@@ -45,6 +45,8 @@ if [ "$num_lvm" -eq 1 ] && [ -n "$lvm_suggest" ]; then
 elif [ -z "$lvm_suggest" ]; then
 	num_lvm=0
 fi
+# $lvm_suggest is a multiline string, we need to convert it to a space separated string
+lvm_suggest=$(echo "$lvm_suggest" | xargs)
 DEBUG "LVM num_lvm: $num_lvm, lvm_suggest: $lvm_suggest"
 
 # get all LUKS container devices
@@ -55,9 +57,11 @@ num_devices=$(echo "$devices_suggest" | wc -l)
 
 if [ "$num_devices" -eq 1 ] && [ -s "$devices_suggest" ]; then
 	key_devices=$devices_suggest
-else
+elif [ -z "$devices_suggest" ]; then
-	devices_suggest=$(echo $devices_suggest)
+	num_devices=0
 fi
+# $devices_suggest is a multiline string, we need to convert it to a space separated string
+devices_suggest=$(echo "$devices_suggest" | xargs)
 DEBUG "LUKS num_devices: $num_devices, devices_suggest: $devices_suggest"
 
 if [ "$num_lvm" -eq 0 ] && [ "$num_devices" -eq 0 ]; then
@@ -65,6 +69,113 @@ if [ "$num_lvm" -eq 0 ] && [ "$num_devices" -eq 0 ]; then
 	no_encrypted_partition=1
 fi
 
+#Reusable function when user wants to define new TPM DUK for lvms/disks
+prompt_for_existing_encrypted_lvms_or_disks() {
+	TRACE "Under kexec-save-default:prompt_for_existing_encrypted_lvms_or_disks"
+	DEBUG "num_lvm: $num_lvm, lvm_suggest: $lvm_suggest, num_devices: $num_devices, devices_suggest: $devices_suggest"
+
+	# Create an associative array to store the suggested LVMs and their paths
+	declare -A lvms_array
+	# Loop through the suggested LVMs and add them to the array
+	for lvm in $lvm_suggest; do
+		lvms_array[$lvm]=$lvm
+	done
+
+	# Get the number of suggested LVMs
+	num_lvms=${#lvms_array[@]}
+
+	if [ "$num_lvms" -gt 1 ]; then
+		DEBUG "Multiple LVMs found: $lvm_suggest"
+		selected_lvms_not_existing=1
+		# Create an array to store the selected LVMs
+		declare -a key_lvms_array
+
+		while [ $selected_lvms_not_existing -ne 0 ]; do
+			{
+				# Read the user input and store it in a variable
+				read \
+					-p "Encrypted LVMs? (choose between/all: $lvm_suggest): " \
+					key_lvms
+
+				# Split the user input by spaces and add each element to the array
+				IFS=' ' read -r -a key_lvms_array <<<"$key_lvms"
+
+				# Loop through the array and check if each element is in the lvms_array
+				valid=1
+				for lvm in "${key_lvms_array[@]}"; do
+					if [[ ! ${lvms_array[$lvm]+_} ]]; then
+						# If not found, set the flag to indicate invalid input
+						valid=0
+						break
+					fi
+				done
+
+				# If valid, set the flag to indicate valid input
+				if [[ $valid -eq 1 ]]; then
+					selected_lvms_not_existing=0
+				fi
+			}
+		done
+	elif [ "$num_lvms" -eq 1 ]; then
+		echo "Single Encrypted LVM found at $lvm_suggest."
+		key_lvms=$lvm_suggest
+	else
+		echo "No encrypted LVMs found."
+	fi
+
+	# Create an associative array to store the suggested devices and their paths
+	declare -A devices_array
+	# Loop through the suggested devices and add them to the array
+	for device in $devices_suggest; do
+		devices_array[$device]=$device
+	done
+
+	# Get the number of suggested devices
+	num_devices=${#devices_array[@]}
+
+	if [ "$num_devices" -gt 1 ]; then
+		DEBUG "Multiple LUKS devices found: $devices_suggest"
+		selected_luksdevs_not_existing=1
+		# Create an array to store the selected devices
+		declare -a key_devices_array
+
+		while [ $selected_luksdevs_not_existing -ne 0 ]; do
+			{
+				# Read the user input and store it in a variable
+				read \
+					-p "Encrypted devices? (choose between/all: $devices_suggest): " \
+					key_devices
+
+				# Split the user input by spaces and add each element to the array
+				IFS=' ' read -r -a key_devices_array <<<"$key_devices"
+
+				# Loop through the array and check if each element is in the devices_array
+				valid=1
+				for device in "${key_devices_array[@]}"; do
+					if [[ ! ${devices_array[$device]+_} ]]; then
+						# If not found, set the flag to indicate invalid input
+						valid=0
+						break
+					fi
+				done
+
+				# If valid, set the flag to indicate valid input
+				if [[ $valid -eq 1 ]]; then
+					selected_luksdevs_not_existing=0
+				fi
+			}
+		done
+	elif [ "$num_devices" -eq 1 ]; then
+		echo "Single Encrypted Disk found at $devices_suggest."
+		key_devices=$devices_suggest
+	else
+		echo "No encrypted devices found."
+	fi
+
+	DEBUG "Multiple LUKS devices selected: $key_devices"
+
+}
+
 if [ ! -r "$TMP_MENU_FILE" ]; then
 	die "No menu options available, please run kexec-select-boot"
 fi
@@ -75,24 +186,13 @@ if [ -z "$entry" ]; then
 fi
 
 save_key="n"
+
 if [ "$CONFIG_TPM" = "y" ] && [ "$CONFIG_TPM_NO_LUKS_DISK_UNLOCK" != "y" ] && [ "$CONFIG_BASIC" != y ]; then
 	DEBUG "TPM is enabled and TPM_NO_LUKS_DISK_UNLOCK is not set"
 	DEBUG "Checking if a a TPM Disk Unlock Key was previously set up from $KEY_DEVICES"
-	if [ ! -r "$KEY_DEVICES" ]; then
+	#check if $KEY_DEVICES file exists and is not empty
-		DEBUG "No previous TPM Disk Unlock Key was set up for LUKS devices, confirming to add a disk encryption to the TPM"
+	if [ -r "$KEY_DEVICES" ] && [ -s "$KEY_DEVICES" ]; then
-		read \
+		DEBUG "TPM Disk Unlock Key was previously set up from $KEY_DEVICES"
-			-n 1 \
-			-p "Do you wish to add a disk encryption to the TPM [y/N]: " \
-			add_key_confirm
-		echo
-
-		if [ "$add_key_confirm" = "y" \
-			-o "$add_key_confirm" = "Y" ]; then
-			DEBUG "User confirmed to add a disk encryption to the TPM"
-			save_key="y"
-		fi
-	else
-		DEBUG "Previous TPM Disk Unlock Key was set up for LUKS devices"
 		read \
 			-n 1 \
 			-p "Do you want to reseal a disk key to the TPM [y/N]: " \
@@ -117,6 +217,19 @@ if [ "$CONFIG_TPM" = "y" ] && [ "$CONFIG_TPM_NO_LUKS_DISK_UNLOCK" != "y" ] && [
 			devices_suggest="$old_key_devices"
 			save_key="y"
 		fi
+	else
+		DEBUG "No previous TPM Disk Unlock Key was set up for LUKS devices, confirming to add a disk encryption to the TPM"
+		read \
+			-n 1 \
+			-p "Do you wish to add a disk encryption to the TPM [y/N]: " \
+			add_key_confirm
+		echo
+
+		if [ "$add_key_confirm" = "y" \
+			-o "$add_key_confirm" = "Y" ]; then
+			DEBUG "User confirmed to add a disk encryption to the TPM"
+			save_key="y"
+		fi
 	fi
 
 	if [ "$save_key" = "y" ]; then
@@ -134,54 +247,13 @@ if [ "$CONFIG_TPM" = "y" ] && [ "$CONFIG_TPM_NO_LUKS_DISK_UNLOCK" != "y" ] && [
 				if [ -z "$lvm_volume_group" ] && [ -n "$old_lvm_volume_group" ]; then
 					lvm_volume_group="$old_lvm_volume_group"
 				fi
+			#User doesn't want to reuse past devices, so we need to prompt him from devices_suggest and lvm_suggest
+			else
+				prompt_for_existing_encrypted_lvms_or_disks
 			fi
 		else
 			DEBUG "No previous TPM Disk Unlock Key was set up for LUKS devices, setting up new ones"
-			if [ "$num_lvm" -gt 1 ]; then
+			prompt_for_existing_encrypted_lvms_or_disks
-				DEBUG "Multiple LVM groups found"
-				# TODO: untested codepath. What uses lvm+LUKS on top nowadays?!?
-				selected_lvmdev_not_existing=1
-				while [ $selected_lvmdev_not_existing -ne 0 ]; do
-					{
-						read \
-							-p "Encrypted LVM group? choose between: '$lvm_suggest': " \
-							lvm_volume_group
-
-						result=$(echo "$lvm_suggest" | grep -q "$lvm_volume_group") || selected_lvmdev_not_existing=1
-						if [ $? == 0 ]; then
-							selected_lvmdev_not_existing=0
-						fi
-					}
-				done
-			elif [ "$num_lvm" -eq 1 ] && [ -s $lvm_suggest ]; then
-				echo "Single Encrypted LVM group found at $lvm_suggest."
-				lvm_volume_group=$lvm_suggest
-			else
-				echo "No encrypted LVM Group found."
-			fi
-
-			if [ "$num_devices" -gt 1 ]; then
-				DEBUG "Multiple LUKS devices found"
-				selected_luksdev_not_existing=1
-				while [ $selected_luksdev_not_existing -ne 0 ]; do
-					{
-						read \
-							-p "Encrypted devices? (choose between: '$devices_suggest'): " \
-							key_devices
-
-						result=$(echo "$devices_suggest" | grep -q "$key_devices") || selected_luksdev_not_existing=1
-						#if result is not empty, then the device exists
-						if [ -n "$result" ]; then
-							selected_luksdev_not_existing=0
-						fi
-					}
-				done
-			elif [ "$num_devices" -eq 1 ]; then
-				echo "Single Encrypted Disk found at $devices_suggest."
-				key_devices=$devices_suggest
-			else
-				echo "No encrypted devices found."
-			fi
 		fi
 
 		save_key_params="-s -p $paramsdev"

initrd/bin/kexec-save-key

@@ -20,7 +20,7 @@ done
 
 DEBUG "kexec-save-key prior of parsing: paramsdir: $paramsdir, paramsdev: $paramsdev, lvm_volume_group: $lvm_volume_group"
 
-shift `expr $OPTIND - 1`
+shift $(expr $OPTIND - 1)
 key_devices="$@"
 
 DEBUG "kexec-save-key: key_devices: $key_devices"
@@ -40,9 +40,9 @@ paramsdir="${paramsdir%%/}"
 DEBUG "kexec-save-key prior of last override: paramsdir: $paramsdir, paramsdev: $paramsdev, lvm_volume_group: $lvm_volume_group"
 
 if [ -n "$lvm_volume_group" ]; then
-	lvm vgchange -a y $lvm_volume_group \
+	lvm vgchange -a y $lvm_volume_group ||
-	|| die "Failed to activate the LVM group"
+		die "Failed to activate the LVM group"
-
+	#TODO: why reuse key_devices for lvm devices?
 	for dev in /dev/$lvm_volume_group/*; do
 		key_devices="$key_devices $dev"
 	done
@@ -58,22 +58,22 @@ mount -o rw,remount $paramsdev
 rm -f $paramsdir/kexec_key_lvm.txt || true
 if [ -n "$lvm_volume_group" ]; then
 	DEBUG "kexec-save-key saving under $paramsdir/kexec_key_lvm.txt : lvm_volume_group: $lvm_volume_group"
-	echo "$lvm_volume_group" > $paramsdir/kexec_key_lvm.txt \
+	echo "$lvm_volume_group" >$paramsdir/kexec_key_lvm.txt ||
-	|| die "Failed to write lvm group to key config "
+		die "Failed to write lvm group to key config "
 fi
 
 rm -f $paramsdir/kexec_key_devices.txt || true
 for dev in $key_devices; do
-	DEBUG "gettinmg uuid for $dev"
+	DEBUG "Getting UUID for $dev"
-	uuid=`cryptsetup luksUUID "$dev" 2>/dev/null` \
+	uuid=$(cryptsetup luksUUID "$dev" 2>/dev/null) ||
-	|| die "Failed to get UUID for device $dev"
+		die "Failed to get UUID for device $dev"
-	DEBUG "saving under $paramsdir/kexec_key_devices.txt : dev: $dev, uuid: $uuid"
+	DEBUG "Saving under $paramsdir/kexec_key_devices.txt : dev: $dev, uuid: $uuid"
-	echo "$dev $uuid" >> $paramsdir/kexec_key_devices.txt \
+	echo "$dev $uuid" >>$paramsdir/kexec_key_devices.txt ||
-	|| die "Failed to add $dev:$uuid to key devices config"
+		die "Failed to add $dev:$uuid to key devices config"
 done
 
-kexec-seal-key $paramsdir \
+kexec-seal-key $paramsdir ||
-|| die "Failed to save and generate key in TPM"
+	die "Failed to save and generate key in TPM"
 
 if [ "$skip_sign" != "y" ]; then
 	extparam=
@@ -81,8 +81,8 @@ if [ "$skip_sign" != "y" ]; then
 		extparam=-r
 	fi
 	# sign and auto-roll config counter
-	kexec-sign-config -p $paramsdir $extparam \
+	kexec-sign-config -p $paramsdir $extparam ||
-	|| die "Failed to sign updated config"
+		die "Failed to sign updated config"
 fi
 
 # switch back to ro mode

initrd/bin/kexec-seal-key

@@ -27,7 +27,7 @@ KEY_LVM="$paramsdir/kexec_key_lvm.txt"
 if [ ! -r "$KEY_DEVICES" ]; then
 	die "No devices defined for disk encryption"
 else
-	DEBUG "Devices defined for disk encryption: $(cat "$KEY_DEVICES" | cut -d\  -f1)"
+	DEBUG "Devices defined for disk encryption: $(cat "$KEY_DEVICES" | cut -d\  -f1 | tr '\n' ' ')"
 fi
 
 if [ -r "$KEY_LVM" ]; then

initrd/bin/qubes-measure-luks

@@ -1,21 +1,24 @@
 #!/bin/bash
-# Measure all of the luks disk encryption headers into
+# Measure all of the LUKS Disk Encryption headers into
 # a PCR so that we can detect disk swap attacks.
 . /etc/functions
 
 TRACE "Under /bin/qubes-measure-luks"
+DEBUG "Arguments passed to qubes-measure-luks: $@"
 
-die() { echo >&2 "$@"; exit 1; }
+# Measure the LUKS headers into PCR 6
-
-# Measure the luks headers into PCR 6
 for dev in "$@"; do
+	DEBUG "Storing LUKS header for $dev into /tmp/lukshdr-$(echo "$dev" | sed 's/\//_/g')"
 	cryptsetup luksHeaderBackup $dev \
-		   --header-backup-file /tmp/lukshdr-$(echo "$dev" | sed 's/\//_/g') \
+		--header-backup-file /tmp/lukshdr-$(echo "$dev" | sed 's/\//_/g') ||
-		|| die "$dev: Unable to read luks header"
+		die "$dev: Unable to read LUKS header"
 done
 
-sha256sum /tmp/lukshdr-* > /tmp/luksDump.txt || die "Unable to hash luks headers"
+DEBUG "Hashing luks headers into /tmp/luksDump.txt"
+sha256sum /tmp/lukshdr-* >/tmp/luksDump.txt || die "Unable to hash luks headers"
+DEBUG "Removing /tmp/lukshdr-*"
 rm /tmp/lukshdr-*
 
-tpmr extend -ix 6 -if /tmp/luksDump.txt \
+DEBUG "Extending PCR 6 with /tmp/luksDump.txt"
-|| die "Unable to extend PCR"
+tpmr extend -ix 6 -if /tmp/luksDump.txt ||
+	die "Unable to extend PCR"

initrd/etc/functions

@@ -83,7 +83,7 @@ confirm_totp() {
 reseal_tpm_disk_decryption_key() {
 	TRACE "Under /etc/functions:reseal_tpm_disk_decryption_key"
 	#For robustness, exit early if TPM Disk Unlock Key is prohibited in board configs
-	if [ "$CONFIG_TPM_DISK_UNLOCK_KEY" != "y" ]; then
+	if [ "$CONFIG_TPM_DISK_UNLOCK_KEY" == "n" ]; then
 		DEBUG "TPM Disk Unlock Key is prohibited in board configs"
 		return
 	else