[pkg-cryptsetup-devel] Bug#994056: cryptsetup: blkid check fails to take offset option into account

[Thorsten Glaser]

Hi Guilhem,

>(And added unit tests for the use case.)

thanks! I was more interested in getting my system working and did the
fix on the installed system without looking at the source package at
first.

>Thanks for the patch!  FWIW crypttab(5)'s ‘offset=’ passes the value to
>`cryptsetup -o` which is a 512 byte sectors count while `blkid -O` wants
>bytes

*OUCH!* Good catch.

>, so I completed your patch with 2373709bb461a71a7af46e7e9c59355fce63e52e.

-blkid="$(/sbin/blkid -o value -s TYPE -p ${offset:+-O "$offset"} -- "$dev")"
+blkid="$(/sbin/blkid -o value -s TYPE -p ${offset:+-O "$((offset*512))"} -- "$dev")"

That’s only good for offset ≤ 4194303 though and invokes C “Undefined
Behaviour” on larger values. (More on LP64 of course.)

Worse, blkid(8) supports multiplicators, but only KiB and more, not
(512-byte) sectors, so to fix this *properly* we’d have to rely on
bc(1) or dc(1), which unfortunately Debian does not install by default
(unlike Unix). I’d say go for it but I’m not sure how majority-capable
my opinion on this is ☻

Other options:

• ignore this, which will cause misdetection for large offsets (bah)

• document that offsets larger than 4194303 are not supported
  (not worth much if the underlying cryptsetup does support it)

  ‣ option: check for that

• check for large offsets and either “defuse” (skip and permit)
  or “refuse” (skip and deny) the blkid and un_blkid checks

Checking for large offsets is also not trivial, as you cannot
do just $((offset < 4194303) for the same reason. In POSIX sh
this would work:

case x$offset in
(x)
	deny 'offset is empty'  # or rather just skip the -O option
	;;
(x*[!0-9]*)
	deny 'offset is negative or contains non-digits'
	;;
(*)
	if test ${#offset} -gt 7 || test "$offset" -gt 4194303; then
		deny 'offset too large'
	fi
	;;
esac

This code first checks that it’s indeed a positive number,
then its length, and only if that’s within safe bounds,
the actual value.

All of these other than the bc(1)/dc(1) solution, however, impose
32-bit limits on 64-bit platforms. This is, unfortunately, not
avoidable because in POSIX sh it’s impossible to find out whether
the shell has 32-bit or 64-bit arithmetics without triggering UB
on 32-bit. Blaming ISO C is appropriate.

I’m attaching a first cut at my favourite solution. It’s missing
a thing I’m not sure how to achieve: ensure that bc(1) is available
in the initrd. My initramfs-tools “fu” is not very high.

Thanks,
//mirabilos
-- 
“It is inappropriate to require that a time represented as
 seconds since the Epoch precisely represent the number of
 seconds between the referenced time and the Epoch.”
	-- IEEE Std 1003.1b-1993 (POSIX) Section B.2.2.2