Bug#902449: cryptsetup-initramfs: auto-detection of zfs pool(s)

2020-06-11 Thread Jean-Baptiste Lallement

Hi,

Here is a patch that ignores zfs filesystems as you recommended by 
returning an empty devno for such devices and then skipping the warning 
and error messages when there is no major/minor number.


--
Jean-Baptiste Lallement
irc: jibel
diff -Nru cryptsetup-2.3.3/debian/changelog cryptsetup-2.3.3/debian/changelog
--- cryptsetup-2.3.3/debian/changelog   2020-06-03 23:41:44.0 +
+++ cryptsetup-2.3.3/debian/changelog   2020-06-11 06:49:28.0 +
@@ -1,3 +1,13 @@
+cryptsetup (2:2.3.3-1.1) UNRELEASED; urgency=medium
+
+  * Non-maintainer upload.
+  * d/functions: Return an empty devno for ZFS devices as they don't have
+major:minor device numbers
+  * d/initramfs/hooks/cryptroot: Ignore and don't print an error message when
+devices don't have a devno
+
+ -- Jean-Baptiste Lallement   Thu, 11 Jun 
2020 06:49:28 +
+
 cryptsetup (2:2.3.3-1) unstable; urgency=medium
 
   [ Guilhem Moulin ]
diff -Nru cryptsetup-2.3.3/debian/functions cryptsetup-2.3.3/debian/functions
--- cryptsetup-2.3.3/debian/functions   2020-06-03 23:41:44.0 +
+++ cryptsetup-2.3.3/debian/functions   2020-06-11 06:49:28.0 +
@@ -583,6 +583,7 @@
 #   Print the major:minor device ID(s) holding the file system currently
 #   mounted currenty mounted on $mountpoint.
 #   Return 0 on success, 1 on error (if $mountpoint is not a mountpoint).
