pre-upload unblock request: systemd/215-17 for RC bug #751707

[Martin Pitt]

Package: release.debian.org
Severity: normal
User: release.debian.org at packages.debian.org
Usertags: unblock

Hello release team,

yesterday I discovered that systemd breaks a common way of setting up
plain cryptsetup partitions. Turns out that this has already been
known for a while, but the impact wasn't appreciated enough:

  http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=751707

What happens is that systemd's cryptsetup integration ignores the
"offset=" parameter in crypttab and instead uses the whole device. So
if you had a swap or other partition underneath in order to identify
the partition via UUID or label instead of an unreliable hardcoded
device name, switching to systemd destroys the underlying metadata,
and causes a boot hang as crypttab now refers to a nonexisting
UUID/label. This is quite a common way to set up encrypted swap, the
way that ecryptfs' own swap setup tool does it (the Ubuntu installer
calls that if you select "encrypt my home directory"; I'm not sure
whether Debian's installer does the same).

IMHO this qualifies as data loss, and we cannot repair this
automatically after the damage happened. So I'd really like to fix
this in jessie, and I upgraded it to RC.

The patch is quite straightforward. It got a first review by upstream,
I made it a bit more defensive since the first version, and it'll
probably land today. I attached my test script to the upstream bug [1]
which allows you to play around with various offset= options and
verify that it doesn't destroy the initial part of the partition.

I realize this is a somewhat awkward timing as we want to deep-freeze
in two days, and this means an udeb change (although only formally as
there are no effective changes in udev). 215-16 should go into testing
tonight, and I'm prepared to upload 215-17 with that fix right after
that with urgency=high.

What would you recommend how to proceed?

Thank you in advance!

Martin

[1] https://bugs.freedesktop.org/show_bug.cgi?id=87717
-- 
Martin Pitt                        | http://www.piware.de
Ubuntu Developer (www.ubuntu.com)  | Debian Developer  (www.debian.org)
-------------- next part --------------
diff --git a/debian/changelog b/debian/changelog
index 29ff5a3..103d8ce 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,10 @@
+systemd (215-17) UNRELEASED; urgency=medium
+
+  * cryptsetup: Implement offset and skip options. (Closes: #751707,
+    LP: #953875)
+
+ -- Martin Pitt <mpitt at debian.org>  Thu, 16 Apr 2015 07:12:08 -0500
+
 systemd (215-16) unstable; urgency=medium
 
   [ Christian Seiler ]
diff --git a/debian/patches/cryptsetup-Implement-offset-and-skip-options.patch b/debian/patches/cryptsetup-Implement-offset-and-skip-options.patch
new file mode 100644
index 0000000..f392bbc
--- /dev/null
+++ b/debian/patches/cryptsetup-Implement-offset-and-skip-options.patch
@@ -0,0 +1,66 @@
+From: Martin Pitt <martin.pitt at ubuntu.com>
+Date: Thu, 16 Apr 2015 06:44:07 -0500
+Subject: cryptsetup: Implement offset and skip options
+
+These are useful for plain devices as they don't have any metadata by
+themselves. Instead of using an unreliable hardcoded device name in crypttab
+you can then put static metadata at the start of the partition for a stable
+UUID or label.
+
+https://bugs.freedesktop.org/show_bug.cgi?id=87717
+https://bugs.debian.org/751707
+https://launchpad.net/bugs/953875
+---
+ src/cryptsetup/cryptsetup.c | 21 +++++++++++++++++++--
+ 1 file changed, 19 insertions(+), 2 deletions(-)
+
+diff --git a/src/cryptsetup/cryptsetup.c b/src/cryptsetup/cryptsetup.c
+index a67d85e..6257c81 100644
+--- a/src/cryptsetup/cryptsetup.c
++++ b/src/cryptsetup/cryptsetup.c
+@@ -50,12 +50,12 @@ static bool arg_discards = false;
+ static bool arg_tcrypt_hidden = false;
+ static bool arg_tcrypt_system = false;
+ static char **arg_tcrypt_keyfiles = NULL;
++static uint64_t arg_offset = 0;
++static uint64_t arg_skip = 0;
+ static usec_t arg_timeout = 0;
+ 
+ /* Options Debian's crypttab knows we don't:
+ 
+-    offset=
+-    skip=
+     precheck=
+     check=
+     checkargs=
+@@ -168,6 +168,20 @@ static int parse_one_option(const char *option) {
+                         return 0;
+                 }
+ 
++        } else if (startswith(option, "offset=")) {
++
++                if (safe_atou64(option+7, &arg_offset) < 0) {
++                        log_error("offset= parse failure, refusing.");
++                        return -EINVAL;
++                }
++
++        } else if (startswith(option, "skip=")) {
++
++                if (safe_atou64(option+5, &arg_skip) < 0) {
++                        log_error("skip= parse failure, refusing.");
++                        return -EINVAL;
++                }
++
+         } else if (!streq(option, "none"))
+                 log_error("Encountered unknown /etc/crypttab option '%s', ignoring.", option);
+ 
+@@ -403,6 +417,9 @@ static int attach_luks_or_plain(struct crypt_device *cd,
+                 } else
+                         params.hash = "ripemd160";
+ 
++                params.offset = arg_offset;
++                params.skip = arg_skip;
++
+                 if (arg_cipher) {
+                         size_t l;
+ 
diff --git a/debian/patches/series b/debian/patches/series
index 29ceab0..f708c0c 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -204,3 +204,4 @@ PrivateTmp-shouldn-t-require-tmpfs.patch
 sysv-generator-add-support-for-etc-insserv-overrides.patch
 syslog-Increase-max_dgram_qlen-by-pulling-in-systemd.patch
 Skip-filesystem-check-if-already-done-by-the-initram.patch
+cryptsetup-Implement-offset-and-skip-options.patch
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://alioth-lists.debian.net/pipermail/pkg-systemd-maintainers/attachments/20150416/aecaa416/attachment-0002.sig>