-
Notifications
You must be signed in to change notification settings - Fork 5
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
[React Native] Virgil Crypto slow performance with large number of messages #43
Comments
Hi @paolospag! I see few places that you can improve in your code:
Also I created rough benchmark that can provide you the general idea of time required for each action.
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! |
Hi @sun1x , 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 |
@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: When these methods are called, the JS fps drops below zero, showing negative values (e.g. |
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 |
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 Also, could you show us the code with time measurements where you get 1600ms?In RoomScreen, when component did mount, the // 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
...
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 |
There is a lot going on in the code between the
you could try this:
I hope this helps |
I tried to recalculate the beginning and the end of the 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. |
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 I believe that the JS thread suffers all the methods of My enviroment:
|
I'll take a look on my side |
If it can help you understand the problem, when I run the app in debug mode and launch |
I also tried to reproduce the @sun1x example by creating a ...
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): Second video (high performance): The |
@vadimavdeev @sun1x do you have any updates to give me? Do you see what I showed in the videos? I looked at the The problem of slow performance could be related? |
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 |
Ok, I await your updates Just to be clear, the same happens on a real iOS device (I tried on an iPhone 7 plus) |
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 |
@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? |
@vadimavdeev I'm not a security expert and I don't know how On React Native, could it affect the little memory available to run I await your suggestions on JavaScript side |
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. |
@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? |
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. |
@paolospag For now, you can use this library to workaround the RNG issue. This will polyfill the 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. |
@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 with |
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. |
@vadimavdeev do you think this "native" package could compromise all messages already encrypted? or compromises the user generated private/public keys? |
Using insecure rng makes both the keys and encrypted data easy to compromise. In Node.js this library uses cryptographically secure random number generator. |
@vadimavdeev ok, thanks for making this clear. |
+1 |
@vadimavdeev do you have any update? do you think something will come out in the next few days? |
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:
I get this error: now I am no longer able to work on the app, I have to disable Hermes… |
@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:
and Emscripten uses this pattern in it's FS implementation |
@paolospag Let me introduce the React Native bridge for native Virgil Crypto - https://github.com/VirgilSecurity/react-native-virgil-crypto |
Closing because this library should not be used with React Native |
@vadimavdeev good job! I'll let you know something when I test it and, if necessary, I'll open an issue in the new repo. |
Sounds good. Thanks |
Hi,
I'm using
virgil-crypto
andvirgil-sdk
in a React Native project, in which I'm building a real-time chat with Firestore throughreact-native-firebase
. I'm trying to reproduceeThree
kit.I noticed that Virgil Crypto is very slow to perform
decryptThenVerify
method in a loop of snapshot docs, specially when I call messages for the first time.I have this situation:
and in my custom
EThree
class I have this situation:What could the performance problem be related to?
The text was updated successfully, but these errors were encountered: