Re: [PATCH v6 0/2] luks2: Fix decoding of digests and salts with escaped chars

[Glenn Washburn]

On Mon, 15 Aug 2022 17:52:45 +0200
Patrick Steinhardt <ps@pks.im> wrote:

> Hi,
> 
> this is the sixth version of my patch series which fixes decoding of
> digests and salts in LUKS2 headers in case they happen to contain
> escaped characters. While modern cryptsetup versions in fact don't
> escape any characters part of the Base64 alphabet, old versions of
> cryptsetup did this until v2.0.2.
> 
> There's only a single change compared to v5, which is a removed type
> cast that was not in fact needed. I didn't include the feedback from
> Nicholas to make the JSON string parsing more lenient. While sensible,
> it's rather a theoretical concern right now as theer was only a single
> version of cryptsetup that ever wrote escaped characters, and even then
> of the Base64 alphabet only the backslash may have been escaped. So I
> think we should rather defer any improvements until there we discover
> real-world problems or until there are more usecases for this function.

I support and concur with this rationale. We can change it when it
becomes an issue.

Glenn

> 
> Patrick
> 
> Patrick Steinhardt (2):
>   json: Add function to unescape JSON-encoded strings
>   luks2: Fix decoding of digests and salts with escaped chars
> 
>  grub-core/disk/luks2.c    |  28 +++++++--
>  grub-core/lib/json/json.c | 118 ++++++++++++++++++++++++++++++++++++++
>  grub-core/lib/json/json.h |  12 ++++
>  3 files changed, 154 insertions(+), 4 deletions(-)
> 
> Range-diff against v5:
> 1:  ebab6b092 ! 1:  c44675566 json: Add function to unescape JSON-encoded 
> strings
>     @@ Commit message
>          Add a new function `grub_json_unescape ()` that takes a potentially
>          escaped JSON string as input and returns a new unescaped string.
>      
>     +    Reviewed-by: Daniel Kiper <dkiper@net-space.pl>
>          Signed-off-by: Patrick Steinhardt <ps@pks.im>
>      
>       ## grub-core/lib/json/json.c ##
> 2:  60ccd669d ! 2:  16ae4ef05 luks2: Fix decoding of digests and salts with 
> escaped chars
>     @@ Commit message
>          that handles unescaping for us.
>      
>          Reported-by: Afdal
>     +    Reviewed-by: Daniel Kiper <dkiper@net-space.pl>
>          Signed-off-by: Patrick Steinhardt <ps@pks.im>
>      
>       ## grub-core/disk/luks2.c ##
>     @@ grub-core/disk/luks2.c: luks2_scan (grub_disk_t disk, 
> grub_cryptomount_args_t ca
>      +  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);
>     ++  successful = base64_decode (unescaped, unescaped_len, (char *) 
> decoded, decodedlen);
>      +  grub_free (unescaped);
>      +  if (!successful)
>      +    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("could not decode 
> Base64 string"));