+#   devno will be empty if the filesystem must be excluded.
 get_mnt_devno() {
 local wantmount="$1" devnos="" uuid dev IFS
 local spec mountpoint fstype _ DEV MAJ MIN
@@ -596,8 +597,15 @@
 # take the last mountpoint if used several times (shadowed)
 unset -v devnos
 spec="$(printf '%b' "$spec")"
-_resolve_device "$spec" || continue # _resolve_device() already 
warns on error
 fstype="$(printf '%b' "$fstype")"
+if [ "$fstype" = "zfs" ]; then
+# Ignore ZFS entries as they don't have a major/minor and won't
+# be imported when local-top cryptroot script will ran.
+# Returns success with empty devno
+printf ''
+return 0
+fi
+_resolve_device "$spec" || continue # _resolve_device() already 
warns on error
 if [ "$fstype" = "btrfs" ]; then
 # btrfs can span over multiple devices
 if uuid="$(_device_uuid "$DEV")"; then
diff -Nru cryptsetup-2.3.3/debian/initramfs/hooks/cryptroot 
cryptsetup-2.3.3/debian/initramfs/hooks/cryptroot
--- cryptsetup-2.3.3/debian/initramfs/hooks/cryptroot   2020-06-03 
23:41:44.0 +
+++ cryptsetup-2.3.3/debian/initramfs/hooks/cryptroot   2020-06-11 
06:49:28.0 +
@@ -179,16 +179,18 @@
 
 {
 if devnos="$(get_mnt_devno /)"; then
-usage=rootfs foreach_cryptdev crypttab_find_and_print_entry $devnos
+if [ -n "$devnos" ]; then
+usage=rootfs foreach_cryptdev crypttab_find_and_print_entry 
$devnos
+fi
 else
 cryptsetup_message "WARNING: Couldn't determine root device"
 fi
 
-if devnos="$(get_resume_devno)"; then
+if devnos="$(get_resume_devno)" && [ -n "$devnos" ]; then
 usage=resume foreach_cryptdev crypttab_find_and_print_entry $devnos
 fi
 
-if devnos="$(get_mnt_devno /usr)"; then
+if devnos="$(get_mnt_devno /usr)" && [ -n "$devnos" ]; then
 usage="" foreach_cryptdev crypttab_find_and_print_entry $devnos
 fi
 


Bug#902449: cryptsetup-initramfs: auto-detection of zfs pool(s)

2019-07-16 Thread Jiri Novak
Hi,


with current 2.1.0-5 in buster, dist-upgrade of encrypted zfs root
installations fails to boot. took me some while to find this hintwith
the initramfs in crypttab (my original solution was to bring cryptsetup
packages form stretch, noticed this randomly later)




signature.asc
Description: OpenPGP digital signature


Bug#902449: cryptsetup-initramfs: auto-detection of zfs pool(s)

2018-07-21 Thread Guilhem Moulin
Hi,

On Sat, 21 Jul 2018 at 19:27:54 +0200, Michal Humpula wrote:
> Since the ZFS is not strictly speaking alien in Debian (it's in
> contrib, though), it would be nice if cryptsetup-initramfs would
> support it.

Just to be clear, it's only *auto-detection* that's not supported.  It's
still possible to format your root device as ZFS (or any other file
system) held by encrypted disks.  Adding the ‘initramfs’ option to the
crypttab(5) entries for the underlying devices should do the trick, cf.
https://bugs.debian.org/820888#32 .

> So, any other ideas how to integrate the ZFS support?

Not beside parsing `zpool`'s output, which I'm afraid I still object to.

That being said, I don't mind implementing an option to silence the
warning that the root FS can't be found.  At least since 2:2.0.3-5 there
should be only one warning (like with ≤2:2.0.3-1), cf.
https://bugs.debian.org/902449#21 .

-- 
Guilhem.


signature.asc
Description: PGP signature


Bug#902449: cryptsetup-initramfs: auto-detection of zfs pool(s)

2018-07-21 Thread Michal Humpula
Hi Guilhem,

I may have found the answer to the question, why there is still no sysfs 
interface for the ZFS. Kernel object representing the `/sys/fs` directory is 
exported as GPL only, so it's not possible to use it in non GPL module, which 
unfortunately ZFS is (CDDL).

Since the ZFS is not strictly speaking alien in Debian (it's in contrib, 
though), it would be nice if cryptsetup-initramfs would support it. So, any 
other ideas how to integrate the ZFS support?

Cheers
Michal



Bug#902449: cryptsetup-initramfs: auto-detection of zfs pool(s)

2018-07-05 Thread Guilhem Moulin
On Thu, 05 Jul 2018 at 11:05:08 +0200, Michal Humpula wrote:
>> Since ZFS doesn't expose a block device one would need another
>> documented way to resolve /sys/fs/zfs/$FS.  Hopefully ‘tank/my/fs’ is
>> unique and can't be aliased to something else, can it?
> 
>> Do the slash characters in ‘tank/my/fs’ hint at a hierarchy, or is it a
>> flat string?
> 
> It's unique and designates the ZFS specific fs instance which is 
> hierarchical. 
> First level is the zpool name. As far as I know btrfs has something similar 
> with subvolumes. Below is an example of zfs list
> 
> NAME   USED  AVAIL  REFER  MOUNTPOINT
> tank  2,45T   189G   120K  /tank
> tank/backup   75,8G   189G   136K  /tank/backup
> tank/data 1,91T   189G  1,83T  /tank/data
> tank/home  122G   189G  42,2G  /home
> tank/root  148G   189G  13,9G  /

If it's like btrfs subvolumes the zpool (‘tank’) denotes the FS itself,
while the subhierarchy denotes different views of it.

I guess ‘tank’, ‘tank/backup’, ‘tank/data’ etc. need to have the same
set of features enabled, and that the same underlying devices?  In that,
case, I think FS="tank" would be better than FS="tank/my/fs".
Conveniently this also trims the hierarchy :-)

-- 
Guilhem.


signature.asc
Description: PGP signature


Bug#902449: cryptsetup-initramfs: auto-detection of zfs pool(s)

2018-07-05 Thread Michal Humpula
> Since ZFS doesn't expose a block device one would need another
> documented way to resolve /sys/fs/zfs/$FS.  Hopefully ‘tank/my/fs’ is
> unique and can't be aliased to something else, can it?

> Do the slash characters in ‘tank/my/fs’ hint at a hierarchy, or is it a
> flat string?

It's unique and designates the ZFS specific fs instance which is hierarchical. 
First level is the zpool name. As far as I know btrfs has something similar 
with subvolumes. Below is an example of zfs list

NAME   USED  AVAIL  REFER  MOUNTPOINT
tank  2,45T   189G   120K  /tank
tank/backup   75,8G   189G   136K  /tank/backup
tank/data 1,91T   189G  1,83T  /tank/data
tank/home  122G   189G  42,2G  /home
tank/root  148G   189G  13,9G  /

So, thanks for hints. I will try to inspect what the zfs upstream thinks about 
this.



Bug#902449: cryptsetup-initramfs: auto-detection of zfs pool(s)

2018-07-03 Thread Guilhem Moulin
Hi Michal,

On Tue, 03 Jul 2018 at 18:23:12 +0200, Michal Humpula wrote:
> Entry in /proc/mounts for ZFS is different as it refers to the actual ZFS 
> filesystem not to the devices of the underlying zpool. Which, from my point 
> of 
> view, makes more sense. Do you see a way to export all the required 
> information in sysfs that would be acceptable by you? From top of my head, I 
> can think of something like `/sys/fs/zfs/$FS/devices` where $FS can be 
> something like `tank/my/fs`.

So $FS is what's found in /proc/mounts' first column?  For block devices
this value is not canonical (eg. /dev/mapper/xyz vs. /dev/dm-0 vs.
/dev/disks/by-uuid/$UUID vs. /dev/block/$maj:$min, etc.), but there is a
documented way to normalise it and get the corresponding sysfs
directory·ies dev/block/$maj:$min.  For “traditional” file systems it's
enough to run `stat -L -c%t:%T` and convert the values to base 10; for
btrfs (and I guess some other FS:es I'm not aware of) the list is under
/sys/fs/$FSTYPE/$UUID/devices, where $UUID is canonical and can be
obtained from /proc/mounts' first column using a generic (non
FS-specific) command.

Since ZFS doesn't expose a block device one would need another
documented way to resolve /sys/fs/zfs/$FS.  Hopefully ‘tank/my/fs’ is
unique and can't be aliased to something else, can it?  (Again for LVM,
/dev/mapper/$vgname-$lvname, /dev/dm-$n, /dev/$vgname/$lvname denote the
same device, and AFAIK whether /proc/mounts contains one or the other is
not documented.)

