Overwrite/destroy not-empty partition due to lack of vol_id from udev

Bug #474327 reported by Sergey Dolgov
18
This bug affects 2 people
Affects Status Importance Assigned to Milestone
cryptsetup (Ubuntu)
Invalid
Low
Unassigned
Karmic
Won't Fix
High
Unassigned

Bug Description

Binary package hint: cryptsetup

SRU update rationale:
1. impact of the bug on users:
Bugs which may, under realistic circumstances, directly cause a loss of user data.
Bug can destroy entire partition on user system while booting, if the user change hard drivers order.

2. how the bug has been addressed
In Lucid we are using blk_id, which works.
In Karmic the solution is different - the attached patch makes no_vol_id return failure instead of success if vol_id program is missing, therefore it will stop the overwrite of user partition.

3. A minimal patch applicable to the stable version
the one-liner patch is attached

4. Detailed instructions how to reproduce the bug
Was described below by the reporter

5. A discussion of the regression potential of the patch
Very unlikely IMHO.
It just stops overwriting existing partition and using it to create without a question encrypted swap

----

/lib/cryptsetup/checks/{un_,}vol_id are supposed to check for a type of file system on a disk volume. Functions from /lib/cryptsetup/cryptdisks.functions use those checks to determine whether it is safe to destroy the contents of a volume by e.g calling luks create on it:

    PRECHECK="/lib/cryptsetup/checks/un_vol_id"

    [...]

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

    [...]

     cryptsetup $PARAMS create "$dst" "$src"

/lib/cryptsetup/checks/{un_,}vol_id rely on /lib/udev/vol_id from the udev package. In Karmic, vol_id it is no longer present. Most unfortunately, in this case the checks *pass* with mere warning:

if test ! -x "/lib/udev/vol_id"; then
  echo " - WARNING: vol_id from udev is not available, impossible to run checks."
  exit 0
fi

I would argue that exit 0 should be exit 1 instead, otherwise it can lead to silent data corruption in case the disks connected to the machine change. Here is how it happend to me:

I installed Karmic on HDD1; at that time it was the only drive in the box, and thus it was detected as sda. The installer created this entry in /etc/crypttab:

cryptswap1 /dev/sda3 /dev/urandom swap,cipher=aes-cbc-essiv:sha256

After that, I connected my second drive, HDD2, to the box. It happend to be connected to the first port of the SATA controller, so when I booted off HDD1, hard drive were detected as follows: HDD2: sda, HDD1: sdb. As a result, my ext3 partition on HDD2 ("new" sda3) became corrupted because of missing vol_id in udev and this bug.

It looks like the move from vol_id to blkid from util-linux is uderway; Debian already has /mnt/lib/cryptsetup/checks/blkid, but the same problem is present there too:

if test ! -x "/sbin/blkid"; then
  echo " - WARNING: blkid from util-linux is not available, impossible to run checks."
  exit 0
fi

which means data corruption if blkid is missing and your disks changed since the time /etc/crypttab was created.

Revision history for this message
LimCore (limcore) wrote :

This is serious bug.

Also if "old" script with no_vol_id fail (missing command) then swap can be NOT enabled, leading to DDoS of server that was configured so that it can not handle all applications using just RAM

security vulnerability: no → yes
Revision history for this message
LimCore (limcore) wrote :

Lack of no_vol_id can result in server not having any SWAP, allowing easy DoS or just resulting in oom-killers...

Changed in cryptsetup (Ubuntu):
status: New → Confirmed
Revision history for this message
LimCore (limcore) wrote :

There should be IMHO alias or stub command that provides no_vol_id by callking blkid .. with proper flag or something

Steve Langasek (vorlon)
security vulnerability: yes → no
Revision history for this message
C de-Avillez (hggdh2) wrote :

Setting Importance to High. If the description is correct (and I see no reason not to), an error like this, although rare, will most probably result in lost data.

Changed in cryptsetup (Ubuntu):
importance: Undecided → High
Revision history for this message
LimCore (limcore) wrote :

Trivial patch to fix this problem - return 1 (fail no_vol_id) if it is not save to overwrite, because we can not check if vol is "empty" (no fs) or not due to lack of vol_id program

summary: - /lib/cryptsetup/checks/{un_,}vol_id should fail if vol_id from udev is
- not available
+ Overwrite/destroy not-empty partition due to lack of vol_id from udev
Revision history for this message
Steve Langasek (vorlon) wrote :

This is fixed in lucid, where cryptsetup uses /sbin/blkid instead of /lib/udev/vol_id. Because /sbin/blkid has been part of the base system since at least hardy, there is also no upgrade concern where the program will be missing. So the severity of this is greatly reduced for lucid; downgrading the importance.

Changed in cryptsetup (Ubuntu):
importance: High → Medium
tags: added: patch
LimCore (limcore)
description: updated
description: updated
Revision history for this message
LimCore (limcore) wrote :

Steve, btw, I believe similar patch should be applied to Luci, if it would happen that for some reason blkid would be not available or not able to run.

Anyway, please push this update to Karmic?

Revision history for this message
John Dong (jdong) wrote :

ACK from -SRU for Karmic SRU, but make sure you target karmic-proposed, not karmic, when uploading.

In Lucid, I agree with Steve that the bug is not as critical, but nonetheless it is still worthwhile to fix.

Martin Pitt (pitti)
Changed in cryptsetup (Ubuntu Karmic):
importance: Undecided → High
Changed in cryptsetup (Ubuntu):
status: Confirmed → Triaged
importance: Medium → Low
Revision history for this message
Martin Pitt (pitti) wrote : Please test proposed package

Accepted cryptsetup into karmic-proposed, the package will build now and be available in a few hours. Please test and give feedback here. See https://wiki.ubuntu.com/Testing/EnableProposed for documentation how to enable and use -proposed. Thank you in advance!

Changed in cryptsetup (Ubuntu Karmic):
status: New → Fix Committed
tags: added: verification-needed
Revision history for this message
Martin Pitt (pitti) wrote :

Anyone who can test the proposed package? If not, the proposed update will be removed again. Thanks!

Revision history for this message
Martin Pitt (pitti) wrote :

This update has been in karmic-proposed for half a year or longer, without any testing feedback. I removed the proposed package again.

Changed in cryptsetup (Ubuntu):
status: Triaged → Invalid
Changed in cryptsetup (Ubuntu Karmic):
status: Fix Committed → Won't Fix
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.