[PATCH 0/2] Support plain encryption mode.

[PATCH 0/2] Support plain encryption mode.

[Maxim Fomin]

This patch adds support for plain encryption mode (plain dm-crypt) via new
module/command named 'plainmount'. This is an extension of previous patch
(member of crypto enhancement patch series) sent to grub-devel by John Lane.
The plain mode patch was fully reworked, the most important changes are:

1. The code is rewritten as a separate 'plainmount' module and command. This
change addresses several issues raised in previous patch discussion: complex
implementation and expanding too much cryptomount parameter list. Plainmount
module still depends on several functions in cryptodisk module just like LUKS,
LUKS2 and geli modules. I tried to minimize link to cryptodisk, but some link
is unavoidable.

2. Plainmount now uses GPT partition unique UUID to point to exact partition.
Plain mode does not have special header (as LUKS/LUKS2 does), so it does not
have any UUID. Absence of UUID was key issue in discussions of original plain
mode patch - without UUID any link like 'hdX,gptY' in grub.cfg can be broken
in multiboot setup because hdX can map to different disks each boot.

3. The patch brings support for keyfiles and using disk blocks as a keyfile.
This functionality was added in cryptodisk module (and thus was available for
plain mode) via other patch from 'crypto enhancement' patch series. Thus all
features from original patch series are available in plainmount module (except
detached header which is LUKS only feature and multiple password attempts which
are undefined in plain mode). The patch also brings support for large blocks
(1024/2048/4096 byte).

Some issues are still not resolved. Although the problem with UUID is solved,
no changes were made in grub-install or grub-mkconfig. Currently user must
manually edit grub.cfg to add plainmount command. Also, inserting plainmount
in standalone grub does not work because plainmount needs crypto arguments.
Theoretically they can be stored somewhere in efi core image - in this case
plain mode can be fully supported like LUKS. Currently user must manually edit
grub.cfg to use plainmount (or manually type in console). Still, this helps in
some use cases (including some complex setup scenarios). Also, except opening
plain mode encrypted disks plainmount command can decrypt LUKS volumes if
master key is known.

Maxim Fomin (2):
  plainmount: Support decryption of devices encrypted in plain mode.
  plainmount: Document new command syntax and options.

 docs/grub.texi              |  52 +++
 grub-core/Makefile.core.def |   5 +
 grub-core/disk/plainmount.c | 678 ++++++++++++++++++++++++++++++++++++
 3 files changed, 735 insertions(+)
 create mode 100644 grub-core/disk/plainmount.c

--
2.35.1

Re: [PATCH 0/2] Support plain encryption mode.

[Glenn Washburn]

On Sun, 30 Jan 2022 19:40:37 +0000
Maxim Fomin <maxim@fomin.one> wrote:

> This patch adds support for plain encryption mode (plain dm-crypt) via new
> module/command named 'plainmount'. This is an extension of previous patch
> (member of crypto enhancement patch series) sent to grub-devel by John Lane.
> The plain mode patch was fully reworked, the most important changes are:
> 
> 1. The code is rewritten as a separate 'plainmount' module and command. This
> change addresses several issues raised in previous patch discussion: complex
> implementation and expanding too much cryptomount parameter list. Plainmount
> module still depends on several functions in cryptodisk module just like LUKS,
> LUKS2 and geli modules. I tried to minimize link to cryptodisk, but some link
> is unavoidable.
> 
> 2. Plainmount now uses GPT partition unique UUID to point to exact partition.
> Plain mode does not have special header (as LUKS/LUKS2 does), so it does not
> have any UUID. Absence of UUID was key issue in discussions of original plain
> mode patch - without UUID any link like 'hdX,gptY' in grub.cfg can be broken
> in multiboot setup because hdX can map to different disks each boot.

So plain mount can only be used for volumes located on GPT partitions,
leaving out MBR or any other partitioning format, full-disk encryption,
and loopback file encryption. I think this is an undesirable and
unnecessary limitation.

Instead, I suggest assuming the user knows what they are doing in the
GRUB shell/script and letting them do it. This should not be allowed
when automatically generating the grub.cfg using GRUB user-space tools.
GRUB should fail to automagically install to a device hosted on a plain
encrypted container (unless perhaps an override is given). I suspect
that this is the default, but worth mentioning.

