WiP change Intel PHYsical mac (gbe) from Heads #1195
Conversation
|
the flash rom gui menu could use a tweak already IMO. i would like to see all options merged into submenus. It would be nice have it go like this: Flash New Rom > GPG Settings > MAC Settings > Mount USB and Select rom/GPG > Confirmation first option on both prompts retains settings, making routine updates as simple as possible. maybe worth noting that the "add to standalone bios and re-flash" options in the existent gpg options menu could be removed as redundant if the Flash menu gets retooled this way. |
|
@tlaurion I like this approach. I would be concerns that if a user flashed a maximized version over the standard HEADS rom then modified the MAC address before rebooting then the depending on if flashrom read the IFD from the buffer or from the chip may write to the wrong location. This would prevent that. An input mask allowing only hex-ish characters ( |
|
i try to be as correct as possible with what status is returned on exit from nvmutil. you can pretty much just run it and do || in your shell script to make it crap out if there's a problem (e.g. invalid checksum, invalid mac address) etc so like, your build system would let the user specify a mac address and your build system could just... ./nvm gbe.bin setmac macaddresshere || exit 1 or whatever you want after that || that's the way i do it in osboot, the simple way. fail early, fail hard is the way i generally do things |
|
by the way, this should work on basically all intel ifd based machines, all those gbe regions have the same layout in them, and nvmutil intentionally dosen't set anything unless the checksum is correct, to guard against a future possibility that intel changes it; presumably they'd use a different checksum validation then, or maybe put it in a different place but it should work just fine on all the thinkpads you support. works on x200/t400 etc aswell. i also use it on t440p machines |
|
i actually checked linux src and pretty much all these nics use that same algorithm for checksum: the words must add up to 0xBABA, and mac address is first 3 words (linux verifies the checksum before doing anything with the nic, and will actually refuse to set up your nic if both checksums are bad) |
|
btw small nitpick it's call nvmutil, not nvmtool |
tlaurion
force-pushed
the
igbe_nvmtool_ifdtool_addition
branch
2 times, most recently
from
01da904 to
78af41d
Compare
@Thrilleratplay : Yeah thought about that. We could wrap around checking the output of flashrom -p internal output to see if no warning are given there.
@Thrilleratplay : We could, or we could base ourselves on the validation made by nvmutil (nv). I prefer the latest. Long term (PR welcome) we could also borrow other MAC randomization tweaks. Some router will not permit some non existing OUI. So it would be logical to randomize only in the "Intel Corp" OUI space but there I do not want to go beyond user's needs?
@githubisnonfree : The goal here is to build against musl-cross-make in the firmware and provide tools for users to change PHY whenever they want and flash back internally from flashrom. We could permit the user to build a ROM with a custom PHY, but that would go against reproducible goal and mode support. Unfortunately in Heads current use case, since we build ifdtool and nvutil from musl-cross-make to be packed in rom (against musl libc) we could document the process of building from the builder machine, but I kinda not see the point if the firmware is doing the proper thing, that being to pack a gbe.bin that is setuped as "DE:AD:C0:FF:EE" in the goal of having everyone having maximized builds to have reproducible roms in the long term, even if those ROMs contain a GBE, ME, IFD and BIOS regions. And have the user decide if he wants to change it. Let's not forget that user's are better having a randomized MAC setuped through NetworkManager nowadays, this feature is mainly aimed as a safeguard, and for Heads users having multiple machines to be able to connect multiple machines on LAN if they do not apply in software mac randomization. The problem I foresee in the future is that Coreboot is starting to measure other components. 4.17 is now measuring CBFS content, and ME region is desired to be measured since forever. I do not see why GBE (and everything Intel IFD) not being measured at some point. That will modify PCR2 as of now, and in the future we will need to do some tpmfuturecalc to take that into consideration for future upgrades.... Should probably be considered in other future issues, just a note.
Unfortunately, I am not sure this would please all stake holders here, me not being convinced with the approach either, since those actions are normally desired separately, and one rarely needs to change public key (and if he desires so, can do it under GPG options). One that wants to change all of this would most definitely simply flash a clean rom without keeping settings and hit the OEM Factory reset/Re-Ownership wizard to accomplish all of this, minus the PHY MAC changing that is subjected here. @Thrilleratplay @githubisnonfree @weyounsix
That would not require any other logic to be modified, would have the MAC persist between firmware upgrades and let users decide if they want to change their mac address? Changing menu and orders of menu options is a complicated matter (check past PRs), where Purism and other boards constructors might not even have a Compatible Intel Ethernet card (those connectors tend to disappear, and if Ethernet port is there, they are not necessarily Intel made). I prefer small approach and move from there. As far as this PR goes, it is actually feature complete (but xx20 and xx30 board configurations should add the feature disabled). So basically, next steps prior of being even able to merge this is:
|
|
Some notes on actual problems that won't permit t420/x220/w520 to be usable. Master build log on x220-maximized: This PR x220-maximized build log: So the addition of ifdtool alone consumes from the free space available: 71192 - 54808 = 16384 bytes x220-hotp-maximized build log from this PR Only has 47064 bytes free after ifdtool inclusion from global Makefile change. So merging this PR would be the first case where xx30 and xx20 would differ in features that can be offered. More direct todos:
|
tlaurion
force-pushed
the
igbe_nvmtool_ifdtool_addition
branch
from
78af41d to
6283a3f
Compare
|
why not just have a script download it into tmpfs and run it (after checking signatures) also, i've recently started porting openbsd userland to linux+musl, i'm months away from completing it probably but i want to replace busybox. when you take all the extra non-posix stuff away until you're left with only the same features as busybox, openbsd code is actually smaller in almost every case i've checked - even ksh is the same size as busybox sh, and that's with all the bells and whistles i hope to have something to show later this year, that people can really use on their machines. i'm doing it for my own linux i'm working on, but you can be sure i'll be showing it to heads first: i'm doing it because i have the same problem as you: limited flash space. i want a flash distro for osboot, like heads though i'm basing my work off of alpine locally. i'll release later in the year when i have something complete |
|
have a look at chimera linux, they have a few nice things in it in particular, they have almost (if not entirely, haven't looked in great detail yet) a complete freebsd-based userland on top of linux and musl they recently ported freebsd sh to linux freebsd sh is about 12k source lines of code, busybox sh is like 19k last time i checked. you might find more gold like that in chimera linux to try out in your project here's posix cat for free: (this is my one, after stripping down the openbsd one) edit: and to be clear i'm recommending this because i'm aware your distro has the problem: 8MB flash setups have limited feature sets due to low flash space. i'm told alpine linux is also working on their own userland but i have no idea about the status of that |
|
also, i've been experimenting with various compilers and i've found that in a lot of cases, tcc produces smaller binaries than gcc or clang in cases where gcc wins (if one of them beats tcc, it's always gcc) like with -Os and LTO, that's uncompressed binaries, those tcc-built binaries when compressed (e.g. in CBFS with LZMA!) really get quite small indeed freebsd sh (alone) can be made to go down to about 150KB potentially and that's when i built for x86_64, for i686 you can probably get much smaller, whereas busybox ash probably can't get much smaller than say 230KB. these are uncompressed sizes btw |
|
how small? well for a program that is about 300KB compiled with GCC or clang, it's not uncommon for me to see that tcc is about 150KB, half the size, and that's with stuff like -Os and -flto used in GCC you should look into using tcc if you haven't already. it could really save a lot of space in rom when putting all these utils together. tcc is great (also called tinycc) |
|
@githubisnonfree all those comments unfortunately will be lost under a PR once merged. I think you of all other users (libreboot and osboot main collaborator) understand this. All those comments are related to #590 and should be added there. Thanks for your understanding and motivation. Getting away of dropbear would be interesting, while we accomplished a state where dropbear being stripped, as all other binaries and libraries also compressed with xz -9 and -Os missing (700kb freed) I'm not sure moving from musl to tcc is recommended here, where musl have a really good reputation on both security stability and size. Yet again, #599 would be the place to debate those ideas not here, which will be lost in space and time soon enough tracking those ideas. |
|
oh, well musl is a libc and tcc is a c compiler i don't use github so i don't understand its nuances. i'll post on the issues instead |
|
@githubisnonfree sorry for the confusion, heads uses local GCC to build musl-cross-make and then builds everything with it, including coreboot's buildstack which then builds itself with it. Not against those ideas at all, just telling you a PR (merge request equivalent) is not the place to track sidequest ideas. Issues are. As on notabug ;) |
|
i mean notabug sucks. normally i just work with people directly on irc or they email me |
for context: my thinking here was merely that it would be nice to only have to flash the bios once for a totally new configuration. i find it strange that gpg operations are separate, considering any such operation involves reflashing. i wouldn't advocate removing the gpg operations menu, but just to merge the relevant options for "standalone bios images" into the flash menu, so that if a new rom is being flashed (not the "running bios" gpg operations) then you are able to fully configure it in one go. understood and no sweat if this is not desirable. just a thought. |
by the way, i've made a note about musl-cross-make. it sounds like exactly the sort of thing i need in my project |
|
@tlaurion While having a difference between the xx30 and xx20 isn't ideal, I think using nvmtool is the best approach given the restrains of size. Also keep in mind that is it isn't a feature that is completely denied to the xx20 users as they could still use do this using a prebuilt version in external storage or through an OS. They are only losing the convenience of it being built in. Hopefully, this will be short term. |
|
regarding randomization i think the best way to have random mac addresses would be to implement it in nvmutil. command syntax example: This example above is setting a static address. What if I made it possible to do: or maybe See that? Basically, any character that is ? would have nvmutil to randomly generate a number. This is the best way, because you might want a certain pattern. For example, some Intel NICs (on x200/t400 thinkpads) start with 00:1f:16:xx:xx:xx where x characters can be anything. |
The list of Intel Corp possible OUI is large.
@githubisnonfree absolutely, this would fit the need, where collisions on a shared prefix should not happen on LAN. Under Heads, we could lspci and apply Intel OUI accordingly, and/or define randomization patterns under Heads board config if users want to limit range as well. From my limited tests, my own locally used switches never stopped me from using totally randomized Mac addresses in the past, as opposed to WiFi controllers. From my limited experience, and outside of NAC enforced network limitations (which reuses wireshark OUI list as pointed in previous post) I do not think wired MAC are filtered, but I might be wrong for certain corner cases (my Cisco switch for example permitted total local randomization as used under /etc/functions as of now under network-recovery-init under Heads, so took it as a positive PoC and good enough) Would be a nice to have, definitely, though! |
|
well C has rand() which is pretty decent. the only question is, what to use as a seed value. a most conservative choice would be to simply fetch the current time (unix timestamp). of course, another way is to directly read from /dev/urandom i will think about this |
|
there are libs that do randomization but for our purposes, what i want is something simple and reasonably random. this isn't AES |
|
i think a good sane approach is: read from /dev/urandom, get numbers from that. urandom is sufficiently random for our purposes or just return -2 if can't read from urandom currently, mac address validation returns -1 if invalid mac passed. i'm designing my patch to set -2 if randomization is impossible for whatever reason, then if -2, validation says: randomization failed. then it exists with non-zero status basically an FU to non-unix systems |
|
@tlaurion i did it patch: This adds randomization support for setmac, with the scheme previously described. |
|
@tlaurion check again nvmutil git repo. i optimized it a ton. 11224kb compiled with tcc on my x86_64 machine i'm planning to revive nvmutils too, with these optimized techniques from nvmutil. the benefit there is i could make for you much smaller binaries that only do single tasks anyway, compressed with lzma, nvmutil when compiled with gcc will currently get you about 5kb, or with tcc, about 3kb. tested: with |
|
nvmutil 20220828 released! https://notabug.org/osboot/nvmutil/src/20220828/ChangeLog.md No new features have been added. This is a code cleanup and bugfix release.
I have half a mind to submit this to coreboot now. It's in a state where I'm happy for this to go in |
|
https://review.coreboot.org/c/coreboot/+/67129 let's see what they do with it. i submitted nvmutil to coreboot, but adapted it for them, and renamed it (in the coreboot patch) to nvmtool; if they do merge it, and the codebases later diverge, i want there to be as little confusion as possible, hence the renaming |
|
@githubisnonfree no movement on coreboot review, I also see that the project hasn't changed much. |
|
oh, yeah that code is bullet proof |
|
but merge the code with the commit just after the release. with this patch included: https://notabug.org/osboot/nvmutil/commit/debb6d58c35b7b3edce86e5ca6462aadf766487b that fixes a non-exploitable bug, but a bug nonetheless. it was the only flaw i found really, i've audited that code heavily because i rely on it myself, in my work (i randomise the mac address on every machine i sell to customers) |
|
the coreboot review is for the coreboot people to look at. i just put it on review.coreboot.org on a whim. and btw the coreboot one doesn't have the fix i just linked. so you should use nvmutil, from my notabug. cba to update the coreboot one, nobody seems interested so let's just leave it at that. cheers |
|
might wanna patch up those prototypes aswell. look at the coreboot one versus the notabug one. just a really pedantic change really, that coreboot's code checker insisted i do (have variable names in prototypes. on the notabug one i just have the types declared, which imo is fine) |
|
just one little quirk with nvmutil/nvmtool, and this is intentional: if it successfully modified a file, but one of the two checksums was incorrect (with that part then not modified accordingly), the program exits with non-zero status, even if the file was successfully and correctly modified in other words: the only time nvmutil ever returns 0 status is if it successfully opened the file, that the file was the right size, writable (if writes are to be done) and both parts of the gbe image (part 0 and 1) have correct checksums this is intentional behaviour, but it's really implementation defined. the hardware doesn't care, and those checksums are just there for software to use or ignore for example, you can actually patch out the checksum check in linux, and the nic will work so long as everything's correct. but by default, kernels do validate those checksums so it seems only natural that i should make nvmutil always return 1 if one or both of the checksums is wrong |
|
yeah and the program doesn't modify a part if the checksum is bad, but will modify the other part. e.g. setmac will update part 0 but not 1, if 1 has a bad checksum, and vice versa, but in that situation the program will return non-zero status |
|
you can change that all up according to your preference. my code is there as a heavily audited reference that Just Works |
|
i don't plan more changes to the code, at all |
|
in practise, it might be desirable to have the program exit with 0 if at least 1 of the checksums are correct, because someone might store two configurations and use the "brick" command to invalidate the one they don't want to use (presumably part 1), yeah and later if they wanted to swap, they'd just do (assume part0 is correct): a completely legit way to do things, and i often notice that lots of vendor gbe images have 1 bad and 1 good checksum |
|
what do you think? |
|
could just add a line at the end, just before it returns, saying something like just before the return, in that is, if you want to disable the behaviour, and then it'll always return success so long as the file was modified. it's up to you really. you know what, wait there and i'll just patch up the code real quick to fix those prototypes |
|
alright, the code is slightly more bullet proof now. i'm happy for the new nvmutil 20221103 release to be used as a reference. See: https://notabug.org/osboot/nvmutil/src/20221103 Changes:
Not much different, but I addressed the issues described in previous posts. This new version is a bit more user-friendly, because of how more tolerant it is (regarding zero/non-zero status), also it's slightly more debuggable because it returns the value of (the behaviour is still the same. in unix, errno is 0 unless an error was encountered and it was set, and 0 is success, and anything non-zero is fail) |
|
tl;dr errno is absolute |
|
in the release tag, i made sure: errno 0 is set if the file is modified in the buffered version, but only before running that way, if there's a problem actually writing the changes back, errno (and therefore exit status) will still be non-zero |
|
nvmutil 20221106 released. https://notabug.org/osboot/nvmutil/src/20221106 Very minor bugfix release:
|
|
i'm 99.9% sure that the code is perfect now, or if not, any further issues found will be minor enough so as to be irrelevant for the average user |
|
I will stick on latest release to do testing.
Dynamic menus are not so easy to produce with whiptail. Will think about that for further steps, attempting to add a dynamic option when nvmutil is present |
|
The nvmutil code is 99.99% perfect now. I added the extra 0.09% since my last post here. You should know: the nvmutil repo at the osboot project is now deader than dead. I merged osboot with libreboot, see: https://libreboot.org/news/merge.html You can find the improved nvmutil under The documentation for the assimulated lbmk version is here: https://libreboot.org/docs/install/nvmutil.html As of this day, 11 December 2022, I can report that the following changes are present, in the lbmk version: EDIT: I actually pushed those last three patches to notabug, since posting the above. I refer to these patches (pasted from above): |
|
I wonder if I should re-do the argument handling in nvmutil, to permit multiple operations, and/or operations on multiple files. For this, getopt would be used (just getopt, not getopt_long, because I have a special way I use just getopt, to make the code cleaner than in most other programs that use getopt, but only when you don't use the long version) |
|
I committed 3 bangers to nvmutil this morning:
As a result of these changes, the swap and copy functions of nvmutil are much faster due to less I/O in memory. |
…h gbe region(Thanks @githubisnonfree!) - Addition of ifdtool from coreboot project to extract gbe - As of now, its implemented in a hacky way: - ifdtool dir is copied over ifdtool_cross at coreboot's module configure step - then initrd packing step checks for CONFIG_NVMUTIL and builds and pack ifdtool_cross/ifdtool - As a result, what is build under build/coreboot/$BOARD is coreboot's real, where build/coreboot/ content follows Makefile rules - CONFIG_NVMUTIL in board config adds both ifdtool_cross/ifdtool and nvmutil into initrd - Added CONFIG_NVMUTIL to limited number of boards (to test for size changes) Manually tested (working!): - backup rom from: `flashrom -p internal -r /tmp/backup.rom` - go to that dir: `cd /tmp` - extract gbe from ifdtool on backup.rom: `ifdtool -x backup.rom` - source shell functions: `. /etc/functions` - show current PHY mac address: `nvm flashregion_3_gbe.bin dump` - generate mac address from sourced shell functions: `newmac=$(generate_random_mac_address)` - show new mac: `echo $newmac` - change mac from nvmtool on extracted gbe: `nvm flashregion_3_gbe.bin setmac $newmac` - insert modified gbe into backup.rom.new with ifdtool: `ifdtool -i gbe:flashregion_3_gbe.bin backup.rom` - flash back modified gbe only through flashrom: `flashrom -p internal --ifd -i gbe -w backup.rom.new` Signed-off-by: Thierry Laurion <[email protected]>
tlaurion
force-pushed
the
igbe_nvmtool_ifdtool_addition
branch
from
a4b5381 to
08a95b4
Compare
|
Bumped to latest nvmutil from lmbk (libreboot build system) since osboot was merged into libreboot and nvmutil has stopped changing. |
|
Manually tested (working!):
|
|
So next steps here would be On firmware upgrade:
Config:
Next would be to set static mac desired but not really a requirement (OS can set static mac per profile, also randomize further. Idea here is to have static MAC in firmware but permit to randomize it and keep it across upgrades so that rom is reproducible per build but kept customized on final machine). |
…: with host tools for coreboot and muslt-cross-make for inclusion under tools.cpio Signed-off-by: Thierry Laurion <[email protected]>
Question now is what is the proper workflow from here. New menu options from configuration menu?
Fixes #916