Extremely dangerous! cryptswap killed my partition

Bug #474258 reported by Whitehat
120
This bug affects 19 people
Affects Status Importance Assigned to Milestone
cryptsetup (Debian)
Fix Released
Unknown
cryptsetup (Ubuntu)
Fix Released
Critical
Ubuntu Foundations Team
Nominated for Lucid by Flávio Etrusco
Precise
Fix Released
Critical
Ubuntu Foundations Team

Bug Description

Binary package hint: cryptmount

9.10 installed with encrypted "home".
Had root on /dev/sda1, swap on /dev/sda2, and manually created "data" partition on "/dev/sda3"
When I deleted /dev/sda2 partition (wanted to move swap to the second HDD) - ubuntu killed my "data" partition!
I suppose the problem is that /dev/sda3 became /dev/sda2 and the cryptswap utility just killed all the data (about 80 gigs!), because /dev/sda2 is in the /etc/crypttab file as a swap partition...
Cryptswap should check the type of partition before mounting it as swap.

Related branches

security vulnerability: yes → no
visibility: private → public
Revision history for this message
RW Penney (rwpenney) wrote :

This bug report really does not sound like it refers to the "cryptmount" package.

Specifically, "cryptmount" does not use the /etc/crypttab file for any purpose, nor does it include any utility called "cryptswap" (even though if offers similar generic functionality).

More generally, it would appear that the problem is a side effect of repartitioning the main hard disk and not updating configuration files that refered to existing partitions that had been reassigned. It doesn't sound realistic for repartitioning tools to attempt to inform all applications that depend on a particular partition being available that it has been renamed. This is surely the responsibility of the system administrator.

Naturally, I commiserate with the bug-submitter about the possible loss of 80GB of data.

However, the proposed solution of checking partition-types before "mounting" as swap does not seem to offer an effective protection against corrupting data held on raw devices, especially via loopback devices which don't directly have "types" in the same way as disk partitions. Similar risks would seem to apply to any system-management tool which writes to raw devices.

Revision history for this message
Whitehat (i-whitehat) wrote :

"However, the proposed solution of checking partition-types before "mounting" as swap does not seem to offer an effective protection against corrupting data held on raw devices"
What's the problem to mount raw device in read-only mode, check what type the unencrypted partition has? Any linux distribution, including ubuntu 9.04, just shows an error "swapon: /dev/sda2: read swap header failed: Invalid argument" and I expected such behavior. But only Ubuntu 9.10 destroys the partition. I think it would be better to change such behavior, because many users can get into trouble with it

Revision history for this message
RW Penney (rwpenney) wrote :

As a reminder - your bug report does NOT relate to the "cryptmount" package given details in your original post.

The problem with running "swapon" with an encrypted partition is simply that both the /etc/crypttab mechanisms, and those of other encryption tools, perfectly reasonably regenerate the swap partition every time the system is rebooted. This is because one usually doesn't need to preserve the contents of an encrypted swap partition, and usually doesn't want to have to enter a password before being able to use the swap partition. So it is quite appropriate for encrypted swap partitions to use a new encryption key every time the system is rebooted, and to run "mkswap" on each reboot. The changing of the encryption key means that it wouldn't be feasible to "swapon" the pre-reboot state of the partition. Furthermore, the post-reboot state of the partition will actually only be accessed indirectly by either a loopback device or a device-mapper target. Neither of those indirect devices is likely to have a well-defined partition type analogous to a real disk partition.

As before, I agree that it is frustrating that there wasn't a safety net in the encryption tool you chose to use, but I don't think that's any different from "mkswap" or many other system-administration tools which can easily cause catastrophic damage with a single unlucky keystroke.

I'd recommend that this if this issue is treated as a bug, it is re-parented to a more appropriate package.

Revision history for this message
Whitehat (i-whitehat) wrote :