> 3. The patch brings support for keyfiles and using disk blocks as a keyfile.
> This functionality was added in cryptodisk module (and thus was available for
> plain mode) via other patch from 'crypto enhancement' patch series. Thus all
> features from original patch series are available in plainmount module (except
> detached header which is LUKS only feature and multiple password attempts which
> are undefined in plain mode). The patch also brings support for large blocks
> (1024/2048/4096 byte).
> 
> Some issues are still not resolved. Although the problem with UUID is solved,
> no changes were made in grub-install or grub-mkconfig. Currently user must
> manually edit grub.cfg to add plainmount command.

I think this is fine.

> Also, inserting plainmount
> in standalone grub does not work because plainmount needs crypto arguments.
> Theoretically they can be stored somewhere in efi core image - in this case
> plain mode can be fully supported like LUKS. Currently user must manually edit
> grub.cfg to use plainmount (or manually type in console). Still, this helps in
> some use cases (including some complex setup scenarios). Also, except opening
> plain mode encrypted disks plainmount command can decrypt LUKS volumes if
> master key is known.

On the next iteration of this patch series, please use --thread to git
format-patch and git send-email.

Glenn

> Maxim Fomin (2):
>   plainmount: Support decryption of devices encrypted in plain mode.
>   plainmount: Document new command syntax and options.
> 
>  docs/grub.texi              |  52 +++
>  grub-core/Makefile.core.def |   5 +
>  grub-core/disk/plainmount.c | 678 ++++++++++++++++++++++++++++++++++++
>  3 files changed, 735 insertions(+)
>  create mode 100644 grub-core/disk/plainmount.c
> 
> --
> 2.35.1
> 
> 
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH 0/2] Support plain encryption mode.

[Maxim Fomin]

------- Original Message -------

On Monday, January 31st, 2022 at 23:40, Glenn Washburn <development@efficientek.com> wrote:

> On Sun, 30 Jan 2022 19:40:37 +0000
>
> Maxim Fomin maxim@fomin.one wrote:
>
> > This patch adds support for plain encryption mode (plain dm-crypt) via new
> >
> > module/command named 'plainmount'. This is an extension of previous patch
> >
> > (member of crypto enhancement patch series) sent to grub-devel by John Lane.
> >
> > The plain mode patch was fully reworked, the most important changes are:
> >
> > 1.  The code is rewritten as a separate 'plainmount' module and command. This
> >
> >     change addresses several issues raised in previous patch discussion: complex
> >
> >     implementation and expanding too much cryptomount parameter list. Plainmount
> >
> >     module still depends on several functions in cryptodisk module just like LUKS,
> >
> >     LUKS2 and geli modules. I tried to minimize link to cryptodisk, but some link
> >
> >     is unavoidable.
> >
> > 2.  Plainmount now uses GPT partition unique UUID to point to exact partition.
> >
> >     Plain mode does not have special header (as LUKS/LUKS2 does), so it does not
> >
> >     have any UUID. Absence of UUID was key issue in discussions of original plain
> >
> >     mode patch - without UUID any link like 'hdX,gptY' in grub.cfg can be broken
> >
> >     in multiboot setup because hdX can map to different disks each boot.
> >
>
> So plain mount can only be used for volumes located on GPT partitions,
>
> leaving out MBR or any other partitioning format, full-disk encryption,
>
> and loopback file encryption. I think this is an undesirable and
>
> unnecessary limitation.
>
> Instead, I suggest assuming the user knows what they are doing in the
>
> GRUB shell/script and letting them do it. This should not be allowed
>
> when automatically generating the grub.cfg using GRUB user-space tools.
>
> GRUB should fail to automagically install to a device hosted on a plain
>
> encrypted container (unless perhaps an override is given). I suspect
>
> that this is the default, but worth mentioning.
>
> Glenn

Plainmount can work with '(hdX,gptY)' syntax in config or shell (actually, this
is the base syntax) and thus it is not limited to GPT paritions, what is limited
is the ability to use UUID - currently only on GPT. If partition scheme does not
have UUID then UUID as a convenience feature cannot be supported - inconvenient,
but technically fair. I will take a look at MBR UUID and see whether they can be
supported. Possible situations (under current implementaion) are follows:

a) GPT disk, multi-disk environment, disks map unpredictably: can name partitions
   by GPT UUID in config file/shell, no problem, ability to name by UUID has value
b) GPT disk, single disk environment: no problem, UUID feature has less value
b) not GPT, single disk environment: no problem, config like 'plainmount (hd0,gpt2)'
   always works
d) not GPT, multi-disk environment, disks map unpredictably: worst case, link
   '(hd0,gpt2)' can randomly fail, but user can type manually in shell
e) cannot mount plain mode device in grub shell <- this is impossible, plainmount
   can always be called in shell.

Best regards,
Maxim Fomin

Re: [PATCH 0/2] Support plain encryption mode.

[Glenn Washburn]

On Tue, 01 Feb 2022 15:48:01 +0000
Maxim Fomin <maxim@fomin.one> wrote:

> ------- Original Message -------
> 
> On Monday, January 31st, 2022 at 23:40, Glenn Washburn <development@efficientek.com> wrote:
> 
> > On Sun, 30 Jan 2022 19:40:37 +0000
> >
> > Maxim Fomin maxim@fomin.one wrote:
> >
> > > This patch adds support for plain encryption mode (plain dm-crypt) via new
> > >
> > > module/command named 'plainmount'. This is an extension of previous patch
> > >
> > > (member of crypto enhancement patch series) sent to grub-devel by John Lane.
> > >
> > > The plain mode patch was fully reworked, the most important changes are:
> > >
> > > 1.  The code is rewritten as a separate 'plainmount' module and command. This
> > >
> > >     change addresses several issues raised in previous patch discussion: complex
> > >
> > >     implementation and expanding too much cryptomount parameter list. Plainmount
> > >
> > >     module still depends on several functions in cryptodisk module just like LUKS,
> > >
> > >     LUKS2 and geli modules. I tried to minimize link to cryptodisk, but some link
> > >
> > >     is unavoidable.
> > >
> > > 2.  Plainmount now uses GPT partition unique UUID to point to exact partition.
> > >
> > >     Plain mode does not have special header (as LUKS/LUKS2 does), so it does not
> > >
> > >     have any UUID. Absence of UUID was key issue in discussions of original plain
> > >
> > >     mode patch - without UUID any link like 'hdX,gptY' in grub.cfg can be broken
> > >
> > >     in multiboot setup because hdX can map to different disks each boot.
> > >
> >
> > So plain mount can only be used for volumes located on GPT partitions,
> >
> > leaving out MBR or any other partitioning format, full-disk encryption,
> >
> > and loopback file encryption. I think this is an undesirable and
> >
> > unnecessary limitation.
> >
> > Instead, I suggest assuming the user knows what they are doing in the
> >
> > GRUB shell/script and letting them do it. This should not be allowed
> >
> > when automatically generating the grub.cfg using GRUB user-space tools.
> >
> > GRUB should fail to automagically install to a device hosted on a plain
> >
> > encrypted container (unless perhaps an override is given). I suspect
> >
> > that this is the default, but worth mentioning.
> >
> > Glenn
> 
> Plainmount can work with '(hdX,gptY)' syntax in config or shell (actually, this
> is the base syntax) and thus it is not limited to GPT paritions, what is limited
> is the ability to use UUID - currently only on GPT. If partition scheme does not
> have UUID then UUID as a convenience feature cannot be supported - inconvenient,
> but technically fair. I will take a look at MBR UUID and see whether they can be
> supported. Possible situations (under current implementaion) are follows:
> 
> a) GPT disk, multi-disk environment, disks map unpredictably: can name partitions
>    by GPT UUID in config file/shell, no problem, ability to name by UUID has value

I agree that searching by partition UUID is useful and desirable.
However, I don't think this is the right approach. GRUB should have
generic searching by partition UUID. There is already a patch for
this[1]. Perhaps you can test/review this patch to help it gain more
visibility and advocate for it being accepted.

Glenn

[1] https://lists.gnu.org/archive/html/grub-devel/2021-04/msg00055.html

> b) GPT disk, single disk environment: no problem, UUID feature has less value
> b) not GPT, single disk environment: no problem, config like 'plainmount (hd0,gpt2)'
>    always works
> d) not GPT, multi-disk environment, disks map unpredictably: worst case, link
>    '(hd0,gpt2)' can randomly fail, but user can type manually in shell
> e) cannot mount plain mode device in grub shell <- this is impossible, plainmount
>    can always be called in shell.
> 
> Best regards,
> Maxim Fomin
> 
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel

Re: [PATCH 0/2] Support plain encryption mode.

[Maxim Fomin]

------- Original Message -------
> >
> > Plainmount can work with '(hdX,gptY)' syntax in config or shell (actually, this
> >
> > is the base syntax) and thus it is not limited to GPT paritions, what is limited
> >
> > is the ability to use UUID - currently only on GPT. If partition scheme does not
> >
> > have UUID then UUID as a convenience feature cannot be supported - inconvenient,
> >
> > but technically fair. I will take a look at MBR UUID and see whether they can be
> >
> > supported. Possible situations (under current implementaion) are follows:
> >
> > a) GPT disk, multi-disk environment, disks map unpredictably: can name partitions
> >
> > by GPT UUID in config file/shell, no problem, ability to name by UUID has value
>
> I agree that searching by partition UUID is useful and desirable.
>
> However, I don't think this is the right approach. GRUB should have
>
> generic searching by partition UUID. There is already a patch for
>
> this[1]. Perhaps you can test/review this patch to help it gain more
>
> visibility and advocate for it being accepted.
>
> Glenn
>
> [1] https://lists.gnu.org/archive/html/grub-devel/2021-04/msg00055.html
>

Such function (or several functions) should be added into grub 'library', so it can be
used to search disk by PART UUID in different places. The patch you refer to seems to
add this functionality only to 'search' grub command via 'void grub_search_partuuid'
function. Can it be reused on other places? It seems in oder to use it, grub code must
call 'search' command and receive the result from grub environment variable which is
not convinient for other grub code interested in this feature. I think the proper way
to do it is to write some library function which can be used by search, probe (btw I
borrowed some details from it - so there is code duplication in search/probe),
plainmount commands and other commands in grub.

Best regards,
Maxim Fomin

Re: [PATCH 0/2] Support plain encryption mode.

[Glenn Washburn]

On Wed, 02 Feb 2022 15:00:10 +0000
Maxim Fomin <maxim@fomin.one> wrote:

> ------- Original Message -------
> > >
> > > Plainmount can work with '(hdX,gptY)' syntax in config or shell (actually, this
> > >
> > > is the base syntax) and thus it is not limited to GPT paritions, what is limited
> > >
> > > is the ability to use UUID - currently only on GPT. If partition scheme does not
> > >
> > > have UUID then UUID as a convenience feature cannot be supported - inconvenient,
> > >
> > > but technically fair. I will take a look at MBR UUID and see whether they can be
> > >
> > > supported. Possible situations (under current implementaion) are follows:
> > >
> > > a) GPT disk, multi-disk environment, disks map unpredictably: can name partitions
> > >
> > > by GPT UUID in config file/shell, no problem, ability to name by UUID has value
> >
> > I agree that searching by partition UUID is useful and desirable.
> >
> > However, I don't think this is the right approach. GRUB should have
> >
> > generic searching by partition UUID. There is already a patch for
> >
> > this[1]. Perhaps you can test/review this patch to help it gain more
> >
> > visibility and advocate for it being accepted.
> >
> > Glenn
> >
> > [1] https://lists.gnu.org/archive/html/grub-devel/2021-04/msg00055.html
> >
> 
> Such function (or several functions) should be added into grub 'library', so it can be
> used to search disk by PART UUID in different places. The patch you refer to seems to
> add this functionality only to 'search' grub command via 'void grub_search_partuuid'
> function. Can it be reused on other places? It seems in oder to use it, grub code must
> call 'search' command and receive the result from grub environment variable which is
> not convinient for other grub code interested in this feature. I think the proper way
> to do it is to write some library function which can be used by search, probe (btw I
> borrowed some details from it - so there is code duplication in search/probe),
> plainmount commands and other commands in grub.

I agree that reducing code duplication would be a good idea.
Essentially the grub 'library' you're wanting exists as the kernel code
that is always loaded. Would you like to find a good place to put the
common partition uuid matching code and send a patch?

As far as plainmount is concerned, I wasn't envisioning that it use
that code directly and it shouldn't. I was imagining something like
this snippet of GRUB script:

  search --partuuid --set KEYFILEDISK -u $PARTUUID
  plainmount -k ($KEYFILEDISK)/path/to/keyfile <other options>

Glenn