MatrixAI / js-encryptedfs

Encrypted Filesystem for TypeScript/JavaScript Applications
https://polykey.com
Apache License 2.0
10 stars 3 forks source link

Cryptor module is not asynchronous #4

Closed MeanMangosteen closed 4 years ago

MeanMangosteen commented 5 years ago

Following issue #18, the cryptor module ustilises node's 'crypto' module for aes ciphering. But the 'crypto' module only offers a sync api.

Cryptor tries to expose an async method by using to process.nextTick() wrapped around sync calls, but this , as discussed in #18, does not actually make it async. It will still block the event loop.

There is this library: https://www.npmjs.com/package/@ronomon/crypto-async

But even it is not truly async. It uses a c++ add on, but on the main thread. So once again, it will block the event loop. It also does not support aes gcm which efs uses.

There is possible scope to develop a new truly async crypto (or at least aes) library utilising node's thread pool like the linked library mentions in 'some new ideas' section.

As for now, since there seems to be know suitable solution for an async crypto lib, efs will continue using the built-in sync crypto library.

CMCDragonkai commented 5 years ago

I experienced this problem with regards to js-polykey. Basically any crypto is generally slow as it involves actual CPU computation, and crypto is not "IO". This is why crypto is generally not async in JS. In order to make our crypto high performance, we will need to integrate multi-threading/multi-processing into our system.

The problem is that every JS runtime has a different multi-threading/multi-processing solution. It's not yet standardised. There is web workers though.

CMCDragonkai commented 5 years ago

Another option is: https://github.com/PeculiarVentures/node-webcrypto-ossl

This aligns our crypto API usage with the WebCrypto API which is supported by browsers. Of course our system cannot work in browsers due to the lack of native filesystem.

While in NativeScript, none of this will work because they don't have a JS native crypto, instead they will have to call native Android crypto or native iOS crypto.

CMCDragonkai commented 5 years ago

There are some cool libraries here: https://github.com/PeculiarVentures

CMCDragonkai commented 5 years ago

This issue is relevant: https://github.com/PeculiarVentures/webcrypto/issues/1

CMCDragonkai commented 5 years ago

I think we should also explore kbpgp's dependencies and the crypto libraries they use there.

CMCDragonkai commented 5 years ago

