support/download: be more aggressive on missing hashes (branch yem/dl-hash)

Hello All!

This series makes hashes mandatory when a .hash file exists.

Currently, we treat missing hashes as a mere warning. Unfortunately, that often goes un-noticed by most users, and thus we get a lot of package bumps that are missing the new hashes corresponding to the new version.

We now make that a hard error, so users really notice something is wrong.

Of course, if no .hash file exists, the behaviour is as yet unchanged.

Changes v1 -> v2:
- make it work for downloads from git/svn/... repositories

Regards,
Yann E. MORIN.

The following changes since commit 2d05afa42792193fa392c9f5417e8effc73d1e38:

cups: deprecate package due to security issues (2015-03-16 22:26:30 +0100)

are available in the git repository at:

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

for you to fetch changes up to b9c42017c424fae04a5bae3d8a74c3b21de65370:

support/download: always fail when there's no hash (2015-03-17 12:17:20 +0100)

----------------------------------------------------------------
Yann E. MORIN (5):
support/download: make hash file optional
package infra: do not check hashes when downloading from a repository support/download: return different exit codes for different failures support/download: properly catch missing hashes support/download: always fail when there's no hash

 docs/manual/adding-packages-directory.txt |  6 ++----
 package/pkg-download.mk                   |  5 -----
 support/download/check-hash               | 17 ++++++++++-------
 support/download/dl-wrapper               | 14 ++++++++++----
 4 files changed, 22 insertions(+), 20 deletions(-)

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

Currently, specifying a hash file for our download wrapper is mandatory.

However, when we download a git, svn, bzr, hg or cvs tree, there's by design no hash to check the download against.

Since we're going to have hash checking mandatory when a hash file exists, this would break those downloads from a repository.

So, make specifying a hash file optional when calling our download wrapper and bail out early from the check-hash script if no hash file is specified.

---
 support/download/check-hash | 2 +-
 support/download/dl-wrapper | 3 ---
 2 files changed, 1 insertion(+), 4 deletions(-)

index 4c07274..cee64ef 100755
@@ -23,7 +23,7 @@ file="${2}"
 base="${3}"
 
 # Does the hash-file exist?
-if [ ! -f "${h_file}" ]; then
+if [ -z "${h_file}" -o ! -f "${h_file}" ]; then
     exit 0
 fi
 

