Re: [PATCH RFC 0/4] Add firmware signature file check
> > 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC 0/4] Add firmware signature file check
On Tue, Nov 6, 2012 at 6:53 PM, Takashi Iwai 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC 0/4] Add firmware signature file check
At Tue, 6 Nov 2012 18:40:57 +0800, Ming Lei wrote: > > On Tue, Nov 6, 2012 at 6:17 PM, Takashi Iwai wrote: > > At Tue, 6 Nov 2012 18:04:36 +0800, > > Ming Lei wrote: > >> > >> On Tue, Nov 6, 2012 at 4:18 PM, Takashi Iwai 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at
Re: [PATCH RFC 0/4] Add firmware signature file check
On Tue, Nov 6, 2012 at 6:17 PM, Takashi Iwai wrote: > At Tue, 6 Nov 2012 18:04:36 +0800, > Ming Lei wrote: >> >> On Tue, Nov 6, 2012 at 4:18 PM, Takashi Iwai 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC 0/4] Add firmware signature file check
At Tue, 6 Nov 2012 18:04:36 +0800, Ming Lei wrote: > > On Tue, Nov 6, 2012 at 4:18 PM, Takashi Iwai 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC 0/4] Add firmware signature file check
At Tue, 6 Nov 2012 09:20:19 +, 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC 0/4] Add firmware signature file check
On Tue, Nov 6, 2012 at 4:18 PM, Takashi Iwai 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC 0/4] Add firmware signature file check
> 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC 0/4] Add firmware signature file check
At Tue, 6 Nov 2012 16:04:50 +0800,
Ming Lei wrote:
>
> On Tue, Nov 6, 2012 at 3:32 PM, Takashi Iwai wrote:
> > At Tue, 6 Nov 2012 15:16:43 +0800,
> > Ming Lei wrote:
> >>
> >> On Tue, Nov 6, 2012 at 3:03 PM, Takashi Iwai 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC 0/4] Add firmware signature file check
On Tue, Nov 6, 2012 at 3:32 PM, Takashi Iwai wrote:
> At Tue, 6 Nov 2012 15:16:43 +0800,
> Ming Lei wrote:
>>
>> On Tue, Nov 6, 2012 at 3:03 PM, Takashi Iwai 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC 0/4] Add firmware signature file check
On Tue, Nov 6, 2012 at 3:32 PM, Takashi Iwai ti...@suse.de wrote:
At Tue, 6 Nov 2012 15:16:43 +0800,
Ming Lei wrote:
On Tue, Nov 6, 2012 at 3:03 PM, Takashi Iwai ti...@suse.de 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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC 0/4] Add firmware signature file check
At Tue, 6 Nov 2012 16:04:50 +0800,
Ming Lei wrote:
On Tue, Nov 6, 2012 at 3:32 PM, Takashi Iwai ti...@suse.de wrote:
At Tue, 6 Nov 2012 15:16:43 +0800,
Ming Lei wrote:
On Tue, Nov 6, 2012 at 3:03 PM, Takashi Iwai ti...@suse.de 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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC 0/4] Add firmware signature file check
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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC 0/4] Add firmware signature file check
On Tue, Nov 6, 2012 at 4:18 PM, Takashi Iwai ti...@suse.de 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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC 0/4] Add firmware signature file check
At Tue, 6 Nov 2012 09:20:19 +, 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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC 0/4] Add firmware signature file check
At Tue, 6 Nov 2012 18:04:36 +0800, Ming Lei wrote: On Tue, Nov 6, 2012 at 4:18 PM, Takashi Iwai ti...@suse.de 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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC 0/4] Add firmware signature file check
On Tue, Nov 6, 2012 at 6:17 PM, Takashi Iwai ti...@suse.de wrote: At Tue, 6 Nov 2012 18:04:36 +0800, Ming Lei wrote: On Tue, Nov 6, 2012 at 4:18 PM, Takashi Iwai ti...@suse.de 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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC 0/4] Add firmware signature file check
At Tue, 6 Nov 2012 18:40:57 +0800, Ming Lei wrote: On Tue, Nov 6, 2012 at 6:17 PM, Takashi Iwai ti...@suse.de wrote: At Tue, 6 Nov 2012 18:04:36 +0800, Ming Lei wrote: On Tue, Nov 6, 2012 at 4:18 PM, Takashi Iwai ti...@suse.de 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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC 0/4] Add firmware signature file check
On Tue, Nov 6, 2012 at 6:53 PM, Takashi Iwai ti...@suse.de 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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC 0/4] Add firmware signature file check
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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC 0/4] Add firmware signature file check
At Tue, 6 Nov 2012 15:16:43 +0800,
Ming Lei wrote:
>
> On Tue, Nov 6, 2012 at 3:03 PM, Takashi Iwai 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC 0/4] Add firmware signature file check
On Tue, Nov 6, 2012 at 1:36 PM, Li Joey 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC 0/4] Add firmware signature file check
On Tue, Nov 6, 2012 at 3:03 PM, Takashi Iwai 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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC 0/4] Add firmware signature file check
At Tue, 6 Nov 2012 13:36:46 +0800,
Li Joey wrote:
>
> [1 ]
> 2012/11/6 Ming Lei
>
> > On Tue, Nov 6, 2012 at 1:18 AM, Takashi Iwai 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
> 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(_lock);
> return retval;
> --
> 1.6.4.2
> [2 ]
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC 0/4] Add firmware signature file check
At Tue, 6 Nov 2012 10:30:26 +0800,
Ming Lei wrote:
>
> On Tue, Nov 6, 2012 at 1:18 AM, Takashi Iwai 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;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC 0/4] Add firmware signature file check
At Tue, 06 Nov 2012 00:01:52 +, David Howells wrote: > > Takashi Iwai 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC 0/4] Add firmware signature file check
At Mon, 5 Nov 2012 15:43:09 -0500, Josh Boyer wrote: > > On Mon, Nov 5, 2012 at 12:18 PM, 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. > > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC 0/4] Add firmware signature file check
2012/11/6 Ming Lei :
> On Tue, Nov 6, 2012 at 1:18 AM, Takashi Iwai 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 majord...@vger.kernel.org
> 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
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(_lock);
return retval;
--
1.6.4.2
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC 0/4] Add firmware signature file check
On Tue, Nov 6, 2012 at 1:18 AM, Takashi Iwai 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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC 0/4] Add firmware signature file check
David Howells wrote: > Takashi Iwai 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC 0/4] Add firmware signature file check
Takashi Iwai 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC 0/4] Add firmware signature file check
On Mon, Nov 5, 2012 at 12:18 PM, 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. 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC 0/4] Add firmware signature file check
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH RFC 0/4] Add firmware signature file check
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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH RFC 0/4] Add firmware signature file check
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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC 0/4] Add firmware signature file check
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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC 0/4] Add firmware signature file check
On Mon, Nov 5, 2012 at 12:18 PM, Takashi Iwai ti...@suse.de 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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC 0/4] Add firmware signature file check
Takashi Iwai ti...@suse.de 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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC 0/4] Add firmware signature file check
David Howells dhowe...@redhat.com wrote: Takashi Iwai ti...@suse.de 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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC 0/4] Add firmware signature file check
On Tue, Nov 6, 2012 at 1:18 AM, Takashi Iwai ti...@suse.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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC 0/4] Add firmware signature file check
2012/11/6 Ming Lei tom.leim...@gmail.com:
On Tue, Nov 6, 2012 at 1:18 AM, Takashi Iwai ti...@suse.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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
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 j...@suse.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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC 0/4] Add firmware signature file check
At Mon, 5 Nov 2012 15:43:09 -0500, Josh Boyer wrote: On Mon, Nov 5, 2012 at 12:18 PM, Takashi Iwai ti...@suse.de 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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC 0/4] Add firmware signature file check
At Tue, 06 Nov 2012 00:01:52 +, David Howells wrote: Takashi Iwai ti...@suse.de 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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC 0/4] Add firmware signature file check
At Tue, 6 Nov 2012 10:30:26 +0800,
Ming Lei wrote:
On Tue, Nov 6, 2012 at 1:18 AM, Takashi Iwai ti...@suse.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.
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;
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
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.leim...@gmail.com
On Tue, Nov 6, 2012 at 1:18 AM, Takashi Iwai ti...@suse.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 j...@suse.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 majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC 0/4] Add firmware signature file check
On Tue, Nov 6, 2012 at 3:03 PM, Takashi Iwai ti...@suse.de 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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC 0/4] Add firmware signature file check
On Tue, Nov 6, 2012 at 1:36 PM, Li Joey j...@novell.com 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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RFC 0/4] Add firmware signature file check
At Tue, 6 Nov 2012 15:16:43 +0800,
Ming Lei wrote:
On Tue, Nov 6, 2012 at 3:03 PM, Takashi Iwai ti...@suse.de 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
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
