[Buildroot] [PATCH 1/6] Makefile: rename USER_HOOKS_EXTRA_ENV to EXTRA_ENV

[Buildroot] [PATCH 1/6] Makefile: rename USER_HOOKS_EXTRA_ENV to EXTRA_ENV

[Yann E. MORIN]

From: "Yann E. MORIN" <yann.morin.1998@free.fr>

This variable contains extra envirnoment variables that we can not export
since they are clashing with some build systems (eg. BUILD_DIR with
u-boot).

So, we may need these variables for other ueses than the user's hooks
for instrumentation. For example, we'll use them later on to export
BUILD_DIR to the download helper scripts.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
 Makefile               | 4 ++--
 package/Makefile.in    | 2 +-
 package/pkg-generic.mk | 2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index 7624f59..e22f1e4 100644
--- a/Makefile
+++ b/Makefile
@@ -552,7 +552,7 @@ endif
 
 	@$(foreach s, $(call qstrip,$(BR2_ROOTFS_POST_BUILD_SCRIPT)), \
 		$(call MESSAGE,"Executing post-build script $(s)"); \
-		$(USER_HOOKS_EXTRA_ENV) $(s) $(TARGET_DIR) $(call qstrip,$(BR2_ROOTFS_POST_SCRIPT_ARGS))$(sep))
+		$(EXTRA_ENV) $(s) $(TARGET_DIR) $(call qstrip,$(BR2_ROOTFS_POST_SCRIPT_ARGS))$(sep))
 
 ifeq ($(BR2_ENABLE_LOCALE_PURGE),y)
 LOCALE_WHITELIST=$(BUILD_DIR)/locales.nopurge
@@ -598,7 +598,7 @@ endif
 target-post-image:
 	@$(foreach s, $(call qstrip,$(BR2_ROOTFS_POST_IMAGE_SCRIPT)), \
 		$(call MESSAGE,"Executing post-image script $(s)"); \
-		$(USER_HOOKS_EXTRA_ENV) $(s) $(BINARIES_DIR) $(call qstrip,$(BR2_ROOTFS_POST_SCRIPT_ARGS))$(sep))
+		$(EXTRA_ENV) $(s) $(BINARIES_DIR) $(call qstrip,$(BR2_ROOTFS_POST_SCRIPT_ARGS))$(sep))
 
 toolchain-eclipse-register:
 	./support/scripts/eclipse-register-toolchain `readlink -f $(O)` $(notdir $(TARGET_CROSS)) $(BR2_ARCH)
diff --git a/package/Makefile.in b/package/Makefile.in
index f5d6289..b089f32 100644
--- a/package/Makefile.in
+++ b/package/Makefile.in
@@ -281,7 +281,7 @@ HOST_MAKE_ENV=PATH=$(HOST_PATH) \
 # packages use that variable internally, eg. uboot), so we have to
 # explicitly pass it to user-supplied external hooks (eg. post-build,
 # post-images)
-USER_HOOKS_EXTRA_ENV=\
+EXTRA_ENV=\
 	BUILD_DIR=$(BUILD_DIR)
 
 ################################################################################
diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 1b99c2a..e6fc558 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -58,7 +58,7 @@ GLOBAL_INSTRUMENTATION_HOOKS += step_time
 # User-supplied script
 define step_user
 	@$(foreach user_hook, $(BR2_INSTRUMENTATION_SCRIPTS), \
-		$(USER_HOOKS_EXTRA_ENV) $(user_hook) "$(1)" "$(2)" "$(3)"$(sep))
+		$(EXTRA_ENV) $(user_hook) "$(1)" "$(2)" "$(3)"$(sep))
 endef
 ifneq ($(BR2_INSTRUMENTATION_SCRIPTS),)
 GLOBAL_INSTRUMENTATION_HOOKS += step_user
-- 
1.8.1.2

[Buildroot] [PATCH 2/6] pkg-infra: move git download helper to a script

[Yann E. MORIN]

From: "Yann E. MORIN" <yann.morin.1998@free.fr>

The git download helper is getting a bit more complex, and does not
raise an error when a PKG_VERSION is a sha1 that does not exist in
the repository. Instead, it generates an empty archive.

Fixing it in the Makefile proves to be challenging, to say the least.

Move it into a shell script in support/download/git, which will make
it much easier to read, maintain, fix an enhance in the future.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Peter Korsgaard <jacmet@uclibc.org>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 package/pkg-download.mk | 16 +++-------------
 support/download/git    | 36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 39 insertions(+), 13 deletions(-)
 create mode 100755 support/download/git

diff --git a/package/pkg-download.mk b/package/pkg-download.mk
index c00689b..8e6f7a3 100644
--- a/package/pkg-download.mk
+++ b/package/pkg-download.mk
@@ -12,7 +12,7 @@ WGET := $(call qstrip,$(BR2_WGET)) $(QUIET)
 SVN := $(call qstrip,$(BR2_SVN))
 CVS := $(call qstrip,$(BR2_CVS))
 BZR := $(call qstrip,$(BR2_BZR))
-GIT := $(call qstrip,$(BR2_GIT))
+export GIT := $(call qstrip,$(BR2_GIT))
 HG := $(call qstrip,$(BR2_HG)) $(QUIET)
 SCP := $(call qstrip,$(BR2_SCP)) $(QUIET)
 SSH := $(call qstrip,$(BR2_SSH)) $(QUIET)
@@ -84,18 +84,8 @@ github = https://github.com/$(1)/$(2)/tarball/$(3)
 # problems
 define DOWNLOAD_GIT
 	test -e $(DL_DIR)/$($(PKG)_SOURCE) || \
-	(pushd $(DL_DIR) > /dev/null && \
-	 ((test "`git ls-remote $($(PKG)_SITE) $($(PKG)_DL_VERSION)`" && \
-	   echo "Doing shallow clone" && \
-	   $(GIT) clone --depth 1 -b $($(PKG)_DL_VERSION) --bare $($(PKG)_SITE) $($(PKG)_BASE_NAME)) || \
-	  (echo "Doing full clone" && \
-	   $(GIT) clone --bare $($(PKG)_SITE) $($(PKG)_BASE_NAME))) && \
-	pushd $($(PKG)_BASE_NAME) > /dev/null && \
-	$(GIT) archive --format=tar --prefix=$($(PKG)_BASE_NAME)/ $($(PKG)_DL_VERSION) | \
-		gzip -c > $(DL_DIR)/$($(PKG)_SOURCE) && \
-	popd > /dev/null && \
-	rm -rf $($(PKG)_DL_DIR) && \
-	popd > /dev/null)
+	$(EXTRA_ENV) support/download/git $($(PKG)_SITE) $($(PKG)_DL_VERSION) \
+					  $($(PKG)_BASE_NAME)/ $(DL_DIR)/$($(PKG)_SOURCE)
 endef
 
 # TODO: improve to check that the given PKG_DL_VERSION exists on the remote
diff --git a/support/download/git b/support/download/git
new file mode 100755
index 0000000..99ea1b2
--- /dev/null
+++ b/support/download/git
@@ -0,0 +1,36 @@
+#!/bin/sh
+set -e
+
+# Download helper for git
+# Call it with:
+#   $1: git repo
+#   $2: git cset
+#   $3: prefix/
+#   $4: output file
+# And this environment:
+#   GIT      :  the git command to call
+#   BUILD_DIR:  path to Buildroot's build dir
+
+repo="${1}"
+cset="${2}"
+prefix="${3}"
+output="${4}"
+
+pushd "${BUILD_DIR}" >/dev/null
+rm -rf .git-tmp-repo    # In case a previous attempt was interrupted
+
+if [ -n "$(${GIT} ls-remote "${repo}" "${cset}" 2>&1)" ]; then
+    printf "Doing shallow clone\n"
+    ${GIT} clone --depth 1 -b "${cset}" --bare "${repo}" .git-tmp-repo
+else
+    printf "Doing full clone\n"
+    ${GIT} clone --bare "${repo}" .git-tmp-repo
+fi
+
+pushd .git-tmp-repo >/dev/null
+${GIT} archive --prefix="${prefix}" --format=tar "${cset}"  \
+|gzip -c >"${output}"
+popd >/dev/null
+
+rm -rf .git-tmp-repo
+popd >/dev/null
-- 
1.8.1.2

[Buildroot] [PATCH 0/6] [RFC] some download-related changes

[Yann E. MORIN]

From: "Yann E. MORIN" <yann.morin.1998@free.fr>

Hello All!

This series contains two download-related changes:

The first part, patch 1 to 3, are a fix the git download helper, which
is currently broken in case a PKG_VERSION is a sha1 that does not exist
in the repository.

Since the make macro was a bit complex to read and maintain, Peter
suggested it be moved to a shell script. If this change is OK, I'll
move the other download helpers to their own shell script.