Do the slash characters in ‘tank/my/fs’ hint at a hierarchy, or is it a
flat string?  In the latter case it makes sense to encode the slashes as
hex-sequences (\x2f), like udev does for names under /dev/disk/by-label.
FWIW /proc/mounts encodes special characters such as tabs, linefeeds,
and spaces using octal escapes.  While it's our scripts' job and not
ZFS's to decode these, if there is any mangling under /sys/fs/zfs then
this should be documented.

> I think few other upcoming filesystems will have similar problems, e.g.
> bcachefs.

So it'd be great do expose the sysfs directories in a similar way;
thanks for bringing that up :-)

-- 
Guilhem.


signature.asc
Description: PGP signature


Bug#902449: cryptsetup-initramfs: auto-detection of zfs pool(s)

2018-07-03 Thread Michal Humpula
Hi Guilhem,

I can see the point against implementing the specific FS quirks. I'm might be 
able to send some patches upstream to make the ZFS more discoverable, but I 
need some more input from you.

On my machine I see that Btrfs propagates one of the devices to the /proc/
mounts, even if the actual FS is constructed from multiple devices in whatever 
manner the user wanted. Personally this seems confusing to me as user of /
proc/mounts, though it might be more reasonable approach for Btrfs users 
(don't know). I can see that cryptsetup hook uses this to get the UUID of the 
device and from that discover all the devices forming the Btrfs FS instance.

Entry in /proc/mounts for ZFS is different as it refers to the actual ZFS 
filesystem not to the devices of the underlying zpool. Which, from my point of 
view, makes more sense. Do you see a way to export all the required 
information in sysfs that would be acceptable by you? From top of my head, I 
can think of something like `/sys/fs/zfs/$FS/devices` where $FS can be 
something like `tank/my/fs`.

I think few other upcoming filesystems will have similar problems, e.g. 
bcachefs.

Cheers
Michal



Bug#902449: cryptsetup-initramfs: auto-detection of zfs pool(s)

2018-06-27 Thread Guilhem Moulin
On Wed, 27 Jun 2018 at 13:44:23 +0200, Fabian Grünbichler wrote:
> On Wed, Jun 27, 2018 at 12:56:04PM +0200, Guilhem Moulin wrote:
>> On Tue, 26 Jun 2018 at 23:34:20 +0200, Fabian Grünbichler wrote:
>>> cryptsetup: ERROR: Couldn't normalise device rpool/ROOT/debian
>>> cryptsetup: ERROR: Couldn't find sysfs directory corresponding to 
>>>  rpool/ROOT/debian
>> 
>> I guess you had a fake line for that device in your /etc/fstab and
>> that's why up to 2:2.0.3-1 the hook script didn't complain about not
>> being able to normalize the device?  In that case it's not caused by the
>> package split per se, but by the hook using /proc/mounts rather than
>> /etc/fstab to find which device is holding / (and /usr).
> 
> no, never had a fake fstab line, there were always warnings about not
> being able to detect the device for /, but now they got more verbose and
> there are two, so I took the opportunity to file this bug.

:-)

