Skip to contentSkip to conversation

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

PATCH Superseded Yann E. MORIN 4 participants 18 messages Mar 22, 2015 – Mar 27, 2015

v3 of 4 Superseded Latest is v4 →

lore
Share & tools
lore
#0 lore raw

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

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 v2 -> v3:
- further differentiat hash check errors (Arnout)
- add 'none' hash type (Arnout)
- further improve the manual (Arnout)

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

Regards,
Yann E. MORIN.

The following changes since commit e16796942a6f68391e69961e5045ce20b56063e2:

attr: backport upstream patches to fix build on musl (2015-03-21 10:13:51 +0100)

are available in the git repository at:

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

for you to fetch changes up to a30751a9b30f9cc2b22bacffa659ef311622e180:

support/download: warn when there's no .hash file (2015-03-22 15:40:05 +0100)

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

 docs/manual/adding-packages-directory.txt | 27 +++++++++++++++++++++------
 package/pkg-download.mk                   |  5 -----
 support/download/check-hash               | 26 +++++++++++++++++---------
 support/download/dl-wrapper               | 14 ++++++++++----
 4 files changed, 48 insertions(+), 24 deletions(-)
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'
#1 lore raw

PATCH v3 1/7 support/download: make hash file optional

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(-)
support/download/check-hash +1 -1
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
 
support/download/dl-wrapper +0 -3
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
Signature · 1 line
1.9.1
#2 lore raw

PATCH v3 1/7 support/download: make hash file optional

Quoted 8 lines · Yann E. MORIN

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.

Quoted 7 lines

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.

Regards,

Signature · 1 line
Samuel
#3 lore raw

PATCH v3 1/7 support/download: make hash file optional

In reply to Yann E. MORIN

Yann,

Quoted 4 lines · Yann E. MORIN

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.

I was thinking about hashes for the git/svn/(other VCS) and how these sources could be provided by the buildroot sources mirror -
http://sources.buildroot.org/ or a primary download site. Do you have an idea of how we could utilize hash checking if buildroot were to pull the sources from one of these methods? There could be a "man in the middle" attack since the sources mirror or the primary site just provides a tar of these VCS repositories

This could be especially useful for when the BR2_PRIMARY_SITE_ONLY is used. This wouldn't necessarily for "man in the middle" attacks but for ensure that your downloads don't get corrupt.

To be clear - I am not suggesting that this series should take this into consideration but it was something that I would like to get out there before I forget.

Thanks,
-Ryan

[...]

Signature · 5 lines
Ryan Barnett / Sr Software Engineer
Airborne Information Systems / Security Systems and Software
MS 131-100, C Ave NE, Cedar Rapids, IA, 52498, USA
ryan.barnett at rockwellcollins.com
www.rockwellcollins.com
#4 lore raw

PATCH v3 1/7 support/download: make hash file optional

Quoted 16 lines · Ryan Barnett

Yann,

Quoted 4 lines · Yann E. MORIN

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.

I was thinking about hashes for the git/svn/(other VCS) and how these sources could be provided by the buildroot sources mirror -
http://sources.buildroot.org/ or a primary download site. Do you have an idea of how we could utilize hash checking if buildroot were to pull the sources from one of these methods? There could be a "man in the middle" attack since the sources mirror or the primary site just provides a tar of these VCS repositories

Quoted 3 lines

This could be especially useful for when the BR2_PRIMARY_SITE_ONLY is used. This wouldn't necessarily for "man in the middle" attacks but for ensure that your downloads don't get corrupt.

Yes, I had exactly the same concept in mind. The Debian Reproducible Builds
Wiki [1] has some interesting leads. It basically boils down to adding --owner, --group and --mtime options to tar, and making sure that the list of files is passed explicitly instead of relying on directory order.

One difficulty is git archive, because AFAIK it doesn't support passing extra
arguments to tar or choosing the ordering of the files.

Regards,
Arnout

[1] https://wiki.debian.org/ReproducibleBuilds/Howto

Signature · 6 lines
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
#5 lore raw

PATCH v3 2/7 package infra: do not check hashes when downloading from a repository

In reply to Yann E. MORIN

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.

---
Changes v2 -> v3:
- add blurb in the manual (Arnout)

---
 docs/manual/adding-packages-directory.txt | 6 ++++++
 package/pkg-download.mk                   | 5 -----
 2 files changed, 6 insertions(+), 5 deletions(-)
docs/manual/adding-packages-directory.txt +6 -0
index 1ce9a3b..132c702 100644
@@ -474,4 +474,10 @@ 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.
 
+Sources that are downloaded from a version control system (git, subversion,
+etc...) can not have a hash, because the version control system and tar
+may not create exactly the same file (dates, files ordering...), so the
+hash could be wrong even for a valid download. Therefore, the hash check
+is entirely skipped for such sources.
+
 If the +.hash+ file is missing, then no check is done at all.
package/pkg-download.mk +0 -5
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) \
Signature · 1 line
1.9.1
#6 lore raw

PATCH v3 2/7 package infra: do not check hashes when downloading from a repository

Quoted 8 lines · Yann E. MORIN

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.

Regards,

Signature · 1 line
Samuel
#7 lore raw

PATCH v3 3/7 support/download: add explicit no-hash support

In reply to Yann E. MORIN

Add support to explicitly state that an archive has no hash.

This can be used for archives downloaded from a repository, like a git-clone or a subversion checkout, or using the github helper.

This will come in handy when we'll eventually make hashes mandatory as soon as a .hash file exists: for some packages, like gcc, some versions are downloaded as archives from upstream, while other versions may come from a GitHub repository (via the github herlper).

In this case, a .hash file would exist, that contains hashes for the downloaded tarballs, but archives downloaded from the repository would not have a hash (since it is currently not possible to have reproducible such archives). So, we'd need a way to explicitly state there is no hash, on purpose, for those archives.

So, add 'none' as a new type of hash.

Cc Thomas Petazzoni <[email protected]>
Cc Arnout Vandecappelle <[email protected]>
---
 docs/manual/adding-packages-directory.txt | 13 +++++++++++--
 support/download/check-hash               |  2 ++
 2 files changed, 13 insertions(+), 2 deletions(-)
docs/manual/adding-packages-directory.txt +11 -2
index 132c702..0f8bb64 100644
@@ -423,8 +423,9 @@ The format of this file is one line for each file for which to check the
 hash, each line being space-separated, with these three fields:
 
 * the type of hash, one of:
-** +sha1+, +sha224+, +sha256+, +sha384+, +sha512+
+** +sha1+, +sha224+, +sha256+, +sha384+, +sha512+, +none+
 * the hash of the file:
+** for +none+, one or more non-space chars, usually just the string +xxx+
 ** for +sha1+, 40 hexadecimal characters
 ** for +sha224+, 56 hexadecimal characters
 ** for +sha256+, 64 hexadecimal characters
@@ -448,9 +449,14 @@ comment line above the hashes.
 *Note:* the number of spaces does not matter, so one can use spaces to
 properly align the different fields.
 
+The +none+ hash type is reserved to those archives downloaded from a
+repository, like a 'git clone', a 'subversion checkout'... or archives
+downloaded with the xref:github-download-url[github helper].
+
 The example below defines a +sha1+ and a +sha256+ published by upstream for
 the main +libfoo-1.2.3.tar.bz2+ tarball, plus two locally-computed hashes,
-a +sha256+ for a downloaded patch, and a +sha1+ for a downloaded binary blob:
+a +sha256+ for a downloaded patch, a +sha1+ for a downloaded binary blob,
+and an archive with no hash:
 
 ----
 # Hashes from: http://www.foosoftware.org/download/libfoo-1.2.3.tar.bz2.{sha1,sha256}:
@@ -460,6 +466,9 @@ sha256 efc8103cc3bcb06bda6a781532d12701eb081ad83e8f90004b39ab81b65d4369 libfoo-1
 # No upstream hashes for the following:
 sha256 ff52101fb90bbfc3fe9475e425688c660f46216d7e751c4bbdb1dc85cdccacb9 libfoo-fix-blabla.patch
 sha1   2d608f3c318c6b7557d551a5a09314f03452f1a1                         libfoo-data.bin
