From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.90_1) id 1nvD3e-0001jA-PV for mharc-grub-devel@gnu.org; Sun, 29 May 2022 03:09:50 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:59380) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1nvD3c-0001h1-0l for grub-devel@gnu.org; Sun, 29 May 2022 03:09:48 -0400 Received: from wout5-smtp.messagingengine.com ([64.147.123.21]:50555) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1nvD3Z-0006vB-QD for grub-devel@gnu.org; Sun, 29 May 2022 03:09:47 -0400 Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailout.west.internal (Postfix) with ESMTP id A87BB3200805; Sun, 29 May 2022 03:09:43 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute5.internal (MEProxy); Sun, 29 May 2022 03:09:44 -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=1653808183; x=1653894583; bh=RB6brGVm/q wSZwl8ebiBeeZjHwQ2VIwbS9OuDopHLnA=; b=eCLlh7dobtDP4cS8X0RfSdIiNQ 952Ds3RVDNt5wJ2APW5IRDImAChYYTfC+WbVo5cd6VDlBg3LzugtnjccYqNrd7WD lmVQ/SdqKuKHnysY+BEzdOEFD1holCVq9qQ5fx63ysTCktDtJG0lJ7zWGpwiA3tT WIprW7edv+gVxJIJe3a6iH1o9J8d0gOnhk/LTa6hkueVSA4Iu6mbqUR/oBTgeK0u jVuV0AcpCPjXkmMkTOEV6oXK9nJYOSZt2hpst3wGEHDT6zF4bgb7Jz9a8lJJebYr tl0SxLl17c3rTjXOZRci4nD81UZlpST3XxOZEgIgDFL9xrqXzuR66a3K0zOw== 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=1653808183; x=1653894583; bh=RB6brGVm/qwSZwl8ebiBeeZjHwQ2 VIwbS9OuDopHLnA=; b=DTH0kAnLhpY9oFAeuhKaU1SaEEe39Sf6kQI8ytgJ4xj2 7Ne50JMjtFB36bphbwAbwCBoypXzGCXAndSPtd0nEEt/rjDGvLW6gTKLfmxcKRuZ tCIUDvdrEDUh1bMdkWeYXVEcQd5LgvA0XbR39NLcdpiEQW9mpYscFGGI75iafVFC RkJGvCr/aDnSrQ+3mdxxX28udsBNkcIZyUs3PmyJU09rn8rz4Xu/rMLHp+PIm/PI fJUUNzYj7FK+LBQHRCJzva/KxhRGSRT1qLPHxALpT3mH7rh1SvdxPMfWNRLsq33+ j6+ajs911p1jnPTOQEINBGk8uWGbol4I/0qr5/08Kw== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvfedrkeefgdduudcutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenfg hrlhcuvffnffculddujedmnecujfgurhepfffhvfevuffkfhggtggujgesghdtreertddt vdenucfhrhhomheprfgrthhrihgtkhcuufhtvghinhhhrghrughtuceophhssehpkhhsrd himheqnecuggftrfgrthhtvghrnhepfeevfeduleffffduffelteffjedtvdejudeufedt geetueekvefftdffudffffeinecuffhomhgrihhnpehgnhhurdhorhhgpdhpkhhsrdhimh enucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepmhgrihhlfhhrohhmpehpshes phhkshdrihhm X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Sun, 29 May 2022 03:09:41 -0400 (EDT) Received: from localhost (xps [10.192.0.12]) by vm-mail.pks.im (OpenSMTPD) with ESMTPSA id f95cab28 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Sun, 29 May 2022 07:09:39 +0000 (UTC) Date: Sun, 29 May 2022 09:09:38 +0200 From: Patrick Steinhardt To: The development of GNU GRUB Cc: Josselin Poiret , Pierre-Louis Bonicoli , Glenn Washburn , Daniel Kiper 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> <87ee12tpsd.fsf@jpoiret.xyz> <20220510225552.1b9aaa94@crass-HP-ZBook-15-G2> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="ORbKx4B+R9P8SVPK" Content-Disposition: inline In-Reply-To: <20220510225552.1b9aaa94@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 07:09:49 -0000 --ORbKx4B+R9P8SVPK Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, May 10, 2022 at 10:55:52PM -0500, Glenn Washburn wrote: > On Mon, 09 May 2022 22:27:30 +0200 > Josselin Poiret wrote: >=20 > > Hello everyone, > >=20 > > Glenn Washburn writes: > >=20 > > > I don't really like this, but it gets the job done and is a work-arou= nd > > > for a peculiarity of the LUKS2 backend. The cheat mount code for > > > cryptodisk does only calls scan() and not recover_key(). For LUKS1 sc= an > > > 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 devic= e, > > > like crypto algorithm, because they are in the binary header). Howeve= r, > > > for LUKS2 the sector size (along with other properties) is in the json > > > header, which isn't getting parsed in scan().=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 t= he > > > 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 shou= ld > > > be avoided, the parsed json object could be put in the grub_cryptodis= k_t > > > in scan(), and used and freed in recover_key(). We'd probably also wa= nt > > > 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. > > > > > > I think the above is the real fix, a moderate amount more work, and n= ot > > > 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. > > > > > > 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 > > Regarding these last lines, it's also possible to directly ask dm for > > the actual sector size when cheatmounting, as well as the crypto > > algorithm, bypassing the whole issue of parsing the json and finding the > > right slot. This is roughly what's done in patch 2 of [1], maybe this > > workaround would be more to your liking? >=20 > Hah! Yes, thanks for reminding me. I forgot I'd suggested this and its > a better approach than what I suggested above. And probably the one I'd > support, but I need to more thoroughly take a look at it. >=20 > > I've distributed this patch to several people that were having issues on > > GNU Guix and they've been happily using LUKS2 with GRUB with it. >=20 > Yes, this does work and too much of a hack as-is. Regardless, your > contribution is appreciated. I'd like to get your patch with the GRUB fs > tests merged. Do you want to make the changes I suggested and send a new > patch here? If not, at some point I'll probably make them myself and > submit it to the list. >=20 > > [1] https://lists.gnu.org/archive/html/grub-devel/2021-12/msg00079.html > > (20211211122945.6326-1-dev@jpoiret.xyz) >=20 > Glenn I very much agree that we should land the test-patches regardless of what happens to the rest: they cover an important test gap. Other than that the patches look sane to me. The biggest question to me is which of the three patch series we want to include in the end: - Yours has the extra benefit of added tests, but these can go in independently. - Josselin's patches [1] have the benefit that they try to derive a "proper" sector size via device-mapper. - My own patches [2] include two additional patches: one to strip dashes of the UUID so that findfs is easier to use and the same across LUKS and LUKS2. And one out-of-bounds copy of the UUID in LUKS. Both are kind of orthogonal though. One more thing I like better about this patch series is that it clearly discerns LUKS and LUKS2 devices. So ultimately it feels like all of the patch series have their own advantages, but they should be combinable. The tests and my own orthogonal patches can be split out. And if we combined the approach in Josselin's patches to use DM to get the sector size with a proper conceptual split of LUKS and LUKS2 as in my own patches then I'd be more than happy. I may very well be biased here though given that one of the patch series is my own. Patrick [1]: <20220520182039.21654-1-dev@jpoiret.xyz> [2]: --ORbKx4B+R9P8SVPK Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCgAdFiEEF9hrgiFbCdvenl/rVbJhu7ckPpQFAmKTHDEACgkQVbJhu7ck PpRHJA//ZLQPd7RwP+pCJiTLDbgtBBYhtqO9wTQg0Wvsac3bDB3VQlYvWC/+iS66 SoaC1zVSMIF7QJcjDRkjExenEDTyBDIhJgcL2gjPDRKLfCjM8Gmt8BlwrksjUrU5 4nmnkzG3Tstrubz4z3yjEeh34cAFG9Neem1Gl9C8GfCiqnOi2okVcNCItQLoLAGf b0JbGT/6VjZuOYzvJ0ATQQp/ReGULoCCcNTNwcqJfMifWFLSVYsYvpBA1RAV63UY TAEu6XatSJp8eN3/6apCYfO5Fh+/LrTNDtZUBL95L3iToiBC+/N8c01kO9kUncXO my8JjDwm82g666nKiGfHjaokFEZAdZq6Ustk4M02lINuYn6iPlAvoHETcLtOPZWC STweZkyAaSuNl+MTiU69oam6XgWMG8B9waMbDuUWzAL8E6lblULeUgTD1aMwa2QA C2+RF7M+7iQykMor+rdWBmNPoObi2+HPulo0jIzkio7MNkNsKL7FEsTH/gaPSmAH YPAP5R9xQDV3uQG77J/3XTTjzSUpzBc6j4UKlhs7cqJnVt3sjtJ0XVeAlNxtHhIT fCqI1uXQTesXfcvvV8kpbF9JivT8sm0t+CRd8nWjg9tYbL4rxDWwjGq42q2FL4UO HAWsD6sYFr8ieVm1P1aSkW+HiwtRvhCi8TgmSKmcdLcliOHyWT8= =vlH1 -----END PGP SIGNATURE----- --ORbKx4B+R9P8SVPK--