Hi, If I may express my thoughts on this...
> Yeah, I really don't understand your concern here. Having an undocumented (or silently installed, therefore unexpected) dependency is undesirable (especially for a firmware), no question about that. > You seem to imply that this may cause some kind of security or availability > problem Can you guarantee that a silently installed submodule's repo won't get hacked and replaced with malicious code for example? We have seen that happening with other repos already (github, sourceforge and other webhosts too). The fewer dependency you have, the less are the chances for a vulnerability or sechole, simple as that. But the main problem here I think is, following the tutorial https://doc.coreboot.org/tutorial/part1.html does not work, because it results in compilation errors (due to missing dependencies). This is clearly bad and must be resolved. It's not documented, but you MUST install the vboot submodule (which supposed to be OPTIONAL according to Kconfig) to get it work. Right now it is not possible to compile the vanilla coreboot source without vboot at all. I agree with Peter, this is unacceptable. About the solution: I think the best would be to use ifdef guards in the C code (both for coreboot and cbfstools), that's what the precompiler is for, after all. There's already a CONFIG_VBOOT_LIB config option, so why isn't it used? I see no reasonable excuse for that. I also think that "default n" should be added to https://github.com/coreboot/coreboot/blob/master/src/security/vboot/Kconfig However if Kconfig isn't wanted for any reason, it's pretty simple to add "ifneq ($(wildcard ../3rdparty/vboot/somefile*),)" to the Makefile. This way if the vboot submodule is cloned, then coreboot and cbfstool would be automatically compiled with support for it (without a Kconfig option). But for people who just clone the base repo, there would be no compilation errors. Everybody happy, problem solved. IMHO, bzt On 11/17/20, Julius Werner <jwer...@chromium.org> wrote: >> One is what you describe - a generic utility supporting everything >> that gets installed into say /usr/local/bin for lots of different >> invocations to do lots of different things with lots of different >> coreboot images. >> >> Two is specifically what is required to complete the configured build. >> >> It's certainly useful, and I argue highly desirable, to consider >> these two cases distinct. > > Yes, those are different cases and they could use differently > configured builds of cbfstool. Whether that's desirable is a different > question. Adding a configuration system to cbfstool would come with a > lot of maintenance overhead -- rather than just making sure the tool > as a whole keeps working, we'd have to ensure all possible > configurations keep working independently. The way cbfstool is > currently written doesn't really lend itself to easily split out all > these kinds of optional things. You're asking for a lot of work, > basically, and I fail to see the benefit on the other side. The > current monolithic design shouldn't really cause problems or drawbacks > to anyone (and quite frankly, I think it doesn't -- as mentioned > before I think the issues discussed in this thread don't really have > much to do with how big cbfstool is, and it doesn't really seem like a > good "solution" to the HOSTCC problem to just allow switches to stop > building parts that happened to cause compiler problems in this > specific instance. Sooner or later some compiler-specific code would > be added to a core function that you can't configure out). > >> Even worse is that you find it acceptable, or desirable, that a submodule >> is silently pulled in during the build. That may be typical for web >> development, but nothing that should be proliferated. > > Yeah, I really don't understand your concern here. Is there maybe some > confusion about how Git submodules work? You seem to imply that this > may cause some kind of security or availability problem where there is > none: submodule code is mirrored on and downloaded from coreboot.org, > same as the main repository. The currently active submodule repository > HEAD is stored in the main coreboot repository (as a Git SHA), so you > will always get exactly the version of the code you're supposed to > have, and updating that is a step that is manually committed to the > main coreboot repository. Submodules cannot suddenly become > unavailable or taken over by malicious actors due to actions outside > coreboot.org's control -- Git is not node.js. If you're just someone > building code from source in your local checkout, it really makes zero > practical difference to you whether that code comes out of the main > coreboot repository or a submodule (both of them are already on your > disk as part of the checkout and cryptographically guaranteed by > coreboot.org, nobody else). > > Really, your host system's local glibc (which is also linked into > cbfstool) is a lot more unpredictable and potentially problematic here > than the parts of vboot that it includes. > >> > I am just saying I don't think this discussion should be about vboot >> >> It is because that's what consistently causes me extra work and >> frustration every time I want to build a minimal coreboot. > > Well then let's please talk about those specific frustrations and try > to find practical solutions instead. Like I said earlier, we're really > not trying to make vboot annoying (or even visible) to those who don't > want to deal with it, and I'm convinced that it shouldn't be. The > submodule setup we have right now really works very well in general > and there's basically no difference for you in whether something is > built out of coreboot/src or coreboot/3rdparty/xxx. If it does cause > friction for you, let me know and I'll try my best to help solve that. > I understand that there's the HOSTCC issue, but as I said it really > doesn't have anything to do with vboot, and there's nothing vboot > alone could do to solve it, nor would somehow removing vboot really > solve anything about that. > > FWIW I do seem to recall that this already came up back when > __attribute__((fallthrough)) was added to vboot, and back then > everyone seemed to agree that it was reasonable to assume the same > version for the host compiler that we use in crossgcc. If we now > changed our mind and think this keeps causing too much pain to people > all the time, I'm happy to take that back out of vboot and make it > compatible with whatever older version you care about if that would > alleviate people's concerns. We're perfectly willing to fully align > vboot's code base on whatever the coreboot policy is on these sort of > things, the problem is just that coreboot can't seem to decide on a > clear policy for the host compiler, and whenever this is brought back > up again somehow vboot always gets all the blame. > _______________________________________________ > coreboot mailing list -- coreboot@coreboot.org > To unsubscribe send an email to coreboot-le...@coreboot.org > _______________________________________________ coreboot mailing list -- coreboot@coreboot.org To unsubscribe send an email to coreboot-le...@coreboot.org
