[RFC,v1,5/6] efi: efivarfs: prohibit reading random seed variables

Variables in the random seed GUID must remain secret, so deny all reads
to them.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 fs/efivarfs/file.c | 3 +++
 1 file changed, 3 insertions(+)

On Wed, Nov 16, 2022 at 6:05 PM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Wed, 16 Nov 2022 at 17:17, Jason A. Donenfeld <Jason@zx2c4.com> wrote:
> >
> > Variables in the random seed GUID must remain secret, so deny all reads
> > to them.
> >
> > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> > ---
> >  fs/efivarfs/file.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/fs/efivarfs/file.c b/fs/efivarfs/file.c
> > index d57ee15874f9..08996ba3a373 100644
> > --- a/fs/efivarfs/file.c
> > +++ b/fs/efivarfs/file.c
> > @@ -76,6 +76,9 @@ static ssize_t efivarfs_file_read(struct file *file, char __user *userbuf,
> >         while (!__ratelimit(&file->f_cred->user->ratelimit))
> >                 msleep(50);
> >
> > +       if (guid_equal(&var->var.VendorGuid, &LINUX_EFI_RANDOM_SEED_TABLE_GUID))
> > +               return -EPERM;
> > +
> >         err = efivar_entry_size(var, &datasize);
> >
> >         /*
>
> I'd prefer it if we could just disregard them entirely, i.e., never
> enumerate them so that they don't appear in the file system.

Okay, sure, I can make that happen I think.

And then I suppose that if you try to create anything under that GUID,
it should just fail.

Jason

On Wed, 2022-11-16 at 18:04 +0100, Ard Biesheuvel wrote:
> On Wed, 16 Nov 2022 at 17:17, Jason A. Donenfeld <Jason@zx2c4.com>
> wrote:
> > 
> > Variables in the random seed GUID must remain secret, so deny all
> > reads
> > to them.
> > 
> > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> > ---
> >  fs/efivarfs/file.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/fs/efivarfs/file.c b/fs/efivarfs/file.c
> > index d57ee15874f9..08996ba3a373 100644
> > --- a/fs/efivarfs/file.c
> > +++ b/fs/efivarfs/file.c
> > @@ -76,6 +76,9 @@ static ssize_t efivarfs_file_read(struct file
> > *file, char __user *userbuf,
> >         while (!__ratelimit(&file->f_cred->user->ratelimit))
> >                 msleep(50);
> > 
> > +       if (guid_equal(&var->var.VendorGuid,
> > &LINUX_EFI_RANDOM_SEED_TABLE_GUID))
> > +               return -EPERM;
> > +
> >         err = efivar_entry_size(var, &datasize);
> > 
> >         /*
> 
> I'd prefer it if we could just disregard them entirely, i.e., never
> enumerate them so that they don't appear in the file system.

It would be nice if they could be boot services only ... then they
disappear naturally, but that would mean the rng would have to
initialize and save in the EFI stub before ExitBootServices, which
doesn't seem practical.

James

On Wed, 2022-11-16 at 21:08 +0100, Jason A. Donenfeld wrote:
> On Wed, Nov 16, 2022 at 8:42 PM James Bottomley
> <James.Bottomley@hansenpartnership.com> wrote:
> > It would be nice if they could be boot services only ... then they
> > disappear naturally, but that would mean the rng would have to
> > initialize and save in the EFI stub before ExitBootServices, which
> > doesn't seem practical.
> 
> That would be nice, but the whole idea is it gets updated by Linux's
> RNG, so that won't work. `boot|runtime` it is, then.

But then you can't use the only security mechanism we have in EFI
(keeping sensitive information in BS only variables which can only be
accessed by EFI signed entities).  If you can't take advantage of that
then there's no security point in placing the seed in EFI and you might
as well simply write it to a file.

Artificially trying to hide the variables from efivarfs has no real
security value either, as I think you can appreciate if you try the
thought experiment of trying to get a VFS modification to hide the
random seed file past Al ... I'll get the thought experiment popcorn.

James