Paul Fagerburg has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/35607 )
Change subject: vboot: verify the GBB_HWID as part of the build steps ......................................................................
vboot: verify the GBB_HWID as part of the build steps
The GBB_HWID has to end with a value based on the CRC-32 of the first part of the string. We've been using a manual process to generate the full GBB_HWID, and there have been errors. As part of the build process, verify that the GBB_HWID conforms to the standard, and stop the build if it doesn't, so we don't get the "your computer is configured with a malformed hardware ID" error that prevents Chrome OS from updating.
BUG=b:140067412
Signed-off-by: Paul Fagerburg pfagerburg@chromium.org Change-Id: Ibfef1b118dca0927ef09674351f26cb2a5ad2171 --- M src/security/vboot/Makefile.inc A src/security/vboot/check_hwid.sh 2 files changed, 53 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/07/35607/1
diff --git a/src/security/vboot/Makefile.inc b/src/security/vboot/Makefile.inc index 3078e30..0f96ddb 100644 --- a/src/security/vboot/Makefile.inc +++ b/src/security/vboot/Makefile.inc @@ -241,6 +241,7 @@
$(obj)/gbb.region: $(obj)/gbb.stub @printf " SETUP GBB\n" + check_hwid.sh "$(CONFIG_GBB_HWID)" cp $< $@.tmp $(FUTILITY) gbb_utility -s \ --hwid="$(CONFIG_GBB_HWID)" \ diff --git a/src/security/vboot/check_hwid.sh b/src/security/vboot/check_hwid.sh new file mode 100755 index 0000000..fd1a40d --- /dev/null +++ b/src/security/vboot/check_hwid.sh @@ -0,0 +1,52 @@ +#!/bin/bash +# +# This file is part of the coreboot project. +# +# Copyright 2019 Google LLC. +# +# 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. + +if [[ "$#" -ne 1 ]]; then + echo "Usage: $0 hwid" + echo "e.g. $0 "KOHAKU TEST 1953"" + echo "Determine if the HWID is valid by recalculating the CRC and" + echo "comparing the result with the original." + exit 1 +fi + +HWID="${1}" + +# The hwid string should end with a space and some numbers. +# We want all of the string but the last space and the numbers. +# `cut` only understands counting forward from 1, not counting +# backward from the end, so use `rev`. +BASE=$(echo "${HWID}" | rev | cut -d" " -f 2- | rev) + +# Get a temporary file, because `crc32` will only work on files, +# not stdin or a string on the command line. +TMP="$(mktemp)" +echo -n "${BASE}" > "${TMP}" +CRC=$(crc32 "${TMP}") +rm -f "${TMP}" + +# `crc32` gives us a hex value, but without the leading 0x. +# Convert from hex to decimal. +CRC=$((16#${CRC})) + +# Take the 4 low digits (i.e. modulo 10000). +DIGITS=$(echo "${CRC}" | rev | cut -c 1-4 | rev) + +# The recomputed HWID is the base string plus a space and the 4 digits. +HWID2="${BASE} ${DIGITS}" + +if [[ ! "${HWID}" = "${HWID2}" ]]; then + echo "${HWID} fails validation, should be ${HWID2}" + exit 1 +fi
Aaron Durbin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35607 )
Change subject: vboot: verify the GBB_HWID as part of the build steps ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35607/1/src/security/vboot/check_hw... File src/security/vboot/check_hwid.sh:
https://review.coreboot.org/c/coreboot/+/35607/1/src/security/vboot/check_hw... PS1, Line 52: fi Not sure if it will help, but this the script I use to generate the hwid:
echo $1 $(printf "%d" 0x$(crc32 <(echo -n $1)) | tail -c 4)
Hello Aaron Durbin, 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/+/35607
to look at the new patch set (#2).
Change subject: vboot: verify the GBB_HWID as part of the build steps ......................................................................
vboot: verify the GBB_HWID as part of the build steps
The GBB_HWID has to end with a value based on the CRC-32 of the first part of the string. We've been using a manual process to generate the full GBB_HWID, and there have been errors. As part of the build process, verify that the GBB_HWID conforms to the standard, and stop the build if it doesn't, so we don't get the "your computer is configured with a malformed hardware ID" error that prevents Chrome OS from updating.
BUG=b:140067412
Signed-off-by: Paul Fagerburg pfagerburg@chromium.org Change-Id: Ibfef1b118dca0927ef09674351f26cb2a5ad2171 --- M src/security/vboot/Makefile.inc A src/security/vboot/check_hwid.sh 2 files changed, 53 insertions(+), 0 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/07/35607/2
Paul Fagerburg has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35607 )
Change subject: vboot: verify the GBB_HWID as part of the build steps ......................................................................
Patch Set 2:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35607/1/src/security/vboot/check_hw... File src/security/vboot/check_hwid.sh:
https://review.coreboot.org/c/coreboot/+/35607/1/src/security/vboot/check_hw... PS1, Line 52: fi
Not sure if it will help, but this the script I use to generate the hwid: […]
Thanks! I've got a slightly different use case in that I need to chop off digits first, but I'll see if there's a way to use that to simplify my script.
Paul Fagerburg has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35607 )
Change subject: vboot: verify the GBB_HWID as part of the build steps ......................................................................
Patch Set 7:
(2 comments)
This change is ready for review.
https://review.coreboot.org/c/coreboot/+/35607/6/src/security/vboot/check_hw... File src/security/vboot/check_hwid.sh:
https://review.coreboot.org/c/coreboot/+/35607/6/src/security/vboot/check_hw... PS6, Line 36: crc32
crc32 is not installed by default in the chroot, that will need to change to get this change through […]
Weird. I don't remember installing it, though. It's possible that this shell script is going to be replaced with some python code (using zlib) instead.
https://review.coreboot.org/c/coreboot/+/35607/6/src/security/vboot/check_hw... PS6, Line 50: ${HWID}
Add quotes around the HWIDs
Done
Paul Fagerburg has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35607 )
Change subject: vboot: verify the GBB_HWID as part of the build steps ......................................................................
Patch Set 7:
(1 comment)
https://review.coreboot.org/c/coreboot/+/35607/2/src/security/vboot/check_hw... File src/security/vboot/check_hwid.sh:
https://review.coreboot.org/c/coreboot/+/35607/2/src/security/vboot/check_hw... PS2, Line 50: echo "${HWID} fails validation, should be ${HWID2}"
yay
Done
Paul Fagerburg has abandoned this change. ( https://review.coreboot.org/c/coreboot/+/35607 )
Change subject: vboot: verify the GBB_HWID as part of the build steps ......................................................................
Abandoned
This breaks all kinds of things in the build, and 35634 is a better solution, too.
-
Aaron Durbin (Code Review) -
Paul Fagerburg (Code Review)