From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.90_1) id 1jcbD8-0002gF-Ft for mharc-grub-devel@gnu.org; Sat, 23 May 2020 16:57:38 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:33786) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1jcbD6-0002fe-Kz for grub-devel@gnu.org; Sat, 23 May 2020 16:57:36 -0400 Received: from out5-smtp.messagingengine.com ([66.111.4.29]:50385) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1jcbD4-0000j3-UN for grub-devel@gnu.org; Sat, 23 May 2020 16:57:36 -0400 Received: from compute3.internal (compute3.nyi.internal [10.202.2.43]) by mailout.nyi.internal (Postfix) with ESMTP id 8D4D35C0039; Sat, 23 May 2020 16:57:31 -0400 (EDT) Received: from mailfrontend2 ([10.202.2.163]) by compute3.internal (MEProxy); Sat, 23 May 2020 16:57:31 -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=y3zEd2GfFVgZntj0GmNvbe/bJnI 1bK/5wWKpxTUSyfU=; b=N3iOfCw25hdFzS6oPvbbEwb9jzGns9no6nCWjOTTQPW wMVHNh0A/zhmmF4M7Tq1umnWWYoyONASEjG4l7zGzWaFLvyniYkdZEi+hDyZUgtQ CpRHUuRJZ/jROB6H12NTDUfqVOgCJaH1Rj2J08qKZqwFhFFZrYB7ey0PyDd11agE n8sOQbrw8b2HtulGBPE0w1yPWrT/BP2Mzu4Wac6Xw+IvOXZ3zx0z0Rrk5wbjhow0 ddmoHCch/yPIrqp4sLW+SXV2/0C2FwRoonwMbaALK1S5SnUifqunqbt64WADGbbX YsLAWlqdgGU4O8JnuggFNl8l8jj8wmTEXgIOt1PKx3w== 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=fm2; bh=y3zEd2 GfFVgZntj0GmNvbe/bJnI1bK/5wWKpxTUSyfU=; b=cuopBanp0yDxarLKKbWf+7 x65xQAbCK+XuyLsPndEcjZd9AqlR3ye7I5S9hF3fFz+0ZroE9B8XYJiW6SqPFNoC vbPHJR7yXxzSEyl6e6LFMPeIHdGXG4u7QDj+fbH+kU33cj42s1TXfgO/Q4mq2wue J0GFz1Dt6hAVIhypLBbwmMPU1ROzP/llOCjxBGHWKZwGMU7xU5472nLCCRmtP+X/ /FyjVqhFhEN+nigVb1zEcz/DIv13ByNUWdZs7YlmfPvpCtIzium/Q/ZC7RW6gCrl YeTseCQUXJwfyccSA/loNaYAA81vp1q4TwParL+B86xTGVS/fgI2bA/viIIPpirA == X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduhedrudduhedgudehhecutefuodetggdotefrod ftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfgh necuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmd enucfjughrpeffhffvuffkfhggtggujgesghdtreertddtvdenucfhrhhomheprfgrthhr ihgtkhcuufhtvghinhhhrghrughtuceophhssehpkhhsrdhimheqnecuggftrfgrthhtvg hrnhepheeghfdtfeeuffehkefgffduleffjedthfdvjeektdfhhedvlefgtefgvdettdfh necukfhppeejjedrudekfedrudekrddugedtnecuvehluhhsthgvrhfuihiivgeptdenuc frrghrrghmpehmrghilhhfrhhomhepphhssehpkhhsrdhimh X-ME-Proxy: Received: from vm-mail.pks.im (x4db7128c.dyn.telefonica.de [77.183.18.140]) by mail.messagingengine.com (Postfix) with ESMTPA id 4013C3066509; Sat, 23 May 2020 16:57:30 -0400 (EDT) Received: from localhost (xps [10.192.0.12]) by vm-mail.pks.im (OpenSMTPD) with ESMTPSA id 707b64e6 (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Sat, 23 May 2020 20:52:33 +0000 (UTC) Date: Sat, 23 May 2020 22:52:46 +0200 From: Patrick Steinhardt To: Denis 'GNUtoo' Carikli Cc: Daniel Kiper , The development of GNU GRUB , John Lane Subject: Re: [Patchv3][ 5/6] cryptodisk: enable the backends to implement key files Message-ID: <20200523205246.GA3840@xps> References: <20200507231943.6196-1-GNUtoo@cyberdimension.org> <20200507231943.6196-6-GNUtoo@cyberdimension.org> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="r5Pyd7+fXNt84Ff3" Content-Disposition: inline In-Reply-To: <20200507231943.6196-6-GNUtoo@cyberdimension.org> Received-SPF: pass client-ip=66.111.4.29; envelope-from=ps@pks.im; helo=out5-smtp.messagingengine.com X-detected-operating-system: by eggs.gnu.org: First seen = 2020/05/23 16:57:31 X-ACL-Warn: Detected OS = Linux 2.2.x-3.x [generic] [fuzzy] 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, URIBL_BLOCKED=0.001 autolearn=_AUTOLEARN 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: Sat, 23 May 2020 20:57:36 -0000 --r5Pyd7+fXNt84Ff3 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, May 08, 2020 at 01:19:42AM +0200, Denis 'GNUtoo' Carikli wrote: > From: John Lane >=20 > Signed-off-by: John Lane > GNUtoo@cyberdimension.org: rebase, patch split, small fixes, commit messa= ge > Signed-off-by: Denis 'GNUtoo' Carikli > --- > ChangeLog: > In addition to the requested changes (if any), the following was > changed: > - Changed a bit the error message "Keyfile is too small" > from the one suggested in the review. > --- > grub-core/disk/cryptodisk.c | 87 ++++++++++++++++++++++++++++++++++++- > grub-core/disk/geli.c | 7 +-- > grub-core/disk/luks.c | 7 ++- > grub-core/disk/luks2.c | 7 +-- > include/grub/cryptodisk.h | 5 ++- > include/grub/file.h | 2 + > 6 files changed, 106 insertions(+), 9 deletions(-) >=20 > diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c > index 6ad2e486e..ab4a62b7f 100644 > --- a/grub-core/disk/cryptodisk.c > +++ b/grub-core/disk/cryptodisk.c > @@ -42,6 +42,9 @@ static const struct grub_arg_option options[] =3D > {"all", 'a', 0, N_("Mount all."), 0, 0}, > {"boot", 'b', 0, N_("Mount all volumes with `boot' flag set."), 0, 0= }, > {"header", 'H', 0, N_("Read header from file"), 0, ARG_TYPE_STRING}, > + {"keyfile", 'k', 0, N_("Key file"), 0, ARG_TYPE_STRING}, > + {"keyfile-offset", 'O', 0, N_("Key file offset (bytes)"), 0, ARG_TYP= E_INT}, > + {"keyfile-size", 'S', 0, N_("Key file data size (bytes)"), 0, ARG_TY= PE_INT}, > {0, 0, 0, 0, 0, 0} > }; > =20 > @@ -972,6 +975,8 @@ grub_util_cryptodisk_get_uuid (grub_disk_t disk) > static int check_boot, have_it; > static char *search_uuid; > static grub_file_t hdr; > +static grub_uint8_t *key, keyfile_buffer[GRUB_CRYPTODISK_MAX_KEYFILE_SIZ= E]; > +static grub_ssize_t key_size; > =20 > static void > cryptodisk_close (grub_cryptodisk_t dev) > @@ -1002,7 +1007,7 @@ grub_cryptodisk_scan_device_real (const char *name,= grub_disk_t source) > if (!dev) > continue; > =20 > - err =3D cr->recover_key (source, dev, hdr); > + err =3D cr->recover_key (source, dev, hdr, key, key_size); > if (err) > { > cryptodisk_close (dev); > @@ -1112,6 +1117,86 @@ grub_cmd_cryptomount (grub_extcmd_context_t ctxt, = int argc, char **args) > hdr =3D NULL; > =20 > have_it =3D 0; > + key =3D NULL; > + > + if (state[4].set) /* keyfile */ > + { > + const char *p =3D NULL; > + grub_file_t keyfile; > + int keyfile_offset; > + grub_size_t requested_keyfile_size =3D 0; > + > + > + if (state[5].set) /* keyfile-offset */ > + { > + keyfile_offset =3D grub_strtoul (state[5].arg, &p, 0); > + > + if (grub_errno !=3D GRUB_ERR_NONE) > + return grub_errno; > + > + if (*p !=3D '\0') > + return grub_error (GRUB_ERR_BAD_ARGUMENT, > + N_("unrecognized number")); > + } > + else > + { > + keyfile_offset =3D 0; > + } > + > + if (state[6].set) /* keyfile-size */ > + { > + requested_keyfile_size =3D grub_strtoul(state[6].arg, &p, 0); > + > + if (*p !=3D '\0') > + return grub_error (GRUB_ERR_BAD_ARGUMENT, > + N_("unrecognized number")); > + > + if (grub_errno !=3D GRUB_ERR_NONE) > + return grub_errno; > + > + if (requested_keyfile_size > GRUB_CRYPTODISK_MAX_KEYFILE_SIZE) > + return grub_error(GRUB_ERR_OUT_OF_RANGE, > + N_("Key file size exceeds maximum (%llu)\n"), \ > + (unsigned long long) GRUB_CRYPTODISK_MAX_KEYFILE_SIZE); > + > + if (requested_keyfile_size =3D=3D 0) > + return grub_error(GRUB_ERR_OUT_OF_RANGE, > + N_("Key file size is 0\n")); > + } > + > + > + keyfile =3D grub_file_open (state[4].arg, > + GRUB_FILE_TYPE_CRYPTODISK_ENCRYPTION_KEY); > + if (!keyfile) > + return grub_errno; > + > + if (grub_file_seek (keyfile, keyfile_offset) =3D=3D (grub_off_t)-1) > + return grub_errno; > + > + > + if (requested_keyfile_size) > + { > + if (requested_keyfile_size > (keyfile->size - keyfile_offset)) > + return grub_error (GRUB_ERR_FILE_READ_ERROR, > + N_("Keyfile is too small: " > + "requested %llu bytes, " > + "but the file only has %llu bytes.\n"), > + (unsigned long long) requested_keyfile_size, > + (unsigned long long) keyfile->size); Instead of casting these, you may also just use `PRIuGRUB_SIZE` as formatter. Execept for this minor nit the patches all look good to me. Not sure if it's worth much, but you may add my Reviewed-by tag if you want to. Patrick --r5Pyd7+fXNt84Ff3 Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCgAdFiEEF9hrgiFbCdvenl/rVbJhu7ckPpQFAl7JjR0ACgkQVbJhu7ck PpR3pA//a0WseGGpD/gHwOdBLCXMW7RxNhpun4dvNXJ4/+ui+NZMGjdA130AKuWY 9XRU/VcMx0iqKuJkLL5DLONoaSbRsNIm6NE02P0WIsMTbx0y4FFiMDt62lSCQrx0 huBvQYVrj149+gYwpsusdNVRfNO+kVdg3qt24I74R27u6f1r+bPYRwzzV00avZ9t B1uD0yiacy8kTTCHHdaPfv98gZ6o307jxcDAKYjyEJ1NltuDxHsuX78dzTcmpcl7 cx59RGVT24OoJSkvauMKtSx7o+0mDnti2dOnBv4/xMJaVU77MMYcjGudiLVMVHfh b9QXxSvCc+kI0m98ZhC+mmnd99YuDi0DsJXWcb5AD3Ls/VTuUMslXdjWAqh2U2Mw 8IZ0eUk+MzRBlfRTgVbzhk7IZiF/eqB4J0YY3Y24NWWI2C+bE+Mo+OmngzT0iVlV unekGFThfuEjhLQ0XTYDWjvu5X2Yxrol2OGiEFVAecT0CRIiDokBl/FPsPhZ1dfF ASf0U5RaNrNxIX7g7sLH6L6CFCIbFcSEel9w9W1eNOixDeX49XqINTqgO2j8cuWh Y9XBaFJSd101D77nJiEh5svF8cOn+fm/1WU9sA215NKs/+pOO00VhyRCK9AxvyL7 0U9r6q/3L08mB164JCmH4U5PqT3YUSxW2dIrPMvdKli3rLChkOw= =+fXe -----END PGP SIGNATURE----- --r5Pyd7+fXNt84Ff3--