un_blkid is not a suitable precheck for plain dm-crypt 'tmp' or 'swap'
devices due to the potential for false positives from previous runs
on the same device.  This bug potentially leads to information disclosure
in some configurations.

I had an example of this today:

root@host:~# grep tmp /etc/crypttab
tmp             /dev/vgroup/tmp              /dev/urandom    size=256,cipher=aes-xts-plain,tmp=btrfs
root@host:~# cryptdisks_start tmp
Starting crypto disk...tmp (starting)...
tmp: the precheck for '/dev/vgroup/tmp' failed: - The device /dev/vgroup/tmp contains a filesystem type hfs. ... (warning).
tmp (failed)...failed.
root@host:~# blkid /dev/vgroup/tmp
/dev/vgroup/tmp: UUID="dba39fe4-922e-3fc4-963c-835245a69787" LABEL="0(M-G^W^Yr>~M-2 m{lM- M-8tM-^L^Z0 [nM-BM-^Y))M-^TvM-rM-;tuM-^O^CM-^YM-T'M-^\M-`xM-^]M-eM-I;M-&9M-^[M-`y^\M-\M-^UM-O<M-IsM-LBtM-9M-$1M-^M" TYPE="hfs"

'/dev/vgroup/tmp' contained an encrypted filesystem with a random key
(as it always does).  On the last run, the encrypted data matched the
blkid logic for an HFS filesystem.  The system involved proceeded to boot
using the root filesystem for /tmp, resulting in /tmp files written to
storage without encryption.

severity 762297 minor
tag 762297 wontfix
thanks

Hi Zygo,

thanks for your bugreport.

Am 20.09.2014 um 22:07 schrieb Zygo Blaxell:

That's really unfortunate. To my knowledge, you're the first one who hit
this issue so far.

Unfortunately, no better solution than un_blkid is known to me to
prevent serious data loss in case of device rearrangement or
missconfiguration with plain dm-crypt devices and automated swap or
tmpfs creation. Thus I'll mark the bug as wontfix.

In case that you really know what you're doing, you can set
precheck=/bin/true in the crypttab and prevent the precheck for
particular plain dm-crypt devices that way.
configurations.

This is true for encrypted tmp filesystems if the rootfs is not
encrypted. Still, the boot scripts print clear error messages and the
boot process should fail in that case anyway. It should be obvious that
things don't work as expected in that case ;)

Kind regards,
 jonas

Yay I win!  ;)

Out of every million attempts to create a tmp filesystem there should
be about 31 failures.  I won't be the last.

There are at least three ways to fix this:

1.  Create an encrypted tmpfs header.  A simple static header (or a
stripped down LUKS header) can be unambiguously recognized by blkid or
even cryptdisks itself.  cryptdisks can offset the encrypted payload
by the length of that header.  Since the data is ephemeral there is no
backward compatibility issue.  For upgrades, run the existing un_blkid
check (it is the best we have so far, even though it's still bad),
then install the header when a few tmp FS is created and use the header
from then on.

2.  When the underlying devices are identified by UUID (or through some
layer that uses UUID, such as LVM or md-raid) the precheck can be skipped,
since rearranging devices will not change their UUIDs.  UUID has enough
bits to avoid random collisions.  un_blkid could detect this case the
same way lsblk does, and ignore false positives from blkid.

3.  blkid could be more strict about identifying a HFS filesystem to
avoid false positives.  It looks like blkid detects HFS after inspecting
only 16 bits of the block device, and it ignores implausible superblock
parameter values (there are plenty it could be checking).  HFS is an
obsolete filesystem, so the probability of encountering a real HFS in
the field is certainly less than the probability of a false positive
ID in random data.  Maybe the bug should be reassigned to the blkid
(util-linux) package.  ;)

This is what I have had to do.  Changing both CRYPTDISKS_CHECK and
CRYPTDISKS_PRECHECK to /bin/true in /etc/default/cryptdisks was not
sufficient.

That is not what happens.  There are error messages, but the boot process
continues past them.  The boot could be stopped by a fsck failure when
mounting the tmp filesystem, except that the fsck would normally be
skipped in that case (why would you fsck a filesystem you just created?).
The system operates normally except for leaking data and having the wrong
filesystem on /tmp.

If we weren't auditing the systems regularly, the first sign of trouble
in production would probably be an unexpected ENOSPC on /.

Hey Zygo,

thanks for your feedback. See my comments inline below.

Am 02.10.2014 um 16:43 schrieb Zygo Blaxell:

Mh, interesting idea. You mean to prefix the encrypted data by an
identifying header on the plain device? The precheck could skip the
un_blkid check in case that this header is detected.

While this solution would be reliable for plain dm-crypt encrypted tmpfs
and swap partitions, it wouldn't work for any other plain dm-crypt
encrypted partitions - especially as users do use plain dm-crypt exactly
for the reason that it doesn't write an identifying header (plausible
deniabilty). That's my biggest argument against this solution.

Feel free to provide patches that implement that solution nevertheless.
I would be ok with adding these headers at default tmpfs and swap device
creation/detection logic as long as they and their impact are documented
properly.

While that one might be easier to implement, plain dm-crypt devices that
don't provide a UUID would remain prone to the bug you discovered.
Container files (with loopback) and physical partitions on MSDOS
partition table come to my mind.

To me, the solution sound like a lot of added complexity compared to the
gain.

That's the solution I like the most, it's the easiest one for me ;)

No, jokes aside. If detection of HFS actually can be improved in blkid,
then that should be done, regardless whether one of your proposed
solutions from above will be added to the cryptsetup package or not.

I'm sure that patches to util-linux package are appreciated :)

You're correct. In case that checkfs.sh doesn't (try to) process the
missing filesystem, boot process isn't stopped. Mountall.sh seems to
warn about missing filesystems, but doesn't panic out. While I'm not
sure whether I like that, it's out of scope for this bugreport ;)

Kind regards,
 jonas

Yes.  The presence of the header would state positively that the
partition is intended for use with a tmpfs, as opposed to relying on
each of the growing list of blkid tests to provide a negative result
when fed random data.

It fits well with tools that rely on blkid to identify partitions.
tmpfs/swap encrypted partitions are currently not recognized
automatically, but everything else is (except for plain dm-crypt
non-tmpfs, which is intentionally unrecognizable).

I wouldn't expect the new header to be used for non-tmpfs partitions.
cryptdisks_start would not need to format the partition, and once the
partition is correctly decrypted it will most likely have a signature
that can be recognized by blkid (not to mention the mount or swapon).
We already have LUKS when there is a need for identification prior
to decryption.

The new header would mean "this partition is safe to format and use for
irretrievably encrypted data with a random key" in a way that blkid
can distinguish from other recognized parition types with certainty.
It's a special case that is only needed for encrypted tmpfs and swap.

Any ideas on a header format?

LUKS headers provide most of what we need--places to store key size,
cipher, data offset, and other parameters, and it's already recognized
by blkid.  It could be as simple as extending cryptsetup to store and
understand "Hash spec: random" (instead of the usual sha1) and load a
random key instead of processing key slots.

On the other hand, getting those changes into cryptsetup might not
be simple.

I could make an attempt, but I don't have any real HFS filesystems
to test with, and I'd worry about false negatives too.  I could use
Linux fs/hfs code as a reference but I'm not even sure blkid and Linux
completely agree on what a HFS filesystem is.  I'd prefer to leave that
to someone who knows what they're doing.

Also there would still be a small probability of failure.  Some blkid
recognizers look at 32 bits, which could still fail a few times in a
very large user population.