[Buildroot] [PATCH 1/1] package/pkg-download: add per package download fallback disable

[Yann E. MORIN]

Justin, All,

On 2022-09-08 11:23 -0400, jwood+buildroot at starry.com spake thusly:
> From: Justin Wood <jwood+buildroot at starry.com>
> 
> This is useful in cases where a package is added without hashes (e.g. private packages)
> and you do not want to risk MITM attacks of the package itself.  While still allowing
> download of packages that are third party with hashes, from unreliable upstreams.
> 
> This adds a new ${PKG}_DISABLE_FALLBACK_DOWNLOAD that is checked when DOWNLOAD would be
> called to not include URIs from the backup site.

I think the best solution in such a case, is to actually add hashes for
internal packages anyway, because that allows one to ensure the
reproducibility of a build (e.g. if the package comes from git, it will
detect when/if a tag has been moved).

Additionally, I think internal setups should:

  - not use a backup site at all, i.e. BR2_BACKUP_SITE=""

  - use an internal primary mirror that points to an internal machine,
    e.g. BR2_PRIMARY_SITE="https://internal.my-company/storage/buildroot/"
    and manually fill it with the sources needed by the project, like in
    running:
        $ make my_board_defconfig
        $ BR2_DL_DIR=$(pwd)/dl make source
        $ scp -r dl user at internal.my-company/storage/buildroot/
    If something a bit more fancy is needed, then one can use a bit of
    scripting around the output of "make show-info" to only handle URIs
    of interest.

  - block downloads from the internet to avoid unexpectedly downloading
    data that has not been vetoed yet, e.g. build in a container that
    does not have routes to go outside company network, or has firewall
    rules to DROP packets going outside.

This, too ensures that a build is reproducible, as all the sources are
on company servers and thus there is no log-term reliance on an external
entity that may remove/change sources arbitrarily; this is not
hypothetical at all, that already happened (hence one of the reasons for
the hashes we have to begin with).

I.e. I think this type of behaviour is best served by the environment
and the setup, rather than by adding new features in Buildroot.

Regards,
Yann E. MORIN.

> Additionally we use the new backup URIs if the new variable is unset in the json data
> URI list to ensure consistency for consumers who do not use this feature.
> 
> Signed-off-by: Justin Wood <jwood at starry.com>
> ---
>  package/pkg-download.mk | 9 +++++++--
>  package/pkg-utils.mk    | 5 +++++
>  2 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/package/pkg-download.mk b/package/pkg-download.mk
> index 0718f21aad..af5855230c 100644
> --- a/package/pkg-download.mk
> +++ b/package/pkg-download.mk
> @@ -74,8 +74,12 @@ export BR_NO_CHECK_HASH_FOR =
>  # DOWNLOAD_URIS - List the candidates URIs where to get the package from:
>  # 1) BR2_PRIMARY_SITE if enabled
>  # 2) Download site, unless BR2_PRIMARY_SITE_ONLY is set
> -# 3) BR2_BACKUP_SITE if enabled, unless BR2_PRIMARY_SITE_ONLY is set
>  #
> +# BACKUP_DOWNLOAD_URIS - List the backup candidate URIs where to get packages from:
> +# 1) BR2_BACKUP_SITE if enabled, unless BR2_PRIMARY_SITE_ONLY is set
> +#    and unless ${PKG}_DISABLE_DOWNLOAD_FALLBACK is set
> +#
> +# In both vars above:
>  # Argument 1 is the source location
>  # Argument 2 is the upper-case package name
>  #
> @@ -91,7 +95,7 @@ ifeq ($(BR2_PRIMARY_SITE_ONLY),)
>  DOWNLOAD_URIS += \
>  	$(patsubst %/,%,$(dir $(call qstrip,$(1))))
>  ifneq ($(call qstrip,$(BR2_BACKUP_SITE)),)
> -DOWNLOAD_URIS += \
> +BACKUP_DOWNLOAD_URIS += \
>  	$(call getschemeplusuri,$(call qstrip,$(BR2_BACKUP_SITE)/$($(2)_DL_SUBDIR)),urlencode) \
>  	$(call getschemeplusuri,$(call qstrip,$(BR2_BACKUP_SITE)),urlencode)
>  endif
> @@ -122,6 +126,7 @@ define DOWNLOAD
>  		$(if $($(2)_GIT_SUBMODULES),-r) \
>  		$(if $($(2)_GIT_LFS),-l) \
>  		$(foreach uri,$(call DOWNLOAD_URIS,$(1),$(2)),-u $(uri)) \
> +		$(if( $($(PKG)_DISABLE_DOWNLOAD_FALLBACK),,$(foreach uri,$(call BACKUP_DOWNLOAD_URIS,$(1),$(2)),-u $(uri))) \
>  		$(3) \
>  		$(QUIET) \
>  		-- \
> diff --git a/package/pkg-utils.mk b/package/pkg-utils.mk
> index 6ece27baa2..a279a41df8 100644
> --- a/package/pkg-utils.mk
> +++ b/package/pkg-utils.mk
> @@ -167,6 +167,11 @@ define _json-info-pkg-details
>  					$(foreach uri,$(call DOWNLOAD_URIS,$(dl),$(1)), \
>  						$(call mk-json-str,$(subst \|,|,$(uri))) \
>  					) \
> +                                        $(if $($(PKG)_DISABLE_DOWNLOAD_FALLBACK),,\
> +						$(foreach uri,$(call BACKUP_DOWNLOAD_URIS,$(dl),$(1)), \
> +							$(call mk-json-str,$(subst \|,|,$(uri))) \
> +						) \
> +					) \
>  				)
>  			]
>  		},
> -- 
> 2.37.2
> 
> _______________________________________________
> buildroot mailing list
> buildroot at buildroot.org
> https://lists.buildroot.org/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.  |
'------------------------------^-------^------------------^--------------------'