[OpenWrt-Devel] ath79: make UBNT Nano/Loco AC factory images reproducible
diff mbox series

Message ID 20191007082204.30380-1-ynezz@true.cz
State Rejected
Delegated to: Petr Štetiar
Headers show
Series
  • [OpenWrt-Devel] ath79: make UBNT Nano/Loco AC factory images reproducible
Related show

Commit Message

Petr Štetiar Oct. 7, 2019, 8:22 a.m. UTC
Current factory images are built on top of sysupgrade images which
contains metadata which are causing image reproducibility issues, so
let's build factory images from the scratch. While at it, refactor the
shared vars into common base as well.

Ref: http://lists.infradead.org/pipermail/openwrt-devel/2019-October/019205.html
Reported-by: Paul Spooren <mail@aparcar.org>
Signed-off-by: Petr Štetiar <ynezz@true.cz>
---
 target/linux/ath79/image/generic-ubnt.mk | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

Comments

Paul Spooren Oct. 7, 2019, 9:59 a.m. UTC | #1
On 10/6/19 10:22 PM, Petr Štetiar wrote:
> Current factory images are built on top of sysupgrade images which
> contains metadata which are causing image reproducibility issues, so
> let's build factory images from the scratch. While at it, refactor the
> shared vars into common base as well.

Compiled and the signature is (obviously) gone, perfect!

Can't do any runtime tests, but I guess it's good for merging!

Thanks for the quick response!

Paul

>
> Ref: http://lists.infradead.org/pipermail/openwrt-devel/2019-October/019205.html
> Reported-by: Paul Spooren <mail@aparcar.org>
> Signed-off-by: Petr Štetiar <ynezz@true.cz>
> ---
>   target/linux/ath79/image/generic-ubnt.mk | 24 ++++++++++++------------
>   1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/target/linux/ath79/image/generic-ubnt.mk b/target/linux/ath79/image/generic-ubnt.mk
> index 6ae766e29331..9ab11324b411 100644
> --- a/target/linux/ath79/image/generic-ubnt.mk
> +++ b/target/linux/ath79/image/generic-ubnt.mk
> @@ -114,40 +114,40 @@ define Device/ubnt_bullet-m-xw
>   endef
>   TARGET_DEVICES += ubnt_bullet-m-xw
>   
> +define Device/ubnt-nano-ac
> +  DEVICE_PACKAGES += kmod-ath10k-ct ath10k-firmware-qca988x-ct
> +  IMAGE_SIZE := 15744k
> +  IMAGE/factory.bin := append-kernel | pad-to $$$$(BLOCKSIZE) | \
> +	append-rootfs | pad-rootfs | mkubntimage-split | \
> +	check-size $$$$(IMAGE_SIZE)
> +endef
> +
>   define Device/ubnt_lap-120
>     $(Device/ubnt-wa)
>     DEVICE_MODEL := LiteAP ac
>     DEVICE_VARIANT := LAP-120
> -  DEVICE_PACKAGES += kmod-ath10k-ct ath10k-firmware-qca988x-ct
> -  IMAGE_SIZE := 15744k
> -  IMAGE/factory.bin := $$(IMAGE/sysupgrade.bin) | mkubntimage-split
> +  $(Device/ubnt-nano-ac)
>   endef
>   TARGET_DEVICES += ubnt_lap-120
>   
>   define Device/ubnt_nanobeam-ac
>     $(Device/ubnt-wa)
>     DEVICE_MODEL := NanoBeam AC
> -  DEVICE_PACKAGES += kmod-ath10k-ct ath10k-firmware-qca988x-ct
> -  IMAGE_SIZE := 15744k
> -  IMAGE/factory.bin := $$(IMAGE/sysupgrade.bin) | mkubntimage-split
> +  $(Device/ubnt-nano-ac)
>   endef
>   TARGET_DEVICES += ubnt_nanobeam-ac
>   
>   define Device/ubnt_nanostation-ac
>     $(Device/ubnt-wa)
>     DEVICE_MODEL := Nanostation AC
> -  DEVICE_PACKAGES += kmod-ath10k-ct ath10k-firmware-qca988x-ct
> -  IMAGE_SIZE := 15744k
> -  IMAGE/factory.bin := $$(IMAGE/sysupgrade.bin) | mkubntimage-split
> +  $(Device/ubnt-nano-ac)
>   endef
>   TARGET_DEVICES += ubnt_nanostation-ac
>   
>   define Device/ubnt_nanostation-ac-loco
>     $(Device/ubnt-wa)
>     DEVICE_MODEL := Nanostation AC loco
> -  DEVICE_PACKAGES += kmod-ath10k-ct ath10k-firmware-qca988x-ct
> -  IMAGE_SIZE := 15744k
> -  IMAGE/factory.bin := $$(IMAGE/sysupgrade.bin) | mkubntimage-split
> +  $(Device/ubnt-nano-ac)
>   endef
>   TARGET_DEVICES += ubnt_nanostation-ac-loco
>   
>
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Adrian Schmutzler Oct. 7, 2019, 10:40 a.m. UTC | #2
Hi Petr,

> -----Original Message-----
> From: openwrt-devel [mailto:openwrt-devel-bounces@lists.openwrt.org] On Behalf Of Petr Štetiar
> Sent: Montag, 7. Oktober 2019 10:22
> To: openwrt-devel@lists.openwrt.org
> Cc: Petr Štetiar <ynezz@true.cz>
> Subject: [OpenWrt-Devel] [PATCH] ath79: make UBNT Nano/Loco AC factory images reproducible
> 
> Current factory images are built on top of sysupgrade images which
> contains metadata which are causing image reproducibility issues, so
> let's build factory images from the scratch. While at it, refactor the
> shared vars into common base as well.
> 
> Ref: http://lists.infradead.org/pipermail/openwrt-devel/2019-October/019205.html
> Reported-by: Paul Spooren <mail@aparcar.org>
> Signed-off-by: Petr Štetiar <ynezz@true.cz>
> ---
>  target/linux/ath79/image/generic-ubnt.mk | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/target/linux/ath79/image/generic-ubnt.mk b/target/linux/ath79/image/generic-ubnt.mk
> index 6ae766e29331..9ab11324b411 100644
> --- a/target/linux/ath79/image/generic-ubnt.mk
> +++ b/target/linux/ath79/image/generic-ubnt.mk
> @@ -114,40 +114,40 @@ define Device/ubnt_bullet-m-xw
>  endef
>  TARGET_DEVICES += ubnt_bullet-m-xw
> 
> +define Device/ubnt-nano-ac
> +  DEVICE_PACKAGES += kmod-ath10k-ct ath10k-firmware-qca988x-ct
> +  IMAGE_SIZE := 15744k
> +  IMAGE/factory.bin := append-kernel | pad-to $$$$(BLOCKSIZE) | \
> +	append-rootfs | pad-rootfs | mkubntimage-split | \
> +	check-size $$$$(IMAGE_SIZE)
> +endef
> +
>  define Device/ubnt_lap-120
>    $(Device/ubnt-wa)
>    DEVICE_MODEL := LiteAP ac
>    DEVICE_VARIANT := LAP-120
> -  DEVICE_PACKAGES += kmod-ath10k-ct ath10k-firmware-qca988x-ct
> -  IMAGE_SIZE := 15744k
> -  IMAGE/factory.bin := $$(IMAGE/sysupgrade.bin) | mkubntimage-split
> +  $(Device/ubnt-nano-ac)

If you call it ubnt-nano-ac anyway, you could also include the "$(Device/ubnt-wa)" in the common node.

One could also think about naming it ubnt_nano-ac (with underscore) for consistency, like what we have for ubnt_routerstation_common and ubnt_unifiac.

Best

Adrian
Paul Spooren Oct. 7, 2019, 7:52 p.m. UTC | #3
The issue appear for amips-mt7620-zyxel_keenetic-omni-ii[0] as well.

[0]: 
https://rebuild.aparcar.org/SNAPSHOT/ramips/mt7620/openwrt-ramips-mt7620-zyxel_keenetic-omni-ii-squashfs-factory.bin.html

On 10/6/19 10:22 PM, Petr Štetiar wrote:
> Current factory images are built on top of sysupgrade images which
> contains metadata which are causing image reproducibility issues, so
> let's build factory images from the scratch. While at it, refactor the
> shared vars into common base as well.
>
> Ref: http://lists.infradead.org/pipermail/openwrt-devel/2019-October/019205.html
> Reported-by: Paul Spooren <mail@aparcar.org>
> Signed-off-by: Petr Štetiar <ynezz@true.cz>
> ---
>   target/linux/ath79/image/generic-ubnt.mk | 24 ++++++++++++------------
>   1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/target/linux/ath79/image/generic-ubnt.mk b/target/linux/ath79/image/generic-ubnt.mk
> index 6ae766e29331..9ab11324b411 100644
> --- a/target/linux/ath79/image/generic-ubnt.mk
> +++ b/target/linux/ath79/image/generic-ubnt.mk
> @@ -114,40 +114,40 @@ define Device/ubnt_bullet-m-xw
>   endef
>   TARGET_DEVICES += ubnt_bullet-m-xw
>   
> +define Device/ubnt-nano-ac
> +  DEVICE_PACKAGES += kmod-ath10k-ct ath10k-firmware-qca988x-ct
> +  IMAGE_SIZE := 15744k
> +  IMAGE/factory.bin := append-kernel | pad-to $$$$(BLOCKSIZE) | \
> +	append-rootfs | pad-rootfs | mkubntimage-split | \
> +	check-size $$$$(IMAGE_SIZE)
> +endef
> +
>   define Device/ubnt_lap-120
>     $(Device/ubnt-wa)
>     DEVICE_MODEL := LiteAP ac
>     DEVICE_VARIANT := LAP-120
> -  DEVICE_PACKAGES += kmod-ath10k-ct ath10k-firmware-qca988x-ct
> -  IMAGE_SIZE := 15744k
> -  IMAGE/factory.bin := $$(IMAGE/sysupgrade.bin) | mkubntimage-split
> +  $(Device/ubnt-nano-ac)
>   endef
>   TARGET_DEVICES += ubnt_lap-120
>   
>   define Device/ubnt_nanobeam-ac
>     $(Device/ubnt-wa)
>     DEVICE_MODEL := NanoBeam AC
> -  DEVICE_PACKAGES += kmod-ath10k-ct ath10k-firmware-qca988x-ct
> -  IMAGE_SIZE := 15744k
> -  IMAGE/factory.bin := $$(IMAGE/sysupgrade.bin) | mkubntimage-split
> +  $(Device/ubnt-nano-ac)
>   endef
>   TARGET_DEVICES += ubnt_nanobeam-ac
>   
>   define Device/ubnt_nanostation-ac
>     $(Device/ubnt-wa)
>     DEVICE_MODEL := Nanostation AC
> -  DEVICE_PACKAGES += kmod-ath10k-ct ath10k-firmware-qca988x-ct
> -  IMAGE_SIZE := 15744k
> -  IMAGE/factory.bin := $$(IMAGE/sysupgrade.bin) | mkubntimage-split
> +  $(Device/ubnt-nano-ac)
>   endef
>   TARGET_DEVICES += ubnt_nanostation-ac
>   
>   define Device/ubnt_nanostation-ac-loco
>     $(Device/ubnt-wa)
>     DEVICE_MODEL := Nanostation AC loco
> -  DEVICE_PACKAGES += kmod-ath10k-ct ath10k-firmware-qca988x-ct
> -  IMAGE_SIZE := 15744k
> -  IMAGE/factory.bin := $$(IMAGE/sysupgrade.bin) | mkubntimage-split
> +  $(Device/ubnt-nano-ac)
>   endef
>   TARGET_DEVICES += ubnt_nanostation-ac-loco
>   
>
> _______________________________________________
> openwrt-devel mailing list
> openwrt-devel@lists.openwrt.org
> https://lists.openwrt.org/mailman/listinfo/openwrt-devel
Petr Štetiar Oct. 8, 2019, 4:38 a.m. UTC | #4
Paul Spooren <mail@aparcar.org> [2019-10-07 09:52:23]:

Hi,

> The issue appear for amips-mt7620-zyxel_keenetic-omni-ii[0] as well.

as discussed on IRC, this issue is caused by your custom build step[1] and
doesn't exist in the tree, so the proposed patch can be seen just a workaround
and not proper fix so I've rejected it and not going to apply.

