From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.90_1) id 1mKlTB-0003NF-8B for mharc-grub-devel@gnu.org; Mon, 30 Aug 2021 13:53:17 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:35620) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mKlT9-0003MU-0Y for grub-devel@gnu.org; Mon, 30 Aug 2021 13:53:15 -0400 Received: from out3-smtp.messagingengine.com ([66.111.4.27]:33577) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mKlT6-0004FE-Sa for grub-devel@gnu.org; Mon, 30 Aug 2021 13:53:14 -0400 Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id 5CD7D5C0159; Mon, 30 Aug 2021 13:53:11 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute1.internal (MEProxy); Mon, 30 Aug 2021 13:53:11 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=date :from:to:cc:subject:message-id:references:mime-version :content-type:in-reply-to; s=fm1; bh=r4pD8KEyKvBh0jlGKGjvmKrgDdq hqywmiCG01n0XtAU=; b=Nfhilue2rG6HshTXn36lXF/WKHoyPcW+4SUieeJwr7R wgS9GvxHwwzCm9yEYbMVNpK55GbxO1T7Ty0HWaVPzK6HCqhao1cvMHinIPMQDn1T QfL3eH5byDe2CyR5M0l/+AiQIhFKugT6YeEyog4rfWWiLr8Xdr1ka1zt2mqQVM8F 5XFum0fJHZtV5BeqQfEvhX07mnIC9LvM2F52HHborsxzCKFuqAiUKw+7KXAOw0ES 0ub3Xk7eVfZ8rduzg2NF+wDApSWayg1icDbgtzdpHGkQerCpD2NsBT8f9pdJW+FR n7Xm2uSH8w6onuzyoDyau49YBB9tQNVbrp6aGfgxTkg== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to:x-me-proxy :x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=fm3; bh=r4pD8K EyKvBh0jlGKGjvmKrgDdqhqywmiCG01n0XtAU=; b=GAgEe3AAE5LWlMfeq2jYYq eetlhyPvonBNEJT8QVuiybGwd3b/B9vacae0SYlqr1OL8XZfOnKQv/6Ylacl/dwI Dgi78UPdC/RD1h+q7nsY5NIAVp0ytCrYFewgh2Bc0R6rlUDq1FC8Ko4vWsFuSVvW ryjFATsRy0OX9HxkV/tpniDHW/OcaOgHgr3EiucD3ChxeKr1GMy+EiPBpYrwCgHZ xdVUOeANniRXSidUzVb9w2jtKUxfufX6ZioaGWSZx86THhJ5xeP2kC1rksgoik1d Hhtkq3ZnHaeko3FG8itUAp7QLJG/XlPJj3NnPLrVxydBGi/YAWoe7UatrzB1bgDg == X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvtddrudduledguddukecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpeffhffvuffkfhggtggujgesghdtreertddtvdenucfhrhhomheprfgrthhr ihgtkhcuufhtvghinhhhrghrughtuceophhssehpkhhsrdhimheqnecuggftrfgrthhtvg hrnhepheeghfdtfeeuffehkefgffduleffjedthfdvjeektdfhhedvlefgtefgvdettdfh necuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepphhsse hpkhhsrdhimh X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 30 Aug 2021 13:53:10 -0400 (EDT) Received: from localhost (tanuki [10.192.0.23]) by vm-mail.pks.im (OpenSMTPD) with ESMTPSA id 7d928ae1 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Mon, 30 Aug 2021 17:53:06 +0000 (UTC) Date: Mon, 30 Aug 2021 19:55:59 +0200 From: Patrick Steinhardt To: Glenn Washburn Cc: grub-devel@gnu.org, Daniel Kiper Subject: Re: [PATCH 1/3] cryptodisk: Add infrastructure to pass data from cryptomount to cryptodisk modules Message-ID: References: MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="Fb5y3s36qYbhvXLY" Content-Disposition: inline In-Reply-To: Received-SPF: pass client-ip=66.111.4.27; envelope-from=ps@pks.im; helo=out3-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_H2=-0.001, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 30 Aug 2021 17:53:15 -0000 --Fb5y3s36qYbhvXLY Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Aug 26, 2021 at 12:08:50AM -0500, Glenn Washburn wrote: > As an example, passing a password as a cryptomount argument is implemente= d. > However, the backends are not implemented, so testing this will return a = not > implemented error. >=20 > Signed-off-by: Glenn Washburn > --- > grub-core/disk/cryptodisk.c | 31 ++++++++++++++++++++++--------- > grub-core/disk/geli.c | 4 ++++ > grub-core/disk/luks.c | 4 ++++ > grub-core/disk/luks2.c | 4 ++++ > include/grub/cryptodisk.h | 8 ++++++++ > 5 files changed, 42 insertions(+), 9 deletions(-) >=20 > diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c > index 90f82b2d3..b966b19ab 100644 > --- a/grub-core/disk/cryptodisk.c > +++ b/grub-core/disk/cryptodisk.c > @@ -41,6 +41,7 @@ static const struct grub_arg_option options[] =3D > /* TRANSLATORS: It's still restricted to cryptodisks only. */ > {"all", 'a', 0, N_("Mount all."), 0, 0}, > {"boot", 'b', 0, N_("Mount all volumes with `boot' flag set."), 0, 0= }, > + {"password", 'p', 0, N_("Password to open volumes."), 0, ARG_TYPE_ST= RING}, > {0, 0, 0, 0, 0, 0} > }; > =20 > @@ -996,7 +997,9 @@ cryptodisk_close (grub_cryptodisk_t dev) > } > =20 > static grub_err_t > -grub_cryptodisk_scan_device_real (const char *name, grub_disk_t source) > +grub_cryptodisk_scan_device_real (const char *name, > + grub_disk_t source, > + grub_cryptomount_args_t cargs) > { > grub_err_t err; > grub_cryptodisk_t dev; > @@ -1015,7 +1018,9 @@ grub_cryptodisk_scan_device_real (const char *name,= grub_disk_t source) > if (!dev) > continue; > =20 > + *dev->cargs =3D *cargs; > err =3D cr->recover_key (source, dev); > + *dev->cargs =3D NULL; > if (err) > { > cryptodisk_close (dev); Is there any specific reason why you choose to set `cargs` as a struct field? Seeing uses like this makes me wonder whether it wouldn't be better to pass in `cargs` as explicit arguments whenever required. This would also automatically answer any lifetime questions which arise. [snip] > diff --git a/grub-core/disk/luks.c b/grub-core/disk/luks.c > index 13103ea6a..e2a4a3bf5 100644 > --- a/grub-core/disk/luks.c > +++ b/grub-core/disk/luks.c > @@ -165,6 +165,10 @@ luks_recover_key (grub_disk_t source, > grub_size_t max_stripes =3D 1; > char *tmp; > =20 > + /* Keyfiles are not implemented yet */ > + if (dev->cargs->key_data || dev->cargs->key_len) > + return GRUB_ERR_NOT_IMPLEMENTED_YET; > + > err =3D grub_disk_read (source, 0, 0, sizeof (header), &header); > if (err) > return err; Hum. This makes me wonder whether we'll have to adjust all backends whenever we add a new parameter to `cargs` to return `NOT_IMPLEMENTED`. I fear that it won't scale nicely, and that it is a recipe for passing unsupported arguments to backends even though they don't know to handle them. Not sure about a nice alternative though. The only idea I have right now is something like a capabilities bitfield exposed by backends: only if a specific bit is set will we pass the corresponding field to such a backend. This would allow us to easily centralize the logic into a single function which only receives both the capabilities bitset and the `cargs` struct. Other than that I really like where this is going. Patrick --Fb5y3s36qYbhvXLY Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCgAdFiEEF9hrgiFbCdvenl/rVbJhu7ckPpQFAmEtG64ACgkQVbJhu7ck PpTUgw//TGGirGnWnI79/1sd2ncuZZf0z5WK0M5doMnbQKZBVRzxxh6YU8IYiqh+ i6Nssx/R6zmVO0BeEsqhlyY9yYHLCU530KXbrpj6iTEHKDkJyTbZdSyLB1od8tq2 MCrlOxzsBcranJpIAF0fIESgshd++4q0bAREXUHbnJfVQdLSkS+RdY+gTFiPxLYB m8Uuj/IJ+J1pO17cuqrkM9sMfQqDdT6YkewaS+jZ61GjdbZr1HGMxB0gg30r4EiQ ePcMaSh5TzejjL+bRFqIUYXKdf2rAV2wnzRMiH1xMH9eBpIpoecc9TJBPOu4B6Ll mSvP0OAZDgJ2wsjbdX22ouSBXdb12hGH9GEhCI7ZHCfBNWmO9vXUe2JhnPPPN/GZ hHmSdGlPlIE3alN1M3RazSME4vTOPpvJ0GieMVLwyV2L9DolFy/23dpfoNREd/1Z 1USt3eUOWMn6GyVeOVklnkO2aw+ePMzNssEAElRewIiXfk0KG14qX68we4UV7Dfm jvIvNN2Ja/jk2zs2BcpwzUDqB9JyVtyxE3VIKavCvo19GUEX+5H3CfZx/2xKjuxv 9jZp8aCHMPQshvwTNbMYYpWV9vl9rgHEpG0wVoFP+KJCTxzjw/rBi+rjITWt1fdX IKlPcFRAFJ2OmsZPsOf94H69e9cT8hIMJBns3HZjuXoVrVxbrZg= =Ov59 -----END PGP SIGNATURE----- --Fb5y3s36qYbhvXLY--