On Thu, May 12, 2022 at 01:53:31PM -0500, Glenn Washburn wrote:
> On Thu, 12 May 2022 19:45:48 +0200
> Daniel Kiper <dki...@net-space.pl> wrote:
>
> > On Fri, May 06, 2022 at 03:45:59AM -0500, Glenn Washburn wrote:
> > > From: John Lane <j...@lane.uk.net>
> > >
> > > Add the options --key-file, --keyfile-offset, and --keyfile-size to
> > > cryptomount and code to put read the requested key file data and pass
> > > via the cargs struct. Note, key file data is for all intents and purposes
> > > equivalent to a password given to cryptomount. So there is no need to
> > > enable support for key files in the various crypto backends (eg. LUKS1)
> > > because the key data is passed just as if it were a password.
> > >
> > > Signed-off-by: John Lane <j...@lane.uk.net>
> > > gnu...@cyberdimension.org: rebase, patch split, small fixes, commit
> > > message
> > > Signed-off-by: Denis 'GNUtoo' Carikli <gnu...@cyberdimension.org>
> > > developm...@efficientek.com: rebase and rework to use cryptomount arg
> > > passing,
> > > minor fixes, improve commit message
> > > Signed-off-by: Glenn Washburn <developm...@efficientek.com>
> > > ---
> > > grub-core/disk/cryptodisk.c | 86 ++++++++++++++++++++++++++++++++++++-
> > > include/grub/cryptodisk.h | 2 +
> > > include/grub/file.h | 2 +
> > > 3 files changed, 89 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c
> > > index 9f5dc7acb..19af4fa49 100644
> > > --- a/grub-core/disk/cryptodisk.c
> > > +++ b/grub-core/disk/cryptodisk.c
> > > @@ -42,6 +42,9 @@ static const struct grub_arg_option options[] =
> > > {"all", 'a', 0, N_("Mount all."), 0, 0},
> > > {"boot", 'b', 0, N_("Mount all volumes with `boot' flag set."), 0,
> > > 0},
> > > {"password", 'p', 0, N_("Password to open volumes."), 0,
> > > ARG_TYPE_STRING},
> > > + {"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},
> > > {0, 0, 0, 0, 0, 0}
> > > };
> > >
> > > @@ -1172,6 +1175,85 @@ grub_cmd_cryptomount (grub_extcmd_context_t ctxt,
> > > int argc, char **args)
> > > cargs.key_len = grub_strlen (state[3].arg);
> > > }
> > >
> > > + if (state[4].set) /* keyfile */
> > > + {
> > > + const char *p = NULL;
> > > + grub_file_t keyfile;
> > > + unsigned long long keyfile_offset = 0, keyfile_size = 0;
I think keyfile_size should be "unsigned long" if you use grub_strtoul() below.
> > > +
> > > + if (state[5].set) /* keyfile-offset */
> > > + {
> > > + keyfile_offset = grub_strtoull (state[5].arg, &p, 0);
> >
> > Hmmm... Could not you use grub_strtoul() instead of grub_strtoull()?
>
> I was thinking the allowing the largest possible offset might be useful
> to allow for using unallocated space at the end of large block devices.
> This could still be done with smaller offsets, by using the blocklist
> syntax to create a "file" that starts near the end and then use a much
> smaller offset. Why don't you like it as is?
I would not say I do not like it. I just want to understand discrepancy
between this conversion and one below. If you need "unsigned long long"
go ahead with it. Though I would suggest to check the value does not
exceed maximum value of target type. I think GRUB_TYPE_U_MAX() can be
useful here. Same applies to conversion below.
Daniel
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
