grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH v3 0/2] luks2: Fix decoding of digests and salts with escaped cha


From: Patrick Steinhardt
Subject: [PATCH v3 0/2] luks2: Fix decoding of digests and salts with escaped chars
Date: Mon, 30 May 2022 18:00:56 +0200

Hi,

this is the third 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.

Changes compared to v2:

    - I've split out the logic to unescape JSON-escaped strings into a
      new `grub_json_unescape ()` function. This function now provides
      full support for unescaping as specified in the JSON standard.

    - I've converted the errors returned by `luks2_base64_decode ()` to
      use `grub_error ()` to provide more context. Note though that I
      haven't yet started to use `grub_error_push ()` as Glenn proposes:
      as he said, that's a more general refactoring and I thus feel like
      it should be part of a different patch series.

    - I've done my research and finally found out why this was only
      happening on Ubuntu 18.04, which uses cryptsetup v2.0.2, and
      documented this in the commit message.

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 | 98 +++++++++++++++++++++++++++++++++++++++
 grub-core/lib/json/json.h | 12 +++++
 3 files changed, 134 insertions(+), 4 deletions(-)

Range-diff against v2:
-:  --------- > 1:  3055f9f2f json: Add function to unescape JSON-encoded 
strings
1:  b5a9fcdb5 ! 2:  69424b2d1 luks2: Fix decoding of digests and salts with 
escaped chars
    @@ Commit message
     
         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. Most
    -    importantly, this also includes the forward slash, which is part of the
    -    Base64 alphabet and which may optionally be escaped. Because GRUB
    -    doesn't know to unescape such characters, decoding this string will
    -    rightfully fail.
    +    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.
     
    -    Fix the issue by stripping the escape character for forward slashes.
    -    There is no need to do so for any other escaped character given that
    -    valid Base64 does not contain anything else.
    +    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 <ps@pks.im>
     
      ## grub-core/disk/luks2.c ##
    -@@ grub-core/disk/luks2.c: luks2_scan (grub_disk_t disk, const char 
*check_uuid, int check_boot)
    +@@ grub-core/disk/luks2.c: luks2_scan (grub_disk_t disk, 
grub_cryptomount_args_t cargs)
        return cryptodisk;
      }
      
     +static grub_err_t
    -+luks2_base64_decode (const char *in, size_t inlen, grub_uint8_t *decoded, 
size_t *decodedlen)
    ++luks2_base64_decode (const char *in, size_t inlen, grub_uint8_t *decoded, 
idx_t *decodedlen)
     +{
    ++  size_t unescaped_len;
     +  char *unescaped;
     +  bool successful;
    -+  size_t i, j;
     +
    -+  unescaped = grub_malloc (inlen);
    -+  if (!unescaped)
    -+    return GRUB_ERR_OUT_OF_MEMORY;
    ++  if (grub_json_unescape (&unescaped, &unescaped_len, in, inlen) != 
GRUB_ERR_NONE)
    ++    return grub_error (GRUB_ERR_BAD_ARGUMENT, "Could not unescape Base64 
string");
     +
    -+  grub_memmove (unescaped, in, inlen);
    -+
    -+  /*
    -+   * Characters in JSON strings may be escaped, either via their 
six-character
    -+   * "\uXXXX" representation or (at least for a subset of characters) via 
a
    -+   * single backslash. In context of the Base64-encoded string we expect 
here,
    -+   * the only character that gets escaped by cryptsetup is the forward 
slash.
    -+   * So while incomplete, we only strip away the escape character if we 
see
    -+   * '\/' in the input.
    -+   *
    -+   * See https://datatracker.ietf.org/doc/html/rfc8259#section-7 for more
    -+   * information on escaping in JSON.
    -+   */
    -+  for (i = j = 0; i < inlen; i++) {
    -+    if (i + 1 < inlen && unescaped[i] == '\\' && unescaped[i + 1] == '/')
    -+      continue;
    -+    unescaped[j++] = unescaped[i];
    -+  }
    -+
    -+  successful = base64_decode (unescaped, j, (char *)decoded, decodedlen);
    ++  successful = base64_decode (unescaped, unescaped_len, (char *)decoded, 
decodedlen);
     +  grub_free (unescaped);
     +  if (!successful)
    -+    return GRUB_ERR_BAD_ARGUMENT;
    ++    return grub_error (GRUB_ERR_BAD_ARGUMENT, "Could not decode Base64 
string");
     +
     +  return GRUB_ERR_NONE;
     +}
-- 
2.36.1

Attachment: signature.asc
Description: PGP signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]