[1/1] package/ipmitool: only accept local PEN registry

Message ID 20240902210055.239859-1-dario.binacchi@amarulasolutions.com
State New
Headers show
Series
  • [1/1] package/ipmitool: only accept local PEN registry
Related show

Commit Message

Dario Binacchi Sept. 2, 2024, 9 p.m. UTC
The https://www.iana.org/assignments/enterprise-numbers.txt is not a
versioned URL, and that file is regularly updated, so it is not
acceptable to change its hash each time.

Following Yann's suggestions [1], only local files are now accepted,
removing the need to download the PEN registry and consequently verify
the hash's correctness.

Fixes:
- http://autobuild.buildroot.org/results/5ae5ee948d99679cd50d1115a7d46f4368347b4f

[1]: https://patchwork.ozlabs.org/project/buildroot/patch/20240824103634.1955431-1-dario.binacchi@amarulasolutions.com/
Co-Developed-by: Yann E. MORIN <yann.morin.1998@free.fr>
Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
---
 package/ipmitool/Config.in   | 12 ++++++------
 package/ipmitool/ipmitool.mk | 13 +++++--------
 2 files changed, 11 insertions(+), 14 deletions(-)

Comments

Yann E. MORIN Sept. 3, 2024, 7:41 p.m. UTC | #1
Thomas, All,

On 2024-09-03 21:33 +0200, Thomas Petazzoni spake thusly:
> On Mon,  2 Sep 2024 23:00:55 +0200
> Dario Binacchi <dario.binacchi@amarulasolutions.com> wrote:
> > Following Yann's suggestions [1], only local files are now accepted,
> > removing the need to download the PEN registry and consequently verify
> > the hash's correctness.
[--SNIP--]
> I don't like this solution, having to download separately a file is
> annoying.
> Why not using:
>   https://github.com/larseggert/iana-assignments
> which precisely provides a versioned set of IANA documents, including
> that enterprise-numbers file?

Damned, I looked but could not find it something like that... How did
you find that?

Of course it is a better solution!

But should it be a separate package, that installs a subset of the data
files, or should it be an IPMITOOL_EXTRA_DOWNLOAD?

Regards,
Yann E. MORIN.
'Krzysztof Kozlowski' via Amarula Linux Sept. 4, 2024, 7:16 a.m. UTC | #2
Hello,

On Tue, 3 Sep 2024 21:41:03 +0200
"Yann E. MORIN" <yann.morin.1998@free.fr> wrote:

> Damned, I looked but could not find it something like that... How did
> you find that?

I've used this thing called G-o-o-g-l-e :-)

> But should it be a separate package, that installs a subset of the data
> files, or should it be an IPMITOOL_EXTRA_DOWNLOAD?

I don't have a super strong opinion. I think a separate package would
make sense if multiple packages needed this IANA data. If that's not
the case, I would handle it with an EXTRA_DOWNLOADS in ipmitool (and we
can always change it to a separate package later on).

Thomas

Patch

diff --git a/package/ipmitool/Config.in b/package/ipmitool/Config.in
index 9516ff8596d3..fefa8c34fd7f 100644
--- a/package/ipmitool/Config.in
+++ b/package/ipmitool/Config.in
@@ -9,14 +9,14 @@  config BR2_PACKAGE_IPMITOOL
 
 if BR2_PACKAGE_IPMITOOL
 
-config BR2_PACKAGE_IPMITOOL_PEN_REG_URI
-	string "IANA PEN registry URL or path"
-	default "https://www.iana.org/assignments/enterprise-numbers.txt"
+config BR2_PACKAGE_IPMITOOL_PEN_REG_PATH
+	string "PEN registry path"
+	default ""
 	help
-	  Enter an URL or a file path to the PEN registry to use.
+	  Enter file path to the PEN registry to use.
 
-	  Note that the official registry is 4MiB+ and may change any
-	  time and is thus not guaranteed to be reproducible.
+	  The official registry (4MiB+) can be downloaded from:
+	  https://www.iana.org/assignments/enterprise-numbers.txt
 
 	  Leave empty to not use a registry; vendor IDs will be
 	  displayed instead of the corresponding names.
diff --git a/package/ipmitool/ipmitool.mk b/package/ipmitool/ipmitool.mk
index 4f2151904d43..40bc8bb9dd13 100644
--- a/package/ipmitool/ipmitool.mk
+++ b/package/ipmitool/ipmitool.mk
@@ -49,20 +49,17 @@  endef
 IPMITOOL_POST_INSTALL_TARGET_HOOKS += IPMITOOL_REMOVE_IPMIEVD
 endif
 
-IPMITOOL_PEN_REG_URI = $(call qstrip,$(BR2_PACKAGE_IPMITOOL_PEN_REG_URI))
-ifneq ($(IPMITOOL_PEN_REG_URI),)
-ifneq ($(findstring ://,$(IPMITOOL_PEN_REG_URI)),)
-IPMITOOL_EXTRA_DOWNLOADS += $(IPMITOOL_PEN_REG_URI)
-BR_NO_CHECK_HASH_FOR += $(notdir $(IPMITOOL_PEN_REG_URI))
-IPMITOOL_PEN_REG = $(IPMITOOL_DL_DIR)/$(notdir $(IPMITOOL_PEN_REG_URI))
+IPMITOOL_PEN_REG = $(call qstrip,$(BR2_PACKAGE_IPMITOOL_PEN_REG_PATH))
+ifneq ($(IPMITOOL_PEN_REG),)
+ifneq ($(findstring ://,$(IPMITOOL_PEN_REG)),)
+$(error "URL paths are no supported")
 else
-IPMITOOL_PEN_REG = $(IPMITOOL_PEN_REG_URI)
 endif #findstring
 define IPMITOOL_INSTALL_PEN_REG
 	$(INSTALL) -D -m 0644 $(IPMITOOL_PEN_REG) \
 		$(TARGET_DIR)/usr/share/misc/enterprise-numbers
 endef
 IPMITOOL_POST_INSTALL_TARGET_HOOKS += IPMITOOL_INSTALL_PEN_REG
-endif # IPMITOOL_PEN_REG_URI !empty
+endif # IPMITOOL_PEN_REG !empty
 
 $(eval $(autotools-package))