[PATCH v3 0/3] Cryptomount detached headers

* [PATCH v3 0/3] Cryptomount detached headers
@ 2022-06-08 15:34 Glenn Washburn
  2022-06-08 15:34 ` [PATCH v3 1/3] disk: Allow read hook callback to take read buffer to potentially modify it Glenn Washburn
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Glenn Washburn @ 2022-06-08 15:34 UTC (permalink / raw)
  To: grub-devel, Daniel Kiper
  Cc: Denis 'GNUtoo' Carikli, Patrick Steinhardt, John Lane,
	Glenn Washburn

Updates since v2:
 * Address uneeded ret variable pointed out by Patrick
 * Rebased onto latest master with keyfile and security changes. I don't think
   this actually changed these patches though.

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    | 92 ++++++++++++++++++++++++++++++++--
 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, 146 insertions(+), 38 deletions(-)

Range-diff against v2:
1:  91fa9a8eb ! 1:  a74dfb620 disk: Allow read hook callback to take read buffer to potentially modify it
    @@ Commit message
         hook. Also changed is that now when the read hook callback is called it can
         also indicate what error code should be sent back to the read caller.

    +    Signed-off-by: Glenn Washburn <development@efficientek.com>
    +    Reviewed-by: Patrick Steinhardt <ps@pks.im>
    +
      ## grub-core/commands/blocklist.c ##
     @@ grub-core/commands/blocklist.c: print_blocklist (grub_disk_addr_t sector, unsigned num,
      }
2:  f16d0a8b0 ! 2:  39bbab75c cryptodisk: Add support for using detached header files
    @@ Commit message
         Also add a --header (short -H) option to cryptomount which takes a file
         argument.

    +    Signed-off-by: Glenn Washburn <development@efficientek.com>
    +    Reviewed-by: Patrick Steinhardt <ps@pks.im>
    +
      ## grub-core/disk/cryptodisk.c ##
     @@ grub-core/disk/cryptodisk.c: enum
          OPTION_PASSWORD,
    @@ grub-core/disk/cryptodisk.c: cryptodisk_close (grub_cryptodisk_t dev)
     +cryptodisk_read_hook (grub_disk_addr_t sector, unsigned offset,
     +		      unsigned length, char *buf, void *data)
     +{
    -+  grub_err_t ret = GRUB_ERR_NONE;
     +  cryptodisk_read_hook_ctx_t ctx = data;
     +
     +  if (ctx->hdr_file == NULL)
    @@ grub-core/disk/cryptodisk.c: cryptodisk_close (grub_cryptodisk_t dev)
     +      return grub_errno;
     +    }
     +
    -+  return ret;
    ++  return GRUB_ERR_NONE;
     +}
     +
      static grub_cryptodisk_t
    @@ include/grub/cryptodisk.h: struct grub_cryptomount_args
      ## include/grub/file.h ##
     @@ include/grub/file.h: enum grub_file_type
          GRUB_FILE_TYPE_ZFS_ENCRYPTION_KEY,
    -     /* File holding the encryption key */
    +     /* File holding the encryption key. */
          GRUB_FILE_TYPE_CRYPTODISK_ENCRYPTION_KEY,
     +    /* File holding the encryption metadata header */
     +    GRUB_FILE_TYPE_CRYPTODISK_DETACHED_HEADER,
3:  f13a6e840 ! 3:  2deb90d33 docs: Add documentation on detached header option to cryptomount
    @@ Metadata
      ## Commit message ##
         docs: Add documentation on detached header option to cryptomount

    +    Signed-off-by: Glenn Washburn <development@efficientek.com>
    +    Reviewed-by: Patrick Steinhardt <ps@pks.im>
    +
      ## docs/grub.texi ##
     @@ docs/grub.texi: Alias for @code{hashsum --hash crc32 arg @dots{}}. See command @command{hashsum}
      @node cryptomount
-- 
2.34.1

^ permalink raw reply	[flat|nested] 5+ messages in thread