Hung-Te Lin has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35634 )
Change subject: vboot: create board-specific test-only GBB HWID if not set ......................................................................
vboot: create board-specific test-only GBB HWID if not set
The HWID in vboot GBB is an identifier for machine model. On Chrome OS, that should be provisioned in manufacturing process (by collecting real hardware information), and will be checked in system startup.
For bring up developers, they usually prefer to generate a test-only string for HWID. However that format was not well documented and cause problems. Further more, most Chromebooks are using HWID v3+ today while the test-only HWID is usually v2. Non-Chrome OS developers may also prefer their own format.
To simplify development process, the GBB_CONFIG now defaults to empty string, and will be replaced by a board-specific test-only v2 HWID automatically. Developers can still override that in mainboard Kconfig if they prefer v3 or other arbitrary format.
BUG=b:140067412 TEST=Built 'kukui' successfully. Removed kukui GBB config and built again, still seeing correct test HWID.
Change-Id: I0cda17a374641589291ec8dfb1d66c553f7cbf35 Signed-off-by: Hung-Te Lin hungte@chromium.org --- M src/security/vboot/Kconfig M src/security/vboot/Makefile.inc A src/security/vboot/gen_hwid.sh 3 files changed, 50 insertions(+), 2 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/34/35634/1
diff --git a/src/security/vboot/Kconfig b/src/security/vboot/Kconfig index 1e372d8..d6d74ca 100644 --- a/src/security/vboot/Kconfig +++ b/src/security/vboot/Kconfig @@ -228,7 +228,12 @@
config GBB_HWID string "Hardware ID" - default "NOCONF HWID" + default "" + help + A hardware identifier for device. On Chrome OS this is used for auto + update and recovery, and will be generated when manufacturing by the + factory software, in a strictly defined format. + Leave empty to get a test-only Chrome OS HWID v2 string generated.
config GBB_BMPFV_FILE string "Path to bmpfv image" diff --git a/src/security/vboot/Makefile.inc b/src/security/vboot/Makefile.inc index 3078e30..2aa9812 100644 --- a/src/security/vboot/Makefile.inc +++ b/src/security/vboot/Makefile.inc @@ -243,7 +243,9 @@ @printf " SETUP GBB\n" cp $< $@.tmp $(FUTILITY) gbb_utility -s \ - --hwid="$(CONFIG_GBB_HWID)" \ + --hwid="$$(src/security/vboot/gen_hwid.sh \ + "$(CONFIG_MAINBOARD_PART_NUMBER)" \ + "$(CONFIG_GBB_HWID)")" \ --rootkey="$(CONFIG_VBOOT_ROOT_KEY)" \ --recoverykey="$(CONFIG_VBOOT_RECOVERY_KEY)" \ --flags=$(GBB_FLAGS) \ diff --git a/src/security/vboot/gen_hwid.sh b/src/security/vboot/gen_hwid.sh new file mode 100755 index 0000000..949e67c7 --- /dev/null +++ b/src/security/vboot/gen_hwid.sh @@ -0,0 +1,41 @@ +#!/bin/sh +# +# This file is part of the coreboot project. +# +# Copyright 2019 Google Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; version 2 of the License. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. + +die() { + echo "$*" >&2 + exit 1 +} + +if [ "$#" != 2 ]; then + die "Usage: $0 MAINBOARD_PARTNUMBER OVERRIDE_HWID" +fi + +part_number="$1" +override="$2" + +if [ -n "${override}" ]; then + echo "${override}" + exit +fi + +# Generate a test-only Chrome OS HWID v2 string +prefix="$(echo "${part_number}" | tr a-z A-Z) TEST" + +# TODO(hungte) Use rhash or crc32 if python is not available. +# Use Python2 or python 3. +crc="$(python -c "import zlib, sys; sys.stdout.write(\ + ('%04u' % (zlib.crc32('${prefix}'.encode('utf8')) & 0xffffffffL))[-4:])")" + +echo "${prefix}" "${crc}"
Hung-Te Lin has removed Aaron Durbin from this change. ( https://review.coreboot.org/c/coreboot/+/35634 )
Change subject: vboot: create board-specific test-only GBB HWID if not set ......................................................................
Removed reviewer Aaron Durbin.
Paul Fagerburg has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35634 )
Change subject: vboot: create board-specific test-only GBB HWID if not set ......................................................................
Patch Set 1: Code-Review+1
Mathew King has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35634 )
Change subject: vboot: create board-specific test-only GBB HWID if not set ......................................................................
Patch Set 1: Code-Review+2
Justin TerAvest has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35634 )
Change subject: vboot: create board-specific test-only GBB HWID if not set ......................................................................
Patch Set 1: Code-Review+2
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35634 )
Change subject: vboot: create board-specific test-only GBB HWID if not set ......................................................................
Patch Set 1:
(3 comments)
https://review.coreboot.org/c/coreboot/+/35634/1/src/security/vboot/gen_hwid... File src/security/vboot/gen_hwid.sh:
https://review.coreboot.org/c/coreboot/+/35634/1/src/security/vboot/gen_hwid... PS1, Line 1: #!/bin/sh Shouldn't this go in $(top)/util/? I think host code (even scripts) is generally not allowed under $(top)/src/. (Then again, this is short enough that you could just put it inline into the Makefile...)
https://review.coreboot.org/c/coreboot/+/35634/1/src/security/vboot/gen_hwid... PS1, Line 28: if [ -n "${override}" ]; then nit: this part would also be easy to do within make (with $(if ...)).
https://review.coreboot.org/c/coreboot/+/35634/1/src/security/vboot/gen_hwid... PS1, Line 38: crc="$(python -c "import zlib, sys; sys.stdout.write(\ IIRC the coreboot build system is not supposed to depend on Python, unless something changed there that I'm not aware of. Individual boards/SoCs may have Python packaging tools if they need them, but not a general, platform-independent feature.
Maybe it's possible to write this in awk (which I think is generally considered "available everywhere")? (Maybe the second version in https://stackoverflow.com/questions/4994537/calculating-crc-in-awk , but haven't tried it.)
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35634 )
Change subject: vboot: create board-specific test-only GBB HWID if not set ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/35634/1/src/security/vboot/gen_hwid... File src/security/vboot/gen_hwid.sh:
https://review.coreboot.org/c/coreboot/+/35634/1/src/security/vboot/gen_hwid... PS1, Line 28: if [ -n "${override}" ]; then
nit: this part would also be easy to do within make (with $(if ...)).
yes but that will make Makefile more complicated. But yes, I can consider that again...
https://review.coreboot.org/c/coreboot/+/35634/1/src/security/vboot/gen_hwid... PS1, Line 38: crc="$(python -c "import zlib, sys; sys.stdout.write(\
IIRC the coreboot build system is not supposed to depend on Python, unless something changed there t […]
That looks horrible. I found a better choice:
https://stackoverflow.com/questions/11553941/use-gunzip-to-calculate-crc32-c...
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35634 )
Change subject: vboot: create board-specific test-only GBB HWID if not set ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35634/1/src/security/vboot/gen_hwid... File src/security/vboot/gen_hwid.sh:
https://review.coreboot.org/c/coreboot/+/35634/1/src/security/vboot/gen_hwid... PS1, Line 38: crc="$(python -c "import zlib, sys; sys.stdout.write(\
That looks horrible. I found a better choice: […]
Dumb question. Why don't we let futility calculate the checksum based on the input string?
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35634 )
Change subject: vboot: create board-specific test-only GBB HWID if not set ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35634/1/src/security/vboot/gen_hwid... File src/security/vboot/gen_hwid.sh:
https://review.coreboot.org/c/coreboot/+/35634/1/src/security/vboot/gen_hwid... PS1, Line 38: crc="$(python -c "import zlib, sys; sys.stdout.write(\
Why don't we let futility calculate the checksum based on the input string?
Like said, the vboot does not define how HWID should look like. It only expects a string. It is Chrome OS browser that expects a string following HWID v2 CRC, or HWID v3/v4/... etc.
Unless you're suggesting to make a "calculate CRC32" general function (not for hwid), otherwise it'll be adding a deprecated feature, or breaking all HWID v3/v4 systems.
Hello Paul Fagerburg, Mathew King, Justin TerAvest, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35634
to look at the new patch set (#2).
Change subject: vboot: create board-specific test-only GBB HWID if not set ......................................................................
vboot: create board-specific test-only GBB HWID if not set
The HWID in vboot GBB is an identifier for machine model. On Chrome OS, that should be provisioned in manufacturing process (by collecting real hardware information), and will be checked in system startup.
For bring up developers, they usually prefer to generate a test-only string for HWID. However that format was not well documented and cause problems. Further more, most Chromebooks are using HWID v3+ today while the test-only HWID is usually v2. Non-Chrome OS developers may also prefer their own format.
To simplify development process, the GBB_CONFIG now defaults to empty string, and will be replaced by a board-specific test-only v2 HWID automatically. Developers can still override that in mainboard Kconfig if they prefer v3 or other arbitrary format.
BUG=b:140067412 TEST=Built 'kukui' successfully. Removed kukui GBB config and built again, still seeing correct test HWID.
Change-Id: I0cda17a374641589291ec8dfb1d66c553f7cbf35 Signed-off-by: Hung-Te Lin hungte@chromium.org --- M src/security/vboot/Kconfig M src/security/vboot/Makefile.inc M util/chromeos/README.md A util/chromeos/gen_test_hwid.sh 4 files changed, 58 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/34/35634/2
Hung-Te Lin has removed Aaron Durbin from this change. ( https://review.coreboot.org/c/coreboot/+/35634 )
Change subject: vboot: create board-specific test-only GBB HWID if not set ......................................................................
Removed reviewer Aaron Durbin.
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35634 )
Change subject: vboot: create board-specific test-only GBB HWID if not set ......................................................................
Patch Set 2:
(2 comments)
https://review.coreboot.org/c/coreboot/+/35634/1/src/security/vboot/gen_hwid... File src/security/vboot/gen_hwid.sh:
https://review.coreboot.org/c/coreboot/+/35634/1/src/security/vboot/gen_hwid... PS1, Line 1: #!/bin/sh
Shouldn't this go in $(top)/util/? I think host code (even scripts) is generally not allowed under $ […]
Done
https://review.coreboot.org/c/coreboot/+/35634/1/src/security/vboot/gen_hwid... PS1, Line 38: crc="$(python -c "import zlib, sys; sys.stdout.write(\
Why don't we let futility calculate the checksum based on the input string? […]
Done
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35634 )
Change subject: vboot: create board-specific test-only GBB HWID if not set ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35634/1/src/security/vboot/gen_hwid... File src/security/vboot/gen_hwid.sh:
https://review.coreboot.org/c/coreboot/+/35634/1/src/security/vboot/gen_hwid... PS1, Line 38: crc="$(python -c "import zlib, sys; sys.stdout.write(\
Like said, the vboot does not define how HWID should look like. It only expects a string.
Understood.
Unless you're suggesting to make a "calculate CRC32" general function (not for hwid), otherwise it'll be adding a deprecated feature, or breaking all HWID v3/v4 systems.
Kind of. I was thinking of having a calculate crc32 function in futility. Then you could either: a) Call futility to calculate that and use in Makefile. b) Add an option in gbb_utility to say append CRC32 to HWID. It will end up being used only by coreboot. And, it will require adding some knowledge to vboot/futility what append to HWID means.
Anyways, I was just curious and hence the question. I am okay if you want to just add a script to do this since its use is limited to coreboot.
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35634 )
Change subject: vboot: create board-specific test-only GBB HWID if not set ......................................................................
Patch Set 3:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35634/1/src/security/vboot/gen_hwid... File src/security/vboot/gen_hwid.sh:
https://review.coreboot.org/c/coreboot/+/35634/1/src/security/vboot/gen_hwid... PS1, Line 38: crc="$(python -c "import zlib, sys; sys.stdout.write(\
b) Add an option in gbb_utility to say append CRC32 to HWID. And, it will require adding some knowledge to vboot/futility what append to HWID means.
Definitely not this. It'll confuse people who use HWID v3.
It will end up being used only by coreboot.
Let's keep it only in coreboot :) Since this is actually caused by our moving build logic from chromeos-bootimage into coreboot.
Paul Fagerburg has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35634 )
Change subject: vboot: create board-specific test-only GBB HWID if not set ......................................................................
Patch Set 3: Code-Review+2
Furquan Shaikh has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35634 )
Change subject: vboot: create board-specific test-only GBB HWID if not set ......................................................................
Patch Set 3:
(2 comments)
https://review.coreboot.org/c/coreboot/+/35634/3/src/security/vboot/Makefile... File src/security/vboot/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/35634/3/src/security/vboot/Makefile... PS3, Line 244: util/chromeos/gen_test_hwid.sh shouldn't this be $(top)/util/...
https://review.coreboot.org/c/coreboot/+/35634/1/src/security/vboot/gen_hwid... File src/security/vboot/gen_hwid.sh:
https://review.coreboot.org/c/coreboot/+/35634/1/src/security/vboot/gen_hwid... PS1, Line 38: crc="$(python -c "import zlib, sys; sys.stdout.write(\
b) Add an option in gbb_utility to say append CRC32 to HWID. […]
SGTM.
Hello Aaron Durbin, Paul Fagerburg, Mathew King, Justin TerAvest, build bot (Jenkins), Patrick Georgi, Martin Roth,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/35634
to look at the new patch set (#4).
Change subject: vboot: create board-specific test-only GBB HWID if not set ......................................................................
vboot: create board-specific test-only GBB HWID if not set
The HWID in vboot GBB is an identifier for machine model. On Chrome OS, that should be provisioned in manufacturing process (by collecting real hardware information), and will be checked in system startup.
For bring up developers, they usually prefer to generate a test-only string for HWID. However that format was not well documented and cause problems. Further more, most Chromebooks are using HWID v3+ today while the test-only HWID is usually v2. Non-Chrome OS developers may also prefer their own format.
To simplify development process, the GBB_CONFIG now defaults to empty string, and will be replaced by a board-specific test-only v2 HWID automatically. Developers can still override that in mainboard Kconfig if they prefer v3 or other arbitrary format.
BUG=b:140067412 TEST=Built 'kukui' successfully. Removed kukui GBB config and built again, still seeing correct test HWID.
Change-Id: I0cda17a374641589291ec8dfb1d66c553f7cbf35 Signed-off-by: Hung-Te Lin hungte@chromium.org --- M src/security/vboot/Kconfig M src/security/vboot/Makefile.inc M util/chromeos/README.md A util/chromeos/gen_test_hwid.sh 4 files changed, 58 insertions(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/34/35634/4
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35634 )
Change subject: vboot: create board-specific test-only GBB HWID if not set ......................................................................
Patch Set 4:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35634/3/src/security/vboot/Makefile... File src/security/vboot/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/35634/3/src/security/vboot/Makefile... PS3, Line 244: util/chromeos/gen_test_hwid.sh
shouldn't this be $(top)/util/...
Done
Patrick Georgi has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35634 )
Change subject: vboot: create board-specific test-only GBB HWID if not set ......................................................................
Patch Set 4: Code-Review+2
Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/35634 )
Change subject: vboot: create board-specific test-only GBB HWID if not set ......................................................................
vboot: create board-specific test-only GBB HWID if not set
The HWID in vboot GBB is an identifier for machine model. On Chrome OS, that should be provisioned in manufacturing process (by collecting real hardware information), and will be checked in system startup.
For bring up developers, they usually prefer to generate a test-only string for HWID. However that format was not well documented and cause problems. Further more, most Chromebooks are using HWID v3+ today while the test-only HWID is usually v2. Non-Chrome OS developers may also prefer their own format.
To simplify development process, the GBB_CONFIG now defaults to empty string, and will be replaced by a board-specific test-only v2 HWID automatically. Developers can still override that in mainboard Kconfig if they prefer v3 or other arbitrary format.
BUG=b:140067412 TEST=Built 'kukui' successfully. Removed kukui GBB config and built again, still seeing correct test HWID.
Change-Id: I0cda17a374641589291ec8dfb1d66c553f7cbf35 Signed-off-by: Hung-Te Lin hungte@chromium.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/35634 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Patrick Georgi pgeorgi@google.com --- M src/security/vboot/Kconfig M src/security/vboot/Makefile.inc M util/chromeos/README.md A util/chromeos/gen_test_hwid.sh 4 files changed, 58 insertions(+), 1 deletion(-)
Approvals: build bot (Jenkins): Verified Patrick Georgi: Looks good to me, approved
diff --git a/src/security/vboot/Kconfig b/src/security/vboot/Kconfig index 1e372d8..d6d74ca 100644 --- a/src/security/vboot/Kconfig +++ b/src/security/vboot/Kconfig @@ -228,7 +228,12 @@
config GBB_HWID string "Hardware ID" - default "NOCONF HWID" + default "" + help + A hardware identifier for device. On Chrome OS this is used for auto + update and recovery, and will be generated when manufacturing by the + factory software, in a strictly defined format. + Leave empty to get a test-only Chrome OS HWID v2 string generated.
config GBB_BMPFV_FILE string "Path to bmpfv image" diff --git a/src/security/vboot/Makefile.inc b/src/security/vboot/Makefile.inc index 3078e30..abb8863 100644 --- a/src/security/vboot/Makefile.inc +++ b/src/security/vboot/Makefile.inc @@ -239,6 +239,11 @@ mv $@.tmp $@ endif
+# Generate a test-only HWID +ifeq ($(CONFIG_GBB_HWID),) +CONFIG_GBB_HWID := $$($(top)/util/chromeos/gen_test_hwid.sh "$(CONFIG_MAINBOARD_PART_NUMBER)") +endif + $(obj)/gbb.region: $(obj)/gbb.stub @printf " SETUP GBB\n" cp $< $@.tmp diff --git a/util/chromeos/README.md b/util/chromeos/README.md index 7a3897d..0b9a7d7 100644 --- a/util/chromeos/README.md +++ b/util/chromeos/README.md @@ -26,3 +26,19 @@
Right now it will produce the ME firmware blob, IFD, VGA option rom, and `mrc.bin`. + +## gen_test_hwid.sh + +`gen_test_hwid.sh` generates a test-only identifier in Chrome OS HWID v2 +compatible format. + +Usage: +``` +$ ./gen_test_hwid.sh BOARD_NAME +``` + +Example: +``` +$ ./gen_test_hwid.sh Kukui +KUKUI TEST 9847 +``` diff --git a/util/chromeos/gen_test_hwid.sh b/util/chromeos/gen_test_hwid.sh new file mode 100755 index 0000000..849ff6a --- /dev/null +++ b/util/chromeos/gen_test_hwid.sh @@ -0,0 +1,31 @@ +#!/bin/sh +# +# This file is part of the coreboot project. +# +# Copyright 2019 Google Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; version 2 of the License. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. + +main() { + if [ "$#" != 1 ]; then + echo "Usage: $0 MAINBOARD_PARTNUMBER" >&2 + exit 1 + fi + + # Generate a test-only Chrome OS HWID v2 string + local board="$1" + local prefix="$(echo "${board}" | tr a-z A-Z) TEST" + # gzip has second-to-last 4 bytes in CRC32. + local crc32="$(printf "${prefix}" | gzip -1 | tail -c 8 | head -c 4 | \ + hexdump -e '1/4 "%04u" ""' | tail -c 4)" + + echo "${prefix}" "${crc32}" +} +main "$@"
-
Furquan Shaikh (Code Review) -
Hung-Te Lin (Code Review)
-
Julius Werner (Code Review)
-
Justin TerAvest (Code Review)
-
Mathew King (Code Review)
-
Patrick Georgi (Code Review)
-
Paul Fagerburg (Code Review)