+
+# Explicitly no hash for that file, comes from a git-clone:
+none   xxx                                                              libfoo-1234.tar.gz
 ----
 
 If the +.hash+ file is present, and it contains one or more hashes for a
support/download/check-hash +2 -0
index cee64ef..d0a4ab9 100755
@@ -38,7 +38,9 @@ check_one_hash() {
     # Note: md5 is supported, but undocumented on purpose.
     # Note: sha3 is not supported, since there is currently no implementation
     #       (the NIST has yet to publish the parameters).
+    # Note: 'none' means there is explicitly no hash for that file.
     case "${_h}" in
+        none) return 0;;
         md5|sha1)                       ;;
         sha224|sha256|sha384|sha512)    ;;
         *) # Unknown hash, exit with error
Signature · 1 line
1.9.1
#8 lore raw

PATCH v3 3/7 support/download: add explicit no-hash support

Quoted 16 lines · Yann E. MORIN

Add support to explicitly state that an archive has no hash.

This can be used for archives downloaded from a repository, like a git-clone or a subversion checkout, or using the github helper.

This will come in handy when we'll eventually make hashes mandatory as soon as a .hash file exists: for some packages, like gcc, some versions are downloaded as archives from upstream, while other versions may come from a GitHub repository (via the github herlper).

In this case, a .hash file would exist, that contains hashes for the downloaded tarballs, but archives downloaded from the repository would not have a hash (since it is currently not possible to have reproducible such archives). So, we'd need a way to explicitly state there is no hash, on purpose, for those archives.

Quoted 5 lines

So, add 'none' as a new type of hash.

Cc Thomas Petazzoni <[email protected]>
Cc Arnout Vandecappelle <[email protected]>

with a small nit:

[snip]

support/download/check-hash check_one_hash() { · line 43

Quoted 11 lines
support/download/check-hash +2 -0
index cee64ef..d0a4ab9 100755
@@ -38,7 +38,9 @@ check_one_hash() {
     # Note: md5 is supported, but undocumented on purpose.
     # Note: sha3 is not supported, since there is currently no implementation
     #       (the NIST has yet to publish the parameters).
+    # Note: 'none' means there is explicitly no hash for that file.
     case "${_h}" in
+        none) return 0;;

I'd put the return 0 on a separate line.

Perhaps we should also consider combining none with another hash an error.
Although the complexity of checking for that is probably not worth it.

Regards,
Arnout

Quoted 4 lines
support/download/check-hash +0 -0
         md5|sha1)                       ;;
         sha224|sha256|sha384|sha512)    ;;
         *) # Unknown hash, exit with error
Signature · 6 lines
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
#9 lore raw

PATCH v3 3/7 support/download: add explicit no-hash support

Quoted 17 lines · Arnout Vandecappelle
Quoted 16 lines · Yann E. MORIN

Add support to explicitly state that an archive has no hash.

This can be used for archives downloaded from a repository, like a git-clone or a subversion checkout, or using the github helper.

This will come in handy when we'll eventually make hashes mandatory as soon as a .hash file exists: for some packages, like gcc, some versions are downloaded as archives from upstream, while other versions may come from a GitHub repository (via the github herlper).

In this case, a .hash file would exist, that contains hashes for the downloaded tarballs, but archives downloaded from the repository would not have a hash (since it is currently not possible to have reproducible such archives). So, we'd need a way to explicitly state there is no hash, on purpose, for those archives.

Quoted 7 lines
Quoted 5 lines

So, add 'none' as a new type of hash.

Cc Thomas Petazzoni <[email protected]>
Cc Arnout Vandecappelle <[email protected]>

Regards,

Signature · 1 line
Samuel
#10 lore raw

PATCH v3 4/7 support/download: return different exit codes for different failures

In reply to Yann E. MORIN

Return different exit codes depending on the error that occured:

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

This will be used in a later patch to decide whether the downloaded file should be kept or removed.

Cc Thomas Petazzoni <[email protected]>
Cc Samuel Martin <[email protected]>
Cc Arnout Vandecappelle <[email protected]>

---
Changes v2 -> v3:
- further differentiate errors for unknown hashes and unknown command-line options (Arnout)

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

---
 support/download/check-hash | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)
support/download/check-hash +12 -3
index d0a4ab9..0234ab0 100755
@@ -9,6 +9,15 @@ 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
+#
+# Exit codes:
+#   0:  the hash file exists and the file to check matches all its hashes,
+#       or the hash file does not exist
+#   1:  unknown command-line option
+#   2:  the hash file exists and the file to check does not match at least
+#       of its hashes
+#   3:  the hash file exists and there was no hash to check the file against
+#   4:  the hash file exists and at least one hash type is unknown
 
 while getopts :q OPT; do
     case "${OPT}" in
@@ -46,7 +55,7 @@ check_one_hash() {
         *) # Unknown hash, exit with error
             printf "ERROR: unknown hash '%s' for '%s'\n"  \
                    "${_h}" "${base}" >&2
-            exit 1
+            exit 4
             ;;
     esac
 
@@ -62,7 +71,7 @@ check_one_hash() {
     printf "ERROR: got     : %s\n" "${_hash}" >&2
     printf "ERROR: Incomplete download, or man-in-the-middle (MITM) attack\n" >&2
 
-    exit 1
+    exit 2
 }
 
 # Do we know one or more hashes for that file?
@@ -85,7 +94,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 3
     else
         printf "WARNING: No hash found for %s\n" "${base}" >&2
     fi
Signature · 1 line
1.9.1
#11 lore raw

PATCH v3 4/7 support/download: return different exit codes for different failures

Yann,

Quoted 32 lines · Yann E. MORIN

Return different exit codes depending on the error that occured:

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

This will be used in a later patch to decide whether the downloaded file should be kept or removed.

Cc Thomas Petazzoni <[email protected]>
Cc Samuel Martin <[email protected]>
Cc Arnout Vandecappelle <[email protected]>

---
Changes v2 -> v3:
- further differentiate errors for unknown hashes and unknown command-line options (Arnout)

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

---
 support/download/check-hash | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)
support/download/check-hash +0 -0
index d0a4ab9..0234ab0 100755

support/download/check-hash set -e · line 18

Quoted 10 lines
support/download/check-hash +7 -0
@@ -9,6 +9,15 @@ 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
+#
+# Exit codes:
+#   0:  the hash file exists and the file to check matches all its hashes,
+#       or the hash file does not exist
+#   1:  unknown command-line option
+#   2:  the hash file exists and the file to check does not match at least
+#       of its hashes

maybe s/of its hashes/one of its hashes/

Quoted 33 lines
support/download/check-hash +5 -4
+#   3:  the hash file exists and there was no hash to check the file against
+#   4:  the hash file exists and at least one hash type is unknown
 
 while getopts :q OPT; do
     case "${OPT}" in
@@ -46,7 +55,7 @@ check_one_hash() {
         *) # Unknown hash, exit with error
             printf "ERROR: unknown hash '%s' for '%s'\n"  \
                    "${_h}" "${base}" >&2
-            exit 1
+            exit 4
             ;;
     esac
 
@@ -62,7 +71,7 @@ check_one_hash() {
     printf "ERROR: got     : %s\n" "${_hash}" >&2
     printf "ERROR: Incomplete download, or man-in-the-middle (MITM) attack\n" >&2
 
-    exit 1
+    exit 2
 }
 
 # Do we know one or more hashes for that file?
@@ -85,7 +94,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 3
     else
         printf "WARNING: No hash found for %s\n" "${base}" >&2
     fi
--
Quoted 2 lines

1.9.1

Regards,

Signature · 1 line
Samuel
#12 lore raw

PATCH v3 4/7 support/download: return different exit codes for different failures

