Incorrect HMAC result!

[kirked]

Trello uses the crypto subsystem to provide authentication for webhooks. In some cases an incorrect HMAC is being returned by that subsystem.

In the following snippets, the dash is a 3-byte character:

var crypto = require('crypto');
var hmac = crypto.createHmac('sha1', 'mysecretkey')
hmac.update('{"text":"Accountant I – Corporate Services"}}}https://b985c0c0.ngrok.io/listUpdate').digest('base64')
// ==> 'Zds0ZYuJRAiQnh1utYJjgYW1TLA='

echo -n '{"text":"Accountant I – Corporate Services"}}}https://b985c0c0.ngrok.io/listUpdate' | openssl dgst -sha1 -hmac "mysecretkey" -binary | base64
wnoQmp4wveV73iYetjeaq82WgUY=

• Version: v5.7.0
• Platform: Darwin xxx.local 15.3.0 Darwin Kernel Version 15.3.0: Thu Dec 10 18:40:58 PST 2015; root:xnu-3248.30.4~1/RELEASE_X86_64 x86_64
• Subsystem: crypto

[MylesBorins]

/cc @nodejs/crypto

[mscdex]

@kirked The problem is that the default encoding for strings passed to .update() is not 'utf8', but 'binary' (hash.update() and hmac.update() use the same implementation).

If you are passing utf8 strings, you will need to explicitly pass that encoding as the second argument to update(). I'm not sure why hmac.update() method is missing the encoding parameter in the documentation.

[mscdex]

I've submitted #5500 to fix this doc issue.

[kirked]

Heh, thanks for the quick reply and the clarification. Now to just figure out how to match that with the JDK...

[kirked]

I'm going to cross-post this comment (originally from #5500) just for completeness sake, because I talk about what I perceive as the real problem (incompatibility).

As the originator of #5499, I have to say that the most misleading part of the doc is near the top of the page where it states

The crypto module provides cryptographic functionality that includes a set of wrappers for OpenSSL's hash, HMAC, cipher, decipher, sign and verify functions.

Once I saw that it should be OpenSSL compatible, it was easy to prove that it isn't.

I don't know of a way to arrive at the same digest that node does using OpenSSL when multibyte characters are in the digest stream (which is probably just ignorance on my part). My real problem now is that other systems (e.g., JDK) digest the same as OpenSSL (there is no such thing as binary encoding), so node seems to be the outlier.

[shigeki]

@kirked You can have the same result if you specify an encoding parameter with hamc.update().

var crypto = require('crypto');
var hmac = crypto.createHmac('sha1', 'mysecretkey')
var out = hmac.update('{"text":"Accountant I – Corporate Services"}}}https://b985c0c0.ngrok.io/listUpdate', 'utf8').digest('base64')
console.log(out);

ohtsu@ubuntu:~/tmp/hmac_test$ node test.js
wnoQmp4wveV73iYetjeaq82WgUY=
ohtsu@ubuntu:~/tmp/hmac_test$ echo -n '{"text":"Accountant I – Corporate Services"}}}https://b985c0c0.ngrok.io/listUpdate' | openssl dgst -sha1 -hmac "mysecretkey" -binary | base64
wnoQmp4wveV73iYetjeaq82WgUY=

[kirked]

Thanks, @shigeki, but that doesn't help me using Scala/Akka on the JVM!

All I'm really trying to accomplish is verification of Trello's HMAC so I can process their webhook.

[shigeki]

@kirked Or you can change the string into buffer. Doesn't it help you?

var crypto = require('crypto');
var hmac = crypto.createHmac('sha1', 'mysecretkey');
var buf = new Buffer('{"text":"Accountant I – Corporate Services"}}}https://b985c0c0.ngrok.io/listUpdate');
var out = hmac.update(buf).digest('base64');
console.log(out);

ohtsu@ubuntu:~/tmp/hmac_test$ node test2.js
wnoQmp4wveV73iYetjeaq82WgUY=