The second part, patches 4 to 6, add the infrastructure to check the
hashes of the files Buildroot may download, so as to check their
integrity.

This is needed for some packages for which a trusted channel (eg. https)
is not available, and only a insecure channel (eg. http) is available.

Regards,
Yann E. MORIN.


----------------------------------------------------------------
Yann E. MORIN (6):
      Makefile: rename USER_HOOKS_EXTRA_ENV to EXTRA_ENV
      pkg-infra: move git download helper to a script
      pkg-infra: git helper creates an empty archive if PKG_VERSION is a missing hash
      package infra: DOWNLOAD is never called with two arguments
      pkg-infra: add possiblity to check downloaded files against known hashes
      package/ca-certificates: add tarball's hash

 Makefile                                     |  4 +--
 docs/manual/adding-packages-directory.txt    | 35 +++++++++++++++++++++++
 package/Makefile.in                          |  2 +-
 package/ca-certificates/ca-certificates.hash |  1 +
 package/pkg-download.mk                      | 36 ++++++++++++------------
 package/pkg-generic.mk                       |  2 +-
 support/download/check-hash                  | 38 +++++++++++++++++++++++++
 support/download/git                         | 42 ++++++++++++++++++++++++++++
 8 files changed, 138 insertions(+), 22 deletions(-)
 create mode 100644 package/ca-certificates/ca-certificates.hash
 create mode 100755 support/download/check-hash
 create mode 100755 support/download/git

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 3/6] pkg-infra: git helper creates an empty archive if PKG_VERSION is a missing hash

[Yann E. MORIN]

From: "Yann E. MORIN" <yann.morin.1998@free.fr>

If the PKG_VERSION is a sha1 that does not exist in the repository, the git
helper just creates an empty archive instead of failing and erroring out.

This is because of the way we create the archive:
    git archive ${cset} |gzip -c >"${archive}"

In this construct, git does exit with a non-zero exit-code, but gzip does not,
as it was successful in creating the archive (from an empty output, but gzip
does not mind that).

Since a pipe exits with the exit-code of the right-most simple command (see
POSIX.1-2008), the pipe above exits with gzip's exit-code. And we miss the
error altogether.

