Test longer SHA inputs

[jldodds]

According to FIPS 180-4 SHA <= 256 should work for messages up to 2⁶⁴ bits long and the rest for messages up to 2¹²⁸. If you are writing a C API for SHA256 then, the signature should be:

void SHA256(unsigned char *in, unsigned char *out, unsigned long long inlen);

You could maybe make a case for representing inlen with an int for 32 bit compilation, but using a short would clearly be wrong. Calling the function (if otherwise correctly implemented)

void SHA256(unsigned char *in, unsigned char *out, unsigned short inlen);

will pass SHAVS, which is part of CAVP. This is because the longest test SHAVS gives is 6400 bytes which fits into the 524288 bytes representable by a short (if it is in bytes, not bits).

This highlights that for block digests and cyphers, it is much more important to measure variable message lengths than variable message contents. It would be hard for CAVP to test much longer messages though, since it can only use input/output pairs written to a file.

By changing the type of the inlen argument to short, you get an implementation that passes SHAVS, but can't correctly hash any message longer than 530KB. Our test suite should aim to test longer messages: perhaps using a streaming interface.

[briansmith]

Check out NSS Bug 610162 - SHA-512 and SHA-384 hashes are incorrect for inputs of 512MB or larger when running under Windows and other 32-bit platforms for an example of a real-world bug that would have been caught with such tests.

[jldodds]

Wow, exactly. Thanks for the link. In the current NSS SHA512 in our repo, the signature for the hash uses a 32 bit unsigned int to represent the number of bytes being hashed. This means the hash will start to be wrong (likely predictably so, if it just overflows, it'll just be the hash of part of the beginning of the file) for inputs a bit over 4gb long. Probably not the end of the world, but it certainly doesn't meet the spec for SHA512.

[briansmith]

In the current NSS SHA512 in our repo, the signature for the hash uses a 32 bit unsigned int to represent the number of bytes being hashed. This means the hash will start to be wrong (likely predictably so, if it just overflows, it'll just be the hash of part of the beginning of the file) for inputs a bit over 4gb long. Probably not the end of the world, but it certainly doesn't meet the spec for SHA512.

It isn't that simple. If you need to support huge inputs then you can't use that function; you have to use the I-U-F interface instead, if you want to support inputs >= 2^32 bytes on a 32-bit system. That is the case with most crypto libraries, because most crypto libraries use size_t for such functions.

Also that function isn't part of the official NSS API, anyway. The NSSLOWHASH_* (sp?) functions, the PKCS#11 implementation, the pk11wrap library, and the HASH_* API in cryptohi.h (I think) are the external API of NSS. In general, the stuff in freebl is considered internal to NSS.

[jldodds]

Thanks for the clarification!

[briansmith]

See also https://github.com/bitwiseshiftleft/sjcl/issues/299 and https://www.dancvrcek.com/software-reliability/.

[kiniry]

Yup, I tweeted about that just prior to seeing your nudge here.