[Buildroot] [PATCH 0/5 v2] pkg-download: check hashes before the download (branch yem/download-hash)

[Buildroot] [PATCH 0/5 v2] pkg-download: check hashes before the download (branch yem/download-hash)

[Yann E. MORIN]

Hello All!

This series introduces a way to check hashes prior to doing a download.

This is required for when upstream silently update their release tarballs
without renaming them, and the user is left with a stray locally cached
tarball that no longer match the hashes with have for that package.

In so doing, this series:
  - moves the check for a cached file into the wrapper;
  - moves the post-download check for hashes into the wrapper;
  - adds a pre-download check for hashes in the wrapper.

Doing the pre-download checks in the Makefile, like the post-download
checks were done, made the Makefile a bit harder to read. On the other
hand, we have a download wrapper shell script, so it is easier to do
trickey stuff in there (shell syntax) than in the Makefile (make syntax
can become unreadable pretty fast).

This has a side effect of cleaning up the pkg-download.mk Makefile, too,
but that was not the goal.


Changes v1 -> v2:
  - add options parsing to the wrapper  (Thomas)
  - typoes  (Thomas)
  - rename the wrapper to dl-wrapper so it looks better in the traces,
    and it is not confused with another wrapper


Regards,
Yann E. MORIN.


The following changes since commit c5f0e93d2cc6107cce249e1f11158aeccb4a3449:

  rp-pppoe: install default conf file (2014-12-03 23:26:21 +0100)

are available in the git repository at:

  git://git.busybox.net/~ymorin/git/buildroot yem/download-hash

for you to fetch changes up to 661d8a00260bcabd71bf7dad6ae4f1a7d3b919e9:

  pkg-download: check hashes for locally cached files (2014-12-07 11:48:06 +0100)

----------------------------------------------------------------
Yann E. MORIN (5):
      pkg-download: fix arguments to hash checking script
      suppot/download: add option parsing to the download wrapper
      pkg-download: check for already downloaded file in the download wrapper
      pkg-download: verify the hashes from the download wrapper
      pkg-download: check hashes for locally cached files

 package/pkg-download.mk     |  72 ++++++++---------
 support/download/check-hash |  18 +++--
 support/download/dl-wrapper | 191 ++++++++++++++++++++++++++++++++++++++++++++
 support/download/wrapper    |  99 -----------------------
 4 files changed, 235 insertions(+), 145 deletions(-)
 create mode 100755 support/download/dl-wrapper
 delete mode 100755 support/download/wrapper

-- 
.-----------------.--------------------.------------------.--------------------.
|  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 1/5 v2] pkg-download: fix arguments to hash checking script

[Yann E. MORIN]

The argument are correctly used, but incorrectly documented.
Inverse the comments to match the actual usage.

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

diff --git a/support/download/check-hash b/support/download/check-hash
index 067e7a2..13e361a 100755
--- a/support/download/check-hash
+++ b/support/download/check-hash
@@ -3,8 +3,8 @@ set -e
 
 # Helper to check a file matches its known hash
 # Call it with:
-#   $1: the full path to the file to check
-#   $2: the path of the file containing all the the expected hashes
+#   $1: the path of the file containing all the the expected hashes
+#   $2: the full path to the file to check
 
 h_file="${1}"
 file="${2}"
-- 
1.9.1

[Buildroot] [PATCH 2/5 v2] suppot/download: add option parsing to the download wrapper

[Yann E. MORIN]

Instead of relying on argument ordering, use actual options in the
download wrapper.

Download backends (bzr, cp, hg...) are left as-is, because it does not
make sense to complexifies them, since they are almost very trivial
shell scripts, and adding option parsing would be really overkill.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 package/pkg-download.mk     |  42 ++++++-----
 support/download/dl-wrapper | 166 ++++++++++++++++++++++++++++++++++++++++++++
 support/download/wrapper    |  99 --------------------------
 3 files changed, 192 insertions(+), 115 deletions(-)
 create mode 100755 support/download/dl-wrapper
 delete mode 100755 support/download/wrapper

diff --git a/package/pkg-download.mk b/package/pkg-download.mk
index f3409bd..e76bb38 100644
--- a/package/pkg-download.mk
+++ b/package/pkg-download.mk
@@ -18,6 +18,8 @@ export SCP := $(call qstrip,$(BR2_SCP)) $(QUIET)
 SSH := $(call qstrip,$(BR2_SSH)) $(QUIET)
 export LOCALFILES := $(call qstrip,$(BR2_LOCALFILES))
 
+DL_WRAPPER = support/download/dl-wrapper
+
 # Default spider mode is 'DOWNLOAD'. Other possible values are 'SOURCE_CHECK'
 # used by the _source-check target and 'SHOW_EXTERNAL_DEPS', used by the
 # external-deps target.
@@ -95,8 +97,9 @@ endef
 # problems
 define DOWNLOAD_GIT
 	test -e $(DL_DIR)/$($(PKG)_SOURCE) || \
-	$(EXTRA_ENV) support/download/wrapper git \
-		$(DL_DIR)/$($(PKG)_SOURCE) \
+	$(EXTRA_ENV) $(DL_WRAPPER) -b git \
+		-o $(DL_DIR)/$($(PKG)_SOURCE) \
+		-- \
 		$($(PKG)_SITE) \
 		$($(PKG)_DL_VERSION) \
 		$($(PKG)_BASE_NAME)
