Re: [PATCH v3] grub-fs-tester: Add luks1 and luks2 support

[Fabian Vogt <fvogt@suse.de>]

Hi,

Am Mittwoch, 15. Juni 2022, 20:03:57 CEST schrieb Glenn Washburn:
> On Wed, 15 Jun 2022 11:43:25 +0200
> Fabian Vogt <fvogt@suse.de> wrote:
> 
> > Hi,
> > 
> > Am Mittwoch, 15. Juni 2022, 04:43:03 CEST schrieb Glenn Washburn:
> > > From: Pierre-Louis Bonicoli <pierre-louis.bonicoli@libregerbil.fr>
> > > 
> > > The logical sector size used by LUKS1 is 512 bytes and LUKS2 uses 512 to
> > > 4069 bytes. The deafualt password used is "pass", but can be overridden
> > > by setting the PASS environment variable. The device mapper name is set
> > > to the name of the temp directory so that its easy to corrolate device
> > > mapper name with a particular test run. Also since this name is unique
> > > per test run, multiple simultaneous test runs are allowed.
> > > 
> > > Note that cryptsetup is passing the --disable-locks parameter to allow
> > > cryptsetup run successfully when /run/lock/cryptsetup is not accessible.
> > > Since the device mapper name is unique per test run, there is no need to
> > > worry about locking the device to serialize access.
> > > 
> > > Signed-off-by: Pierre-Louis Bonicoli <pierre-louis.bonicoli@libregerbil.fr>
> > > Signed-off-by: Glenn Washburn <development@efficientek.com>
> > > ---
> > > This is a heavily modified version of Pierre-Louis's v2 patch. It has been
> > > tested with Fabian's v3 and Josselin's v4 series. Some notable differences
> > > from the previous version:
> > >  * Rebase on to master accounting for cleanup() changes
> > >  * Allow multple tests runs to run simultaneously
> > >  * Allow specifying alternate password with environment variable
> > >  * Fixed bug in previous version where LC_ALL=C was being set for echo and
> > >    not run_it
> > >  * Make output on UUID fail consistent with other filesystems
> > >  * Allow tests to work with older cryptsetups
> > >  * Fixed bug where luks1 tests were actually testing luks2
> > >  * Address my review comments
> > > 
> > > Note: The luks2 test will fail without some form of working grub-probe
> > > support for luks2. This patch is independent of the above mentioned
> > > patch series, will apply without them just fine, and can be reviewed
> > > independently.
> > > 
> > > Glenn
> > > ---
> > >  .gitignore                   |  2 ++
> > >  Makefile.util.def            | 12 ++++++++
> > >  tests/luks1_test.in          | 23 +++++++++++++++
> > >  tests/luks2_test.in          | 23 +++++++++++++++
> > >  tests/util/grub-fs-tester.in | 57 ++++++++++++++++++++++++++++++++++--
> > >  5 files changed, 115 insertions(+), 2 deletions(-)
> > >  create mode 100644 tests/luks1_test.in
> > >  create mode 100644 tests/luks2_test.in
> > > 
> > > diff --git a/.gitignore b/.gitignore
> > > index f6a1bd051..4064d3d1e 100644
> > > --- a/.gitignore
> > > +++ b/.gitignore
> > > @@ -230,6 +230,8 @@ widthspec.bin
> > >  /lib/libgcrypt-grub
> > >  /libgrub_a_init.c
> > >  /lzocompress_test
> > > +/luks1_test
> > > +/luks2_test
> > >  /m4/
> > >  /minixfs_test
> > >  /missing
> > > diff --git a/Makefile.util.def b/Makefile.util.def
> > > index d919c562c..3f1162b76 100644
> > > --- a/Makefile.util.def
> > > +++ b/Makefile.util.def
> > > @@ -1213,6 +1213,18 @@ script = {
> > >    common = tests/syslinux_test.in;
> > >  };
> > >  
> > > +script = {
> > > +  testcase = native;
> > > +  name = luks1_test;
> > > +  common = tests/luks1_test.in;
> > > +};
> > > +
> > > +script = {
> > > +  testcase = native;
> > > +  name = luks2_test;
> > > +  common = tests/luks2_test.in;
> > > +};
> > > +
> > >  program = {
> > >    testcase = native;
> > >    name = example_unit_test;
> > > diff --git a/tests/luks1_test.in b/tests/luks1_test.in
> > > new file mode 100644
> > > index 000000000..cd28fd714
> > > --- /dev/null
> > > +++ b/tests/luks1_test.in
> > > @@ -0,0 +1,23 @@
> > > +#!@BUILD_SHEBANG@
> > > +
> > > +set -e
> > > +
> > > +if [ "x$EUID" = "x" ] ; then
> > > +  EUID=`id -u`
> > > +fi
> > > +
> > > +if [ "$EUID" != 0 ] ; then
> > > +   exit 99
> > > +fi
> > > +
> > > +if ! which mkfs.ext2 >/dev/null 2>&1; then
> > > +   echo "mkfs.ext2 not installed; cannot test luks."
> > > +   exit 99
> > > +fi
> > > +
> > > +if ! which cryptsetup >/dev/null 2>&1; then
> > > +   echo "cryptsetup not installed; cannot test luks."
> > > +   exit 99
> > > +fi
> > > +
> > > +"@builddir@/grub-fs-tester" luks1
> > > diff --git a/tests/luks2_test.in b/tests/luks2_test.in
> > > new file mode 100644
> > > index 000000000..6a26ba626
> > > --- /dev/null
> > > +++ b/tests/luks2_test.in
> > > @@ -0,0 +1,23 @@
> > > +#!@BUILD_SHEBANG@
> > > +
> > > +set -e
> > > +
> > > +if [ "x$EUID" = "x" ] ; then
> > > +  EUID=`id -u`
> > > +fi
> > > +
> > > +if [ "$EUID" != 0 ] ; then
> > > +   exit 99
> > > +fi
> > > +
> > > +if ! which mkfs.ext2 >/dev/null 2>&1; then
> > > +   echo "mkfs.ext2 not installed; cannot test luks2."
> > > +   exit 99
> > > +fi
> > > +
> > > +if ! which cryptsetup >/dev/null 2>&1; then
> > > +   echo "cryptsetup not installed; cannot test luks2."
> > > +   exit 99
> > > +fi
> > > +
> > > +"@builddir@/grub-fs-tester" luks2
> > > diff --git a/tests/util/grub-fs-tester.in b/tests/util/grub-fs-tester.in
> > > index 43f6175c3..e488c0e41 100644
> > > --- a/tests/util/grub-fs-tester.in
> > > +++ b/tests/util/grub-fs-tester.in
> > > @@ -6,6 +6,7 @@ export BLKID_FILE=/dev/null
> > >  fs="$1"
> > >  
> > >  GRUBFSTEST="@builddir@/grub-fstest"
> > > +GRUBPROBE="@builddir@/grub-probe"
> > >  
> > >  tempdir=`mktemp -d "${TMPDIR:-/tmp}/${0##*/}.$(date '+%Y%m%d%H%M%S%N').${fs}.XXX"` ||
> > >  { echo "Failed to make temporary directory"; exit 99; }
> > > @@ -13,6 +14,8 @@ tempdir=`mktemp -d "${TMPDIR:-/tmp}/${0##*/}.$(date '+%Y%m%d%H%M%S%N').${fs}.XXX
> > >  # xorriso -as mkisofs options to ignore locale when processing file names and
> > >  # FSLABEL. This is especially needed for the conversion to Joliet UCS-2.
> > >  XORRISOFS_CHARSET="-input-charset UTF-8 -output-charset UTF-8"
> > > +DMNAME="${tempdir##*/}"
> > > +PASS="${PASS:-pass}"
> > >  
> > >  MOUNTS=
> > >  LODEVICES=
> > > @@ -28,6 +31,10 @@ cleanup() {
> > >  	umount "$i" || :
> > >      done
> > >  
> > > +    if [ -e /dev/mapper/"$DMNAME" ]; then
> > > +	cryptsetup close --disable-locks "$DMNAME"
> > > +    fi
> > > +
> > >      for lodev in $LODEVICES; do
> > >  	local i=600
> > >  	while losetup -l -O NAME | grep -q "^$lodev\$"; do
> > > @@ -68,7 +75,12 @@ run_grubfstest () {
> > >  	need_images="$need_images $FSIMAGEP${i}.img";
> > >      done
> > >  
> > > -    run_it -c $NEED_IMAGES_N $need_images  "$@"
> > > +    case x"$fs" in
> > > +	xluks*)
> > > +	    echo -n "$PASS" | run_it -C -c $NEED_IMAGES_N $need_images  "$@";;
> > > +	*)
> > > +	    run_it -c $NEED_IMAGES_N $need_images  "$@";;
> > > +    esac
> > >  }
> > >  
> > >  # OS LIMITATION: GNU/Linux has no AFS support, so we use a premade image and a reference tar file. I.a. no multiblocksize test
> > > @@ -76,6 +88,8 @@ run_grubfstest () {
> > >  MINLOGSECSIZE=9
> > >  MAXLOGSECSIZE=9
> > >  case x"$fs" in
> > > +    xluks2)
> > > +	MAXLOGSECSIZE=12;;
> > >      xntfs*)
> > >  	MINLOGSECSIZE=8
> > >  	MAXLOGSECSIZE=12;;
> > > @@ -363,7 +377,7 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE" "$MAXLOGSECSIZE" 1); do
> > >  		    #FSLABEL="g;/_é莭莽😁кит u"
> > >  		    ;;
> > >  		# FS LIMITATION: reiserfs, extN and jfs label is at most 16 UTF-8 characters
> > > -		x"reiserfs_old" | x"reiserfs" | x"ext"* | x"lvm"* | x"mdraid"* | x"jfs" | x"jfs_caseins")
> > > +		x"reiserfs_old" | x"reiserfs" | x"ext"* | x"lvm"* | x"luks"* | x"mdraid"* | x"jfs" | x"jfs_caseins")
> > >  		    FSLABEL="g;/éт 莭😁";;
> > >  		# FS LIMITATION: No underscore, space, semicolon, slash or international characters in UFS* in label. Limited to 32 UTF-8 characters
> > >  		x"ufs1" | x"ufs1_sun" | x"ufs2")
> > > @@ -832,6 +846,12 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE" "$MAXLOGSECSIZE" 1); do
> > >  		    MOUNTDEVICE="/dev/mapper/grub_test-testvol"
> > >  		    MOUNTFS=ext2
> > >  		    "mkfs.ext2" -L "$FSLABEL" -q "${MOUNTDEVICE}"  ;;
> > > +		x"luks"*)
> > > +		    echo -n "$PASS" | cryptsetup luksFormat --type "$fs" --sector-size $SECSIZE --pbkdf pbkdf2 --disable-locks $LODEVICE
> > 
> > With the default "pass" password this fails here due to pwquality checks.
> > Can you add "--force-password"? With that it works fine here, both LUKS1 and
> > with the required patches also LUKS2.
> 
> Yes, I can, but I'm curious why I'm not seeing this. What version of
> cryptsetup are you using and for what distro?

openSUSE Tumbleweed, cryptsetup 2.4.3 built with --enable-pwquality.

Cheers,
Fabian

> Glenn
> 
> > 
> > Thanks,
> > Fabian
> > 
> > > +		    echo -n "$PASS" | cryptsetup open --disable-locks $LODEVICE "$DMNAME"
> > > +		    MOUNTDEVICE="/dev/mapper/${DMNAME}"
> > > +		    MOUNTFS=ext2
> > > +		    "mkfs.ext2" -L "$FSLABEL" -q "${MOUNTDEVICE}"  ;;
> > >  		xf2fs)
> > >  		    "mkfs.f2fs" -l "$FSLABEL" -q "${MOUNTDEVICE}" ;;
> > >  		xnilfs2)
> > > @@ -944,6 +964,22 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE" "$MAXLOGSECSIZE" 1); do
> > >  		    GRUBDEVICE="mduuid/`mdadm --detail --export $MOUNTDEVICE | grep MD_UUID=|sed 's,MD_UUID=,,g;s,:,,g'`";;
> > >  		xlvm*)
> > >  		    GRUBDEVICE="lvm/grub_test-testvol";;
> > > +		xluks*)
> > > +		    if test x"$fs" = xluks2 && ! (cryptsetup luksDump --debug-json --disable-locks $LODEVICE | grep -q "\"sector_size\":$SECSIZE"); then
> > > +			    echo "Unexpected sector size for $LODEVICE (expected: $SECSIZE)"
> > > +			    exit 1
> > > +		    fi
> > > +
> > > +		    UUID=$(cryptsetup luksUUID --disable-locks $LODEVICE | tr -d '-')
> > > +		    PROBE_UUID=$("$GRUBPROBE" --device $MOUNTDEVICE --target=cryptodisk_uuid)
> > > +		    if [ x"$UUID" != x"$PROBE_UUID" ]; then
> > > +			echo "UUID FAIL"
> > > +			echo "$UUID"
> > > +			echo "$PROBE_UUID"
> > > +			exit 1
> > > +		    fi
> > > +		    GRUBDEVICE="cryptouuid/${UUID}"
> > > +		    ;;
> > >  	    esac
> > >  	    GRUBDIR="($GRUBDEVICE)"
> > >  	    case x"$fs" in
> > > @@ -1102,6 +1138,15 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE" "$MAXLOGSECSIZE" 1); do
> > >  		    sleep 1
> > >  		    vgchange -a n grub_test
> > >  		    ;;
> > > +		xluks*)
> > > +		    for try in $(range 0 20 1); do
> > > +			if umount "$MNTPOINTRW" ; then
> > > +			    break;
> > > +			fi
> > > +		    done
> > > +		    UMOUNT_TIME=$(date -u "+%Y-%m-%d %H:%M:%S")
> > > +		    cryptsetup close --disable-locks "$DMNAME"
> > > +		    ;;
> > >  		xmdraid*)
> > >  		    sleep 1
> > >  		    for try in $(range 0 20 1); do
> > > @@ -1152,6 +1197,11 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE" "$MAXLOGSECSIZE" 1); do
> > >  		    mount -t "$MOUNTFS" "${MOUNTDEVICE}" "$MNTPOINTRO" -o ${MOUNTOPTS}${SELINUXOPTS}ro
> > >  		    MOUNTS="$MOUNTS $MNTPOINTRO"
> > >  		    ;;
> > > +		xluks*)
> > > +		    echo -n "$PASS" | cryptsetup open --disable-locks $LODEVICE "$DMNAME"
> > > +		    mount -t "$MOUNTFS" "${MOUNTDEVICE}" "$MNTPOINTRO" -o ${MOUNTOPTS}${SELINUXOPTS}ro
> > > +		    MOUNTS="$MOUNTS $MNTPOINTRO"
> > > +		    ;;
> > >  		xmdraid*)
> > >  		    mdadm --assemble /dev/md/"${fs}_$NDEVICES" $LODEVICES
> > >  		    sleep 1
> > > @@ -1600,6 +1650,9 @@ for LOGSECSIZE in $(range "$MINLOGSECSIZE" "$MAXLOGSECSIZE" 1); do
> > >  		    vgchange -a n grub_test
> > >  		    sleep 1
> > >  		    ;;
> > > +		xluks*)
> > > +		    cryptsetup close --disable-locks "$DMNAME"
> > > +		    ;;
> > >  	    esac
> > >  	    case x"$fs" in
> > >  		x"tarfs" | x"cpio_"* | x"iso9660" | xrockridge | xjoliet | xrockridge_joliet | x"ziso9660" | x"romfs" | x"squash4_"* | x"iso9660_1999" | xrockridge_1999 | xjoliet_1999 | xrockridge_joliet_1999) ;;
> > >