Here are the dimensions we have to evaluate:

  1. Sync vs Async - We need async in most cases, but a sync API would be nice to be easy to test
  2. Concurrent vs Parallel - Async implies its either concurrent or parallel, we need libraries that can be truly parallel because even if it's concurrent, encryption is real computation and will slow down our platform if it's still single threaded
  3. Platform support - Node, iOS, Android (browser support is unnecessary because native fs doesn't exist there anyway, browser support is nice for other possible uses)
CMCDragonkai commented 5 years ago

Abstract the crypto module so that we can have pluggable parallelism. We can use some sort of "multiprocessing" interface. Maybe something like a common denominator between them all possible platforms. So I suggest something similar to web-workers. So if we create our own "interface" for a web worker object, we could use that to parallelise whatever needs to be done. See typescript for creating an interface.

robert-cronin commented 4 years ago

Web workers look like a viable alternative to the 'process.nextTick' option of making sync functions async. Async from IO purposes is already supplied by lower fs

Async crypto computation could be where the web workers come in. To minimize the communication between main thread and webworker process, all of the fs IO should be done in the webworker context.

Another issue is how to ensure the host machine is not flooded with processes from multiple webworkers running concurrently, one solution is to create a workerpool. Crypto tasks can be dispatched to the worker pool (not separating fs IO which can be run inside the same sync scope as the crypto task)

In this way, we implement the sync functions and then for async we can send them to the workerpool if the user wants to run them async but the default can remain as running sync operations.

Next step is to create a workerpool and write async methods that wrap the sync methods in calls to the pool, something like createTask(<syncFunction>) then runAsync(<task>).

robert-cronin commented 4 years ago

threads.js is using nodejs worker_threads module available after node 12 and tiny-worker for versions 8-11.

threads.js allows one to define a worker object or function (not class) in a file worker.ts and use it on the main thread, lets say in Cryptor.js by spawning a new worker with type object/function defined in worker.js then call that function/object normally as you would using await in an async function.

The issue here is, if we have encryptSync, how do we then send it off to the worker? we can't serialize it or pass in the class context using this so we need to figure out some other way.

One way would be to just keep our async methods and only send off the very essential CPU intensive tasks (i.e. crypto tasks) to the worker. So we have things like _cipher.update(<Buffer>) running in the worker but that wouldn't work because _cipher is in the main thread context.

I think the best way to do it is to let the webworker have everything it needs to create the cipher itself, so this would include algo, key and initVector. Then it can encrypt or decrypt the chunks to be ferried back to the main thread (using Buffers).

This will achieve 'true async' for CPU intensive tasks.

One final consideration is whether the webworker does the file I/O itself. Need to write 2 prototypes and compare.

CMCDragonkai commented 4 years ago

@robert-cronin PR #13 should not be addressing this issue, but a new PR should be created for this.

CMCDragonkai commented 4 years ago

As I mentioned in chat. There could be multiple ways to achieve this. Here are some ideas:

  1. On read - the webworker performs the IO, propagates the async to the crypto decrypt, and then returns the chunk to the main thread. On write, the main thread sends the chunk to the webworker, and the webworker performs the crypto encrypt and then writes to disk. This means both IO and crypto gets executed by the webworker.
  2. The webworker is only used for crypto, and exchanges plaintext and ciphertext with the main thread, and the main thread still performs the underlying IO.

Which we choose depends on complexity of implementation and performance and code elegance. I'm not yet settled on which is the best, which is why I'm asking for some prototypes to test between them.

One major issue with the web worker performing the IO, it means the webworker child process owns the underlying file descriptor. And because we cannot rely on Unix Domain Sockets, this file descriptor cannot be passed back to the main thread. And the main thread cannot pass a file descriptor to the web worker. So you end up having to pass the filename instead and have the web worker perform the open call. But this could be problematic if the main thread logically needs to perform some operation on the file descriptor before or after the IO operation.

Finally, the number of web workers matter as well. I'm thinking of a worker pool that is created that is equal to the number of virtual cores available. But this might be something that is "elective" that is you start with a min pool and gradually grow the pool as necessary, while tasks to be pulled by any worker.

CMCDragonkai commented 4 years ago

Also the IPC between the main thread and the web workers are important as well. Since we are sending chunks. We want to reduce copying operations. Thus if it is possible to perform a zero copy IPC that would be preferable. Barring zero copy, then we would at least want to avoid serialization and deserialization costs, this should be possible given that both parts of the IPC are JavaScript programs, so as long as raw bytes can be passed, then it should be possible to not need any serialization or deserialization.

Also try to avoid using too many extravagant libraries, if we can use the buffer library that would be great since that's what js-virtualfs does. The less libraries, the less weighty our app is as well and the more nimble the final distribution is.

CMCDragonkai commented 4 years ago

Web workers should not be empty husks. They should be constructed with the tools necessary for it to do whatever work it needs.

I'm not sure, but are webworkers fundamentally a fork+exec system? If so, then they are acquiring a COW memory block from the parent process. Can you find out @robert-cronin.

robert-cronin commented 4 years ago

@CMCDragonkai yes, webworkers look like they implement Copy-On-Write so this necessitates the need to provide the webworker with everything it needs to recreate the operations (e.g. the Cryptor would need a copy of the initVector, the key and the pass in order to decrypt/encrypt in the same way as would happen via sync on the main thread)

robert-cronin commented 4 years ago

as for worker pool, threads.js provides a great way to manage pools (with a default of 8 workers) so we can just use it and extend the user the option to specify the number of workers.

robert-cronin commented 4 years ago

so our final (async) architecture looks something like this: