[Buildroot] [EXTERNAL] Re: [PATCH 1/1] Allow users to specifiy a hash file to verify custom linux kernels and custom kernel headers
Ian Merin
Ian.Merin at entrust.com
Sun Jun 13 22:21:06 UTC 2021
Hi Yann, Thomas
Thanks for your comments. I started with something fairly generic, but a few things led me to designing a specific solution
1) I was following the paradigm of packages with officially supported custom versions. e.g. BR2_LINUX_KERNEL_CUSTOM_VERSION, BR2_LINUX_KERNEL_CUSTOM_TARBALL since those are the instances where this support is likely needed.
2)
> Also note that, BR_NO_CHECK_HASH_FOR is just a hint to ignore a no-hash condition. If there in fact are hashes, they *are* checked, *and* they
>*must* match. So, by adding an extra-hash file, you do not need to conditionalise the setting of BR_NO_CHECK_HASH_FOR in packages.
Consider the following scenario: I have an extra-hashes file that has the hash for some version of linux-5.4.123. I decide to upgrade my kernel to linux-5.4.124, but I forget to add the hash to my extra-hashes file. Since BR_NO_CHECK_HASH_FOR is set to linux-5.4.124 buildroot will silently fail the hash check if I have no hash matching linux-5.4.124.tar.xz. I think this behavior is antithetical to the intention of this feature. I see no option but to conditionalise the setting and constrain the custom hash files to specific packages.
If you have insight into how to work around this problem I welcome the suggestion.
Thanks,
Ian
-----Original Message-----
From: Yann E. MORIN <yann.morin.1998 at free.fr>
Sent: Saturday, June 12, 2021 5:06 PM
To: Ian Merin <Ian.Merin at entrust.com>
Cc: buildroot at buildroot.org; thomas.petazzoni at bootlin.com
Subject: [EXTERNAL] Re: [Buildroot] [PATCH 1/1] Allow users to specifiy a hash file to verify custom linux kernels and custom kernel headers
WARNING: This email originated outside of Entrust.
DO NOT CLICK links or attachments unless you trust the sender and know the content is safe.
______________________________________________________________________
Ian, All,
On 2021-06-11 17:20 +0000, Ian Merin via buildroot spake thusly:
> From 9873437ad7b4f4e95f970e843a7ed908603c25d7 Mon Sep 17 00:00:00 2001
> From: Ian Merin <Ian.Merin at nCipher.com>
> Date: Fri, 11 Jun 2021 13:02:18 -0400
> Subject: [PATCH 1/1] Allow users to specifiy a hash file to verify
> custom linux kernels and custom kernel headers
I think going with the extra-hashes file is a good idea overall.
> linux/Config.in: add linux_kernel_custom_hash options
> linux/linux.mk: add ability to override hash file
> package/linux-headers/Config.in.host: add kernel_headers_custom_hash
> options
> package/linux-headers/linux-headers.mk: add ability to override hash
> file
> package/pkg-generic.mk: don't use default hash file if it is already
> set
A commit log should not describe what is done; it should explain it:
what the problem is, and how it is solved. If there are tricky things in the code, the commit log can explain this too.
However, I think this patch makes the feature really too-specific to just the kernel (and its headers). Instead, I think we will want something that can be used to check hashes for other packages where the version can be specified:
* uboot:
- BR2_TARGET_UBOOT_CUSTOM_VERSION
- BR2_TARGET_UBOOT_CUSTOM_TARBALL
- BR2_TARGET_UBOOT_CUSTOM_GIT
- BR2_TARGET_UBOOT_CUSTOM_HG
- BR2_TARGET_UBOOT_CUSTOM_SVN
* arm-trusted-firmware:
- BR2_TARGET_ARM_TRUSTED_FIRMWARE_CUSTOM_VERSION
- BR2_TARGET_ARM_TRUSTED_FIRMWARE_CUSTOM_TARBALL
- BR2_TARGET_ARM_TRUSTED_FIRMWARE_CUSTOM_GIT
* aufs-util:
- BR2_PACKAGE_AUFS_UTIL_VERSION
* refpolicy:
- BR2_PACKAGE_REFPOLICY_CUSTOM_GIT
* xenomai;
- BR2_PACKAGE_XENOMAI_CUSTOM_VERSION
- BR2_PACKAGE_XENOMAI_CUSTOM_TARBALL
- BR2_PACKAGE_XENOMAI_CUSTOM_GIT
and so on...
Also, we don't need a boolean to drive this feature: if the path is set, just use it as extra hash file; otherwise there is no extra hash file to check against.
Also note that, BR_NO_CHECK_HASH_FOR is just a hint to ignore a no-hash condition. If there in fact are hashes, they *are* checked, *and* they
*must* match. So, by adding an extra-hash file, you do not need to conditionalise the setting of BR_NO_CHECK_HASH_FOR in packages.
Probably something like the following would be relatively easy to push toward completion (the hardest part is in support/download/check-hash):
diff --git a/Config.in b/Config.in
index c05485173b..2101d1cafd 100644
--- a/Config.in
+++ b/Config.in
@@ -294,6 +294,12 @@ endif
endmenu
+config BR2_EXTRA_HASH_FILES
+ string "Paths to files containing extra packages hashes"
+ help
+ Set to a space-separated list of file paths to use to check
+ packages hashes against.
+
config BR2_JLEVEL
int "Number of jobs to run simultaneously (0 for auto)"
default "0"
diff --git a/package/pkg-download.mk b/package/pkg-download.mk
index 2527ba5c60..b024cdb499 100644
--- a/package/pkg-download.mk
+++ b/package/pkg-download.mk
@@ -114,6 +114,7 @@ define DOWNLOAD
-D '$(DL_DIR)' \
-f '$(notdir $(1))' \
-H '$($(2)_HASH_FILE)' \
+ $(foreach f,$(BR2_EXTRA_HASH_FILES),-H $(f))\
-n '$($(2)_BASENAME_RAW)' \
-N '$($(2)_RAWNAME)' \
-o '$($(2)_DL_DIR)/$(notdir $(1))' \
diff --git a/support/download/dl-wrapper b/support/download/dl-wrapper
index 3315bd410e..d86b0d9747 100755
--- a/support/download/dl-wrapper
+++ b/support/download/dl-wrapper
@@ -21,8 +21,9 @@ export BR_BACKEND_DL_GETOPTS=":hc:d:o:n:N:H:ru:qf:e"
main() {
local OPT OPTARG
- local backend output hfile recurse quiet rc
+ local backend output recurse quiet rc
local -a uris
+ local -a hash_files
# Parse our options; anything after '--' is for the backend
while getopts ":c:d:D:o:n:N:H:rf:u:q" OPT; do
@@ -33,7 +34,7 @@ main() {
o) output="${OPTARG}";;
n) raw_base_name="${OPTARG}";;
N) base_name="${OPTARG}";;
- H) hfile="${OPTARG}";;
+ H) hash_files+=("${OPTARG}");;
r) recurse="-r";;
f) filename="${OPTARG}";;
u) uris+=( "${OPTARG}" );;
@@ -68,7 +69,7 @@ main() {
# - fails at least one of its hashes: force a re-download
# - there's no hash (but a .hash file): consider it a hard error
if [ -e "${output}" ]; then
- if support/download/check-hash ${quiet} "${hfile}" "${output}" "${output##*/}"; then
+ if support/download/check-hash ${quiet} "${output}" "${output##*/}" "${hash_files[@]}"; then
exit 0
elif [ ${?} -ne 2 ]; then
# Do not remove the file, otherwise it might get re-downloaded
@@ -140,7 +141,7 @@ main() {
# Check if the downloaded file is sane, and matches the stored hashes
# for that file
- if support/download/check-hash ${quiet} "${hfile}" "${tmpf}" "${output##*/}"; then
+ if support/download/check-hash ${quiet} "${tmpf}" "${output##*/}" "${hash_files[@]}"; then
rc=0
else
if [ ${?} -ne 3 ]; then
diff --git a/support/download/check-hash b/support/download/check-hash
index fe9c10570e..227924819e 100755
--- a/support/download/check-hash
+++ b/support/download/check-hash
@@ -3,12 +3,12 @@ set -e
# Helper to check a file matches its known hash
# Call it with:
-# $1: the path of the file containing all the expected hashes
-# $2: the full path to the temporary file that was downloaded, and
+# $1: the full path to the temporary file that was downloaded, and
# that is to be checked
-# $3: the final basename of the file, to which it will be ultimately
+# $2: the final basename of the file, to which it will be ultimately
# saved as, to be able to match it to the corresponding hashes
# in the .hash file
+# $3+: the path of the files containing all the expected hashes
#
# Exit codes:
# 0: the hash file exists and the file to check matches all its hashes,
@@ -27,10 +27,12 @@ while getopts :q OPT; do
done
shift $((OPTIND-1))
-h_file="${1}"
-file="${2}"
-base="${3}"
+file="${1}"
+base="${2}"
+shift 2
+declare -a hash_files=("${@}")
+## The following will have to be heavily lifted...
# Bail early if no hash to check
if [ -z "${h_file}" ]; then
exit 0
Finally, this would probably have to be done in separate patches, probably a series like this;
1. change support/download/check-hash to move the hash file at the end
2. change it to be able to use more than one hash file
3. extend support/download/dl-wrapper to accept more than one hash file
4. add my proposed BR2_EXTRA_HASH_FILES option and pass the list to
dl-wrapper
And this should be about it. Of course, the devil is in the details, but overall, I believe this is a better solution than having changes specific to linux and linux-headers.
Regards,
Yann E. MORIN.
> Signed-off-by: Ian Merin <Ian.Merin at nCipher.com>
> Signed-off-by: Ian Merin <Ian.Merin at entrust.com>
> ---
> linux/Config.in | 12 ++++++++++++
> linux/linux.mk | 4 ++++
> package/linux-headers/Config.in.host | 12 ++++++++++++
> package/linux-headers/linux-headers.mk | 18 ++++++++++++++++++
> package/pkg-generic.mk | 17 +++++++++++------
> 5 files changed, 57 insertions(+), 6 deletions(-)
>
> diff --git a/linux/Config.in b/linux/Config.in index
> c893c8dc82..8955c1994b 100644
> --- a/linux/Config.in
> +++ b/linux/Config.in
> @@ -123,6 +123,18 @@ config BR2_LINUX_KERNEL_CUSTOM_REPO_VERSION
>
> endif
>
> +config BR2_LINUX_KERNEL_CUSTOM_HASH
> + bool "Enable checking of custom hash file for linux kernel"
> + default n
> + depends on !BR2_LINUX_KERNEL_LATEST_VERSION && \
> + !BR2_LINUX_KERNEL_LATEST_CIP_VERSION && !BR2_LINUX_KERNEL_LATEST_CIP_RT_VERSION
> + help
> + This option allows to specify a file containing hashes for your
> +kernel version if using a non default kernel version
> +
> +config BR2_LINUX_KERNEL_CUSTOM_HASH_FILE
> + string "path of custom linux.hash file"
> + depends on BR2_LINUX_KERNEL_CUSTOM_HASH
> +
> config BR2_LINUX_KERNEL_VERSION
> string
> default "5.12.4" if BR2_LINUX_KERNEL_LATEST_VERSION diff --git
> a/linux/linux.mk b/linux/linux.mk index 1457228eb9..203d46a93b 100644
> --- a/linux/linux.mk
> +++ b/linux/linux.mk
> @@ -54,8 +54,12 @@ endif
> endif
>
> ifeq ($(BR2_LINUX_KERNEL)$(BR2_LINUX_KERNEL_LATEST_VERSION),y)
> +ifeq ($(BR2_LINUX_KERNEL_CUSTOM_HASH),y)
> +LINUX_HASH_FILE = $(call qstrip,$(BR2_LINUX_KERNEL_CUSTOM_HASH_FILE))
> +else
> BR_NO_CHECK_HASH_FOR += $(LINUX_SOURCE) endif
> +endif
>
> LINUX_PATCHES = $(call qstrip,$(BR2_LINUX_KERNEL_PATCH))
>
> diff --git a/package/linux-headers/Config.in.host
> b/package/linux-headers/Config.in.host
> index b68567deeb..991bdc957a 100644
> --- a/package/linux-headers/Config.in.host
> +++ b/package/linux-headers/Config.in.host
> @@ -97,6 +97,18 @@ config BR2_KERNEL_HEADERS_CUSTOM_GIT
>
> endchoice
>
> +config BR2_KERNEL_HEADERS_CUSTOM_HASH
> + bool "Custom hash"
> + default n
> + depends on BR2_KERNEL_HEADERS_AS_KERNEL || BR2_KERNEL_HEADERS_VERSION || \
> + BR2_KERNEL_HEADERS_CUSTOM_TARBALL || BR2_KERNEL_HEADERS_CUSTOM_GIT
> + help
> + This option allows to specify a file containing hashes for your
> +kernel version
> +
> +config BR2_KERNEL_HEADERS_CUSTOM_HASH_FILE
> + string "path of custom linux.hash file"
> + depends on BR2_KERNEL_HEADERS_CUSTOM_HASH
> +
> # Select this for the latest kernel headers version (for license
> hashes) config BR2_KERNEL_HEADERS_LATEST
> bool
> diff --git a/package/linux-headers/linux-headers.mk
> b/package/linux-headers/linux-headers.mk
> index a8d1c2ccaf..9d216922c3 100644
> --- a/package/linux-headers/linux-headers.mk
> +++ b/package/linux-headers/linux-headers.mk
> @@ -10,6 +10,15 @@
> # Set variables depending on whether we are using headers from a
> kernel # build or a standalone header package.
> ifeq ($(BR2_KERNEL_HEADERS_AS_KERNEL),y)
> +LINUX_HEADERS_CUSTOM_HASH = $(BR2_LINUX_KERNEL_CUSTOM_HASH)
> +LINUX_HEADERS_CUSTOM_HASH_FILE = $(call
> +qstrip,$(BR2_LINUX_KERNEL_CUSTOM_HASH_FILE))
> +# Are we using a custom kernel version?
> +ifeq ($(BR2_LINUX_KERNEL)$(BR2_LINUX_KERNEL_LATEST_VERSION),y)
> +# Use the custom hash file, if provided ifeq
> +($(BR2_LINUX_KERNEL_CUSTOM_HASH),y)
> +LINUX_HEADERS_HASH_FILE = $(LINUX_HEADERS_CUSTOM_HASH_FILE) endif #
> +BR2_LINUX_KERNEL_CUSTOM_HASH endif # BR2_LINUX_KERNEL,
> +BR2_LINUX_KERNEL_LATEST_VERSION
> LINUX_HEADERS_CUSTOM_TARBALL = $(call
> qstrip,$(BR2_LINUX_KERNEL_CUSTOM_TARBALL))
> LINUX_HEADERS_CUSTOM_GIT = $(call
> qstrip,$(BR2_LINUX_KERNEL_CUSTOM_GIT))
> LINUX_HEADERS_CUSTOM_HG = $(call
> qstrip,$(BR2_LINUX_KERNEL_CUSTOM_HG))
> @@ -23,6 +32,8 @@ $(error LINUX_HEADERS_OVERRIDE_SRCDIR must not be
> set when BR2_KERNEL_HEADERS_AS endif LINUX_HEADERS_OVERRIDE_SRCDIR =
> $(LINUX_OVERRIDE_SRCDIR) else # ! BR2_KERNEL_HEADERS_AS_KERNEL
> +LINUX_HEADERS_CUSTOM_HASH = $(BR2_KERNEL_HEADERS_CUSTOM_HASH)
> +LINUX_HEADERS_CUSTOM_HASH_FILE = $(call
> +qstrip,$(BR2_KERNEL_HEADERS_CUSTOM_HASH_FILE))
> LINUX_HEADERS_CUSTOM_TARBALL = $(call
> qstrip,$(BR2_KERNEL_HEADERS_CUSTOM_TARBALL))
> LINUX_HEADERS_CUSTOM_GIT = $(call
> qstrip,$(BR2_KERNEL_HEADERS_CUSTOM_GIT))
> LINUX_HEADERS_CUSTOM_HG =
> @@ -91,10 +102,17 @@ endef
> LINUX_HEADERS_POST_PATCH_HOOKS += LINUX_HEADERS_APPLY_LOCAL_PATCHES
> endif # BR2_KERNEL_HEADERS_AS_KERNEL
>
> +
> +
> # Skip hash checking for custom kernel headers.
> ifeq
> ($(BR2_KERNEL_HEADERS_VERSION)$(BR2_KERNEL_HEADERS_CUSTOM_TARBALL)$(BR
> 2_KERNEL_HEADERS_CUSTOM_GIT),y)
> +# Unless the user has specified an external hash file ifeq
> +($(LINUX_HEADERS_CUSTOM_HASH),y) LINUX_HEADERS_HASH_FILE =
> +LINUX_HEADERS_CUSTOM_HASH_FILE else
> BR_NO_CHECK_HASH_FOR += $(LINUX_HEADERS_SOURCE) endif
> +endif
>
> # linux-headers really is the same as the linux package
> LINUX_HEADERS_DL_SUBDIR = linux diff --git a/package/pkg-generic.mk
> b/package/pkg-generic.mk index 9fbc63d19e..5d5b479fcf 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -474,12 +474,17 @@ else
> $(2)_DL_VERSION := $$(strip $$($(2)_VERSION)) endif $(2)_VERSION
> := $$(call sanitize,$$($(2)_DL_VERSION))
> -
> -$(2)_HASH_FILE = \
> - $$(strip \
> - $$(if $$(wildcard $$($(2)_PKGDIR)/$$($(2)_VERSION)/$$($(2)_RAWNAME).hash),\
> - $$($(2)_PKGDIR)/$$($(2)_VERSION)/$$($(2)_RAWNAME).hash,\
> - $$($(2)_PKGDIR)/$$($(2)_RAWNAME).hash))
> +ifndef $(2)_HASH_FILE
> + ifdef $(3)_HASH_FILE
> + $(2)_HASH_FILE = $$($(3)_HASH_FILE)
> + else
> + $(2)_HASH_FILE = \
> + $$(strip \
> + $$(if $$(wildcard $$($(2)_PKGDIR)/$$($(2)_VERSION)/$$($(2)_RAWNAME).hash),\
> + $$($(2)_PKGDIR)/$$($(2)_VERSION)/$$($(2)_RAWNAME).hash,\
> + $$($(2)_PKGDIR)/$$($(2)_RAWNAME).hash))
> + endif
> +endif
>
> ifdef $(3)_OVERRIDE_SRCDIR
> $(2)_OVERRIDE_SRCDIR ?= $$($(3)_OVERRIDE_SRCDIR)
> --
> 2.17.1
>
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> https://urldefense.com/v3/__http://lists.busybox.net/mailman/listinfo/
> buildroot__;!!FJ-Y8qCqXTj2!Ib7PbPme0RX22G2i-BhfFcKUQMYUiXUoweoBdMRO8FN
> -e4FB5Lex2Q_f9l7GOIU$
--
.-----------------.--------------------.------------------.--------------------.
| Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics'
| conspiracy: |
| +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ |
| +33 561 099 427 `------------.-------: X AGAINST | \e/ There is no |
| https://urldefense.com/v3/__http://ymorin.is-a-geek.org/__;!!FJ-Y8qCqXTj2!Ib7PbPme0RX22G2i-BhfFcKUQMYUiXUoweoBdMRO8FN-e4FB5Lex2Q_f1TK7bOU$ | _/*\_ | / \ HTML MAIL | v conspiracy. |
'------------------------------^-------^------------------^--------------------'
More information about the buildroot
mailing list