This is most harmful, in that the error may not occur until much later, when
we try to configure the package, with the error message buried deep down
ipotentially thousands of lines above... :-(

Fix that by checking that the hash (or whatever treeish we've been passed, for
that matters) does indeed exist in the repository, before we even attempt to
create the archive.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
 support/download/git | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/support/download/git b/support/download/git
index 99ea1b2..508f8a1 100755
--- a/support/download/git
+++ b/support/download/git
@@ -28,9 +28,15 @@ else
 fi
 
 pushd .git-tmp-repo >/dev/null
-${GIT} archive --prefix="${prefix}" --format=tar "${cset}"  \
-|gzip -c >"${output}"
+res=1
+if ${GIT} rev-list -n 1 "${cset}" >/dev/null 2>&1; then
+    ${GIT} archive --prefix="${prefix}" --format=tar "${cset}"  \
+    |gzip -c >"${output}"
+    res=0
+fi
 popd >/dev/null
 
 rm -rf .git-tmp-repo
 popd >/dev/null
+
+exit ${res}
-- 
1.8.1.2

[Buildroot] [PATCH 4/6] package infra: DOWNLOAD is never called with two arguments

[Yann E. MORIN]

From: "Yann E. MORIN" <yann.morin.1998@free.fr>

Currently, the DOWNLOAD helper accepts one or two arguments:
  - the source URI
  - the source filename, if missing, derived from the URI

But we have no caller of DOWNLOAD that passes a second argument.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
 package/pkg-download.mk | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/package/pkg-download.mk b/package/pkg-download.mk
index 8e6f7a3..f3354d1 100644
--- a/package/pkg-download.mk
+++ b/package/pkg-download.mk
@@ -227,14 +227,13 @@ endef
 # 3) BR2_BACKUP_SITE if enabled, unless BR2_PRIMARY_SITE_ONLY is set
 #
 # Argument 1 is the source location
-# Argument 2 is the source filename
 #
 # E.G. use like this:
-# $(call DOWNLOAD,$(FOO_SITE),$(FOO_SOURCE))
+# $(call DOWNLOAD,$(FOO_SITE))
 ################################################################################
 
 define DOWNLOAD
-	$(call DOWNLOAD_INNER,$(1),$(if $(2),$(2),$(notdir $(1))))
+	$(call DOWNLOAD_INNER,$(1),$(notdir $(1)))
 endef
 
 define DOWNLOAD_INNER
-- 
1.8.1.2

[Buildroot] [PATCH 5/6] pkg-infra: add possiblity to check downloaded files against known hashes

[Yann E. MORIN]

From: "Yann E. MORIN" <yann.morin.1998@free.fr>

Some of the packages that Buildroot might build are sensitive pacakges,
related to security: openssl, dropbear, ca-certificates...

Some of those packages are download over plain http, because there is
no way to get them over a secure channel, such as https.

In these dark times of pervasive surveillance, the potential for harm
that a tampered package could generate, we may want to check the integrity
of those sensitive packages.

So, each package may now provide a list of hashes for all files that
needs to be downloaded, and Buildroot will just fail if any download file
does not match its known hash.

The choosen hash function is SHA1 since it is widely available, though
theoretical attacks have been devised (but no known practical attack is
known).

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
Note: this is not a bullet-proof solution, since Buildroot may itself be
compromised. But if we eventually sign our releases, then we secure the
list of hashes at the same time. Only random snapshots from the repository
may be at risk of tampering, although this is highly doubtfull, given how
git stores its data.

Also, before we commit a list of hashes to the tree, we may want to
setup a chain-of-trust to validate that thos hashes are correct.
We may want to discuss this during our next developpers' day in
Brussels in February.

Note-2: The laternative to sha1 would be sha2 (256- or 512-bit), but
oldish "enterprise-class" distributions  may be missing them entirely.
sha256sum and sha512sum were added to coreutils in 2005-10-23, and RHEL5
seems to have them. But better be safe than sorry. If sha2 should be
considered instead of sha1, then it is very easy to switch now. Switching
later would require that we revalidate all packages that have hashes,
which could prove to be quite time-demanding if we have lots of
packages using hashes.
---
 docs/manual/adding-packages-directory.txt | 35 ++++++++++++++++++++++++++++
 package/pkg-download.mk                   | 15 ++++++++++--
 support/download/check-hash               | 38 +++++++++++++++++++++++++++++++
 3 files changed, 86 insertions(+), 2 deletions(-)
 create mode 100755 support/download/check-hash

diff --git a/docs/manual/adding-packages-directory.txt b/docs/manual/adding-packages-directory.txt
index 754a145..c5a92bd 100644
--- a/docs/manual/adding-packages-directory.txt
+++ b/docs/manual/adding-packages-directory.txt
@@ -325,3 +325,38 @@ different way, using different infrastructures:
 
 Further formatting details: see xref:writing-rules-mk[the writing
 rules].
+
+The +.hash+ file
+~~~~~~~~~~~~~~~~
+[[adding-packages-hash]]
+
+Optionally, you can add a third file, named +libfoo.hash+, that contains
+the https://en.wikipedia.org/wiki/Sha-1[SHA1] hashes of the downloaded
+files for the +libfoo+ package.
+
+The hashes stored in that file are used to validate the integrity of the
+downloaded files.
+
+The format for this file matches the output of +sha1sum+, that is, each
+line contains:
+
+* the SHA1 hash
+* two spaces
+* the name fo the file
+
+Specific to +Buildroot+, the filenames should not contain any directory
+component, such as:
+
+----
+486fb55c3efa71148fe07895fd713ea3a5ae343a  libfoo-1.2.3.tar.bz2
+a045ff9ecaf8d5342daa4cd23a4084da65af49e1  libfoo-fix-blabla.patch
+f729372d25deddd6191864a48484d4d268fd23ec  libfoo-data.bin
+----
+
+If the +.hash+ file is present, and there is an entry for a downloaded
+file, the hash of the downloaded file must match the hash stored in the
++.hash+ file. If it does not match, this is considered an error, and
+Buildroot aborts.
+
+If the +.hash+ file is present, but there is no entry for a downloaded
+file, or if the +.hash+ file is missing, then no check is done.
diff --git a/package/pkg-download.mk b/package/pkg-download.mk
index f3354d1..5627850 100644
--- a/package/pkg-download.mk
+++ b/package/pkg-download.mk
@@ -58,6 +58,14 @@ domainseparator=$(if $(1),$(1),/)
 # github(user,package,version): returns site of github repository
 github = https://github.com/$(1)/$(2)/tarball/$(3)
 
+# Helper for checking a tarball's checksum
+# $(1): the basename of the tarball to check
+# $(2): the full path to the file to check
+define VERIFY_SHA256
+	support/download/check-hash $(1) $(2) \
+		$($(PKG)_DIR_PREFIX)/$($(PKG)_NAME)/$($(PKG)_NAME).hash
+endef
+
 ################################################################################
 # The DOWNLOAD_* helpers are in charge of getting a working copy
 # of the source repository for their corresponding SCM,
@@ -153,7 +161,8 @@ endef
 # to prepend the path with a slash: scp://[user@]host:/absolutepath
 define DOWNLOAD_SCP
 	test -e $(DL_DIR)/$(2) || \
-	$(SCP) '$(call stripurischeme,$(call qstrip,$(1)))' $(DL_DIR)/$(2)
+	$(SCP) '$(call stripurischeme,$(call qstrip,$(1)))' $(DL_DIR)/$(2) && \
+	$(call VERIFY_SHA256,$(DL_DIR)/$(2))
 endef
 
 define SOURCE_CHECK_SCP
@@ -195,6 +204,7 @@ endef
 define DOWNLOAD_WGET
 	test -e $(DL_DIR)/$(2) || \
 	($(WGET) -O $(DL_DIR)/$(2).tmp '$(call qstrip,$(1))' && \
+	 $(call VERIFY_SHA256,$(2),$(DL_DIR)/$(2).tmp) && \
 	 mv $(DL_DIR)/$(2).tmp $(DL_DIR)/$(2)) || \
 	(rm -f $(DL_DIR)/$(2).tmp ; exit 1)
 endef
@@ -209,7 +219,8 @@ endef
 
 define DOWNLOAD_LOCALFILES
 	test -e $(DL_DIR)/$(2) || \
-		$(LOCALFILES) $(call stripurischeme,$(call qstrip,$(1))) $(DL_DIR)
+		$(LOCALFILES) $(call stripurischeme,$(call qstrip,$(1))) $(DL_DIR) && \
+	$(call VERIFY_SHA256,$(DL_DIR)/$(2))
 endef
 
 define SOURCE_CHECK_LOCALFILES
diff --git a/support/download/check-hash b/support/download/check-hash
new file mode 100755
index 0000000..5cf708f
--- /dev/null
+++ b/support/download/check-hash
@@ -0,0 +1,38 @@
+#!/bin/sh
+set -e
+
+# Helper to check a file matches its known hash
+# Call it with:
+#   $1: the basename of the package's tarball
+#   $2: the full path to the file to check
+#   $3: the path of the file containing all the the expected hashes
+
+tarball="${1}"
+file="${2}"
+h_file="${3}"
+
+# Does the hash-file exist?
+if [ ! -f "${h_file}" ]; then
+    exit 0
+fi
+
+# Do we know a hash for that tarball?
+known=$( grep -E '^[[:xdigit:]]+[[:space:]]{2}'"${tarball}"'$$' "${h_file}" \
+         |cut -d ' ' -f 1
+       )
+if [ -z "${known}" ]; then
+    exit 0
+fi
+
+# Do the hashes match?
+hash=$( sha1sum "${file}" |cut -d ' ' -f 1 )
+if [ "${hash}" = "${known}" ]; then
+    exit 0
+fi
+
+printf "ERROR: %s has wrong SHA256\n" "${tarball}"
+printf "ERROR: expected: %s\n" "${known}"
+printf "ERROR: got     : %s\n" "${hash}"
+printf "ERROR: Incomplete download, or MITM attack\n"
+
+exit 1
-- 
1.8.1.2

[Buildroot] [PATCH 6/6] package/ca-certificates: add tarball's hash

[Yann E. MORIN]

From: "Yann E. MORIN" <yann.morin.1998@free.fr>

ca-certificates contains sensitive security-related information,
and we want to ensure the archive that we download has not been
compromised.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Martin Bark <martin@barkynet.com>
---
 package/ca-certificates/ca-certificates.hash | 1 +
 1 file changed, 1 insertion(+)
 create mode 100644 package/ca-certificates/ca-certificates.hash

diff --git a/package/ca-certificates/ca-certificates.hash b/package/ca-certificates/ca-certificates.hash
new file mode 100644
index 0000000..6bc31e6
--- /dev/null
+++ b/package/ca-certificates/ca-certificates.hash
@@ -0,0 +1 @@
+7f197c1bf7c7fc82e9f8f2fec6d8cc65f6a6187b  ca-certificates_20130906.tar.gz
-- 
1.8.1.2

[Buildroot] [PATCH 5/6] pkg-infra: add possiblity to check downloaded files against known hashes

[Baruch Siach]

Hi Yann,

On Mon, Jan 13, 2014 at 12:44:48AM +0100, Yann E. MORIN wrote:

[...]

> diff --git a/support/download/check-hash b/support/download/check-hash
> new file mode 100755
> index 0000000..5cf708f
> --- /dev/null
> +++ b/support/download/check-hash
> @@ -0,0 +1,38 @@
> +#!/bin/sh
> +set -e
> +
> +# Helper to check a file matches its known hash
> +# Call it with:
> +#   $1: the basename of the package's tarball
> +#   $2: the full path to the file to check
> +#   $3: the path of the file containing all the the expected hashes
> +
> +tarball="${1}"
> +file="${2}"
> +h_file="${3}"
> +
> +# Does the hash-file exist?
> +if [ ! -f "${h_file}" ]; then
> +    exit 0
> +fi
> +
> +# Do we know a hash for that tarball?
> +known=$( grep -E '^[[:xdigit:]]+[[:space:]]{2}'"${tarball}"'$$' "${h_file}" \
> +         |cut -d ' ' -f 1
> +       )
> +if [ -z "${known}" ]; then
> +    exit 0
> +fi
> +
> +# Do the hashes match?
> +hash=$( sha1sum "${file}" |cut -d ' ' -f 1 )
> +if [ "${hash}" = "${known}" ]; then
> +    exit 0
> +fi
> +
> +printf "ERROR: %s has wrong SHA256\n" "${tarball}"

That's SHA1.

> +printf "ERROR: expected: %s\n" "${known}"
> +printf "ERROR: got     : %s\n" "${hash}"
> +printf "ERROR: Incomplete download, or MITM attack\n"
> +
> +exit 1

baruch

-- 
     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -

[Buildroot] [PATCH 2/6] pkg-infra: move git download helper to a script

[Luca Ceresoli]

Hi Yann,

Yann E. MORIN wrote:
> From: "Yann E. MORIN" <yann.morin.1998@free.fr>
>
> The git download helper is getting a bit more complex, and does not
> raise an error when a PKG_VERSION is a sha1 that does not exist in
> the repository. Instead, it generates an empty archive.
>
> Fixing it in the Makefile proves to be challenging, to say the least.
>
> Move it into a shell script in support/download/git, which will make
> it much easier to read, maintain, fix an enhance in the future.
>
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Peter Korsgaard <jacmet@uclibc.org>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

This is a very welcome change, thanks for caring!

Just a minor note below, but looks ok anyway:
Acked-by: Luca Ceresoli <luca@lucaceresoli.net>


> ---
>   package/pkg-download.mk | 16 +++-------------
>   support/download/git    | 36 ++++++++++++++++++++++++++++++++++++
>   2 files changed, 39 insertions(+), 13 deletions(-)
>   create mode 100755 support/download/git
>
> diff --git a/package/pkg-download.mk b/package/pkg-download.mk
> index c00689b..8e6f7a3 100644
> --- a/package/pkg-download.mk
> +++ b/package/pkg-download.mk
> @@ -12,7 +12,7 @@ WGET := $(call qstrip,$(BR2_WGET)) $(QUIET)
>   SVN := $(call qstrip,$(BR2_SVN))
>   CVS := $(call qstrip,$(BR2_CVS))
>   BZR := $(call qstrip,$(BR2_BZR))
> -GIT := $(call qstrip,$(BR2_GIT))
> +export GIT := $(call qstrip,$(BR2_GIT))
>   HG := $(call qstrip,$(BR2_HG)) $(QUIET)
>   SCP := $(call qstrip,$(BR2_SCP)) $(QUIET)
>   SSH := $(call qstrip,$(BR2_SSH)) $(QUIET)
> @@ -84,18 +84,8 @@ github = https://github.com/$(1)/$(2)/tarball/$(3)
>   # problems
>   define DOWNLOAD_GIT
>   	test -e $(DL_DIR)/$($(PKG)_SOURCE) || \
> -	(pushd $(DL_DIR) > /dev/null && \
> -	 ((test "`git ls-remote $($(PKG)_SITE) $($(PKG)_DL_VERSION)`" && \
> -	   echo "Doing shallow clone" && \
> -	   $(GIT) clone --depth 1 -b $($(PKG)_DL_VERSION) --bare $($(PKG)_SITE) $($(PKG)_BASE_NAME)) || \
> -	  (echo "Doing full clone" && \
> -	   $(GIT) clone --bare $($(PKG)_SITE) $($(PKG)_BASE_NAME))) && \
> -	pushd $($(PKG)_BASE_NAME) > /dev/null && \
> -	$(GIT) archive --format=tar --prefix=$($(PKG)_BASE_NAME)/ $($(PKG)_DL_VERSION) | \
> -		gzip -c > $(DL_DIR)/$($(PKG)_SOURCE) && \
> -	popd > /dev/null && \
> -	rm -rf $($(PKG)_DL_DIR) && \
> -	popd > /dev/null)
> +	$(EXTRA_ENV) support/download/git $($(PKG)_SITE) $($(PKG)_DL_VERSION) \
> +					  $($(PKG)_BASE_NAME)/ $(DL_DIR)/$($(PKG)_SOURCE)
>   endef
>
>   # TODO: improve to check that the given PKG_DL_VERSION exists on the remote
> diff --git a/support/download/git b/support/download/git
> new file mode 100755
> index 0000000..99ea1b2
> --- /dev/null
> +++ b/support/download/git
> @@ -0,0 +1,36 @@
> +#!/bin/sh
> +set -e
> +
> +# Download helper for git
> +# Call it with:
> +#   $1: git repo
> +#   $2: git cset
> +#   $3: prefix/
> +#   $4: output file
> +# And this environment:
> +#   GIT      :  the git command to call
> +#   BUILD_DIR:  path to Buildroot's build dir
> +
> +repo="${1}"
> +cset="${2}"
> +prefix="${3}"
> +output="${4}"
> +
> +pushd "${BUILD_DIR}" >/dev/null
> +rm -rf .git-tmp-repo    # In case a previous attempt was interrupted

This rm did not exist in the original version. I would move this into a
separate commit.

> +
> +if [ -n "$(${GIT} ls-remote "${repo}" "${cset}" 2>&1)" ]; then
> +    printf "Doing shallow clone\n"
> +    ${GIT} clone --depth 1 -b "${cset}" --bare "${repo}" .git-tmp-repo
> +else
> +    printf "Doing full clone\n"
> +    ${GIT} clone --bare "${repo}" .git-tmp-repo
> +fi
> +
> +pushd .git-tmp-repo >/dev/null
> +${GIT} archive --prefix="${prefix}" --format=tar "${cset}"  \
> +|gzip -c >"${output}"
> +popd >/dev/null
> +
> +rm -rf .git-tmp-repo
> +popd >/dev/null
>


-- 
Luca

[Buildroot] [PATCH 3/6] pkg-infra: git helper creates an empty archive if PKG_VERSION is a missing hash

[Luca Ceresoli]

Hi Yann,

Yann E. MORIN wrote:
> From: "Yann E. MORIN" <yann.morin.1998@free.fr>
>
> If the PKG_VERSION is a sha1 that does not exist in the repository, the git
> helper just creates an empty archive instead of failing and erroring out.
>
> This is because of the way we create the archive:
>      git archive ${cset} |gzip -c >"${archive}"
>
> In this construct, git does exit with a non-zero exit-code, but gzip does not,
> as it was successful in creating the archive (from an empty output, but gzip
> does not mind that).
>
> Since a pipe exits with the exit-code of the right-most simple command (see
> POSIX.1-2008), the pipe above exits with gzip's exit-code. And we miss the
> error altogether.
>
> This is most harmful, in that the error may not occur until much later, when
> we try to configure the package, with the error message buried deep down
> ipotentially thousands of lines above... :-(

s/ipotentially/potentially/

>
> Fix that by checking that the hash (or whatever treeish we've been passed, for
> that matters) does indeed exist in the repository, before we even attempt to
> create the archive.
>
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

Acked-by: Luca Ceresoli <luca@lucaceresoli.net>

-- 
Luca

[Buildroot] [PATCH 3/6] pkg-infra: git helper creates an empty archive if PKG_VERSION is a missing hash

[Yann E. MORIN]

Luca, All,

On 2014-01-13 15:22 +0100, Luca Ceresoli spake thusly:
> Yann E. MORIN wrote:
> >From: "Yann E. MORIN" <yann.morin.1998@free.fr>
> >
> >If the PKG_VERSION is a sha1 that does not exist in the repository, the git
> >helper just creates an empty archive instead of failing and erroring out.
> >
> >This is because of the way we create the archive:
> >     git archive ${cset} |gzip -c >"${archive}"
> >
> >In this construct, git does exit with a non-zero exit-code, but gzip does not,
> >as it was successful in creating the archive (from an empty output, but gzip
> >does not mind that).
> >
> >Since a pipe exits with the exit-code of the right-most simple command (see
> >POSIX.1-2008), the pipe above exits with gzip's exit-code. And we miss the
> >error altogether.
> >
> >This is most harmful, in that the error may not occur until much later, when
> >we try to configure the package, with the error message buried deep down
> >ipotentially thousands of lines above... :-(
> 
> s/ipotentially/potentially/

Fixed.

This is where you see that I use vi as my editor! ;-)

> >Fix that by checking that the hash (or whatever treeish we've been passed, for
> >that matters) does indeed exist in the repository, before we even attempt to
> >create the archive.
> >
> >Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> 
> Acked-by: Luca Ceresoli <luca@lucaceresoli.net>

Thank you!

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 2/6] pkg-infra: move git download helper to a script

[Yann E. MORIN]

Luca, All,

On 2014-01-13 15:18 +0100, Luca Ceresoli spake thusly:
> Yann E. MORIN wrote:
> >From: "Yann E. MORIN" <yann.morin.1998@free.fr>
> >
> >The git download helper is getting a bit more complex, and does not
> >raise an error when a PKG_VERSION is a sha1 that does not exist in
> >the repository. Instead, it generates an empty archive.
> >
> >Fixing it in the Makefile proves to be challenging, to say the least.
> >
> >Move it into a shell script in support/download/git, which will make
> >it much easier to read, maintain, fix an enhance in the future.
> >
> >Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> >Cc: Peter Korsgaard <jacmet@uclibc.org>
> >Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> 
> This is a very welcome change, thanks for caring!
> 
> Just a minor note below, but looks ok anyway:
> Acked-by: Luca Ceresoli <luca@lucaceresoli.net>
[--SNIP--]
> >diff --git a/support/download/git b/support/download/git
> >new file mode 100755
> >index 0000000..99ea1b2
> >--- /dev/null
> >+++ b/support/download/git
> >@@ -0,0 +1,36 @@
> >+#!/bin/sh
> >+set -e
> >+
> >+# Download helper for git
> >+# Call it with:
> >+#   $1: git repo
> >+#   $2: git cset
> >+#   $3: prefix/
> >+#   $4: output file
> >+# And this environment:
> >+#   GIT      :  the git command to call
> >+#   BUILD_DIR:  path to Buildroot's build dir
> >+
> >+repo="${1}"
> >+cset="${2}"
> >+prefix="${3}"
> >+output="${4}"
> >+
> >+pushd "${BUILD_DIR}" >/dev/null
> >+rm -rf .git-tmp-repo    # In case a previous attempt was interrupted
> 
> This rm did not exist in the original version. I would move this into a
> separate commit.

Indeed, I've moved it to the next commit.

Thank you!

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 5/6] pkg-infra: add possiblity to check downloaded files against known hashes

[Yann E. MORIN]

Baruch, All,

On 2014-01-13 06:53 +0200, Baruch Siach spake thusly:
> On Mon, Jan 13, 2014 at 12:44:48AM +0100, Yann E. MORIN wrote:
[--SNIP--]
> > diff --git a/support/download/check-hash b/support/download/check-hash
> > new file mode 100755
> > index 0000000..5cf708f
> > --- /dev/null
> > +++ b/support/download/check-hash
> > @@ -0,0 +1,38 @@
> > +#!/bin/sh
> > +set -e
> > +
> > +# Helper to check a file matches its known hash
> > +# Call it with:
> > +#   $1: the basename of the package's tarball
> > +#   $2: the full path to the file to check
> > +#   $3: the path of the file containing all the the expected hashes
> > +
> > +tarball="${1}"
> > +file="${2}"
> > +h_file="${3}"
> > +
> > +# Does the hash-file exist?
> > +if [ ! -f "${h_file}" ]; then
> > +    exit 0
> > +fi
> > +
> > +# Do we know a hash for that tarball?
> > +known=$( grep -E '^[[:xdigit:]]+[[:space:]]{2}'"${tarball}"'$$' "${h_file}" \
> > +         |cut -d ' ' -f 1
> > +       )
> > +if [ -z "${known}" ]; then
> > +    exit 0
> > +fi
> > +
> > +# Do the hashes match?
> > +hash=$( sha1sum "${file}" |cut -d ' ' -f 1 )
> > +if [ "${hash}" = "${known}" ]; then
> > +    exit 0
> > +fi
> > +
> > +printf "ERROR: %s has wrong SHA256\n" "${tarball}"
> 
> That's SHA1.

