Re: Patch: Allow Ext4 partitions with encrypted directories.

Re: Patch: Allow Ext4 partitions with encrypted directories.

[Andrei Borzenkov]

On Wed, Nov 2, 2016 at 12:22 AM, Samee Zahur <szahur@google.com> wrote:
> Ext4 filesystem now allows users to choose directory trees to be stored
> encrypted. However, GRUB refuses to boot from such partitions, even if none
> of the boot-critical files are actually affected. The following patch fixes
> this. It was tested on the latest release version of ext4.
>
> Please let me know if more information is needed.
>
> diff --git a/grub-core/fs/ext2.c b/grub-core/fs/ext2.c
> index cdce63b..eca10e4 100644
> --- a/grub-core/fs/ext2.c
> +++ b/grub-core/fs/ext2.c
> @@ -2,6 +2,7 @@
>  /*
>   *  GRUB  --  GRand Unified Bootloader
>   *  Copyright (C) 2003,2004,2005,2007,2008,2009  Free Software Foundation,
> Inc.
> + *  Copyright (C) 2016 Google, Inc.
>   *

Hmm ... I had to sign contributor agreement that transfers copyright
to FSF. Not that I care personally but that may be problem ...

>   *  GRUB is free software: you can redistribute it and/or modify
>   *  it under the terms of the GNU General Public License as published by
> @@ -102,6 +103,7 @@ GRUB_MOD_LICENSE ("GPLv3+");
>  #define EXT4_FEATURE_INCOMPAT_64BIT            0x0080
>  #define EXT4_FEATURE_INCOMPAT_MMP              0x0100
>  #define EXT4_FEATURE_INCOMPAT_FLEX_BG          0x0200
> +#define EXT4_FEATURE_INCOMPAT_ENCRYPT          0x10000
>
>  /* The set of back-incompatible features this driver DOES support. Add (OR)
>   * flags here as the related features are implemented into the driver.  */
> @@ -120,9 +122,12 @@ GRUB_MOD_LICENSE ("GPLv3+");
>   * mmp:            Not really back-incompatible - was added as such to
>   *                 avoid multiple read-write mounts. Safe to ignore for
> this
>   *                 RO driver.
> + * encrypt:        We assume boot files are not encrypted (grub config,
> kernel,
> + *                 initramd etc.). If we are wrong, boot will fail as it
> should.
>   */

Do not assume users won't try to access something else.

>  #define EXT2_DRIVER_IGNORED_INCOMPAT ( EXT3_FEATURE_INCOMPAT_RECOVER \
> -                                    | EXT4_FEATURE_INCOMPAT_MMP)
> +                                    | EXT4_FEATURE_INCOMPAT_MMP     \
> +                                    | EXT4_FEATURE_INCOMPAT_ENCRYPT)
>

And what happens when grub does see encrypted content? Returning
garbage is not an option here.

>
>  #define EXT3_JOURNAL_MAGIC_NUMBER      0xc03b3998U
>
>
> _______________________________________________
> Bug-grub mailing list
> Bug-grub@gnu.org
> https://lists.gnu.org/mailman/listinfo/bug-grub
>

Re: Patch: Allow Ext4 partitions with encrypted directories.

[Samee Zahur]

[-- Attachment #1: Type: text/plain, Size: 3343 bytes --]

> Hmm ... I had to sign contributor agreement that transfers copyright
> to FSF. Not that I care personally but that may be problem ...

Yeah, I don't personally care either. If someone at FSF raises an issue, I
can reach out to legal experts here. But this is not the first time Google
(C) has been checked into GRUB. We patch FSF GNU code all the time.

> And what happens when grub does see encrypted content? Returning
> garbage is not an option here.

Good question. The files simply won't be found. The filenames are also
garbled, so GRUB won't find the files it's looking for.

On Thu, Nov 3, 2016 at 8:16 AM, Andrei Borzenkov <arvidjaar@gmail.com>
wrote:

> On Wed, Nov 2, 2016 at 12:22 AM, Samee Zahur <szahur@google.com> wrote:
> > Ext4 filesystem now allows users to choose directory trees to be stored
> > encrypted. However, GRUB refuses to boot from such partitions, even if
> none
> > of the boot-critical files are actually affected. The following patch
> fixes
> > this. It was tested on the latest release version of ext4.
> >
> > Please let me know if more information is needed.
> >
> > diff --git a/grub-core/fs/ext2.c b/grub-core/fs/ext2.c
> > index cdce63b..eca10e4 100644
> > --- a/grub-core/fs/ext2.c
> > +++ b/grub-core/fs/ext2.c
> > @@ -2,6 +2,7 @@
> >  /*
> >   *  GRUB  --  GRand Unified Bootloader
> >   *  Copyright (C) 2003,2004,2005,2007,2008,2009  Free Software
> Foundation,
> > Inc.
> > + *  Copyright (C) 2016 Google, Inc.
> >   *
>
> Hmm ... I had to sign contributor agreement that transfers copyright
> to FSF. Not that I care personally but that may be problem ...
>
> >   *  GRUB is free software: you can redistribute it and/or modify
> >   *  it under the terms of the GNU General Public License as published by
> > @@ -102,6 +103,7 @@ GRUB_MOD_LICENSE ("GPLv3+");
> >  #define EXT4_FEATURE_INCOMPAT_64BIT            0x0080
> >  #define EXT4_FEATURE_INCOMPAT_MMP              0x0100
> >  #define EXT4_FEATURE_INCOMPAT_FLEX_BG          0x0200
> > +#define EXT4_FEATURE_INCOMPAT_ENCRYPT          0x10000
> >
> >  /* The set of back-incompatible features this driver DOES support. Add
> (OR)
> >   * flags here as the related features are implemented into the driver.
> */
> > @@ -120,9 +122,12 @@ GRUB_MOD_LICENSE ("GPLv3+");
> >   * mmp:            Not really back-incompatible - was added as such to
> >   *                 avoid multiple read-write mounts. Safe to ignore for
> > this
> >   *                 RO driver.
> > + * encrypt:        We assume boot files are not encrypted (grub config,
> > kernel,
> > + *                 initramd etc.). If we are wrong, boot will fail as it
> > should.
> >   */
>
> Do not assume users won't try to access something else.
>
> >  #define EXT2_DRIVER_IGNORED_INCOMPAT ( EXT3_FEATURE_INCOMPAT_RECOVER \
> > -                                    | EXT4_FEATURE_INCOMPAT_MMP)
> > +                                    | EXT4_FEATURE_INCOMPAT_MMP     \
> > +                                    | EXT4_FEATURE_INCOMPAT_ENCRYPT)
> >
>
> And what happens when grub does see encrypted content? Returning
> garbage is not an option here.
>
> >
> >  #define EXT3_JOURNAL_MAGIC_NUMBER      0xc03b3998U
> >
> >
> > _______________________________________________
> > Bug-grub mailing list
> > Bug-grub@gnu.org
> > https://lists.gnu.org/mailman/listinfo/bug-grub
> >
>

