Re: [LEDE-DEV] [PATCH] scripts/download.pl: fail loudly if provided hash is unsupported

2017-10-16 Thread Baptiste Jonglez
On 08-10-17, Stijn Tintel wrote:
> On 25-09-17 18:43, Stijn Tintel wrote:
> > On 25-09-17 00:20, Baptiste Jonglez wrote:
> >> On 24-09-17, Stijn Tintel wrote:
> >>> My bad. When we update a package, we bump the version in the Makefile
> >>> and remove the current hash, then run "make package/foo/dowload; make
> >>> package/foo/check FIXUP=1". This downloads the tarbal for the new
> >>> version, and FIXUP writes its hash to the Makefile. This is what's
> >>> broken since the change.
> >> Ok, I see the issue now.
> >>
> >> As mentioned in the commit message, we definitely should add an explicit
> >> option to download.pl to skip hash verification, and then implement
> >> something like this:
> >>
> >> $ make package/foo/download SKIPHASH=1
> >>
> >> What do you think?
> > We were thinking in that direction on #lede-dev, not sure why we
> > couldn't agree. Let's wait a bit if anybody else chimes in?

> Maybe just send an RFC patch with the suggested change, as nobodoy seems
> to be responding.

Yes, sorry for the delay, I will submit a patch in the coming days/weeks.

Baptiste


signature.asc
Description: PGP signature
___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev


Re: [LEDE-DEV] [PATCH] scripts/download.pl: fail loudly if provided hash is unsupported

2017-10-08 Thread Stijn Tintel
On 25-09-17 18:43, Stijn Tintel wrote:
> On 25-09-17 00:20, Baptiste Jonglez wrote:
>> On 24-09-17, Stijn Tintel wrote:
>>> My bad. When we update a package, we bump the version in the Makefile
>>> and remove the current hash, then run "make package/foo/dowload; make
>>> package/foo/check FIXUP=1". This downloads the tarbal for the new
>>> version, and FIXUP writes its hash to the Makefile. This is what's
>>> broken since the change.
>> Ok, I see the issue now.
>>
>> As mentioned in the commit message, we definitely should add an explicit
>> option to download.pl to skip hash verification, and then implement
>> something like this:
>>
>> $ make package/foo/download SKIPHASH=1
>>
>> What do you think?
> We were thinking in that direction on #lede-dev, not sure why we
> couldn't agree. Let's wait a bit if anybody else chimes in?
Maybe just send an RFC patch with the suggested change, as nobodoy seems
to be responding.

Thanks,
Stijn


___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev


Re: [LEDE-DEV] [PATCH] scripts/download.pl: fail loudly if provided hash is unsupported

2017-09-25 Thread Stijn Tintel
On 25-09-17 00:20, Baptiste Jonglez wrote:
> On 24-09-17, Stijn Tintel wrote:
>> My bad. When we update a package, we bump the version in the Makefile
>> and remove the current hash, then run "make package/foo/dowload; make
>> package/foo/check FIXUP=1". This downloads the tarbal for the new
>> version, and FIXUP writes its hash to the Makefile. This is what's
>> broken since the change.
> Ok, I see the issue now.
>
> As mentioned in the commit message, we definitely should add an explicit
> option to download.pl to skip hash verification, and then implement
> something like this:
>
> $ make package/foo/download SKIPHASH=1
>
> What do you think?

We were thinking in that direction on #lede-dev, not sure why we
couldn't agree. Let's wait a bit if anybody else chimes in?

Stijn


___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev


Re: [LEDE-DEV] [PATCH] scripts/download.pl: fail loudly if provided hash is unsupported

2017-09-24 Thread Baptiste Jonglez
On 24-09-17, Stijn Tintel wrote:
> My bad. When we update a package, we bump the version in the Makefile
> and remove the current hash, then run "make package/foo/dowload; make
> package/foo/check FIXUP=1". This downloads the tarbal for the new
> version, and FIXUP writes its hash to the Makefile. This is what's
> broken since the change.

Ok, I see the issue now.

As mentioned in the commit message, we definitely should add an explicit
option to download.pl to skip hash verification, and then implement
something like this:

$ make package/foo/download SKIPHASH=1

What do you think?


signature.asc
Description: PGP signature
___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev


Re: [LEDE-DEV] [PATCH] scripts/download.pl: fail loudly if provided hash is unsupported