> I think I'd prefer a two-step approach:
> - warn by default on known non-detectable file system in
>  get_mnt_devices, pointing to crypttab (to help with initial setup, and
>  for users that might benefit from this assistance)
> - allow silencing of detection warnings (once the setup is known to
>  work)
> 
> the latter might be a simple on/off switch, or a whitelist of ignored
> (i.e., manually setup) filesystems, or a whitelist of ignored (i.e.,
> manually setup) mountpoints.

Makes sense to be able to silence that warning, indeed.  There are only
two mountpoints we're considering at initramfs stage (/ and /usr), so
I'm not sure if fine-grained control is worth to implement.  That said
it doesn't really add much complexity (fstype and mountpoint are both in
scope already), so I don't have hard feelings either way.

Cheers,
-- 
Guilhem.


signature.asc
Description: PGP signature


Bug#902449: cryptsetup-initramfs: auto-detection of zfs pool(s)

2018-06-27 Thread Fabian Grünbichler
On Wed, Jun 27, 2018 at 12:56:04PM +0200, Guilhem Moulin wrote:
> Control: severity -1 wishlist
> 
> Hi,
> 
> On Tue, 26 Jun 2018 at 23:34:20 +0200, Fabian Grünbichler wrote:
> > cryptsetup: ERROR: Couldn't normalise device rpool/ROOT/debian
> > cryptsetup: ERROR: Couldn't find sysfs directory corresponding to 
> >   rpool/ROOT/debian
> 
> I guess you had a fake line for that device in your /etc/fstab and
> that's why up to 2:2.0.3-1 the hook script didn't complain about not
> being able to normalize the device?  In that case it's not caused by the
> package split per se, but by the hook using /proc/mounts rather than
> /etc/fstab to find which device is holding / (and /usr).

no, never had a fake fstab line, there were always warnings about not
being able to detect the device for /, but now they got more verbose and
there are two, so I took the opportunity to file this bug.

> 
> > I wonder whether you'd accept a patch contributing detection of zpool
> > devices in the cryptroot hook? it is a bit cumbersome, as the only way to
> > get this information from ZFS is by parsing 'zpool status' output, which
> > contains a lot of extra information and is not very well-defined (thus,
> > potential for breakage in the future).
> 
> I'm really not keen on having to run `zpool status` or `zpool list` and
> parse its output.  We used to do something like that (parse `btrfs
> filesystem show`) for btrfs but we're now using proc(5) and sysfs(5)
> which gives a unified solution for all components in the block device &
> FS stack we've supported up to now: mapped devices (in particular LV and
> dm-crypt), MD, mutiple device btrfs.  We could painlessly extend the
> stack to other virtual block devices as long as the driver exposes a
> sysfs directory /sys/dev/block/$maj:$min; as well as file systems
> spanning over multiple devices, as long as the driver lists the slaves's
> sysfs directories in /sys/fs/$FSTYPE/$UUID/devices.