@@ -115,8 +118,9 @@ endef
 
 define DOWNLOAD_BZR
 	test -e $(DL_DIR)/$($(PKG)_SOURCE) || \
-	$(EXTRA_ENV) support/download/wrapper bzr \
-		$(DL_DIR)/$($(PKG)_SOURCE) \
+	$(EXTRA_ENV) $(DL_WRAPPER) -b bzr \
+		-o $(DL_DIR)/$($(PKG)_SOURCE) \
+		-- \
 		$($(PKG)_SITE) \
 		$($(PKG)_DL_VERSION) \
 		$($(PKG)_BASE_NAME)
@@ -132,8 +136,9 @@ endef
 
 define DOWNLOAD_CVS
 	test -e $(DL_DIR)/$($(PKG)_SOURCE) || \
-	$(EXTRA_ENV) support/download/wrapper cvs \
-		$(DL_DIR)/$($(PKG)_SOURCE) \
+	$(EXTRA_ENV) $(DL_WRAPPER) -b cvs \
+		-o $(DL_DIR)/$($(PKG)_SOURCE) \
+		-- \
 		$(call stripurischeme,$(call qstrip,$($(PKG)_SITE))) \
 		$($(PKG)_DL_VERSION) \
 		$($(PKG)_RAWNAME) \
@@ -151,8 +156,9 @@ endef
 
 define DOWNLOAD_SVN
 	test -e $(DL_DIR)/$($(PKG)_SOURCE) || \
-	$(EXTRA_ENV) support/download/wrapper svn \
-		$(DL_DIR)/$($(PKG)_SOURCE) \
+	$(EXTRA_ENV) $(DL_WRAPPER) -b svn \
+		-o $(DL_DIR)/$($(PKG)_SOURCE) \
+		-- \
 		$($(PKG)_SITE) \
 		$($(PKG)_DL_VERSION) \
 		$($(PKG)_BASE_NAME)
@@ -171,8 +177,9 @@ endef
 # to prepend the path with a slash: scp://[user@]host:/absolutepath
 define DOWNLOAD_SCP
 	test -e $(DL_DIR)/$(2) || \
-	$(EXTRA_ENV) support/download/wrapper scp \
-		$(DL_DIR)/$(2) \
+	$(EXTRA_ENV) $(DL_WRAPPER) -b scp \
+		-o $(DL_DIR)/$(2) \
+		-- \
 		'$(call stripurischeme,$(call qstrip,$(1)))' && \
 	$(call VERIFY_HASH,$(PKGDIR)/$($(PKG)_NAME).hash,$(DL_DIR)/$(2))
 endef
@@ -188,8 +195,9 @@ endef
 
 define DOWNLOAD_HG
 	test -e $(DL_DIR)/$($(PKG)_SOURCE) || \
-	$(EXTRA_ENV) support/download/wrapper hg \
-		$(DL_DIR)/$($(PKG)_SOURCE) \
+	$(EXTRA_ENV) $(DL_WRAPPER) -b hg \
+		-o $(DL_DIR)/$($(PKG)_SOURCE) \
+		-- \
 		$($(PKG)_SITE) \
 		$($(PKG)_DL_VERSION) \
 		$($(PKG)_BASE_NAME)
@@ -208,8 +216,9 @@ endef
 
 define DOWNLOAD_WGET
 	test -e $(DL_DIR)/$(2) || \
-	$(EXTRA_ENV) support/download/wrapper wget \
-		$(DL_DIR)/$(2) \
+	$(EXTRA_ENV) $(DL_WRAPPER) -b wget \
+		-o $(DL_DIR)/$(2) \
+		-- \
 		'$(call qstrip,$(1))' && \
 	$(call VERIFY_HASH,$(PKGDIR)/$($(PKG)_NAME).hash,$(DL_DIR)/$(2))
 endef
@@ -224,8 +233,9 @@ endef
 
 define DOWNLOAD_LOCALFILES
 	test -e $(DL_DIR)/$(2) || \
-	$(EXTRA_ENV) support/download/wrapper cp \
-		$(DL_DIR)/$(2) \
+	$(EXTRA_ENV) $(DL_WRAPPER) -b cp \
+		-o $(DL_DIR)/$(2) \
+		-- \
 		$(call stripurischeme,$(call qstrip,$(1))) && \
 	$(call VERIFY_HASH,$(PKGDIR)/$($(PKG)_NAME).hash,$(DL_DIR)/$(2))
 endef
diff --git a/support/download/dl-wrapper b/support/download/dl-wrapper
new file mode 100755
index 0000000..078ae33
--- /dev/null
+++ b/support/download/dl-wrapper
@@ -0,0 +1,166 @@
+#!/usr/bin/env bash
+
+# This script is a wrapper to the other download backends.
+# Its role is to ensure atomicity when saving downloaded files
+# back to BR2_DL_DIR, and not clutter BR2_DL_DIR with partial,
+# failed downloads.
+#
+# Call it with:
+#   -b BACKEND  name of the backend (eg. cvs, git, cp...)
+#   -o FILE     full path to the file in which to save the download
+#   --          everything after '--' are options for the backend
+# Environment:
+#   BUILD_DIR: the path to Buildroot's build dir
+
+# To avoid cluttering BR2_DL_DIR, we download to a trashable
+# location, namely in $(BUILD_DIR).
+# Then, we move the downloaded file to a temporary file in the
+# same directory as the final output file.
+# This allows us to finally atomically rename it to its final
+# name.
+# If anything goes wrong, we just remove all the temporaries
+# created so far.
+
+# We want to catch any unexpected failure, and exit immediately.
+set -e
+
+main() {
+    local OPT OPTARG
+    local backend output
+
+    # Parse our options; anythong after '--' is for the backend
+    # Sop we only care to -b (backend) and -o (output file)
+    while getopts :hb:o: OPT; do
+        case "${OPT}" in
+        h)  help; exit 0;;
+        b)  backend="${OPTARG}";;
+        o)  output="${OPTARG}";;
+        :)  error "option '%s' expects a mandatory argument\n" "${OPTARG}";;
+        \?) error "unknown option '%s'\n" "${OPTARG}";;
+        esac
+    done
+    # Forget our options, and keep only those for the backend
+    shift $((OPTIND-1))
+
+    if [ -z "${backend}" ]; then
+        error "no backend specified, use -b\n"
+    fi
+    if [ -z "${output}" ]; then
+        error "no output specified, use -o\n"
+    fi
+
+    # tmpd is a temporary directory in which backends may store intermediate
+    # by-products of the download.
+    # tmpf is the file in which the backends should put the downloaded content.
+    # tmpd is located in $(BUILD_DIR), so as not to clutter the (precious)
+    # $(BR2_DL_DIR)
+    # We let the backends create tmpf, so they are able to set whatever
+    # permission bits they want (although we're only really interested in
+    # the executable bit.)
+    tmpd="$( mktemp -d "${BUILD_DIR}/.${output##*/}.XXXXXX" )"
+    tmpf="${tmpd}/output"
+
+    # Helpers expect to run in a directory that is *really* trashable, so
+    # they are free to create whatever files and/or sub-dirs they might need.
+    # Doing the 'cd' here rather than in all backends is easier.
+    cd "${tmpd}"
+
+    # If the backend fails, we can just remove the temporary directory to
+    # remove all the cruft it may have left behind. Then we just exit in
+    # error too.
+    if ! "${OLDPWD}/support/download/${backend}" "${tmpf}" "${@}"; then
+        rm -rf "${tmpd}"
+        exit 1
+    fi
+
+    # cd back to free the temp-dir, so we can remove it later
+    cd "${OLDPWD}"
+
+    # tmp_output is in the same directory as the final output, so we can
+    # later move it atomically.
+    tmp_output="$( mktemp "${output}.XXXXXX" )"
+
+    # 'mktemp' creates files with 'go=-rwx', so the files are not accessible
+    # to users other than the one doing the download (and root, of course).
+    # This can be problematic when a shared BR2_DL_DIR is used by different
+    # users (e.g. on a build server), where all users may write to the shared
+    # location, since other users would not be allowed to read the files
+    # another user downloaded.
+    # So, we restore the 'go' access rights to a more sensible value, while
+    # still abiding by the current user's umask. We must do that before the
+    # final 'mv', so just do it now.
+    # Some backends (cp and scp) may create executable files, so we need to
+    # carry the executable bit if needed.
+    [ -x "${tmpf}" ] && new_mode=755 || new_mode=644
+    new_mode=$( printf "%04o" $((0${new_mode} & ~0$(umask))) )
+    chmod ${new_mode} "${tmp_output}"
+
+    # We must *not* unlink tmp_output, otherwise there is a small window
+    # during which another download process may create the same tmp_output
+    # name (very, very unlikely; but not impossible.)
+    # Using 'cp' is not reliable, since 'cp' may unlink the destination file
+    # if it is unable to open it with O_WRONLY|O_TRUNC; see:
+    #   http://pubs.opengroup.org/onlinepubs/9699919799/utilities/cp.html
+    # Since the destination filesystem can be anything, it might not support
+    # O_TRUNC, so 'cp' would unlink it first.
+    # Use 'cat' and append-redirection '>>' to save to the final location,
+    # since that is the only way we can be 100% sure of the behaviour.
+    if ! cat "${tmpf}" >>"${tmp_output}"; then
+        rm -rf "${tmpd}" "${tmp_output}"
+        exit 1
+    fi
+    rm -rf "${tmpd}"
+
+    # tmp_output and output are on the same filesystem, so POSIX guarantees
+    # that 'mv' is atomic, because it then uses rename() that POSIX mandates
+    # to be atomic, see:
+    #   http://pubs.opengroup.org/onlinepubs/9699919799/functions/rename.html
+    if ! mv -f "${tmp_output}" "${output}"; then
+        rm -f "${tmp_output}"
+        exit 1
+    fi
+}
+
+help() {
+    cat <<_EOF_
+NAME
+    ${my_name} - downlaod wrapper for Buildroot
+
+SYNOPSIS
+    ${my_name} [OPTION]... -- [BACKEND OPTION]...
+
+DESCRIPTIOM
+    Wrapper script around different download mechanisms. Ensure that
+    concurrent downloads do not conflict, that partial downloads are
+    properly evicted without leaving temporary files, and that access
+    rights are maintained.
+
+    -h  This help text.
+
+    -b BACKEND
+        Wrap the specified BACKEND. Known backends are:
+            bzr     Bazaar
+            cp      local files via simple copy
+            cvs     Concurrent Versions System
+            git     git
+            hg      Mercurial
+            scp     remote files via Secure copy
+            svn     Subversion
+            wget    remote files via HTTP download
+
+    -o FILE
+        Store the downloaded archive in FILE.
+
+  Exit status:
+    0   if OK
+    !0  in case of error
+_EOF_
+}
+
+trace()  { local msg="${1}"; shift; printf "%s: ${msg}" "${my_name}" "${@}"; }
+warn()   { trace "${@}" >&2; }
+errorN() { local ret="${1}"; shift; warn "${@}"; exit ${ret}; }
+error()  { errorN 1 "${@}"; }
+
+my_name="${0##*/}"
+main "${@}"
diff --git a/support/download/wrapper b/support/download/wrapper
deleted file mode 100755
index 320a37e..0000000
--- a/support/download/wrapper
+++ /dev/null
@@ -1,99 +0,0 @@
-#!/usr/bin/env bash
-
-# This script is a wrapper to the other download helpers.
-# Its role is to ensure atomicity when saving downloaded files
-# back to BR2_DL_DIR, and not clutter BR2_DL_DIR with partial,
-# failed downloads.
-#
-# Call it with:
-#   $1: name of the helper (eg. cvs, git, cp...)
-#   $2: full path to the file in which to save the download
-#   $*: additional arguments to the helper in $1
-# Environment:
-#   BUILD_DIR: the path to Buildroot's build dir
-
-# To avoid cluttering BR2_DL_DIR, we download to a trashable
-# location, namely in $(BUILD_DIR).
-# Then, we move the downloaded file to a temporary file in the
-# same directory as the final output file.
-# This allows us to finally atomically rename it to its final
-# name.
-# If anything goes wrong, we just remove all the temporaries
-# created so far.
-
-# We want to catch any unexpected failure, and exit immediately.
-set -e
-
-helper="${1}"
-output="${2}"
-shift 2
-
-# tmpd is a temporary directory in which helpers may store intermediate
-# by-products of the download.
-# tmpf is the file in which the helpers should put the downloaded content.
-# tmpd is located in $(BUILD_DIR), so as not to clutter the (precious)
-# $(BR2_DL_DIR)
-# We let the helpers create tmpf, so they are able to set whatever
-# permission bits they want (although we're only really interested in
-# the executable bit.)
-tmpd="$( mktemp -d "${BUILD_DIR}/.${output##*/}.XXXXXX" )"
-tmpf="${tmpd}/output"
-
-# Helpers expect to run in a directory that is *really* trashable, so
-# they are free to create whatever files and/or sub-dirs they might need.
-# Doing the 'cd' here rather than in all helpers is easier.
-cd "${tmpd}"
-
-# If the helper fails, we can just remove the temporary directory to
-# remove all the cruft it may have left behind. Then we just exit in
-# error too.
-if ! "${OLDPWD}/support/download/${helper}" "${tmpf}" "${@}"; then
-    rm -rf "${tmpd}"
-    exit 1
-fi
-
-# cd back to free the temp-dir, so we can remove it later
-cd "${OLDPWD}"
-
-# tmp_output is in the same directory as the final output, so we can
-# later move it atomically.
-tmp_output="$( mktemp "${output}.XXXXXX" )"
-
-# 'mktemp' creates files with 'go=-rwx', so the files are not accessible
-# to users other than the one doing the download (and root, of course).
-# This can be problematic when a shared BR2_DL_DIR is used by different
-# users (e.g. on a build server), where all users may write to the shared
-# location, since other users would not be allowed to read the files
-# another user downloaded.
-# So, we restore the 'go' access rights to a more sensible value, while
-# still abiding by the current user's umask. We must do that before the
-# final 'mv', so just do it now.
-# Some helpers (cp and scp) may create executable files, so we need to
-# carry the executable bit if needed.
-[ -x "${tmpf}" ] && new_mode=755 || new_mode=644
-new_mode=$( printf "%04o" $((0${new_mode} & ~0$(umask))) )
-chmod ${new_mode} "${tmp_output}"
-
-# We must *not* unlink tmp_output, otherwise there is a small window
-# during which another download process may create the same tmp_output
-# name (very, very unlikely; but not impossible.)
-# Using 'cp' is not reliable, since 'cp' may unlink the destination file
-# if it is unable to open it with O_WRONLY|O_TRUNC; see:
-#   http://pubs.opengroup.org/onlinepubs/9699919799/utilities/cp.html
-# Since the destination filesystem can be anything, it might not support
-# O_TRUNC, so 'cp' would unlink it first.
-# Use 'cat' and append-redirection '>>' to save to the final location,
-# since that is the only way we can be 100% sure of the behaviour.
-if ! cat "${tmpf}" >>"${tmp_output}"; then
-    rm -rf "${tmpd}" "${tmp_output}"
-    exit 1
-fi
-rm -rf "${tmpd}"
-# tmp_output and output are on the same filesystem, so POSIX guarantees
-# that 'mv' is atomic, because it then uses rename() that POSIX mandates
-# to be atomic, see:
-#   http://pubs.opengroup.org/onlinepubs/9699919799/functions/rename.html
-if ! mv "${tmp_output}" "${output}"; then
-    rm -f "${tmp_output}"
-    exit 1
-fi
-- 
1.9.1

[Buildroot] [PATCH 3/5 v2] pkg-download: check for already downloaded file in the download wrapper

[Yann E. MORIN]

Instead of repeating the same test again and again in all our download
rules, just delegate the check for an already downloaded file to the
download wrapper.

This clears up the path for doing the hash checks on a cached file
before the download.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Peter Korsgaard <jacmet@uclibc.org>
Cc: Gustavo Zacarias <gustavo@zacarias.com.ar>
---
 package/pkg-download.mk     | 8 --------
 support/download/dl-wrapper | 5 +++++
 2 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/package/pkg-download.mk b/package/pkg-download.mk
index e76bb38..9192950 100644
--- a/package/pkg-download.mk
+++ b/package/pkg-download.mk
@@ -96,7 +96,6 @@ endef
 # Messages for the type of clone used are provided to ease debugging in case of
 # problems
 define DOWNLOAD_GIT
-	test -e $(DL_DIR)/$($(PKG)_SOURCE) || \
 	$(EXTRA_ENV) $(DL_WRAPPER) -b git \
 		-o $(DL_DIR)/$($(PKG)_SOURCE) \
 		-- \
@@ -117,7 +116,6 @@ endef
 
 
 define DOWNLOAD_BZR
-	test -e $(DL_DIR)/$($(PKG)_SOURCE) || \
 	$(EXTRA_ENV) $(DL_WRAPPER) -b bzr \
 		-o $(DL_DIR)/$($(PKG)_SOURCE) \
 		-- \
@@ -135,7 +133,6 @@ define SHOW_EXTERNAL_DEPS_BZR
 endef
 
 define DOWNLOAD_CVS
-	test -e $(DL_DIR)/$($(PKG)_SOURCE) || \
 	$(EXTRA_ENV) $(DL_WRAPPER) -b cvs \
 		-o $(DL_DIR)/$($(PKG)_SOURCE) \
 		-- \
@@ -155,7 +152,6 @@ define SHOW_EXTERNAL_DEPS_CVS
 endef
 
 define DOWNLOAD_SVN
-	test -e $(DL_DIR)/$($(PKG)_SOURCE) || \
 	$(EXTRA_ENV) $(DL_WRAPPER) -b svn \
 		-o $(DL_DIR)/$($(PKG)_SOURCE) \
 		-- \
@@ -176,7 +172,6 @@ endef
 # Note that filepath is relative to the user's home directory, so you may want
 # to prepend the path with a slash: scp://[user@]host:/absolutepath
 define DOWNLOAD_SCP
-	test -e $(DL_DIR)/$(2) || \
 	$(EXTRA_ENV) $(DL_WRAPPER) -b scp \
 		-o $(DL_DIR)/$(2) \
 		-- \
@@ -194,7 +189,6 @@ endef
 
 
 define DOWNLOAD_HG
-	test -e $(DL_DIR)/$($(PKG)_SOURCE) || \
 	$(EXTRA_ENV) $(DL_WRAPPER) -b hg \
 		-o $(DL_DIR)/$($(PKG)_SOURCE) \
 		-- \
@@ -215,7 +209,6 @@ endef
 
 
 define DOWNLOAD_WGET
-	test -e $(DL_DIR)/$(2) || \
 	$(EXTRA_ENV) $(DL_WRAPPER) -b wget \
 		-o $(DL_DIR)/$(2) \
 		-- \
@@ -232,7 +225,6 @@ define SHOW_EXTERNAL_DEPS_WGET
 endef
 
 define DOWNLOAD_LOCALFILES
-	test -e $(DL_DIR)/$(2) || \
 	$(EXTRA_ENV) $(DL_WRAPPER) -b cp \
 		-o $(DL_DIR)/$(2) \
 		-- \
diff --git a/support/download/dl-wrapper b/support/download/dl-wrapper
index 078ae33..eadc56d 100755
--- a/support/download/dl-wrapper
+++ b/support/download/dl-wrapper
@@ -49,6 +49,11 @@ main() {
         error "no output specified, use -o\n"
     fi
 
+    # If the output file already exists, do not download it again
+    if [ -e "${output}" ]; then
+        exit 0
+    fi
+
     # tmpd is a temporary directory in which backends may store intermediate
     # by-products of the download.
     # tmpf is the file in which the backends should put the downloaded content.
-- 
1.9.1

[Buildroot] [PATCH 4/5 v2] pkg-download: verify the hashes from the download wrapper

[Yann E. MORIN]

Instead of repeating the check in our download rules, delegate the check
of the hashes to the download wrapper.

This needs three different changes:

  - add a new argument to the download wrapper, that is the full path to
    the hash file; if the hash file does not exist, that does not change
    the current behaviour, as the existence of the hash file is checked
    for in the check-hash script;

  - add a third argument to the check-hash script, to be the basename of
    the file to check; this is required because we no longer check the
    final file with the final filename, but an intermediate file with a
    temporary filename;

  - do the actual call to the check-hash script from within the download
    wrapper.

This further paves the way to doing pre-download checks of the hashes
for the locally cached files.

Note: this patch removes the check for hashes for already downloaded
files, since the wrapper script exits early. The behaviour to check
localy cached files will be restored and enhanced in the following
patch.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Peter Korsgaard <jacmet@uclibc.org>
Cc: Gustavo Zacarias <gustavo@zacarias.com.ar>
---
 package/pkg-download.mk     | 22 ++++++++--------------
 support/download/check-hash | 14 ++++++++------
 support/download/dl-wrapper | 22 +++++++++++++++++++---
 3 files changed, 35 insertions(+), 23 deletions(-)

diff --git a/package/pkg-download.mk b/package/pkg-download.mk
index 9192950..b3ddfe3 100644
--- a/package/pkg-download.mk
+++ b/package/pkg-download.mk
@@ -60,17 +60,6 @@ domainseparator = $(if $(1),$(1),/)
 # github(user,package,version): returns site of GitHub repository
 github = https://github.com/$(1)/$(2)/archive/$(3)
 
-# Helper for checking a tarball's checksum
-# If the hash does not match, remove the incorrect file
-# $(1): the path to the file with the hashes
-# $(2): the full path to the file to check
-define VERIFY_HASH
-	if ! support/download/check-hash $(1) $(2) $(if $(QUIET),>/dev/null); then \
-		rm -f $(2); \
-		exit 1; \
-	fi
-endef
-
 ################################################################################
 # The DOWNLOAD_* helpers are in charge of getting a working copy
 # of the source repository for their corresponding SCM,
@@ -98,6 +87,7 @@ endef
 define DOWNLOAD_GIT
 	$(EXTRA_ENV) $(DL_WRAPPER) -b git \
 		-o $(DL_DIR)/$($(PKG)_SOURCE) \
+		-H $(PKGDIR)/$($(PKG)_NAME).hash \
 		-- \
 		$($(PKG)_SITE) \
 		$($(PKG)_DL_VERSION) \
@@ -118,6 +108,7 @@ endef
 define DOWNLOAD_BZR
 	$(EXTRA_ENV) $(DL_WRAPPER) -b bzr \
 		-o $(DL_DIR)/$($(PKG)_SOURCE) \
+		-H $(PKGDIR)/$($(PKG)_NAME).hash \
 		-- \
 		$($(PKG)_SITE) \
 		$($(PKG)_DL_VERSION) \
@@ -135,6 +126,7 @@ endef
 define DOWNLOAD_CVS
 	$(EXTRA_ENV) $(DL_WRAPPER) -b cvs \
 		-o $(DL_DIR)/$($(PKG)_SOURCE) \
+		-H $(PKGDIR)/$($(PKG)_NAME).hash \
 		-- \
 		$(call stripurischeme,$(call qstrip,$($(PKG)_SITE))) \
 		$($(PKG)_DL_VERSION) \
@@ -154,6 +146,7 @@ endef
 define DOWNLOAD_SVN
 	$(EXTRA_ENV) $(DL_WRAPPER) -b svn \
 		-o $(DL_DIR)/$($(PKG)_SOURCE) \
+		-H $(PKGDIR)/$($(PKG)_NAME).hash \
 		-- \
 		$($(PKG)_SITE) \
 		$($(PKG)_DL_VERSION) \
@@ -174,9 +167,9 @@ endef
 define DOWNLOAD_SCP
 	$(EXTRA_ENV) $(DL_WRAPPER) -b scp \
 		-o $(DL_DIR)/$(2) \
+		-H $(PKGDIR)/$($(PKG)_NAME).hash \
 		-- \
 		'$(call stripurischeme,$(call qstrip,$(1)))' && \
-	$(call VERIFY_HASH,$(PKGDIR)/$($(PKG)_NAME).hash,$(DL_DIR)/$(2))
 endef
 
 define SOURCE_CHECK_SCP
@@ -191,6 +184,7 @@ endef
 define DOWNLOAD_HG
 	$(EXTRA_ENV) $(DL_WRAPPER) -b hg \
 		-o $(DL_DIR)/$($(PKG)_SOURCE) \
+		-H $(PKGDIR)/$($(PKG)_NAME).hash \
 		-- \
 		$($(PKG)_SITE) \
 		$($(PKG)_DL_VERSION) \
@@ -211,9 +205,9 @@ endef
 define DOWNLOAD_WGET
 	$(EXTRA_ENV) $(DL_WRAPPER) -b wget \
 		-o $(DL_DIR)/$(2) \
+		-H $(PKGDIR)/$($(PKG)_NAME).hash \
 		-- \
 		'$(call qstrip,$(1))' && \
-	$(call VERIFY_HASH,$(PKGDIR)/$($(PKG)_NAME).hash,$(DL_DIR)/$(2))
 endef
 
 define SOURCE_CHECK_WGET
@@ -227,9 +221,9 @@ endef
 define DOWNLOAD_LOCALFILES
 	$(EXTRA_ENV) $(DL_WRAPPER) -b cp \
 		-o $(DL_DIR)/$(2) \
+		-H $(PKGDIR)/$($(PKG)_NAME).hash \
 		-- \
 		$(call stripurischeme,$(call qstrip,$(1))) && \
-	$(call VERIFY_HASH,$(PKGDIR)/$($(PKG)_NAME).hash,$(DL_DIR)/$(2))
 endef
 
 define SOURCE_CHECK_LOCALFILES
diff --git a/support/download/check-hash b/support/download/check-hash
index 13e361a..b41a87e 100755
--- a/support/download/check-hash
+++ b/support/download/check-hash
@@ -5,9 +5,11 @@ set -e
 # Call it with:
 #   $1: the path of the file containing all the the expected hashes
 #   $2: the full path to the file to check
+#   $3: the basename of the file to check
 
 h_file="${1}"
 file="${2}"
+base="${3}"
 
 # Does the hash-file exist?
 if [ ! -f "${h_file}" ]; then
@@ -30,7 +32,7 @@ check_one_hash() {
         sha224|sha256|sha384|sha512)    ;;
         *) # Unknown hash, exit with error
             printf "ERROR: unknown hash '%s' for '%s'\n"  \
-                   "${_h}" "${_file##*/}" >&2
+                   "${_h}" "${base}" >&2
             exit 1
             ;;
     esac
@@ -38,11 +40,11 @@ check_one_hash() {
     # Do the hashes match?
     _hash=$( ${_h}sum "${_file}" |cut -d ' ' -f 1 )
     if [ "${_hash}" = "${_known}" ]; then
-        printf "%s: OK (%s: %s)\n" "${_file##*/}" "${_h}" "${_hash}"
+        printf "%s: OK (%s: %s)\n" "${base}" "${_h}" "${_hash}"
         return 0
     fi
 
-    printf "ERROR: %s has wrong %s hash:\n" "${_file##*/}" "${_h}" >&2
+    printf "ERROR: %s has wrong %s hash:\n" "${base}" "${_h}" >&2
     printf "ERROR: expected: %s\n" "${_known}" >&2
     printf "ERROR: got     : %s\n" "${_hash}" >&2
     printf "ERROR: Incomplete download, or man-in-the-middle (MITM) attack\n" >&2
@@ -59,7 +61,7 @@ while read t h f; do
             continue
             ;;
         *)
-            if [ "${f}" = "${file##*/}" ]; then
+            if [ "${f}" = "${base}" ]; then
                 check_one_hash "${t}" "${h}" "${file}"
                 : $((nb_checks++))
             fi
@@ -69,9 +71,9 @@ done <"${h_file}"
 
 if [ ${nb_checks} -eq 0 ]; then
     if [ -n "${BR2_ENFORCE_CHECK_HASH}" ]; then
-        printf "ERROR: No hash found for %s\n" "${file}" >&2
+        printf "ERROR: No hash found for %s\n" "${base}" >&2
         exit 1
     else
-        printf "WARNING: No hash found for %s\n" "${file}" >&2
+        printf "WARNING: No hash found for %s\n" "${base}" >&2
     fi
 fi
diff --git a/support/download/dl-wrapper b/support/download/dl-wrapper
index eadc56d..5552547 100755
--- a/support/download/dl-wrapper
+++ b/support/download/dl-wrapper
@@ -8,6 +8,7 @@
 # Call it with:
 #   -b BACKEND  name of the backend (eg. cvs, git, cp...)
 #   -o FILE     full path to the file in which to save the download
+#   -H FILE     full path to the hash file
 #   --          everything after '--' are options for the backend
 # Environment:
 #   BUILD_DIR: the path to Buildroot's build dir
@@ -26,15 +27,16 @@ set -e
 
 main() {
     local OPT OPTARG
-    local backend output
+    local backend output hfile
 
     # Parse our options; anythong after '--' is for the backend
     # Sop we only care to -b (backend) and -o (output file)
-    while getopts :hb:o: OPT; do
+    while getopts :hb:o:H: OPT; do
         case "${OPT}" in
         h)  help; exit 0;;
         b)  backend="${OPTARG}";;
         o)  output="${OPTARG}";;