Doh, good catch!

Thank you!

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 2/6] pkg-infra: move git download helper to a script

[Arnout Vandecappelle]

On 13/01/14 00:44, Yann E. MORIN wrote:
> From: "Yann E. MORIN" <yann.morin.1998@free.fr>
>
> The git download helper is getting a bit more complex, and does not
> raise an error when a PKG_VERSION is a sha1 that does not exist in
> the repository. Instead, it generates an empty archive.
>
> Fixing it in the Makefile proves to be challenging, to say the least.
>
> Move it into a shell script in support/download/git, which will make
> it much easier to read, maintain, fix an enhance in the future.

  This patch contains a few more changes than just moving it to a script. 
These should ideally be done in separate patches.

[snip]
> diff --git a/support/download/git b/support/download/git
> new file mode 100755
> index 0000000..99ea1b2
> --- /dev/null
> +++ b/support/download/git
> @@ -0,0 +1,36 @@
> +#!/bin/sh
> +set -e
> +
> +# Download helper for git
> +# Call it with:
> +#   $1: git repo
> +#   $2: git cset
> +#   $3: prefix/
> +#   $4: output file
> +# And this environment:
> +#   GIT      :  the git command to call
> +#   BUILD_DIR:  path to Buildroot's build dir
> +
> +repo="${1}"
> +cset="${2}"
> +prefix="${3}"
> +output="${4}"
> +
> +pushd "${BUILD_DIR}" >/dev/null

  This used to be DL_DIR.

  Note that I agree that BUILD_DIR is more appropriate.

  Although the original indeed does a pushd, this is quite redundant when 
it's done in a script. So I'd directly make it a simple cd (without 
redirection).

> +rm -rf .git-tmp-repo    # In case a previous attempt was interrupted
> +
> +if [ -n "$(${GIT} ls-remote "${repo}" "${cset}" 2>&1)" ]; then
> +    printf "Doing shallow clone\n"
> +    ${GIT} clone --depth 1 -b "${cset}" --bare "${repo}" .git-tmp-repo

  It used to be extracted into $($(PKG)_BASE_NAME). That has the 
advantage that it is unique, which makes it possible to do multiple 
downloads in parallel (which I sometimes do BTW, by manually starting 
downloads in separate shells).

  But even better would be $($(PKG)_BASE_NAME).git-tmp-repo :-)

> +else
> +    printf "Doing full clone\n"
> +    ${GIT} clone --bare "${repo}" .git-tmp-repo
> +fi
> +
> +pushd .git-tmp-repo >/dev/null
> +${GIT} archive --prefix="${prefix}" --format=tar "${cset}"  \
> +|gzip -c >"${output}"

  We have about 20 instances of the | at the end of the line (as was the 
original), and only 4 at the beginning of the line (and two of these were 
done by you...).

  In the original, the / at the end of the prefix is added explicitly 
here in the call to git archive. I think that makes more sense than 
adding the / in the call itself.

  Regards,
  Arnout

> +popd >/dev/null
> +
> +rm -rf .git-tmp-repo
> +popd >/dev/null
>


-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

[Buildroot] [PATCH 3/6] pkg-infra: git helper creates an empty archive if PKG_VERSION is a missing hash

[Arnout Vandecappelle]