In this case this encryption tool can at least check the size of mounted partition. In my case, swap was 2g, and destroyed partition was 80g. I'm sure, Ubuntu develepers will find the way to fix this bug.
"but I don't think that's any different from "mkswap" or many other system-administration tools which can easily cause catastrophic damage with a single unlucky keystroke."
They are different. When root uses mkswap - he knows he can make damage to the FS, but when I deleted a swap partition - I didn't even think that this can lead to data loss...

affects: cryptmount (Ubuntu) → cryptsetup (Ubuntu)
Revision history for this message
Brian Burch (brian-pingtoo) wrote :

I sort-of agree with everyone here...
a) I have a 100% raid server running karmic (without cryptswap). I don't trust automated tools on this system and always verify that /etc/fstab points to the correct swap md partition after any significant change and before I reboot.
b) I have a laptop with hardy, jaunty and karmic installed. The swap partitions for hardy and jaunty are defined in fstab by uuid, so they cannot destroy the wrong partiton. The karmic fstab points swap to /dev/mapper/cryptswap1 and /etc/crypttab points to /dev/sda10.... DANGER WILL ROBINSON!

I don't think this implementation is satisfactory for "ordinary" users. Why doesn't an existing encrypted swap partition have an externally visible uuid? That would solve the "ordinary" user issue (I shudder to think of the pain this "bug" is going to cause if it left as-is) in an analogous fashion to the non-encrypted swap case. After all, some of my partitions have a uuid wrapper for the md superblock, then a uuid wrapper for an lvm physical volume, then one for an lvm lovical volume which holds an ext3 file system that holds the user data.

Is it possible - I don't know enough to know how to do it.

tags: added: dataloss
Revision history for this message
JohnnyMcArthur (mailings) wrote :

Hi there

You mentioned, this bug was assigned to the wrong package. Is there a way to reassign the bug or should one of us open a new bug?

I agree, that this is a very serious bug. /etc/crypttab should use UUID's in order to be shure and programs using it schould at least check the partition type.

In my case, i installed Ubuntu 11.04 for test purposes in a multi-boot environment on a second hard drive. it silently used the old swap partition on the first drive as an encrypted swap partition for this test installation - i never added the device manually to /etc/crypttab. After deleting one - completely unrelated partition !! - a third partition was damaged after the next reboot from this test installation (days afterwards). And i am even a rather experienced user. Not to think about "ordinary" user...

Revision history for this message
RW Penney (rwpenney) wrote : Re: [Bug 474258] Re: Extremely dangerous! cryptswap killed my partition

Hello Johnny,

Judging by the Ubuntu bug listings at
https://bugs.launchpad.net/ubuntu/+source/cryptsetup/+bugs?field.status:list=NEW,
it would appear that the bug has already be reassigned to cryptsetup,
and it doesn't appear in the listing of bugs in cryptmount.

I'm still not sure there's a particularly clean solution to this problem,
and all of the ideas suggested so far against this bug seem vulnerable
to quite mundane special cases (e.g. using a large swap file temporarily
installed via a loopback device.)

Your UUID proposal may be part of a better solution, but seems to require
a change in the format of swap partitions themselves. That might
involve ensuring that encrypted swap partitions always have a header in
which a UUID is readable *before* decryption (as with the LUKS format).
However, that could require quite widespread changes in the kernel, mkswap, etc.

The (partial) solution I'm trying in 'cryptmount' is to look at the degree
of randomness (entropy) of a partition before running mkswap. Only if the
raw data on the partition, or its decrypted version, doesn't look totally
random or totally featureless, will mkswap be allowed to run. This feature
is currently part of the beta release of cryptmount-4.2. I'm hoping this
will significantly reduce the risk of accidental dataloss in a way similar
to the /etc/crypttab issue, even if it can't eliminate it entirely.

Kind regards,

RW Penney

Revision history for this message
Whitehat (i-whitehat) wrote :

Well, yes, it's been only two years, and ubuntu still destroys data during installation. Maybe two years later someone we'll fix this bug. Personally, I switched to W7, because my data are more expensive than the MS license

