Forge vs Crypto wrong SHA-1 and question

[alexandercerutti]

Following this issue, I'm having the same problem with SHA-1.
In my case, I'm using Forge to calculate the SHA-1 of a file content from its Buffer.
Comparing Crypto to Forge results, I get something like this:

Forge: 5d62b60ba5ee9fadca597a2fcf5c41a68e2765b9
Crypto: 0d5f7f8fec8cf1e45e5bda4542023d394dcc4e6f

And the crypto is the correct one. How can I say that? I'm working with a Node.JS Apple Passes creator and macOS's Console app reports me that Forge sha1 is wrong (and it gives Crypto's one as the right one).
The issue ticket above has been closed with a "resolving this issue is on the roadmap".

Is this issue expected to be solved in short time or approximately how much? And the resolution regards only the SHA-256 or all SHAs?

Also, I have a question. When in your code say "raw" in the md.update method, you mean Buffer or whatever else? Thank you very much.

[davidlehn]

You may be having buffer or encoding issues. Do you have a short test case? It's easier to find problems that way.

Forge should produce the correct results when used properly. Unfortunately there are some buffer and encoding API issues that make it more difficult to use than it should be. Fixing those are on the roadmap but we don't have a timeline for such improvements.

[alexandercerutti]

First of all, thank you for the answer.
Mmh, I'm doing this:

let hashFlowForge = forge.md.sha1.create();
let hashFlow = crypto.createHash("sha1");
fs.createReadStream( **here a valid path of a file** )
  .on("data", function(data) {
	hashFlowForge.update(data);
	hashFlow.update(data);
  })
  .on("end", function() {
	console.log(hashFlowForge.digest().toHex())
	console.log(hashFlow.digest("hex"))
});

And, as I said before, the correct returned result is the Crypto one. Is the above code okay with your request?

[MaxFichtelmann]

forge digests expect a binary string and does not work as expected when using node buffers directly. You need to convert the buffer to a binary string using buffer.toString('binary').

the following program prints the following text, demonstating the issue (when using asciii only, the result is correct in all 3 cases):

💩
e04cf1c8ddda50b75000261742d9b993f8ea6812
40f18f42db66ce65a9929f68c070b6b764049db9
40f18f42db66ce65a9929f68c070b6b764049db9

const forge = require("node-forge");
const crypto = require("crypto");
const fs = require("fs");

let hashFlowForge1 = forge.md.sha1.create();
let hashFlowForge2 = forge.md.sha1.create();
let hashFlow = crypto.createHash("sha1");

fs
  .createReadStream("foo.js")
  .on("data", function(data) {
    hashFlowForge1.update(data);
    hashFlowForge2.update(data.toString("binary"));
    hashFlow.update(data);
  })
  .on("end", function() {
    console.log(hashFlowForge1.digest().toHex());
    console.log(hashFlowForge2.digest().toHex());
    console.log(hashFlow.digest("hex"));
  });

console.log("💩");

[alexandercerutti]

Thank you very much @MaxFichtelmann! It works!
I think that this library needs an API Reference so bad, not just a bunch of examples... I didn't know this thing and for this reason I preferred to ask. "Raw" was not that clear to me (but it might be my problem). :)

[davidlehn]

Yeah, that should work. There are some other ways to get a similar effect. .update(data, 'utf8') might do about the same thing. It's the easy way when just dealing with utf8 strings, but stream buffers might be a bit different? I thought the stream .setEncoding(...) API might work too, but it's not doing what I expected.

We'd love a better docs. Patches are always welcome!

[alexandercerutti]

I would contribute, but I'm completely a noob in the cryptographic field. I might help into creating a "nice formatted way" in which you can put descriptions, if you are interested.

Not for spamming, but I did this documentation for a project of mine, if you are interested.

[davidlehn]

We probably want docs autogenerated from the code comments. If you want to work on such a thing, open an issue and we can discuss. I imagine the doc strings there now need a bunch of cleanup for starters.

Is this issue resolved now?

[alexandercerutti]

Yep this is resolved, thank you :)