linux-kernel - Re: [PATCH RFC 0/4] Add firmware signature file check

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Date:	Tue, 06 Nov 2012 08:06:15 +0100
From:	Takashi Iwai <tiwai@...e.de>
To:	Li Joey <jlee@...ell.com>
Cc:	Ming Lei <tom.leiming@...il.com>,
	Matthew Garrett <mjg59@...f.ucam.org>,
	Alan Cox <alan@...rguk.ukuu.org.uk>, joeyli <jlee@...e.com>,
	Jiri Kosina <jkosina@...e.cz>,
	David Howells <dhowells@...hat.com>,
	Rusty Russell <rusty@...tcorp.com.au>,
	linux-kernel@...r.kernel.org,
	linux-security-module@...r.kernel.org, linux-efi@...r.kernel.org
Subject: Re: [PATCH RFC 0/4] Add firmware signature file check

At Tue, 6 Nov 2012 13:36:46 +0800,
Li Joey wrote:
> 
> [1  <text/plain; ISO-8859-1 (7bit)>]
> 2012/11/6 Ming Lei <tom.leiming@...il.com>
> 
> > On Tue, Nov 6, 2012 at 1:18 AM, Takashi Iwai <tiwai@...e.de> wrote:
> > >
> > > To be noted, it doesn't support the firmwares via udev but only the
> > > direct loading, and the check for built-in firmware is missing, too.
> >
> > Generally, both direct loading and udev may request one same firmware
> > image. And after check failed, current firmware load will fallback on udev
> > to complete loading, so looks a check-failed firmware still can be loaded
> > into kernel no matter if there is firmware signature check or not.
> >
> >
> > Thanks,
> > --
> > Ming Lei
> 
> 
> The udev direct write firmware through data attribute, maybe we can do the
> same signature verification in firmware_data_write? The following patch
> didn't test yet.

This would work in theory.  But in practice, when the direct file
loading fails and falls back to udev, it means that the firmware is no
file but generated somehow dynamically.  If so, a static signature
won't help, I'm afraid.


thanks,

Takashi

> 
> 
> Thanks
> Joey Lee
> 
> >From 035dde5fadc9e7f4b7811b18d3a5094ef88e8bbb Mon Sep 17 00:00:00 2001
> From: Lee, Chun-Yi <jlee@...e.com>
> Date: Tue, 6 Nov 2012 13:07:04 +0800
> Subject: [PATCH] firmware: Add signature check to firmware_data_write
> 
> ---
>  drivers/base/firmware_class.c |   18 ++++++++++++++++++
>  1 files changed, 18 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
> index 8945f4e..40d8cc6 100644
> --- a/drivers/base/firmware_class.c
> +++ b/drivers/base/firmware_class.c
> @@ -621,6 +621,7 @@ static ssize_t firmware_data_write(struct file *filp,
> struct kobject *kobj,
>         struct firmware_priv *fw_priv = to_firmware_priv(dev);
>         struct firmware_buf *buf;
>         ssize_t retval;
> +       bool success = false;
> 
>         if (!capable(CAP_SYS_RAWIO))
>                 return -EPERM;
> @@ -655,6 +656,23 @@ static ssize_t firmware_data_write(struct file *filp,
> struct kobject *kobj,
>         }
> 
>         buf->size = max_t(size_t, offset, buf->size);
> +
> +#ifdef CONFIG_FIRMWARE_SIG
> +       for (i = 0; i < ARRAY_SIZE(fw_path); i++) {
> +               snprintf(path, PATH_MAX, "%s/%s.sig", fw_path[i],
> buf->fw_id);
> +               if (verify_signature(buf, path))
> +                       success = true;
> +       }
> +       if (!success) {
> +               pr_err("Invalid signature file %s\n", path);
> +               if (sig_enforce) {
> +                       vfree(buf->data);
> +                       buf->data = NULL;
> +                       buf->size = 0;
> +               }
> +               retval = -ENOENT;
> +       }
> +#endif /* CONFIG_FIRMWARE_SIG */
>  out:
>         mutex_unlock(&fw_lock);
>         return retval;
> -- 
> 1.6.4.2
> [2  <text/html; ISO-8859-1 (quoted-printable)>]
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/