Revision history for this message
Steve Langasek (vorlon) wrote :

I agree with Johnny's analysis here; any crypttab setup done via the Ubuntu installer should use UUIDs, not partition names, since the latter are not guaranteed to be stable.

I disagree with the original suggestion to check the partition type, as I don't think that's a reliable check in general. It could prevent this issue for some users, but for others it would just cause silent failures to enable crypted swap at boot. And if crypttab were using UUIDs to begin with, this should be a non-issue.

I'm sorry it's taken so long for this bug to come to come to anyone's attention; unfortunately I don't think anyone was tracking cryptsetup bug reports at the time it was filed, and afterwards it seems to have been lost in the mass of untriaged bugs. Now that it's on our radar, we should be able to fix this in much less than two years... :/

Surbhi, do you think you could take a look at this to make sure the installer is using UUIDs anywhere it touches /etc/crypttab, and possibly fix cryptsetup to migrate /etc/crypttab from partition names to UUIDs on upgrade? (There's code about that's been used to do this in various other packages before, for /etc/fstab and the like.) This probably also needs checking to make sure UUIDs *can* be used for crypted random swap; it's possible that the UUID gets wiped when using randomly-keyed crypt for swap.

Changed in cryptsetup (Ubuntu):
assignee: nobody → Surbhi Palande (csurbhi)
importance: Undecided → Critical
status: New → Triaged
Revision history for this message
Tuomas Heino (iheino+ub) wrote :