index 3b30840..514118c 100755
@@ -44,9 +44,6 @@ main() {
     if [ -z "${output}" ]; then
         error "no output specified, use -o\n"
     fi
-    if [ -z "${hfile}" ]; 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

1.9.1

I would actually have squashed the following patch into this one, since for
reviewing you anyway have to look at the two together.

Regards,
Arnout

[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

An alternative approach would be to allow an empty hash in the hash file, e.g.

# From git => no hash
none xxx avrdude-eabe067c4527bc2eedc5db9288ef5cf1818ec720.tar.gz

This has the advantage that we don't have to revert this patch in the future
when we _do_ make reproducible tarballs (which is not rocket science, the reproducible builds people in Debian and Fedora do it). Of course, we'll be stuck with a s*tload of hash files that have this empty hash...

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

Arnout, All,

On 2015-03-19 22:03 +0100, Arnout Vandecappelle spake thusly:

Well, as I state below, we'll need that. But for git/hg/svn/bzr/cvs clones/checkouts/... there is intrisically no reason to have a hash, by design.

Yes, reproducibility. But that's soooo far away... :-/

However...

At first, I was not too fond of this, but it turns out we'll have to have it. Consider the following:

    ifeq ($(FOO_BAR),y)
    FOO_VERSION = long-git-hash
    FOO_SITE = $(call github,foo,bar,$(FOO_VERSION))
    else
    FOO_VERSION = 1.2.3
    FOO_SITE = http://foosoftware.org/download
    endif

Say we add a hash for version 1.2.3; currently, we do not add hashes for archives downloaded from github, because they seem to be non-reproducible. However, the github helper is not using git-clone, but us really just a way to generate an http:// UEL we download with wget.

So, what happens now is that, since hashes are mandatory as long as the .hash file exists, downloads from github for this foo package is broken.

This is the case for gcc, for example, since we get the arc gcc from github, and the other versions from the GNU mirror.

So, we'll need a way to state that there is no hash for a file, but that will have to be explicit.

I'll rework the series to take that in considreation. Thanks! :-)

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

On 21/03/15 18:00, Yann E. MORIN wrote:
[snip]

Why is there no reason to have a hash? The download helpers will indeed detect
failed clones/checkouts/..., but they won't detect a failed download from the PRIMARY or SECONDARY site, e.g. if a user configures a bad PRIMARY site that always gives you a landing page rather than a 404.

Also, a second reason to have the hash is for "security", to protect against
MITM attacks. git with a sha1 will protect against that, but not if you give it a tag. And svn, well, I'll leave that as an exercise for the reader :-)

Regards,
Arnout

[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

When downloading from a repository, we have no way to ensure the reproducibility of the generated archives, so we can't check the hashes.

Do not specifiy a hash file in those cases.

---
 package/pkg-download.mk | 5 -----
 1 file changed, 5 deletions(-)

index 5e74519..e274712 100644
@@ -87,7 +87,6 @@ github = https://github.com/$(1)/$(2)/archive/$(3)
 define DOWNLOAD_GIT
 	$(EXTRA_ENV) $(DL_WRAPPER) -b git \
 		-o $(DL_DIR)/$($(PKG)_SOURCE) \
-		-H $(PKGDIR)/$($(PKG)_RAWNAME).hash \
 		$(QUIET) \
 		-- \
 		$($(PKG)_SITE) \

@@ -109,7 +108,6 @@ endef
 define DOWNLOAD_BZR
 	$(EXTRA_ENV) $(DL_WRAPPER) -b bzr \
 		-o $(DL_DIR)/$($(PKG)_SOURCE) \
-		-H $(PKGDIR)/$($(PKG)_RAWNAME).hash \
 		$(QUIET) \
 		-- \
 		$($(PKG)_SITE) \

@@ -128,7 +126,6 @@ endef
 define DOWNLOAD_CVS
 	$(EXTRA_ENV) $(DL_WRAPPER) -b cvs \
 		-o $(DL_DIR)/$($(PKG)_SOURCE) \
-		-H $(PKGDIR)/$($(PKG)_RAWNAME).hash \
 		$(QUIET) \
 		-- \
 		$(call stripurischeme,$(call qstrip,$($(PKG)_SITE))) \

@@ -149,7 +146,6 @@ endef
 define DOWNLOAD_SVN
 	$(EXTRA_ENV) $(DL_WRAPPER) -b svn \
 		-o $(DL_DIR)/$($(PKG)_SOURCE) \
-		-H $(PKGDIR)/$($(PKG)_RAWNAME).hash \
 		$(QUIET) \
 		-- \
 		$($(PKG)_SITE) \

@@ -189,7 +185,6 @@ endef
 define DOWNLOAD_HG
 	$(EXTRA_ENV) $(DL_WRAPPER) -b hg \
 		-o $(DL_DIR)/$($(PKG)_SOURCE) \
-		-H $(PKGDIR)/$($(PKG)_RAWNAME).hash \
 		$(QUIET) \
 		-- \
 		$($(PKG)_SITE) \

1.9.1

Regards,
Arnout

[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

Return different exit codes depending on the error that occured:

0: no error (hash file missing, or all hashes match) 1: hash file exists, but at least one hash in error 2: hash file exists, but no hash for file to check

---
Changes v1 -> v2:
- typoes in script and commit (Samuel)

---
 support/download/check-hash | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

index cee64ef..9c62d7f 100755
@@ -9,6 +9,13 @@ set -e
 #   $3: the final basename of the file, to which it will be ultimately
 #       saved as, to be able to match it to the corresponding hashes
 #       in the .hash file
+#
+# Exits with:
+#   0: the hash file exists and the file to check matches all its hashes,
+#      or the hash file does not exist
+#   1: the hash file exists and the file to check does not match at least
+#      of its hashes
+#   2: the hash file exists and there was no hash to check the file against
 
 while getopts :q OPT; do
     case "${OPT}" in

@@ -83,7 +90,7 @@ done <"${h_file}"
 if [ ${nb_checks} -eq 0 ]; then
     if [ -n "${BR2_ENFORCE_CHECK_HASH}" ]; then
         printf "ERROR: No hash found for %s\n" "${base}" >&2
-        exit 1
+        exit 2
     else
         printf "WARNING: No hash found for %s\n" "${base}" >&2
     fi

1.9.1

When checking hashes reports no hash for a file, and this is treated as an error (now: because BR2_ENFORCE_CHECK_HASH is set; later: because that will be the new and only behaviour), exit promptly in error.

---
Changes v1 -> v2:
- typo in commit

---
 support/download/dl-wrapper | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

index 514118c..c5eb582 100755
@@ -45,10 +45,19 @@ main() {
         error "no output specified, use -o\n"
     fi
 
-    # If the output file already exists, do not download it again
+    # If the output file already exists and:
+    # - there's no .hash file: do not download it again and exit promptly
+    # - matches all its hashes: do not download it again and exit promptly
+    # - fails at least one of its hashes: force a re-download
+    # - there's no hash (but a .hash file): consider it a hard error
     if [ -e "${output}" ]; then
         if support/download/check-hash ${quiet} "${hfile}" "${output}" "${output##*/}"; then
             exit 0
+        elif [ ${?} -eq 2 ]; then
+            # Do not remove the file, otherwise it might get re-downloaded
+            # from a later location (i.e. primary -> upstream -> mirror).
+            # Do not print a message, check-hash already did
+            exit 2
         fi
         rm -f "${output}"
         warn "Re-downloading '%s'...\n" "${output##*/}"

1.9.1

Regards,
Arnout

[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

At the time we introduced hashes, we did not want to be too harsh in the beginning, and give people some time to adapt and accept the hashes. So we so far only whined^Wwarned about a missing hash (when the .hash file exists).

Some time has passed now, and people are still missing updating hashes when bumping packages.

Let's make that warning a little bit more annoying...

---
 docs/manual/adding-packages-directory.txt | 6 ++----
 support/download/check-hash               | 8 ++------
 2 files changed, 4 insertions(+), 10 deletions(-)

index 1ce9a3b..febb33c 100644
@@ -469,9 +469,7 @@ not match, Buildroot considers this an error, deletes the downloaded file,
 and aborts.
 
 If the +.hash+ file is present, but it does not contain a hash for a
-downloaded file, no check is done for that file. If you set the
-environment variable +BR2_ENFORCE_CHECK_HASH+ to a non-empty value, and
-there is no hash for a downloaded file, Buildroot considers this an
-error, deletes the downloaded file, and aborts.
+downloaded file, Buildroot considers this an error and aborts (but leaves
+the downloaded file in place).
 
 If the +.hash+ file is missing, then no check is done at all.

index 9c62d7f..0caa619 100755
@@ -88,10 +88,6 @@ while read t h f; do
 done <"${h_file}"
 
 if [ ${nb_checks} -eq 0 ]; then
-    if [ -n "${BR2_ENFORCE_CHECK_HASH}" ]; then
-        printf "ERROR: No hash found for %s\n" "${base}" >&2
-        exit 2
-    else
-        printf "WARNING: No hash found for %s\n" "${base}" >&2
-    fi
+    printf "ERROR: No hash found for %s\n" "${base}" >&2
+    exit 2
 fi

1.9.1