On 08.05.2018 20:35, Julius Werner wrote:
Providers of blobs should sign them and take responsibility that the signed blobs were unaltered after compilation (e.g. do not contain malware). It is *recommended* that the public key needed to verify the signature is obtainable through a specific channel (e.g. single responsible contact person) _independently_ from the blob's distribution path.I'm not quite sure what the point of this would be for blobs that are already checked into 3rdparty/blobs, and I could imagine that it would be a surprisingly big obstacle for a lot of vendors. Please don't only think of Intel when trying to make these rules... not every vendor has the resources to set up their own blob distribution site with regular versioned releases on GitHub. On Arm systems the blob may often be designed specifically for coreboot, and the vendor loses interest as soon as the project they were paid to support (e.g. a Chromebook) is finished. I think this should still be fine as long as they uploaded it with a proper redistribution license to blobs.git, and implemented the coreboot interface with enough documentation to make it usable afterwards. For vendors like Intel that insist on hosting their blobs externally, I think it's fair to impose more rules to avoid the problems you've described, but we should still leave open an easier option for vendors that are willing to redistribute their blobs through coreboot.org (which I think is the much nicer option for us, so we should incentivize it).
Yes, I agree and already did so when writing the above. That's why I made it a recommendation and not a requirement. I also intentionally didn't write "vendor". Just whoever provides the blob should sign it.
For each platform that relies on a blob, the coreboot tree *should* point (e.g. git submodule) to a commit containing both the header files and the matching binary.It seems like you want to move headers from the coreboot into the blobs directory? I don't really see the point in that either. Headers are source, they need to be covered under the GPL... it's easier to make that clear when they are in coreboot. I agree with your point that headers and blobs should always be updated together, but we can still enforce that across two repositories.
Ack. It's technically the same if you change a pointer to both or change the pointer to the binary together with the header files in one commit. But it's easier to make mistakes in the latter case. (I might be too biased here as I've seen too much chaos with these things.)
Unless a pointer as described above exists for a given plat- form that relies on a blob, all changes* to that platform *shall* be refused.I think this is counter-productive, as is removing any old boards that don't comply. I am okay with creating new rules for future platforms, but there is no reason to throw perfectly good and working boards out just because they weren't written to comply with something we only just made up. You wouldn't really be punishing anyone with that (vendors don't care about outdated chips anyway), you'd just be taking choice away from our users.
Well, that's why I brought older Intel based CrOS devices up. I think they are the only ones that currently can't comply. I agree that we should have an exception for older platforms. But it's not really something "just made up", IMHO. Google made their own rules when bringing blobs in with Sandy Bridge, just to break them later. So the problem was obviously already visible when these platforms were added.
We should continue removing old boards when they become too much of a maintenance burden, which may happen due to an unmaintainable blob mess, but we shouldn't categorically remove anything with blobs.
OTOH, what's the benefit of maintaining them in the master branch if nobody else can do a port with the platform code?
I think allowing users to build their own custom firmware for an already supported board is definitely reason enough to keep it in the master branch. coreboot is not just a project for allowing companies large and small to build new mainboards... it's also for private enthusiasts that want to tinker with firmware for a supported board they bought somewhere.
While I don't see why that has to happen on the master branch, you have a point there. Let's just say all the `shall`s only apply to binaries released from 4.8 on.
Of course it would be nice if every chipset was implemented well enough that you can bring up a new mainboard without any help from the vendor, but that's not always that simple, and I don't think it should be a requirement for inclusion.
Sorry, I thought that is what free software is about. Must have been confused to mix that up with coreboot. IMO, blobs are already a huge offense, a disgrace of coreboot and the GPL. But having to sign an NDA to be able to use a GPL licensed project in a product? WTF?!?
I don't care if people think that their loopholes are safe enough and use coreboot that way. But why should the community, including many volunteers, maintain that junk on the master branch?
If that is the attitude moving forward, maybe we should stop licensing new code under the GPL? Just as a courtesy towards free-software develo- pers. And to make it clear for new contributors what they sign up for.
Nico
On 09.05.2018 01:04, Nico Huber wrote:
Unless a pointer as described above exists for a given plat- form that relies on a blob, all changes* to that platform *shall* be refused.I think this is counter-productive, as is removing any old boards that don't comply. I am okay with creating new rules for future platforms, but there is no reason to throw perfectly good and working boards out just because they weren't written to comply with something we only just made up. You wouldn't really be punishing anyone with that (vendors don't care about outdated chips anyway), you'd just be taking choice away from our users.
Well, that's why I brought older Intel based CrOS devices up. I think they are the only ones that currently can't comply. I agree that we should have an exception for older platforms. But it's not really something "just made up", IMHO. Google made their own rules when bringing blobs in with Sandy Bridge, just to break them later. So the problem was obviously already visible when these platforms were added.
I need to clarify this. There were no written rules, AFAIR. What I meant was that Google broke with a practice they established themselves. So they should have noticed the conflict, IMHO.
I'll stop yelling about the past. Let's focus on some rules for the future.
Nico
Sorry for being late to answer to my own thread (busy busy busy).
A few notes : The initial check-in of the kabylake FSP was uploaded with a BSD license : https://github.com/IntelFsp/FSP/tree/d88078a708e768c7b6ee5cbc996299d303c3c70... Later commits added Intel's Restricted Use License Agreement (which basically prevents redistribution (and reverse engineering)). That's also why the header commit I submitted didn't update the headers to match the Gold release, but only what was in the initial 'BSD' release, which thankfully is compatible with the Gold release (besides, most of the changes, other than the license header, were to add typos and remove fields, since the header in coreboot is technically 'newer').
Regarding Nico's idea of signing blobs, for old unmaintained blobs, they could still be signed with a coreboot specific "this is a known working, not guaranteed to be malware free but guaranteed to be unmodified for years, or unmodified since its official release" key, and a coreboot maintainer would have the private key to sign old blobs with that key. That would at least resolve the condition of "all blobs must be signed".
I do agree with Julius that headers should remain in coreboot, and not move to the blobs.git, and I agree that removing old boards doesn't make sense. Yes, I think coreboot is meant for enthusiasts and tinkerers who want to build an image for their boards, and that's why I'm suggesting to make the headers match the github version, because it's now impossible to build anything with FSP (whether a google board, or a Purism board for example) without first modifying coreboot because the only FSP available to enthusiasts and tinkerers is the github one.
In general, I like Nico's idea of setting up rules for blobs, but my worry is that no matter how great and logical the rules are, the blob-makers might simply ignore them.. you can ask for signed blobs, but what if they refuse to sign it? Or even better, you can ask for redistributable blobs, but what if they refuse to make them redistributable? Will you simply stop supporting any boards that depend on those blobs? You'd have to eventually compromise (for the sake of the users/project) and break your own rules that you worked so hard to perfect... Since they control the blobs, they have the power, unless you think "integrate in coreboot" is a hard requirement for them, and in that case, if you set up those rules, they would be forced to follow them, that can actually help get things done the right way instead of just asking nicely and never getting a positive response. I don't really know... it might work for Intel/CrOS for example, but maybe not for other vendors who wouldn't need to be integrated in coreboot...
Anyway, to resolve the immediate issue with the header, how about adding a 'variants' system, like we already have with mainboards? Something like : src/vendorcode/intel/fsp/fsp2_0/variants/google/*.h src/vendorcode/intel/fsp/fsp2_0/variants/github/*.h and in the Kconfig, we could just set FSP_VARIANT_DIR="github" That would be (I assume) much less messy than having #ifdefs to define-or-not specific fields in the UPD structures. The only issue would be with regards to fields that coreboot uses that aren't available in a variant, like for example, the PcieRpClkSrcNumber field that doesn't exist in the github header but is used in coreboot, there are a couple of solutions to that : 1 - add #define HAVE_PCIE_RP_CLK_SRC_NUMBER (for example) in the headers and have #ifdefs everywhere in coreboot code. This is not a great idea 2 - have something like "#define PcieRpClkSrcNumber UnusedUpdSpace26", but that would add so much problems that it's a very stupid idea... why am I ever writing this? 3 - manually add the missing fields somewhere in the Upd and only have the changes between two variants where the field offsets change in the structure. 4 - anyone else with better ideas?
Using solution 3 above, The 'google' variant would be the current header, and the 'github' variant would be the header after my patch [1] applied to it (so it's a mix between the google and the real github header, but with field offsets matching what the github blob would return), all the fields are still there, I think there are 3 fields that coreboot uses which aren't in the official header, and they just end up in unused space, but at least the MemInfoHob will be correct.
What do you think ? Youness.
On Wed, May 9, 2018 at 3:49 AM, Nico Huber nico.h@gmx.de wrote:
On 09.05.2018 01:04, Nico Huber wrote:
Unless a pointer as described above exists for a given plat- form that relies on a blob, all changes* to that platform *shall* be refused.I think this is counter-productive, as is removing any old boards that don't comply. I am okay with creating new rules for future platforms, but there is no reason to throw perfectly good and working boards out just because they weren't written to comply with something we only just made up. You wouldn't really be punishing anyone with that (vendors don't care about outdated chips anyway), you'd just be taking choice away from our users.
Well, that's why I brought older Intel based CrOS devices up. I think they are the only ones that currently can't comply. I agree that we should have an exception for older platforms. But it's not really something "just made up", IMHO. Google made their own rules when bringing blobs in with Sandy Bridge, just to break them later. So the problem was obviously already visible when these platforms were added.
I need to clarify this. There were no written rules, AFAIR. What I meant was that Google broke with a practice they established themselves. So they should have noticed the conflict, IMHO.
I'll stop yelling about the past. Let's focus on some rules for the future.
Nico
Yes, I agree and already did so when writing the above. That's why I made it a recommendation and not a requirement. I also intentionally didn't write "vendor". Just whoever provides the blob should sign it.
I still don't really get what signing in general is solving here. Digital signatures are only useful if you want the signer to be able to make a new release without having to update the thing that's checking the signature. Otherwise you can just store a list of allowed hashes, which is much easier (and already implicit in Git if you use the coreboot.org blobs repository). In the Intel case maybe signing them makes sense, but it shouldn't be a big deal to just maintain a list of FSP releases with their hashes in the coreboot repo either... at least that way you maintain control over which blobs are valid (e.g. you could enforce that they have been tested with coreboot before inclusion). If you just let Intel sign stuff, you're giving them complete control over this thing that I think you intend as a protection for our users. They could sign a special blob for the NSA and you'd never know until you accidentally catch it running on someone's system. (Or do I misunderstand and you're talking about signatures checked at runtime? That wouldn't really make sense on a per blob basis when the rest of coreboot isn't signed, I think. If you want runtime signatures, it's better to use a system like vboot that covers the whole image, coreboot and blobs and all.)
Of course it would be nice if every chipset was implemented well enough that you can bring up a new mainboard without any help from the vendor,
but
that's not always that simple, and I don't think it should be a
requirement
for inclusion.
Sorry, I thought that is what free software is about. Must have been confused to mix that up with coreboot. IMO, blobs are already a huge offense, a disgrace of coreboot and the GPL. But having to sign an NDA to be able to use a GPL licensed project in a product? WTF?!?
I don't care if people think that their loopholes are safe enough and use coreboot that way. But why should the community, including many volunteers, maintain that junk on the master branch?
If that is the attitude moving forward, maybe we should stop licensing new code under the GPL? Just as a courtesy towards free-software develo- pers. And to make it clear for new contributors what they sign up for.
I think you're getting pretty absurd for the sake of argument here. A different mainboard is a completely different device, of course it may not run with the firmware meant for another device. Complaining about this as a GPL violation is like saying that Intel was violating the GPL by adding a Haswell port to coreboot and then not allowing you to support a Skylake board with that. It's a completely different thing, of course the code meant for something else doesn't work on there.
Of course I think we should try to keep blobs mainboard-agnostic, and whenever I have a chance to influence that I do. But it's not always that simple. Often you may not realize that it won't work on another board until you actually build that board and try it out. It's not like anyone is actively preventing or disallowing you from bringing up another mainboard with the same blob that's already there, and if your other board uses 100% the same components as the original then that should always work... but if you find out you need to use some controller or I/O pad that this blob didn't initialize because other mainboards didn't need it, then yeah, sorry, it won't work. It's not like anyone did that intentionally, it's just bad luck. I think this is conceptually the same as when you have a completely blob-free platform but you want to use some peripheral that nobody ever implemented a driver for before. You'll need help from the vendor then as well.
-
Julius Werner -
Nico Huber -
Youness Alaoui