* [Buildroot] [PATCH 1/1] Allow users to specifiy a hash file to verify custom linux kernels and custom kernel headers
@ 2021-06-11 17:20 Ian Merin
2021-06-12 21:06 ` Yann E. MORIN
0 siblings, 1 reply; 5+ messages in thread
From: Ian Merin @ 2021-06-11 17:20 UTC (permalink / raw)
To: buildroot
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Buildroot] [PATCH 1/1] Allow users to specifiy a hash file to verify custom linux kernels and custom kernel headers
2021-06-11 17:20 [Buildroot] [PATCH 1/1] Allow users to specifiy a hash file to verify custom linux kernels and custom kernel headers Ian Merin
@ 2021-06-12 21:06 ` Yann E. MORIN
2021-06-13 7:14 ` Thomas Petazzoni
2021-06-13 22:21 ` [Buildroot] [EXTERNAL] " Ian Merin
0 siblings, 2 replies; 5+ messages in thread
From: Yann E. MORIN @ 2021-06-12 21:06 UTC (permalink / raw)
To: buildroot
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@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@nCipher.com>
> Signed-off-by: Ian Merin <Ian.Merin@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)$(BR2_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
> http://lists.busybox.net/mailman/listinfo/buildroot
--
.-----------------.--------------------.------------------.--------------------.
| 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 |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |
'------------------------------^-------^------------------^--------------------'
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Buildroot] [PATCH 1/1] Allow users to specifiy a hash file to verify custom linux kernels and custom kernel headers
2021-06-12 21:06 ` Yann E. MORIN
@ 2021-06-13 7:14 ` Thomas Petazzoni
2021-06-13 8:59 ` Arnout Vandecappelle
2021-06-13 22:21 ` [Buildroot] [EXTERNAL] " Ian Merin
1 sibling, 1 reply; 5+ messages in thread
From: Thomas Petazzoni @ 2021-06-13 7:14 UTC (permalink / raw)
To: buildroot
Hello,
On Sat, 12 Jun 2021 23:06:27 +0200
"Yann E. MORIN" <yann.morin.1998@free.fr> wrote:
> 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:
I totally agree with this, and wanted to reply the same to Ian's patch.
> +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.
However, I am wondering if we shouldn't be doing something even more
generic.
We already have the BR2_GLOBAL_PATCH_DIRECTORIES option to add custom
patches to package.
Here we have a proposal to address the case of hash files for those
packages where a custom version can be specified. But for such
packages, we also have other aspects that are not nicely handled today:
* The license files + their hashes.
* The CPE ID information, as the version of such packages (typically
some random Git commit or tag) doesn't allow proper matching with
the CPE database version.
Shouldn't we have these requirements in mind as well when trying to
come up with a solution ?
Best regards,
Thomas
--
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Buildroot] [PATCH 1/1] Allow users to specifiy a hash file to verify custom linux kernels and custom kernel headers
2021-06-13 7:14 ` Thomas Petazzoni
@ 2021-06-13 8:59 ` Arnout Vandecappelle
0 siblings, 0 replies; 5+ messages in thread
From: Arnout Vandecappelle @ 2021-06-13 8:59 UTC (permalink / raw)
To: buildroot
I see a danger here that we fail to merge a fairly simple patch because we're
expecting something that "solves the world"...
On 13/06/2021 09:14, Thomas Petazzoni wrote:
> Hello,
>
> On Sat, 12 Jun 2021 23:06:27 +0200
> "Yann E. MORIN" <yann.morin.1998@free.fr> wrote:
>
>> 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:
>
> I totally agree with this,
Same here.
> and wanted to reply the same to Ian's patch.
>
>> +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.
>
> However, I am wondering if we shouldn't be doing something even more
> generic.
>
> We already have the BR2_GLOBAL_PATCH_DIRECTORIES option to add custom
> patches to package.
The problem is that we have a lot of different ways already to customize packages:
- GLOBAL_PATCH_DIR
- _OVERRIDE_SRCDIR
- custom package in BR2_EXTERNAL
- for some packages: specify alternative version/tarball/git repo
So keeping all of this consistent is getting harder and harder.
> Here we have a proposal to address the case of hash files for those
> packages where a custom version can be specified. But for such
> packages, we also have other aspects that are not nicely handled today:
>
> * The license files + their hashes.
>
> * The CPE ID information, as the version of such packages (typically
> some random Git commit or tag) doesn't allow proper matching with
> the CPE database version.
>
> Shouldn't we have these requirements in mind as well when trying to
> come up with a solution ?
That would be nice, but I don't think we should block the merge of a simple
solution for this particular case because it doesn't "solve the world". So
basically I'd be OK with any solution:
- Ian's (but then slightly simplified as indicated by Yann);
- Yann's;
- mine (i.e. BR2_LINUX_KERNEL_CUSTOM_HASH is set directly to the hash).
Regards,
Arnout
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Buildroot] [EXTERNAL] Re: [PATCH 1/1] Allow users to specifiy a hash file to verify custom linux kernels and custom kernel headers
2021-06-12 21:06 ` Yann E. MORIN
2021-06-13 7:14 ` Thomas Petazzoni
@ 2021-06-13 22:21 ` Ian Merin
1 sibling, 0 replies; 5+ messages in thread
From: Ian Merin @ 2021-06-13 22:21 UTC (permalink / raw)
To: buildroot
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@free.fr>
Sent: Saturday, June 12, 2021 5:06 PM
To: Ian Merin <Ian.Merin@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@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@nCipher.com>
> Signed-off-by: Ian Merin <Ian.Merin@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. |
'------------------------------^-------^------------------^--------------------'
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-06-13 22:21 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-11 17:20 [Buildroot] [PATCH 1/1] Allow users to specifiy a hash file to verify custom linux kernels and custom kernel headers Ian Merin
2021-06-12 21:06 ` Yann E. MORIN
2021-06-13 7:14 ` Thomas Petazzoni
2021-06-13 8:59 ` Arnout Vandecappelle
2021-06-13 22:21 ` [Buildroot] [EXTERNAL] " Ian Merin
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.