[Bug] Re3gression in setup handling of SHA512 checksum failures

[Bug] Re3gression in setup handling of SHA512 checksum failures

[Achim Gratz]

I've found out the hard way that a dodgy package file no longer gets
ignored by setup and is installed without any error unless you care to
look in setup.log.full.  That was hard to figure out because in my case
some stupid error on mirroring just made each file miss a few bytes off
the end, so it was always the last file in the archive that went missing
on unpacking.  I think that's evil and needs to be fixed promptly, but I
have no time to do it myself right now.  I honestly don't remember if
the old setup did the uninstall part before not installing the no-good
package (it probably ddin't), but I suggest it shouldn't uninstall
anything that we don't have a good replacement for.  Also, the messages
that go to the console about SHA512 failures need to be reinstated (a
good check can keep quiet).

WIth libsolve it seems that we will have to calculate a new solution
once we have to drop a package from the solution due to SHA512 mismatch,
then check any new packages in that revised solution again.  So it seems
we need to keep state around whether the package archive was already
checked (lest we check everything again each time).


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

SD adaptation for Waldorf microQ V2.22R2:
http://Synth.Stromeko.net/Downloads.html#WaldorfSDada

Re: [Bug] Re3gression in setup handling of SHA512 checksum failures

[Ken Brown]

On 3/20/2018 2:23 PM, Achim Gratz wrote:
> 
> I've found out the hard way that a dodgy package file no longer gets
> ignored by setup and is installed without any error unless you care to
> look in setup.log.full.  That was hard to figure out because in my case
> some stupid error on mirroring just made each file miss a few bytes off
> the end, so it was always the last file in the archive that went missing
> on unpacking.  I think that's evil and needs to be fixed promptly, but I
> have no time to do it myself right now.  I honestly don't remember if
> the old setup did the uninstall part before not installing the no-good
> package (it probably ddin't), but I suggest it shouldn't uninstall
> anything that we don't have a good replacement for.  Also, the messages
> that go to the console about SHA512 failures need to be reinstated (a
> good check can keep quiet).
> 
> WIth libsolve it seems that we will have to calculate a new solution
> once we have to drop a package from the solution due to SHA512 mismatch,
> then check any new packages in that revised solution again.  So it seems
> we need to keep state around whether the package archive was already
> checked (lest we check everything again each time).

It looks like there are two things going on here.  First, there must 
have been a glitch when the topic/libsolv branch was merged into master. 
  An inadvertent result of that glitch is that the response to the query 
'yesno (owner, IDS_SKIP_PACKAGE, e->what())' in do_install_thread() now 
gets discarded.  Second, as you said, we do have to avoid processing an 
'erase' transaction that's paired with an 'install' transaction that 
gets dropped.

I'll look into both of these issues, unless Jon beats me to it.

By the way, this only affects local installs.  For network installs, the 
hash gets checked at an earlier stage.

Ken

Re: [Bug] Re3gression in setup handling of SHA512 checksum failures

[Achim Gratz]

Ken Brown writes:
> I'll look into both of these issues, unless Jon beats me to it.

Thanks.

> By the way, this only affects local installs.  For network installs,
> the hash gets checked at an earlier stage.

That's correct.  I forgot to mention that, but all my installs are from
a local mirror (necessary due to the way network access is restricted at
my workplace).


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Factory and User Sound Singles for Waldorf Blofeld:
http://Synth.Stromeko.net/Downloads.html#WaldorfSounds

Re: [Bug] Re3gression in setup handling of SHA512 checksum failures

[Ken Brown]

[-- Attachment #1: Type: text/plain, Size: 673 bytes --]

On 3/20/2018 4:11 PM, Achim Gratz wrote:
> Ken Brown writes:
>> I'll look into both of these issues, unless Jon beats me to it.
> 
> Thanks.
> 
>> By the way, this only affects local installs.  For network installs,
>> the hash gets checked at an earlier stage.
> 
> That's correct.  I forgot to mention that, but all my installs are from
> a local mirror (necessary due to the way network access is restricted at
> my workplace)

I haven't been able to come up with a safe way to recover from a 
checksum error at this point, at least not without a lot of work.  I 
propose that we just bail out with an appropriate error message in this 
situation.

Patch attached.

Ken

[-- Attachment #2: 0001-Give-a-fatal-error-on-a-checksum-failure-during-inst.patch --]
[-- Type: text/plain, Size: 1238 bytes --]

From 0607cb5da1bbe61cd132499082a62bbbc54c8dfd Mon Sep 17 00:00:00 2001
From: Ken Brown <kbrown@cornell.edu>
Date: Wed, 21 Mar 2018 14:03:00 -0400
Subject: [PATCH setup] Give a fatal error on a checksum failure during install

This only affects local installs, where the hash of an archive is not
checked until we reach do_install_thread().  At this point it seems
too late to recover safely.
---
 install.cc | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/install.cc b/install.cc
index 37dea6f..b5adcc6 100644
--- a/install.cc
+++ b/install.cc
@@ -850,7 +850,15 @@ do_install_thread (HINSTANCE h, HWND owner)
       }
       catch (Exception *e)
       {
-        yesno (owner, IDS_SKIP_PACKAGE, e->what());
+	// We used to give the user a yes/no option to skip this
+	// package (with "no" meaning install it even though the
+	// archive is corrupt), but both options could damage the
+	// user's system.  In the absence of a safe way to recover, we
+	// just bail out.
+	if (e->errNo() == APPERR_CORRUPT_PACKAGE)
+	  fatal (owner, IDS_CORRUPT_PACKAGE, version.Name().c_str());
+	// Unexpected exception.
+	throw e;
       }
       {
         md5sum_total_bytes_sofar += version.source()->size;
-- 
2.16.2

Re: [Bug] Re3gression in setup handling of SHA512 checksum failures

[Brian Inglis]

On 2018-03-21 12:14, Ken Brown wrote:
> On 3/20/2018 4:11 PM, Achim Gratz wrote:
>> Ken Brown writes:
>>> I'll look into both of these issues, unless Jon beats me to it.
>>
>> Thanks.
>>
>>> By the way, this only affects local installs.  For network installs,
>>> the hash gets checked at an earlier stage.
>>
>> That's correct.  I forgot to mention that, but all my installs are from
>> a local mirror (necessary due to the way network access is restricted at
>> my workplace)
> 
> I haven't been able to come up with a safe way to recover from a checksum error
> at this point, at least not without a lot of work.  I propose that we just bail
> out with an appropriate error message in this situation.
> 
> Patch attached.

Skipping a single package install is likely to be /relatively/ safe, but if this
patch causes setup to exit sometime after upgrading a bunch of packages but
before upgrading another bunch of packages, it could leave Cygwin unusable,
especially if there are upgrade dependencies between the packages installed
prior and not installed after the problematic download.

It would be better in such cases to check all the hashes before proceeding with
any of the installs, or at least all packages in a dependency chain, before
installing any package in that dependency chain.

I don't believe there is currently a way in Cygwin setup for a user to easily
determine and rollback all packages in a dependency chain after such a failure,
or otherwise have Cygwin setup restore the packages to a consistent state.

-- 
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada

Re: [Bug] Re3gression in setup handling of SHA512 checksum failures

[Ken Brown]

On 3/21/2018 8:35 PM, Brian Inglis wrote:
> On 2018-03-21 12:14, Ken Brown wrote:
>> On 3/20/2018 4:11 PM, Achim Gratz wrote:
>>> Ken Brown writes:
>>>> I'll look into both of these issues, unless Jon beats me to it.
>>>
>>> Thanks.
>>>
>>>> By the way, this only affects local installs.  For network installs,
>>>> the hash gets checked at an earlier stage.
>>>
>>> That's correct.  I forgot to mention that, but all my installs are from
>>> a local mirror (necessary due to the way network access is restricted at
>>> my workplace)
>>
>> I haven't been able to come up with a safe way to recover from a checksum error
>> at this point, at least not without a lot of work.  I propose that we just bail
>> out with an appropriate error message in this situation.
>>
>> Patch attached.
> 
> Skipping a single package install is likely to be /relatively/ safe, but if this
> patch causes setup to exit sometime after upgrading a bunch of packages but
> before upgrading another bunch of packages, it could leave Cygwin unusable,
> especially if there are upgrade dependencies between the packages installed
> prior and not installed after the problematic download.
> 
> It would be better in such cases to check all the hashes before proceeding with
> any of the installs

We already do that.  My patch deals with the situation where we find a 
corrupt file during this process.

Ken

Re: [Bug] Re3gression in setup handling of SHA512 checksum failures

[Brian Inglis]

On 2018-03-22 08:41, Ken Brown wrote:
> On 3/21/2018 8:35 PM, Brian Inglis wrote:
>> On 2018-03-21 12:14, Ken Brown wrote:
>>> On 3/20/2018 4:11 PM, Achim Gratz wrote:
>>>> Ken Brown writes:
>>>>> I'll look into both of these issues, unless Jon beats me to it.
>>>>
>>>> Thanks.
>>>>
>>>>> By the way, this only affects local installs.  For network installs,
>>>>> the hash gets checked at an earlier stage.
>>>>
>>>> That's correct.  I forgot to mention that, but all my installs are from
>>>> a local mirror (necessary due to the way network access is restricted at
>>>> my workplace)
>>>
>>> I haven't been able to come up with a safe way to recover from a checksum error
>>> at this point, at least not without a lot of work.  I propose that we just bail
>>> out with an appropriate error message in this situation.
>>>
>>> Patch attached.
>>
>> Skipping a single package install is likely to be /relatively/ safe, but if this
>> patch causes setup to exit sometime after upgrading a bunch of packages but
>> before upgrading another bunch of packages, it could leave Cygwin unusable,
>> especially if there are upgrade dependencies between the packages installed
>> prior and not installed after the problematic download.
>>
>> It would be better in such cases to check all the hashes before proceeding with
>> any of the installs
> 
> We already do that.  My patch deals with the situation where we find a corrupt
> file during this process.

Sorry, it sounded like the check was happening during the install action,
instead of before. The process has changed now the solver is being used.

-- 
Take care. Thanks, Brian Inglis, Calgary, Alberta, Canada

Re: [Bug] Re3gression in setup handling of SHA512 checksum failures

[Jon Turney]

On 21/03/2018 18:14, Ken Brown wrote:
> On 3/20/2018 4:11 PM, Achim Gratz wrote:
>> Ken Brown writes:
>>> I'll look into both of these issues, unless Jon beats me to it.
>>
>> Thanks.
>>
>>> By the way, this only affects local installs.  For network installs,
>>> the hash gets checked at an earlier stage.
>>
>> That's correct.  I forgot to mention that, but all my installs are from
>> a local mirror (necessary due to the way network access is restricted at
>> my workplace)
> 
> I haven't been able to come up with a safe way to recover from a 
> checksum error at this point, at least not without a lot of work.  I 
> propose that we just bail out with an appropriate error message in this 
> situation.
> 
> Patch attached.

Thanks.

Yeah, this seems entirely reasonable.

Even the previous behaviour is wrong in unattended mode, i.e. 'setup -q 
-P foo -L' should definitely stop with an error if the package for foo 
is corrupt, rather than silently doing our best.