maybe I'll give that a shot on the ZoL side - this would be nice to have
in general, not just for cryptsetup's use case.

> Unfortunately the ZFS driver exposes neither directory to the sysfs(5)
> pseudo-filesystem.  Personally I'd rather avoid FS-specific code in the
> hook script.  Especially if it involves parsing the non-machine-readable
> output of an FS-specific command.

understandable.

> > at the very least, it would be nice to detect this unsupported situation
> > and print a proper warning instead of the generic one.
> 
> Yes indeed, we can definitely have a list of known unsupported FS:es,
> either skip the warning altogether, or tell the user to add the
> 'initramfs' option to the crypttab(5) entries corresponding to the
> underlying devices.

I think I'd prefer a two-step approach:
- warn by default on known non-detectable file system in
  get_mnt_devices, pointing to crypttab (to help with initial setup, and
  for users that might benefit from this assistance)
- allow silencing of detection warnings (once the setup is known to
  work)

the latter might be a simple on/off switch, or a whitelist of ignored
(i.e., manually setup) filesystems, or a whitelist of ignored (i.e.,
manually setup) mountpoints.

thanks for your consideration (and work on cryptsetup!)



Bug#902449: cryptsetup-initramfs: auto-detection of zfs pool(s)

2018-06-27 Thread Guilhem Moulin
Control: severity -1 wishlist

Hi,

On Tue, 26 Jun 2018 at 23:34:20 +0200, Fabian Grünbichler wrote:
> cryptsetup: ERROR: Couldn't normalise device rpool/ROOT/debian
> cryptsetup: ERROR: Couldn't find sysfs directory corresponding to 
>   rpool/ROOT/debian

I guess you had a fake line for that device in your /etc/fstab and
that's why up to 2:2.0.3-1 the hook script didn't complain about not
being able to normalize the device?  In that case it's not caused by the
package split per se, but by the hook using /proc/mounts rather than
/etc/fstab to find which device is holding / (and /usr).

> I wonder whether you'd accept a patch contributing detection of zpool
> devices in the cryptroot hook? it is a bit cumbersome, as the only way to
> get this information from ZFS is by parsing 'zpool status' output, which
> contains a lot of extra information and is not very well-defined (thus,
> potential for breakage in the future).

I'm really not keen on having to run `zpool status` or `zpool list` and
parse its output.  We used to do something like that (parse `btrfs
filesystem show`) for btrfs but we're now using proc(5) and sysfs(5)
which gives a unified solution for all components in the block device &
FS stack we've supported up to now: mapped devices (in particular LV and
dm-crypt), MD, mutiple device btrfs.  We could painlessly extend the
stack to other virtual block devices as long as the driver exposes a
sysfs directory /sys/dev/block/$maj:$min; as well as file systems
spanning over multiple devices, as long as the driver lists the slaves's
sysfs directories in /sys/fs/$FSTYPE/$UUID/devices.

Unfortunately the ZFS driver exposes neither directory to the sysfs(5)
pseudo-filesystem.  Personally I'd rather avoid FS-specific code in the
hook script.  Especially if it involves parsing the non-machine-readable
output of an FS-specific command.

> at the very least, it would be nice to detect this unsupported situation
> and print a proper warning instead of the generic one.

Yes indeed, we can definitely have a list of known unsupported FS:es,
either skip the warning altogether, or tell the user to add the
'initramfs' option to the crypttab(5) entries corresponding to the
underlying devices.

-- 
Guilhem.


signature.asc
Description: PGP signature


Bug#902449: cryptsetup-initramfs: auto-detection of zfs pool(s)