2017-09-24 Thread Stijn Tintel
On 24-09-17 23:57, Baptiste Jonglez wrote:
> Hi,
>
> On 24-09-17, Stijn Tintel wrote:
>> On 03-09-17 15:01, Baptiste Jonglez wrote:
>>> Note: if some users of scripts/download.pl knowingly provide an empty hash
>>> because they don't need checksum verification, this change will break
>>> them.  This does not seem to be the case currently, but if this feature is
>>> ever needed, an option should be added to download.pl instead of relying
>>> on the hash being empty.
>> Unfortunately this change breaks the make/foo/download feature,
> Can you elaborate on this?  It seems to work fine here:
>
> $ make package/dnsmasq/download V=s
> make[1]: Entering directory '/lede/tmp/lede-project'
> make[2]: Entering directory 
> '/lede/tmp/lede-project/package/network/services/dnsmasq'
> mkdir -p /lede/tmp/lede-project/dl
> SHELL= flock /lede/tmp/lede-project/tmp/.dnsmasq-2.77.tar.xz.flock -c '   
> /lede/tmp/lede-project/scripts/download.pl "/lede/tmp/lede-project/dl" 
> "dnsmasq-2.77.tar.xz" 
> "6eac3b1c50ae25170e3ff8c96ddb55236cf45007633fdb8a35b1f3e02f5f8b8a" "" 
> "http://thekelleys.org.uk/dnsmasq/;'
> + curl -f --connect-timeout 20 --retry 5 --location --insecure 
> http://thekelleys.org.uk/dnsmasq/dnsmasq-2.77.tar.xz
>   % Total% Received % Xferd  Average Speed   TimeTime Time  
> Current
>  Dload  Upload   Total   SpentLeft  
> Speed
> 100  475k  100  475k0 0   237k  0  0:00:02  0:00:02 --:--:--  
> 232k
> make[2]: Leaving directory 
> '/lede/tmp/lede-project/package/network/services/dnsmasq'
> make[1]: Leaving directory '/lede/tmp/lede-project'
My bad. When we update a package, we bump the version in the Makefile
and remove the current hash, then run "make package/foo/dowload; make
package/foo/check FIXUP=1". This downloads the tarbal for the new
version, and FIXUP writes its hash to the Makefile. This is what's
broken since the change.

Stijn


___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev


Re: [LEDE-DEV] [PATCH] scripts/download.pl: fail loudly if provided hash is unsupported

2017-09-24 Thread Baptiste Jonglez
Hi,

On 24-09-17, Stijn Tintel wrote:
> On 03-09-17 15:01, Baptiste Jonglez wrote:
> > Note: if some users of scripts/download.pl knowingly provide an empty hash
> > because they don't need checksum verification, this change will break
> > them.  This does not seem to be the case currently, but if this feature is
> > ever needed, an option should be added to download.pl instead of relying
> > on the hash being empty.
>
> Unfortunately this change breaks the make/foo/download feature,

Can you elaborate on this?  It seems to work fine here:

$ make package/dnsmasq/download V=s
make[1]: Entering directory '/lede/tmp/lede-project'
make[2]: Entering directory 
'/lede/tmp/lede-project/package/network/services/dnsmasq'
mkdir -p /lede/tmp/lede-project/dl
SHELL= flock /lede/tmp/lede-project/tmp/.dnsmasq-2.77.tar.xz.flock -c ' 
/lede/tmp/lede-project/scripts/download.pl "/lede/tmp/lede-project/dl" 
"dnsmasq-2.77.tar.xz" 
"6eac3b1c50ae25170e3ff8c96ddb55236cf45007633fdb8a35b1f3e02f5f8b8a" "" 
"http://thekelleys.org.uk/dnsmasq/;'
+ curl -f --connect-timeout 20 --retry 5 --location --insecure 
http://thekelleys.org.uk/dnsmasq/dnsmasq-2.77.tar.xz
  % Total% Received % Xferd  Average Speed   TimeTime Time  
Current
 Dload  Upload   Total   SpentLeft  
Speed
100  475k  100  475k0 0   237k  0  0:00:02  0:00:02 --:--:--  
232k
make[2]: Leaving directory 
'/lede/tmp/lede-project/package/network/services/dnsmasq'
make[1]: Leaving directory '/lede/tmp/lede-project'

> and because of this also the script we use to update kernel versions and
> refresh patches for all targets. This has been discussed in #lede-dev a
> few times, but we never agreed on a solution. Today, this is biting me
> once again, and therefore I suggest to revert this change until we can
> agree on a solution that is both secure and doesn't break something some
> of use rather frequently.


signature.asc
Description: PGP signature
___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev


Re: [LEDE-DEV] [PATCH] scripts/download.pl: fail loudly if provided hash is unsupported

2017-09-24 Thread Stijn Tintel
On 03-09-17 15:01, Baptiste Jonglez wrote:
> From: Baptiste Jonglez 
>
> Currently, if the provided hash is unsupported (length different from 32
> or 64 bytes), we happily download the requested file without any kind of
> checksum verification.
>
> This is quite dangerous and may provide a false sense of security, because
> a single typo in the hash (e.g. one character deleted by mistake) may skip
> checksum verification entirely.
>
> Instead, fail immediately if we don't support the provided hash.
> In particular, if an external package repository decides to change the
> hash algorithm one day, we will now fail loudly instead of skipping
> checksum verification without complaints.
>
> Note: if some users of scripts/download.pl knowingly provide an empty hash
> because they don't need checksum verification, this change will break
> them.  This does not seem to be the case currently, but if this feature is
> ever needed, an option should be added to download.pl instead of relying
> on the hash being empty.
Unfortunately this change breaks the make/foo/download feature, and
because of this also the script we use to update kernel versions and
refresh patches for all targets. This has been discussed in #lede-dev a
few times, but we never agreed on a solution. Today, this is biting me
once again, and therefore I suggest to revert this change until we can
agree on a solution that is both secure and doesn't break something some
of use rather frequently.

Stijn

___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev


Re: [LEDE-DEV] [PATCH] scripts/download.pl: fail loudly if provided hash is unsupported

2017-09-14 Thread Baptiste Jonglez
Thanks for merging, can this be merged to lede-17.01 as well?

On 03-09-17, Baptiste Jonglez wrote:
> Currently, if the provided hash is unsupported (length different from 32
> or 64 bytes), we happily download the requested file without any kind of
> checksum verification.
> 
> This is quite dangerous and may provide a false sense of security, because
> a single typo in the hash (e.g. one character deleted by mistake) may skip
> checksum verification entirely.
> 
> Instead, fail immediately if we don't support the provided hash.
> In particular, if an external package repository decides to change the
> hash algorithm one day, we will now fail loudly instead of skipping
> checksum verification without complaints.
> 
> Note: if some users of scripts/download.pl knowingly provide an empty hash
> because they don't need checksum verification, this change will break
> them.  This does not seem to be the case currently, but if this feature is
> ever needed, an option should be added to download.pl instead of relying
> on the hash being empty.
> 
> Fixes: eaa4eba10a89 ("scripts/download.pl: add SHA-256 support")
> 
> Signed-off-by: Baptiste Jonglez 
> ---
>  scripts/download.pl | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/scripts/download.pl b/scripts/download.pl
> index bf9fe8c761..775408934a 100755
> --- a/scripts/download.pl
> +++ b/scripts/download.pl
> @@ -88,6 +88,7 @@ sub download_cmd($) {
>  }
>  
>  my $hash_cmd = hash_cmd();
> +$hash_cmd or die "Cannot find appropriate hash command, ensure the provided 
> hash is either a MD5 or SHA256 checksum.\n";
>  
>  sub download
>  {


signature.asc
Description: PGP signature
___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev


[LEDE-DEV] [PATCH] scripts/download.pl: fail loudly if provided hash is unsupported

2017-09-03 Thread Baptiste Jonglez
From: Baptiste Jonglez 

Currently, if the provided hash is unsupported (length different from 32
or 64 bytes), we happily download the requested file without any kind of
checksum verification.

This is quite dangerous and may provide a false sense of security, because
a single typo in the hash (e.g. one character deleted by mistake) may skip
checksum verification entirely.

Instead, fail immediately if we don't support the provided hash.
In particular, if an external package repository decides to change the
hash algorithm one day, we will now fail loudly instead of skipping
checksum verification without complaints.

Note: if some users of scripts/download.pl knowingly provide an empty hash
because they don't need checksum verification, this change will break
them.  This does not seem to be the case currently, but if this feature is
ever needed, an option should be added to download.pl instead of relying
on the hash being empty.

Fixes: eaa4eba10a89 ("scripts/download.pl: add SHA-256 support")

Signed-off-by: Baptiste Jonglez 
---
 scripts/download.pl | 1 +
 1 file changed, 1 insertion(+)

diff --git a/scripts/download.pl b/scripts/download.pl
index bf9fe8c761..775408934a 100755
--- a/scripts/download.pl
+++ b/scripts/download.pl
@@ -88,6 +88,7 @@ sub download_cmd($) {
 }
 
 my $hash_cmd = hash_cmd();
+$hash_cmd or die "Cannot find appropriate hash command, ensure the provided 
hash is either a MD5 or SHA256 checksum.\n";
 
 sub download
 {
-- 
2.14.1


___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev