Generating new key pair is slow

[henryboldi]

Generating a new key pair is quite slow especially when compared to GnuPG. For me, using the example code takes about two minutes to complete vs. around five seconds using GnuPG. Is this the expected performance?

[tanx]

On which browser?

[henryboldi]

Tested in both chrome and node with the same results

On Wed, Apr 27, 2016 at 2:32 AM Tankred Hase notifications@github.com
wrote:

On which browser?

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#449 (comment)

[evilaliv3]

i'm not experiencing particular slowness but i think that what could be for sure optimized is the call to the WrapKeyObject.

@tanx: it appear to me the WrapKeyObject function or part of it is currently being executed in the main thread; i get the browser to lag between it's start and it's end and the lag on a Intel Core i5-4200U is about 3-4 seconds.

[evilaliv3]

@tanx / @bartbutler: i just investigated this issue;

i confirm that all the signing process involved in the generation of a new key is runned in the main javascript thread and causes the browser to stuk on powerful pc and even to cause annoying long running script messages on outdated devices.

This is due to the Signature.sign function that is not implemented nor using the webcrypto (that is not possible given the alghoritms user), nor using the WebWorker (because i'm using a browser that has the webcrypto so that this part is executed outside).

the sign function should be so reimplemented so to ALWAYS benefit of the WebWorker also if the the webCrypto is available;

i've retested what i'm saying emulating to not have the webCrypto (by mocking the getWebCryptoAll()) and using the generateKey and in this way obviously the main thread do not stuck.

\cc @toberndo

[evilaliv3]

i've currently a big headache around this topic.

right now openpgpjs use a model where web workers are used or not depending on the availability of the webcrypto.

but this model does not work; the are conditions like the above where webcrypto is supported but we would have to have to adopt webworker for what is not doable with the webcrypto api.

e.g. return Promise.all([generateSecretKey(), generateSecretSubkey()]).then(wrapKeyObject);
this is the end of the key.generate function, and wrapKeyObject is the inefficient function involving two signatures.

[tanx]

Good point. Starting a worker to offload wrapKeyObject will probably introduce more complexity to the AsyncProxy that delegates operations to the worker. So I don't have an obvious answer here either.

[evilaliv3]

Me neither; all this would require important work but is from my point of view something that we would have to approach sooner than later.

this cpu intensive operations on some browsers/cpu may cause the browser to prompt the user to quit the application, that is worst than having a slow application completely runned inside a worker.

one solution that come to my mind is to run always all inside the webworker, that should be doable in some browsers but obviously not in IE that does not expoe the webcrypto inside a worker.

[tanx]

That sounds like a good solution. Just delegate keygen operations to the worker regardless of WebCrypto support and let the rsa.js module decide (as it already does) if WebCrypto or JS-fallback should be used inside of the worker context.

Re IE: how about just checking for msCrypto and keeping things the way they are for now?

[evilaliv3]

yep! that would be cool actually that would guarantee to:

• always run in webworker when available
• use also webcrypto if available inside the worker
• benefit of only webworker in IE11

basically the code to be changed would be simply to chose to use the webworker only based on the async proxy presence so basically changing code like:

  if (!util.getWebCryptoAll() && asyncProxy) { // use web worker if web crypto apis are not supported
    return asyncProxy.delegate('generateKey', options);
  }

to:

  if (asyncProxy) { // alwaysuse web worker when available
    return asyncProxy.delegate('generateKey', options);
  }

i would suggest to go for this model regardless of the functionality because also for encrypt/decrypt functions we have ascii armor function et similia that are cpu intensive.

sounds like a plan?

if yes i will try to do this pull request.

[evilaliv3]

@tanx: if i'm not wrong this patch should do what i was suggesting.

if you could provide me your hints i will improve that if needed.

@bartbutler / @tanx: does this fits your use cases?

[tanx]

to:
if (asyncProxy) { // alwaysuse web worker when available
return asyncProxy.delegate('generateKey', options);
}
This would cause the worker to always be used, yes. But also on IE. I would suggest a check for just msCrypto instead of getWebCryptoAll() here.

You'd also have to adjust the tests accordingly...

[evilaliv3]

so you are suggesting that in case of mswebcrypto we always discard the worker, for all the other cases instead if the webworker is available we always go for the worker?

something like: if (!window.msCrypto && asyncProxy) ???

what do you mean with updating the tests accordingly?

[tanx]

Basically. Keep in mind that the global object is called self in the worker context. There is no window reference. So perhaps check for typeof msCrypto !== 'undefined' directly or add a util function to cover all cases.

Also keep in mind that we need to update util.getWebCryptoAll to work with self as well. Otherwise WebCrypto support cannot be detected within the worker context. So checking for typeof crypto !== 'undefined' && crypto.subtle directly and skipping the window/self check might be more flexible.

Re tests: things should keep working fine. There are currently integration tests that for all scenarios worker/js-crypto, no-worker/webcrypto, as well as no-worker/js-crypto. We should add the case worker/webcrypto to cover all our bases.

[Meyhem]

I had trouble with the non-worker operations (in my case generateKey), which blocked UI thread for a long time.
If anyone has same trouble as me and doesn't mind trading native crypto APIs for UI reponsiveness
here is workaround which disables native apis and forces Web worker to take the workload:

openpgp.config.use_native = false

What do you think people ? Is this good workaround, which does not compromise any security feature ?

[chesnokovilya]

It should be better now since we use nodeCrypto and webCrypto.