From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.90_1) id 1oAqou-0007Pq-NO for mharc-grub-devel@gnu.org; Mon, 11 Jul 2022 06:39:16 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:50396) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oAqot-0007P2-8s for grub-devel@gnu.org; Mon, 11 Jul 2022 06:39:15 -0400 Received: from wout2-smtp.messagingengine.com ([64.147.123.25]:41015) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1oAqoq-0005wS-VV for grub-devel@gnu.org; Mon, 11 Jul 2022 06:39:14 -0400 Received: from compute3.internal (compute3.nyi.internal [10.202.2.43]) by mailout.west.internal (Postfix) with ESMTP id BE4CA3200904; Mon, 11 Jul 2022 06:39:08 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute3.internal (MEProxy); Mon, 11 Jul 2022 06:39:09 -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=fm3; t=1657535948; x=1657622348; bh=lPLkJftu5o aR8FHv/DdyUpgsXnGMMPcXiZ+zUhptJ+w=; b=JBgVbfKdqbHChxEMhBwqIqHDtq KJaS0ggtW2wORS/ZlAntw/BOcI6HAYc7n9kSuDHDNIqCry0CcaDPDRqzRL5h+kfm +Av1/6bBWxhHM2iSQvYsDV3zNH+QqIKonSQMx3sar95uZAjbrmNiukUZJdl09w8p Q1Vx2DVhO0W9i006SmCNwP02lDAJfy2QyN6rzhocyti4Va5e/9ThJzgvYfVdoaTk SJm3/fkUX/hP8DGmQ8C7IeqzikHz+I9FDrHJyAkgOFcaueToh+bD24SNPfHun27X Fke/zXdBokSBfIpDkbY8jTJW+50tAdux5aoZScs4z1Nf1+/e80/EsgKCQiCg== 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= fm3; t=1657535948; x=1657622348; bh=lPLkJftu5oaR8FHv/DdyUpgsXnGM MPcXiZ+zUhptJ+w=; b=QsJCrMe2revkLvWFgwvKuYzUbQReB7/C+KIpcYFpn32m 17uDRE+jgTpBjefv+ZV4oDcI8qG5NYxhZmP1Mclx9RL5j1Yw1ADA6OY4M8MYgNdO UlSqSuSve109r94ssMZNh4XvwY7PzNGwR8Kws6GULeqJdXI6xc2Kap/LfVKvRJeG PBDyF/7tY3WYBPq5iEFA96+MXV5boD2o3bxpdDpoD3zNtZ6f3cU1DNDgOmnptKjR kKpCDpY56avuNUk0SWISBd3ZDt7yaMtemFLEk64aCWZjxG05u6gXdeuK8s9b9rDp FNtK4r0h+bvpAceKK4MU5mZ1exwcPgvYEdNRgCeyYg== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvfedrudejfedgvdejucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmne cujfgurhepfffhvfevuffkfhggtggujgesghdtreertddtvdenucfhrhhomheprfgrthhr ihgtkhcuufhtvghinhhhrghrughtuceophhssehpkhhsrdhimheqnecuggftrfgrthhtvg hrnhepueektdevtdffveeljeetgfehheeigeekleduvdeffeeghefgledttdehjeelffet necuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehmrghilhhfrhhomhepphhsse hpkhhsrdhimh X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA; Mon, 11 Jul 2022 06:39:07 -0400 (EDT) Received: from localhost (tanuki [10.192.0.23]) by vm-mail.pks.im (OpenSMTPD) with ESMTPSA id e1bbc7eb (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO); Mon, 11 Jul 2022 10:39:05 +0000 (UTC) Date: Mon, 11 Jul 2022 12:39:58 +0200 From: Patrick Steinhardt To: Daniel Kiper Cc: grub-devel@gnu.org, Glenn Washburn Subject: Re: [PATCH v4 2/2] luks2: Fix decoding of digests and salts with escaped chars Message-ID: References: <84370adbaf018be5f3a0b34acef783f78e3500bb.1654493022.git.ps@pks.im> <20220630160512.7rjbtccuxrnzgodv@tomti.i.net-space.pl> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="bhO0aeuuw8j2oBqP" Content-Disposition: inline In-Reply-To: <20220630160512.7rjbtccuxrnzgodv@tomti.i.net-space.pl> Received-SPF: pass client-ip=64.147.123.25; envelope-from=ps@pks.im; helo=wout2-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: Mon, 11 Jul 2022 10:39:15 -0000 --bhO0aeuuw8j2oBqP Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Thu, Jun 30, 2022 at 06:05:12PM +0200, Daniel Kiper wrote: > On Mon, Jun 06, 2022 at 07:29:00AM +0200, Patrick Steinhardt wrote: > > It was reported in the #grub IRC channel on Libera that decryption of > > LUKS2 partitions fails with errors about invalid digests and/or salts. > > In all of these cases, what failed was decoding the Base64 > > representation of these, where the encoded data contained invalid > > characters. > > > > As it turns out, the root cause is that json-c, which is used by > > cryptsetup to read and write the JSON header, will escape some > > characters by prepending a backslash when writing JSON strings by > > default. Most importantly, json-c also escapes the forward slash, which > > is part of the Base64 alphabet. Because GRUB doesn't know to unescape > > such characters, decoding this string will rightfully fail. > > > > Interestingly, this issue has until now only been reported by users of > > Ubuntu 18.04. And a bit of digging in fact reveals that cryptsetup has > > changed the logic in a054206d (Suppress useless slash escaping in json > > lib, 2018-04-20), which has been released with cryptsetup v2.0.3. Ubuntu > > 18.04 is still shipping with cryptsetup v2.0.2 though, which explains > > why this is not a more frequent issue. > > > > Fix the issue by using our new `grub_json_unescape ()` helper function > > that handles unescaping for us. > > > > Reported-by: Afdal > > Signed-off-by: Patrick Steinhardt >=20 > I think this patch should be merged with the first one. Of course it > requires similar fixes mentioned in my earlier reply. Plus a few > additional ones mentioned below... I think it's easier to review this as two independent atomic changes. The decoding logic is complex enough to warrant its own patch given it already requires some focus to properly review, and the second patch has historic context around cryptsetup that's not relevant to JSON itself. I'll send v5 with separate patches, but I'll change it if you insist. > > grub-core/disk/luks2.c | 28 ++++++++++++++++++++++++---- > > 1 file changed, 24 insertions(+), 4 deletions(-) > > > > diff --git a/grub-core/disk/luks2.c b/grub-core/disk/luks2.c > > index bf741d70f..728f93a8c 100644 > > --- a/grub-core/disk/luks2.c > > +++ b/grub-core/disk/luks2.c > > @@ -384,6 +384,24 @@ luks2_scan (grub_disk_t disk, grub_cryptomount_arg= s_t cargs) > > return cryptodisk; > > } > > > > +static grub_err_t > > +luks2_base64_decode (const char *in, grub_size_t inlen, grub_uint8_t *= decoded, idx_t *decodedlen) > > +{ > > + grub_size_t unescaped_len =3D 0; > > + char *unescaped =3D NULL; > > + bool successful; > > + > > + if (grub_json_unescape (&unescaped, &unescaped_len, in, inlen) !=3D = GRUB_ERR_NONE) > > + return grub_error (GRUB_ERR_BAD_ARGUMENT, "Could not unescape Base= 64 string"); > > + > > + successful =3D base64_decode (unescaped, (size_t)unescaped_len, (cha= r *)decoded, decodedlen); >=20 > s/(char *)decoded/(char *) decoded/ >=20 > You need an additional space here. >=20 > s/(size_t)unescaped_len/(size_t) unescaped_len/ >=20 > s/size_t/grub_size_t/? Hmmm... It is idx_t in declaration... > Do we need cast here at all? Yeah, should be `grub_size_t`. But the cast is required, isn't it? `idx_t` is a typedef of `ptrdiff_t`, which is signed, and `grub_size_t` is unsigned. > > + grub_free (unescaped); > > + if (!successful) > > + return grub_error (GRUB_ERR_BAD_ARGUMENT, "Could not decode Base64= string"); > > + > > + return GRUB_ERR_NONE; > > +} > > + > > static grub_err_t > > luks2_verify_key (grub_luks2_digest_t *d, grub_uint8_t *candidate_key, > > grub_size_t candidate_key_len) > > @@ -395,9 +413,11 @@ luks2_verify_key (grub_luks2_digest_t *d, grub_uin= t8_t *candidate_key, > > gcry_err_code_t gcry_ret; > > > > /* Decode both digest and salt */ > > - if (!base64_decode (d->digest, grub_strlen (d->digest), (char *)dige= st, &digestlen)) > > + if (luks2_base64_decode (d->digest, grub_strlen (d->digest), > > + digest, &digestlen) !=3D GRUB_ERR_NONE) > > return grub_error (GRUB_ERR_BAD_ARGUMENT, "Invalid digest"); > > - if (!base64_decode (d->salt, grub_strlen (d->salt), (char *)salt, &s= altlen)) > > + if (luks2_base64_decode (d->salt, grub_strlen (d->salt), > > + salt, &saltlen) !=3D GRUB_ERR_NONE) > > return grub_error (GRUB_ERR_BAD_ARGUMENT, "Invalid digest salt"); > > > > /* Configure the hash used for the digest. */ > > @@ -435,8 +455,8 @@ luks2_decrypt_key (grub_uint8_t *out_key, > > gcry_err_code_t gcry_ret; > > grub_err_t ret; > > > > - if (!base64_decode (k->kdf.salt, grub_strlen (k->kdf.salt), > > - (char *)salt, &saltlen)) > > + if (luks2_base64_decode (k->kdf.salt, grub_strlen (k->kdf.salt), > > + salt, &saltlen) !=3D GRUB_ERR_NONE) > > { > > ret =3D grub_error (GRUB_ERR_BAD_ARGUMENT, "Invalid keyslot salt= "); > > goto err; >=20 > Daniel Patrick --bhO0aeuuw8j2oBqP Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCgAdFiEEF9hrgiFbCdvenl/rVbJhu7ckPpQFAmLL/f0ACgkQVbJhu7ck PpSXoQ/+NnUGYwoKUZ2q4TwSUIRYa3PPnW351qLcpf2I3Ysy+e4pEJVrVmKABzQn B9W+1ncsBkeODRS22YjQMwPKppTFR3etDpGFpMKbebPubKOKpHagY6+fJjM+Dueq SIRjsw4ogsJQJ7TfAuFDj/R8e6AXlUzLW7RWaFR141MaR97PAmfbI6FiW8qhdi8e 5h8eps+JOYSBfhjNcScOwa6qlPFto6sDOvNepiASEb4evRwRe3YJU7InE8ZKxRPs Ty+wg3TqZBHBW59DehsyzR1Dy85VFhpT+MXxCwv/51Chi11Vg249FpnEP31V5FXy Kb3JxvgMtDjeTq2180jW+wpXP0oXP2O/jp7YvGQjqrRZhxei9z8ia9+nQqUdfzQ9 iqURryYaCMrMqgG9SalvG6MNJSfX/qy0ejVUGZddLXSwGPNkvh8BC1+f9v/5n/y7 65NQz6CfHnUshZ8GZfCEFD4sssDbOhzvgoEImow9s2Q0xrnpphs8jksBx0RK6B6L BplOrk+h/ewK1cKUGEkKroA1QhzLOIWbNcnCqKMBSEGI/IN7O1o1MP1pLvGCt1Xe DAqNC5rRbBSLWxC1u7481te1cOvtPW4yj72/dl2uR1yy1N/XA9ucXmJaC9DCgiOR Jt84G/j3vgPxz22a/sxLC4kp196BcDvodOCuZag9qSDas2WvA6U= =lhXt -----END PGP SIGNATURE----- --bhO0aeuuw8j2oBqP--