Merge lp:~tyhicks/ecryptfs/lp1597154-packaging into lp:ecryptfs

[Tyler Hicks]

Changes to the Debian packaging to automatically fixup existing installations which are affected by the GPT unencrypted swap partitions that are marked as auto mount.

[Jason Gerard DeRose (jderose)]

Tyler,

Reading through the code, everything looks solid.

I also (synthetically) tested your proposed postinst script using `sudo dash ecryptfs-utils.postinst configure` on an appropriately configured system with an NVMe drive on which I first deliberately unset bit 63, and it worked fine. Your postinst script likewise worked fine when running it again after bit 63 was already set. (Although note that I didn't yet test using a built package with this change, with all the real-word mechanics of the postinst script being called in exact the circumstance that it would be for normal users.)

There are a few small issue I found with https://www.shellcheck.net/ (within the code added by this merge) that should probably be addressed eventually, but nothing that I consider deal breaking.

Finally, thanks for your comment about keeping ecryptfs-utils.postinst and ecryptfs-setup-swap in-sync. A very smart call there, in my opinion.

As soon as there's a package in xenial-proposed, I'll put this change through its paces in a 100% real-world scenario.

-Jason

review: Approve

[Martin Pitt (pitti)]

I didn't realize that we didn't have a corresponding postinst snippet for un-marking those as swap partition; I did that in the vivid SRU (http://launchpadlibrarian.net/205003673/ecryptfs-utils_107-0ubuntu1_107-0ubuntu1.1.diff.gz), was that lost during merge?

Either way, thanks for bringing it back, and adjusting it for nvme!

review: Approve

[Tyler Hicks (tyhicks)]

Thanks for the reviews.

@Jason I fixed up a couple of the shellcheck warnings before merging.

@Martin Yes, it looks like your postinst snippet for vivid was lost. The way that new upstream eCryptfs releases are cut and uploaded to Ubuntu makes this a possibility for any packaging changes that are not upstreamed to lp:ecryptfs. :/

[Dustin Kirkland  (kirkland)]

@pitti, it would be great if, when you upload to Ubuntu, you also
proposal a merge against lp:ecryptfs, so that we don't lose track of
it. I try to stay on top of non-maintainer uploads, and pull those
back upstream, but occasionally these get missed. Thanks!

Dustin Kirkland
Ubuntu Product & Strategy
Canonical, Ltd.

On Tue, Jul 12, 2016 at 11:03 PM, Tyler Hicks <email address hidden> wrote:
> Thanks for the reviews.
>
> @Jason I fixed up a couple of the shellcheck warnings before merging.
>
> @Martin Yes, it looks like your postinst snippet for vivid was lost. The way that new upstream eCryptfs releases are cut and uploaded to Ubuntu makes this a possibility for any packaging changes that are not upstreamed to lp:ecryptfs. :/
> --
> https://code.launchpad.net/~tyhicks/ecryptfs/lp1597154-packaging/+merge/299605
> Your team eCryptfs is requested to review the proposed merge of lp:~tyhicks/ecryptfs/lp1597154-packaging into lp:ecryptfs.

=== modified file 'debian/changelog'
--- debian/changelog	2016-07-06 22:31:21 +0000
+++ debian/changelog	2016-07-08 21:35:34 +0000
@@ -18,6 +18,12 @@
     erroneously prompted to enter a pass-phrase to unlock their swap partition
     at boot. (LP: #1597154)
 
+  [ Tyler Hicks ]
+  * debian/ecryptfs-utils.postinst: Fix any unencrypted GPT swap partitions
+    that have mistakenly remained marked as auto mount. This should only
+    modify the swap partitions on systems that ecryptfs-setup-swap has been
+    used on. (LP: #1447282, LP: #1597154)
+
  -- Dustin Kirkland <[e-mail]>  Fri, 26 Feb 2016 18:00:18 -0600
 
 ecryptfs-utils (111) xenial; urgency=medium
 
=== modified file 'debian/ecryptfs-utils.postinst'
--- debian/ecryptfs-utils.postinst	2015-08-04 16:29:37 +0000
+++ debian/ecryptfs-utils.postinst	2016-07-08 21:35:34 +0000
@@ -66,6 +66,48 @@
 				done
 			done < /etc/crypttab
 		fi
+
+		# Fix up GPT unencrypted swap partitions that have not been
+		# marked as no-auto mounting in order to prevent systemd from
+		# using them before the encrypted swap can be initialized
+		# (LP: #1447282, LP: #1597154).
+		#
+		# IMPORTANT: Much of this code is duplicated from
+		# ecryptfs-setup-swap. Please keep the two in sync when making
+		# any changes.
+		if [ -e /etc/crypttab ] && [ -e /etc/fstab ]; then
+			while read mapper_dev phys_dev keyfile options; do
+				# only consider cryptswapN devices from ecryptfs-setup-swap
+				[ "$mapper_dev" != "${mapper_dev#cryptswap}" ] || continue
+				[ "${options#*swap,}" != "$options" ] || continue
+				# ignore devices without offset=, they would cause #953875 again
+				[ "${options%offset=*}" != "$options" ] || continue
+				# get/verify UUID
+				uuid="${phys_dev#UUID=}"
+				[ -e /dev/disk/by-uuid/$uuid ] || continue
+
+				# If this is a GPT partition, mark it as no-auto mounting, to avoid
+				# auto-activating it on boot
+				swap_dev=$(blkid -U $uuid)
+				if [ "$(blkid -p -s PART_ENTRY_SCHEME -o value "$swap_dev")" = "gpt" ]; then
+					# Correctly handle NVMe/MMC drives, as well as any similar physical
+					# block device that follow the "/dev/foo0p1" pattern (LP: #1597154)
+					if echo "$swap_dev" | grep -qE "^/dev/.+[0-9]+p[0-9]+$"; then
+						drive=$(echo "$swap_dev" | sed "s:\(.\+[0-9]\)p[0-9]\+:\1:")
+					else
+						drive=$(echo "$swap_dev" | sed "s:\(.\+[^0-9]\)[0-9]\+:\1:")
+					fi
+					partno=$(echo "$swap_dev" | sed "s:.\+[^0-9]\([0-9]\+\):\1:")
+					if [ -b "$drive" ] && \
+					   ! printf "x\np\n" | fdisk "$drive" | grep -q "^$swap_dev .* GUID:.*\b63\b"; then
+							# toggle flag 63 ("no auto")
+							echo "Marking GPT swap partition $swap_dev as no-auto ..."
+							# unfortunately fdisk fails on "cannot re-read part table" and is very verbose
+							printf "x\nS\n$partno\n63\nr\nw\n" | fdisk "$drive" >/dev/null 2>&1 || true
+					fi
+				fi
+			done < /etc/crypttab
+		fi
 	;;
 	abort-upgrade|abort-remove|abort-deconfigure)
 
 
=== modified file 'src/utils/ecryptfs-setup-swap'
--- src/utils/ecryptfs-setup-swap	2016-07-06 22:31:21 +0000
+++ src/utils/ecryptfs-setup-swap	2016-07-08 21:35:34 +0000
@@ -165,6 +165,10 @@
 
 	# If this is a GPT partition, mark it as no-auto mounting, to avoid
 	# auto-activating it on boot
+	#
+	# IMPORTANT: Much of this code is duplicated in
+	# debian/ecryptfs-utils.postinst. Please keep the two in sync when
+	# making any changes.
 	if [ "$(blkid -p -s PART_ENTRY_SCHEME -o value "$swap")" = "gpt" ]; then
 		# Correctly handle NVMe/MMC drives, as well as any similar physical
 		# block device that follow the "/dev/foo0p1" pattern (LP: #1597154)