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

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

[jwood+buildroot]

From: Justin Wood <jwood+buildroot@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.

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@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@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [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@starry.com spake thusly:
> From: Justin Wood <jwood+buildroot@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@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@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@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.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

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

[Thomas Petazzoni via buildroot]

Hello Justin,

On Thu,  8 Sep 2022 11:23:30 -0400
jwood+buildroot@starry.com wrote:

> From: Justin Wood <jwood+buildroot@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.
> 
> 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@starry.com>

We just had a discussion with Peter Korsgaard, and it seems like we
agree with the feedback from Yann. If you're really concerned about
MITM attacks, you should have hashes in your packages, and generally
speaking if you're concerned about "leaking" information about the fact
that you're building something, you should disable using
BR2_BACKUP_SITE.

However, instead of just saying no to this, we put a bit of thought
into it. What we don't like is that you're adding yet another very
specific variable that touches a very particular aspect of the package
behavior. Instead, we are thinking it might make sense to have a
variable that tells Buildroot the package is "private" or "internal"
(or some other similar naming), as opposed to the rest of the
open-source packages. This could tell Buildroot to not use the backup
site for this package, but also not mention the package in the
legal-info output. It should be noted that we already have the
<pkg>_REDISTRIBUTE = YES/NO boolean, but it only controls whether the
source code gets copied into the legal-info output: even with
<pkg>_REDISTRIBUTE = NO, the package gets listed in the legal-info
manifest. I personally believe it would make more sense to have a
variable that says the package is internal/private, and from that
derive the necessary tweaks to the download and legal-info behavior. I
don't have a good name for this variable though :-/

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

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

[Flávio Tapajós]

Sorry to necrobump, but such a feature would be handy when using 
FOO_DL_OPTS to ensure http authentication.

Tokens, usernames or passwords will be leaked to the fallback server in 
case of failure of the primary source. This could be some source of 
vulnerability

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

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

[Yann E. MORIN]

Flávio, All,

On 2024-04-30 14:56 -0300, Flávio Tapajós spake thusly:
> Sorry to necrobump, but such a feature would be handy when using FOO_DL_OPTS
> to ensure http authentication.
> 
> Tokens, usernames or passwords will be leaked to the fallback server in case
> of failure of the primary source. This could be some source of vulnerability

You've had feedback from both Thomas and I about that patch:

    https://lore.kernel.org/buildroot/20220911074734.GF264214@scaer/
    https://lore.kernel.org/buildroot/20220917205253.3737d1c6@windsurf/

So I don't see what reason there is to poke about this patch.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

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

[Arnout Vandecappelle via buildroot]

On 30/04/2024 19:56, Flávio Tapajós wrote:
> Sorry to necrobump, but such a feature would be handy when using FOO_DL_OPTS to 
> ensure http authentication.
> 
> Tokens, usernames or passwords will be leaked to the fallback server in case of 
> failure of the primary source. This could be some source of vulnerability

  I don't see how any of these would be leaked...

  A token is passed either as a password or as a header. There's no way to 
specify per-package wget options anyway, so I guess it will be passed as a password.

  A password can either be passed in .netrc or .wgetrc (in which case it is 
site-specific so won't be sent to PRIMARY_SITE or BACKUP_SITE), or it can be 
passed in the URL, as https://user:pass@server.com/path/source.tar.gz
The whole user:pass part also doesn't get used in PRIMARY_SITE or BACKUP_SITE.

  The above applies to http downloads. For other downloads, it doesn't even 
matter at all, because that download method won't be used for PRIMARY_SITE and 
SECONDARY_SITE so any authentication stuff won't be used either.

  So how does leaking take place?

  Regards,
  Arnout

> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

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

[Yann E. MORIN]

Arnout, All,

On 2024-05-01 21:09 +0200, Arnout Vandecappelle via buildroot spake thusly:
> On 30/04/2024 19:56, Flávio Tapajós wrote:
> > Sorry to necrobump, but such a feature would be handy when using
> > FOO_DL_OPTS to ensure http authentication.
> > 
> > Tokens, usernames or passwords will be leaked to the fallback server in
> > case of failure of the primary source. This could be some source of
> > vulnerability
> 
>  I don't see how any of these would be leaked...
> 
>  A token is passed either as a password or as a header. There's no way to
> specify per-package wget options anyway,

It *is* possible to pass per-package download options:

    FOO_DL_OPTS = --header 'Auth-Token: my-secret'

And those options are passed to all the downloads tentatives. This of
course only works properly when the main download is done with wget,
though, as options for git-as-main are not valid for wget-as-primary or
wget-as-backup.

Note that we used to have such a package in Buildroot, amd-catalyst, but
it was removed a while ago. Still, we're testing that in outr download
runtime tests, for sftp and scp, so we know it works...

Regards,
Yann E. MORIN.

> so I guess it will be passed as a
> password.
> 
>  A password can either be passed in .netrc or .wgetrc (in which case it is
> site-specific so won't be sent to PRIMARY_SITE or BACKUP_SITE), or it can be
> passed in the URL, as https://user:pass@server.com/path/source.tar.gz
> The whole user:pass part also doesn't get used in PRIMARY_SITE or BACKUP_SITE.
> 
>  The above applies to http downloads. For other downloads, it doesn't even
> matter at all, because that download method won't be used for PRIMARY_SITE
> and SECONDARY_SITE so any authentication stuff won't be used either.
> 
>  So how does leaking take place?
> 
>  Regards,
>  Arnout
> 
> > 
> > _______________________________________________
> > buildroot mailing list
> > buildroot@buildroot.org
> > https://lists.buildroot.org/mailman/listinfo/buildroot
> _______________________________________________
> buildroot mailing list
> buildroot@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.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot