From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.90_1) id 1o0TD6-0005lP-Db for mharc-grub-devel@gnu.org; Sun, 12 Jun 2022 15:25:20 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:52652) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1o0TD4-0005ko-Kq for grub-devel@gnu.org; Sun, 12 Jun 2022 15:25:18 -0400 Received: from out2-smtp.messagingengine.com ([66.111.4.26]:58939) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1o0TD2-0006Kd-2v for grub-devel@gnu.org; Sun, 12 Jun 2022 15:25:18 -0400 Received: from compute3.internal (compute3.nyi.internal [10.202.2.43]) by mailout.nyi.internal (Postfix) with ESMTP id A88735C00AB; Sun, 12 Jun 2022 15:25:13 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute3.internal (MEProxy); Sun, 12 Jun 2022 15:25:13 -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=fm2; t=1655061913; x=1655148313; bh=KOXed9mp/H Z7Joh3td4LVnoYjFGeAgsXbrSWFCRaakM=; b=ausRY/mFrM2zXKBtrUW9FcP0UV 9yb0pOWv8f+1cBKysasT4339r9O7YVE37eHqxRZnMthBRJBJ5ZBJMk5vUsoFROC+ OMp8s0AeCwf7/ovfA06+P+iLqaYPJF1EFC8fSaTvwtx/AghOjk6TPpoa43I2RtGm M6OTjmFOVYW+CLJjUDOGPgzN+jFHtJfCqGmpBcVRdsu/e3cUoveQV0KbAGddq1ir x0DDU5SH75F2TSjfAeKokL+U/QSAqwo6woEsVa6zTuM5uJcm1R2lg5E4a7XKA57m PcrQ47YkiaOBY7jZUnoTE8+bWSC0wUeEq9Hs3XKSSTWnaZ3oFJ76f3qZkGFQ== 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= fm2; t=1655061913; x=1655148313; bh=KOXed9mp/HZ7Joh3td4LVnoYjFGe AgsXbrSWFCRaakM=; b=lCHzkelCXT78ZZvsAglR+1hoTG4GzWiLTwev2CP+qf2N 3RK807/IDiH3CtM3VIP2/g9RQoUKBj8Eb8WRqDNODLha3rLbvzLZiX3V4GVOXXos 8XnmgvvQ0ihbytDozQSONnLhnd5GwCPDBZwSVcZUoV2ofe0iAYO2Oc0Y2x+Aln+N gGLIagtZoNKriUA3XCvwZ4BcXLGsTgJ13iMSqz+WQAA/Gtrq1RUWvuQXqH9M2Uha ol6986LcZGl2J34b0ezw56s4G40/N1j4FTju6t4yl7gQXCXOFxyYWY8epN+pQ/7Y 2bmL6lt7GA4781aGHUcdv9O9DfaadLwp62g7xmBeIw== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvfedrudduhedgudeffecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enfghrlhcuvffnffculddujedmnecujfgurhepfffhvfevuffkfhggtggujgesghdtreer tddtvdenucfhrhhomheprfgrthhrihgtkhcuufhtvghinhhhrghrughtuceophhssehpkh hsrdhimheqnecuggftrfgrthhtvghrnhephfeiieeuvdffveeigeejjeffkeektdehleek vdekheeilefgkeefvdegtdekfeeinecuffhomhgrihhnpehgnhhurdhorhhgnecuvehluh hsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepphhssehpkhhsrdhi mh X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Sun, 12 Jun 2022 15:25:11 -0400 (EDT) Received: from localhost (xps [10.192.0.12]) by vm-mail.pks.im (OpenSMTPD) with ESMTPSA id 24b0fb51 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Sun, 12 Jun 2022 19:25:04 +0000 (UTC) Date: Sun, 12 Jun 2022 21:25:03 +0200 From: Patrick Steinhardt To: Glenn Washburn Cc: The development of GNU GRUB , Josselin Poiret , Pierre-Louis Bonicoli , 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> <20220605134318.075d9e87@crass-HP-ZBook-15-G2> <20220606121139.38c18779@crass-HP-ZBook-15-G2> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="JH0RSOPsjoHHbClc" Content-Disposition: inline In-Reply-To: <20220606121139.38c18779@crass-HP-ZBook-15-G2> Received-SPF: pass client-ip=66.111.4.26; envelope-from=ps@pks.im; helo=out2-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, RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001, 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, 12 Jun 2022 19:25:19 -0000 --JH0RSOPsjoHHbClc Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Jun 06, 2022 at 12:11:39PM -0500, Glenn Washburn wrote: > On Mon, 6 Jun 2022 07:32:40 +0200 > Patrick Steinhardt wrote: >=20 > > On Sun, Jun 05, 2022 at 01:43:18PM -0500, Glenn Washburn wrote: > > > On Sun, 29 May 2022 09:09:38 +0200 > > > Patrick Steinhardt wrote: > > >=20 > > > > 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 w= ork-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 co= nstant > > > > > > > (LUKS1 also sets most of the other properties of the cryptodi= sk device, > > > > > > > like crypto algorithm, because they are in the binary header)= =2E However, > > > > > > > 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 sec= tor 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 t= he other > > > > > > > properties, like crypto algorithm, because that depends on th= e keyslot > > > > > > > used, which needs key recovery to be determined. To avoid par= sing the > > > > > > > json data twice, once in scan() and once in recover_key(), wh= ich should > > > > > > > be avoided, the parsed json object could be put in the grub_c= ryptodisk_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 ev= en if > > > > > > > recover_key() is never called. > > > > > > > > > > > > > > I think the above is the real fix, a moderate amount more wor= k, 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. > > > > > > > > > > > > > > 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 fi= nding the > > > > > > right slot. This is roughly what's done in patch 2 of [1], may= be this > > > > > > workaround would be more to your liking? > > > > >=20 > > > > > Hah! Yes, thanks for reminding me. I forgot I'd suggested this an= d its > > > > > a better approach than what I suggested above. And probably the o= ne 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 sen= d 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/msg00= 079.html > > > > > > (20211211122945.6326-1-dev@jpoiret.xyz) > > > > >=20 > > > > > Glenn > > > >=20 > > > > I very much agree that we should land the test-patches regardless of > > > > what happens to the rest: they cover an important test gap. > > > >=20 > > > > Other than that the patches look sane to me. The biggest question t= o me > > > > is which of the three patch series we want to include in the end: > > > >=20 > > > > - Yours has the extra benefit of added tests, but these can go = in > > > > independently. > > > >=20 > > > > - Josselin's patches [1] have the benefit that they try to deri= ve a > > > > "proper" sector size via device-mapper. > > > >=20 > > > > - My own patches [2] include two additional patches: one to str= ip > > > > dashes of the UUID so that findfs is easier to use and the sa= me > > > > 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 li= ke > > > > better about this patch series is that it clearly discerns LU= KS > > > > and LUKS2 devices. > > > >=20 > > > > 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 approac= h 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. > > >=20 > > > I think[1] Fabian's patch[2] for getting sector size is actually bett= er > > > and more general than using DM. My preference is for a combination of > > > Fabian's and Josselin's patches. > >=20 > > I also noted that my own two orthogonal patches to fix the out-of-bounds > > copy and dash-stripping have landed in `master` already. So I agree that > > a combination of the other two series would be best. >=20 > I think those should be a different series, since they are orthogonal. > I prefer my patch series for handling uuids[1], which allows searching > with dashes. This would allow the user to search using uuids that > contain dashes (or without dashes for backwards compatibility), which > would be consistent with filesystem uuid searching. >=20 > Glenn >=20 > [1] https://lists.gnu.org/archive/html/grub-devel/2022-02/msg00101.html I think we misunderstood. The two patches already _are_ in the `master` branch: - ee12785f7 (luks2: Strip dashes off of the UUID, 2020-05-30) - 1066336dc (luks: Fix out-of-bounds copy of UUID, 2020-09-07) So there's nothing that needs to be done here anymore. Patrick --JH0RSOPsjoHHbClc Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCgAdFiEEF9hrgiFbCdvenl/rVbJhu7ckPpQFAmKmPY4ACgkQVbJhu7ck PpTPKA//bxmhe/bEtxYUkpIwzBNBN1+nJ/3tPIYa6UA4QDqhkQEf+eflP2zSPFVz 6u8byuy3FmIjEqnSOVj0DNzcil1E4V3xMKPEswGBWvZZQyz2rv1K0XxpM0t6e2pt XnzaSGiKc099zuNs07C4hqV+/3feFJaUD70tARylfnmBehfg2KK6QvuZZTeK/DPX HyFFR1hh+EpoAMj+J1KH+qFh9pDInrp3bKRgOYdkngNWpeRDEGSQWMLUB+gEWNCC eul2/8X+oZHfERUhFzjMSwQgsrCrJleyTj2TWVYyhfiQjbwzokgqomTwTOBV6H+e 4roxPCcTCaQ+W+CVR997gF82eVsIfFLWM5TLouhNGRtneRc0qI5C9Z0zdtE+TTeg g6snCPDPvuBvqnvLoAI0s0O8oWSfPOUSz5mfYz35YsWd2Pd41knjz8g5lgsqQYTN c/t503u0OZQo5Rc+HinGuuBKRSsSdIclN3qFJTs57LpPGzbBxI3hRYgA6YRZhIoo nS/abPKuKlV6JajFjzU3WHFIgeFhlQTT/AXLkXBwos2GmvNZ2D7/70hD5IEAtiTA slojHu/knjIAYwnlpEKnRfa1+OqKtsRQ2EGtQBrF0NoE+/PatUYu+uwSbkB8Bs9p f4SqH5kWinVaRdEn7dZI8pj+3o5wzfoxTn3Oy510K+FTN+OtgaM= =KMOC -----END PGP SIGNATURE----- --JH0RSOPsjoHHbClc--