On Tue, Jul 12, 2022 at 03:30:22PM +0200, Daniel Kiper wrote: > On Mon, Jul 11, 2022 at 12:44:59PM +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 > > Any chance to add this guy email here?
He didn't want his mail address to be associated with his name here.
> > Signed-off-by: Patrick Steinhardt <p...@pks.im>
> > ---
> > 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..c24c6e98d 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_args_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 = 0;
> > + char *unescaped = NULL;
> > + bool successful;
> > +
> > + if (grub_json_unescape (&unescaped, &unescaped_len, in, inlen) !=
> > GRUB_ERR_NONE)
> > + return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("could not unescape
> > Base64 string"));
> > +
> > + successful = base64_decode (unescaped, (grub_size_t) unescaped_len,
> > (char *) decoded, decodedlen);
>
> Now (grub_size_t) cast seems redundant because unescaped_len is defined
> as grub_size_t.
>
> Otherwise patch LGTM. So, if you fix this you can add my RB.
Oops, right. Will fix.
Patrick
> > + grub_free (unescaped);
> > + if (!successful)
> > + return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("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_uint8_t
> > *candidate_key,
> > gcry_err_code_t gcry_ret;
> >
> > /* Decode both digest and salt */
> > - if (!base64_decode (d->digest, grub_strlen (d->digest), (char *)digest,
> > &digestlen))
> > + if (luks2_base64_decode (d->digest, grub_strlen (d->digest),
> > + digest, &digestlen) != GRUB_ERR_NONE)
> > return grub_error (GRUB_ERR_BAD_ARGUMENT, "Invalid digest");
> > - if (!base64_decode (d->salt, grub_strlen (d->salt), (char *)salt,
> > &saltlen))
> > + if (luks2_base64_decode (d->salt, grub_strlen (d->salt),
> > + salt, &saltlen) != 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) != GRUB_ERR_NONE)
> > {
> > ret = grub_error (GRUB_ERR_BAD_ARGUMENT, "Invalid keyslot salt");
> > goto err;
>
> Daniel
signature.asc
Description: PGP signature
_______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
