[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#995108: d-i: partman-crypto: plain dm-crypt device management issues



Package: partman-crypto
Version: 114
Tags: patch

Hello d-i team,

I found several issues related to plain dm-crypt (i.e. non LUKS) encrypted devices in partman.

1) Partman does not add option "plain" for non LUKS devices in crypttab.
This causes warnings at startup and when running update-initramfs:

"cryptsetup: WARNING: sda10_crypt: couldn't determine device type, assuming default (plain)"

2) Partman adds the option "swap" in crypttab entry for encrypted devices used as swap regardless of the key type. This option is ony needed with random key and uselessly formats persistent encrypted devices (LUKS or plain dm-crypt with keyfile). Fortunately The initramfs ignores it and won't destroy an hibernation image before it is used.

3) Partman adds the option "tmp" in crypttab entry only if the mount point is /tmp. The option should be added for any encrypted device with random key used as a filesystem even if the mount point is not /tmp.

4) Partman does not specify the filesystem type in option "tmp".
This defaults to format as ext4 at system startup. However partman allows only ext2 and sets the filesystem type as ext2 in fstab, causing a mismatch at startup

I propose to set tmp=<filesystem> and allow any POSIX filesystem type (non POSIX filesystems are not suitable for /tmp).

5) Partman uses LUKS UUID in crypttab regarless of its own keytype info.
If a partition containing a LUKS header is re-used as plain dm-crypt, partman will wrongly write the old LUKS UUID in crypttab regardless of the keytype info.
It should use the LUKS UUID only if keytype=passphrase.

6) Partman uses non persistent physical device names for plain dm-crypt in crypttab.

Unlike LUKS, plain dm-crypt physical volumes do not have a UUID so they are designated with their block device name in the generated crypttab.

This is fine with LVM logical volumes for which persistent device names /dev/mapper/* are used. However some devices such as SCSI/ATA/USB drives and partitions (/dev/sd*) or software RAID arrays (/dev/md*) may not be assigned the same names across boots. I have neither experience or knowledge about SD/MMC drives, NVMe SSDs nor hardware RAID arrays. Partition numbers are not so reliable either; for example, logical partitions may be renumbered after creating or deleting another logical partition.

Using non persistent physical device names may lead to boot failure if the device name in /etc/crypttab does not exist or even more catastrophic consequences if the device name is assigned to the wrong device which will be overwritten. This has happened to me and others (cf. bug #697905).

For this reason IMO /etc/crypttab should use a persistent identifier whenever possible. For instance it could use the PARTUUID for partitions on disk labels which provide it, or a symlink in /dev/disk/by-id/ like the grub-pc package does to record installation devices in debconf:

The attached patch makes the following changes:

/lib/partman/finish.d/55crypto_config
- add option "plain" in crypttab for non-LUKS devices
- add option "swap" in crypttab only for devices with random key
- add option "tmp" in crypttab even if mountpoint is not /tmp
- add explicit filesystem to option "tmp" in crypttab
- use partman info instead of only cryptsetup to check if LUKS device before retrieving LUKS UUID - use PARTUUID or /dev/disk/by-id/* if available instead of non-persistent device file for plain devices in crypttab

Discussion: is this adequate for RAID devices /dev/md* ? md* names should be rather persistent when defined in /etc/mdadm/mdadm.conf, and have two symlinks in /dev/disk/by-id/*: one based on the array name attribute and the other based on the array UUID. The one based on the name attribute is selected by the patch because it is found first, but the name attribute is derived from the hostname at creation time and may lack uniqueness.

Discussion: I used blkid to retrieve the PARTUUID because parted_server does not seem to provide this feature. Is blkid supported by non-Linux architectures ?

/lib/partman/veto_filesystems/crypto
- allow all POSIX filesystems on encrypted devices with random key

Discussion: is getting the filesystem type from $id/filesystem the right way of doing things ? It does not work with FAT16 and FAT32, so I excluded them (for not being POSIX compliant thus unusable as /tmp).
diff -ur a/finish.d/crypto_config b/finish.d/crypto_config
--- a/finish.d/crypto_config	2021-05-30 23:37:15.000000000 +0200
+++ b/finish.d/crypto_config	2021-09-26 00:58:47.912184745 +0200
@@ -26,7 +26,7 @@
 				return 1
 			fi
 		done
-		opts="cipher=$cipher-$ivalgorithm,size=$keysize"
+		opts="plain,cipher=$cipher-$ivalgorithm,size=$keysize"
 		if [ $keytype != random ] && [ -n "$keyhash" ]; then
 			opts="$opts,hash=$keyhash"
 		fi
@@ -49,10 +49,10 @@
 	if [ -f $cryptdevdir/mountpoint ]; then
 		mnt=$(cat $cryptdevdir/mountpoint)
 	fi
-	if [ $method = swap ]; then
+	if [ $method = swap ] && [ $keytype = random ]; then
 		opts="$opts,swap"
-	elif [ "$mnt" = /tmp ] && [ $keytype = random ]; then
-		opts="$opts,tmp"
+	elif [ -n "$mnt" ] && [ $keytype = random ]; then
+		opts="$opts,tmp=$(cat $cryptdevdir/filesystem)"
 	fi
 
         # Allow TRIM operations
@@ -65,9 +65,22 @@
 	source=$realdev
 
 	# Use UUID for LUKS devices
-	if cryptsetup isLuks "$source"; then
+	if [ $keytype = passphrase ] && cryptsetup isLuks "$source"; then
 		local uuid=$(cryptsetup luksUUID "$source")
 		source="UUID=$uuid"
+	elif [ ${source##/dev/mapper/*} ]; then
+		# try to use PARTUUID
+		if partuuid=$(blkid -s PARTUUID -o value $source) && [ $partuuid ]; then
+			source="PARTUUID=$partuuid"
+		else
+			# try to use persistent symlink
+			for s in /dev/disk/by-id/* ; do
+				if [ $(mapdevfs $s) = $source ]; then
+					source=$s
+					break
+				fi
+			done
+		fi
 	fi
 
 	# Add entry to crypttab
diff -ur a/veto_filesystems/crypto b/veto_filesystems/crypto
--- a/veto_filesystems/crypto	2021-05-30 23:37:15.000000000 +0200
+++ b/veto_filesystems/crypto	2021-09-26 00:59:56.217897313 +0200
@@ -21,10 +21,12 @@
 		keytype=$(cat $cryptodev/keytype)
 
 		if [ $keytype = random ]; then
-			# Veto anything but ext2
+			# Veto fat, ntfs (non POSIX)
 			for fs in $filesystems; do
 				case $fs in
-				    ext2) echo $fs ;;
+				    *fat*) ;;
+				    ntfs*) ;;
+				    *) echo $fs ;;
 				esac
 			done
 			return 0

Reply to: