Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create Sodium.runIsolated method to easily run computation heavy cryptographic operations in an isolate #24

Closed
imaNNeo opened this issue Jan 22, 2023 · 12 comments

Comments

@imaNNeo
Copy link

imaNNeo commented Jan 22, 2023

Hi. I want to ask a question.
Do we need to run sodium functions (such as _sodium.crypto.genericHash.call() or _sodium.crypto.secretBox.easy()) behind an isolate? Because I am pretty sure that these jobs (hashing, encrypting, ...) take a long time (on a large amount of data) and might lead to frame drop.

If the answer is yes, can we do that in the library (under the hood)? Because at the moment, functions are defined sync. We can make them async and return a Future to be able to run an isolate.

@Skycoder42
Copy link
Owner

Skycoder42 commented Jan 25, 2023

Generelly you are right, these calls are synchronous, as the dart API is simply a wrapper around the C API. However, for small payloads, these methods are quite fast and it should be OK to run them on the main isolate.

When working with bigger files or the computation-heavy APIs (like pwhash), I would recommend you to indeed move the work to an isolate. However, this is out of scope for this library. You can easily use compute or Isolates directly if you need to do this. There are only 2 things to be aware of:

  1. You cannot pass the Sodium instance via a port. Instead you must initialize sodium once more on the target isolate.
  2. If you want to pass a SecureKey to the isolate, you have to use nativeHandle and SecureKey.fromNativeHandle for that, as the instance itself cannot be sent via a port as well.

@imaNNeo
Copy link
Author

imaNNeo commented Jan 25, 2023

When working with bigger files or the computation-heavy APIs (like pwhash), I would recommend you to indeed move the work to an isolate. However, this is out of scope for this library.

Actually, I disagree that this is out of scope for this library.
Because as I know, a package like http does the API job in a separate isolate under the hood. Because it knows that it is an io job and should not be executed in the main isolate.

And in our case, I think we are pretty sure that encryption/decryption is a computation job and here we should not do this in the main isolate (We can get a benchmark to see how long it takes to run the functions, but milliseconds matter when it comes to 60FPS).
So I think it is okay to run the functions on a separate computation isolate.

@Skycoder42
Copy link
Owner

I do get your point, but there are a few problems with using isolates by default:

  1. For pure dart console apps, this would slow down performance, because of the message handling overhead (especially copying over the memory of large binary data between the isolates)
  2. Passing secret keys between isolates is especially complicated, as the underlying data is never extracted from native (secure) memory. Instead, operations all work on pointers. This would not work out of the box with multiple isolates.
  3. Currently, the library is JS-compatible with an identical API for the dart VM and JS.JS does not support multithreading and thus isolates. It would be possible simply implement the asynchrounous API with synchronous methods in the web, but that would lead to even more confusion, as now an asynchronous method becomes fully blocking
  4. The API would need to be completely rewritten. Also, using Isolates adds complexity to error handling and process management (The isolate needs to be manually terminated).

These are the most significant reasons why I currently do not want to change how the library is implemented. Especially with the new Isolate.run API of dart 2.19, it becomes almost trivial to move computation heavy stuff to an Isolate where needed.

@imaNNeo
Copy link
Author

imaNNeo commented Jan 26, 2023

Thanks for explaining.

  1. I don't know whether typed_data has a heavy overhead. But I think it worth it.
  2. Yes. This is a valid argument. We can extract and pass it to the isolate to create another SecureKey immediately.
  3. It totally makes sense and I have no answer for this.
  4. I was thinking about spawning a long-lived isolate to run our jobs in a queue.
    Yes, I heard about the Isolate.run and btw we need to do the two points that you mentioned:
  1. You cannot pass the Sodium instance via a port. Instead you must initialize sodium once more on the target isolate.
  2. If you want to pass a SecureKey to the isolate, you have to use nativeHandle and SecureKey.fromNativeHandle for that, as the instance itself cannot be sent via a port as well.

BTW I need to get a benchmark to see how long it take (with different length of values). I will send the reports here.

@Skycoder42
Copy link
Owner

Hi. Sorry for the late Reply. Regarding your points:

  1. It does, as data needs to be copied between isolates. In can somewhat be optimized by using special transfer objects, but it still ads additional overhead, proportional to the amount of data beeing en/decrypted.
  2. I would like to avoid that, as this would expose the memory to dart, loosing all the security benefits (like paging resistance and access protection) that the key itself has. Also, as the keys would be passed over the dart ports, they are copied multiple times.
  3. Neither do I
  4. Again, possible, but makes the library more error prone due to higher complexity and requires the user to clean up afterwards - which is not needed right now.

I think using the Isolate.run is a fair approach for cases in which you have huge amounts of data. Maybe a compromise would be a Sodium.runIsolated method that takes care of cloning keys and creating the instance, so that it becomes easier to use. Something like:

final result = sodium.runIsolated([secretKey1, secretKey2], (sodium, keys) {
  final secretKey1 = keys[0];
  final secretKey2 = keys[1];
  
  return sodium.crypto.secretBox.easy(...);
});

Or even with a shortcut for a 1/2 keys

final result = sodium.runIsolated1(secretKey, (sodium, secretKey) => sodium.crypto.secretBox.easy(...));

@imaNNeo
Copy link
Author

imaNNeo commented Feb 14, 2023

Okay, now I understand all details and trade-offs.
So your idea (runIsolated() function) makes sense to me.
We can write a detailed doc about the trade-offs of using this function.
BTW still there is a complexity of having a new reference of Sodium() in each isolate.

@Skycoder42
Copy link
Owner

That is true. However, I think I will spend some time seeing if this can be automated, as theoretically, the factories that create the sodium instances can be passed between isolates. So if the sodium instance "remembers" how it was created, this could be passed to the isolate.

@Skycoder42 Skycoder42 changed the title Call functions behind an isolate Create Sodium.runIsolated method to easily run computation heavy cryptographic operations in an isolate Feb 15, 2023
@tomekit
Copy link

tomekit commented Feb 16, 2023

Speaking of using isolates, what's the right way to use libsodium inside of isolate?

If I try to run: SodiumInit.init() inside of the isolate, it fails with: UI actions are only available on root isolate., because of the: WidgetsFlutterBinding.ensureInitialized() being called.

When you try to pass Sodium through parameters the send() method fails with: Invalid argument(s): Illegal argument in isolate message: (object is a Pointer)

I've noticed that there is some way to pass the pointer down to the isolate: dart-lang/language#333 (comment) but I haven't tried it yet.

@Skycoder42
Copy link
Owner

The library will propably need a little adjustment (i.e. not calling ensureInitialized inside the init method), but you can use the new flutter background isolates: https://medium.com/flutter/introducing-background-isolate-channels-7a299609cad8

For, now, until I have removed the ensureInitialized call, you can just access the platform implementations directly: Code would look something like this:

void _isolateMain(RootIsolateToken rootIsolateToken) async {
  BackgroundIsolateBinaryMessenger.ensureInitialized(rootIsolateToken);

  final sodium = await SodiumPlatform.instance.loadSodium();
}

@tomekit
Copy link

tomekit commented Feb 18, 2023

@Skycoder42 Thanks for that, just tried and works flawlessly.

@Skycoder42
Copy link
Owner

Sodium.runIsolated was added in sodium version 2.1.0

@imaNNeo
Copy link
Author

imaNNeo commented Mar 9, 2023

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants