[React Native] Virgil Crypto slow performance with large number of messages about virgil-crypto-javascript HOT 34 CLOSED

Hi @paolospag!

I see few places that you can improve in your code:

• First, you don't need to import the same public key in your loop:

  this._eThree.parsePublicKey(this.publicKey) // this method is called on each iteration

• Second, you don't need to load key multiple times for the same identity and extract public key:

  // in your decrypt method
  const privateKey = await this._keyLoader.loadLocalPrivateKey();
  if (!privateKey) throw new Error('This identity is not registered');
  if (!publicKey) publicKey = this.virgilCrypto.extractPublicKey(privateKey);

• Make sure that you query paginated data in your app. This way you will need to decrypt fixed amount of messages (for example 30) instead of the whole dialog.

Also I created rough benchmark that can provide you the general idea of time required for each action.
Results(time in ms):

signThenEncrypt 100 messages: 692.1588029991835
decryptThenVerify 100 messages: 712.652220999822
importPublicKey 100 times: 226.15956099890172
extractPublicKey 100 times: 215.84223799966276

Note that results in React Native may vary because I ran the benchmark on MBP 13" 2015 - Node.js v8.16.0

Source code:

const { VirgilCrypto } = require('virgil-crypto');
const { performance } = require('perf_hooks');

const TOTAL_MESSAGES = 100;
const virgilCrypto = new VirgilCrypto();
const keyPair = virgilCrypto.generateKeys();

const messages = [];
for (let i = 0; i < TOTAL_MESSAGES; i += 1) {
  messages.push(Buffer.from('Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.', 'utf8'));
}

const signThenEncryptStart = performance.now();
const encryptedMessages = messages.map(message =>
  virgilCrypto.signThenEncrypt(message, keyPair.privateKey, keyPair.publicKey),
);
const signThenEncryptEnd = performance.now();
console.log(`signThenEncrypt ${TOTAL_MESSAGES} messages:`, signThenEncryptEnd - signThenEncryptStart);

const decryptThenVerifyStart = performance.now();
const decryptedMessages = encryptedMessages.map(encryptedMessage =>
  virgilCrypto.decryptThenVerify(encryptedMessage, keyPair.privateKey, keyPair.publicKey),
);
const decryptThenVerifyEnd = performance.now();
console.log(`decryptThenVerify ${TOTAL_MESSAGES} messages:`, decryptThenVerifyEnd - decryptThenVerifyStart);

const exportedPublicKey = virgilCrypto.exportPublicKey(keyPair.publicKey);
const importPublicKeyStart = performance.now();
for (let i = 0; i < TOTAL_MESSAGES; i += 1) {
  virgilCrypto.importPublicKey(exportedPublicKey);
}
const importPublicKeyEnd = performance.now();
console.log(`importPublicKey ${TOTAL_MESSAGES} times:`, importPublicKeyEnd - importPublicKeyStart);

const extractPublicKeyStart = performance.now();
for (let i = 0; i < TOTAL_MESSAGES; i += 1) {
  virgilCrypto.extractPublicKey(keyPair.privateKey);
}
const extractPublicKeyEnd = performance.now();
console.log(`extractPublicKey ${TOTAL_MESSAGES} times:`, extractPublicKeyEnd - extractPublicKeyStart);

Hope it will help!

from virgil-crypto-javascript.

@paolospag Let me introduce the React Native bridge for native Virgil Crypto - https://github.com/VirgilSecurity/react-native-virgil-crypto

from virgil-crypto-javascript.

When will version 4 be released? Can I already try it somehow?

I'm afraid I was wrong. Version 4 will not run on React Native due to lack of cryptographically secure random number generator there. I turns out that current version (v2) uses insecure random number generator in React Native, so having a separate package with native modules for React Native is the only way to go. We will start building it asap. I'll ping you when we have something to look at.

Really there is no way to optimize asm.js for React Native?

It was a guess that asm.js is not optimized. Maybe it is, I haven't been able to verify that. Anyway, even if it isn't, I don't think there's something that can be done in the user space to have it optimized by the JS engine.

On React Native, could it affect the little memory available...

No, I don't think the amount of memory has any effect on performance. It would have thrown an error if there wasn't sufficient memory.

from virgil-crypto-javascript.

I can't tell how long it will take right now. I guess it's on the order of a couple of days to a week.

The The lack of cryptographically secure random number generator makes the crypto system predictable and thus vulnerable to a wide range of attacks.

from virgil-crypto-javascript.

Unfortunately there is no such polyfill that I am aware of. I started working on the package with native module bindings for react native. Hopefully, it will be ready early next week.

from virgil-crypto-javascript.

Using insecure rng makes both the keys and encrypted data easy to compromise. In Node.js this library uses cryptographically secure random number generator.

from virgil-crypto-javascript.

+1

from virgil-crypto-javascript.

@paolospag Writing react native bridge for native Virgil Crypto libraries is taking a bit longer that I anticipated, but something will come out in the next few days.

As for the Hermes issue, apparently the problem is that Hermes does not allow custom Error implementations like this:

function MyError(msg) {
  this.message = msg;
  if (this.stack) { // this line throws
    console.log('never printed');
  }
}
MyError.prototype = new Error();
MyError.prototype.constructor = MyError;
const myError = new MyError('crash_app');

and Emscripten uses this pattern in it's FS implementation
which is emitted in the asm.js code of virgil-crypto. Unfortunately, I don't see a workaround for this issue as it seems to be a problem with Hermes itself.

from virgil-crypto-javascript.

Hi @sun1x ,
I will try to follow your suggestions and I hope to improve the performances!

I will keep you updated.

I still want to point out that this:

const privateKey = await this._keyLoader.loadLocalPrivateKey();
if (!privateKey) throw new Error('This identity is not registered');
if (!publicKey) publicKey = this.virgilCrypto.extractPublicKey(privateKey);

is the same in e3kit-js

from virgil-crypto-javascript.

@sun1x I tried to optimize the code, then I'm loading the private key and extracting the public key one time in loop... but, at least in React Native, for just 4 encrypted messages it takes ~1600ms

The React Native's JS Thread seems to suffer these methods:
virgilCrypto.signThenEncrypt
virgilCrypto.decryptThenVerify

When these methods are called, the JS fps drops below zero, showing negative values (e.g. -2.1 fps)

from virgil-crypto-javascript.

1600ms for 4 messages is indeed very slow. How big are the messages? How many users are the messages encrypted for? Do you use the default type of keys (i.e. call new VirgilCrypto and generateKeys() without parameters)?
It would be helpful to know more about your environment. Where do you run your code, in simulator or on a real device and if so, which one? Do you use Expo client?
Also, could you show us the code with time measurements where you get 1600ms?

from virgil-crypto-javascript.

I try to answer all the questions, I hope they are exhaustive

How big are the messages?

Decrypted messages are really short (max 100 characters) because they are test messages. That's why I'm worried.

How many users are the messages encrypted for?

Just two users, one is the authenticated user and one is other user from which I get the public key

let _publicKey = msg.doc.data().uid !== this._currUser.uid ? this.publicKey : null; // <-- this.publicKey contains other user public key
let decryptedText = await this._eThree.decrypt(msg.doc.data().text, _publicKey);

Do you use the default type of keys (i.e. call new VirgilCrypto and generateKeys() without parameters)?

Yes, I created a Firebase Cloud Function to generate keys-pair and protect private key with password in this way:

const virgil = await require('virgil-crypto');
const virgilCrypto = new virgil.VirgilCrypto();
const keyPair = virgilCrypto.generateKeys();
const publicKeyData = virgilCrypto.exportPublicKey(keyPair.publicKey);
const privateKeyData = virgilCrypto.exportPrivateKey(keyPair.privateKey, keyPsw); // <-- keyPsw is a user password
const payload =  {
  identity: uid, // <-- user identity from firebase
  publicKey: publicKeyData.toString('base64'),
  privateKey: privateKeyData.toString('base64')
};

Where do you run your code, in simulator or on a real device and if so, which one?

Both simulators and real devices and both iOS and Android environment. The problem is much more evident in real devices (Huawei Honor 8 and iPhone 7 Plus). In particular, the performances of JS fps have been tested on Honor 8 (Android).

Do you use Expo client?

No, I'm working on a pure react-native project.

Also, could you show us the code with time measurements where you get 1600ms?

In RoomScreen, when component did mount, the _bootstrapChatRoom method is called. I measured exactly that. In the chat room I've tested there are only 4 messages.

// RoomScreen
...
import PerformanceNow from 'performance-now';
...
  _bootstrapChatRoom = (limit = 10) => {
    const { roomData } = this.state;
    this.chatRoomRef = database.chatRoomMessagesByPath(roomData.path);
    this.unsubscribe = this.chatRoomRef.orderBy('createdAt', 'desc').limit(limit).onSnapshot(this._chatRoomListener);
  }
  _chatRoomListener = async (querySnapshot) => {
    let perfStart = PerformanceNow(); // <-- PERFORMANCE START
    const messages = await Promise.all(
      querySnapshot.docChanges
        .filter(msg => msg.type === 'added')
        .map(async msg => {
        let _publicKey = msg.doc.data().uid !== this._currUser.uid ? this.publicKey : null;
        let decryptedText = await this._eThree.decrypt(msg.doc.data().text, _publicKey);
        let loadedMessage = { ...msg.doc.data(), text: decryptedText};
        return createGiftedChatItem(msg.doc.id, loadedMessage);
      })
    );
    if (messages.length === 0) return;
    this.setState(prevState => {
      let prevMessages = prevState.messages.filter(m => !m.isOutgoing);
      let roomCount = messages.length === 1
        ? [...prevMessages, messages[0]].length
        : querySnapshot.size
      return {
        ...prevState,
        messages: messages.length === 1
          ? GiftedChat.append(prevMessages, messages[0])
          : messages,
        roomCount: roomCount
      }
    }, () => {
      let perfEnd = PerformanceNow(); // <-- PERFORMANCE END
      alert( (perfEnd - perfStart).toFixed(3)+'ms' ); // <-- THIS RETURNS ~1800ms, BUT ~200ms ARE COMPUTABLE TO FIRESTORE
    });
  }
...

This is my custom implementation of EThree class. I tried to store private key and the extracted public key of current authenticated user respectively in this.privateKey and this.publicKey to avoid to load key multiple times for the same identity, but honestly I don't know if I did well

// EThree
...
  constructor(identity, options) {
    ...
    this.privateKey = null;
    this.publicKey = null;
    ...
  }
...
  async encrypt(message, publicKeys) {
    let isMessageString = isString(message);

    let privateKey = this.privateKey;
    if (!privateKey) {
      this.privateKey = await this._keyLoader.loadLocalPrivateKey();
      privateKey = this.privateKey;
    }
    if (!privateKey) throw new Error('This identity is not registered');

    if (publicKeys == null) argument = [];
    else if (isVirgilPublicKey(publicKeys)) argument = [publicKeys];
    else argument = getObjectValues(publicKeys);

    const publicKeysArray = this._addOwnPublicKey(privateKey, publicKeys);

    const res = this.virgilCrypto.signThenEncrypt(message, privateKey, publicKeysArray);
    if (isMessageString) return res.toString('base64');
    return res;
  }
  async decrypt(message, publicKey) {
    const isMessageString = isString(message);

    let privateKey = this.privateKey;
    if (!privateKey) {
      this.privateKey = await this._keyLoader.loadLocalPrivateKey();
      privateKey = this.privateKey;
    }
    if (!privateKey) throw new Error('This identity is not registered');

    if (!this.publicKey) this.publicKey = this.virgilCrypto.extractPublicKey(privateKey);
    if (!publicKey) publicKey = this.publicKey;

    const res = this.virgilCrypto.decryptThenVerify(message, privateKey, publicKey)
    if (isMessageString) return res.toString('utf8');
    return res;
  }
...
  _addOwnPublicKey(privateKey, publicKeys) {
    let argument;
    if (publicKeys == null) argument = [];
    else if (isVirgilPublicKey(publicKeys)) argument = [publicKeys];
    else argument = getObjectValues(publicKeys);
    let ownPublicKey = this.publicKey;
    if (!ownPublicKey) {
      this.publicKey = this.virgilCrypto.extractPublicKey(privateKey);
      ownPublicKey = this.publicKey;
    }
    if (!this._isOwnPublicKeysIncluded(ownPublicKey, argument)) {
      argument.push(ownPublicKey);
    }
    return argument;
  }
...

I'm working in a debug environment and probably the measurement suffers too, but I also tried to run react-native run-android --variant=release and the performance have not improved much.

from virgil-crypto-javascript.

There is a lot going on in the code between the PerformanceNow() calls to say that decryptThenVerify method is to blame for performance degradation. You are measuring the time it takes to decrypt, transform and render the messages. The first time the code is executed it will load the private key from storage and extract the public key even before the decryption. I suggest that you rewrite your code so that the private key is already in memory at the time the messages are loaded. Also, instead of decrypting all of the messages in parallel and then rendering them, you may get a better perceived performance by decrypting and rendering messages one by one, i.e. roughly, instead of this:

const messages = await Promise.all(
  messages.map(async msg => {
    const decrypted = await decrypt(msg);
    return createGiftedChatItem(decrypted);
  })
);
this.setState({ messages });

you could try this:

messages.map(async msg => {
  const decrypted = await decrypt(msg);
  const chatItem = createGiftedChatItem(decrypted);
  this.setState(prevState => {
    return { messages: [...prevState.messages, chatItem] };
  });
})

I hope this helps

from virgil-crypto-javascript.

I tried to recalculate the beginning and the end of the PerformanceNow in this way:

    let perfStart = PerformanceNow(); // <-- PERFORMANCE START
    const messages = await Promise.all(
      querySnapshot.docChanges
        .filter(msg => msg.type === 'added')
        .map(async msg => {
        let _publicKey = msg.doc.data().uid !== this._currUser.uid ? this.publicKey : null;
        let isDeleted = typeof msg.doc.data().text === 'boolean' && !msg.doc.data().text;
        let decryptedText = !isDeleted 
          ? await this._eThree.decrypt(msg.doc.data().text, _publicKey) 
          : i18n.t('chat.messages.deleted_message');
        let loadedMessage = { ...msg.doc.data(), text: decryptedText, deleted: isDeleted };
        return createGiftedChatItem(msg.doc.id, loadedMessage);
      })
    );
    let perfEnd = PerformanceNow(); // <-- PERFORMANCE END
    alert( (perfEnd - perfStart).toFixed(3)+'ms' ); // <-- LOW PERFORMANCE

but the performances remain more or less the same.

I'll try to rewrite the code as you suggest and I'll let you know as soon as possible.

from virgil-crypto-javascript.

I rewrote the code following your suggestions, but the performances remain low on real devices and with the release configuration (no debug mode or simulator).

I have also tried to decrypt and render messages one by one, but it takes ~300-400ms to decrypt single message and the messages appear one by one on the screen at a distance of this time, creating a bad experience.

I also created this method:

  async cacheKeysPair() {
    let privateKey = await this._keyLoader.loadLocalPrivateKey();
    let publicKey = this.virgilCrypto.extractPublicKey(privateKey);
    return this.cachedKeys = { privateKey, publicKey };
  }

to store both privateKey and publicKey in the EThree instance, but I didn't improve the situation much.

I believe that the JS thread suffers all the methods of virgil-crypto library.

My enviroment:

[email protected]
[email protected]
[email protected]
[email protected]  

from virgil-crypto-javascript.

I'll take a look on my side

from virgil-crypto-javascript.

If it can help you understand the problem, when I run the app in debug mode and launch Debug JS remotely with Chrome Developer Tools, the performance (strangely) increases. Probably because it runs the Chrome JS engine and not the React Native one

from virgil-crypto-javascript.

I also tried to reproduce the @sun1x example by creating a TestScreen component and I created two videos, in the second I launch Debug JS remotely and the performance improves sharply.

...
import { VirgilCrypto } from 'virgil-crypto';
import PerformanceNow from 'performance-now';

class TestScreen extends React.Component {
  componentDidMount = () => {
    const TOTAL_MESSAGES = 100;
    const virgilCrypto = new VirgilCrypto();
    const keyPair = virgilCrypto.generateKeys();

    const messages = [];
    for (let i = 0; i < TOTAL_MESSAGES; i += 1) {
      messages.push(Buffer.from('Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.', 'utf8'));
    }

    const signThenEncryptStart = PerformanceNow();
    const encryptedMessages = messages.map(message =>
      virgilCrypto.signThenEncrypt(message, keyPair.privateKey, keyPair.publicKey),
    );
    const signThenEncryptEnd = PerformanceNow();
    alert(`signThenEncrypt ${TOTAL_MESSAGES} messages: ${signThenEncryptEnd - signThenEncryptStart}ms`);

    const decryptThenVerifyStart = PerformanceNow();
    const decryptedMessages = encryptedMessages.map(encryptedMessage =>
      virgilCrypto.decryptThenVerify(encryptedMessage, keyPair.privateKey, keyPair.publicKey),
    );
    const decryptThenVerifyEnd = PerformanceNow();
    alert(`decryptThenVerify ${TOTAL_MESSAGES} messages: ${decryptThenVerifyEnd - decryptThenVerifyStart}ms`);

    const exportedPublicKey = virgilCrypto.exportPublicKey(keyPair.publicKey);
    const importPublicKeyStart = PerformanceNow();
    for (let i = 0; i < TOTAL_MESSAGES; i += 1) {
      virgilCrypto.importPublicKey(exportedPublicKey);
    }
    const importPublicKeyEnd = PerformanceNow();
    alert(`importPublicKey ${TOTAL_MESSAGES} times: ${importPublicKeyEnd - importPublicKeyStart}ms`);

    const extractPublicKeyStart = PerformanceNow();
    for (let i = 0; i < TOTAL_MESSAGES; i += 1) {
      virgilCrypto.extractPublicKey(keyPair.privateKey);
    }
    const extractPublicKeyEnd = PerformanceNow();
    alert(`extractPublicKey ${TOTAL_MESSAGES} times: ${extractPublicKeyEnd - extractPublicKeyStart}ms`);
  }
  render() {
    return (
      <View>
        <Text>Test screen for VirgilCrypto</Text>
      </View>
    )
  }
}

First video (low performance):
https://streamable.com/ioa6j

Second video (high performance):
https://streamable.com/w5vis

The TestScreen is activated clicking on the "Chat" tab at the bottom. In the first video, from the moment of the click to the first alert (ie signThenEncrypt method) pass ~43 seconds while in the second video the same method takes ~ 2 seconds

from virgil-crypto-javascript.

@vadimavdeev @sun1x do you have any updates to give me?

Do you see what I showed in the videos?

I looked at the v4 branch of this repo, and noticed that for React Native the approach will be different due to WebAssembly.

The problem of slow performance could be related?

from virgil-crypto-javascript.

I don't have any updates for you yet. I managed to reproduce the problem on Android and I'm still investigating on what's causing it.

React Native does not support WebAssembly. the asm.js fallback will be used in v4

from virgil-crypto-javascript.

Ok, I await your updates

Just to be clear, the same happens on a real iOS device (I tried on an iPhone 7 plus)

from virgil-crypto-javascript.

Unfortunately, after running some benchmarks I've found that most of the time is spent in the generated asm.js code, which means there is nothing we can do on the wrapper (i.e. this library) side. My guess is that React Native does not optimize for asm.js, but I haven't been able to verify that. Hopefully the version 4 of this library that is about to be released will not have this problem as the underlying native library and code generation for JS have been rewritten with performance in mind. We will get back to this issue when the new version is out.

For now, to get the maximum performance you can create native modules for Android and \ or iOS that use Virgil Crypto library for Android and iOS respectively. We might consider creating a separate package that does this later on if there is a demand and \ or performance of the v4 library is not satisfactory

from virgil-crypto-javascript.

@vadimavdeev probably, the solution to create a package with native modules would be the best solution.

Unfortunately, I'm not an expert of Android and iOS and I don't have time to create native modules.

When will version 4 be released? Can I already try it somehow?

Really there is no way to optimize asm.js for React Native?

from virgil-crypto-javascript.

@vadimavdeev I'm not a security expert and I don't know how virgil-crypto works, so what I'm doing must be taken with caution.

On React Native, could it affect the little memory available to run signThenEncrypt / decryptThenVerify methods? I found the following code, but I don't know if it's the correct intuition:
https://github.com/VirgilSecurity/virgil-crypto-javascript/blob/master/src/lib/browser.ts#L3-L11

I await your suggestions on JavaScript side

from virgil-crypto-javascript.

@vadimavdeev this is not good news at all, I have to finish a project and I don't have much time. Could you estimate the development time of the separate package with native modules?

The lack of cryptographically secure random number generator compromises production apps?

from virgil-crypto-javascript.

@paolospag For now, you can use this library to workaround the RNG issue. This will polyfill the crypto.getRandomValues function that exists in browsers and is cryptographically secure with a native implementation for each platform (iOS and Android). You just need to install and link this module and then import 'react-native-get-random-values' in your index.js file (as the per instructions in the repo)

One caveat is that it will slow down your code even further because the random values will have to cross the bridge from native side to the JS side.

from virgil-crypto-javascript.

@vadimavdeev I need something to help me improve performance, not slow it down.

Is there any polyfill that can optimize asm.js to be combined withreact-native-get-random-values?

from virgil-crypto-javascript.

@vadimavdeev do you think this "native" package could compromise all messages already encrypted? or compromises the user generated private/public keys?
Consider that the keys are generated with a Cloud Function (Node v8) at the time of user registration.

from virgil-crypto-javascript.

@vadimavdeev ok, thanks for making this clear.
I await your updates

from virgil-crypto-javascript.

@vadimavdeev do you have any update? do you think something will come out in the next few days?

from virgil-crypto-javascript.

In the meantime, I updated my app React Native from 0.59.10 to 0.60.4, I activated Hermes for Android and with this configuration:

[email protected]
[email protected]
[email protected]

I get this error:

now I am no longer able to work on the app, I have to disable Hermes…
All this is frustrating!

from virgil-crypto-javascript.

Closing because this library should not be used with React Native

from virgil-crypto-javascript.

@vadimavdeev good job!
Unfortunately, I won't be able to test it these days, I'll try it in two weeks.

I'll let you know something when I test it and, if necessary, I'll open an issue in the new repo.

from virgil-crypto-javascript.

Sounds good. Thanks

from virgil-crypto-javascript.