On Sun, 29 May 2022 08:45:39 +0200
Patrick Steinhardt <p...@pks.im> wrote:
> On Mon, May 16, 2022 at 04:49:47PM -0500, Glenn Washburn wrote:
> > 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.
> >
> > Signed-off-by: Glenn Washburn <developm...@efficientek.com>
> > ---
> > grub-core/disk/cryptodisk.c | 93 +++++++++++++++++++++++++++++++++++--
> > grub-core/disk/geli.c | 4 ++
> > include/grub/cryptodisk.h | 2 +
> > include/grub/file.h | 2 +
> > 4 files changed, 97 insertions(+), 4 deletions(-)
> >
> > diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c
> > index ecbda7ce9..fdb0461a8 100644
> > --- a/grub-core/disk/cryptodisk.c
> > +++ b/grub-core/disk/cryptodisk.c
> > @@ -43,7 +43,8 @@ enum
> > OPTION_PASSWORD,
> > OPTION_KEYFILE,
> > OPTION_KEYFILE_OFFSET,
> > - OPTION_KEYFILE_SIZE
> > + OPTION_KEYFILE_SIZE,
> > + OPTION_HEADER
> > };
> >
> > static const struct grub_arg_option options[] =
> > @@ -56,9 +57,16 @@ 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},
> > {"keyfile-size", 'S', 0, N_("Key file data size (bytes)"), 0,
> > ARG_TYPE_INT},
> > + {"header", 'H', 0, N_("Read header from file"), 0, ARG_TYPE_STRING},
> > {0, 0, 0, 0, 0, 0}
> > };
> >
> > +struct cryptodisk_read_hook_ctx
> > +{
> > + grub_file_t hdr_file;
> > +};
> > +typedef struct cryptodisk_read_hook_ctx *cryptodisk_read_hook_ctx_t;
> > +
> > /* Our irreducible polynom is x^128+x^7+x^2+x+1. Lowest byte of it is: */
> > #define GF_POLYNOM 0x87
> > static inline int GF_PER_SECTOR (const struct grub_cryptodisk *dev)
> > @@ -1004,6 +1012,31 @@ cryptodisk_close (grub_cryptodisk_t dev)
> > grub_free (dev);
> > }
> >
> > +static grub_err_t
> > +cryptodisk_read_hook (grub_disk_addr_t sector, unsigned offset,
> > + unsigned length, char *buf, void *data)
> > +{
> > + grub_err_t ret = GRUB_ERR_NONE;
>
> Nit: `ret` is effectively unused. We only set it here and then return it
> at the end of this function. It prompted me to search whether I just
> missed it being set, but it's not. So I'd just remove this variable and
> return `GRUB_ERR_NONE` directly to simplify this.
Agreed. I think it was more useful in a prior version. I'll remove it.
Glenn
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
