nvm: support SHA256 instead of SHA1

[DomT4]

Supports iojs and Node SHA256 checksumming instead of SHA1 for Node and nothing for iojs. Should support at least Ubuntu, Debian, Linux Mint, Arch Linux, Gentoo, OpenBSD, FreeBSD, CentOS, OS X and Cygwin.

There's a whole range of fallbacks there, from the regular shasums to BSD & OS X's more centralised one shasum which does everything and then has fallbacks on 4 of the more popular/new/shiny SSL tools in PolarSSL, OpenSSL, LibreSSL and BoringSSL.

I know there were concerns about this sort of thing previously because of some systems lacking SHA256, but the introduction of the SSL Toolkits as backups should mitigate that quite a bit. I tested the OpenSSL branch way back to OpenSSL 0.9.8e and found that to support the method used here. OpenBSD is trying to kill off OpenSSL on their system slowly, so LibreSSL as well as the shasum BSD utility should cover those.

[ljharb]

Thanks for taking a crack at this!

Not every version on http://nodejs.org/dist/ has SHASUMS256.txt - specifically, v0.5.x, v0.6.x, v0.7.x, <= v0.8.22, and v0.9.x. Dropping SHA-1 support entirely, at least in the way this PR does, means nvm would be dropping support for these node versions too, which isn't acceptable.

If you want to change this PR to add nvm_checksum_256 for example, and use that only on the io.js code paths, that would be a great first start - and later it could be conditionally used when nvm_version_greater_than_or_equal_to 0.10.0 for the node paths.

[ljharb]

Ideally this error message will contain all of the options that would work.

[DomT4]

No worries! I actually wrote about 100 lines up last night with a rather complicated fallback mechanism, where it'd pull both the SHASUM.txt files (From Node's Dist) and try SHA256 => success => yay or SHA256 => fail => SHA1 > success > yay but it wasn't reliable enough to submit so I dropped the fallback idea and soldiered ahead with broadening the checksum mechanism itself. I didn't want to submit something that was rather delicate and prone to breakage, because that's always a sucky situation.

Dropping SHA-1 support entirely, at least in the way this PR does, means nvm would be dropping support for these node versions too, which isn't acceptable.

Fair point. I kinda hate dealing with the older versions of Node these days, but the point of your script is to offer up that free choice and I don't want to kneecap that capability.

If you want to change this PR to add nvm_checksum_256 for example, and use that only on the io.js code paths, that would be a great first start

This sounds like a great compromise. I'll work on that tweak now. Thanks! 😺

[DomT4]

Pushed some changes. Let me know how it looks now :).

[ljharb]

Oxford comma? :-D

[DomT4]

Done 😉

[ljharb]

Awesome, this looks great! I'll play around with it on a couple systems and convince myself it will work - in the meantime, could you add unit tests for nvm_checksum_256, similar to the ones for nvm_checksum (https://github.com/creationix/nvm/blob/master/test/fast/Unit%20tests/nvm_checksum) ?

[DomT4]

I'll play around with it on a couple systems and convince myself it will work

Cool! Let me know if you run into any troubles with it and I'll try and find more ways of expanding SHA256 coverage 😀.

could you add unit tests for

Sure. Will look at that now.

[DomT4]

Added the unit test for nvm_checksum_256.

[ljharb]

All my local testing looks good, but it looks like there's a test failure.

[ljharb]

Still looks like there's a failure - you should be able to do npm run test/fast and run just the fast tests locally.

[DomT4]

Yeah, the failure is strange. If I run the test suite locally I still get the failure, but having checked the SHA256 checksum with 5 different utilities on the failing file the SHA256 comes back the same every time, so there shouldn't be a bad checksum. I'll look into it a bit more and see if I can shake the failure out.

[DomT4]

Still not quite to the bottom of that one failing test. It should be passing, If I run the commands manually I get everything back I expected to, but running the automatic tests results in the failure. Still working on debugging it, I remain hopeful for a positive soon 😸

[DomT4]

Tested on 3 different machines, 3 different OS', and everything works fine aside that one test element. The SHA checksum is the same on every OS (as it should be), so I'm not quite sure what's going on. Will keep working on the test. It'll pass eventually, hopefully.

[ljharb]

Would some debug output be helpful here to indicate which of these many branches is being run on travis? Specifically, which sha command is being used?

[DomT4]

Yeah, might help narrow it down a bit. Theoretically, if Travis is running Linux it should be using sha256sum, unless they've done something unusual with their build boxes.

[DomT4]

I tweaked the test to include the readout of the SHA256 sum, with an added echo $_$NVM_CHECKSUM_256 step in the test, and had an interesting result.

Test:

nvm_checksum_256 tmp/testfile256 "9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08" && echo $_$NVM_CHECKSUM_256 && die "nvm_checksum_256 allowed a bad checksum"

Result:

 ✗ nvm_checksum_256
9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08
nvm_checksum_256 allowed a bad checksum

So the checksum is actually matching the expected checksum, but we're still seeing the failure. Working on getting it to cough out the tool it's using to gain the checksum next.

[ljharb]

@DomT4 Any chance there's leading or trailing whitespace on one of the checksum values in the comparison?

[DomT4]

@ljharb It's possible. I'll play with this some more this evening and see if I can shake some more information on the failure out. I'll also try running the commands to an external file and see if it pumps in any unexpected whitespacing.

[DomT4]

I've kept working on this, but no dice so far. I changed the awk to specifically cut out any trailing whitespace, but no positive (or negative) effect. I also changed the list of utilities it would check for first to try different tools, but no luck on that either. This is strange. If I make it cough out the checksum on iojs downloads I can see the checksum displayed matches the one on the SHA256SUMS.txt file iojs supply, but it still fails the test.

[DomT4]

I've pushed some of the debugging stuff up here so Travis can take a run at it. See if Travis feels any less grumpy than my local machines 😸

[DomT4]

This is what I see in actual usage with the new debug pointers included:

Dominyks-MacBook-Pro ~> nvm install iojs                                                                            [~]
######################################################################## 100.0%
9edd68ff04e7eb3043dc0b0508dcd3fa32c38926315e3221391701f899197e5d
Now using io.js v1.4.3

Which matches the SHA256SUM for Darwin here.

More debug detail:

Dominyks-MacBook-Pro ~> nvm install iojs                                                                            [~]
######################################################################## 100.0%
SHA256 Checksum expected is 9edd68ff04e7eb3043dc0b0508dcd3fa32c38926315e3221391701f899197e5d
SHA256 Checksum received was
9edd68ff04e7eb3043dc0b0508dcd3fa32c38926315e3221391701f899197e5d
Now using io.js v1.4.3

[ljharb]

ah ha, so it looks like there's a leading newline perhaps? It should be trivial to use sed to strip trailing and leading whitespace (awk is fine too).

[ljharb]

@DomT4 Have you had time to make any progress on this PR?

[DomT4]

@ljharb Hey Jordan. Yeah I keep looking at it every now and again. It got stuck behind a mountain of university work which is why the delay has occurred. Hoping to have that delay finally cleared by the start of next week and then I'll have more time free. I'm still not quite sure what the failure is, I've tried every whitespace-stripping method I know, including sed and awk but even with that code in place the failure comes back repeatedly.

I think my next move will be to deliberately set up an extremely temporary fake mirror of Node and untar the tarball, touch a pointless document, retar it and upload it to the mirror, and then switch the download to that mirror. That way I know the checksum should fail, and if it does it should give me a better idea whether the broken code is in verification or testing.

I'm fairly sure the error is coming out of testing, but it's a bit of a head-scratcher as to why or which little element is going awry and causing breakage.

[ljharb]

Thanks for the update, that all makes sense :-)

[ljharb]

@DomT4 ping! any chance you can take another look at this?

[DomT4]

Yikes, I hadn't even realised this has been sitting since the start of April, massively sorry. Will take a peek at it tonight. I think I'm going to tear this back to basics and then work up from a small starting point, rather than trying to make all the possible options work at the same time. Hopefully doing so will help narrow down why the test kept failing, which was the sticking point previously.

[DomT4]

Even in a rebased, extremely stripped down version of this PR we're still hitting identical test failures, hmm.

[DomT4]

I've tried a few times locally over the last couple weeks, but I can't get the tests to pass and I'm kind of at a loss as to why. The checksums matched the expected checksums of downloaded tarballs during verbose script output, but nonetheless the test is dying.

I'm going to close this one out on the basis that I'm unable to resolve the testing failure and that's a fatal holdup on the PR (wholly understandably). I'll keep looking at it locally if I can, but at this point I'm lost and want to leave the door open for other people to work on this if they want to. Having an open PR for an issue tends to discourage others from wanting to work on it, from experience.

Apologies for my failure on making the test work here Jordan. It's a disappointment on my part and generally a sad thing to not be able to contribute to a really useful tool. Thanks for your patience over the last way too many months.

[ljharb]

No worries - please feel free to reopen it if you ever figure it out!

[woodb]

This has been resolved/merged in #981, incorporating much of @DomT4's research into the different sha256 checksum utilities out there.

Apologies, by the way, for all the force push hooks in here (didn't realize it would behave that way).

Cheers 🍻