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

Bug#782712: marked as done (unblock: systemd/215-17)

Your message dated Wed, 22 Apr 2015 13:46:28 +0100
with message-id <23db45b6ff07d65dc43d0ec938aeb695@mowgli.jungle.funky-badger.org>
and subject line Re: Bug#782712: pre-upload unblock request: systemd/215-17 for RC bug #751707
has caused the Debian Bug report #782712,
regarding unblock: systemd/215-17
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact owner@bugs.debian.org
immediately.)


-- 
782712: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=782712
Debian Bug Tracking System
Contact owner@bugs.debian.org with problems
--- Begin Message --- 
Package: release.debian.org
Severity: normal
User: release.debian.org@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)

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@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@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

Attachment: signature.asc
Description: Digital signature


--- End Message ---
--- Begin Message --- 
On 2015-04-17 18:22, Martin Pitt wrote:

Hello Niels,

Niels Thykier [2015-04-17 17:55 +0200]:
Just to clarify, are we still intending to do a systemd update prior to 
Jessie with -17 and then now also a p-u (i.e. for 8.1) for ecryptfs?


That's still my intent, yes, primarily to avoid people who have this
set up in wheezy already (#751707 has at least two reporters) upgrade
and find their swap partition gone and boot stuck.
After some discussion on IRC, -17 has been migrated as part of the d-i "RC4" prep. 

Regards,

Adam

--- End Message ---
Reply to: