Re: [CRYPTO-LUKS v1 15/19] cryptodisk,luks2: Add header line to procfs entry and crypto and source device names.

[Glenn Washburn]

On Fri, 31 Jul 2020 17:37:46 +0200
Patrick Steinhardt <ps@pks.im> wrote:

> On Fri, Jul 31, 2020 at 07:01:56AM -0500, Glenn Washburn wrote:
> > A header line is added to luks_script for easier comprehension of
> > fields by user. And the crypto disk name and source device names
> > are added.
> > 
> > Signed-off-by: Glenn Washburn <development@efficientek.com>
> > ---
> >  grub-core/disk/cryptodisk.c | 28 ++++++++++++++++++++++------
> >  1 file changed, 22 insertions(+), 6 deletions(-)
> > 
> > diff --git a/grub-core/disk/cryptodisk.c
> > b/grub-core/disk/cryptodisk.c index 9399849b6..2c6e73b39 100644
> > --- a/grub-core/disk/cryptodisk.c
> > +++ b/grub-core/disk/cryptodisk.c
> > @@ -1206,35 +1206,48 @@ hex (grub_uint8_t val)
> >  }
> >  
> >  /* Open a file named NAME and initialize FILE.  */
> > +#define STR(s) #s
> > +#define MAX_ID_PRINT 10000
> >  static char *
> >  luks_script_get (grub_size_t *sz)
> >  {
> >    grub_cryptodisk_t i;
> >    grub_size_t size = 0;
> >    char *ptr, *ret;
> > +  const char header[] = N_("<type> <devname> <source disk> <uuid>
> > <sector "
> > +                      "offset> <sector size> <cipher> <key>
> > <options>\n");
> > +  static char errmsg[] = N_("Can not list more than "
> > STR(MAX_ID_PRINT)
> > +                       " crypto devices.\n");
> 
> Not sure, but this seems like a backwards-incompatible change to me,
> right? In case anybody has been looping over this list, he'll not know
> to skip the first line.
> 
> Patrick

Yes, it is backwards-incompatible. How likely do you think it is that
anyone would be doing pragmatically reading that data? As far as I know
the grub environment isn't powerful enough to do that.  I doubt anyone
writing modules would, since they'd just use the internal
data structures instead.  So that leaves getting that data through the
serial port (am I missing something?). Perhaps as part of some automated
test harness?  I've also added extra fields to each line, which would
be considered backwards-incompatible.  Also, this change should make
future additions more backward-incompatible proof as the header fields
make the output more like an associative array and you can choose your
fields by field name.

Glenn