+        H)  hfile="${OPTARG}";;
         :)  error "option '%s' expects a mandatory argument\n" "${OPTARG}";;
         \?) error "unknown option '%s'\n" "${OPTARG}";;
         esac
@@ -48,6 +50,9 @@ main() {
     if [ -z "${output}" ]; then
         error "no output specified, use -o\n"
     fi
+    if [ -z "${file}" ]; then
+        error "no hash-file specified, use -H\n"
+    fi
 
     # If the output file already exists, do not download it again
     if [ -e "${output}" ]; then
@@ -81,6 +86,13 @@ main() {
     # cd back to free the temp-dir, so we can remove it later
     cd "${OLDPWD}"
 
+    # Check if the downloaded file is sane, and matches the stored hashes
+    # for that file
+    if ! support/download/check-hash "${hfile}" "${tmpf}" "${output##*/}"; then
+        rm -rf "${tmpd}"
+        exit 1
+    fi
+
     # tmp_output is in the same directory as the final output, so we can
     # later move it atomically.
     tmp_output="$( mktemp "${output}.XXXXXX" )"
@@ -147,7 +159,7 @@ DESCRIPTIOM
             bzr     Bazaar
             cp      local files via simple copy
             cvs     Concurrent Versions System
-            git     git
+            git     Git
             hg      Mercurial
             scp     remote files via Secure copy
             svn     Subversion
@@ -156,6 +168,10 @@ DESCRIPTIOM
     -o FILE
         Store the downloaded archive in FILE.
 
+    -H FILE
+        Use FILE to read hashes from, and check them against the downloaded
+        archive.
+
   Exit status:
     0   if OK
     !0  in case of error
-- 
1.9.1

[Buildroot] [PATCH 5/5 v2] pkg-download: check hashes for locally cached files

[Yann E. MORIN]

In some cases, upstream just update their releases in-place, without
renaming them. When that package is updated in Buildroot, a new hash to
match the new upstream release is included in the corresponding .hash
file.

As a consequence, users who previously downloaded that package's tarball
with an older version of Buildroot, will get stuck with an old archive
for that package, and after updating their Buildroot copy, will be greeted
with a failed download, due to the local file not matching the new
hashes.

So, to avoid this situation, check the hashes prior to doing the
download. If the hashes match, consider the locally cached file genuine,
and do not download it. However, if the locally cached file does not
match the known hashes we have for it, it is promptly removed, and a
download is re-attempted.

Note: this does not add any overhead compared to the previous situation,
because we were already checking hashes of localy cached files. It just
changes the order in which we do the checks. For the records, here is the
overhead of hashing a 231MiB file (qt-everywhere-opensource-src-4.8.6.tar.gz)
on a core-i5 @2.5GHz:

            cache-cold  cache-hot
    sha1      1.914s      0.762s
    sha256    2.109s      1.270s

But again, this overhead already existed before this patch.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Peter Korsgaard <jacmet@uclibc.org>
Cc: Gustavo Zacarias <gustavo@zacarias.com.ar>
---
 support/download/dl-wrapper | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/support/download/dl-wrapper b/support/download/dl-wrapper
index 5552547..ad0ed46 100755
--- a/support/download/dl-wrapper
+++ b/support/download/dl-wrapper
@@ -56,7 +56,11 @@ main() {
 
     # If the output file already exists, do not download it again
     if [ -e "${output}" ]; then
-        exit 0
+        if support/download/check-hash "${hfile}" "${output}" "${output##*/}"; then
+            exit 0
+        fi
+        rm -f "${output}"
+        printf "Re-downloading '%s'...\n" "${output##*/}"
     fi
 
     # tmpd is a temporary directory in which backends may store intermediate
-- 
1.9.1

[Buildroot] [PATCH 2/5 v2] suppot/download: add option parsing to the download wrapper

[Thomas Petazzoni]

Dear Yann E. MORIN,

On Sun,  7 Dec 2014 12:02:35 +0100, Yann E. MORIN wrote:

> +help() {
> +    cat <<_EOF_
> +NAME
> +    ${my_name} - downlaod wrapper for Buildroot
> +
> +SYNOPSIS
> +    ${my_name} [OPTION]... -- [BACKEND OPTION]...

Why do you create help text that look like man pages? It is very
unusual, most Unix commands don't have a help text that look like a man
page.

I had the same reflexion on the mke2img thing.

> +DESCRIPTIOM

DESCRIPTION

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 1/5 v2] pkg-download: fix arguments to hash checking script

[Thomas Petazzoni]

Dear Yann E. MORIN,

On Sun,  7 Dec 2014 12:02:34 +0100, Yann E. MORIN wrote:
> The argument are correctly used, but incorrectly documented.
> Inverse the comments to match the actual usage.
> 
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> ---
>  support/download/check-hash | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Applied, thanks.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 4/5 v2] pkg-download: verify the hashes from the download wrapper

[Yann E. MORIN]

All,

On 2014-12-07 12:02 +0100, Yann E. MORIN spake thusly:
> Instead of repeating the check in our download rules, delegate the check
> of the hashes to the download wrapper.
[--SNIP--]
> diff --git a/package/pkg-download.mk b/package/pkg-download.mk
> index 9192950..b3ddfe3 100644
> --- a/package/pkg-download.mk
> +++ b/package/pkg-download.mk
[--SNIP--]
> @@ -174,9 +167,9 @@ endef
>  define DOWNLOAD_SCP
>  	$(EXTRA_ENV) $(DL_WRAPPER) -b scp \
>  		-o $(DL_DIR)/$(2) \
> +		-H $(PKGDIR)/$($(PKG)_NAME).hash \
>  		-- \
>  		'$(call stripurischeme,$(call qstrip,$(1)))' && \

Damn, left-over bug... :-( Uncommitted file. Forgot git add... :-(

Marking this series as "Changes requested", because we suddenly
uncovered another hash-related bug (especially without that series),
which happens for host packages for which the tarball is broken: hashes
are not checked for host packages...

I'll investigate that problem before I respin the series...

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 4/5 v2] pkg-download: verify the hashes from the download wrapper

[Yann E. MORIN]

Thomas, All,

On 2014-12-08 00:10 +0100, Yann E. MORIN spake thusly:
> On 2014-12-07 12:02 +0100, Yann E. MORIN spake thusly:
> > Instead of repeating the check in our download rules, delegate the check
> > of the hashes to the download wrapper.
> [--SNIP--]
> > diff --git a/package/pkg-download.mk b/package/pkg-download.mk
> > index 9192950..b3ddfe3 100644
> > --- a/package/pkg-download.mk
> > +++ b/package/pkg-download.mk
> [--SNIP--]
> > @@ -174,9 +167,9 @@ endef
> >  define DOWNLOAD_SCP
> >  	$(EXTRA_ENV) $(DL_WRAPPER) -b scp \
> >  		-o $(DL_DIR)/$(2) \
> > +		-H $(PKGDIR)/$($(PKG)_NAME).hash \
> >  		-- \
> >  		'$(call stripurischeme,$(call qstrip,$(1)))' && \
> 
> Damn, left-over bug... :-( Uncommitted file. Forgot git add... :-(
> 
> Marking this series as "Changes requested", because we suddenly
> uncovered another hash-related bug (especially without that series),
> which happens for host packages for which the tarball is broken: hashes
> are not checked for host packages...

OK, I found the issue.

When we download a host package, we pass the hash-file as:
    package/PKG/host-PKG.hash

This is wrong, and this series caught the issue.

I'll rework this series to handle the case for host packages.

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.  |
'------------------------------^-------^------------------^--------------------'