node-webcrypto-ossl icon indicating copy to clipboard operation
node-webcrypto-ossl copied to clipboard

Memory leak when verifying ECDSA signatures

Open go1dfish opened this issue 6 years ago • 7 comments
trafficstars

Not sure how to isolate further from here, but I've been tracking down a memory leak in gun and have isolated it to the following line:

https://github.com/amark/gun/blob/master/sea/verify.js#L28

Which is effectively this call:

webcrypto.subtle.verify({ name: 'ECDSA', hash: { name: 'SHA-256' } }, key, sig, new Uint8Array(hash))

If I short circuit the code before this line and pretend validation was successful the leaks stop.

The memory leak seems proportionate to how often this method is called and will continue to grow until node crashes with out of memory errors.

Any help solving this would be appreciated.

Platform is node v10.14.2 on linux 4.9.87.

go1dfish avatar Dec 28 '18 00:12 go1dfish

We will try to look as soon as possible with new years coming up it may take a few days.

rmhrisk avatar Dec 28 '18 00:12 rmhrisk

Sweet, thanks for the fast response, I’ll try to put together an isolated test case tonight.

go1dfish avatar Dec 28 '18 00:12 go1dfish

That would help.

rmhrisk avatar Dec 28 '18 00:12 rmhrisk

I seem to have been slightly off at where the leak is happening.

This causes memory to constantly grow for me:

const WebCrypto = require("node-webcrypto-ossl");
const { subtle } = new WebCrypto({ directory: "ossl" });
setInterval(() => {
  subtle.importKey("jwk", {
    kty: "EC",
    crv: "P-256",
    x: "JG43ynRxqjy1-AemyMUoz14UqKM6cnh7zSPy_EAPgts",
    y: "RCZ5dY7iRaIW_B7cDBlBsDNKwn2QCtbbA1uQ6iL8ENw",
    ext: true
  }, {
    name: "ECDSA",
    namedCurve: "P-256"
  }, false, ["verify"]);
}, 100);

go1dfish avatar Dec 28 '18 05:12 go1dfish

What is the growth like? Each key imported would realistically increase the memory footprint.

rmhrisk avatar Dec 28 '18 06:12 rmhrisk

My test

  • import EC key each 100ms
  • Run NodeJS GC each 10s
const crypto = require("path/to/nodessl.node");

setInterval(() => {
  crypto.Key.importJwk(
    {
      kty: "EC",
      crv: 415,
      x: Buffer.from("JG43ynRxqjy1+AemyMUoz14UqKM6cnh7zSPy/EAPgts=", "base64"),
      y: Buffer.from("RCZ5dY7iRaIW/B7cDBlBsDNKwn2QCtbbA1uQ6iL8ENw=", "base64"),
    },
    crypto.KeyType.PUBLIC,
    (err, key) => {
      if (err) {
        console.error(err);
        throw err;
      } else {
        // console.log("success");
      }
    }
  )
}, 1e2); // 100ms

setInterval(() => {
  global.gc();
}, 1e4); // 10s

Run command node --expose-gc <file.js>

3 minutes

image

10 minutes

image

microshine avatar Dec 28 '18 07:12 microshine

Yeah that's pretty similar growth to what I'm seeing in this test.

In production the impact is about as bad and worse than this contrived test; GUN is doing key import for every signature it verifies; and it verifies signatures individually per node/attribute in the db.

Luckily since the issue is with key import and I'm not dealing with a huge number of keys I can mitigate this with my own key cache (to prevent re-importing same keys) for now; and so far with this approach I'm not seeing the runaway memory growth in production.

Still something to be aware of; would be nice for the module to not grow when re-importing the same key repeatedly if possible, and/or to have some way to unimport a key when done with it.

go1dfish avatar Dec 28 '18 13:12 go1dfish