Re: [LEDE-DEV] [PATCH] scripts/download.pl: fail loudly if provided hash is unsupported
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
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
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
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
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
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
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
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
From: Baptiste JonglezCurrently, 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