From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.90_1) id 1nvCsk-0006mg-0F for mharc-grub-devel@gnu.org; Sun, 29 May 2022 02:58:34 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:58636) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1nvCsi-0006mR-Cs for grub-devel@gnu.org; Sun, 29 May 2022 02:58:32 -0400 Received: from wout5-smtp.messagingengine.com ([64.147.123.21]:49997) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1nvCsg-0005do-68 for grub-devel@gnu.org; Sun, 29 May 2022 02:58:32 -0400 Received: from compute2.internal (compute2.nyi.internal [10.202.2.46]) by mailout.west.internal (Postfix) with ESMTP id BB483320089C; Sun, 29 May 2022 02:58:27 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute2.internal (MEProxy); Sun, 29 May 2022 02:58:27 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc:cc :content-type:date:date:from:from:in-reply-to:in-reply-to :message-id:mime-version:references:reply-to:sender:subject :subject:to:to; s=fm1; t=1653807507; x=1653893907; bh=D9Akd0UGlI 3emgJnWcf2TJtzCQDXJPJMy6VVwtX/5ws=; b=pVTlHB4F4HRIzKJ+qK+AzqaZWQ 2PWuIBHG0d5x7DVPeNBhnjIGE+J12J58bDUcjzGZd14zBRdv3ZOQm1RCy1zeydCh W5vfFNBCIfhSck2wDKlM9luO8umyzU9laF2U6nrY7rZqlbmXBnx+oVBI5nSbd0Zc gmPvmJSmUzXOQft0DvA/uCSxm9IZJoJJBzSgzHYn7+daLSmApbne1WhCiqHUViiW /GtECx84wJwaYxXIVK/arKnTS1DdY7EBb/nxFFQTs+7jyd1GLFFPVafRK4qx4t4D xdJfdRyRWVIkgFao4OKY40JoyIVtOF66CRkuH27Ou6amNiT3ShjJHq9TCizg== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:cc:content-type:date:date:feedback-id :feedback-id:from:from:in-reply-to:in-reply-to:message-id :mime-version:references:reply-to:sender:subject:subject:to:to :x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s= fm1; t=1653807507; x=1653893907; bh=D9Akd0UGlI3emgJnWcf2TJtzCQDX JPJMy6VVwtX/5ws=; b=SjUYtd5sYuGTA83uhLrsjp33kvrUxFbcJ362Oj+QzzHf 5ZRefE2m8XBmrEOgW16qy8bPsOAyHnWQ1lW7Couj+8VpPY9ifyM9phIm8mdlFEkw 1gfzcGy0G5eLstZBxfMMg7FAJyjvHxRM2FYKzhKiiyQXrF55/Lh1T9dKDfu9vXaU 9oK0fkpo4rGVHeENfzcL5IdsIRl006QvB9NUsGLDvFH+cWsjy6LDcV2CHU1gIhPo QInDhHwF3mDJJbNTv/KqIMwEyIcSrECKQNN71WC2XbesZJm0p9X1PBmQ2jME2MMb l8kQ9+rRX/v70WIxHX5p1rUpqQBTQeMME4+YbKx1Ag== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvfedrkeefgddutdcutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpeffhffvvefukfhfgggtuggjsehgtderredttddvnecuhfhrohhmpefrrghtrhhi tghkucfuthgvihhnhhgrrhguthcuoehpshesphhkshdrihhmqeenucggtffrrghtthgvrh hnpefgfefftdejueeugfeliedvieeuudeludelhefghedtledvueeuvedttdfhtdduveen ucffohhmrghinhepfhhoshhsihgvshdrohhrghdpghhnuhdrohhrghenucevlhhushhtvg hrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehpshesphhkshdrihhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Sun, 29 May 2022 02:58:26 -0400 (EDT) Received: from localhost (xps [10.192.0.12]) by vm-mail.pks.im (OpenSMTPD) with ESMTPSA id 83777001 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Sun, 29 May 2022 06:58:21 +0000 (UTC) Date: Sun, 29 May 2022 08:58:20 +0200 From: Patrick Steinhardt To: The development of GNU GRUB Cc: Pierre-Louis Bonicoli Subject: Re: [PATCH v2 3/3] grub-core/kern/disk.c: handle LUKS2 devices Message-ID: References: <20220329103158.4096409-1-pierre-louis.bonicoli@libregerbil.fr> <20220329103158.4096409-4-pierre-louis.bonicoli@libregerbil.fr> <20220504164708.5322406a@crass-HP-ZBook-15-G2> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="p4ol/eh9JeJrHc3v" Content-Disposition: inline In-Reply-To: <20220504164708.5322406a@crass-HP-ZBook-15-G2> Received-SPF: pass client-ip=64.147.123.21; envelope-from=ps@pks.im; helo=wout5-smtp.messagingengine.com X-Spam_score_int: -27 X-Spam_score: -2.8 X-Spam_bar: -- X-Spam_report: (-2.8 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 29 May 2022 06:58:32 -0000 --p4ol/eh9JeJrHc3v Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, May 04, 2022 at 04:47:08PM -0500, Glenn Washburn wrote: > On Tue, 29 Mar 2022 12:31:58 +0200 > Pierre-Louis Bonicoli wrote: >=20 > > Unlike LUKS1, the sector size of LUKS2 devices isn't hardcoded. > >=20 > > Regarding the probe command, the following values of --target switch > > are affected: abstraction, arc_hints, baremetal_hints, bios_hints, > > cryptodisk_uuid, drive, efi_hints, hints_string, ieee1275_hints, > > zero_check. > >=20 > > For example using the --target=3Ddrive option: > >=20 > > # dd if=3D/dev/zero of=3Ddata count=3D10 bs=3D1M > > # losetup --show -f data > > /dev/loop4 > > # echo -n pass | cryptsetup luksFormat -v --type luks2 /dev/loop4 > > Key slot 0 created. > > Command successful. > > # echo -n pass | cryptsetup -v open /dev/loop4 test > > No usable token is available. > > Key slot 0 unlocked. > > Command successful. > > # grub-probe --device /dev/mapper/test --target=3Dcryptodisk_uuid > > grub-probe: error: disk `cryptouuid/f353c0f04a6a4c08bc53a0896130910f'= not found. > >=20 > > The updated output: > >=20 > > # grub-probe --device /dev/mapper/test --target=3Dcryptodisk_uuid > > f353c0f04a6a4c08bc53a0896130910f > > --- > > grub-core/kern/disk.c | 4 +++- > > grub-core/osdep/devmapper/getroot.c | 3 ++- > > 2 files changed, 5 insertions(+), 2 deletions(-) > >=20 > > diff --git a/grub-core/kern/disk.c b/grub-core/kern/disk.c > > index 3a42c007b..fa3177bf0 100644 > > --- a/grub-core/kern/disk.c > > +++ b/grub-core/kern/disk.c > > @@ -237,8 +237,10 @@ grub_disk_open (const char *name) > > name); > > goto fail; > > } > > - if (disk->log_sector_size > GRUB_DISK_CACHE_BITS + GRUB_DISK_SECTOR_= BITS > > + if ((disk->log_sector_size > GRUB_DISK_CACHE_BITS + GRUB_DISK_SECTOR= _BITS > > || disk->log_sector_size < GRUB_DISK_SECTOR_BITS) > > + /* log_sector_size is unset for LUKS2 and that's ok */ > > + && !(disk->log_sector_size =3D=3D 0 && dev->id =3D=3D GRUB_DISK_= DEVICE_CRYPTODISK_ID)) >=20 > I don't really like this, but it gets the job done and is a work-around > for a peculiarity of the LUKS2 backend. The cheat mount code for > cryptodisk does only calls scan() and not recover_key(). For LUKS1 scan > will return a grub_cryptodisk_t with log_sector_size set, but LUKS2 > will not. This is because for LUKS1 the log_sector_size is constant > (LUKS1 also sets most of the other properties of the cryptodisk device, > like crypto algorithm, because they are in the binary header). However, > for LUKS2 the sector size (along with other properties) is in the json > header, which isn't getting parsed in scan().=20 >=20 > For single segment LUKS2 containers, scan() could get the sector size > from the json segment object. The LUKS2 spec says that normal LUKS2 > devices are single segment[1], so this should work in the the cases the > care about (currently). scan() would not be able to fill in the other > properties, like crypto algorithm, because that depends on the keyslot > used, which needs key recovery to be determined. To avoid parsing the > json data twice, once in scan() and once in recover_key(), which should > be avoided, the parsed json object could be put in the grub_cryptodisk_t > in scan(), and used and freed in recover_key(). We'd probably also want > to add a way for grub_cryptodisk_t objects to get cleaned up by the > backend using them, so that the json object could be freed even if > recover_key() is never called. >=20 > I think the above is the real fix, a moderate amount more work, and not > something I'd expect Pierre-Louis to take up. So if we're not going to > do this to get this functionality to work, we'll need a hack to get it > working. However, I'd prefer a different one. >=20 > I've not tested this, but it seems to me that we can set the > log_sector_size field to GRUB_DISK_SECTOR_BITS _if_ it equals zero in > grub_cryptodisk_cheat_insert(). This limits the hack to only GRUB > host/user-space code. >=20 > [1] https://fossies.org/linux/cryptsetup/docs/on-disk-format-luks2.pdf, > section 3.3 >=20 > > { > > grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET, > > "sector sizes of %d bytes aren't supported yet", > > diff --git a/grub-core/osdep/devmapper/getroot.c b/grub-core/osdep/devm= apper/getroot.c > > index 96781714c..4f51c113c 100644 > > --- a/grub-core/osdep/devmapper/getroot.c > > +++ b/grub-core/osdep/devmapper/getroot.c > > @@ -180,7 +180,8 @@ grub_util_pull_devmapper (const char *os_dev) > > grub_util_pull_device (subdev); > > } > > } > > - if (uuid && strncmp (uuid, "CRYPT-LUKS1-", sizeof ("CRYPT-LUKS1-") -= 1) =3D=3D 0 > > + if (uuid && (strncmp (uuid, "CRYPT-LUKS1-", sizeof ("CRYPT-LUKS1-") = - 1) =3D=3D 0 > > + || strncmp (uuid, "CRYPT-LUKS2-", sizeof ("CRYPT-LUKS2-") - 1) = =3D=3D 0) >=20 > It seems better to me to not add another strncmp, but to only check for > the prefix "CRYPT-LUKS". This way when LUKS3 comes out next decade we > won't have to add another strncmp here. I'd actually argue the other way round: I'd rather be defensive and not pretend that we can handle LUKS3, because chances are high that we won't handle it correctly. Patrick > If we do want to keep this, I'd like to see '||' aligned with the start > of "strncmp" of the line above, even though it will push the line past > 80 chars by a few chars. At a minimum indent more than the line below. >=20 > > && lastsubdev) > > { > > char *grdev =3D grub_util_get_grub_dev (lastsubdev); >=20 > Glenn >=20 > _______________________________________________ > Grub-devel mailing list > Grub-devel@gnu.org > https://lists.gnu.org/mailman/listinfo/grub-devel --p4ol/eh9JeJrHc3v Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCgAdFiEEF9hrgiFbCdvenl/rVbJhu7ckPpQFAmKTGYsACgkQVbJhu7ck PpSdkhAAi+YM8ifn4OZfuh40nygVM4XTOh6BGVyRGWGXASoasIDhpClLedWpZ9lk 3ZenwVlGnIieqCRpjPTZrUNNZsTPt2DXRGNxX0fd2t5It8mYHyZjK3aMgXrWyZvD eF/NyrOsvOmWqTNpo96LtOMrJfCeAAH5ZMEdJhAObkDSxz8+sVPu5TfH2voY8yqf tNPr/d+W5BAY7Iel5I1QGu8SKNra7rLIDqCQLUaLzZzTbZdYCr6Azgty71dO8r6p WPib9K8p5nWX+NmeryAZvLk7HJF2t2sHM8xfraB3DpLVu7f7UnaR6WnRIOdLSLpZ z9GpubMNniLU7y1CTso0JCrVh4xr40DbfS5taRTUY683qKYPknbUuGjTzPMp0iRl kaER+MWWlXhz9bJ5TnAvSgFsT1UfI6W5tjWHKTK0xwBqWeDdcBTpWzPd+Oi1av+b ldltcgx4ae6YEidbbi2UWtj0ereClMHmvaCMMeFgpbFioVywnuA0dHPB5on6JpP0 u8HwdiDrqA108bAP/V15fdzSx2TWFmpkolWHrt0Frl3s0Yu+gVUKjgm1PrqZMAeJ 3Ot8/IKjd/igJ9fe1EJ3u35XRRwCxSQx0kZE39UqnoF/ur0Ew1cd0QAD+pVTNWKV ejPjXitaouN5Q/7zFDTtoaSgnD9GBe5MA8HQMCLIGkKRZ+UoeSM= =UWO0 -----END PGP SIGNATURE----- --p4ol/eh9JeJrHc3v--