Skip to content
This repository has been archived by the owner on Nov 3, 2021. It is now read-only.

Memory leak when verifying ECDSA signatures #136

Open
go1dfish opened this issue Dec 28, 2018 · 7 comments
Open

Memory leak when verifying ECDSA signatures #136

go1dfish opened this issue Dec 28, 2018 · 7 comments
Assignees

Comments

@go1dfish
Copy link

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.

@rmhrisk
Copy link
Contributor

rmhrisk commented Dec 28, 2018

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

@go1dfish
Copy link
Author

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

@rmhrisk
Copy link
Contributor

rmhrisk commented Dec 28, 2018

That would help.

@go1dfish
Copy link
Author

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);

@rmhrisk
Copy link
Contributor

rmhrisk commented Dec 28, 2018

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

@microshine
Copy link
Contributor

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

@go1dfish
Copy link
Author

go1dfish commented Dec 28, 2018

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 added a commit to go1dfish/gun that referenced this issue Jan 4, 2019
mmalmi pushed a commit to mmalmi/gun that referenced this issue Jan 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants