Implement ISO hash check #41

SibrenVasse · 2019-01-31T15:15:45Z

As discussed in #16.

I moved assertISOIsOK behind checkSudo because this meant you got double output. Does this have any other implications I missed?

resolves #39

jsamr

@SibrenVasse Thanks again Sibren!

See my comments.

Can you also provide me a summary of the manual tests you did ?

You can write something in the form:

Test 1

Preconditions :

file.iso exists
file.iso.md5 exists
file.iso is valid (...)

Command : bootiso --option file.iso

Behavior: (...text description)

EDIT You can of course have a "foreach" logic if you did the same test for each extension type :-)

bootiso

SibrenVasse · 2019-01-31T19:43:48Z

Alright, so here goes:

Valid *.sha256 file

./bootiso -i test/debian-9.7.0-amd64-netinst.iso
bootiso: Correct sha256 hash found in 'test/debian-9.7.0-amd64-netinst.iso.sha256'

*.sha256 file with incorrect hash (but valid sha256 hash)

./bootiso -i test/debian-9.7.0-amd64-netinst.iso
bootiso: Incorrect hash found found in 'test/debian-9.7.0-amd64-netinst.iso.sha256'
Do you still want to continue? (y/n)> n
bootiso: Exiting...

*.sha256 not containing a hash for filename

./bootiso -i test/debian-9.7.0-amd64-netinst.iso
bootiso: No matching filename found in hash file 'test/debian-9.7.0-amd64-netinst.iso.sha256'

*.sha256 with different hash (BLAKE2 as a test)

Hash is assumed to be SHA256 even if invalid length.

./bootiso -i test/debian-9.7.0-amd64-netinst.iso
bootiso: Incorrect hash found found in 'test/debian-9.7.0-amd64-netinst.iso.sha256'
Do you still want to continue? (y/n)> n
bootiso: Exiting...

SHA1SUMS* file with valid hash

./bootiso -i test/debian-9.7.0-amd64-netinst.iso
bootiso: Correct sha1 hash found in 'test/SHA1SUMS.txt'

SHA1SUMS* file with incorrect hash (but valid sha1 hash)

./bootiso -i test/debian-9.7.0-amd64-netinst.iso
bootiso: Incorrect hash found found in 'test/SHA1SUMS.txt'
Do you still want to continue? (y/n)> n
bootiso: Exiting...

SHA1SUMS* file with different hash (BLAKE2 as a test)

Hash is assumed to be SHA1 even if invalid length.

./bootiso -i test/debian-9.7.0-amd64-netinst.iso
bootiso: Incorrect hash found found in 'test/SHA1SUMS.txt'
Do you still want to continue? (y/n)> n
bootiso: Exiting...

SHA1SUMS* not containing a hash for filename

./bootiso -i test/debian-9.7.0-amd64-netinst.iso
bootiso: No matching filename found in hash file 'test/SHA1SUMS.txt'

No hash files

./bootiso -i test/debian-9.7.0-amd64-netinst.iso
bootiso: Reporting 'test/debian-9.7.0-amd64-netinst.iso' boot capabilities:

No hash files with `--force-hash-check`

./bootiso -i test/debian-9.7.0-amd64-netinst.iso --force-hash-check
bootiso: No valid hashes found. Assert forced by '--force-hash-check'
Exiting...

No hash files with `--no-hash-check`

./bootiso -i test/debian-9.7.0-amd64-netinst.iso --no-hash-check
bootiso: Reporting 'test/debian-9.7.0-amd64-netinst.iso' boot capabilities:

Custom hash file specified with `--hash-file`

./bootiso -i test/debian-9.7.0-amd64-netinst.iso --hash-file test/file-with-hash
Correct sha256 hash found in 'test/file-with-hash'

Custom hash file with incorrect hash

./bootiso -i test/debian-9.7.0-amd64-netinst.iso --hash-file test/file-with-hash
bootiso: Incorrect hash found found in 'test/file-with-hash'
Do you still want to continue? (y/n)> n
bootiso: Exiting...

Custom hash file with unknown hash

Hash is guessed based on length because there is no way to distinguish them otherwise. A different hash of the same length will return as 'incorrect'. A different hash of different length will return as 'unknown hash type'.

./bootiso -i test/debian-9.7.0-amd64-netinst.iso --hash-file test/file-with-hash
bootiso: Unknown hash type found in 'test/file-with-hash'
Do you still want to continue? (y/n)> n
bootiso: Exiting...

jsamr · 2019-01-31T20:15:34Z

@SibrenVasse Great report!

I realised while reading the output : I find the wording "incorrect" a bit too broad. Is the hash file corrupted, the hash of unexpected length or the ISO hash doesn't match the yet valid file hash ?

So I would use the word "match" when the two match, "mismatch" when they don't, and something like "unexpected hash file format" when the hash file isn't following the expected format (no filename, unexpected hash length).

SibrenVasse · 2019-01-31T21:34:46Z

I've changed the strings. Do they seem alright?

SibrenVasse · 2019-01-31T21:37:30Z

bootiso

+		typeset hashName=$3 # Name of command of hash
+
+		# Hash from hash file
+		typeset gHash=$(awk -v pattern="$isoName$" '$0 ~ pattern { print $1; exit }' "$hashPath")


I think isoName should be escaped here somehow, it could contain special regex characters (it's user input). Searching for a filename with a '$' fails to find the filename in the hash file.
I'd also like to match more specific '^\w{1,}\s{2}$isoName$', but no idea how to do that with awk. Any ideas?

This is indeed fragile and error-prone. I suggest the following changes:

Delegate this job to high-profile tools

All the *sum utilities have a --check option ; I think it will be much easier to use them and read the return code with $?. Or just:

if sha256sum --check --strict --status -- "$hashPath" |& indentAll; then # Test OK else # Test fail # I couldn't find which non-zero exit code correspond to a mismatch or ill-formated file # So I checked experimentally: those utilities return 1 in all scenarios, unfortunately. # You can put any hashfile analysis here, and print an error message which gives user # feedback on either a mismatch was encountered, or an ill-formatted hash file was found fi

From info:

The usage and options of these commands are precisely the same as for md5sum and sha1sum. See md5sum invocation.

So we can safely rely on md5sum API for all *sum utilities.

Note the -- which will prevent any misinterpretation of $hashPath.
--strict : "exit non-zero for improperly formatted checksum lines"

Also, from info:

The sums are computed as described in FIPS-180-2. When checking, the input should be a former output of this program. The default mode is to print a line with checksum, a space, a character indicating input mode ('*' for binary, ' ' for text or where binary is insignificant), and name for each FILE.

Reverse the logic of your function

Instead of relying on a pattern match, extract the first word from the hash file to infer the proper hash algorithm and checksum utility to use. That will do the job:

gHash=$(head -- "$hashPath" | cut -d' ' -f1)

jsamr

@SibrenVasse Hi there!
See my comments :-)

jsamr · 2019-02-01T08:24:11Z

bootiso

+		typeset hashName=$3 # Name of command of hash
+
+		# Hash from hash file
+		typeset gHash=$(awk -v pattern="$isoName$" '$0 ~ pattern { print $1; exit }' "$hashPath")


This is indeed fragile and error-prone. I suggest the following changes:

Delegate this job to high-profile tools

All the *sum utilities have a --check option ; I think it will be much easier to use them and read the return code with $?. Or just:

if sha256sum --check --strict --status -- "$hashPath" |& indentAll; then # Test OK else # Test fail # I couldn't find which non-zero exit code correspond to a mismatch or ill-formated file # So I checked experimentally: those utilities return 1 in all scenarios, unfortunately. # You can put any hashfile analysis here, and print an error message which gives user # feedback on either a mismatch was encountered, or an ill-formatted hash file was found fi

From info:

The usage and options of these commands are precisely the same as for md5sum and sha1sum. See md5sum invocation.

So we can safely rely on md5sum API for all *sum utilities.

Note the -- which will prevent any misinterpretation of $hashPath.
--strict : "exit non-zero for improperly formatted checksum lines"

Also, from info:

The sums are computed as described in FIPS-180-2. When checking, the input should be a former output of this program. The default mode is to print a line with checksum, a space, a character indicating input mode ('*' for binary, ' ' for text or where binary is insignificant), and name for each FILE.

Reverse the logic of your function

Instead of relying on a pattern match, extract the first word from the hash file to infer the proper hash algorithm and checksum utility to use. That will do the job:

gHash=$(head -- "$hashPath" | cut -d' ' -f1)

bootiso

SibrenVasse · 2019-02-01T10:35:22Z

Delegate this job to high-profile tools

The disadvantage of using the *sum utilities is that it checks all hashes in the file. This could be a long process, and will also fail if (especially without --ignore-missing):

one of the other files in the hash file are not present
one of the other files in the hash file has an invalid hash

These cases shouldn't fail if the hash for the current file is correct.

So maybe something like this to counter this?

grep -F "$isoName" "$hashFile" | sha256sum --check --strict --status

Also, *sum utilities can't check hashfiles in a different directory as the current one. So this won't work:

./bootiso -i test/debian.iso

Resolves #16

jsamr · 2019-02-01T12:16:40Z

So maybe something like this to counter this?

grep -F "$isoName" "$hashFile" | sha256sum --check --strict --status

Yes, that sounds just perfect to circumvent our issues.

Also, *sum utilities can't check hashfiles in a different directory as the current one. So this won't work:

./bootiso -i test/debian.iso

Good point.

SibrenVasse · 2019-02-01T16:26:08Z

Also, *sum utilities can't check hashfiles in a different directory as the current one. So this won't work:

./bootiso -i test/debian.iso

Good point.

So that was the main reason why I chose to go parsing myself. (I looked at using *sum first)
I think it would be rather limiting when using bootiso to only be able to test iso's in the same directory.

@SibrenVasse

This commit is a rework from @SibrenVasse pull request #41 - fix a minor bug where ISO check was run twice, before and after sudo - disable automatic ISO hash check with `--no-hash-check` flag - exit when hash fails with `--force-hash-check` flag - explicitly set a hash file with `--hash-file <file>` flag

jsamr · 2019-04-04T06:57:23Z

@SibrenVasse I reworked this PR and pushed to next 9efdcbf, which has been force-pushed since. I made you the author of the commit, since you did a majority of the work. You'll have to reset --hard your own next branch to this repo's next.

jsamr · 2019-04-04T08:51:28Z

@SibrenVasse See also c234df7

Implement USB thumb drive check

1eec9dc

resolves #39

jsamr requested changes Jan 31, 2019

View reviewed changes

bootiso Outdated Show resolved Hide resolved

bootiso Outdated Show resolved Hide resolved

SibrenVasse force-pushed the hash-check branch 2 times, most recently from 824f63a to 9f12c1a Compare January 31, 2019 19:43

SibrenVasse force-pushed the hash-check branch from 9f12c1a to 3a65449 Compare January 31, 2019 21:33

SibrenVasse commented Jan 31, 2019

View reviewed changes

jsamr requested changes Feb 1, 2019

View reviewed changes

Implement ISO hash check

fff8c07

Resolves #16

SibrenVasse force-pushed the hash-check branch from 3a65449 to fff8c07 Compare February 1, 2019 10:40

jsamr force-pushed the next branch from f8c50f5 to 5d7f463 Compare March 11, 2019 13:29

jsamr force-pushed the next branch from 5d7f463 to 2514063 Compare April 4, 2019 05:37

jsamr closed this Apr 4, 2019

jsamr added a commit that referenced this pull request Apr 4, 2019

Fix merge with PR #41

c234df7

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement ISO hash check #41

Implement ISO hash check #41

SibrenVasse commented Jan 31, 2019

jsamr left a comment •

edited

Loading

SibrenVasse commented Jan 31, 2019

jsamr commented Jan 31, 2019 •

edited

Loading

SibrenVasse commented Jan 31, 2019

SibrenVasse Jan 31, 2019 •

edited

Loading

jsamr Feb 1, 2019

jsamr left a comment

jsamr Feb 1, 2019

SibrenVasse commented Feb 1, 2019 •

edited

Loading

jsamr commented Feb 1, 2019

SibrenVasse commented Feb 1, 2019

jsamr commented Apr 4, 2019

jsamr commented Apr 4, 2019

Implement ISO hash check #41

Implement ISO hash check #41

Conversation

SibrenVasse commented Jan 31, 2019

jsamr left a comment • edited Loading

Choose a reason for hiding this comment

Test 1

SibrenVasse commented Jan 31, 2019

Valid *.sha256 file

*.sha256 file with incorrect hash (but valid sha256 hash)

*.sha256 not containing a hash for filename

*.sha256 with different hash (BLAKE2 as a test)

SHA1SUMS* file with valid hash

SHA1SUMS* file with incorrect hash (but valid sha1 hash)

SHA1SUMS* file with different hash (BLAKE2 as a test)

SHA1SUMS* not containing a hash for filename

No hash files

No hash files with --force-hash-check

No hash files with --no-hash-check

Custom hash file specified with --hash-file

Custom hash file with incorrect hash

Custom hash file with unknown hash

jsamr commented Jan 31, 2019 • edited Loading

SibrenVasse commented Jan 31, 2019

SibrenVasse Jan 31, 2019 • edited Loading

Choose a reason for hiding this comment

jsamr Feb 1, 2019

Choose a reason for hiding this comment

Delegate this job to high-profile tools

Reverse the logic of your function

jsamr left a comment

Choose a reason for hiding this comment

jsamr Feb 1, 2019

Choose a reason for hiding this comment

Delegate this job to high-profile tools

Reverse the logic of your function

SibrenVasse commented Feb 1, 2019 • edited Loading

jsamr commented Feb 1, 2019

SibrenVasse commented Feb 1, 2019

jsamr commented Apr 4, 2019

jsamr commented Apr 4, 2019

jsamr left a comment •

edited

Loading

No hash files with `--force-hash-check`

No hash files with `--no-hash-check`

Custom hash file specified with `--hash-file`

jsamr commented Jan 31, 2019 •

edited

Loading

SibrenVasse Jan 31, 2019 •

edited

Loading

SibrenVasse commented Feb 1, 2019 •

edited

Loading