[PATCH v2 0/3] Cryptomount detached headers

[Glenn Washburn]

Note: Description here has changed since last time, the last few sentences,
which describe a method for allowing backends to bypass the read hook on
the source disk.

Updates since v1:
 * Add marge comment block describing at a high level the read hook operation
   and assumptions.
 * Add code to unset read hook on source disk when exiting
   grub_cryptodisk_scan_device_real(). A couple return calls were converted
   into gotos so that logical flows to one exit point.

This patch series is, I believe, a better approach to supporting detached
headers for cryptomount and backends. This series will probably not apply
cleanly without the changes from the recent series entitled "[PATCH 0/4]
Cryptomount keyfile support". But its only because they touch code in the
same vicinity, not because there's any real dependency.

Conceptually the approach is different than the previous detach header
series because this one uses the disk objects read hook to hook reads to
the disk in scan() and recover_key() such that if there is an associated
header file, the hook will cause the read to return data from the header
file instead of from the source disk.

For this the read hook implementation needed to be upgaded because prior
it didn't get the read buffer sent from the read caller and so could not
modify its contents. Patch #1 updates the hook accordingly and all instances
of its use, but doesn't functionally change how GRUB operates.

The second patch adds the header option to cryptomount and the read hook
to the source disk during the scan() and recover_key() stages.
The benefit of this approach is its simpler/less code and does not require
the modification of backends, except to potentially cause a failure in
scan() if the backend doesn't support the current implementation of detached
headers, as with the GELI backend. This implementation requires that the
crypto volume header reside at the beginning of the volume. GELI has its
header at the end, which is why it can not currently be supported. In
theory, GELI could be supported if extra assumptions about its source
access pattern during scan() and recovery_key() were made. I don't use GELI,
no one seems to be asking for GELI detached headers, and I don't think that
GELI even support detached headers with the official tools. So for me, not
supporting crypto volumes with headers at the end of the disk is not a big
deal. With the patch series each backend gets the header file through cargs,
so it can implement detached headers by solely operating on that file instead
of the hooked source disk. In the the future, a flag can be added to the
cryptodisk_dev_t that backends can sent when registering to enabled/disable
the use of the read hook, if the backend needs to read from both the detached
header file and the source disk.

Glenn

Glenn Washburn (3):
  disk: Allow read hook callback to take read buffer to potentially
    modify it
  cryptodisk: Add support for using detached header files
  docs: Add documentation on detached header option to cryptomount

 docs/grub.texi                 | 13 +++--
 grub-core/commands/blocklist.c | 10 ++--
 grub-core/commands/loadenv.c   |  8 +--
 grub-core/commands/testload.c  |  4 +-
 grub-core/disk/cryptodisk.c    | 93 ++++++++++++++++++++++++++++++++--
 grub-core/disk/geli.c          |  4 ++
 grub-core/fs/hfspluscomp.c     |  4 +-
 grub-core/fs/ntfscomp.c        | 14 ++---
 grub-core/kern/disk.c          | 12 ++---
 grub-core/lib/progress.c       | 11 ++--
 grub-core/net/net.c            |  2 +-
 include/grub/cryptodisk.h      |  2 +
 include/grub/disk.h            |  6 +--
 include/grub/file.h            |  2 +
 14 files changed, 147 insertions(+), 38 deletions(-)

Range-diff against v1:
1:  b3672a4cf = 1:  ad4a48225 disk: Allow read hook callback to take read 
buffer to potentially modify it
2:  a3ffa4738 ! 2:  25db3635f cryptodisk: Add --header option to cryptomount to 
support detached headers
    @@ Metadata
     Author: Glenn Washburn <development@efficientek.com>
     
      ## Commit message ##
    -    cryptodisk: Add --header option to cryptomount to support detached 
headers
    +    cryptodisk: Add support for using detached header files
     
    -    Add a --header (short -H) option to cryptomount which takes a file 
argument.
    -    Using the improved read hook, setup a read hook on the source device 
which
    -    will read from the given header file during the scan and recovery 
cryptodisk
    -    backend functions. This makes supporting detached headers transparent 
to the
    -    backend, so long as the attached header is located at the start of the
    -    source disk. This is not the case for GELI volumes, which have the 
header at
    -    the end of the volume. So GELI will return an error if a detached 
header is
    -    specified.
    +    Using the disk read hook mechanism, setup a read hook on the source 
disk
    +    which will read from the given header file during the scan and recovery
    +    cryptodisk backend functions. Disk read hooks are executed after the 
data
    +    has been read from the disk. This is okay, because the read hook is 
given
    +    the read buffer before its sent back to the caller. In this case, the 
hook
    +    can then overwrite the data read from the disk device with data from 
the
    +    header file sent in as the read hook data. This is transparent to the
    +    read caller. Since the callers of this function have just opened the
    +    source disk, there are no current read hooks, so there's no need to
    +    save/restore them nor consider if they should be called or not.
    +
    +    This hook assumes that the header is at the start of the volume, which
    +    is not the case for some formats (eg. GELI). So GELI will return an
    +    error if a detached header is specified. It also can only be used
    +    with formats where the detached header file can be written to the
    +    first blocks of the volume and the volume could still be unlocked.
    +    So the header file can not be formatted differently from the on-disk
    +    header. If these assumpts are not met, detached header file processing
    +    must be specially handled in the cryptodisk backend module.
    +
    +    The hook will be called potentially many times by a backend. This is 
fine
    +    because of the assumptions mentioned and the read hook reads from 
absolute
    +    offsets and is stateless.
    +
    +    Also add a --header (short -H) option to cryptomount which takes a file
    +    argument.
     
      ## grub-core/disk/cryptodisk.c ##
    +@@ grub-core/disk/cryptodisk.c: enum
    +     OPTION_PASSWORD,
    +     OPTION_KEYFILE,
    +     OPTION_KEYFILE_OFFSET,
    +-    OPTION_KEYFILE_SIZE
    ++    OPTION_KEYFILE_SIZE,
    ++    OPTION_HEADER
    +   };
    + 
    + static const struct grub_arg_option options[] =
     @@ grub-core/disk/cryptodisk.c: static const struct grub_arg_option 
options[] =
          {"key-file", 'k', 0, N_("Key file"), 0, ARG_TYPE_STRING},
          {"keyfile-offset", 'O', 0, N_("Key file offset (bytes)"), 0, 
ARG_TYPE_INT},
    @@ grub-core/disk/cryptodisk.c: static const struct grub_arg_option 