[kirked]

Unfortunately that doesn't help either. I'm not using nodejs nor Javascript, but I'm trying to consume/verify an HMAC created with nodejs.

What I am using is the standard JDK crypto library, which hashes the same as OpenSSL, and where there is no such thing as binary character encoding (it's not the same as a plain byte array).

[shigeki]

That's too bad. Node historically is using binary encoding in crypto module for a long time. We no longer change it in order to keep backward compatibilities.

[indutny]

I hope #5504 will help to prevent this kind of stuff in future. It may be too hard breakage as it is right now, but I'm sure we will figure out something useful there.

[mscdex]

@kirked There is no incompatibility, it's just a matter of how the input is interpreted.

This is a bit out of scope for node, but FWIW here is Java code that gets you exactly the same answer with your example input and key:

import javax.crypto.*;
import javax.crypto.spec.*;
import java.util.*;
import java.io.*;
import java.security.*;

public class HelloWorld
{
  public static void main(String[] args) throws UnsupportedEncodingException, NoSuchAlgorithmException, InvalidKeyException
  {
    String key = "mysecretkey";
    String message = "{\"text\":\"Accountant I \u2013 Corporate Services\"}}}https://b985c0c0.ngrok.io/listUpdate";
    SecretKey signingKey = new SecretKeySpec(key.getBytes("UTF-8"), "HMACSHA1");
    Mac mac = Mac.getInstance("HMACSHA1");
    mac.init(signingKey);
    byte[] digest = mac.doFinal(message.getBytes("UTF-8"));
    String encoded64 = Base64.getEncoder().encodeToString(digest);
    System.out.println("digest: " + encoded64);
    // outputs:
    // digest: wnoQmp4wveV73iYetjeaq82WgUY=
  }
}

[kirked]

Thanks @mscdex, I appreciate the help. Your Java code is a transliteration of my Scala code, and if it'd worked I never would've had a signature mismatch.

The actual values I'm getting in the failing source text are 3 bytes: 0xe2, 0x80, 0x93. In your example, message.getBytes("UTF-8") returns those 3 bytes and not 0x2013. (Quite thankfully I've been able to avoid character encoding issues until now 😄)

What I have found that seems to work is the implicit conversion toNodeBinaryEncoding:

  implicit class StringHelper(s: String) {
    def fromBase64(encoding: String = "UTF-8"): Array[Byte] = Base64.getDecoder.decode(s.getBytes(encoding))
    def toNodeBinaryEncoding(): Array[Byte] = (0 until s.length).map { i => (s.codePointAt(i) & 0xff).toByte }.toArray
  }

which allows me to say hmac.doFinal(message.toNodeBinaryEncoding).base64 and get the matching Base64-encoded hash, at least in this example.

[mscdex]

@kirked Yes, unfortunately it does not seem that Java has a built-in 'binary' encoding like node has. I think your proposed solution is the best if you are not in control of the generation of the HMACs.

[kirked]

For posterity, here's a Java snippet that provides the conversion matching the above Scala code:

public static byte[] toNodejsBinaryEncoding(final String s) {
  final int count = s.length();
  final byte[] result = new byte[count];
  for (int i = 0; i < count; i++) result[i] = (byte)(s.codePointAt(i) & 0xff);
  return result;
}

[vkurchatkin]

Ok, closing, this is basically a bug in Trello. binary encoding is kind of lossy, although it's probably not a big deal for hmac.

[kirked]

Thanks to all of the helpful nodejs community members for all of the quick attention!

I agree to close this issue, but I think it's unfair to say it's a bug in Trello. After all, the crypto doc itself states it's a wrapper over OpenSSL functionality, when clearly it's incompatible with OpenSSL, at least without the developer specifying non-binary encoding.

[vkurchatkin]

@kirked agreed, there is an obvious documentation problem.

clearly it's incompatible with OpenSSL

OpenSSL works with byte arrays, not strings. If you pass byte array (Buffer) it will be exactly the same.