On 13/01/14 00:44, Yann E. MORIN wrote:
> From: "Yann E. MORIN" <yann.morin.1998@free.fr>
>
> If the PKG_VERSION is a sha1 that does not exist in the repository, the git
> helper just creates an empty archive instead of failing and erroring out.
>
> This is because of the way we create the archive:
>      git archive ${cset} |gzip -c >"${archive}"
>
> In this construct, git does exit with a non-zero exit-code, but gzip does not,
> as it was successful in creating the archive (from an empty output, but gzip
> does not mind that).
>
> Since a pipe exits with the exit-code of the right-most simple command (see
> POSIX.1-2008), the pipe above exits with gzip's exit-code. And we miss the
> error altogether.
>
> This is most harmful, in that the error may not occur until much later, when
> we try to configure the package, with the error message buried deep down
> ipotentially thousands of lines above... :-(
>
> Fix that by checking that the hash (or whatever treeish we've been passed, for
> that matters) does indeed exist in the repository, before we even attempt to
> create the archive.
>
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> ---
>   support/download/git | 10 ++++++++--
>   1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/support/download/git b/support/download/git
> index 99ea1b2..508f8a1 100755
> --- a/support/download/git
> +++ b/support/download/git
> @@ -28,9 +28,15 @@ else
>   fi
>
>   pushd .git-tmp-repo >/dev/null
> -${GIT} archive --prefix="${prefix}" --format=tar "${cset}"  \
> -|gzip -c >"${output}"
> +res=1
> +if ${GIT} rev-list -n 1 "${cset}" >/dev/null 2>&1; then

  When we've done a shallow clone, this check is redundant because we 
have cloned exactly this ref.

  So instead, I'd do the check in the full clone condition, with an 
explicit exit.

  Regards,
  Arnout

> +    ${GIT} archive --prefix="${prefix}" --format=tar "${cset}"  \
> +    |gzip -c >"${output}"
> +    res=0
> +fi
>   popd >/dev/null
>
>   rm -rf .git-tmp-repo
>   popd >/dev/null
> +
> +exit ${res}
>


-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

[Buildroot] [PATCH 1/6] Makefile: rename USER_HOOKS_EXTRA_ENV to EXTRA_ENV

[Arnout Vandecappelle]

On 13/01/14 00:44, Yann E. MORIN wrote:
> From: "Yann E. MORIN"<yann.morin.1998@free.fr>
>
> This variable contains extra envirnoment variables that we can not export
> since they are clashing with some build systems (eg. BUILD_DIR with
> u-boot).
>
> So, we may need these variables for other ueses than the user's hooks
> for instrumentation. For example, we'll use them later on to export
> BUILD_DIR to the download helper scripts.
>
> Signed-off-by: "Yann E. MORIN"<yann.morin.1998@free.fr>

Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>


  Regards,
  Arnout
-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

[Buildroot] [PATCH 4/6] package infra: DOWNLOAD is never called with two arguments

[Arnout Vandecappelle]

On 13/01/14 00:44, Yann E. MORIN wrote:
> From: "Yann E. MORIN" <yann.morin.1998@free.fr>
>
> Currently, the DOWNLOAD helper accepts one or two arguments:
>    - the source URI
>    - the source filename, if missing, derived from the URI

  The idea was that it would be possible to download to a different local 
file than what is in the URL, which would e.g. work around the 
<sha1>.tar.gz URLs of gitorious. However, that currently anyway doesn't 
work (because the primary and secondary download sites should use the 
buildroot-filename rather than the url-filename). So I'm OK with removing it.

>
> But we have no caller of DOWNLOAD that passes a second argument.
>
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

  Regards,
  Arnout

> ---
>   package/pkg-download.mk | 5 ++---
>   1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/package/pkg-download.mk b/package/pkg-download.mk
> index 8e6f7a3..f3354d1 100644
> --- a/package/pkg-download.mk
> +++ b/package/pkg-download.mk
> @@ -227,14 +227,13 @@ endef
>   # 3) BR2_BACKUP_SITE if enabled, unless BR2_PRIMARY_SITE_ONLY is set
>   #
>   # Argument 1 is the source location
> -# Argument 2 is the source filename
>   #
>   # E.G. use like this:
> -# $(call DOWNLOAD,$(FOO_SITE),$(FOO_SOURCE))
> +# $(call DOWNLOAD,$(FOO_SITE))
>   ################################################################################
>
>   define DOWNLOAD
> -	$(call DOWNLOAD_INNER,$(1),$(if $(2),$(2),$(notdir $(1))))
> +	$(call DOWNLOAD_INNER,$(1),$(notdir $(1)))
>   endef
>
>   define DOWNLOAD_INNER
>


-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

[Buildroot] [PATCH 5/6] pkg-infra: add possiblity to check downloaded files against known hashes

[Arnout Vandecappelle]

On 13/01/14 00:44, Yann E. MORIN wrote:
> From: "Yann E. MORIN" <yann.morin.1998@free.fr>
>
> Some of the packages that Buildroot might build are sensitive pacakges,
> related to security: openssl, dropbear, ca-certificates...
>
> Some of those packages are download over plain http, because there is
> no way to get them over a secure channel, such as https.
>
> In these dark times of pervasive surveillance, the potential for harm
> that a tampered package could generate, we may want to check the integrity
> of those sensitive packages.
>
> So, each package may now provide a list of hashes for all files that
> needs to be downloaded, and Buildroot will just fail if any download file
> does not match its known hash.
>
> The choosen hash function is SHA1 since it is widely available, though
> theoretical attacks have been devised (but no known practical attack is
> known).
>
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> ---
> Note: this is not a bullet-proof solution, since Buildroot may itself be
> compromised. But if we eventually sign our releases, then we secure the
> list of hashes at the same time. Only random snapshots from the repository
> may be at risk of tampering, although this is highly doubtfull, given how
> git stores its data.
>
> Also, before we commit a list of hashes to the tree, we may want to
> setup a chain-of-trust to validate that thos hashes are correct.
> We may want to discuss this during our next developpers' day in
> Brussels in February.

  I think the risk is small, because the package will be downloaded by 
multiple users and autobuilders, so an incorrect hash in the buildroot 
sources will lead to download failure reports.

>
> Note-2: The laternative to sha1 would be sha2 (256- or 512-bit), but
> oldish "enterprise-class" distributions  may be missing them entirely.
> sha256sum and sha512sum were added to coreutils in 2005-10-23, and RHEL5
> seems to have them. But better be safe than sorry. If sha2 should be
> considered instead of sha1, then it is very easy to switch now. Switching
> later would require that we revalidate all packages that have hashes,
> which could prove to be quite time-demanding if we have lots of
> packages using hashes.

  We can be more future-safe by storing the hash that is used in the 
.hash file itself.

[snip]
> diff --git a/package/pkg-download.mk b/package/pkg-download.mk
> index f3354d1..5627850 100644
> --- a/package/pkg-download.mk
> +++ b/package/pkg-download.mk
> @@ -58,6 +58,14 @@ domainseparator=$(if $(1),$(1),/)
>   # github(user,package,version): returns site of github repository
>   github = https://github.com/$(1)/$(2)/tarball/$(3)
>
> +# Helper for checking a tarball's checksum
> +# $(1): the basename of the tarball to check
> +# $(2): the full path to the file to check
> +define VERIFY_SHA256

  VERIFY_HASH would be better.

  Regards,
  Arnout

> +	support/download/check-hash $(1) $(2) \
> +		$($(PKG)_DIR_PREFIX)/$($(PKG)_NAME)/$($(PKG)_NAME).hash
> +endef
> +
>   ################################################################################
>   # The DOWNLOAD_* helpers are in charge of getting a working copy
>   # of the source repository for their corresponding SCM,

[snip]

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

[Buildroot] [PATCH 0/6] [RFC] some download-related changes

[Arnout Vandecappelle]

On 13/01/14 00:44, Yann E. MORIN wrote:
> The first part, patch 1 to 3, are a fix the git download helper, which
> is currently broken in case a PKG_VERSION is a sha1 that does not exist
> in the repository.
>
> Since the make macro was a bit complex to read and maintain, Peter
> suggested it be moved to a shell script. If this change is OK, I'll
> move the other download helpers to their own shell script.

  I would actually move all of $(DOWNLOAD) into a single helper script. I 
expect that that will give you a lot of refactoring opportunities.

  But clearly that can wait for follow-up patches.

  Regards,
  Arnout

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

[Buildroot] [PATCH 2/6] pkg-infra: move git download helper to a script

[Yann E. MORIN]

Arnout, All,

On 2014-01-14 21:39 +0100, Arnout Vandecappelle spake thusly:
> On 13/01/14 00:44, Yann E. MORIN wrote:
> >From: "Yann E. MORIN" <yann.morin.1998@free.fr>
> >
> >The git download helper is getting a bit more complex, and does not
> >raise an error when a PKG_VERSION is a sha1 that does not exist in
> >the repository. Instead, it generates an empty archive.
> >
> >Fixing it in the Makefile proves to be challenging, to say the least.
> >
> >Move it into a shell script in support/download/git, which will make
> >it much easier to read, maintain, fix an enhance in the future.
> 
>  This patch contains a few more changes than just moving it to a script.
> These should ideally be done in separate patches.

Doh. You're right. I already fixed some, but I missed quite a few.
Sigh...

> [snip]
> >diff --git a/support/download/git b/support/download/git
> >new file mode 100755
> >index 0000000..99ea1b2
> >--- /dev/null
> >+++ b/support/download/git
[--SNIP--]
> >+pushd "${BUILD_DIR}" >/dev/null
> 
>  This used to be DL_DIR.

I do not like to treat DL_DIR as a scratch area, hence the use of
BUIL_DIR isntead.

>  Note that I agree that BUILD_DIR is more appropriate.

I'll do the change in a spearate patch.

>  Although the original indeed does a pushd, this is quite redundant when
> it's done in a script. So I'd directly make it a simple cd (without
> redirection).

I'll simplify this.

> >+rm -rf .git-tmp-repo    # In case a previous attempt was interrupted
> >+
> >+if [ -n "$(${GIT} ls-remote "${repo}" "${cset}" 2>&1)" ]; then
> >+    printf "Doing shallow clone\n"
> >+    ${GIT} clone --depth 1 -b "${cset}" --bare "${repo}" .git-tmp-repo
> 
>  It used to be extracted into $($(PKG)_BASE_NAME). That has the advantage
> that it is unique, which makes it possible to do multiple downloads in
> parallel (which I sometimes do BTW, by manually starting downloads in
> separate shells).

No problem, since we're now using BUILD_DIR, not DL_DIR. ;-)

>  But even better would be $($(PKG)_BASE_NAME).git-tmp-repo :-)

OK.

> >+else
> >+    printf "Doing full clone\n"
> >+    ${GIT} clone --bare "${repo}" .git-tmp-repo
> >+fi
> >+
> >+pushd .git-tmp-repo >/dev/null
> >+${GIT} archive --prefix="${prefix}" --format=tar "${cset}"  \
> >+|gzip -c >"${output}"
> 
>  We have about 20 instances of the | at the end of the line (as was the
> original), and only 4 at the beginning of the line (and two of these were
> done by you...).

Yes, that's my own coding style. I plead guilty! ;-)
Will fix.

>  In the original, the / at the end of the prefix is added explicitly here in
> the call to git archive. I think that makes more sense than adding the / in
> the call itself.

OK, will fix.

Thank you! :-)

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 3/6] pkg-infra: git helper creates an empty archive if PKG_VERSION is a missing hash

[Yann E. MORIN]

Arnout, All,

On 2014-01-14 21:43 +0100, Arnout Vandecappelle spake thusly:
> On 13/01/14 00:44, Yann E. MORIN wrote:
> >From: "Yann E. MORIN" <yann.morin.1998@free.fr>
[--SNIP--]
> >-${GIT} archive --prefix="${prefix}" --format=tar "${cset}"  \
> >-|gzip -c >"${output}"
> >+res=1
> >+if ${GIT} rev-list -n 1 "${cset}" >/dev/null 2>&1; then
> 
>  When we've done a shallow clone, this check is redundant because we have
> cloned exactly this ref.
> 
>  So instead, I'd do the check in the full clone condition, with an explicit
> exit.

I see. But it complexifies the script.

And doing the check a second time ensures we did not download garbage

Heck, there's even a race condition, since we check the remote, then
clone it, and the ref may have vanished (granted, very unlikely, but
yet...). :-p

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 5/6] pkg-infra: add possiblity to check downloaded files against known hashes

[Yann E. MORIN]

Arnout, All,

On 2014-01-14 22:37 +0100, Arnout Vandecappelle spake thusly:
> On 13/01/14 00:44, Yann E. MORIN wrote:
> >From: "Yann E. MORIN" <yann.morin.1998@free.fr>
> >Some of the packages that Buildroot might build are sensitive pacakges,
> >related to security: openssl, dropbear, ca-certificates...
[--SNIP--]
> >So, each package may now provide a list of hashes for all files that
> >needs to be downloaded, and Buildroot will just fail if any download file
> >does not match its known hash.
[--SNIP--]
> >Also, before we commit a list of hashes to the tree, we may want to
> >setup a chain-of-trust to validate that thos hashes are correct.
> >We may want to discuss this during our next developpers' day in
> >Brussels in February.
> 
>  I think the risk is small, because the package will be downloaded by
> multiple users and autobuilders, so an incorrect hash in the buildroot
> sources will lead to download failure reports.

Yes, anyone may run "make allyespackageconfig source" (although that
will leave out a few packages).

> >Note-2: The laternative to sha1 would be sha2 (256- or 512-bit), but
> >oldish "enterprise-class" distributions  may be missing them entirely.
> >sha256sum and sha512sum were added to coreutils in 2005-10-23, and RHEL5
> >seems to have them. But better be safe than sorry. If sha2 should be
> >considered instead of sha1, then it is very easy to switch now. Switching
> >later would require that we revalidate all packages that have hashes,
> >which could prove to be quite time-demanding if we have lots of
> >packages using hashes.
> 
>  We can be more future-safe by storing the hash that is used in the .hash
> file itself.

Hu?

> >diff --git a/package/pkg-download.mk b/package/pkg-download.mk
> >index f3354d1..5627850 100644
> >--- a/package/pkg-download.mk
> >+++ b/package/pkg-download.mk
> >@@ -58,6 +58,14 @@ domainseparator=$(if $(1),$(1),/)
> >  # github(user,package,version): returns site of github repository
> >  github = https://github.com/$(1)/$(2)/tarball/$(3)
> >
> >+# Helper for checking a tarball's checksum
> >+# $(1): the basename of the tarball to check
> >+# $(2): the full path to the file to check
> >+define VERIFY_SHA256
> 
>  VERIFY_HASH would be better.

Doh. Right.

Thank you!

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 0/6] [RFC] some download-related changes

[Yann E. MORIN]

Arnout, All,

On 2014-01-14 22:39 +0100, Arnout Vandecappelle spake thusly:
> On 13/01/14 00:44, Yann E. MORIN wrote:
> >The first part, patch 1 to 3, are a fix the git download helper, which
> >is currently broken in case a PKG_VERSION is a sha1 that does not exist
> >in the repository.
> >
> >Since the make macro was a bit complex to read and maintain, Peter
> >suggested it be moved to a shell script. If this change is OK, I'll
> >move the other download helpers to their own shell script.
> 
>  I would actually move all of $(DOWNLOAD) into a single helper script. I
> expect that that will give you a lot of refactoring opportunities.

I also wondered if I'd do that.

But for a first iteration of the series, I decided to just dip a toe in
the water to check if it was not too hot! ;-)

And then, I think I prefer multiple scripts, one for each VCS. It's not
like doing a 'gzip -c' is too complex, after all, since that's the only
thing they'll gona share.

But let's see what others think of it.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 5/6] pkg-infra: add possiblity to check downloaded files against known hashes

[Gustavo Zacarias]

On 01/12/2014 08:44 PM, Yann E. MORIN wrote:

> The choosen hash function is SHA1 since it is widely available, though
> theoretical attacks have been devised (but no known practical attack is
> known).

Hi.
Normally you want whichever upstream uses for their announcement, you
shouldn't hardcode the method and you definitely shouldn't compute when
you download.
That's because you may bump some time later after compromise and if you
compute it when bumping then you're just giving your word on a maybe
compromised one.
Just my $.02
Regards.

[Buildroot] [PATCH 3/6] pkg-infra: git helper creates an empty archive if PKG_VERSION is a missing hash

[Arnout Vandecappelle]

On 15/01/14 00:21, Yann E. MORIN wrote:
> Arnout, All,
>
> On 2014-01-14 21:43 +0100, Arnout Vandecappelle spake thusly:
>> On 13/01/14 00:44, Yann E. MORIN wrote:
>>> From: "Yann E. MORIN" <yann.morin.1998@free.fr>
> [--SNIP--]
>>> -${GIT} archive --prefix="${prefix}" --format=tar "${cset}"  \
>>> -|gzip -c >"${output}"
>>> +res=1
>>> +if ${GIT} rev-list -n 1 "${cset}" >/dev/null 2>&1; then
>>
>>   When we've done a shallow clone, this check is redundant because we have
>> cloned exactly this ref.
>>
>>   So instead, I'd do the check in the full clone condition, with an explicit
>> exit.
>
> I see. But it complexifies the script.
>
> And doing the check a second time ensures we did not download garbage
>
> Heck, there's even a race condition, since we check the remote, then
> clone it, and the ref may have vanished (granted, very unlikely, but
> yet...). :-p

  True. I yield to your wisdom.

  Regards,
  Arnout


-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

[Buildroot] [PATCH 5/6] pkg-infra: add possiblity to check downloaded files against known hashes

[Arnout Vandecappelle]

On 15/01/14 00:34, Yann E. MORIN wrote:
> Arnout, All,
>
> On 2014-01-14 22:37 +0100, Arnout Vandecappelle spake thusly:
>> On 13/01/14 00:44, Yann E. MORIN wrote:
[snip]
>>> Note-2: The laternative to sha1 would be sha2 (256- or 512-bit), but
>>> oldish "enterprise-class" distributions  may be missing them entirely.
>>> sha256sum and sha512sum were added to coreutils in 2005-10-23, and RHEL5
>>> seems to have them. But better be safe than sorry. If sha2 should be
>>> considered instead of sha1, then it is very easy to switch now. Switching
>>> later would require that we revalidate all packages that have hashes,
>>> which could prove to be quite time-demanding if we have lots of
>>> packages using hashes.
>>
>>   We can be more future-safe by storing the hash that is used in the .hash
>> file itself.
>
> Hu?

  If the hash file contains the following:

486fb55c3efa71148fe07895fd713ea3a5ae343a  sha1  libfoo-1.2.3.tar.bz2

then you can now let the script check that the second field is sha1, and 
later you can support different hash methods. In that case, it is not 
necessary to update all the files when we want to switch to a new hash 
method.

  (Incidentally, it also enables Gustavo's suggestion to use whatever 
upstream provides.)

[snip]


-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F

[Buildroot] [PATCH 5/6] pkg-infra: add possiblity to check downloaded files against known hashes

[Gustavo Zacarias]

On 01/15/2014 05:22 AM, Arnout Vandecappelle wrote:

>  If the hash file contains the following:
> 
> 486fb55c3efa71148fe07895fd713ea3a5ae343a  sha1  libfoo-1.2.3.tar.bz2
> 
> then you can now let the script check that the second field is sha1, and
> later you can support different hash methods. In that case, it is not
> necessary to update all the files when we want to switch to a new hash
> method.
> 
>  (Incidentally, it also enables Gustavo's suggestion to use whatever
> upstream provides.)

Yes.
A little explanation on why upstream hashes should be used (my mail last
night was a bit rushed out, busy busy).
When upstream releases a tarball normally it'll fire off an announcement
mail to some mailing list with (hopefully) the hash for the tarball(s).
Usually this hash(es) will also live in some project website hosting.
If the website is compromised then the hash there can also be
compromised and you are computing your hash on a bad tarball (if done
when bumping version) or an altered hash in the website.
Sent mail (mailing list archives, personal mail) is much harder to
compromise all at once hence the original hash will still be true if all
is dandy.
Of course there's the possibility that the developer machine has been
compromised or some code has been "sneaked under the carpet" with peer
review failing to notice, but then there's no hash to save you there and
it's not the intention of this either.
Regards.

[Buildroot] [PATCH 3/6] pkg-infra: git helper creates an empty archive if PKG_VERSION is a missing hash

[Yann E. MORIN]

Arnout, All,

On 2014-01-15 09:17 +0100, Arnout Vandecappelle spake thusly:
> On 15/01/14 00:21, Yann E. MORIN wrote:
> >Arnout, All,
> >
> >On 2014-01-14 21:43 +0100, Arnout Vandecappelle spake thusly:
> >>On 13/01/14 00:44, Yann E. MORIN wrote:
> >>>From: "Yann E. MORIN" <yann.morin.1998@free.fr>
> >[--SNIP--]
> >>>-${GIT} archive --prefix="${prefix}" --format=tar "${cset}"  \
> >>>-|gzip -c >"${output}"
> >>>+res=1
> >>>+if ${GIT} rev-list -n 1 "${cset}" >/dev/null 2>&1; then
> >>
> >>  When we've done a shallow clone, this check is redundant because we have
> >>cloned exactly this ref.
> >>
> >>  So instead, I'd do the check in the full clone condition, with an explicit
> >>exit.
> >
> >I see. But it complexifies the script.
> >
> >And doing the check a second time ensures we did not download garbage
> >
> >Heck, there's even a race condition, since we check the remote, then
> >clone it, and the ref may have vanished (granted, very unlikely, but
> >yet...). :-p
> 
>  True. I yield to your wisdom.

Sorry, that race-condition was not meant to be taken in the first degree.
Of course, the chances it happens are so thin we can easily ignore it.

However, I tried massaging the script so that we do the check only once,
and I can't find an easy way, except duplicating the check before
calling git archive.

I've added a comment saying so in the script.

Thank you for the review! :-)

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 5/6] pkg-infra: add possiblity to check downloaded files against known hashes

[Yann E. MORIN]

Arnout, All,

On 2014-01-15 09:22 +0100, Arnout Vandecappelle spake thusly:
> On 15/01/14 00:34, Yann E. MORIN wrote:
> >Arnout, All,
> >
> >On 2014-01-14 22:37 +0100, Arnout Vandecappelle spake thusly:
> >>On 13/01/14 00:44, Yann E. MORIN wrote:
> [snip]
> >>>Note-2: The laternative to sha1 would be sha2 (256- or 512-bit), but
> >>>oldish "enterprise-class" distributions  may be missing them entirely.
> >>>sha256sum and sha512sum were added to coreutils in 2005-10-23, and RHEL5
> >>>seems to have them. But better be safe than sorry. If sha2 should be
> >>>considered instead of sha1, then it is very easy to switch now. Switching
> >>>later would require that we revalidate all packages that have hashes,
> >>>which could prove to be quite time-demanding if we have lots of
> >>>packages using hashes.
> >>
> >>  We can be more future-safe by storing the hash that is used in the .hash
> >>file itself.
> >
> >Hu?
> 
>  If the hash file contains the following:
> 
> 486fb55c3efa71148fe07895fd713ea3a5ae343a  sha1  libfoo-1.2.3.tar.bz2

OK, I see what you meant, now.

> then you can now let the script check that the second field is sha1, and
> later you can support different hash methods. In that case, it is not
> necessary to update all the files when we want to switch to a new hash
> method.

However, that means the file is no longer the output of:
    sha1sum files-to-check* >package.hash

or of any other hash utility: sha*sum all generates similarly-formatted
outputs:
    hash <two spaces> filename

Which was the reason I choose that format.

If we'd use your suggestion, we'd need a simple way to generate that
file, or it'd be error prone.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 5/6] pkg-infra: add possiblity to check downloaded files against known hashes

[Yann E. MORIN]

Gustavo, All,

On 2014-01-15 10:22 -0300, Gustavo Zacarias spake thusly:
> On 01/15/2014 05:22 AM, Arnout Vandecappelle wrote:
> 
> >  If the hash file contains the following:
> > 
> > 486fb55c3efa71148fe07895fd713ea3a5ae343a  sha1  libfoo-1.2.3.tar.bz2
> > 
> > then you can now let the script check that the second field is sha1, and
> > later you can support different hash methods. In that case, it is not
> > necessary to update all the files when we want to switch to a new hash
> > method.
> > 
> >  (Incidentally, it also enables Gustavo's suggestion to use whatever
> > upstream provides.)
> 
> Yes.
> A little explanation on why upstream hashes should be used (my mail last
> night was a bit rushed out, busy busy).
> When upstream releases a tarball normally it'll fire off an announcement
> mail to some mailing list with (hopefully) the hash for the tarball(s).
> Usually this hash(es) will also live in some project website hosting.
> If the website is compromised then the hash there can also be
> compromised and you are computing your hash on a bad tarball (if done
> when bumping version) or an altered hash in the website.
> Sent mail (mailing list archives, personal mail) is much harder to
> compromise all at once hence the original hash will still be true if all
> is dandy.
> Of course there's the possibility that the developer machine has been
> compromised or some code has been "sneaked under the carpet" with peer
> review failing to notice, but then there's no hash to save you there and
> it's not the intention of this either.

I understand your concerns.

But, whether we do compute the hashes ourselves, or retrieve them from
an annoucment mail (which by your own saying is not systematic), we
can't blindly accept a patch that contains hashes without verifying
them.

So, whenever Peter would be about to apply a package, he'd have to check
for himself that the hashes are correct.

For that, he'd have to go the package's website, dig up the announcement
mail from the list, and compare the hashes.

So, whether we compute them, or get them from the annoucement mail,
Peter would still have to check them in the end, prior to applying the
patch.

And we still do not have solved the problem of packages for which no
hash has been publicly posted and archived.

So, all that comes to mind now is that we need signatures, not hashes.
But not all packages have signed releases either. So we're back to
square-one.

In the end, I wonder how much we do want to protect the downloads.
I firmly believe that a set of hashes are enough for what we want to do.
If a user is even most concerned about security, then he should (and
would anyway) audit the download for integrity before shipping a
product.

And since we do sign our releases (peter does it), a user can verify our
releases easily, and assess that the hashes are legit.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH 5/6] pkg-infra: add possiblity to check downloaded files against known hashes

[Gustavo Zacarias]

On 01/17/2014 08:02 PM, Yann E. MORIN wrote:

> I understand your concerns.
> 
> But, whether we do compute the hashes ourselves, or retrieve them from
> an annoucment mail (which by your own saying is not systematic), we
> can't blindly accept a patch that contains hashes without verifying
> them.
> 
> So, whenever Peter would be about to apply a package, he'd have to check
> for himself that the hashes are correct.
> 
> For that, he'd have to go the package's website, dig up the announcement
> mail from the list, and compare the hashes.
> 
> So, whether we compute them, or get them from the annoucement mail,
> Peter would still have to check them in the end, prior to applying the
> patch.
> 
> And we still do not have solved the problem of packages for which no
> hash has been publicly posted and archived.
> 
> So, all that comes to mind now is that we need signatures, not hashes.
> But not all packages have signed releases either. So we're back to
> square-one.
> 
> In the end, I wonder how much we do want to protect the downloads.
> I firmly believe that a set of hashes are enough for what we want to do.
> If a user is even most concerned about security, then he should (and
> would anyway) audit the download for integrity before shipping a
> product.
> 
> And since we do sign our releases (peter does it), a user can verify our
> releases easily, and assess that the hashes are legit.

The hashes are first and foremost a consistency feature, they are not in
any way a security feature and anyone who believes that is fooling himself.
My comment was to just give an extra degree of consistency since the
developer's box can be compromised without his knowledge and would emit
a perfectly valid hashed tarball with a backdoor, weakness or whatnot.
This would need guidelines as to the preferred origin of the hashes, and
yes not everyone does hashes and/or announcements so it's only that,
guidelines :)
I don't know if i'd go as far as a chain of trust, the burden would be
too much at the moment i think, if we add file size combined with the
hash the collisions would be reduced and you get a certain degree of
extra protection.
Peter or some other committer shouldn't need to get too involved in
this, basically make the check against the submitted package and it will
match or not.
If at some point it's discovered there was a compromise upstream you can
compare hashes and know if you're affected (depending on the package
being submitted/updated after the compromise or not).
But remember, short of doing source code audit there's never any
guarantee, so i wouldn't want to sign that everything is fine in an
irresponsible way when many of the packages are just built tested.
Regards.

[Buildroot] [PATCH 5/6] pkg-infra: add possiblity to check downloaded files against known hashes

[Luca Ceresoli]

Hi Yann, Arnout,

Yann E. MORIN wrote:
> Arnout, All,
>
> On 2014-01-15 09:22 +0100, Arnout Vandecappelle spake thusly:
>> On 15/01/14 00:34, Yann E. MORIN wrote:
>>> Arnout, All,
>>>
>>> On 2014-01-14 22:37 +0100, Arnout Vandecappelle spake thusly:
>>>> On 13/01/14 00:44, Yann E. MORIN wrote:
>> [snip]
>>>>> Note-2: The laternative to sha1 would be sha2 (256- or 512-bit), but
>>>>> oldish "enterprise-class" distributions  may be missing them entirely.
>>>>> sha256sum and sha512sum were added to coreutils in 2005-10-23, and RHEL5
>>>>> seems to have them. But better be safe than sorry. If sha2 should be
>>>>> considered instead of sha1, then it is very easy to switch now. Switching
>>>>> later would require that we revalidate all packages that have hashes,
>>>>> which could prove to be quite time-demanding if we have lots of
>>>>> packages using hashes.
>>>>
>>>>   We can be more future-safe by storing the hash that is used in the .hash
>>>> file itself.
>>>
>>> Hu?
>>
>>   If the hash file contains the following:
>>
>> 486fb55c3efa71148fe07895fd713ea3a5ae343a  sha1  libfoo-1.2.3.tar.bz2
>
> OK, I see what you meant, now.
>
>> then you can now let the script check that the second field is sha1, and
>> later you can support different hash methods. In that case, it is not
>> necessary to update all the files when we want to switch to a new hash
>> method.
>
> However, that means the file is no longer the output of:
>      sha1sum files-to-check* >package.hash
>
> or of any other hash utility: sha*sum all generates similarly-formatted
> outputs:
>      hash <two spaces> filename
>
> Which was the reason I choose that format.
>
> If we'd use your suggestion, we'd need a simple way to generate that
> file, or it'd be error prone.

Or keep in BR one file per each hash algorithm: a *.hash.sha1
file, a *.hash.sha256 file etc. Whichever file(s) are found are
checked.

This would save both needs, wouldn't it? It should not take many more
lines of code to implement.

-- 
Luca