[-- Attachment #2: Type: text/html, Size: 4923 bytes --]

Re: Patch: Allow Ext4 partitions with encrypted directories.

[Andrei Borzenkov]

03.11.2016 20:24, Samee Zahur пишет:
>> Hmm ... I had to sign contributor agreement that transfers copyright
>> to FSF. Not that I care personally but that may be problem ...
> 
> Yeah, I don't personally care either. If someone at FSF raises an issue, I
> can reach out to legal experts here. But this is not the first time Google
> (C) has been checked into GRUB. We patch FSF GNU code all the time.
> 
>> And what happens when grub does see encrypted content? Returning
>> garbage is not an option here.
> 
> Good question. The files simply won't be found. The filenames are also
> garbled, so GRUB won't find the files it's looking for.
> 

Do you mean that if grub tries to open this garbled name it succeeds? Is
it possible to detect that directory is encrypted? Then we should refuse
to access this directory with clear explanation.

> On Thu, Nov 3, 2016 at 8:16 AM, Andrei Borzenkov <arvidjaar@gmail.com>
> wrote:
> 
>> On Wed, Nov 2, 2016 at 12:22 AM, Samee Zahur <szahur@google.com> wrote:
>>> Ext4 filesystem now allows users to choose directory trees to be stored
>>> encrypted. However, GRUB refuses to boot from such partitions, even if
>> none
>>> of the boot-critical files are actually affected. The following patch
>> fixes
>>> this. It was tested on the latest release version of ext4.
>>>
>>> Please let me know if more information is needed.
>>>
>>> diff --git a/grub-core/fs/ext2.c b/grub-core/fs/ext2.c
>>> index cdce63b..eca10e4 100644
>>> --- a/grub-core/fs/ext2.c
>>> +++ b/grub-core/fs/ext2.c
>>> @@ -2,6 +2,7 @@
>>>  /*
>>>   *  GRUB  --  GRand Unified Bootloader
>>>   *  Copyright (C) 2003,2004,2005,2007,2008,2009  Free Software
>> Foundation,
>>> Inc.
>>> + *  Copyright (C) 2016 Google, Inc.
>>>   *
>>
>> Hmm ... I had to sign contributor agreement that transfers copyright
>> to FSF. Not that I care personally but that may be problem ...
>>
>>>   *  GRUB is free software: you can redistribute it and/or modify
>>>   *  it under the terms of the GNU General Public License as published by
>>> @@ -102,6 +103,7 @@ GRUB_MOD_LICENSE ("GPLv3+");
>>>  #define EXT4_FEATURE_INCOMPAT_64BIT            0x0080
>>>  #define EXT4_FEATURE_INCOMPAT_MMP              0x0100
>>>  #define EXT4_FEATURE_INCOMPAT_FLEX_BG          0x0200
>>> +#define EXT4_FEATURE_INCOMPAT_ENCRYPT          0x10000
>>>
>>>  /* The set of back-incompatible features this driver DOES support. Add
>> (OR)
>>>   * flags here as the related features are implemented into the driver.
>> */
>>> @@ -120,9 +122,12 @@ GRUB_MOD_LICENSE ("GPLv3+");
>>>   * mmp:            Not really back-incompatible - was added as such to
>>>   *                 avoid multiple read-write mounts. Safe to ignore for
>>> this
>>>   *                 RO driver.
>>> + * encrypt:        We assume boot files are not encrypted (grub config,
>>> kernel,
>>> + *                 initramd etc.). If we are wrong, boot will fail as it
>>> should.
>>>   */
>>
>> Do not assume users won't try to access something else.
>>
>>>  #define EXT2_DRIVER_IGNORED_INCOMPAT ( EXT3_FEATURE_INCOMPAT_RECOVER \
>>> -                                    | EXT4_FEATURE_INCOMPAT_MMP)
>>> +                                    | EXT4_FEATURE_INCOMPAT_MMP     \
>>> +                                    | EXT4_FEATURE_INCOMPAT_ENCRYPT)
>>>
>>
>> And what happens when grub does see encrypted content? Returning
>> garbage is not an option here.
>>
>>>
>>>  #define EXT3_JOURNAL_MAGIC_NUMBER      0xc03b3998U
>>>
>>>
>>> _______________________________________________
>>> Bug-grub mailing list
>>> Bug-grub@gnu.org
>>> https://lists.gnu.org/mailman/listinfo/bug-grub
>>>
>>
> 

Re: Patch: Allow Ext4 partitions with encrypted directories.

[Samee Zahur]

[-- Attachment #1: Type: text/plain, Size: 5380 bytes --]

It should be possible, but I'm not sure if I can test that patch (both for
lack of knowledge, and time constraints). I am happy to send the patch over
if somebody else wants to take over testing it.

Here's what my patch would probably do:
In grub-core/fs/ext2.c, at the end of grub_ext2_read_inode, it can test
inode->flags to see if it's encrypted. If so, return some error code
(suggestions?). I think that will do the trick, although I'm not familiar
with grub enough to know for sure.

Testing: it requires creating scenario grub tries to access a file that it
doesn't normally do during the boot process, and observe what the error is.
Possibly using grub shell? Not sure.

Status without any additional files, as things stand:
I have tested on machines here, and things work without a problem. grub
will not return garbage in the sense of arbitrary data from unknown blocks
of disk ... it will return a valid, well-structured file with actual
ciphertext. This was never a problem since, in our configuration, grub
never tries to access files with names it doesn't recognize.

So we can either:
1) Have me send a patch for proper detection so that somebody else can test
it.
2) Accept the current patch as is, given I have already tested it.
3) Ignore it, and not add this feature to grub at this point.

Let me know what you want do.

On Sun, Nov 6, 2016 at 12:08 AM, Andrei Borzenkov <arvidjaar@gmail.com>
wrote:

> 03.11.2016 20:24, Samee Zahur пишет:
> >> Hmm ... I had to sign contributor agreement that transfers copyright
> >> to FSF. Not that I care personally but that may be problem ...
> >
> > Yeah, I don't personally care either. If someone at FSF raises an issue,
> I
> > can reach out to legal experts here. But this is not the first time
> Google
> > (C) has been checked into GRUB. We patch FSF GNU code all the time.
> >
> >> And what happens when grub does see encrypted content? Returning
> >> garbage is not an option here.
> >
> > Good question. The files simply won't be found. The filenames are also
> > garbled, so GRUB won't find the files it's looking for.
> >
>
> Do you mean that if grub tries to open this garbled name it succeeds? Is
> it possible to detect that directory is encrypted? Then we should refuse
> to access this directory with clear explanation.
>
> > On Thu, Nov 3, 2016 at 8:16 AM, Andrei Borzenkov <arvidjaar@gmail.com>
> > wrote:
> >
> >> On Wed, Nov 2, 2016 at 12:22 AM, Samee Zahur <szahur@google.com> wrote:
> >>> Ext4 filesystem now allows users to choose directory trees to be stored
> >>> encrypted. However, GRUB refuses to boot from such partitions, even if
> >> none
> >>> of the boot-critical files are actually affected. The following patch
> >> fixes
> >>> this. It was tested on the latest release version of ext4.
> >>>
> >>> Please let me know if more information is needed.
> >>>
> >>> diff --git a/grub-core/fs/ext2.c b/grub-core/fs/ext2.c
> >>> index cdce63b..eca10e4 100644
> >>> --- a/grub-core/fs/ext2.c
> >>> +++ b/grub-core/fs/ext2.c
> >>> @@ -2,6 +2,7 @@
> >>>  /*
> >>>   *  GRUB  --  GRand Unified Bootloader
> >>>   *  Copyright (C) 2003,2004,2005,2007,2008,2009  Free Software
> >> Foundation,
> >>> Inc.
> >>> + *  Copyright (C) 2016 Google, Inc.
> >>>   *
> >>
> >> Hmm ... I had to sign contributor agreement that transfers copyright
> >> to FSF. Not that I care personally but that may be problem ...
> >>
> >>>   *  GRUB is free software: you can redistribute it and/or modify
> >>>   *  it under the terms of the GNU General Public License as published
> by
> >>> @@ -102,6 +103,7 @@ GRUB_MOD_LICENSE ("GPLv3+");
> >>>  #define EXT4_FEATURE_INCOMPAT_64BIT            0x0080
> >>>  #define EXT4_FEATURE_INCOMPAT_MMP              0x0100
> >>>  #define EXT4_FEATURE_INCOMPAT_FLEX_BG          0x0200
> >>> +#define EXT4_FEATURE_INCOMPAT_ENCRYPT          0x10000
> >>>
> >>>  /* The set of back-incompatible features this driver DOES support. Add
> >> (OR)
> >>>   * flags here as the related features are implemented into the driver.
> >> */
> >>> @@ -120,9 +122,12 @@ GRUB_MOD_LICENSE ("GPLv3+");
> >>>   * mmp:            Not really back-incompatible - was added as such to
> >>>   *                 avoid multiple read-write mounts. Safe to ignore
> for
> >>> this
> >>>   *                 RO driver.
> >>> + * encrypt:        We assume boot files are not encrypted (grub
> config,
> >>> kernel,
> >>> + *                 initramd etc.). If we are wrong, boot will fail as
> it
> >>> should.
> >>>   */
> >>
> >> Do not assume users won't try to access something else.
> >>
> >>>  #define EXT2_DRIVER_IGNORED_INCOMPAT ( EXT3_FEATURE_INCOMPAT_RECOVER \
> >>> -                                    | EXT4_FEATURE_INCOMPAT_MMP)
> >>> +                                    | EXT4_FEATURE_INCOMPAT_MMP     \
> >>> +                                    | EXT4_FEATURE_INCOMPAT_ENCRYPT)
> >>>
> >>
> >> And what happens when grub does see encrypted content? Returning
> >> garbage is not an option here.
> >>
> >>>
> >>>  #define EXT3_JOURNAL_MAGIC_NUMBER      0xc03b3998U
> >>>
> >>>
> >>> _______________________________________________
> >>> Bug-grub mailing list
> >>> Bug-grub@gnu.org
> >>> https://lists.gnu.org/mailman/listinfo/bug-grub
> >>>
> >>
> >
>
>

[-- Attachment #2: Type: text/html, Size: 7150 bytes --]