Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH 0/5 v2] support/download: be more aggressive on missing hashes (branch yem/dl-hash)
@ 2015-03-17 12:59 Yann E. MORIN
  2015-03-17 12:59 ` [Buildroot] [PATCH 1/5 v2] support/download: make hash file optional Yann E. MORIN
                   ` (4 more replies)
  0 siblings, 5 replies; 14+ messages in thread
From: Yann E. MORIN @ 2015-03-17 12:59 UTC (permalink / raw)
  To: buildroot

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH 1/5 v2] support/download: make hash file optional
  2015-03-17 12:59 [Buildroot] [PATCH 0/5 v2] support/download: be more aggressive on missing hashes (branch yem/dl-hash) Yann E. MORIN
@ 2015-03-17 12:59 ` Yann E. MORIN
  2015-03-19 20:34   ` Arnout Vandecappelle
  2015-03-19 21:03   ` Arnout Vandecappelle
  2015-03-17 12:59 ` [Buildroot] [PATCH 2/5 v2] package infra: do not check hashes when downloading from a repository Yann E. MORIN
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 14+ messages in thread
From: Yann E. MORIN @ 2015-03-17 12:59 UTC (permalink / raw)
  To: buildroot

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.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 support/download/check-hash | 2 +-
 support/download/dl-wrapper | 3 ---
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/support/download/check-hash b/support/download/check-hash
index 4c07274..cee64ef 100755
--- a/support/download/check-hash
+++ b/support/download/check-hash
@@ -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
 
diff --git a/support/download/dl-wrapper b/support/download/dl-wrapper
index 3b30840..514118c 100755
--- a/support/download/dl-wrapper
+++ b/support/download/dl-wrapper
@@ -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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH 2/5 v2] package infra: do not check hashes when downloading from a repository
  2015-03-17 12:59 [Buildroot] [PATCH 0/5 v2] support/download: be more aggressive on missing hashes (branch yem/dl-hash) Yann E. MORIN
  2015-03-17 12:59 ` [Buildroot] [PATCH 1/5 v2] support/download: make hash file optional Yann E. MORIN
@ 2015-03-17 12:59 ` Yann E. MORIN
  2015-03-19 20:36   ` Arnout Vandecappelle
  2015-03-17 12:59 ` [Buildroot] [PATCH 3/5 v2] support/download: return different exit codes for different failures Yann E. MORIN
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 14+ messages in thread
From: Yann E. MORIN @ 2015-03-17 12:59 UTC (permalink / raw)
  To: buildroot

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.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 package/pkg-download.mk | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/package/pkg-download.mk b/package/pkg-download.mk
index 5e74519..e274712 100644
--- a/package/pkg-download.mk
+++ b/package/pkg-download.mk
@@ -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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH 3/5 v2] support/download: return different exit codes for different failures
  2015-03-17 12:59 [Buildroot] [PATCH 0/5 v2] support/download: be more aggressive on missing hashes (branch yem/dl-hash) Yann E. MORIN
  2015-03-17 12:59 ` [Buildroot] [PATCH 1/5 v2] support/download: make hash file optional Yann E. MORIN
  2015-03-17 12:59 ` [Buildroot] [PATCH 2/5 v2] package infra: do not check hashes when downloading from a repository Yann E. MORIN
@ 2015-03-17 12:59 ` Yann E. MORIN
  2015-03-19 20:44   ` Arnout Vandecappelle
  2015-03-17 12:59 ` [Buildroot] [PATCH 4/5 v2] support/download: properly catch missing hashes Yann E. MORIN
  2015-03-17 12:59 ` [Buildroot] [PATCH 5/5 v2] support/download: always fail when there's no hash Yann E. MORIN
  4 siblings, 1 reply; 14+ messages in thread
From: Yann E. MORIN @ 2015-03-17 12:59 UTC (permalink / raw)
  To: buildroot

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

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Samuel Martin <s.martin49@gmail.com>

---
Changes v1 -> v2:
  - typoes in script and commit  (Samuel)
---
 support/download/check-hash | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/support/download/check-hash b/support/download/check-hash
index cee64ef..9c62d7f 100755
--- a/support/download/check-hash
+++ b/support/download/check-hash
@@ -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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH 4/5 v2] support/download: properly catch missing hashes
  2015-03-17 12:59 [Buildroot] [PATCH 0/5 v2] support/download: be more aggressive on missing hashes (branch yem/dl-hash) Yann E. MORIN
                   ` (2 preceding siblings ...)
  2015-03-17 12:59 ` [Buildroot] [PATCH 3/5 v2] support/download: return different exit codes for different failures Yann E. MORIN