Samuel, All,

On 2015-03-23 14:03 +0100, Samuel Martin spake thusly:

Quoted 1 line · Yann E. MORIN

[--SNIP--]

Quoted 8 lines
Quoted 8 lines
support/download/check-hash +0 -0
index d0a4ab9..0234ab0 100755
@@ -9,6 +9,15 @@ 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
Quoted 8 lines

support/download/check-hash set -e · line 18

Quoted 7 lines
support/download/check-hash +7 -0
+#
+# Exit codes:
+#   0:  the hash file exists and the file to check matches all its hashes,
+#       or the hash file does not exist
+#   1:  unknown command-line option
+#   2:  the hash file exists and the file to check does not match at least
+#       of its hashes

maybe s/of its hashes/one of its hashes/

Yes, good catch! I'll fix before resubmitting.

Regards,
Yann E. MORIN.

Signature · 6 lines
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'
#13 lore raw

PATCH v3 5/7 support/download: properly catch missing hashes

In reply to Yann E. MORIN

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 v2 -> v3:
- change the exit code to 1 (no need to differentiate exit codes here)

---
 support/download/dl-wrapper | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)
support/download/dl-wrapper +10 -1
index 514118c..ef2d872 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 [ ${?} -ne 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 1
         fi
         rm -f "${output}"
         warn "Re-downloading '%s'...\n" "${output##*/}"
Signature · 1 line
1.9.1
#14 lore raw

PATCH v3 5/7 support/download: properly catch missing hashes

Quoted 8 lines · Yann E. MORIN

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.

Regards,

Signature · 1 line
Samuel
#15 lore raw

PATCH v3 6/7 support/download: always fail when there's no hash

In reply to Yann E. MORIN

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

Cc Thomas Petazzoni <[email protected]>
Cc Arnout Vandecappelle <[email protected]>

---
Changes v2 -> v3:
- further improve the manual (Arnout)

---
 docs/manual/adding-packages-directory.txt | 8 ++++----
 support/download/check-hash               | 8 ++------
 2 files changed, 6 insertions(+), 10 deletions(-)
docs/manual/adding-packages-directory.txt +4 -4
index 0f8bb64..0051fea 100644
@@ -478,10 +478,10 @@ 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. However,
+the downloaded file is left in the download directory since this
+typically indicates that the +.hash+ file is wrong but the downloaded
+file is probably OK.
 
 Sources that are downloaded from a version control system (git, subversion,
 etc...) can not have a hash, because the version control system and tar
support/download/check-hash +2 -6
index 0234ab0..d200868 100755
@@ -92,10 +92,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 3
-    else
-        printf "WARNING: No hash found for %s\n" "${base}" >&2
-    fi
+    printf "ERROR: No hash found for %s\n" "${base}" >&2
+    exit 3
 fi
Signature · 1 line
1.9.1
#16 lore raw

PATCH v3 7/7 support/download: warn when there's no .hash file

In reply to Yann E. MORIN

Instead of silently accepting a missing .hash file, print a warning.

This can be grepped from a build log, to find packages that still have no hash, with the long-term goal of adding hashes for all packages.

Cc Thomas Petazzoni <[email protected]>
---
 support/download/check-hash | 1 +
 1 file changed, 1 insertion(+)
support/download/check-hash +1 -0
index d200868..82cc750 100755
@@ -33,6 +33,7 @@ base="${3}"
 
 # Does the hash-file exist?
 if [ -z "${h_file}" -o ! -f "${h_file}" ]; then
+    printf "WARNING: no hash file for %s\n" "${base}" >&2
     exit 0
 fi
 
Signature · 1 line
1.9.1
#17 lore raw

PATCH v3 7/7 support/download: warn when there's no .hash file

Quoted 7 lines · Yann E. MORIN

Instead of silently accepting a missing .hash file, print a warning.

This can be grepped from a build log, to find packages that still have no hash, with the long-term goal of adding hashes for all packages.

Cc Thomas Petazzoni <[email protected]>

Regards,

Signature · 1 line
Samuel