Re: [CRYPTO-LUKS v1 03/19] cryptodisk: Incorrect calculation of start sector for grub_disk_read in grub_cryptodisk_read.

[Glenn Washburn]

On Mon, 24 Aug 2020 07:10:51 +0200
Patrick Steinhardt <ps@pks.im> wrote:

> On Sun, Aug 23, 2020 at 11:31:46PM -0500, Glenn Washburn wrote:
> > On Sun, 23 Aug 2020 12:39:03 +0200
> > Patrick Steinhardt <ps@pks.im> wrote:
> > 
...
> > > So the fix does seem correct to me, but I think it's incomplete as
> > > we'd have to do the same for `grub_cryptodisk_write`.
> > 
> > Yep, good catch. I didn't even think to check for
> > grub_cryptodisk_write since I tend to think of grub as read-only.
> > I'm actually not sure how to trigger a disk write in grub.  The
> > only thing that comes to mind is I think there's a way to set some
> > partition flags.  
> 
> I'd assume that things like the grub.env file get updated based e.g.
> on the last chosen boot entry. Which would require support to write to
> disk. But I honestly don't now and have never used write-functionality
> in GRUB.
> 
> Do you want to update your patch to also fix the write part? I'd then
> pull it into a v2 of my patch series for v2.06 fixes.

I sent a single updated patch as opposed to resending the whole patch
set.  I hope that was what was expected.

> > > Out of curiosity: did you test these changes?
> > 
> > Yes, these changes have definitely been tested.  In fact, I found
> > these bugs precisely because I was trying to get grub to decrypt my
> > LUKS2 device which has a 4096 byte sector size.  Feeling like I
> > wasn't getting much traction on my patches, I wrote the
> > CRYPTOMOUNT-TEST patch series to allow independent verification of
> > the bugs and my fixes. There are a series of tests for block sizes
> > 1024, 2048, and 4096 which all are successful for me (all the
> > sector size fixes are needed).
> 
> I'll have to have a look at them. Improved testability is definitely
> something I'm interested in, mostly because I found the manual
> compile-boot-test cycle quite unwieldy, even with virtualization.

When you get around to them, here are some tips I learned.  The full
test suite takes forever to run, so I only run the cryptomount test
using the following command in project root.  Also you'll need to be
root because the tests use dm-crypt to open generated LUKS volumes.  If
you know of a decent way to avoid this, I'd love to hear about it.

  sudo env TESTS="grub_cmd_cryptomount" TEST_SUITE_LOG=partial.log \
          make -e check VERBOSE=yes SUBDIRS=

Glenn