options[] =
      
     +struct cryptodisk_read_hook_ctx
     +{
    -+  grub_disk_read_hook_t prev_read_hook;
    -+  void *prev_read_hook_data;
     +  grub_file_t hdr_file;
     +};
     +typedef struct cryptodisk_read_hook_ctx *cryptodisk_read_hook_ctx_t;
    @@ grub-core/disk/cryptodisk.c: cryptodisk_close (grub_cryptodisk_t dev)
     +  cryptodisk_read_hook_ctx_t ctx = data;
     +
     +  if (ctx->hdr_file == NULL)
    -+    return GRUB_ERR_NONE;
    ++    return grub_error (GRUB_ERR_BAD_ARGUMENT, N_("header file not 
found"));
     +
     +  if (grub_file_seek (ctx->hdr_file,
     +                (sector * GRUB_DISK_SECTOR_SIZE) + offset)
    @@ grub-core/disk/cryptodisk.c: cryptodisk_close (grub_cryptodisk_t dev)
     +    return grub_errno;
     +
     +  if (grub_file_read (ctx->hdr_file, buf, length) != (grub_ssize_t) 
length)
    -+    return grub_errno;
    -+
    -+  if (ctx->prev_read_hook != NULL)
    -+    ret = ctx->prev_read_hook(sector, offset, length, buf,
    -+                        ctx->prev_read_hook_data);
    ++    {
    ++      if (grub_errno == GRUB_ERR_NONE)
    ++  grub_error (GRUB_ERR_OUT_OF_RANGE, N_("header file too small"));
    ++      return grub_errno;
    ++    }
     +
     +  return ret;
     +}
    @@ grub-core/disk/cryptodisk.c: grub_cryptodisk_scan_device_real (const 
char *name,
      
     +  if (cargs->hdr_file != NULL)
     +    {
    ++      /*
    ++       * Set read hook to read header from a file instead of the source 
disk.
    ++       * Disk read hooks are executed after the data has been read from 
the
    ++       * disk. This is okay, because the read hook is given the read 
buffer
    ++       * before its sent back to the caller. In this case, the hook can 
then
    ++       * overwrite the data read from the disk device with data from the
    ++       * header file sent in as the read hook data. This is transparent 
to the
    ++       * read caller. Since the callers of this function have just opened 
the
    ++       * source disk, there are no current read hooks, so there's no need 
to
    ++       * save/restore them nor consider if they should be called or not.
    ++       *
    ++       * This hook assumes that the header is at the start of the volume, 
which
    ++       * is not the case for some formats (eg. GELI). It also can only be 
used
    ++       * with formats where the detached header file can be written to the
    ++       * first blocks of the volume and the volume could still be 
unlocked.
    ++       * So the header file can not be formatted differently from the 
on-disk
    ++       * header. If these assumpts are not met, detached header file 
processing
    ++       * must be specially handled in the cryptodisk backend module.
    ++       *
    ++       * This hook needs only be set once and will be called potentially 
many
    ++       * times by a backend. This is fine because of the assumptions 
mentioned
    ++       * and the read hook reads from absolute offsets and is stateless.
    ++       */
     +      read_hook_data.hdr_file = cargs->hdr_file;
    -+
    -+      if (source->read_hook != NULL)
    -+  {
    -+    read_hook_data.prev_read_hook = source->read_hook;
    -+    read_hook_data.prev_read_hook_data = source->read_hook_data;
    -+  }
    -+
     +      source->read_hook = cryptodisk_read_hook;
     +      source->read_hook_data = (void *) &read_hook_data;
     +    }
     +
        FOR_CRYPTODISK_DEVS (cr)
        {
    ++    /*
    ++     * Loop through each cryptodisk backend that is registered (ie. 
loaded).
    ++     * If the scan returns NULL, then the backend being tested does not
    ++     * recognize the source disk, so move on to the next backend.
    ++     */
          dev = cr->scan (source, cargs);
    +     if (grub_errno)
    +-      return NULL;
    ++      goto error_no_close;
    +     if (!dev)
    +       continue;
    + 
    +@@ grub-core/disk/cryptodisk.c: grub_cryptodisk_scan_device_real (const 
char *name,
    + 
    +   cargs->key_data = grub_malloc (GRUB_CRYPTODISK_MAX_PASSPHRASE);
    +   if (cargs->key_data == NULL)
    +-    return NULL;
    ++    goto error_no_close;
    + 
    +   if (!grub_password_get ((char *) cargs->key_data, 
GRUB_CRYPTODISK_MAX_PASSPHRASE))
    +     {
     @@ grub-core/disk/cryptodisk.c: grub_cryptodisk_scan_device_real (const 
char *name,
    + 
    +  error:
    +   cryptodisk_close (dev);
    ++ error_no_close:
        dev = NULL;
      
       cleanup:
    -+  if (read_hook_data.prev_read_hook != NULL)
    -+    {
    -+      source->read_hook = read_hook_data.prev_read_hook;
    -+      source->read_hook_data = read_hook_data.prev_read_hook_data;
    -+    }
    ++  if (cargs->hdr_file != NULL)
    ++    source->read_hook = NULL;
    ++
        if (askpass)
          {
            cargs->key_len = 0;
     @@ grub-core/disk/cryptodisk.c: grub_cmd_cryptomount 
(grub_extcmd_context_t ctxt, int argc, char **args)
    -   return grub_error (GRUB_ERR_FILE_READ_ERROR, (N_("reading key file")));
    +   return grub_error (GRUB_ERR_FILE_READ_ERROR, (N_("failed to read key 
file")));
          }
      
    -+  if (state[7].set) /* header */
    ++  if (state[OPTION_HEADER].set) /* header */
     +    {
    -+      if (state[0].set)
    ++      if (state[OPTION_UUID].set)
     +  return grub_error (GRUB_ERR_BAD_ARGUMENT,
     +                     N_("cannot use UUID lookup with detached header"));
     +
    -+      cargs.hdr_file = grub_file_open (state[7].arg,
    ++      cargs.hdr_file = grub_file_open (state[OPTION_HEADER].arg,
     +                      GRUB_FILE_TYPE_CRYPTODISK_DETACHED_HEADER);
     +      if (cargs.hdr_file == NULL)
     +  return grub_errno;
     +    }
     +
    -   if (state[0].set) /* uuid */
    +   if (state[OPTION_UUID].set) /* uuid */
          {
            int found_uuid;
     @@ grub-core/disk/cryptodisk.c: GRUB_MOD_INIT (cryptodisk)
3:  01c30f477 = 3:  a0bd5337e docs: Add documentation on detached header option 
to cryptomount
-- 
2.34.1