sjcl.misc.hmac Hash option sjcl.hash.sha512 is ignored

[towangfoo]

Using sjcl@1.0.7 The Hash option in sjcl.misc.hmac seems to be ignored, when set to sjcl.hash.sha512

Example code:

// Create sha256 hmac token.
hmac_sha256(message: string, secret: string): string {
    let secretBits = sjcl.codec.utf8String.toBits(secret);
    let mac = new sjcl.misc.hmac(secretBits, sjcl.hash.sha256);
    return sjcl.codec.hex.fromBits(mac.encrypt(message));
 }

// Create sha512 hmac token.
hmac_sha512(message: string, secret: string): string {
    let secretBits = sjcl.codec.utf8String.toBits(secret);
    let mac = new sjcl.misc.hmac(secretBits, sjcl.hash.sha512);
    return sjcl.codec.hex.fromBits(mac.encrypt(message));
}

console.log("sha256", hmac_sha256("data", "key"));
> sha256 – "5031fe3d989c6d1537a013fa6e739da23463fdaec3b70137d828e36ace221bd0"
console.log("sha512", hmac_sha512("data", "key"));
> sha512 – "5031fe3d989c6d1537a013fa6e739da23463fdaec3b70137d828e36ace221bd0"

Expected behavior:

The hashes for the two methods should differ. The sha512-Version should return a sha512 hash.

Actual behavior:

Both methods return the same sha256 hash.

[owlstead]

Sigh, it always amazes me how badly this lib was tested. I've also seen the AAD not being processed. Not being maintained anymore, ignore the library please.

[Nilos]

@owlstead I'm still here and I still merge bug-related PRs. I just don't have a ton of free time and get nothing from improving the code on my own. If you are unhappy with that, either push money my way or create ready-to-merge PRs with good documentation.

[Nilos]

@towangfoo @owlstead I am 90% sure, you are using sjcl incorrectly and forgot to include sjcl.hash.sha512 as it is not included in the default build. This means that it would be undefined and the code would fallback to sjcl.hash.sha256.

[owlstead]

I stand party corrected on that: the library does seem to be semi-maintained. I'm honestly admiring you from trying to maintain it. OTOH, some kind of reaction would be expected in ~3mo time.

Falling back to a less secure algorithm because another one is not available is of course not acceptable. Besides the obvious security ramifications, there is also a high risk of compatibility problems: having two environments where one delivers one result and the other delivers or tries to validate another.

These kind of configuration related issues could easily be missed during unit testing as well. Test on a system with one configuration, deliver on another and you would be in trouble.

Maybe reopen it or create an issue indicating that this kind of fallback should not be possible.