Create a collection of checksums

[heddn]

Fixes #4 by externalizing the collection into a class that can be generally useful outside of this project. It reads the special formatted hashed files and provides it as an iterator.

[davidstrauss]

I feel conflicted for this because:

• It's probably good for security to fail loudly (with an exception) and halt further processing unless the code intends further processing in the presence of that failure.
• This approach suggests that calling code will use the exception for flow control. That is, the normal operation (in terms of any code showing reports) of the application will throw and catch the exception. This is generally considered bad form.

So, I'm not sure which concern should dominate.

[davidstrauss]

Perhaps there's an argument for two methods:

• ->getMismatchedFiles(), which always returns an array (hopefully empty).
• ->assertValidFiles(), which calls the former but provides the exception-based approach to mismatches.

[mbaynton]

I recall our choice of returning an int from verifyCsigChecksumFile() and its ilk was made pretty arbitrarily. What about changing this to an array of paths as they appear in the checksum list that failed validation, if we also throw in the event that the checksum list contains 0 files?

[davidstrauss]

What about changing this to an array of paths as they appear in the checksum list that failed validation, if we also throw in the event that the checksum list contains 0 files?

I don't believe we have the opportunity to return anything if we throw an exception, and we would currently throw an exception if the mismatches count more than zero.

[mbaynton]

Sorry, I meant to dispense with throwing exceptions for hash mismatches to fix the control-flow style issue, and consult the resulting array length instead.

I'm also fine with two methods, one of which throws and one of which returns reportable data.

[heddn]

We definitely have a report only mode and a need to fail if validation fails. In the case of failing though, we don't want to WSOD. If we throw an exception, wouldn't we still be using the exception as a signal to stop processing.

[davidstrauss]

We definitely have a report only mode and a need to fail if validation fails. In the case of failing though, we don't want to WSOD. If we throw an exception, wouldn't we still be using the exception as a signal to stop processing.

In the two-method model, we would call ->getMismatchedFiles() when generating a report. In cases where we don't care about reporting, only validation, we would call ->assertValidFiles(). We may still catch the exception in the latter case, but it wouldn't be used for "control flow" in the sense that the exception handling represents a common code path.

[heddn]

I'm a little reluctant to have 2 names for every method. It seems rather long for our public interface. Is there another way to do this? Maybe return a list of failures if the verification doesn't pass?

assertMessage
verifyMessage
assertChecksumList
verifyChecksumList
assertChecksumFile
verifyChecksumFile
assertCsigMessage
verifyCsigMessage
assertCsigChecksumList
verifyCsigChecksumList
assertCsigChecksumFile
verifyCsigChecksumFile

[heddn]

This results in the following test failure. Note it isn't a list of failed files but rather the first failed validation throws an exception.

There was 1 error:

1) DrupalAssociation\Signify\Tests\SignifyTest::testVerifyCsigChecksumListCompromisedFiles
DrupalAssociation\Signify\VerifierException: File "b.txt" does not pass checksum verification.

[pwolanin]

I think we should decline this - it's adding extra complexit beyond the scope of this library

[heddn]

Here's some additional refactoring to create a collection/iterable of hashes.

[davidstrauss]

I'm a little reluctant to have 2 names for every method

Agreed, but some of the listed methods don't share the property of the multi-file check where you can have multiple instances of the same type of failure. For example, it would be unnecessary to have both assertMessage and verifyMessage because the entire message either succeeds verification or fails. The verifyMessage method wouldn't provide more than a Boolean response. So, it seems like we could just have the exception-based version for that one. I'd say the same about assertCsigMessage.

Also, I'm not sure I understand the distinction between a checksum list versus a checksum file.