Hi Taylor,
On Sun, Apr 28, 2024 at 08:38:55PM +0000, Taylor R Campbell wrote:
> Cool, thanks! We should definitely have a zfs verification method for
> cgdconfig(8). But I think we can do it a little more robustly -- and
> with fewer dependencies outside libc in cgdconfig(8).
>
> Here's what I wrote the last time someone suggested this -- identical
> to the nvlist approach you adapted from fstyp(8) -- but my round tuit
> supply ran short of taking the approach I suggested of verifying a
> fixed magic number and a SHA-256 hash over the header:
>
> I wonder if we can do this without pulling so much into cgdconfig and
> doing parsing there, though?
>
> A comment in fstyp(8) suggests there's a checksum (which it doesn't
> verify); maybe we can use that?
>
> Would be nice if there were a reference to the data format that we can
> put in a comment -- which should appear in fstyp(8) too.
>
> Would also be nice if we could quantify the false acceptance
> probability under uniform random data (i.e., wrong password, modelling
> password hash as uniform random function). For the current
> verification methods it looks like:
>
> verify_ffs = 1 - (1 - 4/2^32)^4 < one in 250 * 10^6
> verify_gpt < 1 - (1 - 1/2^96)^4 < one in 10^28
> verify_mbr = 1/2^16 (much too high for my comfort!)
>
> For zfs I would hope it can be much lower than for ffs.
>
> I went hunting through the zfs header files and it looks like the vdev
> label has a magic number and some kind of 256-bit checksum -- not
> specific to the vdev label (other zpool data structures have the same
> magic number and checksum), but maybe we can use it:
>
> /* vdev_impl.h */
> typedef struct vdev_phys {
> char vp_nvlist[VDEV_PHYS_SIZE - sizeof (zio_eck_t)];
> zio_eck_t vp_zbt;
> } vdev_phys_t;
>
> typedef struct vdev_label {
> char vl_pad1[VDEV_PAD_SIZE]; /* 8K */
> char vl_pad2[VDEV_PAD_SIZE]; /* 8K */
> vdev_phys_t vl_vdev_phys; /* 112K */
> char vl_uberblock[VDEV_UBERBLOCK_RING]; /* 128K */
> } vdev_label_t; /* 256K total */
>
> /* zio.h */
> #define ZEC_MAGIC 0x210da7ab10c7a11ULL
>
> typedef struct zio_eck {
> uint64_t zec_magic; /* for validation, endianness */
> zio_cksum_t zec_cksum; /* 256-bit checksum */
> } zio_eck_t;
>
> Judging by zio_checksum.c, it looks like it's SHA-256.
>
> I grepped for some of the nvlist keys that zfs uses, and found
> libzfs_import.c, which does a couple things:
>
> - Tries not just offset 0, but up to four different offsets with the
> label_offset function.
>
> - Checks for "state" and "txg" in the nvlist.
>
> vdev_label_read_config also tries several different offsets, but only
> checks for "txg" -- and doesn't even require that. So the nvlist keys
> may be a red herring here.
>
> But with the checksum, it looks like we could get by with:
>
> 1. (optional) for each possible label offset:
>
> 2. verify vl->vl_vdev_phys.vp_zbt.zec_magic == ZEC_MAGIC or
> byte-swapped, and
>
> 3. verify vl->vl_vdev_phys.vp_zbt.zec_cksum is the SHA-256 hash of
> whatever part of vl it's computed over.
>
> The SHA-256 hash appears to be calculated over vl->vl_vdev_phys, but
> with vp_zbt.zec_cksum set to
>
> {labeloffset + offsetof(struct vdev_label, vl_vdev_phys), 0, 0, 0},
>
> with the 64-bit words possibly encoded in the same byte order as
> zec_magic. Except then the bytes of each 64-bit word in the SHA-256
> output hash are byte-swapped.
>
> This seems to check out on the one sample I tried, and it should have
> a false detection probability under uniform random data well below
> 1/2^256 which is extremely comfortable. (Should also have low false
> detection probability under other formats too because of the checksum
> even if you store a file with the magic number in it.)
Thanks for the detailed response!
I did the same reading to try and find out if there is a simple checksum
verification function but didn't find anything. It seems like zpool
import never actually verifies the checksums or magic bytes in
vl->vl_vdev_phys->vp_zbt . There are VDEV_LABELS (4) labels, two at the
beginning of the disk (offset 0 and 256k) and two at the end (at -512k,
-256k) and even with all of their vl->vl_vdev_phys->vp_zbt structures
zeroed out, zpool import shows no complaints. The checksums are updated
on export.
I did a quick test and computed sha256 for vl->vdev_phys and it works as
you described: the magic bytes need to be set first (byte-swapped on my
system) and the offset encoded in the first of the 64bit checksum words
(also byte-swapped). The computed sha256 checksum is then stored as 4
64bit words in byte-swapped order.
Implementing the checksum verification is easy and allows us to get rid
of the libnvpair dependency, so I don't mind implementing it that way.
Since I haven't looked into exactly how much checking libnvpair does, I
don't know what the risk of false positives for the current proposal is.
Cheers,
--
Malte Dehling
Attachment:
signature.asc
Description: PGP signature