Package: release.debian.org Severity: normal User: release.debian....@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 <mp...@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.p...@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
signature.asc
Description: Digital signature