@ 2015-03-17 12:59 ` Yann E. MORIN
  2015-03-19 20:45   ` Arnout Vandecappelle
  2015-03-17 12:59 ` [Buildroot] [PATCH 5/5 v2] support/download: always fail when there's no hash Yann E. MORIN
  4 siblings, 1 reply; 14+ messages in thread
From: Yann E. MORIN @ 2015-03-17 12:59 UTC (permalink / raw)
  To: buildroot

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.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

---
Changes v1 -> v2:
  - typo in commit
---
 support/download/dl-wrapper | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/support/download/dl-wrapper b/support/download/dl-wrapper
index 514118c..c5eb582 100755
--- a/support/download/dl-wrapper
+++ b/support/download/dl-wrapper
@@ -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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH 5/5 v2] support/download: always fail when there's no hash
  2015-03-17 12:59 [Buildroot] [PATCH 0/5 v2] support/download: be more aggressive on missing hashes (branch yem/dl-hash) Yann E. MORIN
                   ` (3 preceding siblings ...)
  2015-03-17 12:59 ` [Buildroot] [PATCH 4/5 v2] support/download: properly catch missing hashes Yann E. MORIN
@ 2015-03-17 12:59 ` Yann E. MORIN
  2015-03-19 20:51   ` Arnout Vandecappelle
  4 siblings, 1 reply; 14+ messages in thread
From: Yann E. MORIN @ 2015-03-17 12:59 UTC (permalink / raw)
  To: buildroot

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

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Reviewed-by: Samuel Martin <s.martin49@gmail.com>
---
 docs/manual/adding-packages-directory.txt | 6 ++----
 support/download/check-hash               | 8 ++------
 2 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/docs/manual/adding-packages-directory.txt b/docs/manual/adding-packages-directory.txt
index 1ce9a3b..febb33c 100644
--- a/docs/manual/adding-packages-directory.txt
+++ b/docs/manual/adding-packages-directory.txt
@@ -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.
diff --git a/support/download/check-hash b/support/download/check-hash
index 9c62d7f..0caa619 100755
--- a/support/download/check-hash
+++ b/support/download/check-hash
@@ -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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH 1/5 v2] support/download: make hash file optional
  2015-03-17 12:59 ` [Buildroot] [PATCH 1/5 v2] support/download: make hash file optional Yann E. MORIN
@ 2015-03-19 20:34   ` Arnout Vandecappelle
  2015-03-19 21:03   ` Arnout Vandecappelle
  1 sibling, 0 replies; 14+ messages in thread
From: Arnout Vandecappelle @ 2015-03-19 20:34 UTC (permalink / raw)
  To: buildroot

On 17/03/15 13:59, Yann E. MORIN wrote:
> 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.
> 
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

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

 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

> ---
>  support/download/check-hash | 2 +-
>  support/download/dl-wrapper | 3 ---
>  2 files changed, 1 insertion(+), 4 deletions(-)
[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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH 2/5 v2] package infra: do not check hashes when downloading from a repository
  2015-03-17 12:59 ` [Buildroot] [PATCH 2/5 v2] package infra: do not check hashes when downloading from a repository Yann E. MORIN
@ 2015-03-19 20:36   ` Arnout Vandecappelle
  0 siblings, 0 replies; 14+ messages in thread
From: Arnout Vandecappelle @ 2015-03-19 20:36 UTC (permalink / raw)
  To: buildroot

On 17/03/15 13:59, Yann E. MORIN wrote:
> 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.
> 
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

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


 Regards,
 Arnout

> ---
>  package/pkg-download.mk | 5 -----
>  1 file changed, 5 deletions(-)
[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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH 3/5 v2] support/download: return different exit codes for different failures
  2015-03-17 12:59 ` [Buildroot] [PATCH 3/5 v2] support/download: return different exit codes for different failures Yann E. MORIN
@ 2015-03-19 20:44   ` Arnout Vandecappelle
  0 siblings, 0 replies; 14+ messages in thread
From: Arnout Vandecappelle @ 2015-03-19 20:44 UTC (permalink / raw)
  To: buildroot

On 17/03/15 13:59, Yann E. MORIN wrote:
> 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
> 
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Cc: Samuel Martin <s.martin49@gmail.com>
> 
> ---
> Changes v1 -> v2:
>   - typoes in script and commit  (Samuel)
> ---
>  support/download/check-hash | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/support/download/check-hash b/support/download/check-hash
> index cee64ef..9c62d7f 100755
> --- a/support/download/check-hash
> +++ b/support/download/check-hash
> @@ -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

 This is not entirely true, it also exits with 1 when:

- an invalid option is given;
- the hash file contains an unknown hash type.

 Perhaps there's something to be said for the latter to also return 2, because
it indicates that there's something wrong with the hash file and that's exactly
what we want to catch with this series.


 Regards,
 Arnout


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


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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH 4/5 v2] support/download: properly catch missing hashes
  2015-03-17 12:59 ` [Buildroot] [PATCH 4/5 v2] support/download: properly catch missing hashes Yann E. MORIN
@ 2015-03-19 20:45   ` Arnout Vandecappelle
  0 siblings, 0 replies; 14+ messages in thread
From: Arnout Vandecappelle @ 2015-03-19 20:45 UTC (permalink / raw)
  To: buildroot

On 17/03/15 13:59, Yann E. MORIN wrote:
> 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.
> 
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

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


 Regards,
 Arnout

> 
> ---
> Changes v1 -> v2:
>   - typo in commit
> ---
>  support/download/dl-wrapper | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
[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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH 5/5 v2] support/download: always fail when there's no hash
  2015-03-17 12:59 ` [Buildroot] [PATCH 5/5 v2] support/download: always fail when there's no hash Yann E. MORIN
@ 2015-03-19 20:51   ` Arnout Vandecappelle
  0 siblings, 0 replies; 14+ messages in thread
From: Arnout Vandecappelle @ 2015-03-19 20:51 UTC (permalink / raw)
  To: buildroot

On 17/03/15 13:59, Yann E. MORIN wrote:
> 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...
> 
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Reviewed-by: Samuel Martin <s.martin49@gmail.com>
> ---
>  docs/manual/adding-packages-directory.txt | 6 ++----
>  support/download/check-hash               | 8 ++------
>  2 files changed, 4 insertions(+), 10 deletions(-)
> 
> diff --git a/docs/manual/adding-packages-directory.txt b/docs/manual/adding-packages-directory.txt
> index 1ce9a3b..febb33c 100644
> --- a/docs/manual/adding-packages-directory.txt
> +++ b/docs/manual/adding-packages-directory.txt
> @@ -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).

 This should be updated to mention the VCS downloads. Also it would be good to
explain why it behaves like this. E.g.:

If the +.hash+ file is present, but it does not contain a hash for a
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 OK.

Sources that are downloaded from a version control system (git, subversion,
...) can not have a hash, because the version control system and tar do not
create exactly the same file, so the hash could be wrong even for a valid
download. Therefore, the hash check is skipped for such sources.


 Regards,
 Arnout


>  
>  If the +.hash+ file is missing, then no check is done at all.
> diff --git a/support/download/check-hash b/support/download/check-hash
> index 9c62d7f..0caa619 100755
> --- a/support/download/check-hash
> +++ b/support/download/check-hash
> @@ -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
> 


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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH 1/5 v2] support/download: make hash file optional
  2015-03-17 12:59 ` [Buildroot] [PATCH 1/5 v2] support/download: make hash file optional Yann E. MORIN
  2015-03-19 20:34   ` Arnout Vandecappelle
@ 2015-03-19 21:03   ` Arnout Vandecappelle
  2015-03-21 17:00     ` Yann E. MORIN
  1 sibling, 1 reply; 14+ messages in thread
From: Arnout Vandecappelle @ 2015-03-19 21:03 UTC (permalink / raw)
  To: buildroot

On 17/03/15 13:59, Yann E. MORIN wrote:
> 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.

 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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH 1/5 v2] support/download: make hash file optional
  2015-03-19 21:03   ` Arnout Vandecappelle
@ 2015-03-21 17:00     ` Yann E. MORIN
  2015-03-21 17:28       ` Arnout Vandecappelle
  0 siblings, 1 reply; 14+ messages in thread
From: Yann E. MORIN @ 2015-03-21 17:00 UTC (permalink / raw)
  To: buildroot

Arnout, All,

On 2015-03-19 22:03 +0100, Arnout Vandecappelle spake thusly:
> On 17/03/15 13:59, Yann E. MORIN wrote:
> > 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.
> 
>  An alternative approach would be to allow an empty hash in the hash file, e.g.

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

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

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.

>  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

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [Buildroot] [PATCH 1/5 v2] support/download: make hash file optional
  2015-03-21 17:00     ` Yann E. MORIN
@ 2015-03-21 17:28       ` Arnout Vandecappelle
  0 siblings, 0 replies; 14+ messages in thread
From: Arnout Vandecappelle @ 2015-03-21 17:28 UTC (permalink / raw)
  To: buildroot

On 21/03/15 18:00, Yann E. MORIN wrote:
[snip]
> But for git/hg/svn/bzr/cvs
> clones/checkouts/... there is intrisically no reason to have a hash, by
> design.

 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

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2015-03-21 17:28 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-03-17 12:59 [Buildroot] [PATCH 0/5 v2] support/download: be more aggressive on missing hashes (branch yem/dl-hash) Yann E. MORIN
2015-03-17 12:59 ` [Buildroot] [PATCH 1/5 v2] support/download: make hash file optional Yann E. MORIN
2015-03-19 20:34   ` Arnout Vandecappelle
2015-03-19 21:03   ` Arnout Vandecappelle
2015-03-21 17:00     ` Yann E. MORIN
2015-03-21 17:28       ` Arnout Vandecappelle
2015-03-17 12:59 ` [Buildroot] [PATCH 2/5 v2] package infra: do not check hashes when downloading from a repository Yann E. MORIN
2015-03-19 20:36   ` Arnout Vandecappelle
2015-03-17 12:59 ` [Buildroot] [PATCH 3/5 v2] support/download: return different exit codes for different failures Yann E. MORIN
2015-03-19 20:44   ` Arnout Vandecappelle
2015-03-17 12:59 ` [Buildroot] [PATCH 4/5 v2] support/download: properly catch missing hashes Yann E. MORIN
2015-03-19 20:45   ` Arnout Vandecappelle
2015-03-17 12:59 ` [Buildroot] [PATCH 5/5 v2] support/download: always fail when there's no hash Yann E. MORIN
2015-03-19 20:51   ` Arnout Vandecappelle

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox