Hi,
this is a patch series to add the support for firmware signature
check. At this time, the kernel checks extra signature file (*.sig)
for each firmware, instead of embedded signature.
It's just a quick hack using the existing module signing mechanism,
thus provided only as a proof of concept for now.
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.
Takashi
At Mon, 05 Nov 2012 18:18:24 +0100,
Takashi Iwai wrote:
>
> Hi,
>
> this is a patch series to add the support for firmware signature
> check. At this time, the kernel checks extra signature file (*.sig)
> for each firmware, instead of embedded signature.
> It's just a quick hack using the existing module signing mechanism,
> thus provided only as a proof of concept for now.
>
> 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.
On the second thought, checking the signature for builtin firmwares is
superfluous. And udev usage for firmware loading should be pretty
rare with 3.7 kernel. So, locking down the udev loading case when
sig_enforce = true should suffice in most cases, I guess.
Takashi
On Mon, Nov 5, 2012 at 12:18 PM, Takashi Iwai <[email protected]> wrote:
> Hi,
>
> this is a patch series to add the support for firmware signature
> check. At this time, the kernel checks extra signature file (*.sig)
> for each firmware, instead of embedded signature.
> It's just a quick hack using the existing module signing mechanism,
> thus provided only as a proof of concept for now.
>
> 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.
Just to make sure I'm reading this correctly, it will sign any of the
firwmare files installed directly from the kernel tree if the option is
set. So for the firmware in the linux-firmware tree we'd need to
either copy that into the kernel tree during build time, or duplicate the
signing bits in the linux-firmware tree itself. However if we do the
latter, we'd probably need to use the same keys as the per-build kernel
key which means copying keys (ew) or tell the kernel to include a
separate firmware key in the extra list.
I feel like I'm rambling a bit, but I'm trying to work out how signed
firmware would look from a distro perspective. A significant amount of
work has been done to decouple linux-firmware from the kernel tree and
if signed firmware is used it seems to couple them together one way or
another. At the moment, using generated per-build keys to come up with
the firmware signatures seems a bit suboptimal in that regard.
josh
Takashi Iwai <[email protected]> wrote:
> this is a patch series to add the support for firmware signature
> check. At this time, the kernel checks extra signature file (*.sig)
> for each firmware, instead of embedded signature.
> It's just a quick hack using the existing module signing mechanism,
> thus provided only as a proof of concept for now.
There is another way to do this. If you look at the patches I proposed to
wrap keys in PE binaries, you'll find that that can handle PKCS#7 format
messages as that's what's in the sort of signed PE binary we're dealing with.
You could use this to put the firmware inside a signed-data PKCS#7 message.
David
David Howells <[email protected]> wrote:
> Takashi Iwai <[email protected]> wrote:
>
> > this is a patch series to add the support for firmware signature
> > check. At this time, the kernel checks extra signature file (*.sig)
> > for each firmware, instead of embedded signature.
> > It's just a quick hack using the existing module signing mechanism,
> > thus provided only as a proof of concept for now.
>
> There is another way to do this. If you look at the patches I proposed to
> wrap keys in PE binaries, you'll find that that can handle PKCS#7 format
> messages as that's what's in the sort of signed PE binary we're dealing with.
See:
http://git.kernel.org/?p=linux/kernel/git/dhowells/linux-modsign.git;a=shortlog;h=refs/heads/devel-pekey
> You could use this to put the firmware inside a signed-data PKCS#7 message.
Note that the ASN.1 decoder in the kernel would need altering to handle
messages larger than 64KB.
David
On Tue, Nov 6, 2012 at 1:18 AM, Takashi Iwai <[email protected]> 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
2012/11/6 Ming Lei <[email protected]>:
> On Tue, Nov 6, 2012 at 1:18 AM, Takashi Iwai <[email protected]> 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
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
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.
Thanks
Joey Lee
>From 035dde5fadc9e7f4b7811b18d3a5094ef88e8bbb Mon Sep 17 00:00:00 2001
From: Lee, Chun-Yi <[email protected]>
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
At Mon, 5 Nov 2012 15:43:09 -0500,
Josh Boyer wrote:
>
> On Mon, Nov 5, 2012 at 12:18 PM, Takashi Iwai <[email protected]> wrote:
> > Hi,
> >
> > this is a patch series to add the support for firmware signature
> > check. At this time, the kernel checks extra signature file (*.sig)
> > for each firmware, instead of embedded signature.
> > It's just a quick hack using the existing module signing mechanism,
> > thus provided only as a proof of concept for now.
> >
> > 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.
>
> Just to make sure I'm reading this correctly, it will sign any of the
> firwmare files installed directly from the kernel tree if the option is
> set. So for the firmware in the linux-firmware tree we'd need to
> either copy that into the kernel tree during build time, or duplicate the
> signing bits in the linux-firmware tree itself. However if we do the
> latter, we'd probably need to use the same keys as the per-build kernel
> key which means copying keys (ew) or tell the kernel to include a
> separate firmware key in the extra list.
Yes, the situation is as same as the external module builds.
> I feel like I'm rambling a bit, but I'm trying to work out how signed
> firmware would look from a distro perspective. A significant amount of
> work has been done to decouple linux-firmware from the kernel tree and
> if signed firmware is used it seems to couple them together one way or
> another.
Well, the primary question is whether the firmware signature check is
required or not. Of course, these patches assume that it is for
secure boot lockdown :)
> At the moment, using generated per-build keys to come up with
> the firmware signatures seems a bit suboptimal in that regard.
But how would distro sign modules that are built externally?
It should be the pretty same situation.
I thought that the current module signing is already supported (at
least accepted) by distro, even for external modules. Isn't it?
thanks,
Takashi
At Tue, 06 Nov 2012 00:01:52 +0000,
David Howells wrote:
>
> Takashi Iwai <[email protected]> wrote:
>
> > this is a patch series to add the support for firmware signature
> > check. At this time, the kernel checks extra signature file (*.sig)
> > for each firmware, instead of embedded signature.
> > It's just a quick hack using the existing module signing mechanism,
> > thus provided only as a proof of concept for now.
>
> There is another way to do this. If you look at the patches I proposed to
> wrap keys in PE binaries, you'll find that that can handle PKCS#7 format
> messages as that's what's in the sort of signed PE binary we're dealing with.
>
> You could use this to put the firmware inside a signed-data PKCS#7 message.
Yeah, embedding the signature is more straightforward. Actually I
tried the embedded signature (just like module) at first, then a
couple of concerns arose:
- Legally unclear about "modifying" the firmware data or file,
- The signed firmware is no longer compatible with the older kernel,
thus bad for distro packaging.
thanks,
Takashi
At Tue, 6 Nov 2012 10:30:26 +0800,
Ming Lei wrote:
>
> On Tue, Nov 6, 2012 at 1:18 AM, Takashi Iwai <[email protected]> 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.
Yeah, it's just uncovered in the patch. As a easy solution, apply the
patch like below to disallow the udev fw loading when signature check
is enforced.
thanks,
Takashi
---
diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index 575bc4c..93121c3 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -912,6 +912,13 @@ static int _request_firmware_load(struct firmware_priv *fw_priv, bool uevent,
goto handle_fw;
}
+ /* signature check isn't handled via udev fw loading */
+ if (sig_enforce) {
+ fw_load_abort(fw_priv);
+ direct_load = 1;
+ goto handle_fw;
+ }
+
/* fall back on userspace loading */
buf->fmt = PAGE_BUF;
At Tue, 6 Nov 2012 13:36:46 +0800,
Li Joey wrote:
>
> [1 <text/plain; ISO-8859-1 (7bit)>]
> 2012/11/6 Ming Lei <[email protected]>
>
> > On Tue, Nov 6, 2012 at 1:18 AM, Takashi Iwai <[email protected]> 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 <[email protected]>
> 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)>]
>
On Tue, Nov 6, 2012 at 3:03 PM, Takashi Iwai <[email protected]> wrote:
>
> Yeah, it's just uncovered in the patch. As a easy solution, apply the
> patch like below to disallow the udev fw loading when signature check
> is enforced.
>
>
> thanks,
>
> Takashi
>
> ---
> diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
> index 575bc4c..93121c3 100644
> --- a/drivers/base/firmware_class.c
> +++ b/drivers/base/firmware_class.c
> @@ -912,6 +912,13 @@ static int _request_firmware_load(struct firmware_priv *fw_priv, bool uevent,
> goto handle_fw;
> }
>
> + /* signature check isn't handled via udev fw loading */
> + if (sig_enforce) {
> + fw_load_abort(fw_priv);
> + direct_load = 1;
> + goto handle_fw;
> + }
> +
The above might be wrong if the firmware file doesn't exist in default
search paths. You should skip loading from user space only if
verify_signature() returns false. And the udev loading should be
resorted to if there is no such firmware file in default search paths.
> /* fall back on userspace loading */
> buf->fmt = PAGE_BUF;
>
Thanks,
--
Ming Lei
On Tue, Nov 6, 2012 at 1:36 PM, Li Joey <[email protected]> wrote:
> 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.
> @@ -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;
> + }
When direct loading failed, it means that the firmware isn't
under the default search path, so the above verification
might return false always.
Thanks,
--
Ming Lei
At Tue, 6 Nov 2012 15:16:43 +0800,
Ming Lei wrote:
>
> On Tue, Nov 6, 2012 at 3:03 PM, Takashi Iwai <[email protected]> wrote:
> >
> > Yeah, it's just uncovered in the patch. As a easy solution, apply the
> > patch like below to disallow the udev fw loading when signature check
> > is enforced.
> >
> >
> > thanks,
> >
> > Takashi
> >
> > ---
> > diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
> > index 575bc4c..93121c3 100644
> > --- a/drivers/base/firmware_class.c
> > +++ b/drivers/base/firmware_class.c
> > @@ -912,6 +912,13 @@ static int _request_firmware_load(struct firmware_priv *fw_priv, bool uevent,
> > goto handle_fw;
> > }
> >
> > + /* signature check isn't handled via udev fw loading */
> > + if (sig_enforce) {
> > + fw_load_abort(fw_priv);
> > + direct_load = 1;
> > + goto handle_fw;
> > + }
> > +
>
> The above might be wrong if the firmware file doesn't exist in default
> search paths.
Heh, I didn't call it's a perfect patch. It's just an easy solution,
as mentioned.
> You should skip loading from user space only if
> verify_signature() returns false. And the udev loading should be
> resorted to if there is no such firmware file in default search paths.
... and the kernel should ask udev again for the corresponding
signature. I'm too lazy to implement that just for unknown corner
cases, so put the patch like above.
Honestly speaking, I have a feeling that we should rather go for
getting rid of udev fw loading. The fw loader code is overly complex
just for udev handshaking.
Do you know how many firmwares still rely on udev...?
thanks,
Takashi
On Tue, Nov 6, 2012 at 3:32 PM, Takashi Iwai <[email protected]> wrote:
> At Tue, 6 Nov 2012 15:16:43 +0800,
> Ming Lei wrote:
>>
>> On Tue, Nov 6, 2012 at 3:03 PM, Takashi Iwai <[email protected]> wrote:
>> >
>> > Yeah, it's just uncovered in the patch. As a easy solution, apply the
>> > patch like below to disallow the udev fw loading when signature check
>> > is enforced.
>> >
>> >
>> > thanks,
>> >
>> > Takashi
>> >
>> > ---
>> > diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
>> > index 575bc4c..93121c3 100644
>> > --- a/drivers/base/firmware_class.c
>> > +++ b/drivers/base/firmware_class.c
>> > @@ -912,6 +912,13 @@ static int _request_firmware_load(struct firmware_priv *fw_priv, bool uevent,
>> > goto handle_fw;
>> > }
>> >
>> > + /* signature check isn't handled via udev fw loading */
>> > + if (sig_enforce) {
>> > + fw_load_abort(fw_priv);
>> > + direct_load = 1;
>> > + goto handle_fw;
>> > + }
>> > +
>>
>> The above might be wrong if the firmware file doesn't exist in default
>> search paths.
>
> Heh, I didn't call it's a perfect patch. It's just an easy solution,
> as mentioned.
>
>> You should skip loading from user space only if
>> verify_signature() returns false. And the udev loading should be
>> resorted to if there is no such firmware file in default search paths.
>
> ... and the kernel should ask udev again for the corresponding
> signature.
I mean you can't skip user space loading if there is no firmware file
in the default search path. And you can do it if verify_signature()
returns false. So you needn't have to implement the signature for
user space loading.
> I'm too lazy to implement that just for unknown corner
> cases, so put the patch like above.
There might be some distributions in which the firmwares aren't stored
under the default search path, so your change may cause regression
on these distributions. And, it is a easy change in your patch to make
the situation working.
Also the default search path in firmware_class.c is from built-in path of
udev, and distributions may customize their firmware path by udev
configure option.
>
> Honestly speaking, I have a feeling that we should rather go for
> getting rid of udev fw loading. The fw loader code is overly complex
Yes, I have the feeling too, but we need to make sure no regressions
introduced.
> just for udev handshaking.
>
> Do you know how many firmwares still rely on udev...?
Do you know how many distributions have switched to 3.7-rcX to
start using direct loading?
Thanks,
--
Ming Lei
At Tue, 6 Nov 2012 16:04:50 +0800,
Ming Lei wrote:
>
> On Tue, Nov 6, 2012 at 3:32 PM, Takashi Iwai <[email protected]> wrote:
> > At Tue, 6 Nov 2012 15:16:43 +0800,
> > Ming Lei wrote:
> >>
> >> On Tue, Nov 6, 2012 at 3:03 PM, Takashi Iwai <[email protected]> wrote:
> >> >
> >> > Yeah, it's just uncovered in the patch. As a easy solution, apply the
> >> > patch like below to disallow the udev fw loading when signature check
> >> > is enforced.
> >> >
> >> >
> >> > thanks,
> >> >
> >> > Takashi
> >> >
> >> > ---
> >> > diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
> >> > index 575bc4c..93121c3 100644
> >> > --- a/drivers/base/firmware_class.c
> >> > +++ b/drivers/base/firmware_class.c
> >> > @@ -912,6 +912,13 @@ static int _request_firmware_load(struct firmware_priv *fw_priv, bool uevent,
> >> > goto handle_fw;
> >> > }
> >> >
> >> > + /* signature check isn't handled via udev fw loading */
> >> > + if (sig_enforce) {
> >> > + fw_load_abort(fw_priv);
> >> > + direct_load = 1;
> >> > + goto handle_fw;
> >> > + }
> >> > +
> >>
> >> The above might be wrong if the firmware file doesn't exist in default
> >> search paths.
> >
> > Heh, I didn't call it's a perfect patch. It's just an easy solution,
> > as mentioned.
> >
> >> You should skip loading from user space only if
> >> verify_signature() returns false. And the udev loading should be
> >> resorted to if there is no such firmware file in default search paths.
> >
> > ... and the kernel should ask udev again for the corresponding
> > signature.
>
> I mean you can't skip user space loading if there is no firmware file
> in the default search path. And you can do it if verify_signature()
> returns false. So you needn't have to implement the signature for
> user space loading.
Right, and it's intentionally dropped so. For the non-default fw
path, it can be added via proc dynamically or via kconfig statically.
If the firmware is generated via udev, then it doesn't make sense to
check a static signature file.
> > I'm too lazy to implement that just for unknown corner
> > cases, so put the patch like above.
>
> There might be some distributions in which the firmwares aren't stored
> under the default search path, so your change may cause regression
> on these distributions. And, it is a easy change in your patch to make
> the situation working.
A "regression" can't happen in this case because the secure boot is
a completely new stuff :) For normal boot, sig_enforce is false, so
no behavior change here (well my patch still applies the signature
check for direct fw loading, but it won't regress at least).
> Also the default search path in firmware_class.c is from built-in path of
> udev, and distributions may customize their firmware path by udev
> configure option.
Well, the default paths in kernel can be changed to follow that as
well, no?
> > Honestly speaking, I have a feeling that we should rather go for
> > getting rid of udev fw loading. The fw loader code is overly complex
>
> Yes, I have the feeling too, but we need to make sure no regressions
> introduced.
Right. And I guess the exceptional firmware case is better found by
checking udev. But it's a bit off topic from secure boot.
> > just for udev handshaking.
> >
> > Do you know how many firmwares still rely on udev...?
>
> Do you know how many distributions have switched to 3.7-rcX to
> start using direct loading?
Obviously no distro releases using 3.7-rc since it's still rc.
But what's your point?
Takashi
> But how would distro sign modules that are built externally?
> It should be the pretty same situation.
I would start with the "would" and lawyers and liability and then stop
worrying about the how. Absent someone actually intending to do it and
saying so.
Alan
On Tue, Nov 6, 2012 at 4:18 PM, Takashi Iwai <[email protected]> wrote:
>
> Right, and it's intentionally dropped so. For the non-default fw
> path, it can be added via proc dynamically or via kconfig statically.
> If the firmware is generated via udev, then it doesn't make sense to
> check a static signature file.
kconfig should be better, and proc isn't a good way because it
is a bit late. Also the firmware might be loaded dynamically from other
where(network, ...). So it is better to fall back on user space if the
firmware file isn't found by direct loading even firmware signature
is enabled.
> A "regression" can't happen in this case because the secure boot is
> a completely new stuff :) For normal boot, sig_enforce is false, so
> no behavior change here (well my patch still applies the signature
> check for direct fw loading, but it won't regress at least).
Got it, so FIRMWARE_SIG and MODULE_SIG should be enabled only
for secure boot.
The regression might still be triggered if falling back on user space is not
supported, see above.
>
>> Also the default search path in firmware_class.c is from built-in path of
>> udev, and distributions may customize their firmware path by udev
>> configure option.
>
> Well, the default paths in kernel can be changed to follow that as
> well, no?
>
>> > Honestly speaking, I have a feeling that we should rather go for
>> > getting rid of udev fw loading. The fw loader code is overly complex
>>
>> Yes, I have the feeling too, but we need to make sure no regressions
>> introduced.
>
> Right. And I guess the exceptional firmware case is better found by
> checking udev. But it's a bit off topic from secure boot.
Not sure, some distributions may not use udev at all. Some application
might need the firmware add/remove event. Some may not store the
firmware in fs.
So now it is difficult to say we can remove firmware loading from user
space. Better to just keep it.
> Obviously no distro releases using 3.7-rc since it's still rc.
> But what's your point?
I mean direct loading hasn't been tested completely, so we don't
know which distributions may fallback on user space loading.
Thanks,
--
Ming Lei
At Tue, 6 Nov 2012 09:20:19 +0000,
Alan Cox wrote:
>
> > But how would distro sign modules that are built externally?
> > It should be the pretty same situation.
>
> I would start with the "would" and lawyers and liability and then stop
> worrying about the how. Absent someone actually intending to do it and
> saying so.
Well, what I meant for external built modules are not about things
like nvidia, but rather normal (legal) drivers or updated modules that
are built from out-of-kernel source. I'm sure that distros will
provide such update module packages on the secure boot system, too.
So, discussing about "how" isn't so bad even for now, since this shall
be anyway mandatory (for distros) once when the module signing is
deployed.
Takashi
At Tue, 6 Nov 2012 18:04:36 +0800,
Ming Lei wrote:
>
> On Tue, Nov 6, 2012 at 4:18 PM, Takashi Iwai <[email protected]> wrote:
> >
> > Right, and it's intentionally dropped so. For the non-default fw
> > path, it can be added via proc dynamically or via kconfig statically.
> > If the firmware is generated via udev, then it doesn't make sense to
> > check a static signature file.
>
> kconfig should be better, and proc isn't a good way because it
> is a bit late. Also the firmware might be loaded dynamically from other
> where(network, ...). So it is better to fall back on user space if the
> firmware file isn't found by direct loading even firmware signature
> is enabled.
It will even with my patch, when enforce_sig is false.
> > A "regression" can't happen in this case because the secure boot is
> > a completely new stuff :) For normal boot, sig_enforce is false, so
> > no behavior change here (well my patch still applies the signature
> > check for direct fw loading, but it won't regress at least).
>
> Got it, so FIRMWARE_SIG and MODULE_SIG should be enabled only
> for secure boot.
The kernels with these kconfigs set can run on normal systems. In
non-secure boot mode, however, sig_enable option are off, thus the
fallback is still applied.
> The regression might still be triggered if falling back on user space is not
> supported, see above.
>
> >
> >> Also the default search path in firmware_class.c is from built-in path of
> >> udev, and distributions may customize their firmware path by udev
> >> configure option.
> >
> > Well, the default paths in kernel can be changed to follow that as
> > well, no?
> >
> >> > Honestly speaking, I have a feeling that we should rather go for
> >> > getting rid of udev fw loading. The fw loader code is overly complex
> >>
> >> Yes, I have the feeling too, but we need to make sure no regressions
> >> introduced.
> >
> > Right. And I guess the exceptional firmware case is better found by
> > checking udev. But it's a bit off topic from secure boot.
>
> Not sure, some distributions may not use udev at all.
Hmm, I can't imagine a system without udev but still supporting the
firmware loading with user-space interaction...
> Some application
> might need the firmware add/remove event.
Then this is already broken with the direct fw loading on 3.7, no?
> Some may not store the
> firmware in fs.
And these won't satisfy the firmware signing, so we don't need to care
too much.
> So now it is difficult to say we can remove firmware loading from user
> space. Better to just keep it.
Yeah, I don't mean to drop it now. But I meant to go for dropping
it. For example, put a deprecated flag and give a warning for udev fw
loading path so that user notices something to be fixed.
> > Obviously no distro releases using 3.7-rc since it's still rc.
> > But what's your point?
>
> I mean direct loading hasn't been tested completely, so we don't
> know which distributions may fallback on user space loading.
The transition to direct fw loading is seamless, so I don't think
you can see which drivers use udev fw loading from the results of
distros... It might reveal some potential issues of direct fw loading
(like udev-trigger dependency), though.
Takashi
On Tue, Nov 6, 2012 at 6:17 PM, Takashi Iwai <[email protected]> wrote:
> At Tue, 6 Nov 2012 18:04:36 +0800,
> Ming Lei wrote:
>>
>> On Tue, Nov 6, 2012 at 4:18 PM, Takashi Iwai <[email protected]> wrote:
>> >
>> > Right, and it's intentionally dropped so. For the non-default fw
>> > path, it can be added via proc dynamically or via kconfig statically.
>> > If the firmware is generated via udev, then it doesn't make sense to
>> > check a static signature file.
>>
>> kconfig should be better, and proc isn't a good way because it
>> is a bit late. Also the firmware might be loaded dynamically from other
>> where(network, ...). So it is better to fall back on user space if the
>> firmware file isn't found by direct loading even firmware signature
>> is enabled.
>
> It will even with my patch, when enforce_sig is false.
It is true if all firmwares are signed on safe boot. If firmware is allowed
to be loaded from network or other non-fs place in secure distribution,
your patch will break this loading.
>
>> > A "regression" can't happen in this case because the secure boot is
>> > a completely new stuff :) For normal boot, sig_enforce is false, so
>> > no behavior change here (well my patch still applies the signature
>> > check for direct fw loading, but it won't regress at least).
>>
>> Got it, so FIRMWARE_SIG and MODULE_SIG should be enabled only
>> for secure boot.
>
> The kernels with these kconfigs set can run on normal systems. In
> non-secure boot mode, however, sig_enable option are off, thus the
> fallback is still applied.
>
>> The regression might still be triggered if falling back on user space is not
>> supported, see above.
>>
>> >
>> >> Also the default search path in firmware_class.c is from built-in path of
>> >> udev, and distributions may customize their firmware path by udev
>> >> configure option.
>> >
>> > Well, the default paths in kernel can be changed to follow that as
>> > well, no?
>> >
>> >> > Honestly speaking, I have a feeling that we should rather go for
>> >> > getting rid of udev fw loading. The fw loader code is overly complex
>> >>
>> >> Yes, I have the feeling too, but we need to make sure no regressions
>> >> introduced.
>> >
>> > Right. And I guess the exceptional firmware case is better found by
>> > checking udev. But it's a bit off topic from secure boot.
>>
>> Not sure, some distributions may not use udev at all.
>
> Hmm, I can't imagine a system without udev but still supporting the
> firmware loading with user-space interaction...
It is not so difficult, :-)
Some embedded systems use mdev in busybox, and some can
just parse the firmware event and run the below script:
Documentation/firmware_class/hotplug-script
on firmware ADD event. I remembered that android loading is
very simple.
>
>> Some application
>> might need the firmware add/remove event.
>
> Then this is already broken with the direct fw loading on 3.7, no?
Maybe, the direct loading hasn't been tested widely, and depends on
user space.
>> Some may not store the
>> firmware in fs.
>
> And these won't satisfy the firmware signing, so we don't need to care
> too much.
>
>> So now it is difficult to say we can remove firmware loading from user
>> space. Better to just keep it.
>
> Yeah, I don't mean to drop it now. But I meant to go for dropping
> it. For example, put a deprecated flag and give a warning for udev fw
> loading path so that user notices something to be fixed.
Maybe we can do it until direct loading has been tested for some time.
>
>> > Obviously no distro releases using 3.7-rc since it's still rc.
>> > But what's your point?
>>
>> I mean direct loading hasn't been tested completely, so we don't
>> know which distributions may fallback on user space loading.
>
> The transition to direct fw loading is seamless, so I don't think
> you can see which drivers use udev fw loading from the results of
> distros... It might reveal some potential issues of direct fw loading
> (like udev-trigger dependency), though.
The clue can be found from debug message.
Thanks,
--
Ming Lei
At Tue, 6 Nov 2012 18:40:57 +0800,
Ming Lei wrote:
>
> On Tue, Nov 6, 2012 at 6:17 PM, Takashi Iwai <[email protected]> wrote:
> > At Tue, 6 Nov 2012 18:04:36 +0800,
> > Ming Lei wrote:
> >>
> >> On Tue, Nov 6, 2012 at 4:18 PM, Takashi Iwai <[email protected]> wrote:
> >> >
> >> > Right, and it's intentionally dropped so. For the non-default fw
> >> > path, it can be added via proc dynamically or via kconfig statically.
> >> > If the firmware is generated via udev, then it doesn't make sense to
> >> > check a static signature file.
> >>
> >> kconfig should be better, and proc isn't a good way because it
> >> is a bit late. Also the firmware might be loaded dynamically from other
> >> where(network, ...). So it is better to fall back on user space if the
> >> firmware file isn't found by direct loading even firmware signature
> >> is enabled.
> >
> > It will even with my patch, when enforce_sig is false.
>
> It is true if all firmwares are signed on safe boot. If firmware is allowed
> to be loaded from network or other non-fs place in secure distribution,
> your patch will break this loading.
Do we already have such a secure mechanism? How is the security
assured?
> >> > A "regression" can't happen in this case because the secure boot is
> >> > a completely new stuff :) For normal boot, sig_enforce is false, so
> >> > no behavior change here (well my patch still applies the signature
> >> > check for direct fw loading, but it won't regress at least).
> >>
> >> Got it, so FIRMWARE_SIG and MODULE_SIG should be enabled only
> >> for secure boot.
> >
> > The kernels with these kconfigs set can run on normal systems. In
> > non-secure boot mode, however, sig_enable option are off, thus the
> > fallback is still applied.
> >
> >> The regression might still be triggered if falling back on user space is not
> >> supported, see above.
> >>
> >> >
> >> >> Also the default search path in firmware_class.c is from built-in path of
> >> >> udev, and distributions may customize their firmware path by udev
> >> >> configure option.
> >> >
> >> > Well, the default paths in kernel can be changed to follow that as
> >> > well, no?
> >> >
> >> >> > Honestly speaking, I have a feeling that we should rather go for
> >> >> > getting rid of udev fw loading. The fw loader code is overly complex
> >> >>
> >> >> Yes, I have the feeling too, but we need to make sure no regressions
> >> >> introduced.
> >> >
> >> > Right. And I guess the exceptional firmware case is better found by
> >> > checking udev. But it's a bit off topic from secure boot.
> >>
> >> Not sure, some distributions may not use udev at all.
> >
> > Hmm, I can't imagine a system without udev but still supporting the
> > firmware loading with user-space interaction...
>
> It is not so difficult, :-)
>
> Some embedded systems use mdev in busybox, and some can
> just parse the firmware event and run the below script:
>
> Documentation/firmware_class/hotplug-script
>
> on firmware ADD event. I remembered that android loading is
> very simple.
But I don't think Android devices will run on secure boot :)
That is, the whole signing madness is just for allowing boot on
upcoming machines that need the secure boot mode forced by Microsoft.
And this doesn't match with systems you suggested in the above.
> >> Some application
> >> might need the firmware add/remove event.
> >
> > Then this is already broken with the direct fw loading on 3.7, no?
>
> Maybe, the direct loading hasn't been tested widely, and depends on
> user space.
>
> >> Some may not store the
> >> firmware in fs.
> >
> > And these won't satisfy the firmware signing, so we don't need to care
> > too much.
> >
> >> So now it is difficult to say we can remove firmware loading from user
> >> space. Better to just keep it.
> >
> > Yeah, I don't mean to drop it now. But I meant to go for dropping
> > it. For example, put a deprecated flag and give a warning for udev fw
> > loading path so that user notices something to be fixed.
>
> Maybe we can do it until direct loading has been tested for some time.
Yeah, it's a future plan. But I'd say it's better clarified that we
should go for that direction instead of keeping the stuff forever.
> >> > Obviously no distro releases using 3.7-rc since it's still rc.
> >> > But what's your point?
> >>
> >> I mean direct loading hasn't been tested completely, so we don't
> >> know which distributions may fallback on user space loading.
> >
> > The transition to direct fw loading is seamless, so I don't think
> > you can see which drivers use udev fw loading from the results of
> > distros... It might reveal some potential issues of direct fw loading
> > (like udev-trigger dependency), though.
>
> The clue can be found from debug message.
Debug messages are turned off on normal machines, unfortunately.
Takashi
On Tue, Nov 6, 2012 at 6:53 PM, Takashi Iwai <[email protected]> wrote:
> At Tue, 6 Nov 2012 18:40:57 +0800,
>> It is true if all firmwares are signed on safe boot. If firmware is allowed
>> to be loaded from network or other non-fs place in secure distribution,
>> your patch will break this loading.
>
> Do we already have such a secure mechanism? How is the security
> assured?
I don't know, and my comments are just on your patch and the condition.
I understand secure guys should know if the condition may be true or false, :-)
>> The clue can be found from debug message.
>
> Debug messages are turned off on normal machines, unfortunately.
Kernel guys will put one eye on bug report, also enabling udev log
can help the problem too.
Thanks,
--
Ming Lei
> > It is true if all firmwares are signed on safe boot. If firmware is allowed
> > to be loaded from network or other non-fs place in secure distribution,
> > your patch will break this loading.
Actually it's not. It should be true that firmware that can harm machine
integrity and is loaded by the OS is signed at some level. However it is
not true that
- firmware that is no integrity threat (eg USB firmware)
- firmware that can be flash updated on another PC and not observed by
the target
are necessarily in any way signed or secure.
> Do we already have such a secure mechanism? How is the security
> assured?
Another thing to consider is that a lot of hardware (particularly
anything aimed at such 'secure boot' machines) is already digitally
signed. Whether you need to enforce external signing is a mix of driver
specific questions ("does this device have signed firmware anyway", "can
bogus firmware do anything interesting") and local policy "do I as admin
want to block any firmware that isn't corporate site approved"
For USB this is quite important because there is a ton of hardware out
there which is intended to have firmware dumped into it for hacking and
fun purposes and should generally be totally outside of the signing
stuff.
Alan
Add a feature to check the firmware signature, specified via Kconfig
CONFIG_FIRMWARE_SIG.
The signature check is performed only for the direct fw loading
without udev. If sig_enforce is set but no firmware file is found in
fs, request_firmware*() returns an error for now. It would be
possible to improve this situation, e.g. by adding an extra request of
signature via yet another uevent, but I'm too lazy to implement it and
also skeptical whether it's needed.
On a kernel with CONFIG_FIRMWARE_SIG=y and sig_enforce=1 set, when no
firmware signature is present or the signature doesn't match, the
kernel rejects such a firmware and proceeds to the next possible one.
With sig_enforce=0, a firmware is loaded even if no signature is found
or the signature doesn't match, but it taints the kernel with
TAINT_USER. This behavior is similar like the signed module loading.
Last to be noted, in this version, the firmware signature support
depends on CONFIG_MODULE_SIG, that is, the system requires the module
support for now.
Signed-off-by: Takashi Iwai <[email protected]>
---
drivers/base/Kconfig | 6 ++++
drivers/base/firmware_class.c | 78 +++++++++++++++++++++++++++++++++++++++----
include/linux/firmware.h | 7 ++++
kernel/module_signing.c | 63 ++++++++++++++++++++++++++++++++++
4 files changed, 147 insertions(+), 7 deletions(-)
diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
index b34b5cd..3696fd7 100644
--- a/drivers/base/Kconfig
+++ b/drivers/base/Kconfig
@@ -145,6 +145,12 @@ config EXTRA_FIRMWARE_DIR
this option you can point it elsewhere, such as /lib/firmware/ or
some other directory containing the firmware files.
+config FIRMWARE_SIG
+ bool "Firmware signature verification"
+ depends on FW_LOADER && MODULE_SIG
+ help
+ Enable firmware signature check.
+
config DEBUG_DRIVER
bool "Driver Core verbose debug messages"
depends on DEBUG_KERNEL
diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index 8945f4e..501cff4 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -36,6 +36,11 @@ MODULE_AUTHOR("Manuel Estrada Sainz");
MODULE_DESCRIPTION("Multi purpose firmware loading support");
MODULE_LICENSE("GPL");
+#ifdef CONFIG_FIRMWARE_SIG
+static bool sig_enforce;
+module_param(sig_enforce, bool, 0644);
+#endif
+
/* Builtin firmware support */
#ifdef CONFIG_FW_LOADER
@@ -287,7 +292,7 @@ static noinline long fw_file_size(struct file *file)
return st.size;
}
-static bool fw_read_file_contents(struct file *file, struct firmware_buf *fw_buf)
+static bool fw_read_file_contents(struct file *file, void **bufp, size_t *sizep)
{
long size;
char *buf;
@@ -302,14 +307,42 @@ static bool fw_read_file_contents(struct file *file, struct firmware_buf *fw_buf
vfree(buf);
return false;
}
- fw_buf->data = buf;
- fw_buf->size = size;
+ *bufp = buf;
+ *sizep = size;
return true;
}
+#ifdef CONFIG_FIRMWARE_SIG
+static int verify_sig_file(struct firmware_buf *buf, const char *path)
+{
+ const unsigned long markerlen = sizeof(FIRMWARE_SIG_STRING) - 1;
+ struct file *file;
+ void *sig_data;
+ size_t sig_size;
+ int ret;
+
+ file = filp_open(path, O_RDONLY, 0);
+ if (IS_ERR(file))
+ return -ENOKEY;
+
+ ret = fw_read_file_contents(file, &sig_data, &sig_size);
+ fput(file);
+ if (!ret)
+ return -ENOKEY;
+ if (sig_size <= markerlen ||
+ memcmp(sig_data, FIRMWARE_SIG_STRING, markerlen))
+ return -EBADMSG;
+ ret = fw_verify_sig(buf->data, buf->size,
+ sig_data + markerlen, sig_size - markerlen);
+ pr_debug("verified signature %s: %d\n", path, ret);
+ vfree(sig_data);
+ return ret;
+}
+#endif /* CONFIG_FIRMWARE_SIG */
+
static bool fw_get_filesystem_firmware(struct firmware_buf *buf)
{
- int i;
+ int i, ret;
bool success = false;
char *path = __getname();
@@ -320,10 +353,30 @@ static bool fw_get_filesystem_firmware(struct firmware_buf *buf)
file = filp_open(path, O_RDONLY, 0);
if (IS_ERR(file))
continue;
- success = fw_read_file_contents(file, buf);
+ success = fw_read_file_contents(file, &buf->data, &buf->size);
fput(file);
- if (success)
- break;
+ if (!success)
+ continue;
+#ifdef CONFIG_FIRMWARE_SIG
+ snprintf(path, PATH_MAX, "%s/%s.sig", fw_path[i], buf->fw_id);
+ ret = verify_sig_file(buf, path);
+ if (ret < 0) {
+ if (ret == -ENOENT)
+ pr_err("Cannot find firmware signature %s\n",
+ path);
+ else
+ pr_err("Invalid firmware signature %s\n", path);
+ if (sig_enforce) {
+ vfree(buf->data);
+ buf->data = NULL;
+ buf->size = 0;
+ success = false;
+ continue;
+ }
+ add_taint(TAINT_USER);
+ }
+#endif /* CONFIG_FIRMWARE_SIG */
+ break;
}
__putname(path);
return success;
@@ -864,6 +917,17 @@ static int _request_firmware_load(struct firmware_priv *fw_priv, bool uevent,
goto handle_fw;
}
+#ifdef CONFIG_FIRMWARE_SIG
+ /* FIXME: we don't handle signature check for fw loaded via udev */
+ if (sig_enforce) {
+ pr_err("Cannot find firmware file %s; aborting fw loading\n",
+ buf->fw_id);
+ fw_load_abort(fw_priv);
+ direct_load = 1;
+ goto handle_fw;
+ }
+#endif
+
/* fall back on userspace loading */
buf->fmt = PAGE_BUF;
diff --git a/include/linux/firmware.h b/include/linux/firmware.h
index e4279fe..2e9e457 100644
--- a/include/linux/firmware.h
+++ b/include/linux/firmware.h
@@ -79,4 +79,11 @@ static inline int uncache_firmware(const char *name)
}
#endif
+#ifdef CONFIG_FIRMWARE_SIG
+#define FIRMWARE_SIG_STRING "~Linux firmware signature~\n"
+/* defined in kernel/module_signing.c */
+int fw_verify_sig(const void *fw_data, size_t fw_size,
+ const void *sig_data, size_t sig_size);
+#endif
+
#endif
diff --git a/kernel/module_signing.c b/kernel/module_signing.c
index ea1b1df..7994452 100644
--- a/kernel/module_signing.c
+++ b/kernel/module_signing.c
@@ -11,6 +11,7 @@
#include <linux/kernel.h>
#include <linux/err.h>
+#include <linux/export.h>
#include <crypto/public_key.h>
#include <crypto/hash.h>
#include <keys/asymmetric-type.h>
@@ -247,3 +248,65 @@ error_put_key:
pr_devel("<==%s() = %d\n", __func__, ret);
return ret;
}
+
+#ifdef CONFIG_FIRMWARE_SIG
+/*
+ * Verify the firmware signature, similar like module signature check
+ * but it's stored in a separate file
+ */
+int fw_verify_sig(const void *fw_data, size_t fw_size,
+ const void *sig_data, size_t sig_size)
+{
+ struct public_key_signature *pks;
+ struct module_signature ms;
+ struct key *key;
+ size_t sig_len;
+ int ret;
+
+ if (sig_size <= sizeof(ms))
+ return -EBADMSG;
+
+ memcpy(&ms, sig_data, sizeof(ms));
+ sig_data += sizeof(ms);
+ sig_size -= sizeof(ms);
+
+ sig_len = be32_to_cpu(ms.sig_len);
+ if (sig_size < sig_len + (size_t)ms.signer_len + ms.key_id_len)
+ return -EBADMSG;
+
+ /* For the moment, only support RSA and X.509 identifiers */
+ if (ms.algo != PKEY_ALGO_RSA ||
+ ms.id_type != PKEY_ID_X509)
+ return -ENOPKG;
+
+ if (ms.hash >= PKEY_HASH__LAST ||
+ !pkey_hash_algo[ms.hash])
+ return -ENOPKG;
+
+ key = request_asymmetric_key(sig_data, ms.signer_len,
+ sig_data + ms.signer_len, ms.key_id_len);
+ if (IS_ERR(key))
+ return PTR_ERR(key);
+
+ pks = mod_make_digest(ms.hash, fw_data, fw_size);
+ if (IS_ERR(pks)) {
+ ret = PTR_ERR(pks);
+ goto error_put_key;
+ }
+
+ sig_data += ms.signer_len + ms.key_id_len;
+ ret = mod_extract_mpi_array(pks, sig_data, sig_len);
+ if (ret < 0)
+ goto error_free_pks;
+
+ ret = verify_signature(key, pks);
+
+error_free_pks:
+ mpi_free(pks->rsa.s);
+ kfree(pks);
+error_put_key:
+ key_put(key);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(fw_verify_sig);
+#endif /* CONFIG_FIRMWARE_SIG */
--
1.8.0
Hi,
this is the revised patches I sent in this week for adding the
firmware signing support. No big changes in the code but a bit of
clean ups and more descriptions in changelog and comments now.
At this point, it still needs to have a proper Kconfig help text, and
move the stuff of CONFIG_MODULE_SIG to be indepdent from CONFIG_MODULE
so that the code required for signature checks can be built for the
firmware loader even without module.
But, before doing that, I'd like to hear whether this approach is
really a way to go, or better to throw away and scratch.
Any suggests / comments appreciated.
thanks,
Takashi
Add -f option to sign-file script for generating a firmware signature
file.
A firmware signature file contains a pretty similar structure like a
signed module but in a different order (because it's a separate file
while the module signature is embedded at the tail of unsigned module
contents). The file consists of
- the magic string
- the signature information, which is identical with the module
signature
- signer's name
- key id
- signature bytes
Signed-off-by: Takashi Iwai <[email protected]>
---
scripts/sign-file | 48 +++++++++++++++++++++++++++++++++++-------------
1 file changed, 35 insertions(+), 13 deletions(-)
diff --git a/scripts/sign-file b/scripts/sign-file
index 87ca59d..5b9d44d 100755
--- a/scripts/sign-file
+++ b/scripts/sign-file
@@ -4,30 +4,40 @@
#
# Format:
#
-# ./scripts/sign-file [-v] <key> <x509> <module> [<dest>]
+# ./scripts/sign-file [-v] [-f] <key> <x509> <module> [<dest>]
#
#
use strict;
use FileHandle;
use IPC::Open2;
+use Getopt::Long;
-my $verbose = 0;
-if ($#ARGV >= 0 && $ARGV[0] eq "-v") {
- $verbose = 1;
- shift;
+sub usage()
+{
+ print "Format: ./scripts/sign-file [options] <key> <x509> <module> [<dest>]
+ -v verbose output
+ -f create a firmware signature file
+";
+ exit;
}
-die "Format: ./scripts/sign-file [-v] <key> <x509> <module> [<dest>]\n"
- if ($#ARGV != 2 && $#ARGV != 3);
+my $verbose = 0;
+my $sign_fw = 0;
+
+GetOptions(
+ 'v|verbose' => \$verbose,
+ 'f|firmware' => \$sign_fw) || usage();
+usage() if ($#ARGV != 2 && $#ARGV != 3);
my $private_key = $ARGV[0];
my $x509 = $ARGV[1];
my $module = $ARGV[2];
-my $dest = ($#ARGV == 3) ? $ARGV[3] : $ARGV[2] . "~";
+my $dest = $ARGV[3] ? $ARGV[3] : $ARGV[2] . ($sign_fw ? ".sig" : "~");
+my $mode_name = $sign_fw ? "firmware" : "module";
die "Can't read private key\n" unless (-r $private_key);
die "Can't read X.509 certificate\n" unless (-r $x509);
-die "Can't read module\n" unless (-r $module);
+die "Can't read $mode_name\n" unless (-r $module);
#
# Read the kernel configuration
@@ -393,7 +403,9 @@ die "openssl rsautl died: $?" if ($? >> 8);
#
my $unsigned_module = read_file($module);
-my $magic_number = "~Module signature appended~\n";
+my $magic_number = $sign_fw ?
+ "~Linux firmware signature~\n" :
+ "~Module signature appended~\n";
my $info = pack("CCCCCxxxN",
$algo, $hash, $id_type,
@@ -402,7 +414,7 @@ my $info = pack("CCCCCxxxN",
length($signature));
if ($verbose) {
- print "Size of unsigned module: ", length($unsigned_module), "\n";
+ print "Size of unsigned $mode_name: ", length($unsigned_module), "\n";
print "Size of signer's name : ", length($signers_name), "\n";
print "Size of key identifier : ", length($key_identifier), "\n";
print "Size of signature : ", length($signature), "\n";
@@ -414,7 +426,16 @@ if ($verbose) {
open(FD, ">$dest") || die $dest;
binmode FD;
-print FD
+if ($sign_fw) {
+ print FD
+ $magic_number,
+ $info,
+ $signers_name,
+ $key_identifier,
+ $signature
+ ;
+} else {
+ print FD
$unsigned_module,
$signers_name,
$key_identifier,
@@ -422,8 +443,9 @@ print FD
$info,
$magic_number
;
+}
close FD || die $dest;
-if ($#ARGV != 3) {
+if (!$sign_fw && $#ARGV != 3) {
rename($dest, $module) || die $module;
}
--
1.8.0
Add a new option -a to sign-file for specifying the hash algorithm
to sign a file, to make it working without .config file.
This will be useful signing external module or firmware files.
Signed-off-by: Takashi Iwai <[email protected]>
---
scripts/sign-file | 40 ++++++++++++++++++++++++++++------------
1 file changed, 28 insertions(+), 12 deletions(-)
diff --git a/scripts/sign-file b/scripts/sign-file
index 5b9d44d..581cdcd 100755
--- a/scripts/sign-file
+++ b/scripts/sign-file
@@ -4,7 +4,7 @@
#
# Format:
#
-# ./scripts/sign-file [-v] [-f] <key> <x509> <module> [<dest>]
+# ./scripts/sign-file [-v] [-f] [-a algo] <key> <x509> <module> [<dest>]
#
#
use strict;
@@ -17,16 +17,19 @@ sub usage()
print "Format: ./scripts/sign-file [options] <key> <x509> <module> [<dest>]
-v verbose output
-f create a firmware signature file
+ -a algo specify hash algorithm
";
exit;
}
my $verbose = 0;
+my $hashalgo = "";
my $sign_fw = 0;
GetOptions(
'v|verbose' => \$verbose,
- 'f|firmware' => \$sign_fw) || usage();
+ 'f|firmware' => \$sign_fw,
+ 'a|algo=s' => \$hashalgo) || usage();
usage() if ($#ARGV != 2 && $#ARGV != 3);
my $private_key = $ARGV[0];
@@ -42,10 +45,7 @@ die "Can't read $mode_name\n" unless (-r $module);
#
# Read the kernel configuration
#
-my %config = (
- CONFIG_MODULE_SIG_SHA512 => 1
- );
-
+my %config;
if (-r ".config") {
open(FD, "<.config") || die ".config";
while (<FD>) {
@@ -56,6 +56,22 @@ if (-r ".config") {
close(FD);
}
+if ($hashalgo eq "") {
+ if (exists $config{"CONFIG_MODULE_SIG_SHA1"}) {
+ $hashalgo="sha1";
+ } elsif (exists $config{"CONFIG_MODULE_SIG_SHA224"}) {
+ $hashalgo="sha224";
+ } elsif (exists $config{"CONFIG_MODULE_SIG_SHA256"}) {
+ $hashalgo="sha256";
+ } elsif (exists $config{"CONFIG_MODULE_SIG_SHA384"}) {
+ $hashalgo="sha384";
+ } elsif (exists $config{"CONFIG_MODULE_SIG_SHA512"}) {
+ $hashalgo="sha512";
+ } else {
+ die "Can't determine hash algorithm";
+ }
+}
+
#
# Function to read the contents of a file into a variable.
#
@@ -332,35 +348,35 @@ my $id_type = 1; # Identifier type: X.509
# Digest the data
#
my ($dgst, $prologue) = ();
-if (exists $config{"CONFIG_MODULE_SIG_SHA1"}) {
+if ($hashalgo eq "sha1") {
$prologue = pack("C*",
0x30, 0x21, 0x30, 0x09, 0x06, 0x05,
0x2B, 0x0E, 0x03, 0x02, 0x1A,
0x05, 0x00, 0x04, 0x14);
$dgst = "-sha1";
$hash = 2;
-} elsif (exists $config{"CONFIG_MODULE_SIG_SHA224"}) {
+} elsif ($hashalgo eq "sha224") {
$prologue = pack("C*",
0x30, 0x2d, 0x30, 0x0d, 0x06, 0x09,
0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x04,
0x05, 0x00, 0x04, 0x1C);
$dgst = "-sha224";
$hash = 7;
-} elsif (exists $config{"CONFIG_MODULE_SIG_SHA256"}) {
+} elsif ($hashalgo eq "sha256") {
$prologue = pack("C*",
0x30, 0x31, 0x30, 0x0d, 0x06, 0x09,
0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x01,
0x05, 0x00, 0x04, 0x20);
$dgst = "-sha256";
$hash = 4;
-} elsif (exists $config{"CONFIG_MODULE_SIG_SHA384"}) {
+} elsif ($hashalgo eq "sha384") {
$prologue = pack("C*",
0x30, 0x41, 0x30, 0x0d, 0x06, 0x09,
0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x02,
0x05, 0x00, 0x04, 0x30);
$dgst = "-sha384";
$hash = 5;
-} elsif (exists $config{"CONFIG_MODULE_SIG_SHA512"}) {
+} elsif ($hashalgo eq "sha512") {
$prologue = pack("C*",
0x30, 0x51, 0x30, 0x0d, 0x06, 0x09,
0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x03,
@@ -368,7 +384,7 @@ if (exists $config{"CONFIG_MODULE_SIG_SHA1"}) {
$dgst = "-sha512";
$hash = 6;
} else {
- die "Can't determine hash algorithm";
+ die "Invalid hash algorithm $hashalgo";
}
#
--
1.8.0
... when CONFIG_FIRMWARE_SIG is set.
Signed-off-by: Takashi Iwai <[email protected]>
---
Makefile | 6 ++++++
scripts/Makefile.fwinst | 18 ++++++++++++++++--
2 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile
index a1ccf22..c6d7a3e 100644
--- a/Makefile
+++ b/Makefile
@@ -729,6 +729,12 @@ mod_sign_cmd = true
endif
export mod_sign_cmd
+ifeq ($(CONFIG_FIRMWARE_SIG),y)
+fw_sign_cmd = perl $(srctree)/scripts/sign-file -f $(MODSECKEY) $(MODPUBKEY)
+else
+fw_sign_cmd = true
+endif
+export fw_sign_cmd
ifeq ($(KBUILD_EXTMOD),)
core-y += kernel/ mm/ fs/ ipc/ security/ crypto/ block/
diff --git a/scripts/Makefile.fwinst b/scripts/Makefile.fwinst
index 4d908d1..df256f0 100644
--- a/scripts/Makefile.fwinst
+++ b/scripts/Makefile.fwinst
@@ -29,6 +29,20 @@ installed-mod-fw := $(addprefix $(INSTALL_FW_PATH)/,$(mod-fw))
installed-fw := $(addprefix $(INSTALL_FW_PATH)/,$(fw-shipped-all))
installed-fw-dirs := $(sort $(dir $(installed-fw))) $(INSTALL_FW_PATH)/./
+ifeq ($(CONFIG_FIRMWARE_SIG),y)
+installed-fw-sig := $(patsubst %,%.sig, $(installed-fw))
+installed-mod-fw-sig := $(patsubst %,%.sig, $(installed-mod-fw))
+else
+installed-fw-sig :=
+installed-mod-fw-sig :=
+endif
+
+quiet_cmd_fwsig = FWSIG $@
+ cmd_fwsig = $(fw_sign_cmd) $(patsubst %.sig,%,$@) $@
+
+%.sig: %
+ $(call cmd,fwsig)
+
# Workaround for make < 3.81, where .SECONDEXPANSION doesn't work.
PHONY += $(INSTALL_FW_PATH)/$$(%) install-all-dirs
$(INSTALL_FW_PATH)/$$(%): install-all-dirs
@@ -49,9 +63,9 @@ PHONY += __fw_install __fw_modinst FORCE
.PHONY: $(PHONY)
-__fw_install: $(installed-fw)
+__fw_install: $(installed-fw) $(installed-fw-sig)
-__fw_modinst: $(installed-mod-fw)
+__fw_modinst: $(installed-mod-fw) $(installed-mod-fw-sig)
@:
__fw_modbuild: $(addprefix $(obj)/,$(mod-fw))
--
1.8.0
於 四，2012-11-08 於 18:35 +0100，Takashi Iwai 提到：
> Add a new option -a to sign-file for specifying the hash algorithm
> to sign a file, to make it working without .config file.
> This will be useful signing external module or firmware files.
>
> Signed-off-by: Takashi Iwai <[email protected]>
Tested-by: Chun-Yi Lee<[email protected]>
Joey Lee
> ---
> scripts/sign-file | 40 ++++++++++++++++++++++++++++------------
> 1 file changed, 28 insertions(+), 12 deletions(-)
>
> diff --git a/scripts/sign-file b/scripts/sign-file
> index 5b9d44d..581cdcd 100755
> --- a/scripts/sign-file
> +++ b/scripts/sign-file
> @@ -4,7 +4,7 @@
> #
> # Format:
> #
> -# ./scripts/sign-file [-v] [-f] <key> <x509> <module> [<dest>]
> +# ./scripts/sign-file [-v] [-f] [-a algo] <key> <x509> <module> [<dest>]
> #
> #
> use strict;
> @@ -17,16 +17,19 @@ sub usage()
> print "Format: ./scripts/sign-file [options] <key> <x509> <module> [<dest>]
> -v verbose output
> -f create a firmware signature file
> + -a algo specify hash algorithm
> ";
> exit;
> }
>
> my $verbose = 0;
> +my $hashalgo = "";
> my $sign_fw = 0;
>
> GetOptions(
> 'v|verbose' => \$verbose,
> - 'f|firmware' => \$sign_fw) || usage();
> + 'f|firmware' => \$sign_fw,
> + 'a|algo=s' => \$hashalgo) || usage();
> usage() if ($#ARGV != 2 && $#ARGV != 3);
>
> my $private_key = $ARGV[0];
> @@ -42,10 +45,7 @@ die "Can't read $mode_name\n" unless (-r $module);
> #
> # Read the kernel configuration
> #
> -my %config = (
> - CONFIG_MODULE_SIG_SHA512 => 1
> - );
> -
> +my %config;
> if (-r ".config") {
> open(FD, "<.config") || die ".config";
> while (<FD>) {
> @@ -56,6 +56,22 @@ if (-r ".config") {
> close(FD);
> }
>
> +if ($hashalgo eq "") {
> + if (exists $config{"CONFIG_MODULE_SIG_SHA1"}) {
> + $hashalgo="sha1";
> + } elsif (exists $config{"CONFIG_MODULE_SIG_SHA224"}) {
> + $hashalgo="sha224";
> + } elsif (exists $config{"CONFIG_MODULE_SIG_SHA256"}) {
> + $hashalgo="sha256";
> + } elsif (exists $config{"CONFIG_MODULE_SIG_SHA384"}) {
> + $hashalgo="sha384";
> + } elsif (exists $config{"CONFIG_MODULE_SIG_SHA512"}) {
> + $hashalgo="sha512";
> + } else {
> + die "Can't determine hash algorithm";
> + }
> +}
> +
> #
> # Function to read the contents of a file into a variable.
> #
> @@ -332,35 +348,35 @@ my $id_type = 1; # Identifier type: X.509
> # Digest the data
> #
> my ($dgst, $prologue) = ();
> -if (exists $config{"CONFIG_MODULE_SIG_SHA1"}) {
> +if ($hashalgo eq "sha1") {
> $prologue = pack("C*",
> 0x30, 0x21, 0x30, 0x09, 0x06, 0x05,
> 0x2B, 0x0E, 0x03, 0x02, 0x1A,
> 0x05, 0x00, 0x04, 0x14);
> $dgst = "-sha1";
> $hash = 2;
> -} elsif (exists $config{"CONFIG_MODULE_SIG_SHA224"}) {
> +} elsif ($hashalgo eq "sha224") {
> $prologue = pack("C*",
> 0x30, 0x2d, 0x30, 0x0d, 0x06, 0x09,
> 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x04,
> 0x05, 0x00, 0x04, 0x1C);
> $dgst = "-sha224";
> $hash = 7;
> -} elsif (exists $config{"CONFIG_MODULE_SIG_SHA256"}) {
> +} elsif ($hashalgo eq "sha256") {
> $prologue = pack("C*",
> 0x30, 0x31, 0x30, 0x0d, 0x06, 0x09,
> 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x01,
> 0x05, 0x00, 0x04, 0x20);
> $dgst = "-sha256";
> $hash = 4;
> -} elsif (exists $config{"CONFIG_MODULE_SIG_SHA384"}) {
> +} elsif ($hashalgo eq "sha384") {
> $prologue = pack("C*",
> 0x30, 0x41, 0x30, 0x0d, 0x06, 0x09,
> 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x02,
> 0x05, 0x00, 0x04, 0x30);
> $dgst = "-sha384";
> $hash = 5;
> -} elsif (exists $config{"CONFIG_MODULE_SIG_SHA512"}) {
> +} elsif ($hashalgo eq "sha512") {
> $prologue = pack("C*",
> 0x30, 0x51, 0x30, 0x0d, 0x06, 0x09,
> 0x60, 0x86, 0x48, 0x01, 0x65, 0x03, 0x04, 0x02, 0x03,
> @@ -368,7 +384,7 @@ if (exists $config{"CONFIG_MODULE_SIG_SHA1"}) {
> $dgst = "-sha512";
> $hash = 6;
> } else {
> - die "Can't determine hash algorithm";
> + die "Invalid hash algorithm $hashalgo";
> }
>
> #
於 四，2012-11-08 於 18:35 +0100，Takashi Iwai 提到：
> Add -f option to sign-file script for generating a firmware signature
> file.
>
> A firmware signature file contains a pretty similar structure like a
> signed module but in a different order (because it's a separate file
> while the module signature is embedded at the tail of unsigned module
> contents). The file consists of
> - the magic string
> - the signature information, which is identical with the module
> signature
> - signer's name
> - key id
> - signature bytes
>
> Signed-off-by: Takashi Iwai <[email protected]>
Tested-by: Chun-Yi Lee <[email protected]>
Joey Lee
> ---
> scripts/sign-file | 48 +++++++++++++++++++++++++++++++++++-------------
> 1 file changed, 35 insertions(+), 13 deletions(-)
>
> diff --git a/scripts/sign-file b/scripts/sign-file
> index 87ca59d..5b9d44d 100755
> --- a/scripts/sign-file
> +++ b/scripts/sign-file
> @@ -4,30 +4,40 @@
> #
> # Format:
> #
> -# ./scripts/sign-file [-v] <key> <x509> <module> [<dest>]
> +# ./scripts/sign-file [-v] [-f] <key> <x509> <module> [<dest>]
> #
> #
> use strict;
> use FileHandle;
> use IPC::Open2;
> +use Getopt::Long;
>
> -my $verbose = 0;
> -if ($#ARGV >= 0 && $ARGV[0] eq "-v") {
> - $verbose = 1;
> - shift;
> +sub usage()
> +{
> + print "Format: ./scripts/sign-file [options] <key> <x509> <module> [<dest>]
> + -v verbose output
> + -f create a firmware signature file
> +";
> + exit;
> }
>
> -die "Format: ./scripts/sign-file [-v] <key> <x509> <module> [<dest>]\n"
> - if ($#ARGV != 2 && $#ARGV != 3);
> +my $verbose = 0;
> +my $sign_fw = 0;
> +
> +GetOptions(
> + 'v|verbose' => \$verbose,
> + 'f|firmware' => \$sign_fw) || usage();
> +usage() if ($#ARGV != 2 && $#ARGV != 3);
>
> my $private_key = $ARGV[0];
> my $x509 = $ARGV[1];
> my $module = $ARGV[2];
> -my $dest = ($#ARGV == 3) ? $ARGV[3] : $ARGV[2] . "~";
> +my $dest = $ARGV[3] ? $ARGV[3] : $ARGV[2] . ($sign_fw ? ".sig" : "~");
> +my $mode_name = $sign_fw ? "firmware" : "module";
>
> die "Can't read private key\n" unless (-r $private_key);
> die "Can't read X.509 certificate\n" unless (-r $x509);
> -die "Can't read module\n" unless (-r $module);
> +die "Can't read $mode_name\n" unless (-r $module);
>
> #
> # Read the kernel configuration
> @@ -393,7 +403,9 @@ die "openssl rsautl died: $?" if ($? >> 8);
> #
> my $unsigned_module = read_file($module);
>
> -my $magic_number = "~Module signature appended~\n";
> +my $magic_number = $sign_fw ?
> + "~Linux firmware signature~\n" :
> + "~Module signature appended~\n";
>
> my $info = pack("CCCCCxxxN",
> $algo, $hash, $id_type,
> @@ -402,7 +414,7 @@ my $info = pack("CCCCCxxxN",
> length($signature));
>
> if ($verbose) {
> - print "Size of unsigned module: ", length($unsigned_module), "\n";
> + print "Size of unsigned $mode_name: ", length($unsigned_module), "\n";
> print "Size of signer's name : ", length($signers_name), "\n";
> print "Size of key identifier : ", length($key_identifier), "\n";
> print "Size of signature : ", length($signature), "\n";
> @@ -414,7 +426,16 @@ if ($verbose) {
>
> open(FD, ">$dest") || die $dest;
> binmode FD;
> -print FD
> +if ($sign_fw) {
> + print FD
> + $magic_number,
> + $info,
> + $signers_name,
> + $key_identifier,
> + $signature
> + ;
> +} else {
> + print FD
> $unsigned_module,
> $signers_name,
> $key_identifier,
> @@ -422,8 +443,9 @@ print FD
> $info,
> $magic_number
> ;
> +}
> close FD || die $dest;
>
> -if ($#ARGV != 3) {
> +if (!$sign_fw && $#ARGV != 3) {
> rename($dest, $module) || die $module;
> }
於 四，2012-11-08 於 18:35 +0100，Takashi Iwai 提到：
> ... when CONFIG_FIRMWARE_SIG is set.
>
> Signed-off-by: Takashi Iwai <[email protected]>
Tested-by: Chun-Yi Lee <[email protected]>
Joey Lee
> ---
> Makefile | 6 ++++++
> scripts/Makefile.fwinst | 18 ++++++++++++++++--
> 2 files changed, 22 insertions(+), 2 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index a1ccf22..c6d7a3e 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -729,6 +729,12 @@ mod_sign_cmd = true
> endif
> export mod_sign_cmd
>
> +ifeq ($(CONFIG_FIRMWARE_SIG),y)
> +fw_sign_cmd = perl $(srctree)/scripts/sign-file -f $(MODSECKEY) $(MODPUBKEY)
> +else
> +fw_sign_cmd = true
> +endif
> +export fw_sign_cmd
>
> ifeq ($(KBUILD_EXTMOD),)
> core-y += kernel/ mm/ fs/ ipc/ security/ crypto/ block/
> diff --git a/scripts/Makefile.fwinst b/scripts/Makefile.fwinst
> index 4d908d1..df256f0 100644
> --- a/scripts/Makefile.fwinst
> +++ b/scripts/Makefile.fwinst
> @@ -29,6 +29,20 @@ installed-mod-fw := $(addprefix $(INSTALL_FW_PATH)/,$(mod-fw))
> installed-fw := $(addprefix $(INSTALL_FW_PATH)/,$(fw-shipped-all))
> installed-fw-dirs := $(sort $(dir $(installed-fw))) $(INSTALL_FW_PATH)/./
>
> +ifeq ($(CONFIG_FIRMWARE_SIG),y)
> +installed-fw-sig := $(patsubst %,%.sig, $(installed-fw))
> +installed-mod-fw-sig := $(patsubst %,%.sig, $(installed-mod-fw))
> +else
> +installed-fw-sig :=
> +installed-mod-fw-sig :=
> +endif
> +
> +quiet_cmd_fwsig = FWSIG $@
> + cmd_fwsig = $(fw_sign_cmd) $(patsubst %.sig,%,$@) $@
> +
> +%.sig: %
> + $(call cmd,fwsig)
> +
> # Workaround for make < 3.81, where .SECONDEXPANSION doesn't work.
> PHONY += $(INSTALL_FW_PATH)/$$(%) install-all-dirs
> $(INSTALL_FW_PATH)/$$(%): install-all-dirs
> @@ -49,9 +63,9 @@ PHONY += __fw_install __fw_modinst FORCE
>
> .PHONY: $(PHONY)
>
> -__fw_install: $(installed-fw)
> +__fw_install: $(installed-fw) $(installed-fw-sig)
>
> -__fw_modinst: $(installed-mod-fw)
> +__fw_modinst: $(installed-mod-fw) $(installed-mod-fw-sig)
> @:
>
> __fw_modbuild: $(addprefix $(obj)/,$(mod-fw))
於 四，2012-11-08 於 18:35 +0100，Takashi Iwai 提到：
> +#ifdef CONFIG_FIRMWARE_SIG
> +static int verify_sig_file(struct firmware_buf *buf, const char
> *path)
> +{
> + const unsigned long markerlen = sizeof(FIRMWARE_SIG_STRING) -
> 1;
> + struct file *file;
> + void *sig_data;
> + size_t sig_size;
> + int ret;
> +
> + file = filp_open(path, O_RDONLY, 0);
> + if (IS_ERR(file))
> + return -ENOKEY;
I think there should return '-ENOENT', otherwise the firmware will show
'Invalid firmware signature' even didn't find the sig file.
> +
> + ret = fw_read_file_contents(file, &sig_data, &sig_size);
> + fput(file);
Tested-by: Chun-Yi Lee <[email protected]>
Thanks a lot!
Joey Lee
At Fri, 23 Nov 2012 14:56:11 +0800,
joeyli wrote:
>
> 於 四，2012-11-08 於 18:35 +0100，Takashi Iwai 提到：
> > +#ifdef CONFIG_FIRMWARE_SIG
> > +static int verify_sig_file(struct firmware_buf *buf, const char
> > *path)
> > +{
> > + const unsigned long markerlen = sizeof(FIRMWARE_SIG_STRING) -
> > 1;
> > + struct file *file;
> > + void *sig_data;
> > + size_t sig_size;
> > + int ret;
> > +
> > + file = filp_open(path, O_RDONLY, 0);
> > + if (IS_ERR(file))
> > + return -ENOKEY;
>
> I think there should return '-ENOENT', otherwise the firmware will show
> 'Invalid firmware signature' even didn't find the sig file.
Actually this is the intentional behavior.
In the secure boot mode, unsigned firmware should be rejected.
In the normal boot mode, the -ENOKEY error is ignored. (Whether we
should taint the kernel with such an unsigned firmware is a bit
different question, though.)
thanks,
Takashi