From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.90_1) id 1mX6nz-0000wG-8W for mharc-grub-devel@gnu.org; Sun, 03 Oct 2021 15:05:47 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:35890) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mX6nx-0000w7-Nh for grub-devel@gnu.org; Sun, 03 Oct 2021 15:05:45 -0400 Received: from out5-smtp.messagingengine.com ([66.111.4.29]:43751) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mX6nw-0001bF-50 for grub-devel@gnu.org; Sun, 03 Oct 2021 15:05:45 -0400 Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailout.nyi.internal (Postfix) with ESMTP id 9B0415C0143; Sun, 3 Oct 2021 15:05:43 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute5.internal (MEProxy); Sun, 03 Oct 2021 15:05:43 -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=fm2; bh=xpSy3vE/FTdzLaU7h/w/eiN/SuL 8M1nPTvbxJo3ONdM=; b=VvFcax/VeJA5gY/lEzFQpl6GBaqnzDz2eu1uviiY67Q 3u4xq+LajAzr6VZWtco2qEP1kzviyfCQOEusWogD4IHqD+wv6AJaZo5o76k31KAw 1vS+u+Cx/CFVGbCcqtJb2/l6htqMA86xF4019PT0jQJoIs5x8HkMeesWig455GVo zPrbdUe6ch4JXCoknkGFROUczreRpV5DRKY3TFSAz54LtHog4oOMi15VpK5SHcie aGJ2daSyTuDpM3kBD04DkpW5NkN6ZM97+W77g9xH/OEXRmdcGCgXTvm477davToG LD2Rn1xims0W9Aet+Xkx+3qNxYIHFhS6ZANTaQeHpTA== 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=fm1; bh=xpSy3v E/FTdzLaU7h/w/eiN/SuL8M1nPTvbxJo3ONdM=; b=CCKhrgHBMS7RDO9KDAxS2a cOw4/21OLxjrdNyaeeLGbFJM/VkquOUhnSBdVAA/+V7ClMd51QXSyN/wYL0Gt3ti T+VJe/e5n+YcE3HVMylDBart0vKwmc9fJYeX+NmhMH+vVL5b2dzpIobEKw4U4oI+ kFqhKBPm9T/r6kqT1s/T0Q2ob0Tv1a3U/LvFEq3x4i0ZnznQx2fCH51c3DeoFApN G4MhTXWLCmtRUiEyyyXBu8f6esrh2K8obcIZ6IW2Rx4cibw1SBqbA9odPBaE7DI2 WNdrXam2zAPkJTQZaX4hZC6Yccgqu7iTWs0c1dRfIz2aMQLfvbdO0+YXRhC60jLg == X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvtddrudeltddgudeftdcutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpeffhffvuffkfhggtggujgesghdtreertddtvdenucfhrhhomheprfgrthhr ihgtkhcuufhtvghinhhhrghrughtuceophhssehpkhhsrdhimheqnecuggftrfgrthhtvg hrnhepheeghfdtfeeuffehkefgffduleffjedthfdvjeektdfhhedvlefgtefgvdettdfh necuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepphhsse hpkhhsrdhimh X-ME-Proxy: Received: by mail.messagingengine.com (Postfix) with ESMTPA; Sun, 3 Oct 2021 15:05:42 -0400 (EDT) Received: from localhost (xps [10.192.0.12]) by vm-mail.pks.im (OpenSMTPD) with ESMTPSA id 985e49e4 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Sun, 3 Oct 2021 19:05:41 +0000 (UTC) Date: Sun, 3 Oct 2021 21:10:01 +0200 From: Patrick Steinhardt To: Glenn Washburn Cc: grub-devel@gnu.org, Daniel Kiper , Denis 'GNUtoo' Carikli , James Bottomley Subject: Re: [PATCH v2 2/4] cryptodisk: Refactor password input from crypto dev modules into cryptodisk Message-ID: References: <20210927231403.642857-1-development@efficientek.com> <20210927231403.642857-3-development@efficientek.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="g+PcQfD0mi8TfS2M" Content-Disposition: inline In-Reply-To: <20210927231403.642857-3-development@efficientek.com> Received-SPF: pass client-ip=66.111.4.29; envelope-from=ps@pks.im; helo=out5-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: Sun, 03 Oct 2021 19:05:45 -0000 --g+PcQfD0mi8TfS2M Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Mon, Sep 27, 2021 at 06:14:01PM -0500, Glenn Washburn wrote: > The crypto device modules should only be setting up the crypto devices and > not getting user input. This has the added benefit of simplifying the code > such that three essentially duplicate pieces of code are merged into one. >=20 > Signed-off-by: Glenn Washburn > --- > grub-core/disk/cryptodisk.c | 52 ++++++++++++++++++++++++++++++------- > grub-core/disk/geli.c | 26 ++++--------------- > grub-core/disk/luks.c | 27 +++---------------- > grub-core/disk/luks2.c | 26 ++++--------------- > include/grub/cryptodisk.h | 1 + > 5 files changed, 57 insertions(+), 75 deletions(-) >=20 > diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c > index ca034859e..86eaabe60 100644 > --- a/grub-core/disk/cryptodisk.c > +++ b/grub-core/disk/cryptodisk.c > @@ -1001,9 +1001,11 @@ grub_cryptodisk_scan_device_real (const char *name, > grub_disk_t source, > grub_cryptomount_args_t cargs) > { > - grub_err_t err; > + grub_err_t ret =3D GRUB_ERR_NONE; > grub_cryptodisk_t dev; > grub_cryptodisk_dev_t cr; > + int askpass =3D 0; > + char *part =3D NULL; > =20 > dev =3D grub_cryptodisk_get_by_source_disk (source); > =20 > @@ -1017,21 +1019,51 @@ grub_cryptodisk_scan_device_real (const char *nam= e, > return grub_errno; > if (!dev) > continue; > - =20 > - err =3D cr->recover_key (source, dev, cargs); > - if (err) > - { > - cryptodisk_close (dev); > - return err; > - } > + > + if (cargs->key_len =3D=3D 0) > + { > + /* Get the passphrase from the user, if no key data. */ > + askpass =3D 1; > + if (source->partition) > + part =3D grub_partition_get_name (source->partition); > + grub_printf_ (N_("Enter passphrase for %s%s%s (%s): "), source->name, > + source->partition ? "," : "", part ? : "", > + dev->uuid); > + grub_free (part); > + > + cargs->key_data =3D grub_malloc (GRUB_CRYPTODISK_MAX_PASSPHRASE); > + if (!cargs->key_data) > + return grub_errno; > + > + if (!grub_password_get ((char *) cargs->key_data, GRUB_CRYPTODISK_MAX_P= ASSPHRASE)) > + { > + ret =3D grub_error (GRUB_ERR_BAD_ARGUMENT, "Passphrase not supplied= "); > + goto error; > + } > + cargs->key_len =3D grub_strlen((char *) cargs->key_data); Missing space. > + } > + > + ret =3D cr->recover_key (source, dev, cargs); > + if (ret) > + goto error; > =20 > grub_cryptodisk_insert (dev, name, source); > =20 > have_it =3D 1; > =20 > - return GRUB_ERR_NONE; > + goto cleanup; > } > - return GRUB_ERR_NONE; > + goto cleanup; > + > +error: > + cryptodisk_close (dev); > +cleanup: > + if (askpass) > + { > + cargs->key_len =3D 0; > + grub_free(cargs->key_data); Here, too. Otherwise, the patch looks good to me. Patrick --g+PcQfD0mi8TfS2M Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCgAdFiEEF9hrgiFbCdvenl/rVbJhu7ckPpQFAmFaAAgACgkQVbJhu7ck PpQGrw/8C/GNP8DjwH3wSx+ZvzYLDXHvAF342Mo1pPKXNuh4HzmGHFJyc/0XhLB7 W7N/FcJilMk0r+SqquSeIpwTFdxX62rAqs94oq5+SF3KI611O/S3z1cIgXcsgumL uSlpM3EtQO61IA5oAhVoxyfigGpow2WT6sQUI5XB8vbi5OxehE94DCPp1XgLcSqb hiVXrc13kcehRu6UzdgVql7t3hs6lkD0RKxvveaHHDYQ8YpjepQMu6lrW/VOD4I+ 5VUO0ORX761AwsuyQpehpdM+4UVRR4ynP2X5yDioDip+0YHsrbzzhVHL5GL8Emv3 uUO6qa/tMrCHQbdFCDonryNZ+nqaX6Akw1Z3nCP26+JOIJy6sBMRLgrnqmEPP/WB BMUcxbsUkcDVpyjufGLsi5/gc0eR3xNUvXx2kZDJhjOhQhLe3ibTNF3o+F40DE/A 9P3yiy2h8taYXl1GWh7q4NR7wq4hWGerkBMgyQJIE3EdhIP60PGuPVD/d65qaW+Y hGYAjYSkais5z1j71qTCA2lVgwCWv4hOlL99nNcKy7DzfWV8KCvDh2SrZXl3B9mz 0vTJFjsJZpGQTBcuMXFYZQg46EfStZJN5ZIxCpXt/ys++qulGcWdL5VC8bEFZf8T OR2EIZlzHYaUYep570LaJHkkj1Xel3vcm0WtTfFt+z+bQbe4k/c= =MO4a -----END PGP SIGNATURE----- --g+PcQfD0mi8TfS2M--