Zfs: ZFS and LUKS may corrupt LUKS Header: Suggestion for magic header detection.

[turrini]

Suggestion: ZFS should check if partition starts with 'LUKS' + 0xBA 0xBE (LUKS Magic) since it may cause corruption in some specific cases.

Steps to reproduce:

Preparation:

dd if=/dev/zero of=/tmp/disk.img bs=10M count=10

1) zpool create tank /tmp/disk.img (GUID: 17281385454692588068)
(Ooops, forgot to encrypt drive first OR the partition/drive had an old pool before!)
1.1) zpool export tank

2) cryptsetup luksFormat /tmp/disk.img

3) cryptsetup luksOpen /tmp/disk.img luks-tank

4) zpool create tank /dev/mapper/luks-tank (new GUID 6114133284129971711)

... user populate pool with data ...

6) zpool export tank && cryptsetup luksClose luks-tank

Now, if user tries to zpool import tank (/tmp/disk.img) BEFORE opening it with cryptsetup, zfs will corrupt luks header because of reminiscences of old pool (GUID: 17281385454692588068) first created on step 1.

zpool import -d /tmp/ tank (LUKS header gets corrupted as ZFS tries to import old pool)

It may happen on boot, since cryptsetup will ask 3 times for a valid password. If an user miss, zfs will try to mount it anyway, corrupting luks header.

Since its user obligation to do a proper luks header backup, he will need to restore that header every time he misses his password for more than three times (eg: initramfs zfs root pool script).

[dasjoe]

As discussed on IRC, this is not a ZFS, nor a cryptsetup issue.

At 1.1) you export the pool instead of destroying it. This leaves the 4 on-disk labels intact.
2) creates the LUKS header at the beginning of the device, possibly overwriting ZFS labels 0 and 1:

A LUKS partition consists of a header, followed by 8 key-slot descriptors, followed by 8 key slots, followed by the encrypted data area.

zpool import -d /tmp/ tank does not use the mapped encrypted device, so ZFS falls back to the two backup labels 2 and 3 near the disk's end. It finds the stale data from 1), imports the pool and restores labels 0 and 1, overwriting the LUKS header in this process.

[turrini]

We've discussed a little bit more:

Since we have a database for disks who lie about physical sector size, I don't see the difference to have an internal database of magic headers that may or not interfere with zfs, since they're not too many and we could alert the user before doing an operation that may be dangerous to pool's health (or to another previously system installed on a device).

It may be not portable at first, but BSD/Illumos folks can do the same for their specific systems since we may be exchanging disks between different systems, and their users could benefit from this as well.

[beren12]

seems like it should be cryptsetup that should detect ZFS and remove the labels if found...

[sempervictus]

I'm with @turrini on this: we work on all sorts of volumes, and maintaining a list of potentially dangerous block devices and their identifiers seems only rational. Zpool labelclear should fix this, but in situations where you want to encapsulate a pool in an encrypted block device on a ZVOL (it happens) this can get hairy.

I imagine this problem can occur with other block crypt methods like TC. GEOM likely has a way around this built in for GELI (my BSD-fu is weaker than i'd like it to be, sorry). So having a safeguard to prevent damage to these underlying volumes is paramount to preserve the pools we put atop them.

[fling-]

@turrini zfs should corrupt luks header because this is how it imports a pool. And it will import because you told it to do so.
If you do not want a pool to be importable you have multiple options to perform:

1. zpool destroy instead of export
2. zpool labelclear
3. wipefs
   Notice it is not happening with zfs only. This is easily reproducible with most filesystems like ext4 or xfs. Just run fsck or xfs_repair to get rid of your luks header.
   Otoh I'm using luks external header and zpool import will corrupt the data but not header in this case. And even if someone will add any detection (which is unlikely) zfs will not be able to detect the container when the header is external.
   Also why should zfs detect anything? What if you accidentally corrupted your pool using cryptsetup and just want your files back?
   Why are you only suggesting to detect luks? There is a lot of tools for zpool corruption ;P

[turrini]

We know that it may happen with any filesystem and/or tool.

In this case we are talking about ZFS and what it can do to protect user's data if possible in this case. Isn't the main reason (and on ZFS: primary goal) of a filesystem to take care of this? Magic Header detection can be a future feature to be implemented on zpool import.

As @sempervictus said: "we work on all sorts of volumes, and maintaining a list of potentially dangerous block devices and their identifiers seems only rational".

If it's not rational, we can close this issue.

[beren12]

ZFS protects your data well… it doesn't, however, protect you from yourself. fling- is right, at a min. you should use zfs's tools to remove the zfs labels from the drive, or dd/bdblocks/something. Better to have big signs saying "Here be sharks" than to have a solution that sometimes protects you and other times doesn't

[GregorKopka]

Maybe zfs import should not modify a (potential pool member vdev) with missing labels, only in case one gives -f?

[beren12]

Except the labels aren't missing. That's the whole debate here: zfs finds valid labels because they weren't properly removed. ZFS does what it's supposed to do and heals itself of the corrupted labels at the start of the disk.

[GregorKopka]

My point was that _on non-forced import_ sliently healing zfs labels might lead to worse results than having the administrator check should zfs find corrupted labels for a member vdev.

[fling-]

@GregorKopka What if your server will fail to import on reboot because of the bios firmware wrote it's copy to one of the drives overwriting just a single header and enhanced zfs import code decided to wait for you to get physical access to the server to force import instead of healing the header? What if the server miles away from you? And the server got rebooted on saturday night?
zfs is capable to recover from a header corruption and we should not change the behavior of this.
bitflips, other breakages are happening all the time and these things should not prevent the pool from the regular import.

[GregorKopka]

I know the problem and i get your point.
Maybe you get mine that auto-heal of missing labels might lead to unwanted results.

Maybe in case ZFS encounters missing labels on a vdev member it could:

1. check all other devices first if one maybe contains the vdev with all labels
2. import the pool without fixing missing labels (since this indicates a real problem), maybe even read-only
3. give the information about this problem in zfs status (and zfs list)
4. have a way so that an administrator can regenerate the lables in case zfs really picked the right devices

Please note that this is open for discussion to find a better solution, not an 'i want'.

[ryao]

This should be reported to the LUKS developers. They should be using libblkid to check for an existing file system and fail requesting -f if forcing it is desired like mkfs.ext4 does.

[fling-]

@ryao cryptsetup manpage suggests to atleast wipe filesystem signatures prior container creation on a previously used partition:
https://github.com/mbroz/cryptsetup/commit/90853cc3abe6675e4ce98be5560ff0ec5e7788b6#diff-e22b561f6a3a3d4762aba63bfdf9cc5e

[pepa65]

"Notice it is not happening with zfs only. This is easily reproducible with most filesystems like ext4 or xfs. Just run fsck or xfs_repair to get rid of your luks header."

It is unexpected that zfs import is so destructive. Then there is the issue here that there are 2 pools 'tank' -- are they both imported?? Is there any check possible that the import actually makes sense? (Because in the case of importing the invalid 'pool' I am sure there is nothing of value in terms of a file system or anything...)