This should be fixed in other places:

 < jow> to me it looks as if two distinct fixes are needed
 < jow> 1) better fwtool signature search heuristics
 < jow> 2) pad the signature before it is factored into the partition size calculation

1. https://gitlab.com/aparcar/rebuild/blob/master/rebuild.py#L146

-- ynezz
Paul Spooren Oct. 8, 2019, 6:50 a.m. UTC | #5
Hi,

On 10/7/19 6:38 PM, Petr Štetiar wrote:
> as discussed on IRC, this issue is caused by your custom build step[1] and
> doesn't exist in the tree, so the proposed patch can be seen just a workaround
> and not proper fix so I've rejected it and not going to apply.
Fine for me if the enhancements mentioned below are implemented at some 
point. For now the padding causes images to be unreproducible which I'd 
consider as a upstream problem. The `exchange_signature` step is the 
only way I can think of to make signed images reproducible...
> This should be fixed in other places:
>
>   < jow> to me it looks as if two distinct fixes are needed
>   < jow> 1) better fwtool signature search heuristics
>   < jow> 2) pad the signature before it is factored into the partition size calculation
>
> 1. https://gitlab.com/aparcar/rebuild/blob/master/rebuild.py#L146
>
> -- ynezz

Patch
diff mbox series

diff --git a/target/linux/ath79/image/generic-ubnt.mk b/target/linux/ath79/image/generic-ubnt.mk
index 6ae766e29331..9ab11324b411 100644
--- a/target/linux/ath79/image/generic-ubnt.mk
+++ b/target/linux/ath79/image/generic-ubnt.mk
@@ -114,40 +114,40 @@  define Device/ubnt_bullet-m-xw
 endef
 TARGET_DEVICES += ubnt_bullet-m-xw
 
+define Device/ubnt-nano-ac
+  DEVICE_PACKAGES += kmod-ath10k-ct ath10k-firmware-qca988x-ct
+  IMAGE_SIZE := 15744k
+  IMAGE/factory.bin := append-kernel | pad-to $$$$(BLOCKSIZE) | \
+	append-rootfs | pad-rootfs | mkubntimage-split | \
+	check-size $$$$(IMAGE_SIZE)
+endef
+
 define Device/ubnt_lap-120
   $(Device/ubnt-wa)
   DEVICE_MODEL := LiteAP ac
   DEVICE_VARIANT := LAP-120
-  DEVICE_PACKAGES += kmod-ath10k-ct ath10k-firmware-qca988x-ct
-  IMAGE_SIZE := 15744k
-  IMAGE/factory.bin := $$(IMAGE/sysupgrade.bin) | mkubntimage-split
+  $(Device/ubnt-nano-ac)
 endef
 TARGET_DEVICES += ubnt_lap-120
 
 define Device/ubnt_nanobeam-ac
   $(Device/ubnt-wa)
   DEVICE_MODEL := NanoBeam AC
-  DEVICE_PACKAGES += kmod-ath10k-ct ath10k-firmware-qca988x-ct
-  IMAGE_SIZE := 15744k
-  IMAGE/factory.bin := $$(IMAGE/sysupgrade.bin) | mkubntimage-split
+  $(Device/ubnt-nano-ac)
 endef
 TARGET_DEVICES += ubnt_nanobeam-ac
 
 define Device/ubnt_nanostation-ac
   $(Device/ubnt-wa)
   DEVICE_MODEL := Nanostation AC
-  DEVICE_PACKAGES += kmod-ath10k-ct ath10k-firmware-qca988x-ct
-  IMAGE_SIZE := 15744k
-  IMAGE/factory.bin := $$(IMAGE/sysupgrade.bin) | mkubntimage-split
+  $(Device/ubnt-nano-ac)
 endef
 TARGET_DEVICES += ubnt_nanostation-ac
 
 define Device/ubnt_nanostation-ac-loco
   $(Device/ubnt-wa)
   DEVICE_MODEL := Nanostation AC loco
-  DEVICE_PACKAGES += kmod-ath10k-ct ath10k-firmware-qca988x-ct
-  IMAGE_SIZE := 15744k
-  IMAGE/factory.bin := $$(IMAGE/sysupgrade.bin) | mkubntimage-split
+  $(Device/ubnt-nano-ac)
 endef
 TARGET_DEVICES += ubnt_nanostation-ac-loco