LP: #584067 and LP: #710340 may be duplicates of this as well.
For the special case of cryptswap on LVM, /dev/mapper/* are stable unlike the /dev/dm-* used at least in lucid...natty, and as such those could be considered in addition to UUIDs.

Changed in cryptsetup (Ubuntu):
assignee: Surbhi Palande (csurbhi) → Ubuntu Foundations Team (ubuntu-foundations-team)
Steve Langasek (vorlon)
tags: added: rls-mgr-p-tracking
Revision history for this message
Max (maxwux) wrote :

this bug taged "Critical"
but why not fix this???
two years....so long.....

Steve Langasek (vorlon)
tags: added: rls-p-tracking
Steve Langasek (vorlon)
Changed in cryptsetup (Ubuntu):
milestone: none → ubuntu-12.04
Changed in cryptsetup (Debian):
status: Unknown → New
Revision history for this message
Cezary Baginski (cezary0) wrote :

I had this continue trashing my new partition at every reboot and I couldn't track the cause ...

One idea/workaround with the UUID:

 - have cryptsetup allow only files or UUIDs instead of devices until UUID support is in the swap filesystem, e.g.:

     partition ->
         "containing" ext3 filesystem with a UUID ->
              file used as encrypted swap

This way UUID can be used even now.

While there is another level of indirection (file -> fs), the security is a performance tradeoff anyway.

The logic for cryptsetup could be changed for now to:

 - create a swap partition with an ext2 filesystem with a swap file taking all the space

 - allow only UUIDs in crypttab and expect a single file at that location

This would allow an easy transition once UUID support is present and even "upgrading" the file to a UUID-marked cryptswap partition.

Revision history for this message
Tuomas Heino (iheino+ub) wrote :

Another alternative (besides using a file on some fs) would be using LVM volumes, and not resolving the related symlinks.
Then /etc/crypttab would look something like:
cryptswap1 /dev/mapper/swap-swap_1 /dev/urandom swap,cipher=aes-cbc-essiv:sha256

where /dev/mapper/swap-swap_1 is a symlink maintained by device mapper. It should NOT be resolved to target of the symlink (like /dev/dm-1) before writing to crypttab, since target of that symlink is dynamic.

Revision history for this message
Cezary Baginski (cezary0) wrote :

@Tuomas:

I agree - LVM is a cleaner solution (although at first, I thought the lvm handling would need too much code).

Good points about linking and /dev/dm*.

@bugfix

It turns out there is an obvious bug with detecting filesystems before running mkswap.

My take on fixing it: https://code.launchpad.net/~cezary0/cryptsetup/bugfix474258

The patch probably applies cleanly to almost every version, so you can apply it locally already.

Revision history for this message
Cezary Baginski (cezary0) wrote :

I have a patch and a workaround.

PATCH
======
I refactored the code to expose the bug and make it easier to test edge cases:

    https://code.launchpad.net/~cezary0/cryptsetup/bugfix474258_refactored

If anyone can think of other/related issues, edge cases or useful test cases - let me know and I'll try to add the tests, so we can nail this once and for all. One I'm still thinking about is when an encrypted partition takes the same device name as the crypttab tmp entry - you might loose that encrypted partition even with the patch (workaround below).

I did consider some weird cases where a new partition contains 2 filesystem headers (reported by wipefs), that can confuse blkid, but this is probably too obscure and statistically improbable or not dangerous.

WORKAROUND (identify by partition size instead of label or UUID)
=====================================================
If you want to protect yourself from possibly loosing data, you can do the following:

1. Add "precheck" option to every swap entry in crypttab
2. set CRYPTDISKS_PRECHECK in /etc/default/cryptdisks to a script returning a nonzero value for any device that has data on it.
3. patch cryptdisks.functions, because the condition is just wrong

Example:

1. add "precheck" in /etc/crypttab:
cryptswap2 /dev/sda28 /dev/urandom swap,precheck,cipher=aes-cbc-essiv:sha256

2. set variable in /etc/default/cryptdisks:
CRYPTDISKS_PRECHECK=/usr/local/lib/chk_size

3. sudo mkdir -p /usr/local/lib/chk_size

4. sudo chmod 0755 /usr/local/lib/chk_size

5. discover the size of the partition you want to use:

  sudo sfdisk -s /dev/sda28 # gives partition size

6. create and edit /usr/local/lib/chk_size:

    #!/bin/sh
    EXPECTED_SIZE=2097152 # set to above size
    [ "$(sfdisk -s "$1")" -eq "$EXPECTED_SIZE" ]

7. patch /lib/cryptsetup/cryptdisks.functions:

# Fix it like this (changes: "" -> "!("; "!=" -> "="; "! /lib" -> "/lib"; "null;" -> "null);" ):

 if ! pre_out=$("$PRECHECK" "$src" 2> /dev/null) && \
    ! ( [ "$MAKESWAP" = "yes" ] && \
    /lib/cryptsetup/checks/blkid "$src" swap >/dev/null); then
  log_warning_msg "$dst: the precheck for '$src' failed: $pre_out"
  return 1

# NOTE: paste to replace the whole section, because it's too easy to make a mistake

P.S. Use backups / RAID / etc anyway - it ultimately costs less money and time.

Revision history for this message
Cezary Baginski (cezary0) wrote :

Errata:

WRONG:
    3. sudo mkdir -p /usr/local/lib/chk_size

SHOULD BE:
    sudo mkdir -p /usr/local/lib

Changed in cryptsetup (Debian):
status: New → Fix Committed
Revision history for this message
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package cryptsetup - 2:1.4.1-2ubuntu2

---------------
cryptsetup (2:1.4.1-2ubuntu2) precise; urgency=low

  * Split up package in cryptsetup and cryptsetup-bin. (LP: #343363).
  * Do not overwrite existing filesystems when creating swap (LP: #474258).
  * Add aesni module when we have hardware encryption.
 -- Jean-Louis Dupond <email address hidden> Mon, 12 Mar 2012 10:14:30 +0100

Changed in cryptsetup (Ubuntu Precise):
status: Triaged → Fix Released
Changed in cryptsetup (Debian):
status: Fix Committed → Fix Released
Revision history for this message
Whitehat (i-whitehat) wrote :

>Well, yes, it's been only two years, and ubuntu still destroys data during installation. Maybe two years later someone we'll fix this bug. Personally, I switched to W7, because my data are more expensive than the MS license

Oh, OK. Only one more year have passed :D
No comments....

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.