2018-06-26 Thread Fabian Grünbichler
Package: cryptsetup-initramfs
Version: 2:2.0.3-4

hello again!

still running the same setup with a fully encrypted single vdev used as
rpool. since the recent refactoring and split into
cryptsetup-{run,initramfs}, I now get the following messages on every
initramfs generation:

cryptsetup: ERROR: Couldn't normalise device rpool/ROOT/debian
cryptsetup: ERROR: Couldn't find sysfs directory corresponding to 
rpool/ROOT/debian

the initramfs is still correctly generated because I specified the
underlying physical disk in my /etc/crypttab.

I wonder whether you'd accept a patch contributing detection of zpool
devices in the cryptroot hook? it is a bit cumbersome, as the only way to
get this information from ZFS is by parsing 'zpool status' output, which
contains a lot of extra information and is not very well-defined (thus,
potential for breakage in the future).

the output looks roughly like this for a single vdev pool:

$ zpool status -P rpool
  pool: rpool
 state: ONLINE
status: Some supported features are not enabled on the pool. The pool can
still be used, but some features are unavailable.
action: Enable all features using 'zpool upgrade'. Once this is done,
the pool may no longer be accessible by software that does not support
the features. See zpool-features(5) for details.
  scan: scrub repaired 0B in 0h6m with 0 errors on Sun May 13 00:30:27 2018
config:

NAMESTATE READ WRITE CKSUM
rpool   ONLINE   0 0 0
  /dev/mapper/root  ONLINE   0 0 0

errors: No known data errors


for more complicated pool setups, there is an additional level of
nesting in the config section, e.g. like this:

 NAME   STATE READ WRITE CKSUM
 rpool  ONLINE   0 0 0
   mirror-0 ONLINE   0 0 0
 /dev/mapper/disk0  ONLINE   0 0 0
 /dev/mapper/disk1  ONLINE   0 0 0
   mirror-1 ONLINE   0 0 0
 /dev/mapper/disk2  ONLINE   0 0 0
 /dev/mapper/disk3  ONLINE   0 0 0

mirror-\d can alse have other forms, like raidz[23]-\d, log, cache, spare, ..

and of course some of those are optional for booting (cache, spare,
redundant vdevs depending on level of redundancy) - not sure whether the
current infrastructure is able to handle that properly?

there already is C code for parsing this exact output to retrieve a list
of underlying devices in grub2[1], which could serve as a base for
re-implementing in sh for cryptsetup-initramfs.

I'd be willing and able to whip a patch in case you are interested ;)

at the very least, it would be nice to detect this unsupported situation
and print a proper warning instead of the generic one.

1: 
https://salsa.debian.org/grub-team/grub/blob/4600d592f6eac6141e8efa503d7ef8cd9e79a3b5/grub-core/osdep/unix/getroot.c#L225

-- Package-specific info:
-- /proc/cmdline
BOOT_IMAGE=/ROOT/debian/@/vmlinuz root=ZFS=rpool/ROOT/debian boot=zfs ro

-- /etc/crypttab
root UUID=XXX none luks,initramfs

-- /etc/fstab

-- System Information:
Debian Release: buster/sid
  APT prefers unstable-debug
  APT policy: (500, 'unstable-debug'), (500, 'unstable'), (500, 'testing'), 
(500, 'stable')
Architecture: amd64 (x86_64)

Kernel: Linux 4.16.0-2-amd64 (SMP w/4 CPU cores)
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8), 
LANGUAGE=en_US.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: systemd (via /run/systemd/system)
LSM: AppArmor: enabled

Versions of packages cryptsetup-initramfs depends on:
ii  busybox-static [busybox]1:1.27.2-2
ii  cryptsetup-run  2:2.0.3-4
ii  initramfs-tools [linux-initramfs-tool]  0.130

Versions of packages cryptsetup-initramfs recommends:
ii  console-setup  1.184
ii  kbd2.0.4-3

cryptsetup-initramfs suggests no packages.

-- Configuration Files:
/etc/cryptsetup-initramfs/conf-hook changed [not included]

-- no debconf information