I recently looked in to why I couldn't build coreboot on one of my systems any longer and I think I found the cause.
It looks like vboot uses features not available in gcc 6 on Debian Stretch.
I actually did manage to get it to build and work by commenting out the offending gcc warning flags and a fallthrough switch attribute, but that's kind of pointless.
Is there an expected minimal system gcc version and if so, is it documented? I couldn't find it noted anywhere.
Branden
Branden Waldner wrote:
Is there an expected minimal system gcc version and if so, is it documented? I couldn't find it noted anywhere.
There's the crossgcc tool and make target to create a known-working toolchain for building coreboot.
//Peter
On Mon, Nov 9, 2020 at 5:38 PM Peter Stuge peter@stuge.se wrote:
Branden Waldner wrote:
Is there an expected minimal system gcc version and if so, is it documented? I couldn't find it noted anywhere.
There's the crossgcc tool and make target to create a known-working toolchain for building coreboot.
See:
https://doc.coreboot.org/tutorial/part1.html Step 3 - Build the coreboot toolchain
Please note that this can take a significant amount of time. Use CPUS= to specify number of make jobs to run in parallel.
This will list toolchain options and supported architectures:
$ make help_toolchain
Here are some examples:
$ make crossgcc-i386 CPUS=$(nproc) # build i386 toolchain $ make crossgcc-aarch64 CPUS=$(nproc) # build Aarch64 toolchain $ make crossgcc-riscv CPUS=$(nproc) # build RISC-V toolchain
Note that the i386 toolchain is currently used for all x86 platforms, including x86_64.
Also note that you can possibly use your system toolchain, but the results are not reproducible, and may have issues, so this is not recommended. See step 5 to use your system toolchain.
//Peter _______________________________________________ coreboot mailing list -- coreboot@coreboot.org To unsubscribe send an email to coreboot-leave@coreboot.org
On 10.11.20 00:38, Peter Stuge wrote:
Branden Waldner wrote:
Is there an expected minimal system gcc version and if so, is it documented? I couldn't find it noted anywhere.
I don't think it's documented. As you already noticed, we depend on a 3rdparty library (vboot), so we actually don't know the minimum.
There's the crossgcc tool and make target to create a known-working toolchain for building coreboot.
Only for the cross-compilation part, though. For the build utils, the system's toolchain is used.
Nico
Nico Huber wrote:
I don't think it's documented. As you already noticed, we depend on a 3rdparty library (vboot), so we actually don't know the minimum.
Whenever I want a build without vboot I get really annoyed about this hardcoded dependency, even when vboot is disabled in Kconfig.
Would a patch to make the dependency conditional on Kconfig get accepted?
//Peter
On Mon, Nov 9, 2020 at 4:51 PM Peter Stuge peter@stuge.se wrote:
Nico Huber wrote:
I don't think it's documented. As you already noticed, we depend on a 3rdparty library (vboot), so we actually don't know the minimum.
Whenever I want a build without vboot I get really annoyed about this hardcoded dependency, even when vboot is disabled in Kconfig.
Would a patch to make the dependency conditional on Kconfig get accepted?
I should hope so, though I recommend starting a new thread to see if experts on vboot can chime in and explain why this is or isn't a good idea.
Whenever I want a build without vboot I get really annoyed about this hardcoded dependency, even when vboot is disabled in Kconfig.
Would a patch to make the dependency conditional on Kconfig get accepted?
I should hope so, though I recommend starting a new thread to see if experts on vboot can chime in and explain why this is or isn't a good idea.
vboot is used for more than just boot verification these days, we use it as a sort of generic crypto toolbox for all of coreboot's crypto needs (because it wouldn't make sense to implement, say, SHA-256 algorithms twice). For host utilities in particular, some of these are needed in cbfstool (e.g. for the --hash-algorithm parameter to add a hash attribute to a file), and there is no Kconfig cbfstool so you can't just configure it out if you don't need it.
I don't think it's fair to single in on vboot as the big problem here. We vboot developers are definitely not trying to make this hard or obnoxious for anyone, and while it is hosted in a separate upstream everyone is welcome to submit patches to that just like you can do to coreboot (e.g. Idwer submitted some stuff to fix compilation under FreeBSD recently) and I'll do what I can to help get them landed. But none of these problems are really vboot-specific -- the situation is just that there is no official toolchain for coreboot host utilities, and the only testing we have is for exactly what Jenkins uses (which I believe is the same thing used for crossgcc? +Patrick to keep me honest). Anyone can land a patch that uses a GCC 8+-only feature in cbfstool just as well as in vboot and it will cause exactly the same problem.
So I think the "official" rule is basically that the minimum requirement for host utilities is the same compiler and version that crossgcc uses, which I think makes some amount of sense (otherwise we'll just have to keep tracking and deploying two separate versions). If you guys want to change that we can have that discussion, but whatever we decide it's probably not going to be a very effective decision unless someone puts in the work to really make Jenkins test that continuously. And either way, I don't think any of this is vboot's fault in particular (vboot uprevs are tested by Jenkins just as much as any other patch and have to pass any compatibility tests enforced there).
Julius,
Julius Werner wrote:
Whenever I want a build without vboot I get really annoyed about this hardcoded dependency, even when vboot is disabled in Kconfig.
Would a patch to make the dependency conditional on Kconfig get accepted?
I should hope so, though I recommend starting a new thread to see if experts on vboot can chime in and explain why this is or isn't a good idea.
vboot is used for more than just boot verification these days, we use it as a sort of generic crypto toolbox for all of coreboot's crypto needs (because it wouldn't make sense to implement, say, SHA-256 algorithms twice). For host utilities in particular, some of these are needed in cbfstool (e.g. for the --hash-algorithm parameter to add a hash attribute to a file), and there is no Kconfig cbfstool so you can't just configure it out if you don't need it.
It is clear that I don't need that functionality when I build a coreboot version without any vboot code, right?
Note that I'm not talking about anything gcc, my question can indeed be considered a tangent of the original thread.
Thanks
//Peter
vboot is used for more than just boot verification these days, we use it as a sort of generic crypto toolbox for all of coreboot's crypto needs (because it wouldn't make sense to implement, say, SHA-256 algorithms twice). For host utilities in particular, some of these are needed in cbfstool (e.g. for the --hash-algorithm parameter to add a hash attribute to a file), and there is no Kconfig cbfstool so you can't just configure it out if you don't need it.
It is clear that I don't need that functionality when I build a coreboot version without any vboot code, right?
It's just an optional feature of cbfstool, it actually doesn't have anything to do with the firmware verification part of vboot. I mean, yes, you may not need it, but you may just as likely not need --alignment or --topswap-size or --empty-fits, or the locate, compact and add-flat-binary commands, or any other optional niche case feature that is supported in cbfstool. cbfstool is a toolbox utility that supports everything people may want to do with CBFS images, not all of which everyone necessarily needs. And it currently doesn't have a configuration infrastructure like Kconfig to disable individual features (and I hope that shouldn't become necessary either, because that would just make it complicated and confusing).
The goal with linking vboot into cbfstool is generally to be transparent, it's just pulling a few routines from a submodule, you're not really supposed to notice it. Just like when you run `make unit-tests` it's pulling in a third-party testing library from a submodule but generally you don't need to care about those details either. Unfortunately, if there is a situation where you can run into issues that Jenkins couldn't test for, you may see those issues anywhere in the code you build including inside those submodules, but I think that's really a problem with Jenkins and not one one with using submodules.
I am just saying I don't think this discussion should be about vboot just because an issue that could have occurred in literally any piece of code happened to occur in vboot code.
Julius Werner wrote:
needed in cbfstool (e.g. for the --hash-algorithm parameter to add a hash attribute to a file), and there is no Kconfig cbfstool so you can't just configure it out if you don't need it.
It is clear that I don't need that functionality when I build a coreboot version without any vboot code, right?
It's just an optional feature of cbfstool, it actually doesn't have anything to do with the firmware verification part of vboot. I mean, yes, you may not need it, but you may just as likely not need --alignment or --topswap-size or --empty-fits
Sure, but those don't pull in some other dependencies.
cbfstool is a toolbox utility that supports everything people may want to do with CBFS images
Thanks for the reminder.
Consider the two different use cases, or build targets, for cbfstool.
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.
The goal with linking vboot into cbfstool is generally to be transparent, it's just pulling a few routines from a submodule, you're not really supposed to notice it.
It's absurd to me that coreboot would require any routines out of any submodule for a build which will not use those routines.
I find that completely flawed.
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.
One purpose of Kconfig is to ensure that only what's neccessary gets built.
Just like when you run `make unit-tests` it's pulling in a third-party testing library from a submodule but generally you don't need to care about those details either.
It's wrong to pull in anything during build. I too am guilty of this by pushing for buildgcc, it would be good to improve that case too.
Anyway, I understand that your culture may be the complete opposite of mine.
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.
What some people always want isn't OK to require when other people do not want it. I think that's just lazy, and not the smart kind. :\
//Peter
Am Di., 17. Nov. 2020 um 05:06 Uhr schrieb Peter Stuge peter@stuge.se:
It's absurd to me that coreboot would require any routines out of any submodule for a build which will not use those routines.
coreboot doesn't, cbfstool does.
One purpose of Kconfig is to ensure that only what's neccessary gets built.
But cbfstool isn't hooked up to Kconfig. Given that it's not part of the final coreboot build, having extra stuff in cbfstool doesn't affect coreboot in the slightest, so it's not clear to me that we should change that.
It's wrong to pull in anything during build. I too am guilty of this
by pushing for buildgcc, it would be good to improve that case too.
Given your reference to buildgcc, I guess you mean "download"? In that case: git clone --recurse-submodules and there's not a single extra download going on at build time. I know because I frequently deal with two systems that forbid downloads at build time: qa.coreboot.org and Chromium OS' build infrastructure.
It is because that's what consistently causes me extra work and
frustration every time I want to build a minimal coreboot.
git clone --recurse-submodules is extra work, really?
What some people always want isn't OK to require when other people do
not want it. I think that's just lazy, and not the smart kind. :\
Some people do not want ramstage. Some people do not want blobs. Some people do not want x86 support. They still carry the baggage when downloading coreboot.
Patrick
Patrick Georgi via coreboot wrote:
It's absurd to me that coreboot would require any routines out of any submodule for a build which will not use those routines.
coreboot doesn't, cbfstool does.
If that were the case things would already be a lot better!
Alas, coreboot unconditionally requires vboot in these files:
Makefile.inc src/commonlib/bsd/cbfs_private.c src/commonlib/bsd/include/commonlib/bsd/cbfs_private.h src/commonlib/bsd/include/commonlib/bsd/cbfs_serialized.h src/commonlib/cbfs.c src/commonlib/include/commonlib/cbfs.h src/include/memlayout.h src/lib/bootmode.c
There are additional, more or less target-specific, dependencies in:
src/drivers/intel/fsp2_0/memory_init.c src/soc/amd/picasso/psp_verstage/psp_verstage.c src/soc/intel/common/block/cse/cse_lite.c src/soc/qualcomm/common/qclib.c src/soc/rockchip/rk3288/crypto.c
The hard requirements increase over time. Two years ago they were:
Makefile.inc src/commonlib/cbfs.c src/commonlib/include/commonlib/cbfs.h src/lib/bootmode.c src/security/vboot/Makefile.inc src/southbridge/intel/common/pmbase.c
In addition to that, since every coreboot build does require cbfstool (no problem per se) the indirect requirement through cbfstool also counts, as long as cbfstool can only be built to support everything.
One purpose of Kconfig is to ensure that only what's neccessary gets built.
But cbfstool isn't hooked up to Kconfig. Given that it's not part of the final coreboot build, having extra stuff in cbfstool doesn't affect coreboot in the slightest, so it's not clear to me that we should change that.
Maybe we view Kconfig differently. I expect it to control not only the final build artefact but also the build process, meaning what gets built and what is needed for successful build.
It's wrong to pull in anything during build. I too am guilty of this by pushing for buildgcc, it would be good to improve that case too.
Given your reference to buildgcc, I guess you mean "download"? In that case: git clone --recurse-submodules and there's not a single extra download going on at build time.
Right, but maybe we at least agree that requiring some submodule(s) for nothing is at a minimum unneccessary?
It is because that's what consistently causes me extra work and
frustration every time I want to build a minimal coreboot.
git clone --recurse-submodules is extra work, really?
I don't want any submodules, so I go over the source and remove the requirement.
What some people always want isn't OK to require when other people do not want it. I think that's just lazy, and not the smart kind. :\
Some people do not want ramstage. Some people do not want blobs. Some people do not want x86 support. They still carry the baggage when downloading coreboot.
Indeed I think that coreboot has a lot of unneccessary baggage, like this vboot requirement. It seems that it's rather one of a few cases of this type of problem, but unneccessary dependencies are never good.
//Peter
Am Di., 17. Nov. 2020 um 18:54 Uhr schrieb Peter Stuge peter@stuge.se:
Patrick Georgi via coreboot wrote:
coreboot doesn't, cbfstool does.
If that were the case things would already be a lot better!
Alas, coreboot unconditionally requires vboot in these files:
Oops, I forgot about those, you're right!
So: we discussed that in today's meeting and had two take-aways:
1. people have issues with older host toolchains failing to build vboot. We'll work on improving those scenarios.
2. We generally prefer to build our utilities fully featured to prevent partial feature sets from popping up in installed binaries. That said, if there were a patch that cleanly cuts out cbfs hashing support from coreboot (and cbfstool used for building coreboot) based on a Kconfig flag, we would consider it.
"Cleanly" meaning: - It needs to be optional - The result needs to be maintainable. Things shouldn't break apart when looking at the flag funny - cbfstool should behave properly and reasonably when built without hashing but the relevant options are used (that is: say that it's a stripped down build, not just "command line error") - cbfstool and cbfs in coreboot without those flags still need to be able to deal with hash attributes sanely, that is, skip them safely. I don't expect that to be an issue (the data structures are robust enough), but something to keep in mind.
Maybe we view Kconfig differently. I expect it to control not only the
final build artefact but also the build process, meaning what gets built and what is needed for successful build.
I'm not entirely happy about the way we use Kconfig to enable ccache (to pick an example) because IMHO changes to config.h should lead to a different coreboot build (outside config.h). I guess with this "feature", the build would be different, so this satisfies my personal criterion.
Right, but maybe we at least agree that requiring some submodule(s) for nothing is at a minimum unneccessary?
As I mentioned elsewhere, we could import vboot as a git subtree (even though I wouldn't recommend it). That changes next-to-nothing for users (but makes life hell for developers), but satisfies that criterion. So, why the hate on submodules?
I don't want any submodules, so I go over the source and remove the
requirement.
I lined out how that could look like above. (Good) Patches accepted.
Patrick
Hello Patrick,
I'm a bit concerned about the tone of your email. Maybe you were just reflecting the meeting literally. In that case, sorry in advance.
In several spots you seem to imply that Peter would settle for less than what is expected. I have no idea where that comes from. Also, I don't remember a list of requirements when the vboot dependency was introduced.
The vboot dependency has been a PITA for a while. I'll happily accept patches that make it less of a pain even if that means a little more maintenance effort. I'd even accept a local hash implementation. Some- where the argument was made that it would be bad to have additional implementations. But I wonder, if that were a policy, would vboot have such implementations? I'm sure they weren't the first. Maybe there were even concerns about external code?
Nico
Am Mi., 18. Nov. 2020 um 22:03 Uhr schrieb Nico Huber nico.h@gmx.de:
The vboot dependency has been a PITA for a while. I'll happily accept patches that make it less of a pain even if that means a little more maintenance effort. I'd even accept a local hash implementation.
That's an option. That isn't what was proposed though. The proposal was "I don't need this, it annoys me, let's drop it".
But I wonder, if that were a policy, would vboot have
such implementations? I'm sure they weren't the first. Maybe there were even concerns about external code?
Suitable license (rules out everything GNU for GPL3+, OpenSSL + offspring for their advertising clause or tomcrypt for not having a license), somewhat recently maintained (rules out libtomcrypt and SPARK crypto), suitable for embedded purposes (rules out Java implementations). Exactly the issues coreboot would face when selecting an implementation to copy. Just that by the time coreboot had to consider hashing data, vboot existed, it ticked the right boxes, and some people with overlap to coreboot were familiar with it.
Patrick
CONFIG_VBOOT enables vboot, the verified boot scheme. The issue here is the submodule, which is drawn in through CONFIG_VBOOT_LIB.
Why is this an issue? CONFIG_VBOOT_LIB isn't set either! Furthermore it is pretty clear to me that if CONFIG_VBOOT is not set, then no vboot related code should be generated (that also includes code ifdef guarded by CONFIG_VBOOT_LIB).
The real problem here is, because of the poor code quality of vboot integration, it doesn't matter what you have in Kconfig, you cannot compile coreboot without the vboot submodule.
That is the actual issue that needs to be addressed.
CBFS hashing (which has nothing to do with verified boot right now, although that could change).
If that's true, then why does cbfs_private.h include vb2_sha.h header at all? Let's just remove vboot dependency altogether, problem solved (see below).
that doesn't solve the problem that cbfstool has its own CBFS implementation (so it also needs to be ifguarded that way, which is a bit annoying because util/* doesn't use Kconfig at this time)
Which could be simply solved by adding something like this to the Makefile (but see below):
ifneq ($(wildcard ../3rdparty/vboot/*),) CFLAGS += -DCONFIG_VBOOT_LIB endif
That's it! You're making this a lot more complicated than actually is. Don't overcomplicate things, look for the simplest solution!
I'd even accept a local hash implementation.
Big plus one to this!
An sha checksum code is pretty small (an sha265 implementation being ca. 50-60 SLoC, no more). And it *does not need* any maintenance (if it provides the correct results for the test vectors, there's no reason to modify it, ever). It is pretty obvious that it's a lot more complicated to maintain vboot integration than having a dependency-free cbfstool.
I'm staring to have a feeling this isn't a technical issue but a political one, which I don't want to participate in. There are simple and easy solutions. I've recommended some, take it or leave it, but I'm out.
Have a nice day bzt
On 11/18/20, Patrick Georgi via coreboot coreboot@coreboot.org wrote:
Am Mi., 18. Nov. 2020 um 22:03 Uhr schrieb Nico Huber nico.h@gmx.de:
The vboot dependency has been a PITA for a while. I'll happily accept patches that make it less of a pain even if that means a little more maintenance effort. I'd even accept a local hash implementation.
That's an option. That isn't what was proposed though. The proposal was "I don't need this, it annoys me, let's drop it".
But I wonder, if that were a policy, would vboot have
such implementations? I'm sure they weren't the first. Maybe there were even concerns about external code?
Suitable license (rules out everything GNU for GPL3+, OpenSSL + offspring for their advertising clause or tomcrypt for not having a license), somewhat recently maintained (rules out libtomcrypt and SPARK crypto), suitable for embedded purposes (rules out Java implementations). Exactly the issues coreboot would face when selecting an implementation to copy. Just that by the time coreboot had to consider hashing data, vboot existed, it ticked the right boxes, and some people with overlap to coreboot were familiar with it.
Patrick
Google Germany GmbH, ABC-Str. 19, 20354 Hamburg Registergericht und -nummer: Hamburg, HRB 86891, Sitz der Gesellschaft: Hamburg Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
I think I should continue here, sorry that my previous response was to gcc requirements.
David Hendricks wrote:
Can you be more specific? I ran thru the instructions and built for a qemu target and did not run into any problems
Here are the commands that I've used. Without submodules, coreboot won't compile. The only reason for this is the badly integrated vboot submodule, nothing else. All the other submodules are optional.
$ git clone https://review.coreboot.org/coreboot $ cd coreboot $ make crossgcc-i386 CPUS=2 $ make -C payloads/coreinfo olddefconfig $ make -C payloads/coreinfo $ make menuconfig
(here I've added coreinfo payload)
$ make safedefconfig $ cat defconfig CONFIG_PAYLOAD_ELF=y CONFIG_PAYLOAD_FILE="payloads/coreinfo/build/coreinfo.elf" $ cat .config | grep VBOOT # CONFIG_VBOOT is not set CONFIG_VBOOT_VBNV_OFFSET=0x2c $ make
(... removed for clearity ...)
FMAP build/util/cbfstool/fmaptool -h build/fmap_config.h build/fmap.fmd build/fmap.fmap SUCCESS: Wrote 182 bytes to file 'build/fmap.fmap' (and generated header) The sections containing CBFSes are: COREBOOT CP bootblock/arch/x86/memlayout.ld In file included from src/arch/x86/memlayout.ld:3: src/include/memlayout.h:9:10: fatal error: vb2_constants.h: No such file or directory #include <vb2_constants.h> ^~~~~~~~~~~~~~~~~ compilation terminated. make: *** [Makefile:362: build/bootblock/arch/x86/memlayout.ld] Error 1 $
(here I've simply commented out the include inside memlayout.h for a quick test)
$ make GEN build.h CC bootblock/arch/x86/id.o CP bootblock/arch/x86/memlayout.ld CC bootblock/arch/x86/memmove.o CC bootblock/arch/x86/memset.o CC bootblock/arch/x86/mmap_boot.o CC bootblock/arch/x86/post.o CC bootblock/arch/x86/timestamp.o CC bootblock/commonlib/bsd/cbfs_private.o In file included from src/commonlib/bsd/include/commonlib/bsd/cbfs_private.h:8, from src/commonlib/bsd/cbfs_private.c:3: src/commonlib/bsd/include/commonlib/bsd/cbfs_serialized.h:7:10: fatal error: vb2_sha.h: No such file or directory #include <vb2_sha.h> ^~~~~~~~~~~ compilation terminated. make: *** [Makefile:362: build/bootblock/commonlib/bsd/cbfs_private.o] Error 1 $
I'd like put emphasis on this part:
$ cat .config | grep VBOOT # CONFIG_VBOOT is not set
and on the fact by removing the include from memlayout.h solved the issue and the compilation went a step further. This means that include clearly doesn't belong there.
David Hendricks wrote:
As Julius mentioned this is possible but it's a fair bit of work with little benefit.
Patrick Georgi wrote:
I lined out how that could look like above.
I believe you are both unnecessarily overcomplicate this. The way I see it the only issue here is a few missing ifdef guards for CONFIG_VBOOT in cbfs, that's all. Quite straightforward to solve.
Forcing vboot on users is not the right answer, imho. Integrating vboot with proper ifdef guards is. But that's just my two cents.
Have a nice day, bzt
On 11/18/20, Patrick Georgi via coreboot coreboot@coreboot.org wrote:
Am Di., 17. Nov. 2020 um 18:54 Uhr schrieb Peter Stuge peter@stuge.se:
Patrick Georgi via coreboot wrote:
coreboot doesn't, cbfstool does.
If that were the case things would already be a lot better!
Alas, coreboot unconditionally requires vboot in these files:
Oops, I forgot about those, you're right!
So: we discussed that in today's meeting and had two take-aways:
- people have issues with older host toolchains failing to build vboot.
We'll work on improving those scenarios.
- We generally prefer to build our utilities fully featured to prevent
partial feature sets from popping up in installed binaries. That said, if there were a patch that cleanly cuts out cbfs hashing support from coreboot (and cbfstool used for building coreboot) based on a Kconfig flag, we would consider it.
"Cleanly" meaning:
- It needs to be optional
- The result needs to be maintainable. Things shouldn't break apart when
looking at the flag funny
- cbfstool should behave properly and reasonably when built without
hashing but the relevant options are used (that is: say that it's a stripped down build, not just "command line error")
- cbfstool and cbfs in coreboot without those flags still need to be able
to deal with hash attributes sanely, that is, skip them safely. I don't expect that to be an issue (the data structures are robust enough), but something to keep in mind.
Maybe we view Kconfig differently. I expect it to control not only the
final build artefact but also the build process, meaning what gets built and what is needed for successful build.
I'm not entirely happy about the way we use Kconfig to enable ccache (to pick an example) because IMHO changes to config.h should lead to a different coreboot build (outside config.h). I guess with this "feature", the build would be different, so this satisfies my personal criterion.
Right, but maybe we at least agree that requiring some submodule(s) for nothing is at a minimum unneccessary?
As I mentioned elsewhere, we could import vboot as a git subtree (even though I wouldn't recommend it). That changes next-to-nothing for users (but makes life hell for developers), but satisfies that criterion. So, why the hate on submodules?
I don't want any submodules, so I go over the source and remove the
requirement.
I lined out how that could look like above. (Good) Patches accepted.
Patrick
Google Germany GmbH, ABC-Str. 19, 20354 Hamburg Registergericht und -nummer: Hamburg, HRB 86891, Sitz der Gesellschaft: Hamburg Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Am Mi., 18. Nov. 2020 um 22:15 Uhr schrieb bzt bztemail@gmail.com:
I believe you are both unnecessarily overcomplicate this. The way I see it the only issue here is a few missing ifdef guards for CONFIG_VBOOT in cbfs, that's all. Quite straightforward to solve.
CONFIG_VBOOT enables vboot, the verified boot scheme. The issue here is the submodule, which is drawn in through CONFIG_VBOOT_LIB. Besides vboot, other users of it are: the TPM drivers, Eltan's mboot, AMD PSP verstage, Intel CSE lite, and CBFS hashing (which has nothing to do with verified boot right now, although that could change).
And even if "just ifdef stuff in CBFS with CONFIG_VBOOT_LIB" creates a working image, that doesn't solve the problem that cbfstool has its own CBFS implementation (so it also needs to be ifguarded that way, which is a bit annoying because util/* doesn't use Kconfig at this time), and with just "ifguarding things", there's some work left to do to handle "cbfstool coreboot.rom add -A sha256 ..." properly: should it error out generically (as if -A is unknown)? provide a useful error message? just ignore the flag?
It's not quite that straightforward.
Patrick
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.
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 jwerner@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-leave@coreboot.org
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.
We are not blindly taking code from github, sourceforge, etc. All submodules are hosted on coreboot.org and the main repo points at the SHA it's using, and have the same guarantees as code in the main coreboot repo.
This example might help illustrate it: libgfxinit submodule: https://review.coreboot.org/plugins/gitiles/libgfxinit When somebody wants to use point at a new version: https://review.coreboot.org/c/coreboot/+/43559
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.
Can you be more specific? I ran thru the instructions and built for a qemu target and did not run into any problems - it cloned the various submodule repositories as expected and the build worked fine.
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.
Sort of, yes. As Julius mentioned this is possible but it's a fair bit of work with little benefit. The common code which uses vboot has to do with cbfs hashing. I personally think of it as a good feature, though I can see arguments for not wanting it as well.
The submodule should be downloaded automatically and does not have a restrictive license, unlike some other submodules that the user must opt-in to. Is there a desire to remove hashing functionality in cbfs, or is the problem here just the fact that the hashing library is part of a submodule?
Am Mi., 18. Nov. 2020 um 09:14 Uhr schrieb David Hendricks < david.hendricks@gmail.com>:
or is the problem here just the fact that the hashing library is part of a submodule?
If it's the submodule that is in question here, we _could_ import vboot as a subtree (and compatibly, too!), although that will create a real mess of our repo history because we'll have two git histories merged in one. I would not recommend going down that route.
Patrick
Hi all,
On 12.11.20 03:29, Julius Werner wrote:
I don't think it's fair to single in on vboot as the big problem here.
it's not by any definition but, FWIW, it became one in practice. I don't think it's because of GCC versions or anything that we should try to fix by testing. One big difference to other build-time dependencies (i.e. what we pull in via buildgcc) is that vboot is not (as much) focused on portability.
Julius also mentioned elsewhere that the same issues that can sneak into vboot can hit us anywhere else. That's generally true but, IMHO, highly unlikely. When we work on central parts of the build process (e.g. cbfs- tool) and review patches there, we take care of portability and keep it to standard C, FWIW. (I know people like to use packed structs, but that's not too funky, I guess.)
There's another issue, once one tries to fix a problem. If it occurred somewhere in our tree, it can be fixed within the hour. But if it's ex- ternal, it needs to be upstreamed there first, then a complete new state of the external software needs to be tested and pass Jenkins and then the submodule pointer can be updated. I never tried but a random guess: for vboot it takes a month on average maybe? That's not on vboot, I know it's a general submodule issue.
Just some thoughts... Nico
Hi Julius,
On 12.11.20 03:29, Julius Werner wrote:
So I think the "official" rule is basically that the minimum requirement for host utilities is the same compiler and version that crossgcc uses, which I think makes some amount of sense (otherwise we'll just have to keep tracking and deploying two separate versions).
I think it's the opposite. Whenever I worked on buildgcc the goal was to not assume a minimum GCC version. In fact, not even assume GCC at all.
I don't think it's hard to achieve the same for our build tools. We might want to make it easier to build them without treating warnings as errors (if we do), but otherwise I don't see any obstacle. It's just simple command line tools after all.
If you guys want to change that we can have that discussion, but
Change what? Your idea about the "official" rule?
whatever we decide it's probably not going to be a very effective decision unless someone puts in the work to really make Jenkins test that continuously. And either way, I don't think any of this is vboot's fault in particular (vboot uprevs are tested by Jenkins just as much as any other patch and have to pass any compatibility tests enforced there).
I also think we should test a little more. But portability is nothing one can test exhaustively. IMHO, we should rather try to keep the code simple enough for a compiler to understand.
Nico
Having an undocumented (or silently installed, therefore unexpected) dependency is undesirable (especially for a firmware), no question about that.
Sorry, I still get the impression that there is a fundamental misunderstanding about what Git submodules are here. It's *just* source code! It's source code that comes cryptographically guaranteed from the main coreboot Git repository, exactly like all the normal coreboot source code. Nothing is "silently installed" on your system here or anything. It's just a way to organize some source code as a separate unit that can manually be resynced with changes from another upstream. It's not really different from what we did for LZ4 compression code in cbfstool, except that it makes organization and regular updates easier than copying in the whole thing directly. And I don't know what you mean by "undocumented", we have more documentation on the vboot stuff than most parts of coreboot (see Documentation/security/vboot... also, there's Documentation/getting_started/submodules.md).
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.
Yes, because the Git SHA of the submodule HEAD is stored in the main coreboot repo. I explained this in my last mail already. This is not node.js, if the submodule changes externally that will not affect coreboot's use of it until a coreboot developer intentionally pushes an update to the coreboot repository itself.
Julius also mentioned elsewhere that the same issues that can sneak into vboot can hit us anywhere else. That's generally true but, IMHO, highly unlikely. When we work on central parts of the build process (e.g. cbfstool) and review patches there, we take care of portability and keep it to standard C, FWIW. (I know people like to use packed structs, but that's not too funky, I guess.)
No, I don't think most people submitting to cbfstool are somehow applying some magic unwritten portability standard that vboot is lacking to be honest. And if they do, and you can tell me what it is, we are happy to apply the same standard to vboot going forward. Like I said earlier, I am perfectly happy to align vboot standards and practices to coreboot to solve this kind of interworking friction, the problem is just that *we don't have any standards*! I mean, it's not like we somehow blindly added __attribute__((fallback)) to vboot thinking it was standard C89, we were fully aware that this was a somewhat modern feature in GCC and clang, and everyone we talked to back then (I think this was on Gerrit somewhere, not sure how to find it right now, sorry) agreed that this was fine and that aligning HOSTCC requirements to crossgcc made sense.
Since apparently enough people here feel that it's a big problem all of a sudden (even though I think it landed close to a year ago) I'm going to take it out again now and then hopefully we can move on. And if people are concerned about getting hit by problems like this in the future (both in submodules and normal coreboot code) it would be great if someone could just put something in Documentation/ to define what we're intending to support, so we don't need to have discussions again about what some but clearly not all people might maybe be looking out for in reviews if they happen to spot it. I don't think "just write it fully portable" is a reasonable goal since clearly we need __attributes__ like ((packed)), and there are other common extensions without which writing C code is just a huge pain in general (e.g. compound statements for double-evaluation safe MIN()/MAX()), so I'd prefer if we could just pick a minimum GCC and clang version number. And then maybe one day we could even get Jenkins to test that.
(Also, while we're on the subject of submodules causing pain, Nico... whenever I want to build test older Intel generations I have to first figure out again which of them don't rely on libgfxinit, or how to hack around in their Kconfigs to disable it, because unlike everything else you need to build coreboot there seems to be no way to get an ADA toolchain from crossgcc. All the problems we have been discussing in this thread can be worked around easily (for 99% of people's host machines, at least) by putting a simple CC=util/crossgcc/xgcc/bin/x86_64-elf-gcc on the command line, but if you try to build in an environment that doesn't bring its own ADA compiler you're just SOL. So I really don't think vboot deserves the award for most cumbersome submodule right now.)
Am Mi., 18. Nov. 2020 um 23:54 Uhr schrieb Julius Werner < jwerner@chromium.org>:
because unlike everything else you need to build coreboot there seems to be no way to get an ADA toolchain from crossgcc.
gnat (gcc's Ada implementation) needs an Ada implementation to bootstrap (just like gcc needs a C++ compiler). If you have gnat[0] installed on the host, you also get a gnat for the targets. (side note: The chromium-os coreboot-sdk package uses a binary toolchain from AdaCore for the bootstrap, so within cros_sdk the cross compilers are all Ada-aware.)
But yes, it's not exactly obvious.
Patrick [0] conditions apply: it needs to be the same build as gcc so that they can interact (some distros mess this up) and it must be sufficiently new (although we integrated a bunch of hacks to support older compilers than usual)
Julius Werner wrote:
Having an undocumented (or silently installed, therefore unexpected) dependency is undesirable (especially for a firmware), no question about that.
Sorry, I still get the impression that there is a fundamental misunderstanding about what Git submodules are here.
I know how submodules work, I believe Nico too. I also know the internal git data model very well.
For me this isn't really a matter of automation or trust but simply one of avoiding complexity where it isn't mandated.
It's source code that comes cryptographically guaranteed
Sure, but more source code and in particular across more repositories is a lot more complexity than less source code in a single repository.
I appreciate that my concern, if pointedly articulated, was considered and found to be valid. I don't believe that it's very complex to find a good solution, but you can maybe understand that I think it unfair that you invite me to provide the remedy for a problem that I didn't create.
Let's see how it goes.
the Git SHA of the submodule HEAD is stored in the main coreboot repo.
My argument is solely on complexity, but please don't trust that hash too much.
No, I don't think most people submitting to cbfstool are somehow applying some magic unwritten portability standard that vboot is lacking to be honest.
The lower common denominator one targets the more effort is required, and different contributors are likely to trade off differently here, I think we've all seen corporate contributions favor raw development speed over everything else in many projects and I guess coreboot has this happen too from time to time.
And if they do, and you can tell me what it is, we are happy to apply the same standard to vboot going forward.
I think that's a fantastic promise! I also understand and agree with your request for standardization/documentation, something to set expectations, that's 100% reasonable.
I don't think "just write it fully portable" is a reasonable goal since clearly we need __attributes__ like ((packed)),
I disagree about packed - I think the argument for (de)serialization is quite strong - but I understand that this may not be universal, and it trades development effort for portability.
libgfxinit
I think that's a fair point as well - that has also tripped me up - but it's been less severe for me than the hard vboot requirement.
So I really don't think vboot deserves the award for most cumbersome submodule right now.
I can't say that it is - only that it's bothered me for a while.
I'm happy that there's an outlook at least, but obviously I'd rather not have to fix a problem I didn't cause. Let's see.
Thanks everyone so far
//Peter
Having an undocumented (or silently installed, therefore unexpected) dependency is undesirable (especially for a firmware), no question about that.
Sorry, I still get the impression that there is a fundamental misunderstanding about what Git submodules are here.
I know how submodules work, I believe Nico too. I also know the internal git data model very well.
Sorry, there were multiple streams of discussion in here, I just want to make sure we're all on the same page on what it actually is. I felt the characterization of "silently installed" that I quoted made it sound like something else.
the Git SHA of the submodule HEAD is stored in the main coreboot repo.
My argument is solely on complexity, but please don't trust that hash too much.
I was trying to say that it is just as secure and trustworthy as code stored in the main Git repository. I don't think we need to get into the details of SHA1 collision resistance here, it's just how Git works right now for everything it does (submodules and normal files). I just wanted to clarify that there should be no security (or availability) concerns with this "dependency" because that was mentioned somewhere above.
Sure, but more source code and in particular across more repositories is a lot more complexity than less source code in a single repository.
I mean, the specific functions that cbfstool uses from vboot are not very complex at all (and since it's a submodule they're right there, you can just look at them, they're just stored under 3rdparty/ instead of under src/). And the main reason we're doing this is to avoid reimplementing the same thing in coreboot again, which would add a lot more complexity to builds that are using vboot for verification -- not only would you have the same algorithm twice, there would also likely be API differences (e.g. how hashes are handled or hash types are encoded) that then require complicated translation whenever coreboot and vboot parts try to talk to each other. You can basically: a) have a coreboot hash API and a vboot hash API, and coreboot code calling into vboot will need to do a lot of translation, or b) have a coreboot hash API for other stuff but use the vboot API for vboot-specific coreboot code, meaning that we'd confusingly use two separate hash APIs within coreboot (and sometimes they may still need to interwork and require translation), or c) just make the vboot API available to all of coreboot and use it everywhere. I really think that last one ends up being the least complex and confusing, overall.
And if they do, and you can tell me what it is, we are happy to apply the same standard to vboot going forward.
I think that's a fantastic promise! I also understand and agree with your request for standardization/documentation, something to set expectations, that's 100% reasonable.
Like I said in my first mail here, I'm really not trying to make this hard for you! I'm really trying to keep things smooth and problem-free both for the people who do and especially also for those who don't want to use vboot stuff. Just please let me find other ways to do that rather than having to hide every small use of utility functions behind a hard switch to turn it off, because in practice that leads to a lot of detail problems that make things very hard to implement and maintain. We should be able to integrate this such that it will just build and link quietly in the background without causing anyone any troubles, same as all of the other support code that's in the main coreboot repository. This thread was the first time I heard that the __attribute__((fallthrough)) is causing a lot of people pain, and that's an absolute non-issue to fix... just give me a day or two and it'll be gone. If there are more issues causing pain with any specific compiler or build environment, let me know and I'll try to help.
Am Do., 19. Nov. 2020 um 01:26 Uhr schrieb Peter Stuge peter@stuge.se:
the Git SHA of the submodule HEAD is stored in the main coreboot repo.
My argument is solely on complexity, but please don't trust that hash too much.
If I shouldn't trust "160000 commit 4c523ed10f25de872ac0513ebd6ca53d3970b9de vboot" too much, why should I trust "040000 tree 4c523ed10f25de872ac0513ebd6ca53d3970b9de vboot" (where the repo referred to through the "commit" entry comes from the very same server)?
And this isn't a rhetorical question, I _really_ don't get that point.
Patrick
Patrick Georgi via coreboot wrote:
My argument is solely on complexity, but please don't trust that hash too much.
If I shouldn't trust "160000 commit 4c523ed10f25de872ac0513ebd6ca53d3970b9de vboot" too much, why should I trust "040000 tree 4c523ed10f25de872ac0513ebd6ca53d3970b9de vboot" (where the repo referred to through the "commit" entry comes from the very same server)?
Let's say that you've audited the files some time in the past, found them to be good, and have noted down the hash to catch obvious repo tampering or changes in the submodule commit, saying to audit anew.
If you later need to re-fetch the submodule contents (maybe in a local clone into a new directory) then merely the hash is not very reliable. SHA-2 would be a lot better than SHA-1, which is in turn a lot better than MD5, but just a hash is a lot weaker than the original audit.
//Peter
Am Do., 19. Nov. 2020 um 18:32 Uhr schrieb Peter Stuge peter@stuge.se:
Patrick Georgi via coreboot wrote:
My argument is solely on complexity, but please don't trust that hash
too
much.
If I shouldn't trust "160000 commit 4c523ed10f25de872ac0513ebd6ca53d3970b9de vboot" too much, why should I trust "040000 tree 4c523ed10f25de872ac0513ebd6ca53d3970b9de vboot" (where the repo referred to through the "commit" entry comes from the very same server)?
Let's say that you've audited the files some time in the past, found them to be good, and have noted down the hash to catch obvious repo tampering or changes in the submodule commit, saying to audit anew.
Then you rely on a hash (which one, commit? That's SHA1 of a collection of SHA1s for files, directories and submodules) for your certification. That's true no matter what kind of object those SHA1s represent.
If you create your own hash for the tree, you can as well create a hash of everything (minus .git which has an unstable representation) which conveniently includes any checked out submodules.
If you later need to re-fetch the submodule contents (maybe in a
local clone into a new directory) then merely the hash is not very reliable. SHA-2 would be a lot better than SHA-1, which is in turn a lot better than MD5, but just a hash is a lot weaker than the original audit.
Which is exactly why git is moving to SHA2 now, but that critique is more one of git, so if you don't trust SHA1, don't use git?
Patrick
On 18.11.20 23:53, Julius Werner wrote:
(Also, while we're on the subject of submodules causing pain, Nico... whenever I want to build test older Intel generations I have to first figure out again which of them don't rely on libgfxinit, or how to hack around in their Kconfigs to disable it, because unlike everything else you need to build coreboot there seems to be no way to get an ADA toolchain from crossgcc. All the problems we have been discussing in this thread can be worked around easily (for 99% of people's host machines, at least) by putting a simple CC=util/crossgcc/xgcc/bin/x86_64-elf-gcc on the command line, but if you try to build in an environment that doesn't bring its own ADA compiler you're just SOL. So I really don't think vboot deserves the award for most cumbersome submodule right now.)
Well, if you want us to pull our pants down and compare... numbers. I see 202 boards + variants in the tree that depend on vboot. There should be exactly 0 boards in the tree where you can't opt out from Ada. If you found some, please just tell me. Because I'm well aware that it's not 100% convenient and don't just pretend to but actually do take care that no hard dependency sneaks in.
Nico
PS. The anger aside, if you have a problem because your host GCC comes without Ada support, I'm happy to help. There are few distributions left that bootstrap their compiler without Ada support. I am only aware of reports from Gentoo users.
-
Branden Waldner -
bzt -
Clay Daniels -
David Hendricks -
Julius Werner -
Nico Huber -
Patrick Georgi -
Peter Stuge