diff mbox series

[v3,1/5] efi: vars: prohibit reading random seed variables

Message ID 20221122020404.3476063-2-Jason@zx2c4.com (mailing list archive)
State Not Applicable
Delegated to: Herbert Xu
Headers show
Series Use EFI variables for random seed | expand

Commit Message

Jason A. Donenfeld Nov. 22, 2022, 2:04 a.m. UTC
In anticipation of putting random seeds in EFI variables, it's important
that the random GUID namespace of variables remains hidden from
userspace. We accomplish this by not populating efivarfs with entries
from that GUID, as well as denying the creation of new ones in that
GUID.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 fs/efivarfs/inode.c | 4 ++++
 fs/efivarfs/super.c | 3 +++
 2 files changed, 7 insertions(+)

Comments

Matthew Garrett Nov. 27, 2022, 9 p.m. UTC | #1
On Tue, Nov 22, 2022 at 03:04:00AM +0100, Jason A. Donenfeld wrote:
> In anticipation of putting random seeds in EFI variables, it's important
> that the random GUID namespace of variables remains hidden from
> userspace. We accomplish this by not populating efivarfs with entries
> from that GUID, as well as denying the creation of new ones in that
> GUID.

What's the concern here? Booting an older kernel would allow a malicious 
actor to either read the seed variable or set it to a value under their 
control, so we can't guarantee that the information is secret.
Jason A. Donenfeld Nov. 28, 2022, 1:10 a.m. UTC | #2
Hi,

On Sun, Nov 27, 2022 at 09:00:40PM +0000, Matthew Garrett wrote:
> On Tue, Nov 22, 2022 at 03:04:00AM +0100, Jason A. Donenfeld wrote:
> > In anticipation of putting random seeds in EFI variables, it's important
> > that the random GUID namespace of variables remains hidden from
> > userspace. We accomplish this by not populating efivarfs with entries
> > from that GUID, as well as denying the creation of new ones in that
> > GUID.
> 
> What's the concern here? Booting an older kernel would allow a malicious 
> actor to either read the seed variable or set it to a value under their 
> control, so we can't guarantee that the information is secret.

The security model is the same as that of random seed files, on, say,
BSD. If you remove the hard drive or change the operating system or what
have you, then sure, you can fiddle with the seed and read it. But the
running operating system shouldn't show it to you if it can help it.
Consider, for example, systemd's use of EFI variables for the
SystemToken. There, they have PID 1 take care of chmod'ing it before
other processes start.  But of course a different OS or even EFI shell
could just read it. So, think of this as just basic runtime safety --
like what people do when they set the umask before writing a random seed
file -- rather than some type of ultimate secrecy.

(And either way, the larger picture is that it's much more important to
get as much random data from as many sources as possible as soon as
possible, rather than being overly paranoid about every one single
source that we start excluding sources. A plethora of sources is better
off here.)

Jason
diff mbox series

Patch

diff --git a/fs/efivarfs/inode.c b/fs/efivarfs/inode.c
index 939e5e242b98..617f3ad2485e 100644
--- a/fs/efivarfs/inode.c
+++ b/fs/efivarfs/inode.c
@@ -91,6 +91,10 @@  static int efivarfs_create(struct user_namespace *mnt_userns, struct inode *dir,
 	err = guid_parse(dentry->d_name.name + namelen + 1, &var->var.VendorGuid);
 	if (err)
 		goto out;
+	if (guid_equal(&var->var.VendorGuid, &LINUX_EFI_RANDOM_SEED_TABLE_GUID)) {
+		err = -EPERM;
+		goto out;
+	}
 
 	if (efivar_variable_is_removable(var->var.VendorGuid,
 					 dentry->d_name.name, namelen))
diff --git a/fs/efivarfs/super.c b/fs/efivarfs/super.c
index 6780fc81cc11..07e82e246666 100644
--- a/fs/efivarfs/super.c
+++ b/fs/efivarfs/super.c
@@ -116,6 +116,9 @@  static int efivarfs_callback(efi_char16_t *name16, efi_guid_t vendor,
 	int err = -ENOMEM;
 	bool is_removable = false;
 
+	if (guid_equal(&vendor, &LINUX_EFI_RANDOM_SEED_TABLE_GUID))
+		return 0;
+
 	entry = kzalloc(sizeof(*entry), GFP_KERNEL);
 	if (!entry)
 		return err;