| Message ID | 20221116161642.1670235-6-Jason@zx2c4.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | Use EFI variables for random seed | expand |
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.
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, 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. Jason
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
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); /*
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(+)